Mailing List Archive

[PATCH v3 8/9] xen/x86: add detection of memory interleaves for different nodes
One NUMA node may contain several memory blocks. In current Xen
code, Xen will maintain a node memory range for each node to cover
all its memory blocks. But here comes the problem, in the gap of
one node's two memory blocks, if there are some memory blocks don't
belong to this node (remote memory blocks). This node's memory range
will be expanded to cover these remote memory blocks.

One node's memory range contains other nodes' memory, this is
obviously not very reasonable. This means current NUMA code only
can support node has no interleaved memory blocks. However, on a
physical machine, the addresses of multiple nodes can be interleaved.

So in this patch, we add code to detect memory interleaves of
different nodes. NUMA initialization will be failed and error
messages will be printed when Xen detect such hardware configuration.

Signed-off-by: Wei Chen <wei.chen@arm.com>
Tested-by: Jiamei Xie <jiamei.xie@arm.com>
---
v2 -> v3:
1. Merge the check code from a separate function to
conflicting_memblks. This will reduce the loop
times of node memory blocks.
2. Use an enumeration to indicate conflict check status.
3. Use a pointer to get conflict memory block id.
v1 -> v2:
1. Update the description to say we're after is no memory
interleaves of different nodes.
2. Only update node range when it passes the interleave check.
3. Don't use full upper-case for "node".
---
xen/arch/x86/srat.c | 115 +++++++++++++++++++++++++++++++++-----------
1 file changed, 86 insertions(+), 29 deletions(-)

diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
index 8ffe43bdfe..53835ae3eb 100644
--- a/xen/arch/x86/srat.c
+++ b/xen/arch/x86/srat.c
@@ -42,6 +42,12 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS];
static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS);

+enum conflicts {
+ NO_CONFLICT = 0,
+ ERR_OVERLAP,
+ ERR_INTERLEAVE,
+};
+
static inline bool node_found(unsigned idx, unsigned pxm)
{
return ((pxm2node[idx].pxm == pxm) &&
@@ -119,20 +125,43 @@ int valid_numa_range(paddr_t start, paddr_t end, nodeid_t node)
return 0;
}

-static __init int conflicting_memblks(paddr_t start, paddr_t end)
+static enum conflicts __init
+conflicting_memblks(nodeid_t nid, paddr_t start, paddr_t end,
+ paddr_t nd_start, paddr_t nd_end, int *mblkid)
{
int i;

+ /*
+ * Scan all recorded nodes' memory blocks to check conflicts:
+ * Overlap or interleave.
+ */
for (i = 0; i < num_node_memblks; i++) {
struct node *nd = &node_memblk_range[i];
+ *mblkid = i;
+
+ /* Skip 0 bytes node memory block. */
if (nd->start == nd->end)
continue;
+ /*
+ * Use memblk range to check memblk overlaps, include the
+ * self-overlap case.
+ */
if (nd->end > start && nd->start < end)
- return i;
+ return ERR_OVERLAP;
if (nd->end == end && nd->start == start)
- return i;
+ return ERR_OVERLAP;
+ /*
+ * Use node memory range to check whether new range contains
+ * memory from other nodes - interleave check. We just need
+ * to check full contains situation. Because overlaps have
+ * been checked above.
+ */
+ if (nid != memblk_nodeid[i] &&
+ (nd_start < nd->start && nd->end < nd_end))
+ return ERR_INTERLEAVE;
}
- return -1;
+
+ return NO_CONFLICT;
}

static __init void cutoff_node(int i, paddr_t start, paddr_t end)
@@ -275,6 +304,9 @@ acpi_numa_processor_affinity_init(const struct acpi_srat_cpu_affinity *pa)
void __init
acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma)
{
+ enum conflicts status;
+ struct node *nd;
+ paddr_t nd_start, nd_end;
paddr_t start, end;
unsigned pxm;
nodeid_t node;
@@ -310,42 +342,67 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma)
bad_srat();
return;
}
+
+ /*
+ * For the node that already has some memory blocks, we will
+ * expand the node memory range temporarily to check memory
+ * interleaves with other nodes. We will not use this node
+ * temp memory range to check overlaps, because it will mask
+ * the overlaps in same node.
+ *
+ * Node with 0 bytes memory doesn't need this expandsion.
+ */
+ nd_start = start;
+ nd_end = end;
+ nd = &nodes[node];
+ if (nd->start != nd->end) {
+ if (nd_start > nd->start)
+ nd_start = nd->start;
+
+ if (nd_end < end)
+ nd_end = nd->end;
+ }
+
/* It is fine to add this area to the nodes data it will be used later*/
- i = conflicting_memblks(start, end);
- if (i < 0)
- /* everything fine */;
- else if (memblk_nodeid[i] == node) {
- bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
- !test_bit(i, memblk_hotplug);
-
- printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n",
- mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end,
- node_memblk_range[i].start, node_memblk_range[i].end);
- if (mismatch) {
+ status = conflicting_memblks(node, start, end, nd_start, nd_end, &i);
+ if (status == ERR_OVERLAP) {
+ if (memblk_nodeid[i] == node) {
+ bool mismatch = !(ma->flags &
+ ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
+ !test_bit(i, memblk_hotplug);
+
+ printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n",
+ mismatch ? KERN_ERR : KERN_WARNING, pxm, start,
+ end, node_memblk_range[i].start,
+ node_memblk_range[i].end);
+ if (mismatch) {
+ bad_srat();
+ return;
+ }
+ } else {
+ printk(KERN_ERR
+ "SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
+ pxm, start, end, node_to_pxm(memblk_nodeid[i]),
+ node_memblk_range[i].start,
+ node_memblk_range[i].end);
bad_srat();
return;
}
- } else {
+ } else if (status == ERR_INTERLEAVE) {
printk(KERN_ERR
- "SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
- pxm, start, end, node_to_pxm(memblk_nodeid[i]),
+ "SRAT? Node %u: (%"PRIpaddr"-%"PRIpaddr") interleaves with node %u memblk (%"PRIpaddr"-%"PRIpaddr")\n",
+ node, nd_start, nd_end, memblk_nodeid[i],
node_memblk_range[i].start, node_memblk_range[i].end);
bad_srat();
return;
}
- if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
- struct node *nd = &nodes[node];

- if (!node_test_and_set(node, memory_nodes_parsed)) {
- nd->start = start;
- nd->end = end;
- } else {
- if (start < nd->start)
- nd->start = start;
- if (nd->end < end)
- nd->end = end;
- }
+ if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) {
+ node_set(node, memory_nodes_parsed);
+ nd->start = nd_start;
+ nd->end = nd_end;
}
+
printk(KERN_INFO "SRAT: Node %u PXM %u %"PRIpaddr"-%"PRIpaddr"%s\n",
node, pxm, start, end,
ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE ? " (hotplug)" : "");
--
2.25.1
Re: [PATCH v3 8/9] xen/x86: add detection of memory interleaves for different nodes [ In reply to ]
On 11.05.2022 03:46, Wei Chen wrote:
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -42,6 +42,12 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS];
> static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
> static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS);
>
> +enum conflicts {
> + NO_CONFLICT = 0,

No need for the "= 0".

> + ERR_OVERLAP,

While this at least can be an error (the self-overlap case is merely
warned about), ...

> + ERR_INTERLEAVE,

... I don't think this is, and hence I'd recommend to drop "ERR_".

> @@ -119,20 +125,43 @@ int valid_numa_range(paddr_t start, paddr_t end, nodeid_t node)
> return 0;
> }
>
> -static __init int conflicting_memblks(paddr_t start, paddr_t end)
> +static enum conflicts __init
> +conflicting_memblks(nodeid_t nid, paddr_t start, paddr_t end,
> + paddr_t nd_start, paddr_t nd_end, int *mblkid)

Why "int"? Can the value passed back be negative?

> {
> int i;
>
> + /*
> + * Scan all recorded nodes' memory blocks to check conflicts:
> + * Overlap or interleave.
> + */
> for (i = 0; i < num_node_memblks; i++) {
> struct node *nd = &node_memblk_range[i];
> + *mblkid = i;

Style: Please maintain a blank line between declaration(s) and
statement(s).

> @@ -310,42 +342,67 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma)
> bad_srat();
> return;
> }
> +
> + /*
> + * For the node that already has some memory blocks, we will
> + * expand the node memory range temporarily to check memory
> + * interleaves with other nodes. We will not use this node
> + * temp memory range to check overlaps, because it will mask
> + * the overlaps in same node.
> + *
> + * Node with 0 bytes memory doesn't need this expandsion.
> + */
> + nd_start = start;
> + nd_end = end;
> + nd = &nodes[node];
> + if (nd->start != nd->end) {
> + if (nd_start > nd->start)
> + nd_start = nd->start;
> +
> + if (nd_end < end)

Did you mean nd->end here on the right side of < ? By intentionally
not adding "default:" in the body, you then also allow the compiler
to point out that addition of a new enumerator also needs handling
here.

> + nd_end = nd->end;
> + }
> +
> /* It is fine to add this area to the nodes data it will be used later*/
> - i = conflicting_memblks(start, end);
> - if (i < 0)
> - /* everything fine */;
> - else if (memblk_nodeid[i] == node) {
> - bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
> - !test_bit(i, memblk_hotplug);
> -
> - printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n",
> - mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end,
> - node_memblk_range[i].start, node_memblk_range[i].end);
> - if (mismatch) {
> + status = conflicting_memblks(node, start, end, nd_start, nd_end, &i);
> + if (status == ERR_OVERLAP) {

Please use switch(status) when checking enumerated values.

> + if (memblk_nodeid[i] == node) {
> + bool mismatch = !(ma->flags &
> + ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
> + !test_bit(i, memblk_hotplug);

Style: The middle line wants indenting by two more characters.

> +
> + printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n",
> + mismatch ? KERN_ERR : KERN_WARNING, pxm, start,
> + end, node_memblk_range[i].start,
> + node_memblk_range[i].end);
> + if (mismatch) {
> + bad_srat();
> + return;
> + }
> + } else {
> + printk(KERN_ERR
> + "SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
> + pxm, start, end, node_to_pxm(memblk_nodeid[i]),
> + node_memblk_range[i].start,
> + node_memblk_range[i].end);
> bad_srat();
> return;
> }
> - } else {
> + } else if (status == ERR_INTERLEAVE) {
> printk(KERN_ERR
> - "SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
> - pxm, start, end, node_to_pxm(memblk_nodeid[i]),
> + "SRAT? Node %u: (%"PRIpaddr"-%"PRIpaddr") interleaves with node %u memblk (%"PRIpaddr"-%"PRIpaddr")\n",
> + node, nd_start, nd_end, memblk_nodeid[i],

Please log pxm (not node) here just like is done in the overlap case.
The remote node ID will then require converting to PXM, of course.

Jan
RE: [PATCH v3 8/9] xen/x86: add detection of memory interleaves for different nodes [ In reply to ]
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022?5?18? 21:31
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: nd <nd@arm.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau
> Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
> <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v3 8/9] xen/x86: add detection of memory interleaves
> for different nodes
>
> On 11.05.2022 03:46, Wei Chen wrote:
> > --- a/xen/arch/x86/srat.c
> > +++ b/xen/arch/x86/srat.c
> > @@ -42,6 +42,12 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS];
> > static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS];
> > static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS);
> >
> > +enum conflicts {
> > + NO_CONFLICT = 0,
>
> No need for the "= 0".
>

Ack.

> > + ERR_OVERLAP,
>
> While this at least can be an error (the self-overlap case is merely
> warned about), ...
>
> > + ERR_INTERLEAVE,
>
> ... I don't think this is, and hence I'd recommend to drop "ERR_".
>

Oh, yes. I all drop it for all above enumerations.

> > @@ -119,20 +125,43 @@ int valid_numa_range(paddr_t start, paddr_t end,
> nodeid_t node)
> > return 0;
> > }
> >
> > -static __init int conflicting_memblks(paddr_t start, paddr_t end)
> > +static enum conflicts __init
> > +conflicting_memblks(nodeid_t nid, paddr_t start, paddr_t end,
> > + paddr_t nd_start, paddr_t nd_end, int *mblkid)
>
> Why "int"? Can the value passed back be negative?
>

The caller "acpi_numa_memory_affinity_init" defines int for node memory
block id, and as a return value at the same time. So I haven't changed it.
As we don't use this "int" for return value any more, I agree, it will
never be negative, I would fix it.

> > {
> > int i;
> >
> > + /*
> > + * Scan all recorded nodes' memory blocks to check conflicts:
> > + * Overlap or interleave.
> > + */
> > for (i = 0; i < num_node_memblks; i++) {
> > struct node *nd = &node_memblk_range[i];
> > + *mblkid = i;
>
> Style: Please maintain a blank line between declaration(s) and
> statement(s).
>

Ok.

> > @@ -310,42 +342,67 @@ acpi_numa_memory_affinity_init(const struct
> acpi_srat_mem_affinity *ma)
> > bad_srat();
> > return;
> > }
> > +
> > + /*
> > + * For the node that already has some memory blocks, we will
> > + * expand the node memory range temporarily to check memory
> > + * interleaves with other nodes. We will not use this node
> > + * temp memory range to check overlaps, because it will mask
> > + * the overlaps in same node.
> > + *
> > + * Node with 0 bytes memory doesn't need this expandsion.
> > + */
> > + nd_start = start;
> > + nd_end = end;
> > + nd = &nodes[node];
> > + if (nd->start != nd->end) {
> > + if (nd_start > nd->start)
> > + nd_start = nd->start;
> > +
> > + if (nd_end < end)
>
> Did you mean nd->end here on the right side of < ? By intentionally

Oh! thanks for pointing out this one! Yes, right side should be nd->end.

> not adding "default:" in the body, you then also allow the compiler
> to point out that addition of a new enumerator also needs handling
> here.
>

Did you mean, we need to add if ... else ... in this block? If yes,
is it ok to update this block like:
if (nd->start != nd->end) {
nd_start = min(nd_start, nd->start);
nd_end = max(nd_end, nd->end);
}
?

> > + nd_end = nd->end;
> > + }
> > +
> > /* It is fine to add this area to the nodes data it will be used
> later*/
> > - i = conflicting_memblks(start, end);
> > - if (i < 0)
> > - /* everything fine */;
> > - else if (memblk_nodeid[i] == node) {
> > - bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
> > - !test_bit(i, memblk_hotplug);
> > -
> > - printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with
> itself (%"PRIpaddr"-%"PRIpaddr")\n",
> > - mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end,
> > - node_memblk_range[i].start, node_memblk_range[i].end);
> > - if (mismatch) {
> > + status = conflicting_memblks(node, start, end, nd_start, nd_end, &i);
> > + if (status == ERR_OVERLAP) {
>
> Please use switch(status) when checking enumerated values.
>

Ok, I will do it.

> > + if (memblk_nodeid[i] == node) {
> > + bool mismatch = !(ma->flags &
> > + ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
> > + !test_bit(i, memblk_hotplug);
>
> Style: The middle line wants indenting by two more characters.
>

Yes, I will do it.

> > +
> > + printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr")
> overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n",
> > + mismatch ? KERN_ERR : KERN_WARNING, pxm, start,
> > + end, node_memblk_range[i].start,
> > + node_memblk_range[i].end);
> > + if (mismatch) {
> > + bad_srat();
> > + return;
> > + }
> > + } else {
> > + printk(KERN_ERR
> > + "SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps
> with PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
> > + pxm, start, end, node_to_pxm(memblk_nodeid[i]),
> > + node_memblk_range[i].start,
> > + node_memblk_range[i].end);
> > bad_srat();
> > return;
> > }
> > - } else {
> > + } else if (status == ERR_INTERLEAVE) {
> > printk(KERN_ERR
> > - "SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with
> PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
> > - pxm, start, end, node_to_pxm(memblk_nodeid[i]),
> > + "SRAT? Node %u: (%"PRIpaddr"-%"PRIpaddr") interleaves
> with node %u memblk (%"PRIpaddr"-%"PRIpaddr")\n",
> > + node, nd_start, nd_end, memblk_nodeid[i],
>
> Please log pxm (not node) here just like is done in the overlap case.
> The remote node ID will then require converting to PXM, of course.
>

Ok, will use PXM here. But I have question for upcoming changes, if we
move this part of code to common. As device tree NUMA doesn't have
PXM concept (even I can use a fake node_to_pxm to do 1:1 mapping), so
can we still use PXM here?

> Jan
Re: [PATCH v3 8/9] xen/x86: add detection of memory interleaves for different nodes [ In reply to ]
On 19.05.2022 05:37, Wei Chen wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 2022?5?18? 21:31
>>
>> On 11.05.2022 03:46, Wei Chen wrote:
>>> @@ -310,42 +342,67 @@ acpi_numa_memory_affinity_init(const struct
>> acpi_srat_mem_affinity *ma)
>>> bad_srat();
>>> return;
>>> }
>>> +
>>> + /*
>>> + * For the node that already has some memory blocks, we will
>>> + * expand the node memory range temporarily to check memory
>>> + * interleaves with other nodes. We will not use this node
>>> + * temp memory range to check overlaps, because it will mask
>>> + * the overlaps in same node.
>>> + *
>>> + * Node with 0 bytes memory doesn't need this expandsion.
>>> + */
>>> + nd_start = start;
>>> + nd_end = end;
>>> + nd = &nodes[node];
>>> + if (nd->start != nd->end) {
>>> + if (nd_start > nd->start)
>>> + nd_start = nd->start;
>>> +
>>> + if (nd_end < end)
>>
>> Did you mean nd->end here on the right side of < ? By intentionally
>
> Oh! thanks for pointing out this one! Yes, right side should be nd->end.
>
>> not adding "default:" in the body, you then also allow the compiler
>> to point out that addition of a new enumerator also needs handling
>> here.
>>
>
> Did you mean, we need to add if ... else ... in this block? If yes,
> is it ok to update this block like:
> if (nd->start != nd->end) {
> nd_start = min(nd_start, nd->start);
> nd_end = max(nd_end, nd->end);
> }
> ?

No. I attached this part about "default:" late in the process of writing
the reply, and I did put it in the wrong spot. I'm sorry. It really was
meant to go ...

>>> + nd_end = nd->end;
>>> + }
>>> +
>>> /* It is fine to add this area to the nodes data it will be used
>> later*/
>>> - i = conflicting_memblks(start, end);
>>> - if (i < 0)
>>> - /* everything fine */;
>>> - else if (memblk_nodeid[i] == node) {
>>> - bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) !=
>>> - !test_bit(i, memblk_hotplug);
>>> -
>>> - printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with
>> itself (%"PRIpaddr"-%"PRIpaddr")\n",
>>> - mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end,
>>> - node_memblk_range[i].start, node_memblk_range[i].end);
>>> - if (mismatch) {
>>> + status = conflicting_memblks(node, start, end, nd_start, nd_end, &i);
>>> + if (status == ERR_OVERLAP) {
>>
>> Please use switch(status) when checking enumerated values.
>>
>
> Ok, I will do it.

... here, explaining the request to use switch().

>>> + printk("%sSRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr")
>> overlaps with itself (%"PRIpaddr"-%"PRIpaddr")\n",
>>> + mismatch ? KERN_ERR : KERN_WARNING, pxm, start,
>>> + end, node_memblk_range[i].start,
>>> + node_memblk_range[i].end);
>>> + if (mismatch) {
>>> + bad_srat();
>>> + return;
>>> + }
>>> + } else {
>>> + printk(KERN_ERR
>>> + "SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps
>> with PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
>>> + pxm, start, end, node_to_pxm(memblk_nodeid[i]),
>>> + node_memblk_range[i].start,
>>> + node_memblk_range[i].end);
>>> bad_srat();
>>> return;
>>> }
>>> - } else {
>>> + } else if (status == ERR_INTERLEAVE) {
>>> printk(KERN_ERR
>>> - "SRAT: PXM %u (%"PRIpaddr"-%"PRIpaddr") overlaps with
>> PXM %u (%"PRIpaddr"-%"PRIpaddr")\n",
>>> - pxm, start, end, node_to_pxm(memblk_nodeid[i]),
>>> + "SRAT? Node %u: (%"PRIpaddr"-%"PRIpaddr") interleaves
>> with node %u memblk (%"PRIpaddr"-%"PRIpaddr")\n",
>>> + node, nd_start, nd_end, memblk_nodeid[i],
>>
>> Please log pxm (not node) here just like is done in the overlap case.
>> The remote node ID will then require converting to PXM, of course.
>>
>
> Ok, will use PXM here. But I have question for upcoming changes, if we
> move this part of code to common. As device tree NUMA doesn't have
> PXM concept (even I can use a fake node_to_pxm to do 1:1 mapping), so
> can we still use PXM here?

This will want properly abstracting once made common, yes. What the correct
model on Arm/DT is I can't really tell. But my (earlier voiced) request
remains: What is logged should by referring the firmware provided values,
not Xen-internal ones. Otherwise someone reading the log cannot easily
know / derive what's wrong where.

Jan