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

[Wei Chen <Wei.Chen@arm.com>]

Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: 2021年8月31日 9:17
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: xen-devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org;
> jbeulich@suse.com; Bertrand Marquis <Bertrand.Marquis@arm.com>
> 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@arm.com>
> > ---
> >  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.