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

["Paweł Mielimonka" <pawel.mielimonka@gmail.com>]

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
>>