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

[Alison Schofield <alison.schofield@intel.com>]

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
>