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

[Wei Chen <Wei.Chen@xxxxxxx>]

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Sent: 2021年9月24日 8:26
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx;
> Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; jbeulich@xxxxxxxx;
> andrew.cooper3@xxxxxxxxxx; roger.pau@xxxxxxxxxx; wl@xxxxxxx
> Subject: Re: [PATCH 08/37] xen/x86: add detection of discontinous node
> memory range
> 
> 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.
> 
> 

Should I need to include/refine above message to commit log?

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