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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.07.2022 12:57, Wei Chen wrote:
> Hi Jan,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 2022年7月12日 22:21
>> 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 v2 6/9] xen/x86: move NUMA scan nodes codes from x86
>> to common
>>
>> On 08.07.2022 16:54, Wei Chen wrote:
>>> x86 has implemented a set of codes to scan NUMA nodes. These
>>> codes will parse NUMA memory and processor information from
>>> ACPI SRAT table. But except some ACPI specific codes, most
>>> of the scan codes like memory blocks validation, node memory
>>> range updates and some sanity check can be reused by other
>>> NUMA implementation.
>>>
>>> So in this patch, we move some variables and related functions
>>> for NUMA memory and processor to common code. At the same time,
>>> numa_set_processor_nodes_parsed has been introduced for ACPI
>>> specific code to update processor parsing results. With this
>>> helper, we can move most of NUMA memory affinity init code from
>>> ACPI. And bad_srat and node_to_pxm functions have been exported
>>> for common code to do architectural fallback and node to proximity
>>> converting.
>>
>> I consider it wrong for generic (ACPI-independent) code to use
>> terms like "srat" or "pxm". This wants abstracting in some way,
>> albeit I have to admit I lack a good idea for a suggestion right
>> now.
>>
> 
> Maybe we can use fw_rsc_table or rsc_table to replace srat, because
> srat is one kind of NUMA resource description table of ACPI?

Is "rsc" meant to stand for "resource"? Would be a somewhat unusual
abbreviation. I could see use using e.g. numa_fw_ as a prefix, as in
e.g. numa_fw_bad() (replacing bad_srat()).

> For PXM, I had tried to keep PXM in x86 ACPI implementation. But the
> cost is that, we have to move some common code to architectural code,
> because some messages use pxm for info, and they have different meanings
> for each platform, we cannot simply remove them.

Well, for functions wanting to emit log messages, suitable abstractions
can likely be made without needing the retain a lot of per-arch code.
E.g. the arch could pass in "PXM" and format strings then would use %s
together with it. Similarly the translation (if any is necessary) could
likely be abstracted by, in the worst case, passing in a function
pointer.

Jan