RE: [PATCH v5 21/27] vfio/cxl: Exclude the HDM decoder registers from direct BAR access

[Manish Honap]

> -----Original Message-----
> From: Richard Cheng <icheng@xxxxxxxxxx>
> Sent: Thursday, September 17, 2026 12:59 PM
> To: Manish Honap <mhonap@xxxxxxxxxx>
> Cc: alex@xxxxxxxxxxx; 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
> Subject: Re: [PATCH v5 21/27] vfio/cxl: Exclude the HDM decoder registers
> from direct BAR access
> 
> On Thu, Sep 17, 2026 at 12:05:34AM +0800, mhonap@xxxxxxxxxx wrote:
> > From: Manish Honap <mhonap@xxxxxxxxxx>
> >
> > The HDM decoder register block lives in the component BAR that
> > vfio-pci owns. The guest reads the decoder only through the trapped,
> > read-only decoder region; it must not reach the physical registers
> > through the BAR directly, and a host read of the range through a
> > kernel mapping could abort as a fatal host SError.
> >
> > Add the block to the generic excluded-range list with
> > VFIO_PCI_EXCLUDE_MMAP | VFIO_PCI_EXCLUDE_READ |
> VFIO_PCI_EXCLUDE_WRITE
> > similar to the MSI-X exclusion.
> >
> 
> Hi Manish,
> 
> I think we need to block eventfd write and BAR DMA as well ?
> 
> Have some comments below.
> 
> 
> > Assisted-by: LLM
> > Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
> > ---
> >  drivers/vfio/pci/cxl/vfio_cxl_core.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > index da04776356e4..37e8a3b54cfb 100644
> > --- a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > +++ b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > @@ -435,8 +435,25 @@ static int vfio_cxl_add_region(struct
> > vfio_pci_core_device *vdev, u32 subtype,  static int
> > vfio_cxl_open_device(struct vfio_pci_core_device *vdev)  {
> >  	struct vfio_cxl_state *cxl = vdev->cxl;
> > +	struct pci_dev *pdev = vdev->pdev;
> >  	int ret;
> >
> > +	/*
> > +	 * Keep the HDM decoder block out of the guest's direct BAR access:
> the
> > +	 * guest reaches it only through the trapped decoder region, and a
> host
> > +	 * read of the range through a kernel mapping could abort as an SError.
> > +	 * Exclude it from mmap, fill reads with -1 and drop writes. The list is
> > +	 * cleared on close, so re-add it per open.
> > +	 */
> > +	ret = vfio_pci_core_add_excluded_range(vdev, pdev->hdm->hdm_bar,
> > +					       pdev->hdm->hdm_offset,
> > +					       cxl->hdm_len,
> > +					       VFIO_PCI_EXCLUDE_MMAP |
> > +					       VFIO_PCI_EXCLUDE_READ |
> > +					       VFIO_PCI_EXCLUDE_WRITE);
> 
> This adds the HDM reg to the exclusion list, but the eventfd and BAR DMA
> path doesn't check that.
> 
> Maybe we need to block those 2 path as well ?
> 

Thanks, yes, I Agree on both. The excluded-range list was only consulted on
the fd read/write and mmap paths, so, I think, the decoder block is still
reachable two other ways suggested above.

For ioeventfd: I will add a guard in vfio_pci_ioeventfd() to reject a
registration whose [pos, pos + count) overlaps a write-excluded range, right
next to the existing MSI-X guard.

For BAR DMA. I will add a check in vfio_pci_core_feature_dma_buf() to reject
an export whose ranges overlap an excluded range on that BAR.

Thanks,
Manish

> Best regards,
> Richard Cheng.
> 
> 
> > +	if (ret)
> > +		return ret;
> > +
> >  	/*
> >  	 * vfio_pci_core_disable() frees all dynamic regions on close, so
> register
> >  	 * them here per open rather than at bind. A failed first open never
> > --
> > 2.25.1
> >
> >