The code in acpi_scan_nodes can be reused for device tree based
NUMA. So we rename acpi_scan_nodes to numa_scan_nodes for a neutral
function name. As acpi_numa variable is available in ACPU based NUMA
system only, we use CONFIG_ACPI_NUMA to protect it.

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

diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
index dcebc7adec..3d4d90a622 100644
--- a/xen/arch/x86/srat.c
+++ b/xen/arch/x86/srat.c
@@ -362,7 +362,7 @@ void __init srat_parse_regions(u64 addr)
 }
 
 /* Use the information discovered above to actually set up the nodes. */
-int __init acpi_scan_nodes(u64 start, u64 end)
+int __init numa_scan_nodes(u64 start, u64 end)
 {
 	int i;
 	nodemask_t all_nodes_parsed;
@@ -371,8 +371,10 @@ int __init acpi_scan_nodes(u64 start, u64 end)
 	for (i = 0; i < MAX_NUMNODES; i++)
 		cutoff_node(i, start, end);
 
+#ifdef CONFIG_ACPI_NUMA
 	if (acpi_numa <= 0)
 		return -1;
+#endif
 
 	if (!nodes_cover_memory()) {
 		bad_srat();
diff --git a/xen/common/numa.c b/xen/common/numa.c
index 74960885a6..4152bbe83b 100644
--- a/xen/common/numa.c
+++ b/xen/common/numa.c
@@ -330,7 +330,7 @@ void __init numa_initmem_init(unsigned long start_pfn, unsigned long end_pfn)
 #endif
 
 #ifdef CONFIG_ACPI_NUMA
-    if ( !numa_off && !acpi_scan_nodes((u64)start_pfn << PAGE_SHIFT,
+    if ( !numa_off && !numa_scan_nodes((u64)start_pfn << PAGE_SHIFT,
          (u64)end_pfn << PAGE_SHIFT) )
         return;
 #endif
diff --git a/xen/include/asm-x86/acpi.h b/xen/include/asm-x86/acpi.h
index d347500a3c..33b71dfb3b 100644
--- a/xen/include/asm-x86/acpi.h
+++ b/xen/include/asm-x86/acpi.h
@@ -102,7 +102,7 @@ extern unsigned long acpi_wakeup_address;
 #define ARCH_HAS_POWER_INIT	1
 
 extern s8 acpi_numa;
-extern int acpi_scan_nodes(u64 start, u64 end);
+extern int numa_scan_nodes(u64 start, u64 end);
 
 extern struct acpi_sleep_info acpi_sinfo;
 #define acpi_video_flags bootsym(video_flags)
-- 
2.25.1

Hi Wei,

On 11/08/2021 11:24, Wei Chen wrote:
> The code in acpi_scan_nodes can be reused for device tree based
> NUMA. So we rename acpi_scan_nodes to numa_scan_nodes for a neutral
> function name. As acpi_numa variable is available in ACPU based NUMA
> system only, we use CONFIG_ACPI_NUMA to protect it.
> 
> Signed-off-by: Wei Chen <wei.chen@arm.com>
> ---
>   xen/arch/x86/srat.c        | 4 +++-
>   xen/common/numa.c          | 2 +-
>   xen/include/asm-x86/acpi.h | 2 +-
>   3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
> index dcebc7adec..3d4d90a622 100644
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -362,7 +362,7 @@ void __init srat_parse_regions(u64 addr)
>   }
>   
>   /* Use the information discovered above to actually set up the nodes. */
> -int __init acpi_scan_nodes(u64 start, u64 end)
> +int __init numa_scan_nodes(u64 start, u64 end)
>   {
>   	int i;
>   	nodemask_t all_nodes_parsed;
> @@ -371,8 +371,10 @@ int __init acpi_scan_nodes(u64 start, u64 end)
>   	for (i = 0; i < MAX_NUMNODES; i++)
>   		cutoff_node(i, start, end);
>   
> +#ifdef CONFIG_ACPI_NUMA
>   	if (acpi_numa <= 0)
>   		return -1;
> +#endif

Looking at the follow-up patches, I find a bit odd that there is a check 
for ACPI but there is none added for DT. Can you explain why?

However, I think this check is going to impair the work to support both 
ACPI and DT on Arm because acpi_numa would end up to be 0 so you would 
bail out here.

With that in mind, I think this check needs to either go away or replace 
by something there is firmware agnostic.

Cheers,

-- 
Julien Grall

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: 2021年8月27日 22:09
> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org;
> sstabellini@kernel.org; jbeulich@suse.com
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>
> Subject: Re: [XEN RFC PATCH 32/40] xen/x86: make acpi_scan_nodes to be
> neutral
> 
> Hi Wei,
> 
> On 11/08/2021 11:24, Wei Chen wrote:
> > The code in acpi_scan_nodes can be reused for device tree based
> > NUMA. So we rename acpi_scan_nodes to numa_scan_nodes for a neutral
> > function name. As acpi_numa variable is available in ACPU based NUMA
> > system only, we use CONFIG_ACPI_NUMA to protect it.
> >
> > Signed-off-by: Wei Chen <wei.chen@arm.com>
> > ---
> >   xen/arch/x86/srat.c        | 4 +++-
> >   xen/common/numa.c          | 2 +-
> >   xen/include/asm-x86/acpi.h | 2 +-
> >   3 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
> > index dcebc7adec..3d4d90a622 100644
> > --- a/xen/arch/x86/srat.c
> > +++ b/xen/arch/x86/srat.c
> > @@ -362,7 +362,7 @@ void __init srat_parse_regions(u64 addr)
> >   }
> >
> >   /* Use the information discovered above to actually set up the nodes.
> */
> > -int __init acpi_scan_nodes(u64 start, u64 end)
> > +int __init numa_scan_nodes(u64 start, u64 end)
> >   {
> >   	int i;
> >   	nodemask_t all_nodes_parsed;
> > @@ -371,8 +371,10 @@ int __init acpi_scan_nodes(u64 start, u64 end)
> >   	for (i = 0; i < MAX_NUMNODES; i++)
> >   		cutoff_node(i, start, end);
> >
> > +#ifdef CONFIG_ACPI_NUMA
> >   	if (acpi_numa <= 0)
> >   		return -1;
> > +#endif
> 
> Looking at the follow-up patches, I find a bit odd that there is a check
> for ACPI but there is none added for DT. Can you explain why?
> 

Oh, I forgot DT check. And simply to add DT check here seems not a
good idea. Because once, when Arm support ACPI NUMA.
CONFIG_ACPI_NUMA and CONFIG_DEVICE_TREE_NUMA can be selected at
the same time. But only acpi_numa or dtb_numa can be > 0.

> However, I think this check is going to impair the work to support both
> ACPI and DT on Arm because acpi_numa would end up to be 0 so you would
> bail out here.
> 
> With that in mind, I think this check needs to either go away or replace
> by something there is firmware agnostic.

Yes, we have discussed about something like fw_numa before.
> 
> Cheers,
> 
> --
> Julien Grall