> -----Original Message----- > From: Shuai Xue <[email protected]> > Sent: Thursday, September 3, 2026 5:58 PM > To: Manish Honap <[email protected]>; [email protected]; [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] > Cc: 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] > 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, [email protected] wrote: > > From: Manish Honap <[email protected]> > > > > 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 <[email protected]> > > --- > > 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
