Hi Julien,
> -----Original Message-----
> From: Julien Grall <jul...@xen.org>
> Sent: 2021年8月26日 0:58
> To: Wei Chen <wei.c...@arm.com>; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org; jbeul...@suse.com
> Cc: Bertrand Marquis <bertrand.marq...@arm.com>
> Subject: Re: [XEN RFC PATCH 26/40] xen/arm: Add boot and secondary CPU to
> NUMA system
>
> Hi Wei,
>
> On 11/08/2021 11:24, Wei Chen wrote:
> > When cpu boot up, we have add them to NUMA system. In current
> > stage, we have not parsed the NUMA data, but we have created
> > a fake NUMA node. So, in this patch, all CPU will be added
> > to NUMA node#0. After the NUMA data has been parsed from device
> > tree, the CPU will be added to correct NUMA node as the NUMA
> > data described.
> >
> > Signed-off-by: Wei Chen <wei.c...@arm.com>
> > ---
> > xen/arch/arm/setup.c | 6 ++++++
> > xen/arch/arm/smpboot.c | 6 ++++++
> > xen/include/asm-arm/numa.h | 1 +
> > 3 files changed, 13 insertions(+)
> >
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 3c58d2d441..7531989f21 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -918,6 +918,12 @@ void __init start_xen(unsigned long
> boot_phys_offset,
> >
> > processor_id();
> >
> > + /*
> > + * If Xen is running on a NUMA off system, there will
> > + * be a node#0 at least.
> > + */
> > + numa_add_cpu(0);
> > +
> > smp_init_cpus();
> > cpus = smp_get_max_cpus();
> > printk(XENLOG_INFO "SMP: Allowing %u CPUs\n", cpus);
> > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> > index a1ee3146ef..aa78958c07 100644
> > --- a/xen/arch/arm/smpboot.c
> > +++ b/xen/arch/arm/smpboot.c
> > @@ -358,6 +358,12 @@ void start_secondary(void)
> > */
> > smp_wmb();
> >
> > + /*
> > + * If Xen is running on a NUMA off system, there will
> > + * be a node#0 at least.
> > + */
> > + numa_add_cpu(cpuid);
> > +
>
> On x86, numa_add_cpu() will be called before the pCPU is brought up. I
> am not quite too sure why we are doing it differently here. Can you
> clarify it?
Of course we can invoke numa_add_cpu before cpu_up as x86. But in my tests,
I found when cpu bring up failed, this cpu still be add to NUMA. Although
this does not affect the execution of the code (because CPU is offline),
But I don't think adding a offline CPU to NUMA makes sense.
>
> > /* Now report this CPU is up */
> > cpumask_set_cpu(cpuid, &cpu_online_map);
> >
> > diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> > index 7a3588ac7f..dd31324b0b 100644
> > --- a/xen/include/asm-arm/numa.h
> > +++ b/xen/include/asm-arm/numa.h
> > @@ -59,6 +59,7 @@ extern mfn_t first_valid_mfn;
> > #define __node_distance(a, b) (20)
> >
> > #define numa_init(x) do { } while (0)
> > +#define numa_add_cpu(x) do { } while (0)
>
> This is a stubs for a common helper. So I think this wants to be moved
> in the !CONFIG_NUMA in xen/numa.h.
>
OK
> > #define numa_set_node(x, y) do { } while (0)
> >
> > #endif
> >
>
> Cheers,
>
> --
> Julien Grall
