Re: [PATCH v13 11/15] cxl: Validate and synchronize HDM ranges around reset

[Cheatham, Benjamin]

On 9/22/2026 3:39 AM, 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 after restoration, holding range reservations and IOMMU
> exclusion until the second invalidation completes.
> 
> Reject normalized-addressing decoders because their cached ranges are not
> system physical addresses. Ignore zero-size decoders because they map no
> address range.
> 
> Signed-off-by: Srirangan Madhavan <smadhavan@xxxxxxxxxx>
> ---
>  drivers/cxl/core/resource.c | 244 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 239 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/core/resource.c b/drivers/cxl/core/resource.c
> index 57eec77cbb21..85a554b6236c 100644
> --- a/drivers/cxl/core/resource.c
> +++ b/drivers/cxl/core/resource.c
> @@ -11,6 +11,8 @@
>  #include <linux/iommu.h>
>  #include <linux/jiffies.h>
>  #include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/memregion.h>
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  
> @@ -437,6 +439,211 @@ static const u32 cxl_reset_timeout_ms[] = {
>  #define CXL_CACHE_WBI_TIMEOUT_US 100000
>  #define CXL_CACHE_WBI_POLL_US 100
>  
> +struct cxl_hdm_range {
> +	struct list_head list;
> +	struct pci_dev *pdev;
> +	struct range hpa_range;
> +	u64 len;
> +	struct resource *res;
> +};
> +
> +struct cxl_hdm_range_context {
> +	struct list_head ranges;
> +};
> +
> +static void cxl_hdm_range_context_destroy(struct cxl_hdm_range_context *ctx)
> +{
> +	struct cxl_hdm_range *range, *next;
> +
> +	list_for_each_entry_safe(range, next, &ctx->ranges, list) {
> +		list_del(&range->list);
> +		if (range->res)
> +			release_mem_region(range->hpa_range.start,
> +					   resource_size(range->res));
> +		kfree(range);
> +	}
> +}
> +
> +/*
> + * 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 pci_dev *pdev,
> +				  const struct range *hpa_range)
> +{
> +	u64 len = range_len(hpa_range);
> +
> +	if (!len)
> +		return -EINVAL;
> +
> +	if (hpa_range->end > RESOURCE_SIZE_MAX) {
> +		pci_err(pdev,
> +			"CXL reset range [%#llx-%#llx] exceeds resource address size\n",
> +			hpa_range->start, hpa_range->end);
> +		return -EOVERFLOW;
> +	}
> +
> +	if (len > RESOURCE_SIZE_MAX) {
> +		pci_err(pdev,
> +			"CXL reset range [%#llx-%#llx] exceeds resource size\n",
> +			hpa_range->start, hpa_range->end);
> +		return -EOVERFLOW;
> +	}
> +
> +	if (len > SIZE_MAX) {
> +		pci_err(pdev,
> +			"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_range_context *ctx,
> +			     struct pci_dev *pdev, const struct range *hpa_range)
> +{
> +	struct cxl_hdm_range *range;
> +	int rc;
> +
> +	rc = cxl_hdm_range_validate(pdev, hpa_range);
> +	if (rc)
> +		return rc;
> +
> +	list_for_each_entry(range, &ctx->ranges, list)
> +		if (range->hpa_range.start == hpa_range->start &&
> +		    range->hpa_range.end == hpa_range->end)

Maybe resource_contains() instead? I don't think there'll be any overlap, but no point in reserving
a fully contained resource. You'd have to add some extra logic to remove the smaller resource though.

> +			return 0;
> +
> +	range = kzalloc_obj(*range);
> +	if (!range)
> +		return -ENOMEM;
> +
> +	range->pdev = pdev;
> +	range->hpa_range = *hpa_range;
> +	range->len = range_len(hpa_range);
> +	list_add_tail(&range->list, &ctx->ranges);
> +
> +	return 0;
> +}
> +
> +static int cxl_hdm_ranges_collect(struct cxl_hdm_range_context *ctx,
> +				  struct pci_dev *pdev)
> +{
> +	struct cxl_hdm_info *info;
> +	int rc;
> +
> +	guard(rwsem_read)(&cxl_rwsem.dpa);
> +	info = pdev->hdm;
> +	if (!info) {
> +		pci_err(pdev, "CXL HDM decoder state unavailable\n");
> +		return -ENXIO;
> +	}
> +
> +	for (int i = 0; i < info->decoder_count; i++) {
> +		struct cxl_decoder_config *config = &info->settings[i].config;
> +
> +		if (!(config->flags & CXL_DECODER_F_ENABLE))
> +			continue;
> +
> +		/* A committed zero-size decoder maps no HPA. */
> +		if (!range_len(&config->hpa_range))
> +			continue;

Nit: these two if statements could be on a single line and just put the comment above it, it's pretty
obvious which condition the comment is talking about.

> +
> +		if (config->flags & CXL_DECODER_F_NORMALIZED_ADDRESSING) {
> +			pci_err(pdev,
> +				"CXL reset does not support normalized address decoders\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		rc = cxl_hdm_range_add(ctx, pdev, &config->hpa_range);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static int cxl_hdm_ranges_request(struct cxl_hdm_range_context *ctx)
> +{
> +	struct cxl_hdm_range *range;
> +
> +	lockdep_assert_held_write(&cxl_rwsem.region);
> +
> +	list_for_each_entry(range, &ctx->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) {
> +			pci_err(range->pdev,
> +				"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_range_context *ctx)
> +{
> +	struct cxl_hdm_range *range;
> +	int rc = 0;
> +
> +	list_for_each_entry(range, &ctx->ranges, list) {
> +		const struct range *hpa_range = &range->hpa_range;
> +		int rc2;
> +
> +		rc2 = cpu_cache_invalidate_memregion(hpa_range->start, range->len);
> +		if (rc2)
> +			pci_err(range->pdev,
> +				"failed to invalidate CPU cache [%#llx-%#llx]: %d\n",
> +				hpa_range->start, hpa_range->end, rc2);
> +		rc = rc ?: rc2;
> +	}
> +
> +	return rc;
> +}
> +
> +static int cxl_hdm_ranges_prepare(struct cxl_hdm_range_context *ctx,
> +				  struct pci_dev *pdev)
> +{
> +	int rc;
> +
> +	lockdep_assert_held_write(&cxl_rwsem.region);
> +
> +	if (!cpu_cache_has_invalidate_memregion()) {
> +		pci_err(pdev, "CPU cache invalidation unavailable\n");
> +		return -ENXIO;
> +	}
> +
> +	rc = cxl_hdm_ranges_collect(ctx, pdev);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_hdm_ranges_request(ctx);
> +	if (rc)
> +		return rc;
> +
> +	return cxl_hdm_ranges_invalidate(ctx);
> +}
> +
> +static int cxl_hdm_ranges_finish(struct cxl_hdm_range_context *ctx)
> +{
> +	int rc;
> +
> +	lockdep_assert_held_write(&cxl_rwsem.region);
> +
> +	rc = cxl_hdm_ranges_invalidate(ctx);
> +	cxl_hdm_range_context_destroy(ctx);
> +
> +	return rc;
> +}
> +
>  #define CXL_RESET_CTRL2_CMD_MASK \
>  	(PCI_DVSEC_CXL_INIT_CACHE_WBI | PCI_DVSEC_CXL_INIT_CXL_RST)
>  
> @@ -560,24 +767,25 @@ static int cxl_reset_wait_done(struct pci_dev *pdev, int dvsec, u16 cap)
>  	}
>  }
>  
> -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_range_context *range_ctx)
>  {
>  	int rc, rc2;
>  
>  	rc = cxl_reset_disable_cache(pdev, dvsec, cap);
>  	if (rc)
> -		goto out_enable_cache;
> +		goto out_destroy_ranges;
>  
>  	if (!pci_wait_for_pending_transaction(pdev)) {
>  		pci_err(pdev, "timed out waiting for pending transactions\n");
>  		rc = -ETIMEDOUT;
> -		goto out_enable_cache;
> +		goto out_destroy_ranges;
>  	}
>  
>  	rc = pci_dev_reset_iommu_prepare(pdev);
>  	if (rc) {
>  		pci_err(pdev, "failed to stop IOMMU for CXL reset: %d\n", rc);
> -		goto out_enable_cache;
> +		goto out_destroy_ranges;
>  	}
>  
>  	/* Clear Memory Clear again even if this command write reports failure. */
> @@ -597,7 +805,14 @@ static int cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap)
>  			rc2);
>  	rc = rc ?: rc2;
>  
> +	/* Evict lines fetched during reset before ending DMA exclusion. */
> +	rc2 = cxl_hdm_ranges_finish(range_ctx);
> +	rc = rc ?: rc2;
>  	pci_dev_reset_iommu_done(pdev);
> +	goto out_enable_cache;
> +
> +out_destroy_ranges:
> +	cxl_hdm_range_context_destroy(range_ctx);
>  

Same thing here as the last patch, but I'll let you do this one ;).

>  out_enable_cache:
>  	/*
> @@ -615,6 +830,7 @@ static int cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap)
>  
>  int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  {
> +	struct cxl_hdm_range_context range_ctx;
>  	int dvsec, rc;
>  	u16 cap, ctrl;
>  
> @@ -646,5 +862,23 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  	if (probe)
>  		return 0;
>  
> -	return cxl_reset_execute(pdev, dvsec, cap);
> +	/* The cache is owned by @pdev and does not require a bound CXL driver. */
> +	scoped_guard(rwsem_read, &cxl_rwsem.dpa)
> +		if (!pdev->hdm || !pdev->hdm->hdm_size)
> +			return -ENOTTY;
> +
> +	if (!cpu_cache_has_invalidate_memregion())
> +		return -ENOTTY;
> +
> +	INIT_LIST_HEAD(&range_ctx.ranges);
> +
> +	scoped_guard(rwsem_write, &cxl_rwsem.region) {
> +		rc = cxl_hdm_ranges_prepare(&range_ctx, pdev);
> +		if (!rc)
> +			rc = cxl_reset_execute(pdev, dvsec, cap, &range_ctx);
> +		else
> +			cxl_hdm_range_context_destroy(&range_ctx);
> +	}

Probably better to just copy-paste the range context destroy call and re-order as:

		rc = cxl_hdm_ranges_prepare(&range_ctx, pdev);
		if (rc) {
			cxl_hdm_range_context_destroy(&range_ctx);
			return rc;
		}


		rc = cxl_reset_execute(pdev, dvsec, cap, &range_ctx);
		cxl_hdm_range_context_destroy(&range_ctx);

instead.

> +
> +	return rc;
>  }