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

[Dave Airlie]

On Thu, 17 Sept 2026 at 02:28, Jason Gunthorpe <jgg@xxxxxxxxxx> 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.

It's currently a library being used as a midlayer. demidlayering it so
drivers control drvdata removes that.

I sent the diffstat that covers all the current vfio pci drivers and
that didn't seem excessive.

>
> 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.

+static pci_ers_result_t hisi_acc_vfio_pci_aer_err_detected(
+       struct pci_dev *pdev, pci_channel_state_t state)
+{
+       struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_drvdata(pdev);
+
+       return vfio_pci_core_aer_err_detected(&hisi_acc_vdev->core_device,
state);
+}
+
+static int hisi_acc_vfio_pci_runtime_suspend(struct device *dev)
+{
+       struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(dev);
+
+       return vfio_pci_core_runtime_suspend(&hisi_acc_vdev->core_device);
+}
+
+static int hisi_acc_vfio_pci_runtime_resume(struct device *dev)
+{
+       struct hisi_acc_vf_core_device *hisi_acc_vdev = dev_get_drvdata(dev);
+
+       return vfio_pci_core_runtime_resume(&hisi_acc_vdev->core_device);
+}
+
+static const struct dev_pm_ops hisi_acc_vfio_pci_pm_ops = {
+       SET_RUNTIME_PM_OPS(hisi_acc_vfio_pci_runtime_suspend,
+                          hisi_acc_vfio_pci_runtime_resume, NULL)
+};

is how much code it adds to current drivers, this doesn't seem
excessive for 10 drivers,
like you could obfuscate it a bit with some macros, but I really don't
see a lot of value in hiding
what is effectively just standard driver boilerplate.

Dave.