Re: [PATCH v3 07/11] vfio/pci: Add CONFIG_VFIO_PCI_CXL with bind-time CXL Type-2 acquisition

[Alex Williamson <alex@shazbot.org>]

On Thu, 25 Jun 2026 22:24:03 +0530
<mhonap@nvidia.com> wrote:

> From: Manish Honap <mhonap@nvidia.com>
> 
> Wire vfio-pci-core to acquire CXL Type-2 device state at PCI bind
> and release it at PCI unbind, mirroring the existing vfio_pci_zdev_*
> integration model.

I don't follow, zpci has the following matching hooks:

 - vfio_pci_zdev_open_device
 - vfio_pci_zdev_close_device

Plus it also has:

 - vfio_pci_info_zdev_add_caps

What about those suggest "zdev state" is acquired at bind and released
on unbind?

>  Four lifecycle hooks are introduced —
> vfio_pci_cxl_acquire / _release / _open / _close — with !-config
> stubs that return -ENODEV / 0 / 0 / no-op respectively so vfio-pci
> behaviour is unchanged when CONFIG_VFIO_PCI_CXL=n.
> 
> vfio_pci_cxl_acquire() implements the bind sequence:
> 
>   - pcie_is_cxl() and CXL Device DVSEC discovery (-ENODEV if absent
>     or if MEM_CAPABLE clear — caller falls back to plain vfio-pci)
>   - devm_cxl_dev_state_create() with struct vfio_pci_cxl_state
>     embedding cxl_dev_state at offset 0 (required by the 7-arg
>     macro's static_assert in include/cxl/cxl.h)
>   - pci_enable_device_mem(), cxl_pci_setup_regs(), cxl_get_hdm_info()
>     (rejecting hdm_count != 1), cxl_regblock_get_bar_info(),
>     cxl_await_range_active()

The cover letter claims otherwise:

  "- cxl_await_range_active stays in cxl-core probe; not exported, vfio
  does not call it."

It's exported in 2/ and called below.

>   - devm_cxl_passthrough_create() to snapshot the DVSEC body, HDM
>     block, and CM cap-array shadows owned by cxl-core
>   - pci_disable_device() — clears PCI_COMMAND_MASTER but NOT
>     PCI_COMMAND_MEMORY, so cxl-core MMIO accesses from the next step
>     still succeed
>   - devm_cxl_probe_mem() to register the cxl_memdev, enumerate the
>     endpoint port, and attach the firmware-committed autoregion
>   - request_mem_region() + memremap_wb() of the autoregion's HPA so
>     the HDM VFIO region can serve guest accesses through it

How does this interact with:

 - The device making use of low power states while idle
 - Repeatability per tenant instance
 - Protection of tenant data per instance

The culmination of all of these, plus the basic housekeeping of
maintaining the lightest touch on the device, including keeping the
device in the minimum state of functionality outside of an actual user,
is why I would expect to perform acquire/release as part of open/close.

Could a low power transition invalidate the state established by
acquire, leading to the issue Richard encountered?

Also, on the direct calls to cxl functions, I thought one of our goals
was to avoid vfio-pci statically pulling in CXL module dependencies.
To achieve that, it seems like at some point we need to detect that we
have a CXL device (pcie_is_cxl(pdev)), do a request_module() to load
vfio-cxl, where the init function would register callback ops with
vfio-pci-core and each dependent device would acquire a reference to
the vfio-cxl module.

> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> index 89165b769e5c..541c1911e090 100644
> --- a/include/linux/vfio_pci_core.h
> +++ b/include/linux/vfio_pci_core.h
> @@ -142,6 +142,13 @@ struct vfio_pci_core_device {
>  	struct notifier_block	nb;
>  	struct rw_semaphore	memory_lock;
>  	struct list_head	dmabufs;
> +	/*
> +	 * Opaque pointer to struct vfio_pci_cxl_state (defined in
> +	 * drivers/vfio/pci/cxl/vfio_cxl_priv.h).  Set by
> +	 * vfio_pci_cxl_acquire() at PCI bind; NULL on non-CXL devices
> +	 * and when CONFIG_VFIO_PCI_CXL=n.
> +	 */
> +	void			*cxl;

Use a forward declaration rather than void, that avoids half your
comment.  The remainder of the comment is just explaining the obvious
parts of the code, unnecessary.  Thanks,

Alex