> -----Original Message-----
> From: Alex Williamson <[email protected]>
> Sent: Tuesday, September 22, 2026 7:44 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 v5 20/27] vfio/cxl: Expose the HDM decoder registers
> read-only to the guest
>
> External email: Use caution opening links or attachments
>
>
> On Thu, 17 Sep 2026 00:05:33 +0530
> <[email protected]> wrote:
>
> > From: Manish Honap <[email protected]>
> >
> > A CXL Type-2 guest reads the HDM decoder registers to learn the HDM
> > region it was handed. Those registers live in the component BAR that
> > vfio-pci owns.
> >
> > Map the decoder block at bind: its location comes from the pdev->hdm
> > enumeration cache, and vfio-pci owns the BAR, so map it without
> > claiming the block and expose it as a second, read-only region under
> > VFIO_REGION_TYPE_PCI_VENDOR_TYPE with the CXL vendor id, registered
> > per open like the HDM memory region.
> >
> > Serve reads live from the mapped block and absorb writes without
> > forwarding them to hardware.
> >
> > Registering a second region is the first point at which an open-time
> > failure must unwind an already-registered region, so add
> > vfio_pci_core_unregister_dev_region() to drop the most recently
> > registered region, and use it to unwind the HDM memory region if the
> > decoder region fails to register.
> >
> > Assisted-by: LLM
> > Signed-off-by: Manish Honap <[email protected]>
> > ---
> > drivers/vfio/pci/cxl/vfio_cxl_core.c | 81
> +++++++++++++++++++++++++++-
> > include/uapi/linux/vfio.h | 2 +
> > 2 files changed, 82 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > index e099e9a70a5a..da04776356e4 100644
> > --- a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > +++ b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > @@ -24,6 +24,8 @@
> > * @cxlmd: memory device joined to the CXL topology at bind
> > * @hpa_range: host physical range of the HDM region
> > * @hdm_pfn_space: HDM-region pfn range registered with
> > memory_failure()
> > + * @hdm_regs: mapped HDM decoder registers, read live by the decoder
> > + region
> > + * @hdm_len: length of the HDM decoder register block
> > * @hdm_valid: true when host CPU access to the HDM range is safe; under
> memory_lock
> > */
> > struct vfio_cxl_state {
> > @@ -31,6 +33,8 @@ struct vfio_cxl_state {
> > struct cxl_memdev *cxlmd;
> > struct range hpa_range;
> > struct pfn_address_space hdm_pfn_space;
> > + void __iomem *hdm_regs;
> > + u32 hdm_len;
> > bool hdm_valid;
> > };
> >
> > @@ -212,6 +216,56 @@ static int vfio_cxl_register_pfn_space(struct
> vfio_pci_core_device *vdev)
> > return register_pfn_address_space(&cxl->hdm_pfn_space);
> > }
> >
> > +static ssize_t vfio_cxl_comp_rw(struct vfio_pci_core_device *vdev,
> > + char __user *buf, size_t count, loff_t *ppos,
> > + bool iswrite) {
> > + struct vfio_cxl_state *cxl = vdev->cxl;
> > + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> > + void *tmp;
> > +
> > + if (pos >= cxl->hdm_len)
> > + return -EINVAL;
> > +
> > + /* The decoder registers take only aligned dword accesses. */
> > + if (pos % sizeof(u32) || count % sizeof(u32))
> > + return -EINVAL;
> > +
> > + count = min_t(size_t, count, cxl->hdm_len - pos);
> > +
> > + /*
> > + * The host committed and locked the physical decoder before the guest
> > + * saw the device, so the guest never drives it: absorb writes without
> > + * forwarding them to hardware. The guest programs a GPA that the
> VMM
> > + * virtualizes; reads return the live registers, which already report
> > the
> > + * decoder committed. BASE_LOW and BASE_HIGH carry the host HPA,
> visible
> > + * only to the trusted VMM that virtualizes it away from the guest.
> > + */
> > + if (iswrite) {
> > + *ppos += count;
> > + return count;
> > + }
> > +
> > + tmp = kmalloc(count, GFP_KERNEL);
> > + if (!tmp)
> > + return -ENOMEM;
> > +
> > + memcpy_fromio(tmp, cxl->hdm_regs + pos, count);
>
> There's no memory-enabled gate on this access to mapped BAR space.
Right. In v6, the read will take a memory_lock and return -EIO while memory is
Disabled.
>
> > + if (copy_to_user(buf, tmp, count)) {
> > + kfree(tmp);
> > + return -EFAULT;
> > + }
> > + kfree(tmp);
> > +
> > + *ppos += count;
> > + return count;
> > +}
> > +
> > +static const struct vfio_pci_regops vfio_cxl_comp_regops = {
> > + .rw = vfio_cxl_comp_rw,
> > + .release = vfio_cxl_region_release, };
> > +
> > static void vfio_cxl_release_hpa(void *data) {
> > struct vfio_cxl_state *cxl = data; @@ -299,6 +353,21 @@ static
> > int vfio_cxl_init_device(struct vfio_pci_core_device *vdev)
> > goto err;
> > }
> >
> > + /*
> > + * Map the HDM decoder registers so the decoder region can read them
> > + * live. The block location comes from the enumeration cache in
> > + * pdev->hdm; vfio-pci owns the BAR, so map without claiming the
> > block.
> > + */
> > + cxl->hdm_regs = devm_ioremap(&pdev->dev,
> > + pci_resource_start(pdev,
> > pdev->hdm->hdm_bar) +
> > + pdev->hdm->hdm_offset,
> > pdev->hdm->hdm_size);
> > + if (!cxl->hdm_regs) {
> > + ret = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + cxl->hdm_len = pdev->hdm->hdm_size;
> > +
> > /*
> > * A Type-2 accelerator has no mailbox and no media-ready register, so
> > * set media ready directly.
> > @@ -381,6 +450,13 @@ static int vfio_cxl_open_device(struct
> vfio_pci_core_device *vdev)
> > if (ret)
> > return ret;
> >
> > + ret = vfio_cxl_add_region(vdev,
> VFIO_REGION_SUBTYPE_CXL_COMP_REGS,
> > + &vfio_cxl_comp_regops, cxl->hdm_len,
> > + VFIO_REGION_INFO_FLAG_READ |
> > + VFIO_REGION_INFO_FLAG_WRITE);
> > + if (ret)
> > + goto err_unregister_mem;
> > +
> > /*
> > * The HDM region is advertised mmap-able, so a fd holder can fault
> > its
> > * struct-page-less device memory in from the host CPU. Register
> > it with @@ -389,7 +465,7 @@ static int vfio_cxl_open_device(struct
> vfio_pci_core_device *vdev)
> > */
> > ret = vfio_cxl_register_pfn_space(vdev);
> > if (ret && ret != -EOPNOTSUPP)
> > - goto err_unregister_mem;
> > + goto err_unregister_comp;
> >
> > /*
> > * The decoder is firmware-committed, so host access to the HDM
> > range is @@ -400,6 +476,9 @@ static int vfio_cxl_open_device(struct
> > vfio_pci_core_device *vdev)
> >
> > return 0;
> >
> > +err_unregister_comp:
> > + vfio_pci_core_unregister_dev_region(vdev);
> > +
> > err_unregister_mem:
> > vfio_pci_core_unregister_dev_region(vdev);
>
> As noted previously, this is a bad interface, this is why. Thanks,
Understood; These calls go away with the patch 18 change.
>
> Alex
>
> >
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 1bf86763c0f7..8927a7a4e8e4 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -373,6 +373,8 @@ struct vfio_region_info_cap_type {
> > /* CXL Type-2 device (0x1e98) sub-types for
> > VFIO_REGION_TYPE_PCI_VENDOR_TYPE */
> > /* CXL.mem HDM region of a Type-2 device, mmap-able */
> > #define VFIO_REGION_SUBTYPE_CXL_MEM (1)
> > +/* CXL HDM decoder registers: read live, guest writes are absorbed */
> > +#define VFIO_REGION_SUBTYPE_CXL_COMP_REGS (2)
> >
> > /* sub-types for VFIO_REGION_TYPE_GFX */
> > #define VFIO_REGION_SUBTYPE_GFX_EDID (1)
