On Wed, Jun 30, 2021 at 06:40:10PM +1000, Daniel Axtens wrote:
> HEAP_MAX_ADDR is confusing. Currently it is set to 32MB, except
> on ieee1275 on x86, where it is 64MB.
>
> There is a comment which purports to explain it:
>
> /* If possible, we will avoid claiming heap above this address, because it
> seems to cause relocation problems with OSes that link at 4 MiB */
>
> This doesn't make a lot of sense when the constants are well above 4MB
> already. It was not always this way. Prior to
> commit 7b5d0fe4440c ("Increase heap limit") in 2010, HEAP_MAX_SIZE and
> HEAP_MAX_ADDR were indeed 4MB. However, when the constants were increased
> the comment was left unchanged.
>
> It's been over a decade. It doesn't seem like we have problems with
> claims over 4MB on powerpc or x86 ieee1275. (sparc does things completely
> differently and never used the constant.)
>
> Drop the constant and the check.
>
> The only use of HEAP_MIN_SIZE was to potentially override the
> HEAP_MAX_ADDR check. It is now unused. Remove it.
>
> Signed-off-by: Daniel Axtens <d...@axtens.net>
Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
> ---
> grub-core/kern/ieee1275/init.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
> index d483e35eed2b..c5d091689f29 100644
> --- a/grub-core/kern/ieee1275/init.c
> +++ b/grub-core/kern/ieee1275/init.c
> @@ -45,9 +45,6 @@
> #include <grub/machine/kernel.h>
> #endif
>
> -/* The minimal heap size we can live with. */
> -#define HEAP_MIN_SIZE (unsigned long) (2 * 1024 * 1024)
> -
> /* The maximum heap size we're going to claim */
> #ifdef __i386__
> #define HEAP_MAX_SIZE (unsigned long) (64 * 1024 * 1024)
> @@ -55,14 +52,6 @@
> #define HEAP_MAX_SIZE (unsigned long) (32 * 1024 * 1024)
> #endif
>
> -/* If possible, we will avoid claiming heap above this address, because it
> - seems to cause relocation problems with OSes that link at 4 MiB */
> -#ifdef __i386__
> -#define HEAP_MAX_ADDR (unsigned long) (64 * 1024 * 1024)
> -#else
> -#define HEAP_MAX_ADDR (unsigned long) (32 * 1024 * 1024)
> -#endif
> -
> extern char _start[];
> extern char _end[];
>
> @@ -183,12 +172,6 @@ heap_init (grub_uint64_t addr, grub_uint64_t len,
> grub_memory_type_t type,
> if (*total + len > HEAP_MAX_SIZE)
> len = HEAP_MAX_SIZE - *total;
>
> - /* Avoid claiming anything above HEAP_MAX_ADDR, if possible. */
> - if ((addr < HEAP_MAX_ADDR) && /* if it's too
> late, don't bother */
> - (addr + len > HEAP_MAX_ADDR) && /* if
> it wasn't available anyway, don't bother */
> - (*total + (HEAP_MAX_ADDR - addr) > HEAP_MIN_SIZE)) /* only limit
> ourselves when we can afford to */
> - len = HEAP_MAX_ADDR - addr;
> -
> /* In theory, firmware should already prevent this from happening by not
> listing our own image in /memory/available. The check below is intended
> as a safeguard in case that doesn't happen. However, it doesn't protect
> --
> 2.30.2
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
