RE: [PATCH v4 17/27] vfio/cxl: Virtualize the CXL DVSEC

[Manish Honap]

> -----Original Message-----
> From: Shuai Xue <xueshuai@xxxxxxxxxxxxxxxxx>
> Sent: Thursday, September 3, 2026 5:58 PM
> To: Manish Honap <mhonap@xxxxxxxxxx>; 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
> Cc: 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 v4 17/27] vfio/cxl: Virtualize the CXL DVSEC
> 
> External email: Use caution opening links or attachments
> 
> 
> On 8/13/26 5:36 PM, mhonap@xxxxxxxxxx wrote:
> > From: Manish Honap <mhonap@xxxxxxxxxx>
> >
> > Serve reads of the CXL DVSEC body from the per-open shadow and keep
> > guest writes in the shadow rather than letting them reach the
> > hardware, so a guest cannot reprogram the device through the DVSEC.
> > Accesses outside the CXL DVSEC return -ENODEV and take the default
> > DVSEC handling, so a device that also exposes a vendor DVSEC is unaffected.
> >
> > Route each shadow write through the CXL r4.0 field class rather than
> > storing it verbatim: Control stays programmable, Status is
> > write-1-to-clear, and Capability, Lock and the Range registers keep
> > their firmware snapshot. The guest can no longer set Config Lock or
> > scribble the capability and range fields.
> >
> > Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
> > ---
> >   drivers/vfio/pci/cxl/vfio_cxl_core.c | 88
> ++++++++++++++++++++++++++++
> >   drivers/vfio/pci/vfio_pci_config.c   | 36 +++++++++++-
> >   include/linux/vfio_pci_core.h        |  5 ++
> >   include/uapi/linux/pci_regs.h        |  1 +
> >   4 files changed, 129 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > index 2e516a0929c6..9fed909cb9d3 100644
> > --- a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > +++ b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > @@ -148,11 +148,99 @@ static void vfio_cxl_close_device(struct
> vfio_pci_core_device *vdev)
> >       cxl->dvsec_shadow = NULL;
> >   }
> >
> > +/* Read a 16-bit DVSEC field from the shadow; @off is DVSEC-relative.
> > +*/ static u16 vfio_cxl_dvsec16(struct vfio_cxl_state *cxl, u32 off) {
> > +     u32 dw = cxl->dvsec_shadow[off / sizeof(u32)];
> > +
> > +     return (dw >> (8 * (off % sizeof(u32)))) & 0xffff; }
> > +
> > +/*
> > + * Apply the CXL r4.0 8.1.3 write class for the 16-bit DVSEC register at @off.
> > + * Control is programmable, Status is write-1-to-clear, and
> > +Capability, Lock and
> > + * the Range registers stay fixed at their firmware snapshot.
> > + */
> > +static u16 vfio_cxl_dvsec_field(u32 off, u16 old, u16 wval, u16
> > +wmask) {
> > +     switch (off) {
> > +     case PCI_DVSEC_CXL_CTRL:
> > +             /*
> > +              * CXL.mem stays enabled for as long as the guest owns the device.
> > +              * The HDM decoder maps the guest window to device memory, so a
> > +              * store to it while CXL.mem is disabled completes on the device as
> > +              * an error that the host fabric reports as an SError, which is
> > +              * fatal. The spec does not pin down accesses to a decoder whose
> > +              * CXL.mem is off and many hosts SError, so ignore a guest request
> > +              * to clear the enable and keep the bit set.
> > +              */
> > +             return ((old & ~wmask) | (wval & wmask)) |
> PCI_DVSEC_CXL_MEM_ENABLE;
> > +     case PCI_DVSEC_CXL_CTRL2:
> > +             return (old & ~wmask) | (wval & wmask);
> 
> 
> Control2 is routed through the r4.0 write class as plain RW, but both INITIATE
> bits are self-clearing doorbells per spec: the guest sets the bit, the device
> performs the operation and clears it, and completion is observed in STATUS2
> (Cache_Invalid for the WBI). With the plain-RW class the shadow latches the
> bit at 1 forever, and because reads are served from the open-time snapshot,
> Cache_Invalid
> (CXL_DVSEC_STATUS2_CACHE_INVALID) never changes either.
> Both polling paths are dead ends.
> 
> The interesting part is that the series already models this correctly for the
> sibling command bit -- the vfio_cxl_reset() epilogue does:
> 
>         /*
>          * The guest-facing DVSEC bookkeeping only applies while the device
>          * is open. Initiate_CXL_Reset self-clears in hardware; mirror that
>          * and stamp the outcome onto a fresh hardware STATUS2 read for the
>          * polling guest.
>          */
> 
> INIT_CXL_RST is self-cleared in the shadow and the outcome is stamped into
> STATUS2 for the polling guest. Initiate_Cache_WBI just never gets the same
> treatment.
> 
> The polling contract is one this series itself implements on the host
> side: cxl_reset_wait_cache_wbi() sets WBI, then polls STATUS2 Cache_Invalid
> with a 100ms budget. A guest kernel running the same sequence would set
> WBI, poll the frozen snapshot, and time out before ever reaching
> INIT_CXL_RST, so whether a guest-initiated CXL reset succeeds depends on the
> open-time STATUS2 value rather than on device state. The stuck command bit
> is also directly visible to any guest reading CTRL2 back.
> 
> Would the minimal fix be to mirror the epilogue's INIT_CXL_RST handling for
> WBI: self-clear the bit on the 0->1 write and set Cache_Invalid in the shadow
> (the host already runs the real WBI at VM power on/off via
> cxl_reset_dvsec_sequence(), and the existing W1C class on STATUS2 lets the
> guest clear Cache_Invalid before the next round)? Or is forwarding the WBI to
> hardware and keeping Cache_Invalid live the intended long-term model?
> 
> Thanks.
> Shuai

Your minimal fix is the intended model, and it cleanly fits the
vfio_pci_config.c rework which Alex suggested.  Control2 becomes a cxl_perms
write handler (the way msi_perms uses vfio_msi_config_write), and on the guest
0->1 write it self-clears the Initiate bit and stamps STATUS2.Cache_Invalid in
vconfig as a side effect, the same self-clear-and-stamp the reset path applies
to Initiate_CXL_Reset. (WBI will not be forwarded to hardware. The real
write-back and invalidate runs host-side in cxl_reset_dvsec_sequence() at the
reset boundary)

Thanks,
Manish