Re: [PATCH v16 17/18] PCI/CXL: Restore CXL state after CXL bus reset

[Richard Cheng <icheng@nvidia.com>]

On Thu, Oct 29, 2026 at 02:57:02PM +0800, Srirangan Madhavan wrote:
> CXL bus reset can clear HDM decoder programming and CXL Device DVSEC
> control state. Restore the cached state after a successful cxl_bus reset
> while IOMMU exclusion remains active.
> 
> Restore PCI configuration first so HDM MMIO is accessible, and preserve a
> disabled state if restoration fails.
> 
> Copy the cached state before the reset, then refresh its decoder settings
> from the cache under the CXL region quiesce once the reset completes. A
> region decommit during the reset updates the cache, and restoring the
> earlier decoders would leave the endpoint decoding an HPA range that the
> CXL core considers free. Keep global and DVSEC control from before the
> reset, since a refresh during the reset can read them as all-ones.
> 
> The prepare/restore interface and decoder-refresh structure follow Dave
> Jiang's cxl-type2-reset reference branch.
> 
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com> # pci/pci.c
> Assisted-by: LLM
> 
> ---
> Reference branch: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/commit/?id=b93f0c8e67b198a63786fd3721b7177ab9df60be
> ---
>  drivers/cxl/core/hdm_state.c | 31 ++++++++++++++++
>  drivers/pci/cxl.c            | 68 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.c            |  8 +++++
>  drivers/pci/pci.h            | 16 +++++++++
>  include/cxl/hdm.h            |  2 ++
>  5 files changed, 125 insertions(+)
> 
> diff --git a/drivers/cxl/core/hdm_state.c b/drivers/cxl/core/hdm_state.c
> index cb7ec0f86ab1..6b91721dbcd8 100644
> --- a/drivers/cxl/core/hdm_state.c
> +++ b/drivers/cxl/core/hdm_state.c
> @@ -188,6 +188,37 @@ struct cxl_hdm_info *cxl_hdm_cache_snapshot(struct cxl_hdm_info *const *slot)
>  	return copy;
>  }
>  
> +/**
> + * cxl_hdm_cache_copy_decoders() - refresh the decoders of a snapshot
> + * @slot: cache pointer owned by the device
> + * @dst: snapshot of the same device from cxl_hdm_cache_snapshot()
> + *
> + * Copy only the per-decoder settings, which region commit and teardown keep
> + * current. Leave the rest of @dst, including global and DVSEC control, as
> + * captured: a refresh during a reset may have read those registers from a
> + * device that returned all-ones.
> + *
> + * Return: 0 on success, -ENXIO when nothing is published, or -EINVAL when
> + * @dst holds a different number of decoders.
> + */
> +int cxl_hdm_cache_copy_decoders(struct cxl_hdm_info *const *slot,
> +				struct cxl_hdm_info *dst)
> +{
> +	struct cxl_hdm_info *info;
> +
> +	lockdep_assert_held_write(&cxl_rwsem.region);
> +	guard(rwsem_read)(&cxl_rwsem.dpa);
> +	info = *slot;
> +	if (!info)
> +		return -ENXIO;
> +	if (info->decoder_count != dst->decoder_count)
> +		return -EINVAL;
> +
> +	memcpy(dst->settings, info->settings,
> +	       flex_array_size(info, settings, info->decoder_count));
> +	return 0;
> +}
> +
>  void cxl_region_quiesce_lock(void)
>  	__acquires(cxl_region_quiesce)
>  	__context_unsafe(token maps to cxl_rwsem.region)
> diff --git a/drivers/pci/cxl.c b/drivers/pci/cxl.c
> index 0c4ac0832b34..5aa97705a669 100644
> --- a/drivers/pci/cxl.c
> +++ b/drivers/pci/cxl.c
> @@ -389,6 +389,74 @@ static int pci_cxl_restore_state(struct pci_dev *pdev,
>  	return cxl_restore_state(pdev, snapshot);
>  }
>  
> +/**
> + * pci_cxl_reset_prepare() - allocate HDM state storage ahead of a bus reset
> + * @pdev: device about to be reset
> + *
> + * Copy the cached HDM state before the reset, so that restoring cannot fail
> + * for lack of memory. The decoder settings are refreshed when the state is
> + * restored.
> + *
> + * Return: storage for cxl_restore_state_after_pci_reset(), to be freed with
> + * kfree(); NULL when @pdev has no cached HDM state; or an ERR_PTR() on
> + * failure.
> + */
> +struct cxl_hdm_info *pci_cxl_reset_prepare(struct pci_dev *pdev)
> +{
> +	struct cxl_hdm_info *snapshot;
> +
> +	device_lock_assert(&pdev->dev);
> +	snapshot = cxl_hdm_cache_snapshot(&pdev->hdm);
> +	if (IS_ERR(snapshot))
> +		return PTR_ERR(snapshot) == -ENXIO ? NULL : snapshot;
> +
> +	/* Raw reset callers may not have saved the PCI state needed for restore. */
> +	if (!pdev->state_saved) {
> +		pci_err(pdev, "CXL bus reset requires saved PCI state\n");
> +		kfree(snapshot);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return snapshot;
> +}
> +
> +/**
> + * cxl_restore_state_after_pci_reset() - restore CXL state after a bus reset
> + * @pdev: device that was reset
> + * @snapshot: storage from pci_cxl_reset_prepare()
> + *
> + * Under the region quiesce, refresh the decoder settings in @snapshot from the
> + * cache and restore: first the PCI configuration needed to reach HDM MMIO,
> + * then the HDM decoder and CXL Device DVSEC state. Leave @pdev disabled on
> + * failure.
> + *
> + * Refresh the decoders now rather than rely on the copy taken before the
> + * reset: a region commit or teardown during the reset has updated the cache,
> + * and restoring older decoders would leave the device decoding ranges the
> + * CXL core considers free.
> + */
> +int cxl_restore_state_after_pci_reset(struct pci_dev *pdev,
> +				      struct cxl_hdm_info *snapshot)
> +{
> +	u16 command;
> +	int rc;
> +
> +	device_lock_assert(&pdev->dev);
> +	cxl_region_quiesce_lock();
> +
> +	rc = cxl_hdm_cache_copy_decoders(&pdev->hdm, snapshot);
> +	if (!rc) {
> +		rc = pci_cxl_restore_state(pdev, snapshot, &command);
> +	}

Hi Srirangan,

Looks like BME is going to be restored here?
The caller haven't unblock IOMMU, if we enable BME first the requesst will hit the
blocking domain.

I think we should enable BME after IOMMU restoration.

Best regards,
Richard Cheng.


> +	if (rc)
> +		cxl_reset_save_disabled_state(pdev);
> +	else
> +		rc = cxl_reset_save_restored_state(pdev, command);
> +
> +	cxl_region_quiesce_unlock();
> +	return rc;
> +}
> +
>  /*
>   * CXL r4.0 sec 9.7.2 defines the reset completion timeout encodings.
>   * Sec 9.7.3 leaves config-space access behavior undefined for 100 ms after
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index f5990d6cc51b..22f740417673 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5014,6 +5014,7 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
>  
>  static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
>  {
> +	struct cxl_hdm_info *snapshot __free(kfree) = NULL;
>  	struct pci_dev *bridge;
>  	u16 dvsec, reg, val;
>  	int rc;
> @@ -5036,6 +5037,10 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
>  	if (rc)
>  		return -ENOTTY;
>  
> +	snapshot = pci_cxl_reset_prepare(dev);
> +	if (IS_ERR(snapshot))
> +		return PTR_ERR(snapshot);
> +
>  	rc = pci_dev_reset_iommu_prepare(dev);
>  	if (rc) {
>  		pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", rc);
> @@ -5056,6 +5061,9 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
>  		pci_write_config_word(bridge, dvsec + PCI_DVSEC_CXL_PORT_CTL,
>  				      reg);
>  
> +	if (!rc && snapshot)
> +		rc = cxl_restore_state_after_pci_reset(dev, snapshot);
> +
>  	pci_dev_reset_iommu_done(dev);
>  	return rc;
>  }
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 743e65abd758..476e8eefafb2 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -730,10 +730,14 @@ static inline void pci_doe_destroy(struct pci_dev *pdev) { }
>  static inline void pci_doe_disconnected(struct pci_dev *pdev) { }
>  #endif
>  
> +struct cxl_hdm_info;
>  #ifdef CONFIG_CXL_RESET
>  void pci_cxl_hdm_cache_init(struct pci_dev *pdev);
>  void pci_cxl_hdm_cache_release(struct pci_dev *pdev);
>  int cxl_reset_function(struct pci_dev *pdev, bool probe);
> +struct cxl_hdm_info *pci_cxl_reset_prepare(struct pci_dev *pdev);
> +int cxl_restore_state_after_pci_reset(struct pci_dev *pdev,
> +				      struct cxl_hdm_info *snapshot);
>  #else
>  static inline void pci_cxl_hdm_cache_init(struct pci_dev *pdev) { }
>  static inline void pci_cxl_hdm_cache_release(struct pci_dev *pdev) { }
> @@ -741,6 +745,18 @@ static inline int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  {
>  	return -ENOTTY;
>  }
> +
> +static inline struct cxl_hdm_info *pci_cxl_reset_prepare(struct pci_dev *pdev)
> +{
> +	return NULL;
> +}
> +
> +static inline int
> +cxl_restore_state_after_pci_reset(struct pci_dev *pdev,
> +				  struct cxl_hdm_info *snapshot)
> +{
> +	return 0;
> +}
>  #endif
>  
>  #ifdef CONFIG_PCI_NPEM
> diff --git a/include/cxl/hdm.h b/include/cxl/hdm.h
> index 80d6ebb9f4aa..090eea2c0b46 100644
> --- a/include/cxl/hdm.h
> +++ b/include/cxl/hdm.h
> @@ -66,6 +66,8 @@ void cxl_hdm_cache_publish(struct cxl_hdm_info **slot,
>  void cxl_hdm_cache_release(struct cxl_hdm_info **slot);
>  bool cxl_hdm_cache_dvsec_decode(struct cxl_hdm_info *const *slot);
>  struct cxl_hdm_info *cxl_hdm_cache_snapshot(struct cxl_hdm_info *const *slot);
> +int cxl_hdm_cache_copy_decoders(struct cxl_hdm_info *const *slot,
> +				struct cxl_hdm_info *dst);
>  
>  /*
>   * Region quiesce blocks CXL region commit and teardown, and with them decoder
> -- 
> 2.43.0
>