RE: [PATCH v4 09/27] vfio/cxl: Create the CXL memory device at bind

[Manish Honap]

> -----Original Message-----
> From: Alex Williamson <alex@xxxxxxxxxxx>
> Sent: Friday, August 28, 2026 2:14 AM
> To: Manish Honap <mhonap@xxxxxxxxxx>
> Cc: jgg@xxxxxxxx; Ankit Agrawal <ankita@xxxxxxxxxx>; jic23@xxxxxxxxxx;
> dave.jiang@xxxxxxxxx; alejandro.lucero-palau@xxxxxxx; Srirangan Madhavan
> <smadhavan@xxxxxxxxxx>; corbet@xxxxxxx; skhan@xxxxxxxxxxxxxxxxxxx;
> dave@xxxxxxxxxxxx; alison.schofield@xxxxxxxxx; vishal.l.verma@xxxxxxxxx;
> iweiny@xxxxxxxxxx; ming.li@xxxxxxxxxxxx; Yishai Hadas
> <yishaih@xxxxxxxxxx>; Shameer Kolothum Thodi
> <skolothumtho@xxxxxxxxxx>; kevin.tian@xxxxxxxxx; bhelgaas@xxxxxxxxxx;
> dmatlack@xxxxxxxxxx; kees@xxxxxxxxxx; gustavoars@xxxxxxxxxx; Neo Jia
> <cjia@xxxxxxxxxx>; Krishnakant Jaju <kjaju@xxxxxxxxxx>; Vikram Sethi
> <vsethi@xxxxxxxxxx>; Zhi Wang <zhiw@xxxxxxxxxx>; linux-
> doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx;
> linux-cxl@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> kselftest@xxxxxxxxxxxxxxx; linux-hardening@xxxxxxxxxxxxxxx; alex@xxxxxxxxxxx
> Subject: Re: [PATCH v4 09/27] vfio/cxl: Create the CXL memory device at bind
> 
> External email: Use caution opening links or attachments
> 
> 
> On Thu, 13 Aug 2026 15:06:13 +0530
> <mhonap@xxxxxxxxxx> wrote:
> 
> > From: Manish Honap <mhonap@xxxxxxxxxx>
> >
> > Read the HDM region size from decoder 0, cached in pdev->hdm at
> > enumeration, and use it to size the accelerator device state. A Type-2
> > accelerator has no mailbox, so cxl_set_capacity() establishes the
> > capacity that devm_cxl_probe_mem() then uses to join the device to the
> > CXL topology and resolve the host physical range.
> >
> > Everything here is devm-scoped to the PCI device, so it is unwound
> > when vfio-pci unbinds. If pdev->hdm is not populated yet, defer the bind.
> >
> > Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
> > ---
> >  drivers/vfio/pci/cxl/vfio_cxl_core.c | 53
> > ++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >
> > diff --git a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > index cbec7319452c..125e11354a46 100644
> > --- a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > +++ b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > @@ -6,15 +6,67 @@
> >   */
> >
> >  #include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/range.h>
> >  #include <linux/vfio_pci_core.h>
> > +#include <cxl/cxl.h>
> > +
> > +/**
> > + * struct vfio_cxl_state - per-device state for a vfio-cxl device
> > + * @cxlds: CXL device state; kept first for
> > +devm_cxl_dev_state_create()
> > + * @cxlmd: memory device joined to the CXL topology at bind
> > + * @hpa_range: host physical range of the HDM region  */ struct
> > +vfio_cxl_state {
> > +     struct cxl_dev_state cxlds;
> > +     struct cxl_memdev *cxlmd;
> > +     struct range hpa_range;
> > +};
> >
> >  static int vfio_cxl_init_device(struct vfio_pci_core_device *vdev)  {
> > +     struct pci_dev *pdev = vdev->pdev;
> > +     struct vfio_cxl_state *cxl;
> > +     struct cxl_memdev *cxlmd;
> > +     u64 hdm_size, serial;
> > +     u16 dvsec;
> > +     int ret;
> > +
> > +     /* pdev->hdm is populated at PCI enumeration; defer until it is. */
> > +     if (!pdev->hdm)
> > +             return -EPROBE_DEFER;
> 
> How would a device get to a PCI driver probe function without being
> enumerated by the PCI core?  This also looks like an infinite loop; if there's an
> error setting the hdm pointer, this is forever deferred.

Sorry for this confusion; I will drop this.

> 
> > +
> > +     hdm_size = range_len(&pdev->hdm->settings[0].hpa_range);
> 
> We don't validate the number of hdm ranges until the next patch.
> 
> > +     if (!hdm_size)
> > +             return -ENXIO;
> > +
> > +     dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> > +                                       PCI_DVSEC_CXL_DEVICE);
> > +     serial = pci_get_dsn(pdev);
> > +
> > +     cxl = devm_cxl_dev_state_create(&pdev->dev, CXL_DEVTYPE_DEVMEM,
> serial,
> > +                                     dvsec, struct vfio_cxl_state, cxlds,
> > +                                     false);
> > +     if (!cxl)
> > +             return -ENOMEM;
> > +
> > +     ret = cxl_set_capacity(&cxl->cxlds, hdm_size);
> > +     if (ret)
> > +             return ret;
> > +
> > +     cxlmd = devm_cxl_probe_mem(&cxl->cxlds, &cxl->hpa_range);
> > +     if (IS_ERR(cxlmd))
> > +             return PTR_ERR(cxlmd);
> > +
> > +     cxl->cxlmd = cxlmd;
> > +     vdev->cxl = cxl;
> > +
> >       return 0;
> >  }
> >
> >  static void vfio_cxl_release_device(struct vfio_pci_core_device
> > *vdev)  {
> > +     vdev->cxl = NULL;
> >  }
> >
> >  static const struct vfio_cxl_ops vfio_cxl_ops = { @@ -39,3 +91,4 @@
> > module_exit(vfio_cxl_exit);  MODULE_LICENSE("GPL");
> > MODULE_DESCRIPTION("VFIO support for CXL Type-2 devices");
> > MODULE_ALIAS("vfio-cxl");
> 
> I missed commenting on this in the previous patch, but why do we need to
> declare an alias to the name the module already has?  AIUI this is for providing
> an alternate name for the actual name of the module.  Thanks,

Okay, I will remove this redundant code.

Manish

> 
> Alex
> 
> > +MODULE_IMPORT_NS("CXL");