> -----Original Message-----
> From: Alex Williamson <[email protected]>
> Sent: Friday, August 28, 2026 8:40 PM
> 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 v4 16/27] vfio/cxl: Shadow the CXL DVSEC body at open
>
> External email: Use caution opening links or attachments
>
>
> On Thu, 13 Aug 2026 15:06:20 +0530
> <[email protected]> wrote:
>
> > From: Manish Honap <[email protected]>
> >
> > Sample the CXL DVSEC body into a per-open shadow when the guest opens
> > the device, and free it at close. Reading it here rather than at bind
> > picks up any change from a low-power transition, and gives the DVSEC
> > access handler added next a per-tenant copy to serve from.
> >
> > Annotate the shadow with __counted_by_ptr(dvsec_dwords) so its
> > accesses are bounds-checked against the recorded dword count.
>
> It would be useful to describe why we want to shadow the DVSEC capability
> here. Also, it's the whole DVSEC capability, not just the body. We're again
> mentioning that low power transition that the earlier path prevented (but
> shouldn't have).
>
> >
> > Signed-off-by: Manish Honap <[email protected]>
> > ---
> > drivers/vfio/pci/cxl/vfio_cxl_core.c | 39
> > ++++++++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> >
> > diff --git a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > index d19fd638f538..2e516a0929c6 100644
> > --- a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > +++ b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > @@ -8,6 +8,7 @@
> > #include <linux/module.h>
> > #include <linux/pci.h>
> > #include <linux/range.h>
> > +#include <linux/slab.h>
> > #include <linux/vfio_pci_core.h>
> > #include <cxl/cxl.h>
> > #include <cxl/pci.h>
> > @@ -17,11 +18,19 @@
> > * @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
> > + * @dvsec: CXL device DVSEC config-space offset
> > + * @dvsec_len: length of the DVSEC body
>
> Nit, not just the DVSEC body.
>
> > + * @dvsec_dwords: dword count of @dvsec_shadow
> > + * @dvsec_shadow: guest view of the CXL DVSEC body, sampled at open
> > */
> > struct vfio_cxl_state {
> > struct cxl_dev_state cxlds;
> > struct cxl_memdev *cxlmd;
> > struct range hpa_range;
> > + u16 dvsec;
> > + u32 dvsec_len;
> > + u32 dvsec_dwords;
> > + u32 *dvsec_shadow __counted_by_ptr(dvsec_dwords);
>
> dvsec_len is bound by PCI_DVSEC_HEADER1_LEN, which is 12-bits, so it fits
> comfortably in a u16. dvsec_dwords is therefore bound at 10-bits.
> Both of these fit comfortably in u16. I'd argue that length is easily
> derived from
> dwords, but we end up with a hole in the data structure regardless, so it's
> arguably useful to keep both. We can drop 4-bytes from the structure though,
> which actually turns into an 8-byte savings with the two holes above (2 + 4)
> vs
> one hole below (2):
>
> u16 dvsec;
> u16 dvsec_len;
> u16 dvsec_dwords;
> u32 *dvsec_shadow __counted_by_ptr(dvsec_dwords);
>
> > };
> >
> > static int vfio_cxl_init_device(struct vfio_pci_core_device *vdev) @@
> > -71,6 +80,8 @@ static int vfio_cxl_init_device(struct vfio_pci_core_device
> *vdev)
> > if (!cxl)
> > return -ENOMEM;
> >
> > + cxl->dvsec = dvsec;
> > +
> > /*
> > * vfio-pci requests the whole component BAR when the guest opens the
> > * device. Declare the BAR owned so the CXL core maps the
> > HDM/RAS @@ -102,11 +113,39 @@ static void
> > vfio_cxl_release_device(struct vfio_pci_core_device *vdev)
> >
> > 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;
> > + u32 hdr, *shadow;
> > + int i, dwords;
> > +
> > + /*
> > + * Sample the DVSEC body now rather than at bind: a low-power
> > + * transition could have changed it since the device was bound.
> > + */
> > + pci_read_config_dword(pdev, cxl->dvsec + PCI_DVSEC_HEADER1, &hdr);
> > + cxl->dvsec_len = PCI_DVSEC_HEADER1_LEN(hdr);
> > + dwords = cxl->dvsec_len / sizeof(u32);
> > +
> > + shadow = kcalloc(dwords, sizeof(u32), GFP_KERNEL);
> > + if (!shadow)
> > + return -ENOMEM;
>
> dvsec_len becomes inconsistent with the other fields if we take this return.
>
> > +
> > + for (i = 0; i < dwords; i++)
> > + pci_read_config_dword(pdev, cxl->dvsec + i * sizeof(u32),
> > + &shadow[i]);
> > +
> > + cxl->dvsec_dwords = dwords;
> > + cxl->dvsec_shadow = shadow;
> > +
> > return 0;
> > }
> >
> > static void vfio_cxl_close_device(struct vfio_pci_core_device *vdev)
> > {
> > + struct vfio_cxl_state *cxl = vdev->cxl;
> > +
> > + kfree(cxl->dvsec_shadow);
> > + cxl->dvsec_shadow = NULL;
>
> This makes the counted-by field inconsistent. All fields should be cleared.
> Thanks,
>
> Alex
With the DVSEC handled in vfio_pci_config.c on top of vconfig (patch 17), there
is no
separate shadow to set up. I will assess whether I can drop this patch entirely.
Manish
>
> > }
> >
> > static const struct vfio_cxl_ops vfio_cxl_ops = {
