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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 28 Sep 2021, Wei Chen wrote:
> > -----Original Message-----
> > From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Sent: 2021年9月28日 1:17
> > To: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Julien Grall <julien.grall.oss@xxxxxxxxx>; Stefano Stabellini
> > <sstabellini@xxxxxxxxxx>; Wei Chen <Wei.Chen@xxxxxxx>; xen-devel <xen-
> > devel@xxxxxxxxxxxxxxxxxxxx>; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> > 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@xxxxxxxx> 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 :)