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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan