All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Kiper <dkiper@net-space.pl>
To: Daniel Axtens <dja@axtens.net>
Cc: grub-devel@gnu.org, rashmica.g@gmail.com, alastair@d-silva.org,
	nayna@linux.ibm.com, chris.coulson@canonical.com,
	hanson.char@gmail.com, hchar@amazon.com, javierm@redhat.com,
	leif@nuviainc.com, phcoder@gmail.com, pjones@redhat.com,
	pmenzel@molgen.mpg.de, ps@pks.im, stefanb@linux.ibm.com,
	xnox@ubuntu.com
Subject: Re: [PATCH v2 02/22] ieee1275: claim more memory
Date: Thu, 15 Jul 2021 23:51:04 +0200	[thread overview]
Message-ID: <20210715215104.dgcfefezytxq6exe@tomti.i.net-space.pl> (raw)
In-Reply-To: <20210630084031.2663622-3-dja@axtens.net>

CC-in a few people who can be interested in this...

On Wed, Jun 30, 2021 at 06:40:11PM +1000, Daniel Axtens wrote:
> On powerpc-ieee1275, we are running out of memory trying to verify
> anything. This is because:
>
>  - we have to load an entire file into memory to verify it. This is
>    extremely difficult to change with appended signatures.
>  - We only have 32MB of heap.
>  - Distro kernels are now often around 30MB.
>
> So we want to claim more memory from OpenFirmware for our heap.

AFAICT it is common problem in the GRUB right now. Please take a look at
[1], [2]. It would be nice to find general solution for all. Of course
if possible. So, if somebody could take a look at memory management in
the GRUB and propose a solution for the problem that would be perfect.
Any volunteers?

Daniel

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

> There are some complications:
>
>  - The grub mm code isn't the only thing that will make claims on
>    memory from OpenFirmware:
>
>     * PFW/SLOF will have claimed some for their own use.
>
>     * The ieee1275 loader will try to find other bits of memory that we
>       haven't claimed to place the kernel and initrd when we go to boot.
>
>     * Once we load Linux, it will also try to claim memory. It claims
>       memory without any reference to /memory/available, it just starts
>       at min(top of RMO, 768MB) and works down. So we need to avoid this
>       area. See arch/powerpc/kernel/prom_init.c as of v5.11.
>
>  - The smallest amount of memory a ppc64 KVM guest can have is 256MB.
>    It doesn't work with distro kernels but can work with custom kernels.
>    We should maintain support for that. (ppc32 can boot with even less,
>    and we shouldn't break that either.)
>
>  - Even if a VM has more memory, the memory OpenFirmware makes available
>    as Real Memory Area can be restricted. A freshly created LPAR on a
>    PowerVM machine is likely to have only 256MB available to OpenFirmware
>    even if it has many gigabytes of memory allocated.
>
> EFI systems will attempt to allocate 1/4th of the available memory,
> clamped to between 1M and 1600M. That seems like a good sort of
> approach, we just need to figure out if 1/4 is the right fraction
> for us.
>
> We don't know in advance how big the kernel and initrd are going to be,
> which makes figuring out how much memory we can take a bit tricky.
>
> To figure out how much memory we should leave unused, I looked at:
>
>  - an Ubuntu 20.04.1 ppc64le pseries KVM guest:
>     vmlinux: ~30MB
>     initrd:  ~50MB
>
>  - a RHEL8.2 ppc64le pseries KVM guest:
>     vmlinux: ~30MB
>     initrd:  ~30MB
>
> Ubuntu VMs struggle to boot with just 256MB under SLOF.
> RHEL likewise has a higher minimum supported memory figure.
> So lets first consider a distro kernel and 512MB of addressible memory.
> (This is the default case for anything booting under PFW.) Say we lose
> 131MB to PFW (based on some tests). This leaves us 381MB. 1/4 of 381MB
> is ~95MB. That should be enough to verify a 30MB vmlinux and should
> leave plenty of space to load Linux and the initrd.
>
> If we consider 256MB of RMA under PFW, we have just 125MB remaining. 1/4
> of that is a smidge under 32MB, which gives us very poor odds of verifying
> a distro-sized kernel. However, if we need 80MB just to put the kernel
> and initrd in memory, we can't claim any more than 45MB anyway. So 1/4
> will do. We'll come back to this later.
>
> grub is always built as a 32-bit binary, even if it's loading a ppc64
> kernel. So we can't address memory beyond 4GB. This gives a natural cap
> of 1GB for powerpc-ieee1275.
>
> Also apply this 1/4 approach to i386-ieee1275, but keep the 32MB cap.
>
> make check still works for both i386 and powerpc and I've booted
> powerpc grub with this change under SLOF and PFW.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
>  docs/grub-dev.texi             |  6 ++-
>  grub-core/kern/ieee1275/init.c | 81 +++++++++++++++++++++++++++-------
>  2 files changed, 69 insertions(+), 18 deletions(-)
>
> diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
> index 6c629a23e2dc..c11f1ac46de2 100644
> --- a/docs/grub-dev.texi
> +++ b/docs/grub-dev.texi
> @@ -1047,7 +1047,9 @@ space is limited to 4GiB. GRUB allocates pages from EFI for its heap, at most
>  1.6 GiB.
>
>  On i386-ieee1275 and powerpc-ieee1275 GRUB uses same stack as IEEE1275.
> -It allocates at most 32MiB for its heap.
> +
> +On i386-ieee1275, GRUB allocates at most 32MiB for its heap. On
> +powerpc-ieee1275, GRUB allocates up to 1GiB.
>
>  On sparc64-ieee1275 stack is 256KiB and heap is 2MiB.
>
> @@ -1075,7 +1077,7 @@ In short:
>  @item i386-qemu               @tab 60 KiB  @tab < 4 GiB
>  @item *-efi                   @tab ?       @tab < 1.6 GiB
>  @item i386-ieee1275           @tab ?       @tab < 32 MiB
> -@item powerpc-ieee1275        @tab ?       @tab < 32 MiB
> +@item powerpc-ieee1275        @tab ?       @tab < 1 GiB
>  @item sparc64-ieee1275        @tab 256KiB  @tab 2 MiB
>  @item arm-uboot               @tab 256KiB  @tab 2 MiB
>  @item mips(el)-qemu_mips      @tab 2MiB    @tab 253 MiB
> diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
> index c5d091689f29..4162b5949df4 100644
> --- a/grub-core/kern/ieee1275/init.c
> +++ b/grub-core/kern/ieee1275/init.c
> @@ -45,11 +45,12 @@
>  #include <grub/machine/kernel.h>
>  #endif
>
> -/* The maximum heap size we're going to claim */
> +/* The maximum heap size we're going to claim. Not used by sparc.
> +   We allocate 1/4 of the available memory under 4G, up to this limit. */
>  #ifdef __i386__
>  #define HEAP_MAX_SIZE		(unsigned long) (64 * 1024 * 1024)
> -#else
> -#define HEAP_MAX_SIZE		(unsigned long) (32 * 1024 * 1024)
> +#else // __powerpc__
> +#define HEAP_MAX_SIZE		(unsigned long) (1 * 1024 * 1024 * 1024)
>  #endif
>
>  extern char _start[];
> @@ -145,16 +146,45 @@ grub_claim_heap (void)
>  				 + GRUB_KERNEL_MACHINE_STACK_SIZE), 0x200000);
>  }
>  #else
> -/* Helper for grub_claim_heap.  */
> +/* Helper for grub_claim_heap on powerpc. */
> +static int
> +heap_size (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
> +	   void *data)
> +{
> +  grub_uint32_t total = *(grub_uint32_t *)data;
> +
> +  if (type != GRUB_MEMORY_AVAILABLE)
> +    return 0;
> +
> +  /* Do not consider memory beyond 4GB */
> +  if (addr > 0xffffffffUL)
> +    return 0;
> +
> +  if (addr + len > 0xffffffffUL)
> +    len = 0xffffffffUL - addr;
> +
> +  total += len;
> +  *(grub_uint32_t *)data = total;
> +
> +  return 0;
> +}
> +
>  static int
>  heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
>  	   void *data)
>  {
> -  unsigned long *total = data;
> +  grub_uint32_t total = *(grub_uint32_t *)data;
>
>    if (type != GRUB_MEMORY_AVAILABLE)
>      return 0;
>
> +  /* Do not consider memory beyond 4GB */
> +  if (addr > 0xffffffffUL)
> +    return 0;
> +
> +  if (addr + len > 0xffffffffUL)
> +    len = 0xffffffffUL - addr;
> +
>    if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_NO_PRE1_5M_CLAIM))
>      {
>        if (addr + len <= 0x180000)
> @@ -168,10 +198,6 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
>      }
>    len -= 1; /* Required for some firmware.  */
>
> -  /* Never exceed HEAP_MAX_SIZE  */
> -  if (*total + len > HEAP_MAX_SIZE)
> -    len = HEAP_MAX_SIZE - *total;
> -
>    /* 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
> @@ -183,6 +209,18 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
>        len = 0;
>      }
>
> +  /* If this block contains 0x30000000 (768MB), do not claim below that.
> +     Linux likes to claim memory at min(RMO top, 768MB) and works down
> +     without reference to /memory/available. */
> +  if ((addr < 0x30000000) && ((addr + len) > 0x30000000))
> +    {
> +      len = len - (0x30000000 - addr);
> +      addr = 0x30000000;
> +    }
> +
> +  if (len > total)
> +    len = total;
> +
>    if (len)
>      {
>        grub_err_t err;
> @@ -191,10 +229,12 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
>        if (err)
>  	return err;
>        grub_mm_init_region ((void *) (grub_addr_t) addr, len);
> +      total -= len;
>      }
>
> -  *total += len;
> -  if (*total >= HEAP_MAX_SIZE)
> +  *(grub_uint32_t *)data = total;
> +
> +  if (total == 0)
>      return 1;
>
>    return 0;
> @@ -203,13 +243,22 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
>  static void
>  grub_claim_heap (void)
>  {
> -  unsigned long total = 0;
> +  grub_uint32_t total = 0;
>
>    if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_FORCE_CLAIM))
> -    heap_init (GRUB_IEEE1275_STATIC_HEAP_START, GRUB_IEEE1275_STATIC_HEAP_LEN,
> -	       1, &total);
> -  else
> -    grub_machine_mmap_iterate (heap_init, &total);
> +    {
> +      heap_init (GRUB_IEEE1275_STATIC_HEAP_START, GRUB_IEEE1275_STATIC_HEAP_LEN,
> +		 1, &total);
> +      return;
> +    }
> +
> +  grub_machine_mmap_iterate (heap_size, &total);
> +
> +  total = total / 4;
> +  if (total > HEAP_MAX_SIZE)
> +    total = HEAP_MAX_SIZE;
> +
> +  grub_machine_mmap_iterate (heap_init, &total);
>  }
>  #endif
>
> --
> 2.30.2


  parent reply	other threads:[~2021-07-15 21:51 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-30  8:40 [PATCH v2 00/22] appended signature secure boot support Daniel Axtens
2021-06-30  8:40 ` [PATCH v2 01/22] ieee1275: drop HEAP_MAX_ADDR, HEAP_MIN_SIZE Daniel Axtens
2021-07-12 12:33   ` Stefan Berger
2021-07-14 16:21   ` Daniel Kiper
2021-06-30  8:40 ` [PATCH v2 02/22] ieee1275: claim more memory Daniel Axtens
2021-07-12 12:35   ` Stefan Berger
2021-07-15 21:51   ` Daniel Kiper [this message]
2021-07-16  3:59     ` Patrick Steinhardt
2021-07-21 14:45       ` Daniel Kiper
2021-07-21 15:24         ` Stefan Berger
2021-07-22 17:11         ` Stefan Berger
2021-07-28 11:17       ` Daniel Kiper
2021-06-30  8:40 ` [PATCH v2 03/22] ieee1275: request memory with ibm, client-architecture-support Daniel Axtens
2021-07-12 12:40   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 04/22] Add suport for signing grub with an appended signature Daniel Axtens
2021-07-12 12:43   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 05/22] docs/grub: Document signing grub under UEFI Daniel Axtens
2021-07-12 12:44   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 06/22] docs/grub: Document signing grub with an appended signature Daniel Axtens
2021-07-12 12:46   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 07/22] dl: provide a fake grub_dl_set_persistent for the emu target Daniel Axtens
2021-07-12 12:48   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 08/22] pgp: factor out rsa_pad Daniel Axtens
2021-07-12 12:52   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 09/22] crypto: move storage for grub_crypto_pk_* to crypto.c Daniel Axtens
2021-07-12 12:54   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 10/22] posix_wrap: tweaks in preparation for libtasn1 Daniel Axtens
2021-07-12 12:56   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 11/22] libtasn1: import libtasn1-4.16.0 Daniel Axtens
2021-07-20 21:46   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 12/22] libtasn1: disable code not needed in grub Daniel Axtens
2021-07-20 21:47   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 13/22] libtasn1: changes for grub compatibility Daniel Axtens
2021-07-12 13:04   ` Stefan Berger
2022-04-21  6:16     ` Daniel Axtens
2021-06-30  8:40 ` [PATCH v2 14/22] libtasn1: compile into asn1 module Daniel Axtens
2021-07-12 13:05   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 15/22] test_asn1: test module for libtasn1 Daniel Axtens
2021-07-12 19:35   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 16/22] grub-install: support embedding x509 certificates Daniel Axtens
2021-07-12 20:24   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 17/22] appended signatures: import GNUTLS's ASN.1 description files Daniel Axtens
2021-07-19 21:09   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 18/22] appended signatures: parse PKCS#7 signedData and X.509 certificates Daniel Axtens
2021-07-19 22:02   ` Stefan Berger
2022-04-21  6:36     ` Daniel Axtens
2021-06-30  8:40 ` [PATCH v2 19/22] appended signatures: support verifying appended signatures Daniel Axtens
2021-07-20  1:31   ` Stefan Berger
2022-04-21  7:10     ` Daniel Axtens
2021-06-30  8:40 ` [PATCH v2 20/22] appended signatures: verification tests Daniel Axtens
2021-07-20 12:49   ` Stefan Berger
2021-06-30  8:40 ` [PATCH v2 21/22] appended signatures: documentation Daniel Axtens
2021-07-19 22:24   ` Stefan Berger
2022-04-21  7:15     ` Daniel Axtens
2021-06-30  8:40 ` [PATCH v2 22/22] ieee1275: enter lockdown based on /ibm,secure-boot Daniel Axtens
2021-07-19 22:08   ` Stefan Berger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210715215104.dgcfefezytxq6exe@tomti.i.net-space.pl \
    --to=dkiper@net-space.pl \
    --cc=alastair@d-silva.org \
    --cc=chris.coulson@canonical.com \
    --cc=dja@axtens.net \
    --cc=grub-devel@gnu.org \
    --cc=hanson.char@gmail.com \
    --cc=hchar@amazon.com \
    --cc=javierm@redhat.com \
    --cc=leif@nuviainc.com \
    --cc=nayna@linux.ibm.com \
    --cc=phcoder@gmail.com \
    --cc=pjones@redhat.com \
    --cc=pmenzel@molgen.mpg.de \
    --cc=ps@pks.im \
    --cc=rashmica.g@gmail.com \
    --cc=stefanb@linux.ibm.com \
    --cc=xnox@ubuntu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.