Re: [PATCH v10 04/10] admin: introduce virtio admin virtqueues

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Mar 03, 2023 at 08:28:40AM -0500, Stefan Hajnoczi wrote:
> On Thu, Mar 02, 2023 at 07:05:14PM -0500, Michael S. Tsirkin wrote:
> > On Thu, Mar 02, 2023 at 03:40:07PM -0500, Stefan Hajnoczi wrote:
> > > On Thu, Mar 02, 2023 at 08:05:06AM -0500, Michael S. Tsirkin wrote:
> > > > The admin virtqueues will be the first interface to issue admin commands.
> > > > 
> > > > Currently virtio specification defines control virtqueue to manipulate
> > > > features and configuration of the device it operates on. However,
> > > > control virtqueue commands are device type specific, which makes it very
> > > > difficult to extend for device agnostic commands.
> > > > 
> > > > To support this requirement in a more generic way, this patch introduces
> > > > a new admin virtqueue interface.
> > > 
> > > Is this referring to the existing virtio-net, virtio-scsi, etc control
> > > virtqueues?
> > > 
> > > I see the admin virtqueue as the virtqueue equivalent to transport
> > > feature bits. The admin queue does nothing device type-specific (net,
> > > scsi, etc) and instead focusses on transport and core virtio tasks.
> > > 
> > > Keeping the device-specific virtqueue separate from the admin virtqueue
> > > is simpler and has fewer potential problems. I don't think creating
> > > common infrastructure for device-specific control virtqueues across
> > > device types worthwhile or within the scope of this patch series.
> > 
> > yes this commit log is outdated. referred to original
> > proposal by Max which also planned to replace cvq.
> > will update.
> > 
> > 
> > > > We also support more than one admin virtqueue, for QoS and
> > > > scalability requirements.
> > > > 
> > > > Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >  admin.tex   | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  content.tex |  7 +++--
> > > >  2 files changed, 79 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/admin.tex b/admin.tex
> > > > index 7e28b77..3ffac2e 100644
> > > > --- a/admin.tex
> > > > +++ b/admin.tex
> > > > @@ -155,3 +155,77 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
> > > >  \field{command_specific_data} and \field{command_specific_result}
> > > >  depends on these structures and is described separately or is
> > > >  implicit in the structure description.
> > > > +
> > > > +\section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
> > > > +
> > > > +An administration virtqueue of an owner device is used to submit
> > > > +group administration commands. An owner device can have more
> > > > +than one administration virtqueue.
> > > > +
> > > > +If VIRTIO_F_ADMIN_VQ has been negotiated, an owner device exposes one
> > > > +or more adminstration virtqueues. The number and locations of the
> > > > +administration virtqueues are exposed by the owner device in a transport
> > > > +specific manner.
> > > > +
> > > > +The driver submits commands by queueing them to an arbitrary
> > > > +administration virtqueue, and they are processed by the
> > > > +device in the order in which they are queued. It is the
> > > > +responsibility of the driver to ensure ordering for commands
> > > > +placed on different administration virtqueues, because they will
> > > > +be executed with no order constraints.
> > > 
> > > Does "they are processed by the device in the order in which they are
> > > queued" mean only 1 admin command can be running at any given time and
> > > therefore they will complete in order? This would allow pipelining
> > > dependent commands but prevent long-running commands because the
> > > virtqueue is blocked while executing a command.
> > 
> > we have multiple vqs for that.
> 
> This reminds me of the async challenges with QEMU's QMP monitor.
> 
> Will it ever be possible to abort an in-flight command? I guess the
> abort command would need to be submitted on another virtqueue, but how
> do you identify the in-flight command that you wish to cancel?
> 
> Please clarify the blocking semantics in the spec because it wasn't
> clear to me.
> 
> Thanks,
> Stefan

I don't really get what does "blocking" mean. Nothing is required to
block I think.
I guess for abort it is still in order but a different vq
should not be needed.

Maybe we can use ID of buffer to cancel commands?

1. driver submits command as ID = 1
2. driver submits abort as ID 2
3. device sees the abort and cancels ID 2
4. device uses buffer 1
5. device uses buffer 3 - abort is complete

Device gets commands and processes them in order. It's just like scsi:
multiple queues, if you break it you get to keep both pieces.

-- 
MST