diff mbox series

[36/37] xen/arm: Provide Kconfig options for Arm to enable NUMA

Message ID 20210923120236.3692135-37-wei.chen@arm.com (mailing list archive)
State New, archived
Headers show
Series Add device tree based NUMA support to Arm | expand

Commit Message

Wei Chen Sept. 23, 2021, 12:02 p.m. UTC
Arm platforms support both ACPI and device tree. We don't
want users to select device tree NUMA or ACPI NUMA manually.
We hope usrs can just enable NUMA for Arm, and device tree
NUMA and ACPI NUMA can be selected depends on device tree
feature and ACPI feature status automatically. In this case,
these two kinds of NUMA support code can be co-exist in one
Xen binary. Xen can check feature flags to decide using
device tree or ACPI as NUMA based firmware.

So in this patch, we introduce a generic option:
CONFIG_ARM_NUMA for user to enable NUMA for Arm.
And one CONFIG_DEVICE_TREE_NUMA option for ARM_NUMA
to select when HAS_DEVICE_TREE option is enabled.
Once when ACPI NUMA for Arm is supported, ACPI_NUMA
can be selected here too.

Signed-off-by: Wei Chen <wei.chen@arm.com>
---
 xen/arch/arm/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Stefano Stabellini Sept. 24, 2021, 3:31 a.m. UTC | #1
On Thu, 23 Sep 2021, Wei Chen wrote:
> Arm platforms support both ACPI and device tree. We don't
> want users to select device tree NUMA or ACPI NUMA manually.
> We hope usrs can just enable NUMA for Arm, and device tree
          ^ users

> NUMA and ACPI NUMA can be selected depends on device tree
> feature and ACPI feature status automatically. In this case,
> these two kinds of NUMA support code can be co-exist in one
> Xen binary. Xen can check feature flags to decide using
> device tree or ACPI as NUMA based firmware.
> 
> So in this patch, we introduce a generic option:
> CONFIG_ARM_NUMA for user to enable NUMA for Arm.
                      ^ users

> And one CONFIG_DEVICE_TREE_NUMA option for ARM_NUMA
> to select when HAS_DEVICE_TREE option is enabled.
> Once when ACPI NUMA for Arm is supported, ACPI_NUMA
> can be selected here too.
> 
> Signed-off-by: Wei Chen <wei.chen@arm.com>
> ---
>  xen/arch/arm/Kconfig | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 865ad83a89..ded94ebd37 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -34,6 +34,17 @@ config ACPI
>  	  Advanced Configuration and Power Interface (ACPI) support for Xen is
>  	  an alternative to device tree on ARM64.
>  
> + config DEVICE_TREE_NUMA
> +	def_bool n
> +	select NUMA
> +
> +config ARM_NUMA
> +	bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)" if UNSUPPORTED
> +	select DEVICE_TREE_NUMA if HAS_DEVICE_TREE

Should it be: depends on HAS_DEVICE_TREE ?
(And eventually depends on HAS_DEVICE_TREE || ACPI)


> +	---help---
> +
> +	  Enable Non-Uniform Memory Access (NUMA) for Arm architecutres
                                                      ^ architectures


> +
>  config GICV3
>  	bool "GICv3 driver"
>  	depends on ARM_64 && !NEW_VGIC
> -- 
> 2.25.1
>
Wei Chen Sept. 24, 2021, 10:13 a.m. UTC | #2
Hi Stefano,

On 2021/9/24 11:31, Stefano Stabellini wrote:
> On Thu, 23 Sep 2021, Wei Chen wrote:
>> Arm platforms support both ACPI and device tree. We don't
>> want users to select device tree NUMA or ACPI NUMA manually.
>> We hope usrs can just enable NUMA for Arm, and device tree
>            ^ users
> 
>> NUMA and ACPI NUMA can be selected depends on device tree
>> feature and ACPI feature status automatically. In this case,
>> these two kinds of NUMA support code can be co-exist in one
>> Xen binary. Xen can check feature flags to decide using
>> device tree or ACPI as NUMA based firmware.
>>
>> So in this patch, we introduce a generic option:
>> CONFIG_ARM_NUMA for user to enable NUMA for Arm.
>                        ^ users
>

OK

>> And one CONFIG_DEVICE_TREE_NUMA option for ARM_NUMA
>> to select when HAS_DEVICE_TREE option is enabled.
>> Once when ACPI NUMA for Arm is supported, ACPI_NUMA
>> can be selected here too.
>>
>> Signed-off-by: Wei Chen <wei.chen@arm.com>
>> ---
>>   xen/arch/arm/Kconfig | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index 865ad83a89..ded94ebd37 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -34,6 +34,17 @@ config ACPI
>>   	  Advanced Configuration and Power Interface (ACPI) support for Xen is
>>   	  an alternative to device tree on ARM64.
>>   
>> + config DEVICE_TREE_NUMA
>> +	def_bool n
>> +	select NUMA
>> +
>> +config ARM_NUMA
>> +	bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)" if UNSUPPORTED
>> +	select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
> 
> Should it be: depends on HAS_DEVICE_TREE ?
> (And eventually depends on HAS_DEVICE_TREE || ACPI)
> 

As the discussion in RFC [1]. We want to make ARM_NUMA as a generic
option can be selected by users. And depends on has_device_tree
or ACPI to select DEVICE_TREE_NUMA or ACPI_NUMA.

If we add HAS_DEVICE_TREE || ACPI as dependencies for ARM_NUMA,
does it become a loop dependency?

https://lists.xenproject.org/archives/html/xen-devel/2021-08/msg00888.html
> 
>> +	---help---
>> +
>> +	  Enable Non-Uniform Memory Access (NUMA) for Arm architecutres
>                                                        ^ architectures
> 
> 
>> +
>>   config GICV3
>>   	bool "GICv3 driver"
>>   	depends on ARM_64 && !NEW_VGIC
>> -- 
>> 2.25.1
>>
Jan Beulich Sept. 24, 2021, 10:25 a.m. UTC | #3
On 23.09.2021 14:02, Wei Chen wrote:
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -34,6 +34,17 @@ config ACPI
>  	  Advanced Configuration and Power Interface (ACPI) support for Xen is
>  	  an alternative to device tree on ARM64.
>  
> + config DEVICE_TREE_NUMA
> +	def_bool n
> +	select NUMA

Two nits here: There's a stray blank on the first line, and you
appear to mean just "bool", not "def_bool n" (there's no point
in having defaults for select-only options).

> +config ARM_NUMA
> +	bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)" if UNSUPPORTED
> +	select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
> +	---help---

And another nit here: We try to move away from "---help---", which
is no longer supported by Linux'es newer kconfig. Please use just
"help" in new code.

Jan
Wei Chen Sept. 24, 2021, 10:37 a.m. UTC | #4
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2021年9月24日 18:26
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen-
> devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org
> Subject: Re: [PATCH 36/37] xen/arm: Provide Kconfig options for Arm to
> enable NUMA
> 
> On 23.09.2021 14:02, Wei Chen wrote:
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -34,6 +34,17 @@ config ACPI
> >  	  Advanced Configuration and Power Interface (ACPI) support for Xen
> is
> >  	  an alternative to device tree on ARM64.
> >
> > + config DEVICE_TREE_NUMA
> > +	def_bool n
> > +	select NUMA
> 
> Two nits here: There's a stray blank on the first line, and you
> appear to mean just "bool", not "def_bool n" (there's no point
> in having defaults for select-only options).
> 

Ok

> > +config ARM_NUMA
> > +	bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)" if
> UNSUPPORTED
> > +	select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
> > +	---help---
> 
> And another nit here: We try to move away from "---help---", which
> is no longer supported by Linux'es newer kconfig. Please use just
> "help" in new code.
> 

Thanks, I will do it.

> Jan
Stefano Stabellini Sept. 24, 2021, 7:39 p.m. UTC | #5
On Fri, 24 Sep 2021, Wei Chen wrote:
> Hi Stefano,
> 
> On 2021/9/24 11:31, Stefano Stabellini wrote:
> > On Thu, 23 Sep 2021, Wei Chen wrote:
> > > Arm platforms support both ACPI and device tree. We don't
> > > want users to select device tree NUMA or ACPI NUMA manually.
> > > We hope usrs can just enable NUMA for Arm, and device tree
> >            ^ users
> > 
> > > NUMA and ACPI NUMA can be selected depends on device tree
> > > feature and ACPI feature status automatically. In this case,
> > > these two kinds of NUMA support code can be co-exist in one
> > > Xen binary. Xen can check feature flags to decide using
> > > device tree or ACPI as NUMA based firmware.
> > > 
> > > So in this patch, we introduce a generic option:
> > > CONFIG_ARM_NUMA for user to enable NUMA for Arm.
> >                        ^ users
> > 
> 
> OK
> 
> > > And one CONFIG_DEVICE_TREE_NUMA option for ARM_NUMA
> > > to select when HAS_DEVICE_TREE option is enabled.
> > > Once when ACPI NUMA for Arm is supported, ACPI_NUMA
> > > can be selected here too.
> > > 
> > > Signed-off-by: Wei Chen <wei.chen@arm.com>
> > > ---
> > >   xen/arch/arm/Kconfig | 11 +++++++++++
> > >   1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > index 865ad83a89..ded94ebd37 100644
> > > --- a/xen/arch/arm/Kconfig
> > > +++ b/xen/arch/arm/Kconfig
> > > @@ -34,6 +34,17 @@ config ACPI
> > >   	  Advanced Configuration and Power Interface (ACPI) support for Xen is
> > >   	  an alternative to device tree on ARM64.
> > >   + config DEVICE_TREE_NUMA
> > > +	def_bool n
> > > +	select NUMA
> > > +
> > > +config ARM_NUMA
> > > +	bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)" if
> > > UNSUPPORTED
> > > +	select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
> > 
> > Should it be: depends on HAS_DEVICE_TREE ?
> > (And eventually depends on HAS_DEVICE_TREE || ACPI)
> > 
> 
> As the discussion in RFC [1]. We want to make ARM_NUMA as a generic
> option can be selected by users. And depends on has_device_tree
> or ACPI to select DEVICE_TREE_NUMA or ACPI_NUMA.
> 
> If we add HAS_DEVICE_TREE || ACPI as dependencies for ARM_NUMA,
> does it become a loop dependency?
> 
> https://lists.xenproject.org/archives/html/xen-devel/2021-08/msg00888.html

OK, I am fine with that. I was just trying to catch the case where a
user selects "ARM_NUMA" but actually neither ACPI nor HAS_DEVICE_TREE
are selected so nothing happens. I was trying to make it clear that
ARM_NUMA depends on having at least one between HAS_DEVICE_TREE or ACPI
because otherwise it is not going to work.

That said, I don't think this is important because HAS_DEVICE_TREE
cannot be unselected. So if we cannot find a way to express the
dependency, I think it is fine to keep the patch as is.
Jan Beulich Sept. 27, 2021, 8:33 a.m. UTC | #6
On 24.09.2021 21:39, Stefano Stabellini wrote:
> On Fri, 24 Sep 2021, Wei Chen wrote:
>> On 2021/9/24 11:31, Stefano Stabellini wrote:
>>> On Thu, 23 Sep 2021, Wei Chen wrote:
>>>> --- a/xen/arch/arm/Kconfig
>>>> +++ b/xen/arch/arm/Kconfig
>>>> @@ -34,6 +34,17 @@ config ACPI
>>>>   	  Advanced Configuration and Power Interface (ACPI) support for Xen is
>>>>   	  an alternative to device tree on ARM64.
>>>>   + config DEVICE_TREE_NUMA
>>>> +	def_bool n
>>>> +	select NUMA
>>>> +
>>>> +config ARM_NUMA
>>>> +	bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)" if
>>>> UNSUPPORTED
>>>> +	select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
>>>
>>> Should it be: depends on HAS_DEVICE_TREE ?
>>> (And eventually depends on HAS_DEVICE_TREE || ACPI)
>>>
>>
>> As the discussion in RFC [1]. We want to make ARM_NUMA as a generic
>> option can be selected by users. And depends on has_device_tree
>> or ACPI to select DEVICE_TREE_NUMA or ACPI_NUMA.
>>
>> If we add HAS_DEVICE_TREE || ACPI as dependencies for ARM_NUMA,
>> does it become a loop dependency?
>>
>> https://lists.xenproject.org/archives/html/xen-devel/2021-08/msg00888.html
> 
> OK, I am fine with that. I was just trying to catch the case where a
> user selects "ARM_NUMA" but actually neither ACPI nor HAS_DEVICE_TREE
> are selected so nothing happens. I was trying to make it clear that
> ARM_NUMA depends on having at least one between HAS_DEVICE_TREE or ACPI
> because otherwise it is not going to work.
> 
> That said, I don't think this is important because HAS_DEVICE_TREE
> cannot be unselected. So if we cannot find a way to express the
> dependency, I think it is fine to keep the patch as is.

So how about doing things the other way around: ARM_NUMA has no prompt
and defaults to ACPI_NUMA || DT_NUMA, and DT_NUMA gains a prompt instead
(and, for Arm at least, ACPI_NUMA as well; this might even be worthwhile
to have on x86 down the road).

Jan
Julien Grall Sept. 27, 2021, 8:45 a.m. UTC | #7
On Mon, 27 Sep 2021, 10:33 Jan Beulich, <jbeulich@suse.com> wrote:

> On 24.09.2021 21:39, Stefano Stabellini wrote:
> > On Fri, 24 Sep 2021, Wei Chen wrote:
> >> On 2021/9/24 11:31, Stefano Stabellini wrote:
> >>> On Thu, 23 Sep 2021, Wei Chen wrote:
> >>>> --- a/xen/arch/arm/Kconfig
> >>>> +++ b/xen/arch/arm/Kconfig
> >>>> @@ -34,6 +34,17 @@ config ACPI
> >>>>      Advanced Configuration and Power Interface (ACPI) support for
> Xen is
> >>>>      an alternative to device tree on ARM64.
> >>>>   + config DEVICE_TREE_NUMA
> >>>> +  def_bool n
> >>>> +  select NUMA
> >>>> +
> >>>> +config ARM_NUMA
> >>>> +  bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)"
> if
> >>>> UNSUPPORTED
> >>>> +  select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
> >>>
> >>> Should it be: depends on HAS_DEVICE_TREE ?
> >>> (And eventually depends on HAS_DEVICE_TREE || ACPI)
> >>>
> >>
> >> As the discussion in RFC [1]. We want to make ARM_NUMA as a generic
> >> option can be selected by users. And depends on has_device_tree
> >> or ACPI to select DEVICE_TREE_NUMA or ACPI_NUMA.
> >>
> >> If we add HAS_DEVICE_TREE || ACPI as dependencies for ARM_NUMA,
> >> does it become a loop dependency?
> >>
> >>
> https://lists.xenproject.org/archives/html/xen-devel/2021-08/msg00888.html
> >
> > OK, I am fine with that. I was just trying to catch the case where a
> > user selects "ARM_NUMA" but actually neither ACPI nor HAS_DEVICE_TREE
> > are selected so nothing happens. I was trying to make it clear that
> > ARM_NUMA depends on having at least one between HAS_DEVICE_TREE or ACPI
> > because otherwise it is not going to work.
> >
> > That said, I don't think this is important because HAS_DEVICE_TREE
> > cannot be unselected. So if we cannot find a way to express the
> > dependency, I think it is fine to keep the patch as is.
>
> So how about doing things the other way around: ARM_NUMA has no prompt
> and defaults to ACPI_NUMA || DT_NUMA, and DT_NUMA gains a prompt instead
> (and, for Arm at least, ACPI_NUMA as well; this might even be worthwhile
> to have on x86 down the road).
>

As I wrote before, I don't think the user should say "I want to enable NUMA
with Device-Tree or ACPI". Instead, they say whether they want to use NUMA
and let Xen decide to enable the DT/ACPI support.

In other word, the prompt should stay on ARM_NUMA.

Cheers,


> Jan
>
>
Jan Beulich Sept. 27, 2021, 9:17 a.m. UTC | #8
On 27.09.2021 10:45, Julien Grall wrote:
> On Mon, 27 Sep 2021, 10:33 Jan Beulich, <jbeulich@suse.com> wrote:
> 
>> On 24.09.2021 21:39, Stefano Stabellini wrote:
>>> On Fri, 24 Sep 2021, Wei Chen wrote:
>>>> On 2021/9/24 11:31, Stefano Stabellini wrote:
>>>>> On Thu, 23 Sep 2021, Wei Chen wrote:
>>>>>> --- a/xen/arch/arm/Kconfig
>>>>>> +++ b/xen/arch/arm/Kconfig
>>>>>> @@ -34,6 +34,17 @@ config ACPI
>>>>>>      Advanced Configuration and Power Interface (ACPI) support for
>> Xen is
>>>>>>      an alternative to device tree on ARM64.
>>>>>>   + config DEVICE_TREE_NUMA
>>>>>> +  def_bool n
>>>>>> +  select NUMA
>>>>>> +
>>>>>> +config ARM_NUMA
>>>>>> +  bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)"
>> if
>>>>>> UNSUPPORTED
>>>>>> +  select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
>>>>>
>>>>> Should it be: depends on HAS_DEVICE_TREE ?
>>>>> (And eventually depends on HAS_DEVICE_TREE || ACPI)
>>>>>
>>>>
>>>> As the discussion in RFC [1]. We want to make ARM_NUMA as a generic
>>>> option can be selected by users. And depends on has_device_tree
>>>> or ACPI to select DEVICE_TREE_NUMA or ACPI_NUMA.
>>>>
>>>> If we add HAS_DEVICE_TREE || ACPI as dependencies for ARM_NUMA,
>>>> does it become a loop dependency?
>>>>
>>>>
>> https://lists.xenproject.org/archives/html/xen-devel/2021-08/msg00888.html
>>>
>>> OK, I am fine with that. I was just trying to catch the case where a
>>> user selects "ARM_NUMA" but actually neither ACPI nor HAS_DEVICE_TREE
>>> are selected so nothing happens. I was trying to make it clear that
>>> ARM_NUMA depends on having at least one between HAS_DEVICE_TREE or ACPI
>>> because otherwise it is not going to work.
>>>
>>> That said, I don't think this is important because HAS_DEVICE_TREE
>>> cannot be unselected. So if we cannot find a way to express the
>>> dependency, I think it is fine to keep the patch as is.
>>
>> So how about doing things the other way around: ARM_NUMA has no prompt
>> and defaults to ACPI_NUMA || DT_NUMA, and DT_NUMA gains a prompt instead
>> (and, for Arm at least, ACPI_NUMA as well; this might even be worthwhile
>> to have on x86 down the road).
>>
> 
> As I wrote before, I don't think the user should say "I want to enable NUMA
> with Device-Tree or ACPI". Instead, they say whether they want to use NUMA
> and let Xen decide to enable the DT/ACPI support.
> 
> In other word, the prompt should stay on ARM_NUMA.

Okay. In which case I'm confused by Stefano's question.

Jan
Stefano Stabellini Sept. 27, 2021, 5:17 p.m. UTC | #9
On Mon, 27 Sep 2021, Jan Beulich wrote:
> On 27.09.2021 10:45, Julien Grall wrote:
> > On Mon, 27 Sep 2021, 10:33 Jan Beulich, <jbeulich@suse.com> wrote:
> > 
> >> On 24.09.2021 21:39, Stefano Stabellini wrote:
> >>> On Fri, 24 Sep 2021, Wei Chen wrote:
> >>>> On 2021/9/24 11:31, Stefano Stabellini wrote:
> >>>>> On Thu, 23 Sep 2021, Wei Chen wrote:
> >>>>>> --- a/xen/arch/arm/Kconfig
> >>>>>> +++ b/xen/arch/arm/Kconfig
> >>>>>> @@ -34,6 +34,17 @@ config ACPI
> >>>>>>      Advanced Configuration and Power Interface (ACPI) support for
> >> Xen is
> >>>>>>      an alternative to device tree on ARM64.
> >>>>>>   + config DEVICE_TREE_NUMA
> >>>>>> +  def_bool n
> >>>>>> +  select NUMA
> >>>>>> +
> >>>>>> +config ARM_NUMA
> >>>>>> +  bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)"
> >> if
> >>>>>> UNSUPPORTED
> >>>>>> +  select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
> >>>>>
> >>>>> Should it be: depends on HAS_DEVICE_TREE ?
> >>>>> (And eventually depends on HAS_DEVICE_TREE || ACPI)
> >>>>>
> >>>>
> >>>> As the discussion in RFC [1]. We want to make ARM_NUMA as a generic
> >>>> option can be selected by users. And depends on has_device_tree
> >>>> or ACPI to select DEVICE_TREE_NUMA or ACPI_NUMA.
> >>>>
> >>>> If we add HAS_DEVICE_TREE || ACPI as dependencies for ARM_NUMA,
> >>>> does it become a loop dependency?
> >>>>
> >>>>
> >> https://lists.xenproject.org/archives/html/xen-devel/2021-08/msg00888.html
> >>>
> >>> OK, I am fine with that. I was just trying to catch the case where a
> >>> user selects "ARM_NUMA" but actually neither ACPI nor HAS_DEVICE_TREE
> >>> are selected so nothing happens. I was trying to make it clear that
> >>> ARM_NUMA depends on having at least one between HAS_DEVICE_TREE or ACPI
> >>> because otherwise it is not going to work.
> >>>
> >>> That said, I don't think this is important because HAS_DEVICE_TREE
> >>> cannot be unselected. So if we cannot find a way to express the
> >>> dependency, I think it is fine to keep the patch as is.
> >>
> >> So how about doing things the other way around: ARM_NUMA has no prompt
> >> and defaults to ACPI_NUMA || DT_NUMA, and DT_NUMA gains a prompt instead
> >> (and, for Arm at least, ACPI_NUMA as well; this might even be worthwhile
> >> to have on x86 down the road).
> >>
> > 
> > As I wrote before, I don't think the user should say "I want to enable NUMA
> > with Device-Tree or ACPI". Instead, they say whether they want to use NUMA
> > and let Xen decide to enable the DT/ACPI support.
> > 
> > In other word, the prompt should stay on ARM_NUMA.
> 
> Okay. In which case I'm confused by Stefano's question.

Let me clarify: I think it is fine to have a single prompt for NUMA in
Kconfig. However, I am just pointing out that it is theoretically
possible with the current code to present an ARM_NUMA prompt to the user
but actually have no NUMA enabled at the end because both DEVICE TREE
and ACPI are disabled. This is only a theoretical problem because DEVICE
TREE support (HAS_DEVICE_TREE) cannot be disabled today. Also I cannot
imagine how a configuration with neither DEVICE TREE nor ACPI can be
correct. So I don't think it is a critical concern.

That said, you can see that, at least theoretically, ARM_NUMA depends on
either HAS_DEVICE_TREE or ACPI, so I suggested to add:

depends on HAS_DEVICE_TREE || ACPI

Wei answered that it might introduce a circular dependency, but I did
try the addition of "depends on HAS_DEVICE_TREE || ACPI" under ARM_NUMA
in Kconfig and everything built fine here.
Wei Chen Sept. 28, 2021, 2:59 a.m. UTC | #10
Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: 2021年9月28日 1:17
> To: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien.grall.oss@gmail.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Chen <Wei.Chen@arm.com>; xen-devel <xen-
> devel@lists.xenproject.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>
> Subject: Re: [PATCH 36/37] xen/arm: Provide Kconfig options for Arm to
> enable NUMA
> 
> On Mon, 27 Sep 2021, Jan Beulich wrote:
> > On 27.09.2021 10:45, Julien Grall wrote:
> > > On Mon, 27 Sep 2021, 10:33 Jan Beulich, <jbeulich@suse.com> wrote:
> > >
> > >> On 24.09.2021 21:39, Stefano Stabellini wrote:
> > >>> On Fri, 24 Sep 2021, Wei Chen wrote:
> > >>>> On 2021/9/24 11:31, Stefano Stabellini wrote:
> > >>>>> On Thu, 23 Sep 2021, Wei Chen wrote:
> > >>>>>> --- a/xen/arch/arm/Kconfig
> > >>>>>> +++ b/xen/arch/arm/Kconfig
> > >>>>>> @@ -34,6 +34,17 @@ config ACPI
> > >>>>>>      Advanced Configuration and Power Interface (ACPI) support
> for
> > >> Xen is
> > >>>>>>      an alternative to device tree on ARM64.
> > >>>>>>   + config DEVICE_TREE_NUMA
> > >>>>>> +  def_bool n
> > >>>>>> +  select NUMA
> > >>>>>> +
> > >>>>>> +config ARM_NUMA
> > >>>>>> +  bool "Arm NUMA (Non-Uniform Memory Access) Support
> (UNSUPPORTED)"
> > >> if
> > >>>>>> UNSUPPORTED
> > >>>>>> +  select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
> > >>>>>
> > >>>>> Should it be: depends on HAS_DEVICE_TREE ?
> > >>>>> (And eventually depends on HAS_DEVICE_TREE || ACPI)
> > >>>>>
> > >>>>
> > >>>> As the discussion in RFC [1]. We want to make ARM_NUMA as a generic
> > >>>> option can be selected by users. And depends on has_device_tree
> > >>>> or ACPI to select DEVICE_TREE_NUMA or ACPI_NUMA.
> > >>>>
> > >>>> If we add HAS_DEVICE_TREE || ACPI as dependencies for ARM_NUMA,
> > >>>> does it become a loop dependency?
> > >>>>
> > >>>>
> > >> https://lists.xenproject.org/archives/html/xen-devel/2021-
> 08/msg00888.html
> > >>>
> > >>> OK, I am fine with that. I was just trying to catch the case where a
> > >>> user selects "ARM_NUMA" but actually neither ACPI nor
> HAS_DEVICE_TREE
> > >>> are selected so nothing happens. I was trying to make it clear that
> > >>> ARM_NUMA depends on having at least one between HAS_DEVICE_TREE or
> ACPI
> > >>> because otherwise it is not going to work.
> > >>>
> > >>> That said, I don't think this is important because HAS_DEVICE_TREE
> > >>> cannot be unselected. So if we cannot find a way to express the
> > >>> dependency, I think it is fine to keep the patch as is.
> > >>
> > >> So how about doing things the other way around: ARM_NUMA has no
> prompt
> > >> and defaults to ACPI_NUMA || DT_NUMA, and DT_NUMA gains a prompt
> instead
> > >> (and, for Arm at least, ACPI_NUMA as well; this might even be
> worthwhile
> > >> to have on x86 down the road).
> > >>
> > >
> > > As I wrote before, I don't think the user should say "I want to enable
> NUMA
> > > with Device-Tree or ACPI". Instead, they say whether they want to use
> NUMA
> > > and let Xen decide to enable the DT/ACPI support.
> > >
> > > In other word, the prompt should stay on ARM_NUMA.
> >
> > Okay. In which case I'm confused by Stefano's question.
> 
> Let me clarify: I think it is fine to have a single prompt for NUMA in
> Kconfig. However, I am just pointing out that it is theoretically
> possible with the current code to present an ARM_NUMA prompt to the user
> but actually have no NUMA enabled at the end because both DEVICE TREE
> and ACPI are disabled. This is only a theoretical problem because DEVICE
> TREE support (HAS_DEVICE_TREE) cannot be disabled today. Also I cannot
> imagine how a configuration with neither DEVICE TREE nor ACPI can be
> correct. So I don't think it is a critical concern.
> 
> That said, you can see that, at least theoretically, ARM_NUMA depends on
> either HAS_DEVICE_TREE or ACPI, so I suggested to add:
> 
> depends on HAS_DEVICE_TREE || ACPI
> 
> Wei answered that it might introduce a circular dependency, but I did
> try the addition of "depends on HAS_DEVICE_TREE || ACPI" under ARM_NUMA
> in Kconfig and everything built fine here.

Ok, I will add "depends on HAS_DEVICE_TREE" in next version, but "|| ACPI"
will be later when we have ACPI NUMA for Arm : )
Stefano Stabellini Sept. 28, 2021, 3:30 a.m. UTC | #11
On Tue, 28 Sep 2021, Wei Chen wrote:
> > -----Original Message-----
> > From: Stefano Stabellini <sstabellini@kernel.org>
> > Sent: 2021年9月28日 1:17
> > To: Jan Beulich <jbeulich@suse.com>
> > Cc: Julien Grall <julien.grall.oss@gmail.com>; Stefano Stabellini
> > <sstabellini@kernel.org>; Wei Chen <Wei.Chen@arm.com>; xen-devel <xen-
> > devel@lists.xenproject.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>
> > Subject: Re: [PATCH 36/37] xen/arm: Provide Kconfig options for Arm to
> > enable NUMA
> > 
> > On Mon, 27 Sep 2021, Jan Beulich wrote:
> > > On 27.09.2021 10:45, Julien Grall wrote:
> > > > On Mon, 27 Sep 2021, 10:33 Jan Beulich, <jbeulich@suse.com> wrote:
> > > >
> > > >> On 24.09.2021 21:39, Stefano Stabellini wrote:
> > > >>> On Fri, 24 Sep 2021, Wei Chen wrote:
> > > >>>> On 2021/9/24 11:31, Stefano Stabellini wrote:
> > > >>>>> On Thu, 23 Sep 2021, Wei Chen wrote:
> > > >>>>>> --- a/xen/arch/arm/Kconfig
> > > >>>>>> +++ b/xen/arch/arm/Kconfig
> > > >>>>>> @@ -34,6 +34,17 @@ config ACPI
> > > >>>>>>      Advanced Configuration and Power Interface (ACPI) support
> > for
> > > >> Xen is
> > > >>>>>>      an alternative to device tree on ARM64.
> > > >>>>>>   + config DEVICE_TREE_NUMA
> > > >>>>>> +  def_bool n
> > > >>>>>> +  select NUMA
> > > >>>>>> +
> > > >>>>>> +config ARM_NUMA
> > > >>>>>> +  bool "Arm NUMA (Non-Uniform Memory Access) Support
> > (UNSUPPORTED)"
> > > >> if
> > > >>>>>> UNSUPPORTED
> > > >>>>>> +  select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
> > > >>>>>
> > > >>>>> Should it be: depends on HAS_DEVICE_TREE ?
> > > >>>>> (And eventually depends on HAS_DEVICE_TREE || ACPI)
> > > >>>>>
> > > >>>>
> > > >>>> As the discussion in RFC [1]. We want to make ARM_NUMA as a generic
> > > >>>> option can be selected by users. And depends on has_device_tree
> > > >>>> or ACPI to select DEVICE_TREE_NUMA or ACPI_NUMA.
> > > >>>>
> > > >>>> If we add HAS_DEVICE_TREE || ACPI as dependencies for ARM_NUMA,
> > > >>>> does it become a loop dependency?
> > > >>>>
> > > >>>>
> > > >> https://lists.xenproject.org/archives/html/xen-devel/2021-
> > 08/msg00888.html
> > > >>>
> > > >>> OK, I am fine with that. I was just trying to catch the case where a
> > > >>> user selects "ARM_NUMA" but actually neither ACPI nor
> > HAS_DEVICE_TREE
> > > >>> are selected so nothing happens. I was trying to make it clear that
> > > >>> ARM_NUMA depends on having at least one between HAS_DEVICE_TREE or
> > ACPI
> > > >>> because otherwise it is not going to work.
> > > >>>
> > > >>> That said, I don't think this is important because HAS_DEVICE_TREE
> > > >>> cannot be unselected. So if we cannot find a way to express the
> > > >>> dependency, I think it is fine to keep the patch as is.
> > > >>
> > > >> So how about doing things the other way around: ARM_NUMA has no
> > prompt
> > > >> and defaults to ACPI_NUMA || DT_NUMA, and DT_NUMA gains a prompt
> > instead
> > > >> (and, for Arm at least, ACPI_NUMA as well; this might even be
> > worthwhile
> > > >> to have on x86 down the road).
> > > >>
> > > >
> > > > As I wrote before, I don't think the user should say "I want to enable
> > NUMA
> > > > with Device-Tree or ACPI". Instead, they say whether they want to use
> > NUMA
> > > > and let Xen decide to enable the DT/ACPI support.
> > > >
> > > > In other word, the prompt should stay on ARM_NUMA.
> > >
> > > Okay. In which case I'm confused by Stefano's question.
> > 
> > Let me clarify: I think it is fine to have a single prompt for NUMA in
> > Kconfig. However, I am just pointing out that it is theoretically
> > possible with the current code to present an ARM_NUMA prompt to the user
> > but actually have no NUMA enabled at the end because both DEVICE TREE
> > and ACPI are disabled. This is only a theoretical problem because DEVICE
> > TREE support (HAS_DEVICE_TREE) cannot be disabled today. Also I cannot
> > imagine how a configuration with neither DEVICE TREE nor ACPI can be
> > correct. So I don't think it is a critical concern.
> > 
> > That said, you can see that, at least theoretically, ARM_NUMA depends on
> > either HAS_DEVICE_TREE or ACPI, so I suggested to add:
> > 
> > depends on HAS_DEVICE_TREE || ACPI
> > 
> > Wei answered that it might introduce a circular dependency, but I did
> > try the addition of "depends on HAS_DEVICE_TREE || ACPI" under ARM_NUMA
> > in Kconfig and everything built fine here.
> 
> Ok, I will add "depends on HAS_DEVICE_TREE" in next version, but "|| ACPI"
> will be later when we have ACPI NUMA for Arm : )

Good point :)
diff mbox series

Patch

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 865ad83a89..ded94ebd37 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -34,6 +34,17 @@  config ACPI
 	  Advanced Configuration and Power Interface (ACPI) support for Xen is
 	  an alternative to device tree on ARM64.
 
+ config DEVICE_TREE_NUMA
+	def_bool n
+	select NUMA
+
+config ARM_NUMA
+	bool "Arm NUMA (Non-Uniform Memory Access) Support (UNSUPPORTED)" if UNSUPPORTED
+	select DEVICE_TREE_NUMA if HAS_DEVICE_TREE
+	---help---
+
+	  Enable Non-Uniform Memory Access (NUMA) for Arm architecutres
+
 config GICV3
 	bool "GICv3 driver"
 	depends on ARM_64 && !NEW_VGIC