> -----Original Message-----
> From: Alex Williamson <[email protected]>
> Sent: Thursday, August 27, 2026 3:47 AM
> To: Manish Honap <[email protected]>
> Cc: [email protected]; Ankit Agrawal <[email protected]>; [email protected];
> [email protected]; [email protected]; Srirangan Madhavan
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Yishai Hadas
> <[email protected]>; Shameer Kolothum Thodi
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Neo Jia
> <[email protected]>; Krishnakant Jaju <[email protected]>; Vikram Sethi
> <[email protected]>; Zhi Wang <[email protected]>; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH v4 07/27] vfio/pci: Detect CXL devices and load vfio-cxl
> on
> demand
>
> External email: Use caution opening links or attachments
>
>
> On Thu, 13 Aug 2026 15:06:11 +0530
> <[email protected]> wrote:
>
> > From: Manish Honap <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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;
>
Okay, I will update this.
> > +
> > +/*
> > + * 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().
Okay, I will fold pcie_is_cxl() into 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.
Okay.
>
> > + 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.
Agreed. I will modify this so that the CXL init failure becomes non-fatal
I will log the failure, drop module ref and continue as plain vfio-pci in this
case)
>
> 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.
Okay, I will make sure the opt-out no longer gates the fallback.
>
> > + }
> > + 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.
I will Move disable_idle_d3 into the CXL .init path with the justification
added there and
for this series, add the full guest-D3 block in the cover-letter as a follow-up
item.
>
> > + } 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?
Okay, I will add a call to driver_deferred_probe_trigger() from register.
>
> > + }
> > + }
> > +
> > 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,
Okay.
Manish
>
> 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 */
