On Thu, 28 Jan 2021 17:29:30 +0100
Cornelia Huck <cohuck@xxxxxxxxxx> wrote:
On Tue, 26 Jan 2021 15:27:43 +0200I'm still pretty nervous about the userspace aspect of this as well.
Max Gurtovoy <mgurtovoy@xxxxxxxxxx> wrote:
On 1/26/2021 5:34 AM, Alex Williamson wrote:One other thing I'd like to bring up: What needs to be done in
On Mon, 25 Jan 2021 20:45:22 -0400I think I got the main idea and I'll try to summarize it:
Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
On Mon, Jan 25, 2021 at 04:31:51PM -0700, Alex Williamson wrote:That's not really what I'm getting from your feedback, indicating
extensions potentially break vendor drivers, etc. We're only even handThis is a RFC, not a complete patch series. The RFC is to get feedback
waving that existing device specific support could be farmed out to new
device specific drivers without even going to the effort to prove that.
on the general design before everyone comits alot of resources and
positions get dug in.
Do you really think the existing device specific support would be a
problem to lift? It already looks pretty clean with the
vfio_pci_regops, looks easy enough to lift to the parent.
So far the TODOs rather mask the dirty little secrets of theIt would be helpful to get actual feedback on the high level design -
extension rather than showing how a vendor derived driver needs to
root around in struct vfio_pci_device to do something useful, so
probably porting actual device specific support rather than further
hand waving would be more helpful.
someting like this was already tried in May and didn't go anywhere -
are you surprised that we are reluctant to commit alot of resources
doing a complete job just to have it go nowhere again?
vfio-pci is essentially done, the mlx stub driver should be enough to
see the direction, and additional concerns can be handled with TODO
comments. Sorry if this is not construed as actual feedback, I think
both Connie and I are making an effort to understand this and being
hampered by lack of a clear api or a vendor driver that's anything more
than vfio-pci plus an aux bus interface. Thanks,
The separation to vfio-pci.ko and vfio-pci-core.ko is acceptable, and we
do need it to be able to create vendor-vfio-pci.ko driver in the future
to include vendor special souse inside.
userspace? Does a userspace driver like QEMU need changes to actually
exploit this? Does management software like libvirt need to be involved
in decision making, or does it just need to provide the knobs to make
the driver configurable?
QEMU and other actual vfio drivers are probably the least affected,
at least for QEMU, it'll happily open any device that has a pointer to
an IOMMU group that's reflected as a vfio group device. Tools like
libvirt, on the other hand, actually do driver binding and we need to
consider how they make driver decisions. Jason suggested that the
vfio-pci driver ought to be only spec compliant behavior, which sounds
like some deprecation process of splitting out the IGD, NVLink, zpci,
etc. features into sub-drivers and eventually removing that device
specific support from vfio-pci. Would we expect libvirt to know, "this
is an 8086 graphics device, try to bind it to vfio-pci-igd" or "uname
-m says we're running on s390, try to bind it to vfio-zpci"? Maybe we
expect derived drivers to only bind to devices they recognize, so
libvirt could blindly try a whole chain of drivers, ending in vfio-pci.
Obviously if we have competing drivers that support the same device in
different ways, that quickly falls apart.
Libvirt could also expand its available driver models for the user to
specify a variant, I'd support that for overriding a choice that libvirt
might make otherwise, but forcing the user to know this information is
just passing the buck.
Some derived drivers could probably actually include device IDs rather
than only relying on dynamic ids, but then we get into the problem that
we're competing with native host driver for a device. The aux bus
example here is essentially the least troublesome variation since it
works in conjunction with the native host driver rather than replacing
it. Thanks,
Alex