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

[Cornelia Huck <cohuck@redhat.com>]

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.) If we are going with a
distinct command, we can skip the re-read. (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 seems less
confusing to me.