Re: [virtio-comment] [PATCH V2 2/2] virtio: introduce STOP status bit

[Jason Wang <jasowang@redhat.com>]

å 2021/7/14 äå2:20, Cornelia Huck åé:
> On Wed, Jul 14 2021, Jason Wang <jasowang@redhat.com> wrote:
>
> > å 2021/7/13 äå8:28, Cornelia Huck åé:
> > > On Tue, Jul 13 2021, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > > å 2021/7/13 äå7:31, Cornelia Huck åé:
> > > > > On Tue, Jul 13 2021, Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > > > å 2021/7/13 äå4:19, Cornelia Huck åé:
> > > > > > > On Tue, Jul 13 2021, Jason Wang <jasowang@redhat.com> wrote:
> > > > > > >
> > > > > > > > å 2021/7/12 äå5:57, Stefan Hajnoczi åé:
> > > > > > > > > When migrating a guest with many VIRTIO devices a busy waiting approach
> > > > > > > > > extends downtime if implemented sequentially (stopping one device at a
> > > > > > > > > time).
> > > > > > > > Well. You need some kinds of waiting for sure, the device/DMA needs
> > > > > > > > sometime to be stopped. The downtime is determined by a specific virtio
> > > > > > > > implementation which is hard to be restricted at the spec level. We can
> > > > > > > > clarify that the device must set the STOP bit in e.g 100ms.
> > > > > > > I don't think we can introduce arbitrary upper bounds here. At most, we
> > > > > > > can say that the device SHOULD try to set the STOP bit as early as
> > > > > > > possible (and make use of the mechanism to expose in-flight buffers.)
> > > > > > Yes, that's my understanding.
> > > > > >
> > > > > >
> > > > > > > If we want to avoid polling for the STOP bit, we need some kind of
> > > > > > > notification mechanism, I guess. For ccw, I'd just use a channel
> > > > > > > command to stop the device; completion of that channel program would
> > > > > > > indicate that the device is done with the stop procedure.
> > > > > > A question, is interrupt used for such notification, or the VMM can
> > > > > > choose to poll for the completion?
> > > > > You can poll for the subchannel to become status pending.
> > > > >
> > > > > > > Not sure how
> > > > > > > well that translates to other transports.
> > > > > > Actually, it's not necessarily a busy polling. VMM can schedule other
> > > > > > process in and recheck the bit periodically.
> > > > > >
> > > > > > Or as you mentioned before, we can use some kind of interrupt but it
> > > > > > would be more complicated than the simple status bit. It's better to
> > > > > > introduce the interrupt only if the status bit doesn't fit.
> > > > > At least for ccw, waiting for the status bit to be set also involves an
> > > > > interrupt or polling (we use another channel program to retrieve the
> > > > > status.) A dedicated channel command would actually be better, as the
> > > > > interrupt/status pending would already inform us of success.
> > > > So it looks to me it doesn't conflict with this design: the device must
> > > > wait for the device to be stopped to signal the success of the ccw command?
> > > Yes, the difference is mainly how that information can be extracted.
> >
> > So I had a look at how reset is described for ccw:
> >
> > "
> >
> > In order to reset a device, a driver sends the CCW_CMD_VDEV_RESET command.
> >
> > "
> >
> > This implies something similar, that is the success of the command means
> > the success of the reset.
> Yes, indeed.
>
> > I wonder maybe I can remove the "re-read" from the basic facility and
> > let the transport to decide what to do.
> >
> > - for PCI, if a registers is used, we need re-read
> > - for CCW, follow the current implication, re-read is not needed and we
> > can wait/poll for the success of the ccw command
> If we are going with a status bit, it would be the same as for pci (we
> have WRITE_STATUS and READ_STATUS commands.)


So spec is unclear of the implications of the success of a command:

E.g for RESET (CCW_CMD_VDEV_RESET), the success of the command implies 
the success or the reset.

But for set_status (CCW_CMD_WRITE_STATUS), the success of the command 
does not imply the bit is set by the device.

If I understand this correctly, we still need re-read here.


>   If we are going with a
> distinct command, we can skip the re-read.


Then it would be better to introduce the STOP as a dedicated device 
facility (as reset):

The device MUST present STOP bit after it has been stopped.

And for PCI:

- it was set via set the bit in the registers

for ccw:

- a distinct command (as reset) is introduced, and STOP is forbidden to 
set via device status?


> (I'd probably go with a more
> generic 'trigger an action' meta-command, but that would work just the
> same.)
>
> > - for admin virtqueue, it should be something similar to ccw, wait/poll
> > for the success of the admin virtqueue command
> Or we should maybe standardize on the admin virtqueue?


That's one way to go.


>   That seems less
> confusing to me.


But it's just one of the possible interface to carry the commands. We 
still need to define the semantic or facility of "stop" first.

And we still need to clarify the implication for the success of each 
specific command as ccw. (E.g whether or not a re-read(get after set) is 
need)

The only difference is the transport: ccw command vs virtqqueue.

Thanks


>