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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.09.2022 11:12, Wei Chen wrote:
> +static bool __init nodes_cover_memory(void)
> +{
> +    unsigned int i;
> +
> +    for ( i = 0; ; i++ )
> +    {
> +        int err;
> +        bool found;
> +        unsigned int j;
> +        paddr_t start, end;
> +
> +        /* Try to loop memory map from index 0 to end to get RAM ranges. */
> +        err = arch_get_ram_range(i, &start, &end);
> +
> +        /* Reached the end of the memory map? */
> +        if ( err == -ENOENT )
> +            break;
> +
> +        /* Skip non-RAM entries. */
> +        if ( err )
> +            continue;
> +
> +        do {
> +            found = false;
> +            for_each_node_mask ( j, memory_nodes_parsed )
> +                if ( start < nodes[j].end
> +                    && end > nodes[j].start )

Nit: Style (placement of && and indentation). Does this actually need
splitting across two lines?

> --- 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.

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.

Jan