Re: [PATCH 12/13] vfio/nvidia-vgpu: add the NVIDIA vGPU VFIO variant driver

["Danilo Krummrich" <dakr@kernel.org>]

On Wed Sep 16, 2026 at 6:28 PM CEST, Jason Gunthorpe wrote:
> On Wed, Sep 16, 2026 at 05:38:21PM +0200, Danilo Krummrich wrote:
>> Usually, class device implementations can't make assumptions of the underlying
>> bus, because they have to work for any bus. I.e. there's no other way than
>> providing helpers and letting drivers do the glue code between the bus and the
>> class device.
>
> You should think of VFIO as a series of helper libraries. One of those
> libraries is "here are all the PCI PM ops you need".
>
> Drivers rarely need to customize these ops, they just need to wire it
> up to the support library to avoid a bunch of code duplication.
>
> Stated another way - the very point of vfio-pci is to remove
> duplicated code from the drivers.
>
> So if we wanted to push hard on removing drvdata, and don't want to
> touch the struct device I would probably say to #define up a way for
> the driver to build its unique trampolines. It wastes a bunch of .text
> but at least it doesn't duplicate code.
>
>> The reason this seems undesirable from a vfio-pci perspective is that it is
>> special in the sense that it is a class device that is specifically built to sit
>> on top of a spcific bus device (i.e. struct pci_dev).
>
> It is a library, all these ideas to do things with the driver core to
> implement a library make no architectural sense.

Both is true, it is a library, and it is also a class device built on top of
another class device (struct vfio_device) that accomodates to a specific
underlying bus (PCI).

Which means that this library has the need to make a generic connection between
the class (VFIO) and the bus (PCI). And this is something that the kernel has no
generic solution for (and isn't represented by the driver core in any way).

Your dev->subsystem_data idea would accomodate this connection. However, I
object to this, as it'd be for a very special case and I'd be worried it is
abused by other subsystems in odd ways.

Besides that, I still think that the correct thing to do is to have the glue in
the driver and just provide the helpers in the best possible form. In fact, this
is what a library should do - provide the helpers, but do not directly
interfere with with other layers.

>> It still makes me think that there should be some closer integration of vfio-pci
>> with the PCI core, as it is specifically built for this bus.
>
> It has such a basic need I don't see this as a reason to pollute pci
> core with any vfio specific things. Like I would nak your [1], that's
> completely wrong layering.

As I said in the beginning, it is just a "hack"; and in fact it is the exact
same layering violation as abusing dev->driver_data or adding
dev->subsystem_data.

The reasons I called it "more reasonable" and "less of a hack" in my previous
reply are that it doesn't break an existing (driver core) API contract and it
only affects the the exact two layers this is about in the first place.

>> > I don't really understand in a nutshell why we should do this for nova
>> > the mails were so long... Can we not just ignore the lifetime
>> > imperfection for this?
>> 
>> I mentioned some points in the first two paragraphs of [2]. Besides that, I
>> don't see a reason why we should spend time and effort for working out the
>> inferior solution, where the better alternative is even less effort, contributes
>> to better quality and stability of the whole driver project and also offers a
>> chance for the vfio subsystem to gain new contributors and gather experience
>> with the language that has proven itself in many areas already.
>
> It seems to be quite a leap that it is less effort. IDK..
>
>> TBH, I don't think it makes a difference; having Rust abstractions is pretty
>> much the same as having another driver. I.e. it would be equivalent to saying
>> "before we accept another pci-vfio driver we need to do some rework".
>
> Well, it is, but thats the point when judging effort..

So it seems that we agree that there isn't really a difference between adding a
new C or Rust driver in this regard.