Re: [PATCH v5 11/27] vfio/pci: Virtualize the CXL DVSEC in vfio_pci_config.c

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 17 Sep 2026 00:05:24 +0530
<mhonap@nvidia.com> wrote:

> From: Manish Honap <mhonap@nvidia.com>
> 
> A CXL Type-2 device is reprogrammable through its CXL DVSEC: a guest could
> set Config Lock, toggle CXL.cache and CXL.mem enable, or rewrite the HDM
> range registers that govern host memory decode. Virtualize the DVSEC so
> the guest sees a shadow it cannot use to reprogram the hardware.
> 
> Build a per-device cxl_perm permission map, modeled on msi_perm, when the
> device is bound through the CXL provider (e.g. vfio-cxl). The whole CXL
> DVSEC is served from the vconfig shadow; only Control and Control2 are
> guest programmable, while Capability, Status, Lock and the Range registers
> keep their firmware snapshot. A vendor DVSEC on the same device is
> unaffected: the map is selected only for the CXL DVSEC offset.
> 
> Control2 carries the CXL reset and cache write-back-invalidate initiate
> bits as self-clearing doorbells. vfio never forwards them to hardware, so
> a custom writefn synthesizes their completion in the shadow: the initiate
> bit self-clears and the matching Status2 bit (Cache Invalid or Reset Done)
> is set, so a guest following the spec reset sequence
> (INIT_CACHE_WBI, poll Cache Invalid, INIT_CXL_RST, poll Reset Done)
> progresses instead of timing out. The host performs the real cache
> write-back and reset at the vfio reset points.
> 
> Assisted-by: LLM
> Signed-off-by: Manish Honap <mhonap@nvidia.com>
A few minor comments inline.

J
> ---
>  drivers/vfio/pci/vfio_pci_config.c | 132 ++++++++++++++++++++++++++++-
>  include/linux/vfio_pci_core.h      |   3 +
>  2 files changed, 133 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index 9914f3ac69ae..9a020a768055 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c

> +
> +static int init_cxl_dvsec_perm(struct perm_bits *perm, int len)
> +{
> +	int i;
> +
> +	if (alloc_perm_bits(perm, len))
> +		return -ENOMEM;
> +
> +	perm->writefn = vfio_cxl_dvsec_write;
> +
> +	/* Serve the whole CXL DVSEC from the shadow. */
> +	for (i = 0; i < len; i++)
	for (int i = 0;

Is mostly acceptable in the kernel these days and keeps
the scope tightly defined.

> +		p_setb(perm, i, (u8)ALL_VIRT, NO_WRITE);
> +
> +	/*
> +	 * Control and Control2 are guest programmable; Capability, Status,
> +	 * Lock and the Range registers keep their firmware snapshot, so the
> +	 * guest cannot set Config Lock or rewrite the capability and ranges.
> +	 */
> +	p_setw(perm, PCI_DVSEC_CXL_CTRL, (u16)ALL_VIRT, (u16)ALL_WRITE);
> +	p_setw(perm, PCI_DVSEC_CXL_CTRL2, (u16)ALL_VIRT, (u16)ALL_WRITE);
> +
> +	return 0;
> +}
> +
> +/* Virtualize the CXL DVSEC so a guest cannot reprogram the device through it. */
> +static int vfio_cxl_dvsec_init(struct vfio_pci_core_device *vdev)
> +{
> +	struct pci_dev *pdev = vdev->pdev;
> +	u32 dword;
> +	u16 dvsec;
> +	int len, ret;
> +
> +	dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> +					  PCI_DVSEC_CXL_DEVICE);
> +	if (!dvsec)
> +		return 0;
> +
> +	ret = pci_read_config_dword(pdev, dvsec + PCI_DVSEC_HEADER1, &dword);
> +	if (ret)
> +		return pcibios_err_to_errno(ret);
> +	len = PCI_DVSEC_HEADER1_LEN(dword);
> +
> +	/*
> +	 * The virtualization writes fixed DVSEC offsets up to Status2 (the reset
> +	 * doorbell stamps it). A device that reports a shorter DVSEC is not a
> +	 * usable Type-2 function; leave it as plain vfio-pci rather than index the
> +	 * device-length-sized perm allocation past its end.
> +	 */
> +	if (len < PCI_DVSEC_CXL_STATUS2 + 2)
> +		return 0;
> +
> +	vdev->cxl_perm = kmalloc_obj(struct perm_bits, GFP_KERNEL_ACCOUNT);

I'd use *vdev->cxl_perm instead of struct perm_bits just because
that saves anyone checking types.

> +	if (!vdev->cxl_perm)
> +		return -ENOMEM;
> +
> +	ret = init_cxl_dvsec_perm(vdev->cxl_perm, len);
> +	if (ret) {
> +		kfree(vdev->cxl_perm);
> +		vdev->cxl_perm = NULL;
> +		return ret;
> +	}
> +
> +	vdev->cxl_dvsec = dvsec;
> +	vdev->cxl_dvsec_len = len;
> +
> +	return 0;
> +}
> +
>  int vfio_config_init(struct vfio_pci_core_device *vdev)
>  {
>  	struct pci_dev *pdev = vdev->pdev;
> @@ -1842,6 +1955,12 @@ int vfio_config_init(struct vfio_pci_core_device *vdev)
>  	if (ret)
>  		goto out;
>  
> +	if (vdev->cxl_ops) {
> +		ret = vfio_cxl_dvsec_init(vdev);
> +		if (ret)
> +			goto out;
> +	}
> +
>  	return 0;
>  
>  out:
> @@ -1863,6 +1982,12 @@ void vfio_config_free(struct vfio_pci_core_device *vdev)
>  		kfree(vdev->msi_perm);
>  		vdev->msi_perm = NULL;
>  	}
> +	if (vdev->cxl_perm) {
> +		free_perm_bits(vdev->cxl_perm);
> +		kfree(vdev->cxl_perm);
> +		vdev->cxl_perm = NULL;
> +		vdev->cxl_dvsec = 0;

I'd define a vfio_cxl_dvsec_exit() or _unint() for this so
it is clear it pairs with vfio_cxl_dvsec_init() above.


> +	}
>  }
>  
>  /*
> @@ -1926,12 +2051,15 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
>  			 * of the extended capability list.  Use default, ro
>  			 * access, which will virtualize the id and next values.
>  			 */
> +			cap_start = vfio_find_cap_start(vdev, *ppos);
> +
>  			if (cap_id > PCI_EXT_CAP_ID_MAX)
>  				perm = &direct_ro_perms;
> +			else if (cap_id == PCI_EXT_CAP_ID_DVSEC && vdev->cxl_perm &&
> +				 cap_start == vdev->cxl_dvsec)

If it is only used here (I haven't read on in series)
				 vfio_find_cap_start(vdev, *ppos) == vdev->cxl_dvsec)
seems fine to me.  It's only just over 80 chars and hopeful this bit of the kernel
is flexible on that!

> +				perm = vdev->cxl_perm;
>  			else
>  				perm = &ecap_perms[cap_id];
> -
> -			cap_start = vfio_find_cap_start(vdev, *ppos);
>  		} else {
>  			WARN_ON(cap_id > PCI_CAP_ID_MAX);
>