On 10/10/2023 18:58, Michael S. Tsirkin wrote:
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.
How about the cyclic dependencies issue inside VIRTIO that I mentioned
below ?
In my suggestion it's fine, VFIO will get the PF and give it to VIRTIO
per command.
Yishai
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
