Re: [PATCH v3 0/2] virtio: introduce STOP status bit

[Jason Wang <jasowang@redhat.com>]

On Thu, Nov 25, 2021 at 3:25 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Thu, Nov 25, 2021 at 4:05 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> > 在 2021/11/24 上午12:19, Eugenio Perez Martin 写道:
> > > On Tue, Nov 23, 2021 at 12:33 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >> On Thu, Nov 18, 2021 at 05:49:11PM +0100, Eugenio Perez Martin wrote:
> > >>> On Thu, Nov 18, 2021 at 3:45 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >>>> On Thu, Nov 11, 2021 at 07:58:10PM +0100, Eugenio Pérez wrote:
> > >>>>> This patch introduces a new status bit STOP. This can be used by the
> > >>>>> driver to stop the device in order to safely fetch used descriptors
> > >>>>> status, making sure the device will not fetch new available ones.
> > >>>>>
> > >>>>> Its main use case is live migration, although it has other orthogonal
> > >>>>> use cases. It can be used to safely discard requests that have not been
> > >>>>> used: in other words, to rewind available descriptors.
> > >>>> This sounds non-trivial and would require more explanation.
> > >>>>
> > >>> You are right and it's not well explained here, I will try to develop
> > >>> it better for the next version. It's in the cover letter explaining
> > >>> one use case proposed by MST [1] and the answer by Jason [2].
> > >>>
> > >>> When the VQ is stopped, it is forced to flush all the descriptors (in
> > >>> this version) and the used index in the case of split. With that
> > >>> information, the driver can modify any available descriptor that has
> > >>> not been used by the device, and to rewind the virtqueue available
> > >>> index to an extent.
> > >>>
> > >>> If we add Jason's virtqueue state (first patch of previous version),
> > >>> which I need to recover in later versions, the driver can move freely
> > >>> available and used index prior to queue resetting.
> > >>>
> > >>> The (in comments) proposed solution for the device that cannot flush
> > >>> its descriptor timely is to provide it with a way to report in-flight
> > >>> descriptors. As a reference, vhost-user has done it before, but with a
> > >>> memory region shared by a file descriptor [3]. If we add something
> > >>> similar, the driver still knows what file descriptors is able to
> > >>> rewind.
> > >>>
> > >>> Does that explanation make the driver rewind use case more clear?
> > >> I understand the use case but it's not clear how the mechanism is
> > >> supposed to work. Let's continue discussing in the sub-thread where I
> > >> posted details.
> > >>
> > > Ok!
> > >
> > >>>>> Stopping the device in the live migration context is done by per-device
> > >>>>> operations in vhost backends, but the introduction of STOP as a basic
> > >>>>> virtio facility comes with advantages:
> > >>>>> * All the device virtio-specific state is summarized in a single entity,
> > >>>>>    making easier to reason about it.
> > >>>> This point isn't clear to me. I think it's saying that using STOP
> > >>>> somehow unifies things compared to the way that vhost devices are
> > >>>> stopped today. Given that vhost already syncs state back to the VMM's
> > >>>> emulated VIRTIO device, I'm not sure how STOP is different.
> > >>>>
> > >>> It also achieves that, but it's more related to the fact that the
> > >>> current way of getting the index through vhost net, user, ... is not
> > >>> reusable by others methods to expose the device to VMM.
> > >> ->vhost_get_vring_base() is a common interface across vhost-kernel,
> > >> vhost-user, etc and all VIRTIO Device Types. Is there a problem with it
> > >> that I'm missing?
> > >>
> > > It is usable by all VIRTIO Device types *exposed through vhost*, so it
> > > fails to address the case when we cannot use vhost to expose the
> > > device. It can happen with the cases you explained better than me in
> > > [1], or when exposing a vdpa device as a pure virtio device using
> > > virtio_vdpa.
> >
> >
> > I don't get this and may miss something. The stop is a basic facility
> > that is required for the building block of live migration. We need that
> > regardless what kind of software technology or framework is used.
> >
>
> Yes, those are just examples using vdpa, to expose the actual
> limitation that vhost protocol has. But there can be other examples
> with other frameworks for sure.
>
> And they are valid for both getting vq index and stop.
>
> > For the case of virtio_vdpa, it's not because it can't use STOP but
> > because we don't have a valid use case for that.
> >
>
> I had rewinding descriptors in mind actually.

Yes, STOP could be one of the building blocks for this. But it
actually requires more. e.g the ability to let the driver to change
the last_avail_idx in the device. And it depends on if we had other
requirements during the rewind, e.g can we afford to stop the whole
device or just a specific virtqueue. If it's the latter, we had
another special example (rewind to 0), Ali is proposing per virtqueue
reset which could be used to death unused buffers safely from a
specific queue (then it's something unrelated to STOP itself).

So I think STOP + virtqueue index state should be sufficient for
rewind if we can afford to stop the whole device (which might not be
the case for Ali's proposal).

Thanks

>
> Thanks!
>
> > Thanks
> >
>