Re: [RFC PATCH v4 15/20] vhost: Shadow virtqueue buffers forwarding

[Eugenio Perez Martin <eperezma@redhat.com>]

On Tue, Oct 12, 2021 at 7:21 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Eugenio Pérez <eperezma@redhat.com> writes:
>
> > Initial version of shadow virtqueue that actually forward buffers. There
> > are no iommu support at the moment, and that will be addressed in future
> > patches of this series. Since all vhost-vdpa devices uses forced IOMMU,
> > this means that SVQ is not usable at this point of the series on any
> > device.
> >
> > For simplicity it only supports modern devices, that expects vring
> > in little endian, with split ring and no event idx or indirect
> > descriptors. Support for them will not be added in this series.
> >
> > It reuses the VirtQueue code for the device part. The driver part is
> > based on Linux's virtio_ring driver, but with stripped functionality
> > and optimizations so it's easier to review. Later commits add simpler
> > ones.
> >
> > SVQ uses VIRTIO_CONFIG_S_DEVICE_STOPPED to pause the device and
> > retrieve its status (next available idx the device was going to
> > consume) race-free. It can later reset the device to replace vring
> > addresses etc. When SVQ starts qemu can resume consuming the guest's
> > driver ring from that state, without notice from the latter.
> >
> > This status bit VIRTIO_CONFIG_S_DEVICE_STOPPED is currently discussed
> > in VirtIO, and is implemented in qemu VirtIO-net devices in previous
> > commits.
> >
> > Removal of _S_DEVICE_STOPPED bit (in other words, resuming the device)
> > can be done in the future if an use case arises. At this moment we can
> > just rely on reseting the full device.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  qapi/net.json                      |   2 +-
> >  hw/virtio/vhost-shadow-virtqueue.c | 237 ++++++++++++++++++++++++++++-
> >  hw/virtio/vhost-vdpa.c             | 109 ++++++++++++-
> >  3 files changed, 337 insertions(+), 11 deletions(-)
> >
> > diff --git a/qapi/net.json b/qapi/net.json
> > index fe546b0e7c..1f4a55f2c5 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -86,7 +86,7 @@
> >  #
> >  # @name: the device name of the VirtIO device
> >  #
> > -# @enable: true to use the alternate shadow VQ notifications
> > +# @enable: true to use the alternate shadow VQ buffers fowarding path
>
> Uh, why does the flag change meaning half-way through this series?
>

Before this patch, the SVQ mode just makes an extra hop for
notifications. Guest ones are now received by qemu via ioeventfd, and
qemu forwards them to the device using a different eventfd. The
reverse is also true: the device ones will be received by qemu by
device call fd, and then qemu will forward them to the guest using a
different irqfd.

This intermediate step is not very useful by itself, but helps for
checking that that part of the communication works fine, with no need
for shadow virtqueue to understand vring format. Doing that way also
produces smaller patches.

So it makes sense to me to tell what QMP command does exactly at every
point of the series. However I can directly document it as "use the
alternate shadow VQ buffers forwarding path" from the beginning.

Does this make sense, or will it be better to write the final
intention of the command?

Thanks!

> >  #
> >  # Returns: Error if failure, or 'no error' for success.
> >  #
>
> [...]
>