> -----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 */
