Re: [PATCH] xen/arm: Add Kconfig parameter for memory banks number

[Julien Grall <julien@xxxxxxx>]

Hi Luca,

On 06/12/2021 15:37, Luca Fancellu wrote:
> Currently the maximum number of memory banks is fixed to
> 128, but on some new platforms that have a large amount
> of memory, this value is not enough 
Can you provide some information on the setup? Is it using UEFI?

> and prevents Xen
> from booting.
AFAIK, the restriction should only prevent Xen to use all the memory. If 
that's not the case, then this should be fixed.
> Create a Kconfig parameter to set the value, by default
> 128.
I think Xen should be able to boot on any platform with the default 
configuration. So the value should at least be bumped.
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
>   xen/arch/arm/Kconfig        | 8 ++++++++
>   xen/include/asm-arm/setup.h | 2 +-
>   2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index ecfa6822e4d3..805e3c417e89 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -25,6 +25,14 @@ menu "Architecture Features"
>   
>   source "arch/Kconfig"
>   
> +config MEM_BANKS
> +       int "Maximum number of memory banks."
> +       default "128"
> +       help
> +         Controls the build-time size memory bank array.
> +         It is the upper bound of the number of logical entities describing
> +         the memory.
NR_MEM_BANKS is going to be used by multiple internal structure in Xen 
(e.g. static memory, reserved memory, normal memory). So how could an 
admin decide the correct value?
In particular for UEFI, we are at the mercy of the firmware that can 
expose any kind of memory map (that's why we had to increase the 
original number of banks).
So maybe it is time for us to move out from a static array and re-think 
how we discover the memory.
That this is probably going to take some time to get it properly, so
I would be OK with bumping the value + a config gated UNSUPPORTED.

> +
>   config ACPI
>         bool "ACPI (Advanced Configuration and Power Interface) Support 
> (UNSUPPORTED)" if UNSUPPORTED
>         depends on ARM_64
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 95da0b7ab9cd..785a8fe81450 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -6,7 +6,7 @@
>   #define MIN_FDT_ALIGN 8
>   #define MAX_FDT_SIZE SZ_2M
>   
> -#define NR_MEM_BANKS 128
> +#define NR_MEM_BANKS CONFIG_MEM_BANKS
>   
>   #define MAX_MODULES 32 /* Current maximum useful modules */
>   
Cheers,

--
Julien Grall