Re: [PATCH v15 11/16] cxl: Validate and synchronize HDM ranges around reset

[Alison Schofield <alison.schofield@intel.com>]

On Sun, Oct 11, 2026 at 02:14:17AM +0000, Srirangan Madhavan wrote:
> Refuse reset unless enabled system-physical HDM ranges can be reserved
> exclusively and CPU-cache invalidation is available. Invalidate before
> reset and again before ending IOMMU exclusion, holding range reservations
> until the second invalidation completes. A later patch places state
> restoration before the second invalidation.
> 
> Reject normalized-addressing decoders because their cached ranges are not
> system physical addresses. Ignore zero-size decoders because they map no
> address range. Refuse reset when the device decodes memory through its
> DVSEC ranges instead of HDM decoders, since the cache holds no range to
> reserve for that memory.
> 
> Provide the range handling as a region quiesce in built-in CXL code. The
> quiesce lock blocks region commit and teardown for the whole reset, and
> the HDM state copy taken under it supplies both the reserved ranges and,
> later, the state to restore.
> 
> The range handling and region-quiesce structure follow Dave Jiang's
> reference implementation on his cxl-type2-reset branch.
> 
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/commit/?id=5944c6e61f14c0dd28d54537be15edb2cadb0e8b
> Suggested-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>

Hi Srirangan,

Please revisit the tags and the inclusion of a link to DaveJ's
private branch.

If DaveJ contributed code that you used, then the tags would be
Co-developed-by: DaveJ followed by Signed-off-by: DaveJ.
(Assuming it was non trivial which seems true based on branch)

If DaveJ only suggested you create the patch, ie, you need this
functionality, then it's Suggested-by.

I'm not sure you should include the links in the commit log since
they are to a private git tree. Maybe just include below the line.
Those links are on more patches than just the ones you added
DaveJ as Suggested-by, so very that is odd too.

Anyway, that's for you and DaveJ to sort out :)

-- Alison



> Assisted-by: LLM
> ---
>  drivers/cxl/core/hdm_state.c | 274 ++++++++++++++++++++++++++++++++++-
>  drivers/pci/cxl.c            |  50 ++++++-
>  include/cxl/hdm.h            |  47 +++++-
>  3 files changed, 366 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm_state.c b/drivers/cxl/core/hdm_state.c
> index 9a31f5996c74..cef604013350 100644
> --- a/drivers/cxl/core/hdm_state.c
> +++ b/drivers/cxl/core/hdm_state.c
> @@ -1,7 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright (c) 2026 NVIDIA Corporation & Affiliates */
>  #include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
>  #include <linux/export.h>
> +#include <linux/ioport.h>
> +#include <linux/list.h>
> +#include <linux/memregion.h>
> +#include <linux/overflow.h>
> +#include <linux/pci.h>
> +#include <linux/rwsem.h>
>  #include <linux/slab.h>
>  
>  #include <cxl/hdm.h>
> @@ -10,7 +18,8 @@
>  
>  /*
>   * CXL state that must stay available while no CXL driver is bound: the CXL
> - * locks and the per-device HDM cache filled during PCI enumeration.
> + * locks, the per-device HDM cache filled during PCI enumeration, and the
> + * region quiesce used around a reset.
>   */
>  struct cxl_rwsem cxl_rwsem = {
>  	.region = __RWSEM_INITIALIZER(cxl_rwsem.region),
> @@ -62,3 +71,266 @@ void cxl_hdm_cache_release(struct cxl_hdm_info **slot)
>  	}
>  	kfree(info);
>  }
> +
> +/**
> + * cxl_hdm_cache_dvsec_decode() - check for memory decoded by DVSEC ranges
> + * @slot: cache pointer owned by the device
> + *
> + * Return: true if the published state shows CXL.mem enabled while HDM
> + * decoders are globally disabled, so that the device decodes memory through
> + * its DVSEC ranges, which the cache does not describe.
> + */
> +bool cxl_hdm_cache_dvsec_decode(struct cxl_hdm_info *const *slot)
> +{
> +	struct cxl_hdm_info *info;
> +
> +	guard(rwsem_read)(&cxl_rwsem.dpa);
> +	info = *slot;
> +	return info && (info->dvsec_ctrl & PCI_DVSEC_CXL_MEM_ENABLE) &&
> +	       !(info->global_ctrl & CXL_HDM_DECODER_ENABLE);
> +}
> +
> +/**
> + * cxl_hdm_cache_snapshot() - copy the published HDM state
> + * @slot: cache pointer owned by the device
> + *
> + * Return: a copy the caller frees with kfree(), ERR_PTR(-ENXIO) when nothing
> + * is published, or another ERR_PTR() on failure.
> + */
> +struct cxl_hdm_info *cxl_hdm_cache_snapshot(struct cxl_hdm_info *const *slot)
> +{
> +	struct cxl_hdm_info *info, *copy;
> +	size_t size;
> +
> +	guard(rwsem_read)(&cxl_rwsem.dpa);
> +	info = *slot;
> +	if (!info)
> +		return ERR_PTR(-ENXIO);
> +	if (info->decoder_count <= 0)
> +		return ERR_PTR(-EINVAL);
> +
> +	size = struct_size(info, settings, info->decoder_count);
> +	if (size == SIZE_MAX)
> +		return ERR_PTR(-EOVERFLOW);
> +
> +	copy = kmemdup(info, size, GFP_KERNEL);
> +	if (!copy)
> +		return ERR_PTR(-ENOMEM);
> +
> +	return copy;
> +}
> +
> +void cxl_region_quiesce_lock(void)
> +	__acquires(cxl_region_quiesce)
> +	__context_unsafe(token maps to cxl_rwsem.region)
> +{
> +	down_write(&cxl_rwsem.region);
> +	__acquire(cxl_region_quiesce);
> +}
> +
> +void cxl_region_quiesce_unlock(void)
> +	__releases(cxl_region_quiesce)
> +	__context_unsafe(token maps to cxl_rwsem.region)
> +{
> +	__release(cxl_region_quiesce);
> +	up_write(&cxl_rwsem.region);
> +}
> +
> +/*
> + * Bound the range twice: request_mem_region() takes resource_size_t while
> + * cpu_cache_invalidate_memregion() takes size_t, and the two differ on
> + * 32-bit builds with CONFIG_PHYS_ADDR_T_64BIT. range_len() can also reach
> + * RESOURCE_SIZE_MAX + 1 for a full-width range, and wraps to zero when
> + * resource_size_t is 64-bit, which the !len test catches.
> + */
> +static int cxl_hdm_range_validate(struct device *dev,
> +				  const struct range *hpa_range)
> +{
> +	u64 len = range_len(hpa_range);
> +
> +	if (!len)
> +		return -EINVAL;
> +
> +	if (hpa_range->end > RESOURCE_SIZE_MAX) {
> +		dev_err(dev,
> +			"CXL reset range [%#llx-%#llx] exceeds resource address size\n",
> +			hpa_range->start, hpa_range->end);
> +		return -EOVERFLOW;
> +	}
> +
> +	if (len > RESOURCE_SIZE_MAX) {
> +		dev_err(dev,
> +			"CXL reset range [%#llx-%#llx] exceeds resource size\n",
> +			hpa_range->start, hpa_range->end);
> +		return -EOVERFLOW;
> +	}
> +
> +	if (len > SIZE_MAX) {
> +		dev_err(dev,
> +			"CXL reset range [%#llx-%#llx] exceeds cache flush size\n",
> +			hpa_range->start, hpa_range->end);
> +		return -EOVERFLOW;
> +	}
> +
> +	return 0;
> +}
> +
> +static int cxl_hdm_range_add(struct cxl_hdm_quiesce *q, struct device *dev,
> +			     const struct range *hpa_range)
> +{
> +	struct cxl_hdm_range *range, *next, *new_range;
> +	int rc;
> +
> +	rc = cxl_hdm_range_validate(dev, hpa_range);
> +	if (rc)
> +		return rc;
> +
> +	list_for_each_entry(range, &q->ranges, list)
> +		if (range_contains(&range->hpa_range, hpa_range))
> +			return 0;
> +
> +	new_range = kzalloc_obj(*new_range);
> +	if (!new_range)
> +		return -ENOMEM;
> +
> +	new_range->dev = dev;
> +	new_range->hpa_range = *hpa_range;
> +	new_range->len = range_len(hpa_range);
> +
> +	list_for_each_entry_safe(range, next, &q->ranges, list) {
> +		if (range_contains(hpa_range, &range->hpa_range)) {
> +			list_del(&range->list);
> +			kfree(range);
> +		}
> +	}
> +	list_add_tail(&new_range->list, &q->ranges);
> +
> +	return 0;
> +}
> +
> +static int cxl_hdm_ranges_collect(struct cxl_hdm_quiesce *q, struct device *dev,
> +				  const struct cxl_hdm_info *snapshot)
> +{
> +	int rc;
> +
> +	for (int i = 0; i < snapshot->decoder_count; i++) {
> +		const struct cxl_decoder_config *config =
> +			&snapshot->settings[i].config;
> +
> +		/* A committed zero-size decoder maps no HPA. */
> +		if (!(config->flags & CXL_DECODER_F_ENABLE) ||
> +		    !range_len(&config->hpa_range))
> +			continue;
> +
> +		if (config->flags & CXL_DECODER_F_NORMALIZED_ADDRESSING) {
> +			dev_err(dev,
> +				"CXL reset does not support normalized address decoders\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		rc = cxl_hdm_range_add(q, dev, &config->hpa_range);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static int cxl_hdm_ranges_request(struct cxl_hdm_quiesce *q)
> +{
> +	struct cxl_hdm_range *range;
> +
> +	lockdep_assert_held_write(&cxl_rwsem.region);
> +
> +	list_for_each_entry(range, &q->ranges, list) {
> +		const struct range *hpa_range = &range->hpa_range;
> +
> +		range->res = request_mem_region(hpa_range->start, range->len,
> +						"cxl_reset");
> +		if (!range->res) {
> +			dev_err(range->dev,
> +				"cannot reset while CXL memory range is busy [%#llx-%#llx]\n",
> +				hpa_range->start, hpa_range->end);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int cxl_hdm_ranges_invalidate(struct cxl_hdm_quiesce *q)
> +{
> +	struct cxl_hdm_range *range;
> +	int rc = 0;
> +
> +	lockdep_assert_held_write(&cxl_rwsem.region);
> +
> +	list_for_each_entry(range, &q->ranges, list) {
> +		const struct range *hpa_range = &range->hpa_range;
> +		int rc2;
> +
> +		rc2 = cpu_cache_invalidate_memregion(hpa_range->start, range->len);
> +		if (rc2)
> +			dev_err(range->dev,
> +				"failed to invalidate CPU cache [%#llx-%#llx]: %d\n",
> +				hpa_range->start, hpa_range->end, rc2);
> +		rc = rc ?: rc2;
> +	}
> +
> +	return rc;
> +}
> +
> +/**
> + * cxl_hdm_quiesce_prepare() - take the HPA ranges of a device out of use
> + * @dev: device for error messages
> + * @q: quiesce state to initialize
> + * @snapshot: HDM state from cxl_hdm_cache_snapshot()
> + *
> + * Reserve every enabled system-physical range in @snapshot exclusively, then
> + * write back and invalidate CPU caches for them. Call
> + * cxl_hdm_quiesce_destroy() afterwards whether or not this succeeds.
> + *
> + * Return: 0 on success, or a negative errno.
> + */
> +int cxl_hdm_quiesce_prepare(struct device *dev, struct cxl_hdm_quiesce *q,
> +			    const struct cxl_hdm_info *snapshot)
> +{
> +	int rc;
> +
> +	lockdep_assert_held_write(&cxl_rwsem.region);
> +	INIT_LIST_HEAD(&q->ranges);
> +
> +	if (!cpu_cache_has_invalidate_memregion()) {
> +		dev_err(dev, "CPU cache invalidation unavailable\n");
> +		return -ENXIO;
> +	}
> +
> +	rc = cxl_hdm_ranges_collect(q, dev, snapshot);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_hdm_ranges_request(q);
> +	if (rc)
> +		return rc;
> +
> +	return cxl_hdm_ranges_invalidate(q);
> +}
> +
> +/* Evict lines fetched during reset before DMA exclusion ends. */
> +int cxl_hdm_quiesce_flush(struct cxl_hdm_quiesce *q)
> +{
> +	return cxl_hdm_ranges_invalidate(q);
> +}
> +
> +void cxl_hdm_quiesce_destroy(struct cxl_hdm_quiesce *q)
> +{
> +	struct cxl_hdm_range *range, *next;
> +
> +	list_for_each_entry_safe(range, next, &q->ranges, list) {
> +		list_del(&range->list);
> +		if (range->res)
> +			release_mem_region(range->hpa_range.start,
> +					   resource_size(range->res));
> +		kfree(range);
> +	}
> +}
> diff --git a/drivers/pci/cxl.c b/drivers/pci/cxl.c
> index 9eb13adf0daa..3da8b316857f 100644
> --- a/drivers/pci/cxl.c
> +++ b/drivers/pci/cxl.c
> @@ -15,6 +15,7 @@
>  #include <linux/iommu.h>
>  #include <linux/jiffies.h>
>  #include <linux/kernel.h>
> +#include <linux/memregion.h>
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  
> @@ -392,7 +393,8 @@ static int cxl_clear_memory(struct pci_dev *pdev, int dvsec, bool initiate)
>  					      PCI_DVSEC_CXL_RST_MEM_CLR_EN);
>  }
>  
> -static int __cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap)
> +static int __cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap,
> +			      struct cxl_hdm_quiesce *q)
>  {
>  	int rc, rc2;
>  
> @@ -417,12 +419,18 @@ static int __cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap)
>  		pci_err(pdev, "failed to clear CXL Reset Memory Clear: %d\n", rc2);
>  	rc = rc ?: rc2;
>  
> +	rc2 = cxl_hdm_quiesce_flush(q);
> +	rc = rc ?: rc2;
> +
>  	pci_dev_reset_iommu_done(pdev);
>  	return rc;
>  }
>  
> -static int cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap)
> +static int cxl_reset_quiesced(struct pci_dev *pdev, int dvsec, u16 cap)
> +	__must_hold(cxl_region_quiesce)
>  {
> +	struct cxl_hdm_info *snapshot __free(kfree) = NULL;
> +	struct cxl_hdm_quiesce q;
>  	u16 saved_ctrl2;
>  	int rc, rc2;
>  
> @@ -432,14 +440,39 @@ static int cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap)
>  	if (PCI_POSSIBLE_ERROR(saved_ctrl2))
>  		return -ENODEV;
>  
> +	snapshot = cxl_hdm_cache_snapshot(&pdev->hdm);
> +	if (IS_ERR(snapshot)) {
> +		pci_err(pdev, "CXL HDM decoder state unavailable: %ld\n",
> +			PTR_ERR(snapshot));
> +		return PTR_ERR(snapshot);
> +	}
> +
> +	rc = cxl_hdm_quiesce_prepare(&pdev->dev, &q, snapshot);
> +	if (rc) {
> +		cxl_hdm_quiesce_destroy(&q);
> +		return rc;
> +	}
> +
>  	rc = cxl_reset_disable_cache(pdev, dvsec, cap);
>  	if (!rc)
> -		rc = __cxl_reset_execute(pdev, dvsec, cap);
> +		rc = __cxl_reset_execute(pdev, dvsec, cap, &q);
>  	/* Restore cache policy after any attempt to disable caching. */
>  	rc2 = cxl_reset_restore_cache_policy(pdev, dvsec, saved_ctrl2);
> +	cxl_hdm_quiesce_destroy(&q);
>  	return rc ?: rc2;
>  }
>  
> +static int cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap)
> +{
> +	int rc;
> +
> +	cxl_region_quiesce_lock();
> +	rc = cxl_reset_quiesced(pdev, dvsec, cap);
> +	cxl_region_quiesce_unlock();
> +
> +	return rc;
> +}
> +
>  int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  {
>  	u16 dvsec, cap, ctrl;
> @@ -466,5 +499,16 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  	if (probe)
>  		return 0;
>  
> +	/* The cache is owned by @pdev and does not require a bound CXL driver. */
> +	if (!cxl_hdm_cache_present(&pdev->hdm))
> +		return -ENOTTY;
> +
> +	/* Memory decoded through DVSEC ranges has no HPA ranges to quiesce. */
> +	if (cxl_hdm_cache_dvsec_decode(&pdev->hdm))
> +		return -ENOTTY;
> +
> +	if (!cpu_cache_has_invalidate_memregion())
> +		return -ENOTTY;
> +
>  	return cxl_reset_execute(pdev, dvsec, cap);
>  }
> diff --git a/include/cxl/hdm.h b/include/cxl/hdm.h
> index af7cef649446..f6446a2a7fcd 100644
> --- a/include/cxl/hdm.h
> +++ b/include/cxl/hdm.h
> @@ -3,9 +3,14 @@
>  #ifndef __CXL_HDM_H__
>  #define __CXL_HDM_H__
>  
> +#include <linux/list.h>
>  #include <linux/types.h>
> +
>  #include <cxl/cxl.h>
>  
> +struct device;
> +struct resource;
> +
>  /**
>   * struct cxl_endpoint_decoder_settings - endpoint HDM decoder programming
>   * @config: common decoder configuration
> @@ -42,7 +47,6 @@ struct cxl_hdm_info {
>  	bool dvsec_ctrl_valid;
>  	struct cxl_endpoint_decoder_settings settings[] __counted_by(decoder_count);
>  };
> -struct device;
>  
>  int cxl_hdm_decoder_count_mapped(struct device *dev, void __iomem *hdm,
>  				 resource_size_t size);
> @@ -52,5 +56,46 @@ bool cxl_hdm_cache_present(struct cxl_hdm_info *const *slot);
>  void cxl_hdm_cache_publish(struct cxl_hdm_info **slot,
>  			   struct cxl_hdm_info *info);
>  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);
> +
> +/*
> + * Region quiesce blocks CXL region commit and teardown, and with them decoder
> + * reprogramming, while reset handling captures or restores HDM decoder state.
> + */
> +token_context_lock(cxl_region_quiesce);
> +
> +/**
> + * struct cxl_hdm_range - HPA range held exclusively across a reset
> + * @list: entry in &cxl_hdm_quiesce.ranges
> + * @dev: device for error messages
> + * @hpa_range: system physical address range decoded by the device
> + * @len: length of @hpa_range
> + * @res: reservation of @hpa_range, once requested
> + */
> +struct cxl_hdm_range {
> +	struct list_head list;
> +	struct device *dev;
> +	struct range hpa_range;
> +	u64 len;
> +	struct resource *res;
> +};
> +
> +/**
> + * struct cxl_hdm_quiesce - HPA ranges held exclusively across a reset
> + * @ranges: list of &struct cxl_hdm_range, see cxl_hdm_quiesce_prepare()
> + */
> +struct cxl_hdm_quiesce {
> +	struct list_head ranges;
> +};
> +
> +void cxl_region_quiesce_lock(void);
> +void cxl_region_quiesce_unlock(void);
> +int cxl_hdm_quiesce_prepare(struct device *dev, struct cxl_hdm_quiesce *q,
> +			    const struct cxl_hdm_info *snapshot)
> +	__must_hold(cxl_region_quiesce);
> +int cxl_hdm_quiesce_flush(struct cxl_hdm_quiesce *q)
> +	__must_hold(cxl_region_quiesce);
> +void cxl_hdm_quiesce_destroy(struct cxl_hdm_quiesce *q);
>  
>  #endif /* __CXL_HDM_H__ */
> -- 
> 2.43.0
>