diff mbox series

[14/37] xen/x86: use name fw_numa to replace acpi_numa

Message ID 20210923120236.3692135-15-wei.chen@arm.com (mailing list archive)
State New, archived
Headers show
Series Add device tree based NUMA support to Arm | expand

Commit Message

Wei Chen Sept. 23, 2021, 12:02 p.m. UTC 
Xen is using acpi_numa as a switch for ACPI based NUMA. We want to
use this switch logic for other firmware based NUMA implementation,
like device tree based NUMA in follow-up patches. As Xen will never
use both ACPI and device tree based NUMA at runtime. So I rename
acpi_numa to a more generic name fw_name. This will also allow to
have the code mostly common.

Signed-off-by: Wei Chen <wei.chen@arm.com>
---
 xen/arch/x86/numa.c        |  6 +++---
 xen/arch/x86/setup.c       |  2 +-
 xen/arch/x86/srat.c        | 10 +++++-----
 xen/include/asm-x86/acpi.h |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

Comments

Stefano Stabellini Sept. 24, 2021, 12:40 a.m. UTC | #1 
+x86 maintainers


On Thu, 23 Sep 2021, Wei Chen wrote:
> Xen is using acpi_numa as a switch for ACPI based NUMA. We want to
> use this switch logic for other firmware based NUMA implementation,
> like device tree based NUMA in follow-up patches. As Xen will never
> use both ACPI and device tree based NUMA at runtime. So I rename
> acpi_numa to a more generic name fw_name. This will also allow to
> have the code mostly common.
> 
> Signed-off-by: Wei Chen <wei.chen@arm.com>
> ---
>  xen/arch/x86/numa.c        |  6 +++---
>  xen/arch/x86/setup.c       |  2 +-
>  xen/arch/x86/srat.c        | 10 +++++-----
>  xen/include/asm-x86/acpi.h |  2 +-
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
> index 6bc4ade411..2ef385ae3f 100644
> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -51,11 +51,11 @@ cpumask_t node_to_cpumask[MAX_NUMNODES] __read_mostly;
>  nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
>  
>  bool numa_off;
> -s8 acpi_numa = 0;
> +s8 fw_numa = 0;
>  
>  int srat_disabled(void)
>  {
> -    return numa_off || acpi_numa < 0;
> +    return numa_off || fw_numa < 0;
>  }
>  
>  /*
> @@ -315,7 +315,7 @@ static __init int numa_setup(const char *opt)
>      else if ( !strncmp(opt,"noacpi",6) )
>      {
>          numa_off = false;
> -        acpi_numa = -1;
> +        fw_numa = -1;
>      }
>  #endif
>      else
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index b101565f14..1a2093b554 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -313,7 +313,7 @@ void srat_detect_node(int cpu)
>      node_set_online(node);
>      numa_set_node(cpu, node);
>  
> -    if ( opt_cpu_info && acpi_numa > 0 )
> +    if ( opt_cpu_info && fw_numa > 0 )
>          printk("CPU %d APIC %d -> Node %d\n", cpu, apicid, node);
>  }
>  
> diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
> index 9276a52138..4921830f94 100644
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -167,7 +167,7 @@ static __init void bad_srat(void)
>  {
>  	int i;
>  	printk(KERN_ERR "SRAT: SRAT not used.\n");
> -	acpi_numa = -1;
> +	fw_numa = -1;
>  	for (i = 0; i < MAX_LOCAL_APIC; i++)
>  		apicid_to_node[i] = NUMA_NO_NODE;
>  	for (i = 0; i < ARRAY_SIZE(pxm2node); i++)
> @@ -242,7 +242,7 @@ acpi_numa_x2apic_affinity_init(const struct acpi_srat_x2apic_cpu_affinity *pa)
>  
>  	apicid_to_node[pa->apic_id] = node;
>  	numa_set_processor_nodes_parsed(node);
> -	acpi_numa = 1;
> +	fw_numa = 1;
>  
>  	if (opt_acpi_verbose)
>  		printk(KERN_INFO "SRAT: PXM %u -> APIC %08x -> Node %u\n",
> @@ -277,7 +277,7 @@ acpi_numa_processor_affinity_init(const struct acpi_srat_cpu_affinity *pa)
>  	}
>  	apicid_to_node[pa->apic_id] = node;
>  	numa_set_processor_nodes_parsed(node);
> -	acpi_numa = 1;
> +	fw_numa = 1;
>  
>  	if (opt_acpi_verbose)
>  		printk(KERN_INFO "SRAT: PXM %u -> APIC %02x -> Node %u\n",
> @@ -492,7 +492,7 @@ void __init srat_parse_regions(paddr_t addr)
>  	u64 mask;
>  	unsigned int i;
>  
> -	if (acpi_disabled || acpi_numa < 0 ||
> +	if (acpi_disabled || fw_numa < 0 ||
>  	    acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat))
>  		return;
>  
> @@ -521,7 +521,7 @@ int __init acpi_scan_nodes(paddr_t start, paddr_t end)
>  	for (i = 0; i < MAX_NUMNODES; i++)
>  		cutoff_node(i, start, end);
>  
> -	if (acpi_numa <= 0)
> +	if (fw_numa <= 0)
>  		return -1;
>  
>  	if (!nodes_cover_memory()) {
> diff --git a/xen/include/asm-x86/acpi.h b/xen/include/asm-x86/acpi.h
> index 7032f3a001..83be71fec3 100644
> --- a/xen/include/asm-x86/acpi.h
> +++ b/xen/include/asm-x86/acpi.h
> @@ -101,7 +101,7 @@ extern unsigned long acpi_wakeup_address;
>  
>  #define ARCH_HAS_POWER_INIT	1
>  
> -extern s8 acpi_numa;
> +extern s8 fw_numa;
>  extern int acpi_scan_nodes(u64 start, u64 end);
>  #define NR_NODE_MEMBLKS (MAX_NUMNODES*2)
>  
> -- 
> 2.25.1
>
Jan Beulich Jan. 25, 2022, 10:12 a.m. UTC | #2 
On 23.09.2021 14:02, Wei Chen wrote:
> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -51,11 +51,11 @@ cpumask_t node_to_cpumask[MAX_NUMNODES] __read_mostly;
>  nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
>  
>  bool numa_off;
> -s8 acpi_numa = 0;
> +s8 fw_numa = 0;

In x86 code I'd prefer this to remain "acpi_numa". If you need to access
the variable from to-become-generic code, introduce an inline wrapper
(possibly named numa_mode()), allowing you to do whatever you need in the
DT case. It may be helpful to fold this with numa_off then, seeing e.g.
...

>  int srat_disabled(void)
>  {
> -    return numa_off || acpi_numa < 0;
> +    return numa_off || fw_numa < 0;

... this. Actually I think the underlying enumeration could even be made
generic:

enum numa_mode {
    numa_off,
    numa_on,
    numa_acpi,
};

is, I believe, sufficient to express the present (numa_off,acpi_numa)
tuple. In this context I'd like to point out that the two uses of
acpi_numa in srat_parse_regions() and srat_detect_node() should likely
be invocations of srat_disabled() instead, to also take numa_off into
account. This would then be addressed effectively as a side effect by
replacing open-coded uses as well as srat_disabled() by numa_mode() (or
whichever name the new helper would gain).

Jan
Wei Chen Jan. 27, 2022, 8:09 a.m. UTC | #3 
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年1月25日 18:13
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen-
> devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org
> Subject: Re: [PATCH 14/37] xen/x86: use name fw_numa to replace acpi_numa
> 
> On 23.09.2021 14:02, Wei Chen wrote:
> > --- a/xen/arch/x86/numa.c
> > +++ b/xen/arch/x86/numa.c
> > @@ -51,11 +51,11 @@ cpumask_t node_to_cpumask[MAX_NUMNODES]
> __read_mostly;
> >  nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
> >
> >  bool numa_off;
> > -s8 acpi_numa = 0;
> > +s8 fw_numa = 0;
> 
> In x86 code I'd prefer this to remain "acpi_numa". If you need to access
> the variable from to-become-generic code, introduce an inline wrapper
> (possibly named numa_mode()), allowing you to do whatever you need in the
> DT case. It may be helpful to fold this with numa_off then, seeing e.g.
> ...
> 
> >  int srat_disabled(void)
> >  {
> > -    return numa_off || acpi_numa < 0;
> > +    return numa_off || fw_numa < 0;
> 
> ... this. Actually I think the underlying enumeration could even be made
> generic:
> 
> enum numa_mode {
>     numa_off,
>     numa_on,
>     numa_acpi,
> };
> 
> is, I believe, sufficient to express the present (numa_off,acpi_numa)
> tuple. In this context I'd like to point out that the two uses of
> acpi_numa in srat_parse_regions() and srat_detect_node() should likely
> be invocations of srat_disabled() instead, to also take numa_off into
> account. This would then be addressed effectively as a side effect by
> replacing open-coded uses as well as srat_disabled() by numa_mode() (or
> whichever name the new helper would gain).
> 

Yes, that seems more reasonable, I will try this in next version.

> Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index 6bc4ade411..2ef385ae3f 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -51,11 +51,11 @@  cpumask_t node_to_cpumask[MAX_NUMNODES] __read_mostly;
 nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
 
 bool numa_off;
-s8 acpi_numa = 0;
+s8 fw_numa = 0;
 
 int srat_disabled(void)
 {
-    return numa_off || acpi_numa < 0;
+    return numa_off || fw_numa < 0;
 }
 
 /*
@@ -315,7 +315,7 @@  static __init int numa_setup(const char *opt)
     else if ( !strncmp(opt,"noacpi",6) )
     {
         numa_off = false;
-        acpi_numa = -1;
+        fw_numa = -1;
     }
 #endif
     else
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b101565f14..1a2093b554 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -313,7 +313,7 @@  void srat_detect_node(int cpu)
     node_set_online(node);
     numa_set_node(cpu, node);
 
-    if ( opt_cpu_info && acpi_numa > 0 )
+    if ( opt_cpu_info && fw_numa > 0 )
         printk("CPU %d APIC %d -> Node %d\n", cpu, apicid, node);
 }
 
diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
index 9276a52138..4921830f94 100644
--- a/xen/arch/x86/srat.c
+++ b/xen/arch/x86/srat.c
@@ -167,7 +167,7 @@  static __init void bad_srat(void)
 {
 	int i;
 	printk(KERN_ERR "SRAT: SRAT not used.\n");
-	acpi_numa = -1;
+	fw_numa = -1;
 	for (i = 0; i < MAX_LOCAL_APIC; i++)
 		apicid_to_node[i] = NUMA_NO_NODE;
 	for (i = 0; i < ARRAY_SIZE(pxm2node); i++)
@@ -242,7 +242,7 @@  acpi_numa_x2apic_affinity_init(const struct acpi_srat_x2apic_cpu_affinity *pa)
 
 	apicid_to_node[pa->apic_id] = node;
 	numa_set_processor_nodes_parsed(node);
-	acpi_numa = 1;
+	fw_numa = 1;
 
 	if (opt_acpi_verbose)
 		printk(KERN_INFO "SRAT: PXM %u -> APIC %08x -> Node %u\n",
@@ -277,7 +277,7 @@  acpi_numa_processor_affinity_init(const struct acpi_srat_cpu_affinity *pa)
 	}
 	apicid_to_node[pa->apic_id] = node;
 	numa_set_processor_nodes_parsed(node);
-	acpi_numa = 1;
+	fw_numa = 1;
 
 	if (opt_acpi_verbose)
 		printk(KERN_INFO "SRAT: PXM %u -> APIC %02x -> Node %u\n",
@@ -492,7 +492,7 @@  void __init srat_parse_regions(paddr_t addr)
 	u64 mask;
 	unsigned int i;
 
-	if (acpi_disabled || acpi_numa < 0 ||
+	if (acpi_disabled || fw_numa < 0 ||
 	    acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat))
 		return;
 
@@ -521,7 +521,7 @@  int __init acpi_scan_nodes(paddr_t start, paddr_t end)
 	for (i = 0; i < MAX_NUMNODES; i++)
 		cutoff_node(i, start, end);
 
-	if (acpi_numa <= 0)
+	if (fw_numa <= 0)
 		return -1;
 
 	if (!nodes_cover_memory()) {
diff --git a/xen/include/asm-x86/acpi.h b/xen/include/asm-x86/acpi.h
index 7032f3a001..83be71fec3 100644
--- a/xen/include/asm-x86/acpi.h
+++ b/xen/include/asm-x86/acpi.h
@@ -101,7 +101,7 @@  extern unsigned long acpi_wakeup_address;
 
 #define ARCH_HAS_POWER_INIT	1
 
-extern s8 acpi_numa;
+extern s8 fw_numa;
 extern int acpi_scan_nodes(u64 start, u64 end);
 #define NR_NODE_MEMBLKS (MAX_NUMNODES*2)