Re: [PATCH v5 1/6] xen/x86: Provide helpers for common code to access acpi_numa

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.09.2022 11:12, Wei Chen wrote:
> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -50,9 +50,28 @@ nodemask_t __read_mostly node_online_map = { { [0] = 1UL } 
> };
>  bool numa_off;
>  s8 acpi_numa = 0;
>  
> -int srat_disabled(void)
> +int __init arch_numa_setup(const char *opt)
>  {
> -    return numa_off || acpi_numa < 0;
> +#ifdef CONFIG_ACPI_NUMA
> +    if ( !strncmp(opt, "noacpi", 6) )
> +    {
> +        numa_off = false;
> +        acpi_numa = -1;

When making the v5 changes, did you go over the results to check they are
actually consistent? I'm afraid they still aren't, because of the line
above: Here we disable NUMA, but that doesn't mean there's broken firmware.
Therefore I guess I need to ask for another round of renaming of the two
helper functions; I'm sorry for that. What you introduce ...

> +        return 0;
> +    }
> +#endif
> +
> +    return -EINVAL;
> +}
> +
> +bool arch_numa_broken(void)
> +{
> +    return acpi_numa < 0;
> +}

... here wants to be arch_numa_disabled(), whereas the function currently
named this way (in patch 5) wants to be e.g. arch_numa_unavailable() (or,
using inverted sense, arch_numa_available()). Of course I'll be happy to
see other naming suggestions for both functions, as long as they reflect
the respective purposes.

Alternatively, to retain the current naming, the assignments to acpi_numa
would need revising. But I think that would be the more fragile approach.

Jan