1. Patchew
  2. linux
  3. [RFC PATCH v1 0/2] cxl/cli: HPA-ordered destroy-region teardown
  4. View patch

[RFC PATCH v1 1/2] cxl/cli: add helpers to collect and sort regions by HPA

Pawel Mielimonka posted 2 patches 2 months, 2 weeks ago
There is a newer version of this series
[RFC PATCH v1 1/2] cxl/cli: add helpers to collect and sort regions by HPA
Posted by Pawel Mielimonka 2 months, 2 weeks ago 
Introduce cmp_region_hpa() and collect_regions_sorted() helpers to
enumerate CXL regions under a given decoder and sort them by their host
physical address.
These helpers will be used by the "cxl destroy-region" command to tear
down regions in HPA-descending order, i.e. in the reverse order of
region creation. This matches the decoder programming requirements from
the CXL specification (8.2.4.20.12 - continuous HPA coverage at all
times when Lock On Commit is used) and avoids teardown sequences that
can leve decoder state inconsistent when the decoder is fully populated
(known problem).
This patch only adds the helpers; no functional changes is intended yet.

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

diff --git a/cxl/region.c b/cxl/region.c
index 207cf2d0..58765b3d 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -831,6 +831,61 @@ 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 left_start = cxl_region_get_resource((struct cxl_region *) *left);
+	u64 right_start = cxl_region_get_resource((struct cxl_region *) *right);
+
+	if (left_start < right_start)
+		return -1;
+	if (left_start > right_start)
+		return 1;
+	return 0;
+}
+
+static int collect_regions_sorted(struct cxl_decoder *root,
+								  const char *filter,
+								  struct cxl_region ***out,
+								  int *out_nr)
+{
+	struct cxl_region *region;
+	struct cxl_region **list = NULL;
+	int nr = 0, alloc = 0;
+
+	cxl_region_foreach(root, region) {
+		if (filter && !util_cxl_region_filter(region, filter))
+			continue;
+		if (nr == alloc) {
+			int new_alloc = alloc ? alloc * 2 : 8;
+			int 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);
-- 
2.45.1.windows.1
Re: [RFC PATCH v1 1/2] cxl/cli: add helpers to collect and sort regions by HPA
Posted by Alison Schofield 2 months, 1 week ago 
On Tue, Nov 25, 2025 at 11:38:23PM +0900, Pawel Mielimonka wrote:
> Introduce cmp_region_hpa() and collect_regions_sorted() helpers to
> enumerate CXL regions under a given decoder and sort them by their host
> physical address.
> These helpers will be used by the "cxl destroy-region" command to tear
> down regions in HPA-descending order, i.e. in the reverse order of
> region creation. This matches the decoder programming requirements from
> the CXL specification (8.2.4.20.12 - continuous HPA coverage at all
> times when Lock On Commit is used) and avoids teardown sequences that
> can leve decoder state inconsistent when the decoder is fully populated
> (known problem).
> This patch only adds the helpers; no functional changes is intended yet.
> 
> Signed-off-by: Pawel Mielimonka <pawel.mielimonka@fujitsu.com>
> ---
>  cxl/region.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 207cf2d0..58765b3d 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -831,6 +831,61 @@ 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 left_start = cxl_region_get_resource((struct cxl_region *) *left);
> +	u64 right_start = cxl_region_get_resource((struct cxl_region *) *right);
> +
> +	if (left_start < right_start)
> +		return -1;
> +	if (left_start > right_start)
> +		return 1;

Suggest calling them hpa's and using this more common kernel compare pattern.
(yeah, in ndctl, cxl/cli, we try to be like kernel)

static int cmp_region_hpa(const void *a, const void *b)
{
        const struct cxl_region *const *r1 = a;
        const struct cxl_region *const *r2 = b;
        u64 hpa1 = cxl_region_get_resource((struct cxl_region *) *r1);
        u64 hpa2 = cxl_region_get_resource((struct cxl_region *) *r2);

        return (hpa1 > hpa2) - (hpa1 < hpa2);
}

> +	return 0;
> +}
> +
> +static int collect_regions_sorted(struct cxl_decoder *root,
> +								  const char *filter,
> +								  struct cxl_region ***out,
> +								  int *out_nr)
> +{

I think there is an alignment issue above.
The filter parameter is always called w NULL in patcht 2.
If it's not going to be used, remove it.

> +	struct cxl_region *region;
> +	struct cxl_region **list = NULL;
> +	int nr = 0, alloc = 0;
> +
> +	cxl_region_foreach(root, region) {
> +		if (filter && !util_cxl_region_filter(region, filter))
> +			continue;
> +		if (nr == alloc) {
> +			int new_alloc = alloc ? alloc * 2 : 8;
> +			int new_size = (size_t)new_alloc * sizeof(*list);

Looks like new_size should be size_t to match what realloc() expects.


> +			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);
> -- 
> 2.45.1.windows.1
>
[RFC PATCH v1 1/2] cxl/cli: add helpers to collect and sort regions by HPA
Posted by Paweł Mielimonka 2 months, 1 week ago 
W dniu 3.12.2025 o 04:52, Alison Schofield pisze:
> On Tue, Nov 25, 2025 at 11:38:23PM +0900, Pawel Mielimonka wrote:
>> Introduce cmp_region_hpa() and collect_regions_sorted() helpers to
>> enumerate CXL regions under a given decoder and sort them by their host
>> physical address.
>> These helpers will be used by the "cxl destroy-region" command to tear
>> down regions in HPA-descending order, i.e. in the reverse order of
>> region creation. This matches the decoder programming requirements from
>> the CXL specification (8.2.4.20.12 - continuous HPA coverage at all
>> times when Lock On Commit is used) and avoids teardown sequences that
>> can leve decoder state inconsistent when the decoder is fully populated
>> (known problem).
>> This patch only adds the helpers; no functional changes is intended yet.
>>
>> Signed-off-by: Pawel Mielimonka <pawel.mielimonka@fujitsu.com>
>> ---
>>   cxl/region.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 55 insertions(+)
>>
>> diff --git a/cxl/region.c b/cxl/region.c
>> index 207cf2d0..58765b3d 100644
>> --- a/cxl/region.c
>> +++ b/cxl/region.c
>> @@ -831,6 +831,61 @@ 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 left_start = cxl_region_get_resource((struct cxl_region *) *left);
>> +	u64 right_start = cxl_region_get_resource((struct cxl_region *) *right);
>> +
>> +	if (left_start < right_start)
>> +		return -1;
>> +	if (left_start > right_start)
>> +		return 1;
> Suggest calling them hpa's and using this more common kernel compare pattern.
> (yeah, in ndctl, cxl/cli, we try to be like kernel)
>
> static int cmp_region_hpa(const void *a, const void *b)
> {
>          const struct cxl_region *const *r1 = a;
>          const struct cxl_region *const *r2 = b;
>          u64 hpa1 = cxl_region_get_resource((struct cxl_region *) *r1);
>          u64 hpa2 = cxl_region_get_resource((struct cxl_region *) *r2);
>
>          return (hpa1 > hpa2) - (hpa1 < hpa2);
> }
I followed the pattern in region.c but you're right, I'll fix it in v2.
>
>> +	return 0;
>> +}
>> +
>> +static int collect_regions_sorted(struct cxl_decoder *root,
>> +								  const char *filter,
>> +								  struct cxl_region ***out,
>> +								  int *out_nr)
>> +{
> I think there is an alignment issue above.
> The filter parameter is always called w NULL in patcht 2.
> If it's not going to be used, remove it.
>
>> +	struct cxl_region *region;
>> +	struct cxl_region **list = NULL;
>> +	int nr = 0, alloc = 0;
>> +
>> +	cxl_region_foreach(root, region) {
>> +		if (filter && !util_cxl_region_filter(region, filter))
>> +			continue;
>> +		if (nr == alloc) {
>> +			int new_alloc = alloc ? alloc * 2 : 8;
>> +			int new_size = (size_t)new_alloc * sizeof(*list);
> Looks like new_size should be size_t to match what realloc() expects.
Both changes I'll make in v2. Thanks for noticing.
>
>> +			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);
>> -- 
>> 2.45.1.windows.1
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.