On Tue, Oct 10, 2023 at 06:43:32PM +0300, Yishai Hadas wrote: > On 10/10/2023 18:14, Michael S. Tsirkin wrote: > > On Tue, Oct 10, 2023 at 06:09:44PM +0300, Yishai Hadas wrote: > > > On 10/10/2023 17:54, Michael S. Tsirkin wrote: > > > > On Tue, Oct 10, 2023 at 11:08:49AM -0300, Jason Gunthorpe wrote: > > > > > On Tue, Oct 10, 2023 at 09:56:00AM -0400, Michael S. Tsirkin wrote: > > > > > > > > > > > > However - the Intel GPU VFIO driver is such a bad experiance I don't > > > > > > > want to encourage people to make VFIO drivers, or code that is only > > > > > > > used by VFIO drivers, that are not under drivers/vfio review. > > > > > > So if Alex feels it makes sense to add some virtio functionality > > > > > > to vfio and is happy to maintain or let you maintain the UAPI > > > > > > then why would I say no? But we never expected devices to have > > > > > > two drivers like this does, so just exposing device pointer > > > > > > and saying "use regular virtio APIs for the rest" does not > > > > > > cut it, the new APIs have to make sense > > > > > > so virtio drivers can develop normally without fear of stepping > > > > > > on the toes of this admin driver. > > > > > Please work with Yishai to get something that make sense to you. He > > > > > can post a v2 with the accumulated comments addressed so far and then > > > > > go over what the API between the drivers is. > > > > > > > > > > Thanks, > > > > > Jason > > > > /me shrugs. I pretty much posted suggestions already. Should not be hard. > > > > Anything unclear - post on list. > > > > > > > Yes, this is the plan. > > > > > > We are working to address the comments that we got so far in both VFIO & > > > VIRTIO, retest and send the next version. > > > > > > Re the API between the modules, It looks like we have the below > > > alternatives. > > > > > > 1) Proceed with current approach where we exposed a generic API to execute > > > any admin command, however, make it much more solid inside VIRTIO. > > > 2) Expose extra APIs from VIRTIO for commands that we can consider future > > > client usage of them as of LIST_QUERY/LIST_USE, however still have the > > > generic execute admin command for others. > > > 3) Expose API per command from VIRTIO and fully drop the generic execute > > > admin command. > > > > > > Few notes: > > > Option #1 looks the most generic one, it drops the need to expose multiple > > > symbols / APIs per command and for now we have a single client for them > > > (i.e. VFIO). > > > Options #2 & #3, may still require to expose the virtio_pci_vf_get_pf_dev() > > > API to let VFIO get the VIRTIO PF (struct virtio_device *) from its PCI > > > device, each command will get it as its first argument. > > > > > > Michael, > > > What do you suggest here ? > > > > > > Thanks, > > > Yishai > > I suggest 3 but call it on the VF. commands will switch to PF > > internally as needed. For example, intel might be interested in exposing > > admin commands through a memory BAR of VF itself. > > > The driver who owns the VF is VFIO, it's not a VIRTIO one. > > The ability to get the VIRTIO PF is from the PCI device (i.e. struct > pci_dev). > > In addition, > virtio_pci_vf_get_pf_dev() was implemented for now in virtio-pci as it > worked on pci_dev. On pci_dev of vf, yes? So again just move this into each command, that's all. I.e. pass pci_dev to each. > Assuming that we'll put each command inside virtio as the generic layer, we > won't be able to call/use this API internally to get the PF as of cyclic > dependencies between the modules, link will fail. > > So in option #3 we may still need to get outside into VFIO the VIRTIO PF and > give it as pointer to VIRTIO upon each command. > > Does it work for you ? > > Yishai
