Re: [PATCH 1/5] Add virtio Admin Virtqueue specification

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

On Tue, Jan 18, 2022 at 08:23:57AM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, January 18, 2022 1:36 PM
> 
> [..]
> > > > > > You can map it to a queue, yes. But something static and read
> > > > > > only such as list of commands maps well to config space. And
> > > > > > it's not controlling one device from another, so does not really
> > > > > > seem to belong in
> > > > the admin queue.
> > > > > >
> > > > > Aq serves the writing device config too in patch-5 in this patchset.
> > > >
> > > > List of available admin commands does not need to be written.
> > > >
> > > It is not written into the aq commands.
> > > It is part of the feature bit VIRTIO_F_ADMIN_PCI_VIRT_MANAGER indicating
> > a given functionality supported or not in patch-5.
> > 
> > 
> > Btw I don't see what does "VIRT_MANAGER" mean here. "manager" is just a
> > generic thing that means nothing, and VIRT just repeats VIRTIO.
> > 
> VIRT doesn't repeat VIRTIO.

It's literally a substring ;)

> VIRT indicates PCI virtual functions.

I'd use something like "SRIOV" then.

> Manager is generic thing to manage.

generic to the point of being meaningless.

> Below is the snippet from patch-5. 
> 
> +  \item[VIRTIO_F_ADMIN_PCI_VIRT_MANAGER (44)] This feature indicates  
> + that the device can manage PCI related capabilities for its managed 
> + PCI VF  devices and supports VIRTIO_ADMIN_PCI_VIRT_MGMT_ATTR_IDENTIFY,

manage twice here. VFs are always managed. And manage here again
does not mean much. I'd say something like:
"allow control over VFs through admin vq of the PF"

> +  VIRTIO_ADMIN_PCI_VIRT_PROPERTY_SET and 
> + VIRTIO_ADMIN_PCI_VIRT_PROPERTY_GET
> +  admin commands. This feature can be supported only by PCI devices.
> +
> 
> > 
> > > And structure like, virtio_admin_pci_virt_mgmt_attr_identify_result can
> > potentially grow and storing those fields on on-chip resource is less efficient.
> > > Hence, they are shared via AQ.
> > 
> > The issue is this: VIRTIO_F_ADMIN_PCI_VIRT_MANAGER seems to mean there
> > are pci related admin commands. OK I guess. 
> Right.
> > However you then say this same
> > feature bit implies generic functionality like list of supported commands.
> > Confusing.
> >
> Not sure where I mentioned generic functionality.
> This feature bit implies pci virtualization related functionality by means of above listed commands.

pci virtualization being SRIOV here with PF controlling VFs.

-- 
MST