Virtualization - Oct 2023 - [PATCH vfio 10/11] vfio/virtio: Expose admin commands over virtio device

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.

-- 
MST

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