On Tue, Jan 18, 2022 at 03:36:19AM +0000, Parav Pandit wrote:
>
>
> > From: Michael S. Tsirkin <[email protected]>
> > 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 <[email protected]>
> > > > 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.
device id is not a good name, but yes. However this is why I think we
should have a slightly more generic terminology, and more space for
these IDs, and then we'd have a specific binding for VFs.
> > 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?
just that the text is not generic. would be nicer if adding
new types would involve only changing one or two places
> 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;
> };
you do not need a union straight away, Simply use something like this "device
identifier" everywhere and then add some text explaining that currently
it is a VF number and that admin device is a PF.
However we need better names, device ID is already used in the spec
for enumeration/discovery. come up with something else please.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
