grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Make EFI minimum heap size configurable via --enable-efi-min


From: Patrick Steinhardt
Subject: Re: [PATCH] Make EFI minimum heap size configurable via --enable-efi-min-heap-mb
Date: Tue, 5 Jan 2021 11:47:36 +0100

On Tue, Jan 05, 2021 at 10:02:12AM +0100, Paul Menzel wrote:
> Dear Hanson,
> 
> 
> Am 04.01.21 um 20:34 schrieb Char, Hanson via Grub-devel:
> > When booted in UEFI mode, Grub would fail to load a ramdisk of size larger 
> > than "(total_pages >> 2)" with
> > 
> >      "error: out of memory"
> > 
> > (https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/efi/mm.c#n616)
> > 
> > This proposed patch adds a new option that can be used to specify the "EFI 
> > min heap size" in MB.  For example,
> > 
> >      ./configure --with-platform=efi --target=x86_64 
> > --enable-efi-min-heap-mb=600
> > 
> > The patch has been successfully tested to load large ramdisk with size that 
> > would otherwise fail.
> 
> Thank you for the patch and description. Could you please add it as the 
> git commit message, and send a v2, maybe even using `git send-email`?
> 
> >  From 9bd4220a6b0fe2d49c6aed347f5d2bbff5fb2d8d Mon Sep 17 00:00:00 2001
> > From: Hanson Char <hchar@amazon.com>
> > Date: Thu, 31 Dec 2020 12:49:33 -0800
> > Subject: [PATCH] Make EFI minimum heap size configurable via 
> > --enable-efi-min-heap-mb
> 
> Please excuse my ignorance, but I do not fully understand the problem 
> and solution yet. What has the minimum heap size to do with the problem, 
> and why does increasing it fixes it?

The memory allocator is quite limited in that it can only allocate a
given maximum amount of bytes. I don't know about the author's problem
context here, but I faced this problem when implementing Argon2i support
for LUKS2 disk encryption. Due to its memory-hardness, it typically
requires at least 1GB of RAM, which is currently impossible to allocate
via the EFI allocator. I guess this patch here squarely aims at a
similar problem.

I'm not sure making this configurable at build time is the right thing
to do, though. It's quite an easy fix, but it doesn't sound like it's
going to be viable for distributions. They cannot know up front how much
RAM is available in their users' machines, so picking a value that suits
all users is next to impossible.

I do have a patch series on this mailing list which instead alters the
memory allocation strategy to allocate more RAM via the EFI service when
we're hitting the limit [1]. It's not polished and the current strategy
is probably debatable, but I think going into that direction is a much
saner approach as it dynamically scales with whatever the system
requires without having to make it a build-time switch. It's been some
time since I last visited that patch, which can be blamed on the pending
v2.06 release. But as soon as that's out of the door, I want to revisit
it to finally get usable Argon2i support.

Anyway, I don't want to discourage you to work on this topic, I just
wanted to make you aware of my plans here to make the allocator itself
more flexible.

Patrick

[1]: https://lists.gnu.org/archive/html/grub-devel/2020-06/msg00009.html

> ```
>    /* By default, request a quarter of the available memory.  */
>    total_pages = get_total_pages (filtered_memory_map, desc_size,
>                                   filtered_memory_map_end);
>    required_pages = (total_pages >> 2);
>    if (required_pages < BYTES_TO_PAGES (MIN_HEAP_SIZE))
>      required_pages = BYTES_TO_PAGES (MIN_HEAP_SIZE);
>    else if (required_pages > BYTES_TO_PAGES (MAX_HEAP_SIZE))
>      required_pages = BYTES_TO_PAGES (MAX_HEAP_SIZE);
> ```
> 
> Currently, MIN_HEAP_SIZE is 1 MB, and setting it to 600 MB fixes the 
> issue for you. If `required_pages` is smaller than “1 MB pages” it’s 
> also smaller than “600 MB pages“, isn’t it?
> 
> 
> Kind regards,
> 
> Paul
> 
> > ---
> > config.h.in             |  2 ++
> > configure.ac            | 18 ++++++++++++++++++
> > grub-core/kern/efi/mm.c |  2 +-
> > 3 files changed, 21 insertions(+), 1 deletion(-)
> > 
> > diff --git a/config.h.in b/config.h.in
> > index 9e8f9911b..413287410 100644
> > --- a/config.h.in
> > +++ b/config.h.in
> > @@ -13,6 +13,8 @@
> > #define DISK_CACHE_STATS @DISK_CACHE_STATS@
> > #define BOOT_TIME_STATS @BOOT_TIME_STATS@
> > 
> > +#define EFI_MIN_HEAP_MB @EFI_MIN_HEAP_MB@
> > +
> > /* We don't need those.  */
> > #define MINILZO_CFG_SKIP_LZO_PTR 1
> > #define MINILZO_CFG_SKIP_LZO_UTIL 1
> > diff --git a/configure.ac b/configure.ac
> > index 7c10a4db7..8c4bb820e 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1934,6 +1934,21 @@ AC_SUBST(HOST_CCASFLAGS)
> > 
> > AC_SUBST(BUILD_LIBM)
> > 
> > +#
> > +# EFI minimum heap size in MB ranging from 1 to 1600.  Default is 1.
> > +#
> > +AC_ARG_ENABLE([efi-min-heap-mb],
> > +              [AS_HELP_STRING([--enable-efi-min-heap-mb=SIZE_IN_MB],
> > +                              [set the EFI minimum heap size in MB between 
> > 1 and 1600 inclusive])])
> > +AS_IF([test x"$enable_efi_min_heap_mb" = x -o x"$enable_efi_min_heap_mb" = 
> > xno],
> > +      [EFI_MIN_HEAP_MB=1],
> > +      [AS_IF([test x"${enable_efi_min_heap_mb//@<:@0-9@:>@/}" != x \
> > +           || test "$enable_efi_min_heap_mb" -lt 1 -o 
> > "$enable_efi_min_heap_mb" -gt 1600],
> > +             [AC_MSG_ERROR([EFI minimum heap size must be between 1 and 
> > 1600 inclusive])],
> > +             [EFI_MIN_HEAP_MB=$enable_efi_min_heap_mb])])
> > +
> > +AC_SUBST([EFI_MIN_HEAP_MB])
> > +
> > #
> > # Automake conditionals
> > #
> > @@ -2132,5 +2147,8 @@ echo "Without liblzma (no support for XZ-compressed 
> > mips images) ($liblzma_excus
> > else
> > echo "With liblzma from $LIBLZMA (support for XZ-compressed mips images)"
> > fi
> > +if [ x"$enable_efi_min_heap_mb" != x ]; then
> > +echo efi-min-heap-mb: $EFI_MIN_HEAP_MB
> > +fi
> > echo "*******************************************************"
> > ]
> > diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> > index 457772d57..ac5112b57 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 minimum and maximum heap size for GRUB itself.  */
> > -#define MIN_HEAP_SIZE  0x100000
> > +#define MIN_HEAP_SIZE  (EFI_MIN_HEAP_MB * 0x100000)
> > #define MAX_HEAP_SIZE   (1600 * 0x100000)
> > 
> > static void *finish_mmap_buf = 0;
> > --
> > 2.29.2

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]