Hi, > On 7 Dec 2021, at 10:52, Luca Fancellu <[email protected]> wrote: > > > >> On 6 Dec 2021, at 17:05, Julien Grall <[email protected]> wrote: >> >> 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 >> > > Hi Julien, > >> Can you provide some information on the setup? Is it using UEFI? > > Yes it is a platform with 32gb of ram, I’ve built Xen with ACPI support and > it’s starting using UEFI+ACPI. > >> >>> 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. > > The code that it’s failing is this, inside efi_process_memory_map_bootinfo(…) > in the arch/arm/efi/efi-boot.h: > > #ifdef CONFIG_ACPI > else if ( desc_ptr->Type == EfiACPIReclaimMemory ) > { > if ( !meminfo_add_bank(&bootinfo.acpi, desc_ptr) ) > { > PrintStr(L"Error: All " __stringify(NR_MEM_BANKS) > " acpi meminfo mem banks exhausted.\r\n"); > return EFI_LOAD_ERROR; > } > } > #endif > >> >>> 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 <[email protected]> >>> --- >>> 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.
Looking at what we have, the memory is actually fragmented by ACPI but a long term solution could be to make the code a little bit more smart and try to merge together adjacent banks. I would suggest to just increase the existing define to 256 to fix the current issue (which might be encountered by anybody using ACPI) and put a comment in the code for now with a TODO explaining why we currently need such a high value and what should be done to fix this. Cheers Bertrand > > I can do that. > > Cheers, > Luca > >> >>> + >>> 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
