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

Wei Chen posted 40 patches 4 years, 6 months ago
[XEN RFC PATCH 31/40] xen/x86: move nodes_cover_memory to common
Posted by Wei Chen 4 years, 6 months ago
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);
+			return 0;
+		}
+	}
+	return 1;
+}
+
 void numa_add_cpu(int cpu)
 {
     cpumask_set_cpu(cpu, &node_to_cpumask[cpu_to_node(cpu)]);
diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
index 6d18059bcd..094ab904c9 100644
--- a/xen/include/xen/numa.h
+++ b/xen/include/xen/numa.h
@@ -92,6 +92,7 @@ static inline void clear_node_cpumask(int cpu)
 extern uint32_t arch_meminfo_get_nr_bank(void);
 extern int arch_meminfo_get_ram_bank_range(int bank,
     unsigned long long *start, unsigned long long *end);
+extern int nodes_cover_memory(void);
 
 #endif /* CONFIG_NUMA */
 
-- 
2.25.1


Re: [XEN RFC PATCH 31/40] xen/x86: move nodes_cover_memory to common
Posted by Stefano Stabellini 4 years, 5 months ago
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.

RE: [XEN RFC PATCH 31/40] xen/x86: move nodes_cover_memory to common
Posted by Wei Chen 4 years, 5 months ago
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.