Re: [PATCH 2/5] Introduce VIRTIO_F_ADMIN_VQ_INDIRECT_DESC/VIRTIO_F_ADMIN_VQ_IN_ORDER

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

On Thu, Jan 27, 2022 at 03:49:58AM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, January 26, 2022 7:42 PM
> > 
> > On Wed, Jan 26, 2022 at 01:29:27PM +0000, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Tuesday, January 25, 2022 5:39 PM
> > >
> > > > >
> > > > > So you do agree that managing a managed
> > > > > (create/destroy/setup/etc...) will be done using the AQ of the managing
> > device ?
> > > >
> > > > I think Jason asked that the management commands are split from the
> > > > queue itself, such that they can be implemented in more ways down the
> > road.
> > >
> > > Admin commands involved DMA. And also multiple commands can be
> > enqueued to the device.
> > > I don't see any other construct in the spec other than vq that enables driver
> > and device to achieve both.
> > > Hence split the queue from command is very weird for spec to do.
> > 
> >  VIRTIO_PCI_CAP_SHARED_MEMORY_CFG can do this.
> >
> Device needs to always have shared memory region always in available range for driver to read.
> Driver still needs to copy data from shared region to own memory.
> How DMA Can be done in the host driver PA using shared memory region?

It can't but you don't need DMA if device is writing its own memory.

> In future when SIOV occurs, each new SIOV SF/ADI will require new
> window in the shared regions exposed statically during PCI discovery
> time.

Donnu if it's statically. But we are talking about AQ thing, that
is just per group not per SF, so might not be too bad.

> So it doesn't scale.


Maybe. You said that's the only way to queue commands to device in the
spec, I just responded with a way. No need to code that up right now,
but just something to keep in mind, things are not absolute here.

> > 
> > > A recent addition like virtio_fs_req didnât adopt this suggestion.
> > >
> > > If we just want to split so that admin commands can be transported via non
> > admin queue, we should remove below line from spec:
> > >
> > > "The Admin command set defines the commands that may be issued only to
> > the admin virtqueue"
> > 
> > sure
> > 
> > > And reword it as below,
> > >
> > > The Admin command set defines the commands to manipulate various
> > features/attributes of another device within the same group...
> > > When VIRTIO_F_ADMIN_VQ feature is negotiated, admin commands must
> > be transported through the admin queue.
> > >
> > > Looks ok?
> > 
> > I see no problem with that.
> 
> Since we agree for this, I considered AQ vs no_AQ as closed topic?

It's always hard to answer questions like this when all I have to go on
is a vague idea of what you mean.  Based on the discussion I am guessing
that your v3 will use the VQ in a way that is not controversial if that
is what you are asking about.
-- 
MST