Re: [PATCH v3 1/1] vfio/pci: Remove the core dependency on driver data

[Alex Williamson <alex@shazbot.org>]

On Fri, 9 Oct 2026 16:28:00 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Mon, Oct 05, 2026 at 11:42:36AM -0700, Simon Song wrote:
> > vfio-pci-core currently has runtime functions that interpret pci
> > driver_data as a pointer to vfio_pci_core_device, and enforce vfio
> > variant drivers must set vfio_pci_core_device to their pci driver_data.
> > This constrains variant drivers' private-data layout, including the
> > typed driver data used by the Rust PCI infrastructure.
> > 
> > Add VFIO_PCI_CORE_DEFINE_CALLBACKS marcos to generate wrapper code for
> > each vfio variant driver, update vfio variant drivers to store their
> > private strctures in driver_data and use generated wrapper to pass the
> > vfio core device to helpers.
> > 
> > Select the vga decode callback during vfio_pci_vga_init based on if
> > vfio_vga_disabled, the callback only use pci_dev and does not require
> > driver_data
> > 
> > Assisted-by: LLM
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > Suggested-by: Alex Williamson <alex.williamson@nvidia.com>
> > Suggested-by: Danilo Krummrich <dakr@kernel.org>
> > Link: https://lore.kernel.org/all/DLFD2ZDSK9YQ.3A4R66G8UJMD8@kernel.org/
> > Signed-off-by: Simon Song <xinmengs@nvidia.com>
> > ---
> >  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    | 27 ++++----
> >  drivers/vfio/pci/ism/main.c                   | 19 +++---
> >  drivers/vfio/pci/mlx5/main.c                  | 13 ++--
> >  drivers/vfio/pci/nvgrace-gpu/main.c           | 19 +++---
> >  drivers/vfio/pci/pds/pci_drv.c                |  8 ++-
> >  drivers/vfio/pci/pds/vfio_dev.c               |  5 +-
> >  drivers/vfio/pci/qat/main.c                   | 12 ++--
> >  drivers/vfio/pci/vfio_pci.c                   | 33 +++++++++-
> >  drivers/vfio/pci/vfio_pci_core.c              | 61 +++++++------------
> >  drivers/vfio/pci/virtio/main.c                |  8 ++-
> >  drivers/vfio/pci/xe/main.c                    |  8 ++-
> >  include/linux/vfio_pci_core.h                 | 41 ++++++++++++-
> >  12 files changed, 159 insertions(+), 95 deletions(-)  
> 
> This looks OK to me, but it probably should have been 3 patches,
> mixing the VGA stuff into all of this (and you have a smart approach)
> is not how upstream likes to see things arranged.

The VGA stuff actually goes away entirely when this is rebased onto
Dave's vgaarb changes in linux-next[1].  All 3 VGA related chunks are
simply dropped.  vga_client_register() takes an opaque pointer, which
then already removes the dependency on the pci_dev drvdata.

To apply, the below 3 chunks are dropped in their entirety.  If this
looks right, please someone double check, I'll apply it.  Thanks,

Alex

[1]https://lore.kernel.org/all/20260922071807.2533884-1-airlied@gmail.com/

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 362c375a0579..f8ed54841723 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -153,6 +153,12 @@ static inline void vfio_pci_core_debugfs_init(struct vfio_pci_core_device *vdev)
 }
 #endif /* CONFIG_VFIO_DEBUGFS */
 
+static unsigned int vfio_pci_vga_decode_all(struct pci_dev *pdev, bool single_vga)
+{
+	return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM |
+	       VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM;
+}
+
 /*
  * Our VGA arbiter participation is limited since we don't know anything
  * about the device itself.  However, if the device is the only VGA device
@@ -161,16 +167,14 @@ static inline void vfio_pci_core_debugfs_init(struct vfio_pci_core_device *vdev)
  * has no way to get to it and routing can be disabled externally at the
  * bridge.
  */
-static unsigned int vfio_pci_set_decode(struct pci_dev *pdev, bool single_vga)
+static unsigned int vfio_pci_vga_decode_novga(struct pci_dev *pdev, bool single_vga)
 {
-	struct vfio_pci_core_device *vdev = dev_get_drvdata(&pdev->dev);
 	struct pci_dev *tmp = NULL;
 	unsigned char max_busnr;
 	unsigned int decodes;
 
-	if (single_vga || !vfio_vga_disabled(vdev) || pci_is_root_bus(pdev->bus))
-		return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM |
-		       VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM;
+	if (single_vga || pci_is_root_bus(pdev->bus))
+		return vfio_pci_vga_decode_all(pdev, single_vga);
 
 	max_busnr = pci_bus_max_busnr(pdev->bus);
 	decodes = VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
@@ -2148,20 +2138,27 @@ static void vfio_pci_vf_uninit(struct vfio_pci_core_device *vdev)
 
 static int vfio_pci_vga_init(struct vfio_pci_core_device *vdev)
 {
+	unsigned int (*set_decode)(struct pci_dev *pdev, bool single_vga);
 	struct pci_dev *pdev = vdev->pdev;
 	int ret;
 
 	if (!vfio_pci_is_vga(pdev))
 		return 0;
 
+	/* VGA access is fixed for the lifetime of this registration. */
+	if (vfio_vga_disabled(vdev))
+		set_decode = vfio_pci_vga_decode_novga;
+	else
+		set_decode = vfio_pci_vga_decode_all;
+
 	ret = aperture_remove_conflicting_pci_devices(pdev, vdev->vdev.ops->name);
 	if (ret)
 		return ret;
 
-	ret = vga_client_register(pdev, vfio_pci_set_decode);
+	ret = vga_client_register(pdev, set_decode);
 	if (ret)
 		return ret;
-	vga_set_legacy_decoding(pdev, vfio_pci_set_decode(pdev, false));
+	vga_set_legacy_decoding(pdev, set_decode(pdev, false));
 	return 0;
 }