[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7eaf281d-6b92-4d8b-a633-e81925116364@gmail.com>
Date: Wed, 3 Dec 2025 10:09:15 +0100
From: Paweł Mielimonka <pawel.mielimonka@...il.com>
To: Alison Schofield <alison.schofield@...el.com>,
Pawel Mielimonka <pawel.mielimonka@...itsu.com>
Cc: dan.j.williams@...el.com, Smita.KoralahalliChannabasappa@....com,
linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org, dave@...olabs.net,
jonathan.cameron@...wei.com, dave.jiang@...el.com, vishal.l.verma@...el.com,
ira.weiny@...el.com
Subject: [RFC PATCH v1 1/2] cxl/cli: add helpers to collect and sort regions
by HPA
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@...itsu.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
>>
Powered by blists - more mailing lists