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

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Sent: 2021年9月24日 10:26
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; julien@xxxxxxx;
> Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [PATCH 26/37] xen/arm: build NUMA cpu_to_node map in
> dt_smp_init_cpus
> 
> 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.
> 
> 

Yes, this message would be unexpected on a regular non-NUMA machine.
I will some check condition to print this message.

> 
> > +                /*
> > +                 * 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
> >