RE: [XEN RFC PATCH 31/40] xen/x86: move nodes_cover_memory to common

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Sent: 2021年8月31日 9:17
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx;
> jbeulich@xxxxxxxx; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [XEN RFC PATCH 31/40] xen/x86: move nodes_cover_memory to
> common
> 
> On Wed, 11 Aug 2021, Wei Chen wrote:
> > Not only ACPU NUMA, but also Arm device tree based NUMA
> > will use nodes_cover_memory to do sanity check. So we move
> > this function from arch/x86 to common.
> >
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > ---
> >  xen/arch/x86/srat.c    | 40 ----------------------------------------
> >  xen/common/numa.c      | 40 ++++++++++++++++++++++++++++++++++++++++
> >  xen/include/xen/numa.h |  1 +
> >  3 files changed, 41 insertions(+), 40 deletions(-)
> >
> > diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
> > index dd3aa30843..dcebc7adec 100644
> > --- a/xen/arch/x86/srat.c
> > +++ b/xen/arch/x86/srat.c
> > @@ -308,46 +308,6 @@ acpi_numa_memory_affinity_init(const struct
> acpi_srat_mem_affinity *ma)
> >     num_node_memblks++;
> >  }
> >
> > -/* Sanity check to catch more bad SRATs (they are amazingly common).
> > -   Make sure the PXMs cover all memory. */
> > -static int __init nodes_cover_memory(void)
> > -{
> > -   int i;
> > -   uint32_t nr_banks = arch_meminfo_get_nr_bank();
> > -
> > -   for (i = 0; i < nr_banks; i++) {
> > -           int j, found;
> > -           unsigned long long start, end;
> > -
> > -           if (arch_meminfo_get_ram_bank_range(i, &start, &end)) {
> > -                   continue;
> > -           }
> > -
> > -           do {
> > -                   found = 0;
> > -                   for_each_node_mask(j, memory_nodes_parsed)
> > -                           if (start < nodes[j].end
> > -                               && end > nodes[j].start) {
> > -                                   if (start >= nodes[j].start) {
> > -                                           start = nodes[j].end;
> > -                                           found = 1;
> > -                                   }
> > -                                   if (end <= nodes[j].end) {
> > -                                           end = nodes[j].start;
> > -                                           found = 1;
> > -                                   }
> > -                           }
> > -           } while (found && start < end);
> > -
> > -           if (start < end) {
> > -                   printk(KERN_ERR "SRAT: No PXM for e820 range: "
> > -                           "%016Lx - %016Lx\n", start, end);
> > -                   return 0;
> > -           }
> > -   }
> > -   return 1;
> > -}
> > -
> >  void __init acpi_numa_arch_fixup(void) {}
> >
> >  static uint64_t __initdata srat_region_mask;
> > diff --git a/xen/common/numa.c b/xen/common/numa.c
> > index 79ab250543..74960885a6 100644
> > --- a/xen/common/numa.c
> > +++ b/xen/common/numa.c
> > @@ -193,6 +193,46 @@ void __init cutoff_node(int i, u64 start, u64 end)
> >     }
> >  }
> >
> > +/* Sanity check to catch more bad SRATs (they are amazingly common).
> > +   Make sure the PXMs cover all memory. */
> > +int __init nodes_cover_memory(void)
> > +{
> > +   int i;
> > +   uint32_t nr_banks = arch_meminfo_get_nr_bank();
> > +
> > +   for (i = 0; i < nr_banks; i++) {
> > +           int j, found;
> > +           unsigned long long start, end;
> > +
> > +           if (arch_meminfo_get_ram_bank_range(i, &start, &end)) {
> > +                   continue;
> > +           }
> > +
> > +           do {
> > +                   found = 0;
> > +                   for_each_node_mask(j, memory_nodes_parsed)
> > +                           if (start < nodes[j].end
> > +                               && end > nodes[j].start) {
> > +                                   if (start >= nodes[j].start) {
> > +                                           start = nodes[j].end;
> > +                                           found = 1;
> > +                                   }
> > +                                   if (end <= nodes[j].end) {
> > +                                           end = nodes[j].start;
> > +                                           found = 1;
> > +                                   }
> > +                           }
> > +           } while (found && start < end);
> > +
> > +           if (start < end) {
> > +                   printk(KERN_ERR "SRAT: No PXM for e820 range: "
> > +                           "%016Lx - %016Lx\n", start, end);
> 
> I don't know if you are already doing this in a later patch but the
> message shouldn't say e820 as it doesn't exist on all architecture.
> Maybe "for address range" or "for memory range" would suffice.
> 
> Normally we don't do change together with code movement but in this case
> I think it would be OK.

OK, I will do it in next version.