> -----Original Message-----
> From: Alex Williamson <[email protected]>
> Sent: 11 July 2026 03:53
> To: Manish Honap <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Ankit Agrawal <[email protected]>;
> [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]>; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v3 08/11] vfio/pci/cxl: Add HDM + COMP_REGS regions
> and DVSEC clipping shim
>
> External email: Use caution opening links or attachments
>
>
> On Thu, 25 Jun 2026 22:24:04 +0530
> <[email protected]> wrote:
> > diff --git a/drivers/vfio/pci/vfio_pci_config.c
> > b/drivers/vfio/pci/vfio_pci_config.c
> > index a10ed733f0e3..b9f30a33515a 100644
> > --- a/drivers/vfio/pci/vfio_pci_config.c
> > +++ b/drivers/vfio/pci/vfio_pci_config.c
> > @@ -1898,8 +1898,15 @@ ssize_t vfio_pci_config_rw_single(struct
> vfio_pci_core_device *vdev,
> > /*
> > * Chop accesses into aligned chunks containing no more than a
> > * single capability. Caller increments to the next chunk.
> > + *
> > + * For CXL Type-2 devices also clip at the CXL Device DVSEC body
> > + * boundary so the generic perm-bits path handles the DVSEC
> > + * header bytes and the CXL hook handles the body bytes; without
> > + * this clip a 32-bit access at dvsec + 0x08 would span the
> > + * generic Header2 word and the CXL CAPABILITY word.
> > */
> > count = min(count, vfio_pci_cap_remaining_dword(vdev, *ppos));
> > + count = min(count, vfio_pci_cxl_config_boundary(vdev, *ppos));
> > if (count >= 4 && !(*ppos % 4))
> > count = 4;
> > else if (count >= 2 && !(*ppos % 2)) @@ -1909,6 +1916,30 @@
> > ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
> >
> > ret = count;
> >
> > + /*
> > + * Give the CXL Type-2 hook first claim on this access: if the
> > + * range lies inside the CXL Device DVSEC body, forward it to
> > + * cxl-core's register-virtualization helpers instead of the
> > + * standard perm-bits path. -ENOENT means "not for me; use the
> > + * default path"; any other negative value is a hard error.
> > + */
> > + if (vdev->cxl) {
> > + __le32 le_val = 0;
> > + ssize_t cxl_ret;
> > +
> > + if (iswrite && copy_from_user(&le_val, buf, count))
> > + return -EFAULT;
> > + cxl_ret = vfio_pci_cxl_config_rw(vdev, *ppos, count,
> &le_val,
> > + iswrite);
> > + if (cxl_ret >= 0) {
> > + if (!iswrite && copy_to_user(buf, &le_val,
> count))
> > + return -EFAULT;
> > + return cxl_ret;
> > + }
> > + if (cxl_ret != -ENOENT)
> > + return cxl_ret;
> > + }
> > +
>
> I think the solution here is just to set the .readfn and .writefn for
> PCI_EXT_CAP_ID_DVSEC to dvsec specific handlers, rather than the raw
> write and direct read handlers. The new handlers would detect whether
> the reference is to the CXL DVSEC body, possibly via ranges stored in
> vdev->cxl, and either call through to CXL handlers via cxl_ops
> (previously suggested), or fall through to the raw/direct handlers.
okay, I will have handling for this in v4.
>
> > cap_id = vdev->pci_config_map[*ppos];
> >
> > if (cap_id == PCI_CAP_ID_INVALID) { diff --git
> > a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> > index 05ab4ae59157..2d2dae278d1e 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -501,6 +501,23 @@ static void vfio_pci_core_map_bars(struct
> vfio_pci_core_device *vdev)
> > if (!pci_resource_len(pdev, i))
> > continue;
> >
> > + /*
> > + * cxl-core already holds request_mem_region() on the
> CXL
> > + * component register sub-range of this BAR. Skip the
> > + * full-BAR request so we do not collide with that
> > + * sub-region; vfio still owns the BAR via the driver
> > + * binding and the iomap below succeeds without a region
> > + * claim.
> > + */
> > + if (vdev->cxl && bar ==
> vfio_pci_cxl_get_component_reg_bar(vdev)) {
> > + vdev->barmap[bar] = pci_iomap(pdev, bar, 0);
> > + if (!vdev->barmap[bar]) {
> > + pci_dbg(pdev, "Failed to iomap region
> %d\n", bar);
> > + vdev->barmap[bar] = IOMEM_ERR_PTR(-
> ENOMEM);
> > + }
> > + continue;
> > + }
> > +
> > if (pci_request_selected_regions(pdev, 1 << bar,
> "vfio")) {
> > pci_dbg(pdev, "Failed to reserve region %d\n",
> bar);
> > vdev->barmap[bar] = IOMEM_ERR_PTR(-EBUSY); @@
> > -701,7 +718,10 @@ void vfio_pci_core_disable(struct
> vfio_pci_core_device *vdev)
> > if (IS_ERR_OR_NULL(vdev->barmap[bar]))
> > continue;
> > pci_iounmap(pdev, vdev->barmap[bar]);
> > - pci_release_selected_regions(pdev, 1 << bar);
> > + /* Mirror the asymmetric setup-time skip in map_bars().
> */
> > + if (!(vdev->cxl &&
> > + i == vfio_pci_cxl_get_component_reg_bar(vdev)))
> > + pci_release_selected_regions(pdev, 1 << bar);
>
> It would be much less ugly to create
> vfio_pci_{request,release}_selected_region() wrappers that mask whether
> the region is actually requested or released than to disrupt the code
> flow like this. Likewise below, think about creating wrappers that do
> the right thing for cxl and are no-ops otherwise. For example, embed
> the vdev->cxl test into the function to cleanup the callers. Thanks,
>
Agreed. I will add helper routines for this as suggested.
> Alex
>
> > vdev->barmap[bar] = NULL;
> > }
> >
> > @@ -1051,6 +1071,16 @@ static int vfio_pci_ioctl_get_info(struct
> vfio_pci_core_device *vdev,
> > info.num_regions = VFIO_PCI_NUM_REGIONS + vdev->num_regions;
> > info.num_irqs = VFIO_PCI_NUM_IRQS;
> >
> > + if (vdev->cxl) {
> > + ret = vfio_pci_cxl_get_info(vdev, &caps);
> > + if (ret) {
> > + pci_warn(vdev->pdev,
> > + "Failed to add CXL info capability\n");
> > + return ret;
> > + }
> > + info.flags |= VFIO_DEVICE_FLAGS_CXL;
> > + }
> > +
> > ret = vfio_pci_info_zdev_add_caps(vdev, &caps);
> > if (ret && ret != -ENODEV) {
> > pci_warn(vdev->pdev,
> > @@ -1093,6 +1123,12 @@ int vfio_pci_ioctl_get_region_info(struct
> vfio_device *core_vdev,
> > struct pci_dev *pdev = vdev->pdev;
> > int i, ret;
> >
> > + if (vdev->cxl) {
> > + ret = vfio_pci_cxl_get_region_info(vdev, info, caps);
> > + if (ret != -ENOTTY)
> > + return ret;
> > + }
> > +
> > switch (info->index) {
> > case VFIO_PCI_CONFIG_REGION_INDEX:
> > info->offset = VFIO_PCI_INDEX_TO_OFFSET(info->index);
