On 23.09.2021 14:02, Wei Chen wrote:
> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -51,11 +51,11 @@ cpumask_t node_to_cpumask[MAX_NUMNODES] __read_mostly;
> nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
>
> bool numa_off;
> -s8 acpi_numa = 0;
> +s8 fw_numa = 0;
In x86 code I'd prefer this to remain "acpi_numa". If you need to access
the variable from to-become-generic code, introduce an inline wrapper
(possibly named numa_mode()), allowing you to do whatever you need in the
DT case. It may be helpful to fold this with numa_off then, seeing e.g.
...
> int srat_disabled(void)
> {
> - return numa_off || acpi_numa < 0;
> + return numa_off || fw_numa < 0;
... this. Actually I think the underlying enumeration could even be made
generic:
enum numa_mode {
numa_off,
numa_on,
numa_acpi,
};
is, I believe, sufficient to express the present (numa_off,acpi_numa)
tuple. In this context I'd like to point out that the two uses of
acpi_numa in srat_parse_regions() and srat_detect_node() should likely
be invocations of srat_disabled() instead, to also take numa_off into
account. This would then be addressed effectively as a side effect by
replacing open-coded uses as well as srat_disabled() by numa_mode() (or
whichever name the new helper would gain).
Jan
