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

[Paweł Mielimonka]

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