[RFC PATCH v1 0/2] cxl/cli: HPA-ordered destroy-region teardown

Pawel Mielimonka posted 2 patches 2 months, 2 weeks ago
There is a newer version of this series
cxl/region.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 111 insertions(+), 3 deletions(-)
[RFC PATCH v1 0/2] cxl/cli: HPA-ordered destroy-region teardown
Posted by Pawel Mielimonka 2 months, 2 weeks ago
This series addresses an issue in destroy-region, where
region teardown relied on libcxl enumeration order, which is not
guaranteed to match the increasing HPA order exposed by the kernel. 
When a decoder window is fully populated, attempting to destroy a
non-last region causes kernel-side validation to fail (e.g.
set_dpa_size(..., 0) returns an error), and subsequent destroy/create
sequences may become impossible.

The CXL specification requires that decoder programming (and the
implicit teardown path) must preserve continuous HPA coverage and
proceed strictly in order: decoder m before decoder m+1, with each
covering an HPA range below the next one. Practically, this means that
region teardown must follow HPA-descending order and must stop as soon
as a gap in the requested suffix is encountered.

The patch introduces destroy_multiple_regions(), which collects all
regions under a given root decoder, sorts them by HPA, and destroys
only the suffix requested by the user (or all regions in the case of
“all”). The implementation guarantees that only valid teardown
sequences are attempted and prevents decoder state inconsistencies
observed during repeated destroy/create cycles.

Enable/disable paths and all existing bus/port/decoder filtering
remain unchanged.

Pawel Mielimonka (2):
  cxl/cli: add helpers to collect and sort regions by HPA
  cxl/cli: enforce HPA-descending teardown order for destroy-region

 cxl/region.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 111 insertions(+), 3 deletions(-)

-- 
2.45.1.windows.1

[PATCH v2 0/1] cxl/cli: HPA-ordered destroy-region teardown
Posted by Pawel Mielimonka 2 weeks, 5 days ago
This is v2 of the series.

Changes since v1:
 - Rework teardown ordering to account endpoint decoders shared across
   regions under the same bus/port
 - This addresses scenario described by Alison when a memdev is
   targeted by regions under multiple root decoders.

Alison, if you have a chance, could you please retest this version
against your multi-root-decoder configuration to confim that the
teardown now behaves correctly in that scenario?

Pawel Mielimonka (1):
  cxl/cli: enforce HPA-descening teardown order for destroy-region

 cxl/region.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 2 deletions(-)

-- 
2.45.1.windows.1
Re: [PATCH v2 0/1] cxl/cli: HPA-ordered destroy-region teardown
Posted by Alison Schofield 2 weeks, 4 days ago
On Tue, Jan 20, 2026 at 11:32:11PM +0900, Pawel Mielimonka wrote:
> This is v2 of the series.
> 
> Changes since v1:
>  - Rework teardown ordering to account endpoint decoders shared across
>    regions under the same bus/port
>  - This addresses scenario described by Alison when a memdev is
>    targeted by regions under multiple root decoders.
> 
> Alison, if you have a chance, could you please retest this version
> against your multi-root-decoder configuration to confim that the
> teardown now behaves correctly in that scenario?

Hi Pawel,

Patch isn't compiling - first error:
../cxl/region.c:854:48: error: ‘decoder’ undeclared (first use in this function); did you mean ‘cxl_decoder’?
  854 |                 cxl_decoder_foreach(root_port, decoder) {
      |                                                ^~~~~~~

For v3, please send the patch as a new thread, not in response to
previous. See here [1] for some ndctl patch sending guidelines, like
labelling as [ndctl PATCH v3] and sending to the nvdimm mailing list.

I think rolling into one patch is fine, and would like to see the
entire previous cover letter info in the commit log of the one
patch. It set this work up very well.

[1] https://github.com/pmem/ndctl?tab=contributing-ov-file#readme

Thanks,
Alison

> 
> Pawel Mielimonka (1):
>   cxl/cli: enforce HPA-descening teardown order for destroy-region
> 
>  cxl/region.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 108 insertions(+), 2 deletions(-)
> 
> -- 
> 2.45.1.windows.1
> 
[PATCH v2 1/1] cxl/cli: enforce HPA-descening teardown order for destroy-region
Posted by Pawel Mielimonka 2 weeks, 5 days ago
Note: This revision collapses the previous 2patch series into a single
patch.

Signed-off-by: Pawel Mielimonka <pawel.mielimonka@fujitsu.com>
---
 cxl/region.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 2 deletions(-)

diff --git a/cxl/region.c b/cxl/region.c
index 207cf2d0..2e0f215d 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -831,6 +831,59 @@ out:
 	return cxl_region_disable(region);
 }
 
+static int cmp_region_hpa(const void *l, const void *r)
+{
+	const struct cxl_region *const *left = l;
+	const struct cxl_region *const *right = r;
+	u64 hpa1 = cxl_region_get_resource((struct cxl_region *) *left);
+	u64 hpa2 = cxl_region_get_resource((struct cxl_region *) *right);
+
+	return (hpa1 > hpa2) - (hpa1 < hpa2);
+}
+
+static int collect_regions_sorted(struct cxl_decoder *root,
+	struct cxl_region ***out,
+	int *out_nr)
+{
+	struct cxl_region *region;
+	struct cxl_region **list = NULL;
+	int nr = 0, alloc = 0;
+
+		struct cxl_port *root_port = cxl_decoder_get_port(root)
+
+		cxl_decoder_foreach(root_port, decoder) {
+		if (!cxl_port_is_root(port))
+			continue;
+			cxl_region_foreach(decoder, region) {
+			if (nr == alloc) {
+				int new_alloc = alloc ? alloc * 2 : 8;
+				size_t new_size = (size_t)new_alloc * sizeof(*list);
+				struct cxl_region **tmp;
+
+				tmp = realloc(list, new_size);
+				if (!tmp) {
+					free(list);
+					return -ENOMEM;
+				}
+				list = tmp;
+				alloc = new_alloc;
+			}
+			list[nr++] = region;
+		}
+
+		if (!nr) {
+			free(list);
+			*out = NULL;
+			*out_nr = 0;
+			return 0;
+		}
+	}
+	qsort(list, nr, sizeof(*list), cmp_region_hpa);
+	*out = list;
+	*out_nr = nr;
+	return 0;
+}
+
 static int destroy_region(struct cxl_region *region)
 {
 	const char *devname = cxl_region_get_devname(region);
@@ -895,6 +948,58 @@ static int destroy_region(struct cxl_region *region)
 	return cxl_region_delete(region);
 }
 
+static int destroy_multiple_regions(struct cxl_ctx *ctx,
+	struct parsed_params *p,
+	int *count)
+{
+	struct cxl_region **list;
+	int nr, rc, i;
+	bool skipped = false;
+
+	rc = collect_regions_sorted(ctx, NULL, &list, &nr);
+	if (rc) {
+		log_err(&rl, "failed to allocate region list: %s\n", strerror(-rc));
+		return rc;
+	}
+
+	for (i = nr - 1; i >= 0; --i) {
+		struct cxl_region *region = NULL;
+
+		for (int j = 0; j < p->argc; j++) {
+			region = util_cxl_region_filter(list[i], p->argv[j]);
+			if (region)
+				break;
+		}
+
+		if (!region) {
+			skipped = true;
+			continue;
+		}
+
+		/*
+		 * If current region matches filter, but previous didn't, destroying would
+		 * result in breaking HPA continuity
+		 */
+		if (skipped) {
+			log_err(&rl, "failed to destroy %s: out of order %s reset\n",
+				cxl_region_get_devname(region),
+				cxl_decoder_get_devname(decoder));
+			rc = -EINVAL;
+			break;
+		}
+
+		rc = destroy_region(region);
+		if (rc) {
+			log_err(&rl, "%s: failed: %s\n",
+				cxl_region_get_devname(region), strerror(-rc));
+			break;
+		}
+		++(*count);
+	}
+	free(list);
+	return rc;
+}
+
 static int do_region_xable(struct cxl_region *region, enum region_actions action)
 {
 	switch (action) {
@@ -902,8 +1007,6 @@ static int do_region_xable(struct cxl_region *region, enum region_actions action
 		return cxl_region_enable(region);
 	case ACTION_DISABLE:
 		return disable_region(region);
-	case ACTION_DESTROY:
-		return destroy_region(region);
 	default:
 		return -EINVAL;
 	}
@@ -956,6 +1059,9 @@ static int region_action(int argc, const char **argv, struct cxl_ctx *ctx,
 	if (action == ACTION_CREATE)
 		return create_region(ctx, count, p);
 
+	if (action == ACTION_DESTROY)
+		return destroy_multiple_regions(ctx, p, count);
+
 	cxl_bus_foreach(ctx, bus) {
 		struct cxl_decoder *decoder;
 		struct cxl_port *port;
-- 
2.45.1.windows.1
Re: [RFC PATCH v1 0/2] cxl/cli: HPA-ordered destroy-region teardown
Posted by Alison Schofield 2 months, 1 week ago
On Tue, Nov 25, 2025 at 11:38:22PM +0900, Pawel Mielimonka wrote:
> This series addresses an issue in destroy-region, where
> region teardown relied on libcxl enumeration order, which is not
> guaranteed to match the increasing HPA order exposed by the kernel. 
> When a decoder window is fully populated, attempting to destroy a
> non-last region causes kernel-side validation to fail (e.g.
> set_dpa_size(..., 0) returns an error), and subsequent destroy/create
> sequences may become impossible.
> 
> The CXL specification requires that decoder programming (and the
> implicit teardown path) must preserve continuous HPA coverage and
> proceed strictly in order: decoder m before decoder m+1, with each
> covering an HPA range below the next one. Practically, this means that
> region teardown must follow HPA-descending order and must stop as soon
> as a gap in the requested suffix is encountered.
> 
> The patch introduces destroy_multiple_regions(), which collects all
> regions under a given root decoder, sorts them by HPA, and destroys
> only the suffix requested by the user (or all regions in the case of
> “all”). The implementation guarantees that only valid teardown
> sequences are attempted and prevents decoder state inconsistencies
> observed during repeated destroy/create cycles.
> 
> Enable/disable paths and all existing bus/port/decoder filtering
> remain unchanged.

Hi Pawel,

Thanks - this is much needed!

It's very valuable that now trying to destroy any region out of 
order will fail 'gracefully'.

This worked for 'all' and needs a small fixup for the decoder option.
That's noted in Patch 2 reply. I didn't test the other filtering yet
but started updating a unit test to cover these cases.

See https://github.com/pmem/ndctl/blob/main/CONTRIBUTING.md for
a few housekeeping tips on submitting to NDCTL.

-- Alison


> 
> Pawel Mielimonka (2):
>   cxl/cli: add helpers to collect and sort regions by HPA
>   cxl/cli: enforce HPA-descending teardown order for destroy-region
> 
>  cxl/region.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 111 insertions(+), 3 deletions(-)
> 
> -- 
> 2.45.1.windows.1
> 
>