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

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, January 18, 2022 3:33 AM
> 
> On Mon, Jan 17, 2022 at 02:12:33PM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Thursday, January 13, 2022 11:24 PM
> >
> > >
> > > We had an off-list meeting where I proposed addressing one device
> > > from another or grouping multiple devices as a more specific scope.
> > > That would be one way to address this.
> > >
> > > Following this idea, all commands would then gain fields for
> > > addressing one device from another.
> > >
> > Can you please explain your idea more and a need for grouping?
> > What do you want to group? VFs of parent pci device?
> > How to refer to each VF within a group?
> 
> So for example, VFs of a PF are a group right? And they are all controlled by a
> PF.
> 
> I can think of setups like nesting where we might want to create a group of VFs
> and pass them to L1, one of the VFs to act as an admin for the reset of them for
> purposes of L2.  subfunctions with PASID etc are another example. 

Subfunctions with PASID can be similarly managed by extending device identification and its MSIX/IMS vector details.
May be vf_number should be put in the union as,

union device_id {
	struct pci_vf vf_id; /* current */
	struct pci_sf sf_id; /* future */
};

So that they both can use command opcode.

> I am not
> asking you to add such mechanisms straight away but the current proposal
> kind of obscures this to the point where I don't see how would we extend it
> with these things down the road.
> 
Which part in specific make it obscure? New device type can be identifiable by above union.

May be a better structure would be in patch-5 is:
Something like below,

struct virtio_admin_pci_virt_property_set {
	enum virtio_device_identifier_type type; /* pci pf, pci vf, subfunction */
	union virtio_device_identifier {
		struct virtio_pci_dev_id pf_vf; /* current */
		struct virtio_subfunction sf; /* future */
	};
	enum virtio_interrupt_type interrupt_type; /* msix, ims=device specific, intx, something else */
	union virtio_interrupt_config {
		struct virtio_pci_msix_config msix_config;
	};
};

struct virtio_pci_interrupt_config {
	le16 msix_count;
};