Re: [PATCH v2 3/6] ieee1275: support runtime memory claiming

[Daniel Kiper]

On Thu, Dec 01, 2022 at 04:11:58PM -0500, Stefan Berger wrote:
> From: Daniel Axtens <dja@axtens.net>
>
> 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
>    difficult to change with appended signatures.
>  - We only have 32MB of heap.
>  - Distro kernels are now often around 30MB.
>
> So we want to be able to claim more memory from OpenFirmware for our heap
> at runtime.
>
> 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. Even with our CAS work, an LPAR
>    on a PowerVM box is likely to have only 512MB available to OpenFirmware
>    even if it has many gigabytes of memory allocated.
>
> What should we do?
>
> 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
>
> So to give us a little wriggle room, I think we want to leave at least
> 128MB for the loader to put vmlinux and initrd in memory and leave Linux
> with space to satisfy its early allocations.
>
> Allow other space to be allocated at runtime.
>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

[...]

> +static grub_err_t grub_ieee1275_mm_add_region (grub_size_t size, unsigned 
> int flags)

static grub_err_t
grub_ieee1275_mm_add_region (grub_size_t size, unsigned int flags)

> +{
> +  grub_uint32_t total = size;
> +  grub_uint32_t free_memory = 0;
> +
> +  grub_dprintf ("ieee1275", "mm requested region of size %x, flags %x\n",
> +               size, flags);
> +
> +  /*
> +   * Update free memory each time, which is a bit inefficient but guards us
> +   * against a situation where some OF driver goes out to firmware for
> +   * memory and we don't realise.
> +   */
> +  grub_machine_mmap_iterate (count_free, &free_memory);
> +
> +  /* Ensure we leave enough space to boot. */
> +  if (free_memory <= RUNTIME_MIN_SPACE + size)
> +    {
> +      grub_dprintf ("ieee1275", "Cannot satisfy allocation and retain 
> minimum runtime space\n");
> +      return GRUB_ERR_OUT_OF_MEMORY;
> +    }
> +
> +  grub_dprintf ("ieee1275", "free = 0x%x\n", free_memory);
> +
> +  if (flags & GRUB_MM_ADD_REGION_CONSECUTIVE)
> +    {
> +      /* first try rounding up hard for the sake of speed */
> +      total = grub_max (ALIGN_UP(size, 1024 * 1024) + 1024 * 1024, 32 * 1024 
> * 1024);

s/ALIGN_UP(/ALIGN_UP (/

> +      total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
> +
> +      grub_dprintf ("ieee1275", "looking for %x bytes of memory (%x 
> requested)\n", total, size);
> +
> +      grub_machine_mmap_iterate (region_claim, &total);
> +      grub_dprintf ("ieee1275", "get memory from fw %s\n", total == 0 ? 
> "succeeded" : "failed");
> +
> +      /* fallback: requested size + padding for a grub_mm_header_t and 
> region */

This...

> +      if (total != 0)
> +        {
> +          total = size + 48;

... and this should be dropped. I think this patch set, [1], [2], solves
this kinds of problems in general. Please take a look...

> +          total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
> +
> +          grub_dprintf ("ieee1275", "fallback for %x bytes of memory (%x 
> requested)\n", total, size);
> +
> +          grub_machine_mmap_iterate (region_claim, &total);
> +          grub_dprintf ("ieee1275", "fallback from fw %s\n", total == 0 ? 
> "succeeded" : "failed");
> +        }
> +    }
> +  else
> +    {
> +      /* provide padding for a grub_mm_header_t and region */
> +      total = size + 48;

Ditto... And then probably total can be dropped too...

> +      total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
> +      grub_machine_mmap_iterate (heap_init, &total);
> +      grub_dprintf ("ieee1275", "get noncontig memory from fw %s\n", total 
> == 0 ? "succeeded" : "failed");
> +    }
> +
> +  if (total == 0)
> +    return GRUB_ERR_NONE;
> +  else
> +    return GRUB_ERR_OUT_OF_MEMORY;
> +}

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2022-11/msg00147.html
[2] https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00115.html