Re: [PATCH v4 07/27] vfio/pci: Detect CXL devices and load vfio-cxl on demand

[Alex Williamson <alex@shazbot.org>]

On Thu, 13 Aug 2026 15:06:11 +0530
<mhonap@nvidia.com> wrote:

> From: Manish Honap <mhonap@nvidia.com>
> 
> A CXL device needs the vfio-cxl callbacks, but pulling vfio-cxl and the
> CXL core in unconditionally would bloat every vfio-pci setup. At bind,
> detect a CXL device with pcie_is_cxl() and request_module("vfio-cxl")
> only then, and hand the device to the registered ops.
> 
> Each bound CXL device pins vfio-cxl through try_module_get() and drops
> the reference at release, so vfio-cxl can unload once no CXL device is
> bound. If vfio-cxl is absent the device is driven as plain vfio-pci.
> 
> Signed-off-by: Manish Honap <mhonap@nvidia.com>
> ---
>  drivers/vfio/pci/vfio_pci_core.c | 81 ++++++++++++++++++++++++++++++--
>  include/linux/vfio_pci_core.h    |  3 ++
>  2 files changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 88e68d43af9a..0f9b5dfeea66 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -2176,6 +2176,44 @@ static void vfio_pci_vga_uninit(struct vfio_pci_core_device *vdev)
>  					      VGA_RSRC_LEGACY_MEM);
>  }
>  
> +static const struct vfio_cxl_ops *vfio_pci_cxl_ops;
> +static DEFINE_MUTEX(vfio_pci_cxl_ops_lock);
> +
> +static const struct vfio_cxl_ops *vfio_pci_get_cxl_ops(void)
> +{
> +	const struct vfio_cxl_ops *ops;
> +
> +	mutex_lock(&vfio_pci_cxl_ops_lock);
> +	ops = vfio_pci_cxl_ops;
> +	if (ops && !try_module_get(ops->owner))
> +		ops = NULL;
> +	mutex_unlock(&vfio_pci_cxl_ops_lock);
> +
> +	return ops;
> +}

Awkward flow, resolved with guards:

	guard(rwsem_read)(&vfio_pci_cxl_ops_lock);
	ops = vfio_pci_cxl_ops;
	if (!ops || !try_module_get(ops->owner))
		return NULL;

	return ops;

> +
> +/*
> + * A CXL Type-2 device advertises both CXL.cache and CXL.mem in its CXL DVSEC.
> + * pcie_is_cxl() is also true for Type-1 (cache only) and Type-3 (mem only)
> + * devices, which the vfio-cxl provider does not handle, so confirm the Type-2
> + * identity before engaging it.
> + */
> +static bool vfio_pci_is_cxl_type2(struct pci_dev *pdev)
> +{
> +	u16 dvsec, cap;
> +
> +	dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> +					  PCI_DVSEC_CXL_DEVICE);
> +	if (!dvsec)
> +		return false;
> +
> +	if (pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_CAP, &cap))
> +		return false;
> +
> +	return (cap & PCI_DVSEC_CXL_CACHE_CAPABLE) &&
> +	       (cap & PCI_DVSEC_CXL_MEM_CAPABLE);
> +}
> +
>  int vfio_pci_core_init_dev(struct vfio_device *core_vdev)
>  {
>  	struct vfio_pci_core_device *vdev =
> @@ -2197,6 +2235,41 @@ int vfio_pci_core_init_dev(struct vfio_device *core_vdev)
>  	init_rwsem(&vdev->memory_lock);
>  	xa_init(&vdev->ctx);
>  
> +	/*
> +	 * Load vfio-cxl on demand for a CXL device. If it is absent, drive the
> +	 * device as plain vfio-pci rather than failing the bind.
> +	 */
> +	if (pcie_is_cxl(vdev->pdev) && vfio_pci_is_cxl_type2(vdev->pdev)) {

Nit, embed the pcie_is_cxl() test in vfio_pci_is_cxl_type2().

> +		const struct vfio_cxl_ops *ops;
> +
> +		request_module("vfio-cxl");
> +		ops = vfio_pci_get_cxl_ops();
> +		if (ops) {
> +			ret = ops->init_device(vdev);
> +			if (ret) {
> +				module_put(ops->owner);

Create a trivial vfio_pci_put_cxl_ops() for consistency.

> +				return ret;

This looks like a regression, a device that previously worked with
vfio-pci now fails if vfio-cxl .init returns an error.  It should
continue with a log message.

The disable_cxl option that comes later is a global opt-out, not an
opt-in.  Users can opt-in to a feature that might fail previous
behavior but they should not be required to opt-out to retain existing
functionality, especially with only global granularity.

> +			}
> +			vdev->cxl_ops = ops;
> +			/*
> +			 * Pin the device in D0 while bound rather than let
> +			 * the host power it down between opens.
> +			 */
> +			vdev->disable_idle_d3 = true;

Why?  Letting the host power down the device between opens is exactly
what we want for non-cxl devices.  If we're trying to do something
around preserving the coherent memory configuration, it needs to be
justified as such, and should happen at the point where it's relevant,
ie. in the vfio-cxl .init path.

However, this alone doesn't prevent the user from using low power
states, so it also seems insufficient by itself.

> +		} else if (IS_BUILTIN(CONFIG_VFIO_CXL)) {
> +			/*
> +			 * Only DEFER for a built-in provider so the bind
> +			 * retries once vfio-cxl registers its ops.
> +			 * A modular provider was already loaded synchronously
> +			 * by request_module() above, so if it is still absent
> +			 * it is missing, blocked, or failed to init; drive the
> +			 * device as plain vfio-pci then rather than defer the
> +			 * bind forever.
> +			 */
> +			return -EPROBE_DEFER;

LLM asks if the registration function should call
driver_deferred_probe_trigger() to make the retry explicit?

> +		}
> +	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(vfio_pci_core_init_dev);
> @@ -2206,6 +2279,11 @@ void vfio_pci_core_release_dev(struct vfio_device *core_vdev)
>  	struct vfio_pci_core_device *vdev =
>  		container_of(core_vdev, struct vfio_pci_core_device, vdev);
>  
> +	if (vdev->cxl_ops) {
> +		vdev->cxl_ops->release_device(vdev);
> +		module_put(vdev->cxl_ops->owner);
> +	}

Turn both of these into helpers:

static int vfio_pci_core_cxl_init(struct vfio_device *core_vdev);
static void vfio_pci_core_cxl_release(struct vfio_device *core_vdev);

Include the tests is-cxl/cxl_ops tests in the helpers to
compartmentalize cxl init/release.  Thanks,

Alex

> +
>  	mutex_destroy(&vdev->igate);
>  	mutex_destroy(&vdev->ioeventfds_lock);
>  	kfree(vdev->region);
> @@ -2670,9 +2748,6 @@ static void vfio_pci_dev_set_try_reset(struct vfio_device_set *dev_set)
>  	}
>  }
>  
> -static const struct vfio_cxl_ops *vfio_pci_cxl_ops;
> -static DEFINE_MUTEX(vfio_pci_cxl_ops_lock);
> -
>  int vfio_pci_core_register_cxl_ops(const struct vfio_cxl_ops *ops)
>  {
>  	int ret = 0;
> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> index 14753972e714..117cd67995d8 100644
> --- a/include/linux/vfio_pci_core.h
> +++ b/include/linux/vfio_pci_core.h
> @@ -29,6 +29,7 @@ struct vfio_pci_core_device;
>  struct vfio_pci_region;
>  struct p2pdma_provider;
>  struct dma_buf_attachment;
> +struct vfio_cxl_state;
>  
>  struct vfio_pci_eventfd {
>  	struct eventfd_ctx	*ctx;
> @@ -109,6 +110,8 @@ struct vfio_pci_core_device {
>  	struct vfio_device	vdev;
>  	struct pci_dev		*pdev;
>  	const struct vfio_pci_device_ops *pci_ops;
> +	const struct vfio_cxl_ops *cxl_ops;
> +	struct vfio_cxl_state	*cxl;
>  	void __iomem		*barmap[PCI_STD_NUM_BARS];
>  	bool			bar_mmap_supported[PCI_STD_NUM_BARS];
>  	/* Flags modified at runtime - dedicated storage unit */