Re: [XEN RFC PATCH 21/40] xen/arm: introduce device_tree_numa as a switch for device tree NUMA

[Julien Grall <julien@xxxxxxx>]

Hi Wei,

On 11/08/2021 11:24, Wei Chen wrote:
> Like acpi_numa in x86 as a switch for ACPI based NUMA, we introduce
> device_tree_numa as a switch for Arm device tree based NUMA. When
> NUMA information in device tree is invalid, this switch will be set
> to -1, then NUMA support for Arm will be disabled, even if user set
> numa_off=0.
The hypervisor will never use both ACPI and DT at runtime. In fact...

> Keep using bad_srat and srat_disabled functions name, because we will
> reuse node_covers_memory and acpi_scan_nodes code for Arm.
... given that both functions will be called from the common code, it 
will be a lot more difficult to add ACPI afterwards.
So I think we should either rename acpi_numa to something more generic 
(maybe fw_numa) or convert numa_off to a tri-state.
This will allow to have the code mostly common.

> These
> functions are using these two API names. And, as device tree can be
> treated as one kind of static resource table. So we keep these two
> function names.
>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> ---
>   xen/arch/arm/Makefile           |  1 +
>   xen/arch/arm/numa_device_tree.c | 35 +++++++++++++++++++++++++++++++++
>   xen/include/asm-arm/numa.h      |  2 ++
>   3 files changed, 38 insertions(+)
>   create mode 100644 xen/arch/arm/numa_device_tree.c
>
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 6e3fb8033e..13e1549be0 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -36,6 +36,7 @@ obj-y += mem_access.o
>   obj-y += mm.o
>   obj-y += monitor.o
>   obj-$(CONFIG_NUMA) += numa.o
> +obj-$(CONFIG_DEVICE_TREE_NUMA) += numa_device_tree.o
>   obj-y += p2m.o
>   obj-y += percpu.o
>   obj-y += platform.o
> diff --git a/xen/arch/arm/numa_device_tree.c b/xen/arch/arm/numa_device_tree.c
> new file mode 100644
> index 0000000000..1c74ad135d
> --- /dev/null
> +++ b/xen/arch/arm/numa_device_tree.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Arm Architecture support layer for NUMA.
> + *
> + * Copyright (C) 2021 Arm Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +#include <xen/init.h>
> +#include <xen/nodemask.h>
> +#include <xen/numa.h>
> +
> +s8 device_tree_numa = 0;
> +
> +int srat_disabled(void)
You export this one and ...

> +void __init bad_srat(void)
... this one without providing in a prototype.

Looking at the rest of the series... they will be turned static in the 
next patch (#21) but then re-exported in patch #33.
In general, we should refrain to modify code that was added in the same 
patch unless it is not possible for split reason (e.g code clean-up and 
then code movement).
In this case, the helpers should be exported from now.

> +{
> +    printk(KERN_ERR "DT: NUMA information is not used.\n");
> +    device_tree_numa = -1;
> +}
> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> index 559b028a01..756ad82d07 100644
> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -23,6 +23,8 @@ typedef u8 nodeid_t;
>   #define NUMA_LOCAL_DISTANCE     10
>   #define NUMA_REMOTE_DISTANCE    20
>   
> +extern s8 device_tree_numa;
> +
>   extern void numa_init(bool acpi_off);
>   extern void numa_set_distance(nodeid_t from, nodeid_t to, uint32_t distance);
>   
Cheers,

--
Julien Grall