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

[Shuai Xue <xueshuai@linux.alibaba.com>]

On 8/13/26 5:36 PM, mhonap@nvidia.com wrote:
> From: Manish Honap <mhonap@nvidia.com>
> 
> 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@nvidia.com>
> ---
>   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