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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.