Re: [PATCH] kern/efi/mm: Double the default heap size

[Heinrich Schuchardt]

On 8/20/22 17:24, Hector Martin via Grub-devel wrote:
> GRUB is already running out of memory on Apple M1 systems, causing
> graphics init to fail, as of the latest Git changes. Since dynamic
> growing of the heap isn't done yet, double the default heap size for
> now.
>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   grub-core/kern/efi/mm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> index d290c9a76270..377d8d3a1c1b 100644
> --- a/grub-core/kern/efi/mm.c
> +++ b/grub-core/kern/efi/mm.c
> @@ -39,7 +39,7 @@
>   #define MEMORY_MAP_SIZE       0x3000
>   
>   /* The default heap size for GRUB itself in bytes.  */
> -#define DEFAULT_HEAP_SIZE      0x100000
> +#define DEFAULT_HEAP_SIZE      0x200000

GRUB's memory is allocated as LoaderCode and therefore released at 
ExitBootServices(). There should be no problem to allocate even much 
higher amounts of memory.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Beyond the scope of this patch:

Using LoaderCode here seems wrong. Heap should be LoaderData.

The problematic part of heap allocation is function add_memory_regions() 
that does not let AllocatePages() do its job to find a fitting region 
but just forces a random free memory region. It would be preferable to 
call AllocatePages() not with GRUB_EFI_ALLOCATE_ADDRESS but with 
GRUB_EFI_ALLOCATE_MAX_ADDRESS to avoid problems on architectures that 
want to move the kernel into low memory.

Best regards

Heinrich

>   
>   static void *finish_mmap_buf = 0;
>   static grub_efi_uintn_t finish_mmap_size = 0;