Re: [PATCH v12 07/12] cxl: Validate HDM ranges before CXL reset

[Jonathan Cameron]

On Thu, 10 Sep 2026 07:08:03 +0000
Srirangan Madhavan <smadhavan@xxxxxxxxxx> wrote:

> Before reset, require cached HDM decoder state, collect enabled decoder
> ranges, and reserve them with request_mem_region(). This rejects reset
> while affected CXL memory is busy and keeps the validation stable
> through reset.
> 
> If CPU cache invalidation support is available, invalidate the affected
> ranges before reset.

If it is not what happens? I'm thinking dead system as coherency just broke
but maybe not. My gut is no reset without cache invalidation support.

In general document your cache invalidation flow in this commit message. I'd
suspicious you don't have a pair of invalidations, one before to ensure
all caches are clean and one after to evict prefetched lines if your
device was mean enough to serve garbage during the reset
(which many will do!) 

If more comes in later patches, but a breadcrumb in here to say so.

> If the runtime backend is unavailable, continue
> after the range reservation succeeds.
> 
> Reject CXL Reset when no cached HDM decoder state is available. The reset
> path needs the cached address map to validate affected ranges and perform
> CPU cache invalidation. Also reject normalized-addressing decoders for
> now because the cached decoder range is not a system physical address.
> 
> Signed-off-by: Srirangan Madhavan <smadhavan@xxxxxxxxxx>



> +static int cxl_hdm_range_flush_cache(struct cxl_hdm_range *range)
> +{
> +	struct pci_dev *pdev = range->pdev;
> +	const struct range *hpa_range = &range->hpa_range;
> +	int rc;
> +
> +	rc = cpu_cache_invalidate_memregion(hpa_range->start, range->len);
> +	if (rc)
> +		pci_err(pdev,
> +			"failed to invalidate CPU cache [%#llx-%#llx]: %d\n",
> +			hpa_range->start, hpa_range->end, rc);
What does the wrapper really bring?

> +
> +	return rc;
> +}
> +
> +static int cxl_hdm_ranges_flush_cpu_caches(struct cxl_hdm_range_context *ctx,
> +					   struct pci_dev *pdev)
> +{
> +	struct cxl_hdm_range *range;
> +	int rc;
> +
> +	if (list_empty(&ctx->ranges))
> +		return 0;
> +
> +	if (!cpu_cache_has_invalidate_memregion()) {
> +		pci_warn(pdev,
> +			 "CPU cache synchronization unavailable; continuing without cache invalidation\n");

boom.  Unless you have very strong reasonsing for me this is a no.
This introduces extremely subtle cache coherency corruption into your
system and no one wants to debug results of that.

> +		return 0;
> +	}
> +
> +	list_for_each_entry(range, &ctx->ranges, list) {
> +		rc = cxl_hdm_range_flush_cache(range);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return 0;
> +}