[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH 1/5] Add virtio Admin Virtqueue specification
On Tue, Jan 18, 2022 at 03:36:19AM +0000, Parav Pandit wrote: > > > > 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. 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.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]