Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake NUMA API

[Julien Grall <julien@xxxxxxx>]

On 20/08/2021 13:23, Wei Chen wrote:
> Hi Julien,
Hi Wei,

> > -----Original Message-----
> > From: Julien Grall <julien@xxxxxxx>
> > Sent: 2021年8月20日 19:24
> > To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> > sstabellini@xxxxxxxxxx
> > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> > Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep fake
> > NUMA API
> >
> >
> >
> > On 20/08/2021 11:24, Wei Chen wrote:
> > > Hi Julien,
> > Hi Wei,
> >
> > > > -----Original Message-----
> > > > From: Julien Grall <julien@xxxxxxx>
> > > > Sent: 2021年8月20日 16:24
> > > > To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> > > > sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> > > > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> > > > Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep
> > fake
> > > > NUMA API
> > > >
> > > >
> > > >
> > > > On 20/08/2021 03:08, Wei Chen wrote:
> > > > > Hi Julien,
> > > > Hi Wei,
> > > >
> > > > > > -----Original Message-----
> > > > > > From: Julien Grall <julien@xxxxxxx>
> > > > > > Sent: 2021年8月19日 21:34
> > > > > > To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> > > > > > sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> > > > > > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> > > > > > Subject: Re: [XEN RFC PATCH 07/40] xen/arm: use !CONFIG_NUMA to keep
> > > > fake
> > > > > > NUMA API
> > > > > >
> > > > > > Hi Wei,
> > > > > >
> > > > > > On 11/08/2021 11:23, Wei Chen wrote:
> > > > > > > Only Arm64 supports NUMA, the CONFIG_NUMA could not be
> > > > > > > enabled for Arm32.
> > > > > > What do you mean by "could not be enabled"?
> > > > > I have not seen any Arm32 hardware support NUMA, so I think
> > > > > we don't need to support Arm32 NUMA.
> > > > I understand that there may not be 32-bit platform with NUMA. And
> > that's
> > > > fine stating that in the commit message. However...
> > > >
> > > > > In this case, this Kconfig
> > > > > option could not be enabled on Arm32.
> > > > ... you continue to say "couldn't be enabled" without clarifying
> > whether
> > > > this mean that the build will break or this was just not tested because
> > > > you don't have any platform.
> > > Ok, I understand your concern. Yes, my words would lead to mis-
> > understanding.
> > > If we make CONFIG_NUMA enabled in Arm32, it need Arm32 to implement some
> > > code to support NUMA common code. Otherwise the Arm32 build will failed.
> > When I skimmed through the series, most of the code seems to be either
> > in common, arm (bitness neutral). So I am not quite too sure why it
> > would not build. Do you have more details?
> It could not build because I have not tried to enable device_tree_numa
> option for Arm32 but enabled NUMA for arm32.
>
> I have tested it again, yes, simple enable device_tree_numa and NUMA
> for arm32 can build a image successfully.
>
> So, I think it's OK to enable this on Arm32, and I will do it in next
> version. But, can we still keep these FAKE APIs? If user don't want to
> enable NUMA they still can make Xen work? 
Yes, we still need to keep the FAKE APIs. I was only commenting about 
the wording in the commit message.
> And I will remove "could not
> enable for Arm32" from commit log.
>
> > > I have not tried to implement those code for Arm32. And I found there is
> > > no Arm32 machine support NUMA, so I wanted Arm32 to use fake NUMA API
> > > as before.
> > >
> > > > To put it differently, the code for NUMA looks bitness neutral. So I
> > > > cannot really what what prevent us to potentially use it on Arm 32-bit.
> > > Yes, you're right, it's neutral. But do we really need to add code to an
> > > ARCH that it may never use?
> > Technically you already added the code because arch/arm/ is common
> > between arm32 and arm64. My only ask is to not make the new config
> > depends on arm64. If you only build test it that fine because...
> >
> > And how can we test this code?
> >
> > I don't expect any of the code to be an issue on arm32 as the code
> > should mostly be arch neutral.
> I mean, we don't have Arm32 NUMA machine to test, I don't know
> the code works well on Arm32 NUMA or not. I only can verify them
> on non-NUMA arm32, and make sure this code will not break existed
> machines.
I understood you don't have any Arm32 NUMA machine. But I don't see the 
lack of testing as an issue because the code doesn't look to contain 
bits that may rely on arm64. So there are very limited reasons for the 
code to break on arm32.
If we really want to test it, then it should be feasible to fake the 
NUMA node in the DT. However, I don't expect you to do it.
Cheers,

--
Julien Grall