Re: [PATCH 26/37] xen/arm: build NUMA cpu_to_node map in dt_smp_init_cpus

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 23 Sep 2021, Wei Chen wrote:
> NUMA implementation has a cpu_to_node array to store CPU to NODE
> map. Xen is using CPU logical ID in runtime components, so we
> use CPU logical ID as CPU index in cpu_to_node.
> 
> In device tree case, cpu_logical_map is created in dt_smp_init_cpus.
> So, when NUMA is enabled, dt_smp_init_cpus will fetch CPU NUMA id
> at the same time for cpu_to_node.
> 
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> ---
>  xen/arch/arm/smpboot.c     | 37 ++++++++++++++++++++++++++++++++++++-
>  xen/include/asm-arm/numa.h |  5 +++++
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 60c0e82fc5..6e3cc8d3cc 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -121,7 +121,12 @@ static void __init dt_smp_init_cpus(void)
>      {
>          [0 ... NR_CPUS - 1] = MPIDR_INVALID
>      };
> +    static nodeid_t node_map[NR_CPUS] __initdata =
> +    {
> +        [0 ... NR_CPUS - 1] = NUMA_NO_NODE
> +    };
>      bool bootcpu_valid = false;
> +    uint32_t nid = 0;
>      int rc;
>  
>      mpidr = system_cpuinfo.mpidr.bits & MPIDR_HWID_MASK;
> @@ -172,6 +177,28 @@ static void __init dt_smp_init_cpus(void)
>              continue;
>          }
>  
> +        if ( IS_ENABLED(CONFIG_NUMA) )
> +        {
> +            /*
> +             * When CONFIG_NUMA is set, try to fetch numa infomation
> +             * from CPU dts node, otherwise the nid is always 0.
> +             */
> +            if ( !dt_property_read_u32(cpu, "numa-node-id", &nid) )
> +            {
> +                printk(XENLOG_WARNING
> +                       "cpu[%d] dts path: %s: doesn't have numa 
> information!\n",
                               ^ %u


> +                       cpuidx, dt_node_full_name(cpu));

I think that this message shouldn't be a warning: CONFIG_NUMA is a
compile time option. Anybody that enables CONFIG_NUMA in the Xen build
will get this warning printed out at boot time if Xen is booting on a
regular non-NUMA machine, right?

The warning should only be printed if NUMA is actively enabled, e.g.
there is a distance-map but the cpus don't have numa-node-id.



> +                /*
> +                 * During the early stage of NUMA initialization, when Xen
> +                 * found any CPU dts node doesn't have numa-node-id info, the
> +                 * NUMA will be treated as off, all CPU will be set to a FAKE
> +                 * node 0. So if we get numa-node-id failed here, we should
> +                 * set nid to 0.
> +                 */
> +                nid = 0;
> +            }
> +        }
> +
>          /*
>           * 8 MSBs must be set to 0 in the DT since the reg property
>           * defines the MPIDR[23:0]
> @@ -231,9 +258,12 @@ static void __init dt_smp_init_cpus(void)
>          {
>              printk("cpu%d init failed (hwid %"PRIregister"): %d\n", i, hwid, 
> rc);
>              tmp_map[i] = MPIDR_INVALID;
> +            node_map[i] = NUMA_NO_NODE;
>          }
> -        else
> +        else {
>              tmp_map[i] = hwid;
> +            node_map[i] = nid;
> +        }
>      }
>  
>      if ( !bootcpu_valid )
> @@ -249,6 +279,11 @@ static void __init dt_smp_init_cpus(void)
>              continue;
>          cpumask_set_cpu(i, &cpu_possible_map);
>          cpu_logical_map(i) = tmp_map[i];
> +
> +        nid = node_map[i];
> +        if ( nid >= MAX_NUMNODES )
> +            nid = 0;
> +        numa_set_node(i, nid);
>      }
>  }
>  
> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> index 758eafeb05..8a4ad379e0 100644
> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -46,6 +46,11 @@ extern mfn_t first_valid_mfn;
>  #define node_start_pfn(nid) (mfn_x(first_valid_mfn))
>  #define __node_distance(a, b) (20)
>  
> +static inline void numa_set_node(int cpu, nodeid_t node)
> +{
> +
> +}
> +
>  #endif
>  
>  static inline unsigned int arch_have_default_dmazone(void)
> -- 
> 2.25.1
>