RE: [PATCH v5 5/6] xen/x86: move NUMA scan nodes codes from x86 to common

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 2022年9月30日 14:03
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: nd <nd@xxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Roger Pau
> Monné <roger.pau@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; George Dunlap
> <george.dunlap@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano
> Stabellini <sstabellini@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 5/6] xen/x86: move NUMA scan nodes codes from x86
> to common
> 
> On 30.09.2022 03:40, Wei Chen wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: 2022年9月29日 20:21
> >>
> >> On 29.09.2022 10:21, Wei Chen wrote:
> >>> On 2022/9/27 23:48, Jan Beulich wrote:
> >>>> On 20.09.2022 11:12, Wei Chen wrote:
> >>>>> --- a/xen/drivers/acpi/Kconfig
> >>>>> +++ b/xen/drivers/acpi/Kconfig
> >>>>> @@ -7,4 +7,5 @@ config ACPI_LEGACY_TABLES_LOOKUP
> >>>>>
> >>>>>   config ACPI_NUMA
> >>>>>         bool
> >>>>> +       select HAS_NUMA_NODE_FWID
> >>>>>         select NUMA
> >>>>
> >>>> While I might guess that you've chosen the insertion point to have
> >>>> things sorted alphabetically, I think here it would be more natural
> >>>> to select the wider option first and then also select the more
> >>>> narrow one.
> >>>>
> >>>
> >>> Ok, I will adjust the order.
> >>>
> >>>> One further question though: How is this going to work for Arm64
> >>>> once it wants to support both the form of NUMA you're working to
> >>>> enable _and_ ACPI-based NUMA? There better wouldn't be a requirement
> >>>> to pick one of the two at build time - it would be nice for support
> >>>> of both forms to be able to co-exist in a single binary.
> >>>
> >>> We are also working in this way. In part#3, we will check ACPI first,
> >>> only when ACPI is off, the DT NUMA will be used by Arm. If ACPI is on,
> >>> we will skip DT NUMA.
> >>
> >> Even more so an answer to my question would be nice: You'll then have
> >> CONFIG_HAS_NUMA_NODE_FWID=y even on Arm (using PXM as mandated by ACPI
> >> when in ACPI mode). But then what's the FWID for DT? I know it was me
> >> to suggest this build time distinction, but I'm afraid I wasn't doing
> >> much good with that (and I'm sorry).
> >
> > How about introducing a flag for selected NUMA implementation to
> > set it in runtime?
> > For example:
> > bool numa_has_fw_nodeid;
> >
> > ACPI NUMA will set this flag to 1, but 0 for DT NUMA.
> 
> That's an option alongside going back to what you had in an earlier
> version. Another would be (name subject to improvement)
> 
> const char *__ro_after_init numa_fw_nid_name;
> 
> which for ACPI would be set to "PXM" (eliminating the need to pass
> it to certain functions, albeit the fw_nid will continue to need to
> be passed anyway). I guess I'm not really certain which of this and
> your earlier approach I prefer; the boolean you suggest above looks
> less desirable to me, though.
> 

Ok, I will follow your suggestion.

Cheers,
Wei Chen

> Jan