RE: [PATCH v5 20/27] vfio/cxl: Expose the HDM decoder registers read-only to the guest

[Manish Honap]

> -----Original Message-----
> From: Alex Williamson <alex@xxxxxxxxxxx>
> Sent: Tuesday, September 22, 2026 7:44 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 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
> <mhonap@xxxxxxxxxx> wrote:
> 
> > From: Manish Honap <mhonap@xxxxxxxxxx>
> >
> > 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 <mhonap@xxxxxxxxxx>
> > ---
> >  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)