Re: [PATCH 08/37] xen/x86: add detection of discontinous node memory range

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

CC'ing x86 maintainers

On Thu, 23 Sep 2021, Wei Chen wrote:
> 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 othe nodes' memory, this is obviously
> not very reasonable. This means current NUMA code only can support
> node has continous memory blocks. However, on a physical machine, the
> addresses of multiple nodes can be interleaved.
> 
> So in this patch, we add code to detect discontinous memory blocks
> for one node. NUMA initializtion will be failed and error messages
> will be printed when Xen detect such hardware configuration.

At least on ARM, it is not just memory that can be interleaved, but also
MMIO regions. For instance:

node0 bank0 0-0x1000000
MMIO 0x1000000-0x1002000
Hole 0x1002000-0x2000000
node0 bank1 0x2000000-0x3000000

So I am not familiar with the SRAT format, but I think on ARM the check
would look different: we would just look for multiple memory ranges
under a device_type = "memory" node of a NUMA node in device tree.



> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> ---
>  xen/arch/x86/srat.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
> index 7d20d7f222..2f08fa4660 100644
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -271,6 +271,36 @@ acpi_numa_processor_affinity_init(const struct 
> acpi_srat_cpu_affinity *pa)
>                      pxm, pa->apic_id, node);
>  }
>  
> +/*
> + * Check to see if there are other nodes within this node's range.
> + * We just need to check full contains situation. Because overlaps
> + * have been checked before by conflicting_memblks.
> + */
> +static bool __init is_node_memory_continuous(nodeid_t nid,
> +    paddr_t start, paddr_t end)
> +{
> +     nodeid_t i;
> +
> +     struct node *nd = &nodes[nid];
> +     for_each_node_mask(i, memory_nodes_parsed)
> +     {
> +             /* Skip itself */
> +             if (i == nid)
> +                     continue;
> +
> +             nd = &nodes[i];
> +             if (start < nd->start && nd->end < end)
> +             {
> +                     printk(KERN_ERR
> +                            "NODE %u: (%"PRIpaddr"-%"PRIpaddr") intertwine 
> with NODE %u (%"PRIpaddr"-%"PRIpaddr")\n",
> +                            nid, start, end, i, nd->start, nd->end);
> +                     return false;
> +             }
> +     }
> +
> +     return true;
> +}
> +
>  /* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
>  void __init
>  acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma)
> @@ -344,6 +374,12 @@ acpi_numa_memory_affinity_init(const struct 
> acpi_srat_mem_affinity *ma)
>                               nd->start = start;
>                       if (nd->end < end)
>                               nd->end = end;
> +
> +                     /* Check whether this range contains memory for other 
> nodes */
> +                     if (!is_node_memory_continuous(node, nd->start, 
> nd->end)) {
> +                             bad_srat();
> +                             return;
> +                     }
>               }
>       }
>       printk(KERN_INFO "SRAT: Node %u PXM %u %"PRIpaddr"-%"PRIpaddr"%s\n",
> -- 
> 2.25.1
>