[PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Axtens]

Hi all,

This is, at long last, an updated version of my series extending Patrick's
dynamic memory regions to ieee1275.

Noteworthy changes:

 - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
   ieee1275 patches.

 - reworked the ieee1275 runtime memory claiming to be more resilient and better
   documented.

 - fixed comment style and hopefully addressed all other change requests.

 - grub will now try asking for contiguous memory and then, if that fails, for
   discontiguous memory - in case region merging with the discontiguous memory
   is sufficient to allow the eventual allocation to succeed.

 - The ieee1275 code agressively rounds up the size of the region for a dynamic
   allocation - it will now retry with a more precise size if the larger
   allocation fails.

The memtool module is included as an RFC only and will require more work, as
discussed in the patch.

I've also included Stefan's vTPM patch - Stefan kindly tested that his patch
worked with my memory rework series. I have added his tested-by to relevant
Power-specific patches only.

Kind regards,
Daniel

Daniel Axtens (8):
  grub-shell: pseries: don't pass fw_opt to qemu
  mm: assert that we preserve header vs region alignment
  mm: when adding a region, merge with region after as well as before
  mm: debug support for region operations
  ieee1275: request memory with ibm,client-architecture-support
  ieee1275: drop len -= 1 quirk in heap_init
  ieee1275: support runtime memory claiming
  [RFC] Add memtool module with memory allocation stress-test

Patrick Steinhardt (6):
  mm: Drop unused unloading of modules on OOM
  mm: Allow dynamically requesting additional memory regions
  efi: mm: Always request a fixed number of pages on init
  efi: mm: Extract function to add memory regions
  efi: mm: Pass up errors from `add_memory_regions ()`
  efi: mm: Implement runtime addition of pages

Stefan Berger (1):
  ibmvtpm: Add support for trusted boot using a vTPM 2.0

[PATCH v2 01/15] grub-shell: only pass SeaBIOS fw_opt in for x86 BIOS platforms

[Daniel Axtens]

This breaks the tests on pseries - just restrict it to x86 platforms
that don't specify a BIOS.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
v2: Thanks Daniel K and Glenn for feedback.
---
 tests/util/grub-shell.in | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 33590baeb13c..4828afb7cc5a 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -376,7 +376,11 @@ if test -z "$debug"; then
   # workaround unfortunately causes qemu to issue a warning 'externally
   # provided fw_cfg item names should be prefixed with "opt/"', but there
   # doesn't seem to be a better option.
-  qemuopts="${qemuopts} -fw_cfg name=etc/sercon-port,string=0"
+  #
+  # SeaBIOS is used for i386, except on EFI.
+  if [ ${grub_modinfo_target_cpu} == 'i386' ] && [ ${grub_modinfo_platform} != 'efi' ]; then
+    qemuopts="${qemuopts} -fw_cfg name=etc/sercon-port,string=0"
+  fi
 fi
 
 if [ x$boot != xnet ] && [ x$boot != xemu ]; then
-- 
2.32.0

Re: [PATCH v2 01/15] grub-shell: only pass SeaBIOS fw_opt in for x86 BIOS platforms

[Daniel Kiper]

On Mon, Mar 28, 2022 at 05:22:26PM +1100, Daniel Axtens wrote:
> This breaks the tests on pseries - just restrict it to x86 platforms
> that don't specify a BIOS.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

[PATCH v2 02/15] mm: assert that we preserve header vs region alignment

[Daniel Axtens]

grub_mm_region_init() does:

  h = (grub_mm_header_t) (r + 1);

where h is a grub_mm_header_t and r is a grub_mm_region_t.

Cells are supposed to be GRUB_MM_ALIGN aligned, but while grub_mm_dump
ensures this vs the region header, grub_mm_region_init() does not.

It's better to be explicit than implicit here: rather than changing
grub_mm_region_init() to ALIGN_UP(), require that the struct is
explictly a multiple of the header size.

Signed-off-by: Daniel Axtens <dja@axtens.net>

---
v2: Thanks Daniel K for feedback.
---
 include/grub/mm_private.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/grub/mm_private.h b/include/grub/mm_private.h
index 203533cc3d42..d3f2321e14fb 100644
--- a/include/grub/mm_private.h
+++ b/include/grub/mm_private.h
@@ -20,6 +20,7 @@
 #define GRUB_MM_PRIVATE_H	1
 
 #include <grub/mm.h>
+#include <grub/misc.h>
 
 /* For context, see kern/mm.c */
 
@@ -89,4 +90,17 @@ typedef struct grub_mm_region
 extern grub_mm_region_t EXPORT_VAR (grub_mm_base);
 #endif
 
+static inline void
+grub_mm_size_sanity_check (void) {
+  /* Ensure we preserve alignment when doing h = (grub_mm_header_t) (r + 1) */
+  COMPILE_TIME_ASSERT ((sizeof (struct grub_mm_region) %
+		        sizeof (struct grub_mm_header)) == 0);
+
+  /*
+   * GRUB_MM_ALIGN is supposed to represent cell size, and a mm_header is
+   * supposed to be 1 cell.
+   */
+  COMPILE_TIME_ASSERT (sizeof (struct grub_mm_header) == GRUB_MM_ALIGN);
+}
+
 #endif
-- 
2.32.0

Re: [PATCH v2 02/15] mm: assert that we preserve header vs region alignment

[Daniel Kiper]

On Mon, Mar 28, 2022 at 05:22:27PM +1100, Daniel Axtens wrote:
> grub_mm_region_init() does:
>
>   h = (grub_mm_header_t) (r + 1);
>
> where h is a grub_mm_header_t and r is a grub_mm_region_t.
>
> Cells are supposed to be GRUB_MM_ALIGN aligned, but while grub_mm_dump
> ensures this vs the region header, grub_mm_region_init() does not.
>
> It's better to be explicit than implicit here: rather than changing
> grub_mm_region_init() to ALIGN_UP(), require that the struct is
> explictly a multiple of the header size.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

[PATCH v2 03/15] mm: when adding a region, merge with region after as well as before

[Daniel Axtens]

On x86_64-efi (at least) regions seem to be added from top down. The mm
code will merge a new region with an existing region that comes
immediately before the new region. This allows larger allocations to be
satisfied that would otherwise be the case.

On powerpc-ieee1275, however, regions are added from bottom up. So if
we add 3x 32MB regions, we can still only satisfy a 32MB allocation,
rather than the 96MB allocation we might otherwise be able to satisfy.

 * Define 'post_size' as being bytes lost to the end of an allocation
   due to being given weird sizes from firmware that are not multiples
   of GRUB_MM_ALIGN.

 * Allow merging of regions immediately _after_ existing regions, not
   just before. As with the other approach, we create an allocated
   block to represent the new space and the pass it to grub_free() to
   get the metadata right.

Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>

---
v2: Thanks Daniel K for feedback.
---
 grub-core/kern/mm.c       | 123 +++++++++++++++++++++++---------------
 include/grub/mm_private.h |   9 +++
 2 files changed, 85 insertions(+), 47 deletions(-)

diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index 079c28da7cdf..94e78f9a910d 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -130,53 +130,81 @@ grub_mm_init_region (void *addr, grub_size_t size)
 
   /* Attempt to merge this region with every existing region */
   for (p = &grub_mm_base, q = *p; q; p = &(q->next), q = *p)
-    /*
-     * Is the new region immediately below an existing region? That
-     * is, is the address of the memory we're adding now (addr) + size
-     * of the memory we're adding (size) + the bytes we couldn't use
-     * at the start of the region we're considering (q->pre_size)
-     * equal to the address of q? In other words, does the memory
-     * looks like this?
-     *
-     * addr                          q
-     *   |----size-----|-q->pre_size-|<q region>|
-     */
-    if ((grub_uint8_t *) addr + size + q->pre_size == (grub_uint8_t *) q)
-      {
-	/*
-	 * Yes, we can merge the memory starting at addr into the
-	 * existing region from below. Align up addr to GRUB_MM_ALIGN
-	 * so that our new region has proper alignment.
-	 */
-	r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
-	/* Copy the region data across */
-	*r = *q;
-	/* Consider all the new size as pre-size */
-	r->pre_size += size;
-
-	/*
-	 * If we have enough pre-size to create a block, create a
-	 * block with it. Mark it as allocated and pass it to
-	 * grub_free (), which will sort out getting it into the free
-	 * list.
-	 */
-	if (r->pre_size >> GRUB_MM_ALIGN_LOG2)
-	  {
-	    h = (grub_mm_header_t) (r + 1);
-	    /* block size is pre-size converted to cells */
-	    h->size = (r->pre_size >> GRUB_MM_ALIGN_LOG2);
-	    h->magic = GRUB_MM_ALLOC_MAGIC;
-	    /* region size grows by block size converted back to bytes */
-	    r->size += h->size << GRUB_MM_ALIGN_LOG2;
-	    /* adjust pre_size to be accurate */
-	    r->pre_size &= (GRUB_MM_ALIGN - 1);
-	    *p = r;
-	    grub_free (h + 1);
-	  }
-	/* Replace the old region with the new region */
-	*p = r;
-	return;
-      }
+    {
+      /*
+       * Is the new region immediately below an existing region? That
+       * is, is the address of the memory we're adding now (addr) + size
+       * of the memory we're adding (size) + the bytes we couldn't use
+       * at the start of the region we're considering (q->pre_size)
+       * equal to the address of q? In other words, does the memory
+       * looks like this?
+       *
+       * addr                          q
+       *   |----size-----|-q->pre_size-|<q region>|
+       */
+      if ((grub_uint8_t *) addr + size + q->pre_size == (grub_uint8_t *) q)
+        {
+          /*
+           * Yes, we can merge the memory starting at addr into the
+           * existing region from below. Align up addr to GRUB_MM_ALIGN
+           * so that our new region has proper alignment.
+           */
+          r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
+          /* Copy the region data across */
+          *r = *q;
+          /* Consider all the new size as pre-size */
+          r->pre_size += size;
+
+          /*
+           * If we have enough pre-size to create a block, create a
+           * block with it. Mark it as allocated and pass it to
+           * grub_free (), which will sort out getting it into the free
+           * list.
+           */
+          if (r->pre_size >> GRUB_MM_ALIGN_LOG2)
+            {
+              h = (grub_mm_header_t) (r + 1);
+              /* block size is pre-size converted to cells */
+              h->size = (r->pre_size >> GRUB_MM_ALIGN_LOG2);
+              h->magic = GRUB_MM_ALLOC_MAGIC;
+              /* region size grows by block size converted back to bytes */
+              r->size += h->size << GRUB_MM_ALIGN_LOG2;
+              /* adjust pre_size to be accurate */
+              r->pre_size &= (GRUB_MM_ALIGN - 1);
+              *p = r;
+              grub_free (h + 1);
+            }
+          /* Replace the old region with the new region */
+          *p = r;
+          return;
+        }
+
+      /*
+       * Is the new region immediately above an existing region? That
+       * is:
+       *   q                       addr
+       *   |<q region>|-q->post_size-|----size-----|
+       */
+      if ((grub_uint8_t *)q + sizeof(*q) + q->size + q->post_size ==
+	  (grub_uint8_t *) addr)
+	{
+          /*
+           * Yes! Follow a similar pattern to above, but simpler.
+           * Our header starts at address - post_size, which should align us
+           * to a cell boundary.
+           */
+	  h = (grub_mm_header_t) ((grub_uint8_t *)addr - q->post_size);
+          /* our size is the allocated size plus post_size, in cells */
+	  h->size = (size + q->post_size) >> GRUB_MM_ALIGN_LOG2;
+	  h->magic = GRUB_MM_ALLOC_MAGIC;
+          /* region size grows by block size converted back to bytes */
+	  q->size += h->size << GRUB_MM_ALIGN_LOG2;
+          /* adjust new post_size to be accurate */
+	  q->post_size = (q->post_size + size) & (GRUB_MM_ALIGN - 1);
+	  grub_free (h + 1);
+	  return;
+	}
+    }
 
   /* Allocate a region from the head.  */
   r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
@@ -195,6 +223,7 @@ grub_mm_init_region (void *addr, grub_size_t size)
   r->first = h;
   r->pre_size = (grub_addr_t) r - (grub_addr_t) addr;
   r->size = (h->size << GRUB_MM_ALIGN_LOG2);
+  r->post_size = size - r->size;
 
   /* Find where to insert this region. Put a smaller one before bigger ones,
      to prevent fragmentation.  */
diff --git a/include/grub/mm_private.h b/include/grub/mm_private.h
index d3f2321e14fb..64d88ee86915 100644
--- a/include/grub/mm_private.h
+++ b/include/grub/mm_private.h
@@ -81,8 +81,17 @@ typedef struct grub_mm_region
    */
   grub_size_t pre_size;
 
+  /*
+   * Likewise, the post-size is the number of bytes we wasted at the end
+   * of the allocation because it wasn't a multiple of GRUB_MM_ALIGN
+   */
+  grub_size_t post_size;
+
   /* How many bytes are in this region? (free and allocated) */
   grub_size_t size;
+
+  /* pad to a multiple of cell size */
+  char padding[3 * GRUB_CPU_SIZEOF_VOID_P];
 }
 *grub_mm_region_t;
 
-- 
2.32.0

Re: [PATCH v2 03/15] mm: when adding a region, merge with region after as well as before

[Daniel Kiper]

On Mon, Mar 28, 2022 at 05:22:28PM +1100, Daniel Axtens wrote:
> On x86_64-efi (at least) regions seem to be added from top down. The mm
> code will merge a new region with an existing region that comes
> immediately before the new region. This allows larger allocations to be
> satisfied that would otherwise be the case.
>
> On powerpc-ieee1275, however, regions are added from bottom up. So if
> we add 3x 32MB regions, we can still only satisfy a 32MB allocation,
> rather than the 96MB allocation we might otherwise be able to satisfy.
>
>  * Define 'post_size' as being bytes lost to the end of an allocation
>    due to being given weird sizes from firmware that are not multiples
>    of GRUB_MM_ALIGN.
>
>  * Allow merging of regions immediately _after_ existing regions, not
>    just before. As with the other approach, we create an allocated
>    block to represent the new space and the pass it to grub_free() to
>    get the metadata right.
>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>

This tag should be added by Stefan in a reply to the email(s) send to
the grub-devel. Though I will accept it this time. And it should be
behind SOB.

> Signed-off-by: Daniel Axtens <dja@axtens.net>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

But there some nits below...

> ---
> v2: Thanks Daniel K for feedback.
> ---
>  grub-core/kern/mm.c       | 123 +++++++++++++++++++++++---------------
>  include/grub/mm_private.h |   9 +++
>  2 files changed, 85 insertions(+), 47 deletions(-)
>
> diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
> index 079c28da7cdf..94e78f9a910d 100644
> --- a/grub-core/kern/mm.c
> +++ b/grub-core/kern/mm.c
> @@ -130,53 +130,81 @@ grub_mm_init_region (void *addr, grub_size_t size)
>
>    /* Attempt to merge this region with every existing region */
>    for (p = &grub_mm_base, q = *p; q; p = &(q->next), q = *p)
> -    /*
> -     * Is the new region immediately below an existing region? That
> -     * is, is the address of the memory we're adding now (addr) + size
> -     * of the memory we're adding (size) + the bytes we couldn't use
> -     * at the start of the region we're considering (q->pre_size)
> -     * equal to the address of q? In other words, does the memory
> -     * looks like this?
> -     *
> -     * addr                          q
> -     *   |----size-----|-q->pre_size-|<q region>|
> -     */
> -    if ((grub_uint8_t *) addr + size + q->pre_size == (grub_uint8_t *) q)
> -      {
> -	/*
> -	 * Yes, we can merge the memory starting at addr into the
> -	 * existing region from below. Align up addr to GRUB_MM_ALIGN
> -	 * so that our new region has proper alignment.
> -	 */
> -	r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
> -	/* Copy the region data across */
> -	*r = *q;
> -	/* Consider all the new size as pre-size */
> -	r->pre_size += size;
> -
> -	/*
> -	 * If we have enough pre-size to create a block, create a
> -	 * block with it. Mark it as allocated and pass it to
> -	 * grub_free (), which will sort out getting it into the free
> -	 * list.
> -	 */
> -	if (r->pre_size >> GRUB_MM_ALIGN_LOG2)
> -	  {
> -	    h = (grub_mm_header_t) (r + 1);
> -	    /* block size is pre-size converted to cells */
> -	    h->size = (r->pre_size >> GRUB_MM_ALIGN_LOG2);
> -	    h->magic = GRUB_MM_ALLOC_MAGIC;
> -	    /* region size grows by block size converted back to bytes */
> -	    r->size += h->size << GRUB_MM_ALIGN_LOG2;
> -	    /* adjust pre_size to be accurate */
> -	    r->pre_size &= (GRUB_MM_ALIGN - 1);
> -	    *p = r;
> -	    grub_free (h + 1);
> -	  }
> -	/* Replace the old region with the new region */
> -	*p = r;
> -	return;
> -      }
> +    {
> +      /*
> +       * Is the new region immediately below an existing region? That
> +       * is, is the address of the memory we're adding now (addr) + size
> +       * of the memory we're adding (size) + the bytes we couldn't use
> +       * at the start of the region we're considering (q->pre_size)
> +       * equal to the address of q? In other words, does the memory
> +       * looks like this?
> +       *
> +       * addr                          q
> +       *   |----size-----|-q->pre_size-|<q region>|
> +       */
> +      if ((grub_uint8_t *) addr + size + q->pre_size == (grub_uint8_t *) q)
> +        {
> +          /*
> +           * Yes, we can merge the memory starting at addr into the
> +           * existing region from below. Align up addr to GRUB_MM_ALIGN
> +           * so that our new region has proper alignment.
> +           */
> +          r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
> +          /* Copy the region data across */
> +          *r = *q;
> +          /* Consider all the new size as pre-size */
> +          r->pre_size += size;
> +
> +          /*
> +           * If we have enough pre-size to create a block, create a
> +           * block with it. Mark it as allocated and pass it to
> +           * grub_free (), which will sort out getting it into the free
> +           * list.
> +           */
> +          if (r->pre_size >> GRUB_MM_ALIGN_LOG2)
> +            {
> +              h = (grub_mm_header_t) (r + 1);
> +              /* block size is pre-size converted to cells */
> +              h->size = (r->pre_size >> GRUB_MM_ALIGN_LOG2);
> +              h->magic = GRUB_MM_ALLOC_MAGIC;
> +              /* region size grows by block size converted back to bytes */
> +              r->size += h->size << GRUB_MM_ALIGN_LOG2;
> +              /* adjust pre_size to be accurate */
> +              r->pre_size &= (GRUB_MM_ALIGN - 1);
> +              *p = r;
> +              grub_free (h + 1);
> +            }
> +          /* Replace the old region with the new region */
> +          *p = r;
> +          return;
> +        }
> +
> +      /*
> +       * Is the new region immediately above an existing region? That
> +       * is:
> +       *   q                       addr
> +       *   |<q region>|-q->post_size-|----size-----|
> +       */
> +      if ((grub_uint8_t *)q + sizeof(*q) + q->size + q->post_size ==
> +	  (grub_uint8_t *) addr)
> +	{
> +          /*
> +           * Yes! Follow a similar pattern to above, but simpler.
> +           * Our header starts at address - post_size, which should align us
> +           * to a cell boundary.
> +           */
> +	  h = (grub_mm_header_t) ((grub_uint8_t *)addr - q->post_size);
> +          /* our size is the allocated size plus post_size, in cells */
> +	  h->size = (size + q->post_size) >> GRUB_MM_ALIGN_LOG2;
> +	  h->magic = GRUB_MM_ALLOC_MAGIC;
> +          /* region size grows by block size converted back to bytes */
> +	  q->size += h->size << GRUB_MM_ALIGN_LOG2;
> +          /* adjust new post_size to be accurate */

Comments and code are incorrectly aligned...

Anyway, I can fix this before committing.

Daniel

[PATCH v2 04/15] mm: debug support for region operations

[Daniel Axtens]

This is handy for debugging. Enable with `set debug=regions`.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 grub-core/kern/mm.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index 94e78f9a910d..f36499865632 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -115,9 +115,8 @@ grub_mm_init_region (void *addr, grub_size_t size)
   grub_mm_header_t h;
   grub_mm_region_t r, *p, q;
 
-#if 0
-  grub_printf ("Using memory for heap: start=%p, end=%p\n", addr, addr + (unsigned int) size);
-#endif
+  grub_dprintf ("regions", "Using memory for heap: start=%p, end=%p\n",
+                addr, (char *) addr + (unsigned int) size);
 
   /* Exclude last 4K to avoid overflows. */
   /* If addr + 0x1000 overflows then whole region is in excluded zone.  */
@@ -142,8 +141,14 @@ grub_mm_init_region (void *addr, grub_size_t size)
        * addr                          q
        *   |----size-----|-q->pre_size-|<q region>|
        */
+      grub_dprintf ("regions", "Can we extend into region above?"
+                    " %p + %" PRIxGRUB_SIZE " + %" PRIxGRUB_SIZE " ?=? %p\n",
+                    (grub_uint8_t *) addr, size, q->pre_size, (grub_uint8_t *) q);
       if ((grub_uint8_t *) addr + size + q->pre_size == (grub_uint8_t *) q)
         {
+	  grub_dprintf ("regions", "Yes: extending a region: (%p -> %p) -> (%p -> %p)\n",
+                        q, (grub_uint8_t *) q + sizeof (*q) + q->size,
+                        addr, (grub_uint8_t *) q + sizeof (*q) + q->size);
           /*
            * Yes, we can merge the memory starting at addr into the
            * existing region from below. Align up addr to GRUB_MM_ALIGN
@@ -185,9 +190,15 @@ grub_mm_init_region (void *addr, grub_size_t size)
        *   q                       addr
        *   |<q region>|-q->post_size-|----size-----|
        */
+      grub_dprintf ("regions", "Can we extend into region below?"
+                    " %p + %" PRIxGRUB_SIZE " + %" PRIxGRUB_SIZE " + %" PRIxGRUB_SIZE " ?=? %p\n",
+                    (grub_uint8_t *) q, sizeof(*q), q->size, q->post_size, (grub_uint8_t *) addr);
       if ((grub_uint8_t *)q + sizeof(*q) + q->size + q->post_size ==
 	  (grub_uint8_t *) addr)
 	{
+	  grub_dprintf ("regions", "Yes: extending a region: (%p -> %p) -> (%p -> %p)\n",
+                        q, (grub_uint8_t *) q + sizeof (*q) + q->size,
+                        q, (grub_uint8_t *) addr + size);
           /*
            * Yes! Follow a similar pattern to above, but simpler.
            * Our header starts at address - post_size, which should align us
@@ -206,6 +217,8 @@ grub_mm_init_region (void *addr, grub_size_t size)
 	}
     }
 
+  grub_dprintf ("regions", "No: considering a new region at %p of size %" PRIxGRUB_SIZE "\n",
+                addr, size);
   /* Allocate a region from the head.  */
   r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
 
-- 
2.32.0

Re: [PATCH v2 04/15] mm: debug support for region operations

[Daniel Kiper]

On Mon, Mar 28, 2022 at 05:22:29PM +1100, Daniel Axtens wrote:
> This is handy for debugging. Enable with `set debug=regions`.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

[PATCH v2 05/15] mm: Drop unused unloading of modules on OOM

[Daniel Axtens]

From: Patrick Steinhardt <ps@pks.im>

In `grub_memalign ()`, there's a commented section which would allow for
unloading of unneeded modules in case where there is not enough free
memory available to satisfy a request. Given that this code is never
compiled in, let's remove it together with `grub_dl_unload_unneeded()`

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 grub-core/kern/dl.c | 20 --------------------
 grub-core/kern/mm.c |  8 --------
 include/grub/dl.h   |  1 -
 3 files changed, 29 deletions(-)

diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
index b1d3a103ec98..e447fd0fab4e 100644
--- a/grub-core/kern/dl.c
+++ b/grub-core/kern/dl.c
@@ -811,23 +811,3 @@ grub_dl_unload (grub_dl_t mod)
   grub_free (mod);
   return 1;
 }
-
-/* Unload unneeded modules.  */
-void
-grub_dl_unload_unneeded (void)
-{
-  /* Because grub_dl_remove modifies the list of modules, this
-     implementation is tricky.  */
-  grub_dl_t p = grub_dl_head;
-
-  while (p)
-    {
-      if (grub_dl_unload (p))
-	{
-	  p = grub_dl_head;
-	  continue;
-	}
-
-      p = p->next;
-    }
-}
diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index f36499865632..90fa78a06735 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -437,14 +437,6 @@ grub_memalign (grub_size_t align, grub_size_t size)
       count++;
       goto again;
 
-#if 0
-    case 1:
-      /* Unload unneeded modules.  */
-      grub_dl_unload_unneeded ();
-      count++;
-      goto again;
-#endif
-
     default:
       break;
     }
diff --git a/include/grub/dl.h b/include/grub/dl.h
index d0f4115fe736..acb4d42327d7 100644
--- a/include/grub/dl.h
+++ b/include/grub/dl.h
@@ -203,7 +203,6 @@ grub_dl_t EXPORT_FUNC(grub_dl_load) (const char *name);
 grub_dl_t grub_dl_load_core (void *addr, grub_size_t size);
 grub_dl_t EXPORT_FUNC(grub_dl_load_core_noinit) (void *addr, grub_size_t size);
 int EXPORT_FUNC(grub_dl_unload) (grub_dl_t mod);
-extern void grub_dl_unload_unneeded (void);
 extern int EXPORT_FUNC(grub_dl_ref) (grub_dl_t mod);
 extern int EXPORT_FUNC(grub_dl_unref) (grub_dl_t mod);
 extern int EXPORT_FUNC(grub_dl_ref_count) (grub_dl_t mod);
-- 
2.32.0

[PATCH v2 06/15] mm: Allow dynamically requesting additional memory regions

[Daniel Axtens]

From: Patrick Steinhardt <ps@pks.im>

Currently, all platforms will set up their heap on initialization of the
platform code. While this works mostly fine, it poses some limitations
on memory management on us. Most notably, allocating big chunks of
memory in the gigabyte range would require us to pre-request this many
bytes from the firmware and add it to the heap from the beginning on
some platforms like EFI. As this isn't needed for most configurations,
it is inefficient and may even negatively impact some usecases when,
e.g., chainloading. Nonetheless, allocating big chunks of memory is
required sometimes, where one example is the upcoming support for the
Argon2 key derival function in LUKS2.

In order to avoid pre-allocating big chunks of memory, this commit
implements a runtime mechanism to add more pages to the system. When a
given allocation cannot be currently satisfied, we'll call a given
callback set up by the platform's own memory management subsystem,
asking it to add a memory area with at least `n` bytes. If this
succeeds, we retry searching for a valid memory region, which should now
succeed.

If this fails, we try asking for `n` bytes, possibly spread across
multiple regions, in hopes that region merging means that we end up
with enough memory for things to work out.

Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
[dja: add this to the documentation at the top of mm.c
      v2: fallback to non-contiguous]
Signed-off-by: Daniel Axtens <dja@axtens.net>

---

Daniel K: you had a Reviewed-by based on v1, but as the v2 changes are non-trivial
I have not included it.

I currently default to a fallback to non-contiguous on all platforms: if anyone
objects I'm happy to tweak it.
---
 grub-core/kern/mm.c | 30 ++++++++++++++++++++++++++++++
 include/grub/mm.h   | 16 ++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index 90fa78a06735..c3bf4646f55e 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -28,6 +28,9 @@
   - multiple regions may be used as free space. They may not be
   contiguous.
 
+  - if existing regions are insufficient to satisfy an allocation, a new
+  region can be requested from firmware.
+
   Regions are managed by a singly linked list, and the meta information is
   stored in the beginning of each region. Space after the meta information
   is used to allocate memory.
@@ -81,6 +84,7 @@
 \f
 
 grub_mm_region_t grub_mm_base;
+grub_mm_add_region_func_t grub_mm_add_region_fn;
 
 /* Get a header from the pointer PTR, and set *P and *R to a pointer
    to the header and a pointer to its region, respectively. PTR must
@@ -437,6 +441,32 @@ grub_memalign (grub_size_t align, grub_size_t size)
       count++;
       goto again;
 
+    case 1:
+      /* Request additional pages, contiguous */
+      count++;
+
+      if (grub_mm_add_region_fn != NULL &&
+          grub_mm_add_region_fn (size, GRUB_MM_ADD_REGION_CONSECUTIVE) == GRUB_ERR_NONE)
+	goto again;
+
+      /* fallthrough  */
+
+    case 2:
+      /* Request additional pages, anything at all */
+      count++;
+
+      if (grub_mm_add_region_fn != NULL)
+        {
+          /*
+           * Try again even if this fails, in case it was able to partially
+           * satisfy the request
+           */
+          grub_mm_add_region_fn (size, GRUB_MM_ADD_REGION_NONE);
+          goto again;
+        }
+
+      /* fallthrough */
+
     default:
       break;
     }
diff --git a/include/grub/mm.h b/include/grub/mm.h
index 44fde7cb9033..5d916809666c 100644
--- a/include/grub/mm.h
+++ b/include/grub/mm.h
@@ -20,6 +20,7 @@
 #ifndef GRUB_MM_H
 #define GRUB_MM_H	1
 
+#include <grub/err.h>
 #include <grub/types.h>
 #include <grub/symbol.h>
 #include <config.h>
@@ -28,6 +29,21 @@
 # define NULL	((void *) 0)
 #endif
 
+#define GRUB_MM_ADD_REGION_NONE        0
+#define GRUB_MM_ADD_REGION_CONSECUTIVE (1 << 0)
+
+/*
+ * Function used to request memory regions of `grub_size_t` bytes. The second
+ * parameter is a bitfield of `GRUB_MM_ADD_REGION` flags.
+ */
+typedef grub_err_t (*grub_mm_add_region_func_t) (grub_size_t, unsigned int);
+
+/*
+ * Set this function pointer to enable adding memory-regions at runtime in case
+ * a memory allocation cannot be satisfied with existing regions.
+ */
+extern grub_mm_add_region_func_t EXPORT_VAR(grub_mm_add_region_fn);
+
 void grub_mm_init_region (void *addr, grub_size_t size);
 void *EXPORT_FUNC(grub_calloc) (grub_size_t nmemb, grub_size_t size);
 void *EXPORT_FUNC(grub_malloc) (grub_size_t size);
-- 
2.32.0

Re: [PATCH v2 06/15] mm: Allow dynamically requesting additional memory regions

[Daniel Kiper]

On Mon, Mar 28, 2022 at 05:22:31PM +1100, Daniel Axtens wrote:
> From: Patrick Steinhardt <ps@pks.im>
>
> Currently, all platforms will set up their heap on initialization of the
> platform code. While this works mostly fine, it poses some limitations
> on memory management on us. Most notably, allocating big chunks of
> memory in the gigabyte range would require us to pre-request this many
> bytes from the firmware and add it to the heap from the beginning on
> some platforms like EFI. As this isn't needed for most configurations,
> it is inefficient and may even negatively impact some usecases when,
> e.g., chainloading. Nonetheless, allocating big chunks of memory is
> required sometimes, where one example is the upcoming support for the
> Argon2 key derival function in LUKS2.
>
> In order to avoid pre-allocating big chunks of memory, this commit
> implements a runtime mechanism to add more pages to the system. When a
> given allocation cannot be currently satisfied, we'll call a given
> callback set up by the platform's own memory management subsystem,
> asking it to add a memory area with at least `n` bytes. If this
> succeeds, we retry searching for a valid memory region, which should now
> succeed.
>
> If this fails, we try asking for `n` bytes, possibly spread across
> multiple regions, in hopes that region merging means that we end up
> with enough memory for things to work out.
>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> [dja: add this to the documentation at the top of mm.c
>       v2: fallback to non-contiguous]
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

[PATCH v2 07/15] efi: mm: Always request a fixed number of pages on init

[Daniel Axtens]

From: Patrick Steinhardt <ps@pks.im>

When initializing the EFI memory subsytem, we will by default request a
quarter of the available memory, bounded by a minimum/maximum value.
Given that we're about to extend the EFI memory system to dynamically
request additional pages from the firmware as required, this scaling of
requested memory based on available memory will not make a lot of sense
anymore.

Remove this logic as a preparatory patch such that we'll instead defer
to the runtime memory allocator. Note that ideally, we'd want to change
this after dynamic requesting of pages has been implemented for the EFI
platform. But because we'll need to split up initialization of the
memory subsystem and the request of pages from the firmware, we'd have
to duplicate quite some logic at first only to remove it afterwards
again. This seems quite pointless, so we instead have patches slightly
out of order.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 grub-core/kern/efi/mm.c | 35 +++--------------------------------
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index d8e4114541a4..0bccd24f304f 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -38,9 +38,8 @@
    a multiplier of 4KB.  */
 #define MEMORY_MAP_SIZE	0x3000
 
-/* The minimum and maximum heap size for GRUB itself.  */
-#define MIN_HEAP_SIZE	0x100000
-#define MAX_HEAP_SIZE	(1600 * 0x100000)
+/* The default heap size for GRUB itself in bytes.  */
+#define DEFAULT_HEAP_SIZE	0x100000
 
 static void *finish_mmap_buf = 0;
 static grub_efi_uintn_t finish_mmap_size = 0;
@@ -478,23 +477,6 @@ filter_memory_map (grub_efi_memory_descriptor_t *memory_map,
   return filtered_desc;
 }
 
-/* Return the total number of pages.  */
-static grub_efi_uint64_t
-get_total_pages (grub_efi_memory_descriptor_t *memory_map,
-		 grub_efi_uintn_t desc_size,
-		 grub_efi_memory_descriptor_t *memory_map_end)
-{
-  grub_efi_memory_descriptor_t *desc;
-  grub_efi_uint64_t total = 0;
-
-  for (desc = memory_map;
-       desc < memory_map_end;
-       desc = NEXT_MEMORY_DESCRIPTOR (desc, desc_size))
-    total += desc->num_pages;
-
-  return total;
-}
-
 /* Add memory regions.  */
 static void
 add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
@@ -583,8 +565,6 @@ grub_efi_mm_init (void)
   grub_efi_memory_descriptor_t *filtered_memory_map_end;
   grub_efi_uintn_t map_size;
   grub_efi_uintn_t desc_size;
-  grub_efi_uint64_t total_pages;
-  grub_efi_uint64_t required_pages;
   int mm_status;
 
   /* Prepare a memory region to store two memory maps.  */
@@ -624,22 +604,13 @@ grub_efi_mm_init (void)
   filtered_memory_map_end = filter_memory_map (memory_map, filtered_memory_map,
 					       desc_size, memory_map_end);
 
-  /* 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);
-
   /* Sort the filtered descriptors, so that GRUB can allocate pages
      from smaller regions.  */
   sort_memory_map (filtered_memory_map, desc_size, filtered_memory_map_end);
 
   /* Allocate memory regions for GRUB's memory management.  */
   add_memory_regions (filtered_memory_map, desc_size,
-		      filtered_memory_map_end, required_pages);
+		      filtered_memory_map_end, BYTES_TO_PAGES (DEFAULT_HEAP_SIZE));
 
 #if 0
   /* For debug.  */
-- 
2.32.0

[PATCH v2 08/15] efi: mm: Extract function to add memory regions

[Daniel Axtens]

From: Patrick Steinhardt <ps@pks.im>

In preparation of support for runtime-allocating additional memory
region, this patch extracts the function to retrieve the EFI memory map
and add a subset of it to GRUB's own memory regions.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 grub-core/kern/efi/mm.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 0bccd24f304f..c8d3968f7f68 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -556,8 +556,8 @@ print_memory_map (grub_efi_memory_descriptor_t *memory_map,
 }
 #endif
 
-void
-grub_efi_mm_init (void)
+static grub_err_t
+grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
 {
   grub_efi_memory_descriptor_t *memory_map;
   grub_efi_memory_descriptor_t *memory_map_end;
@@ -570,7 +570,7 @@ grub_efi_mm_init (void)
   /* Prepare a memory region to store two memory maps.  */
   memory_map = grub_efi_allocate_any_pages (2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
   if (! memory_map)
-    grub_fatal ("cannot allocate memory");
+    return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory for memory map");
 
   /* Obtain descriptors for available memory.  */
   map_size = MEMORY_MAP_SIZE;
@@ -588,14 +588,14 @@ grub_efi_mm_init (void)
 
       memory_map = grub_efi_allocate_any_pages (2 * BYTES_TO_PAGES (map_size));
       if (! memory_map)
-	grub_fatal ("cannot allocate memory");
+	return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory for new memory map");
 
       mm_status = grub_efi_get_memory_map (&map_size, memory_map, 0,
 					   &desc_size, 0);
     }
 
   if (mm_status < 0)
-    grub_fatal ("cannot get memory map");
+    return grub_error (GRUB_ERR_OUT_OF_MEMORY, "error fetching memory map from EFI");
 
   memory_map_end = NEXT_MEMORY_DESCRIPTOR (memory_map, map_size);
 
@@ -610,7 +610,7 @@ grub_efi_mm_init (void)
 
   /* Allocate memory regions for GRUB's memory management.  */
   add_memory_regions (filtered_memory_map, desc_size,
-		      filtered_memory_map_end, BYTES_TO_PAGES (DEFAULT_HEAP_SIZE));
+		      filtered_memory_map_end, BYTES_TO_PAGES (required_bytes));
 
 #if 0
   /* For debug.  */
@@ -628,6 +628,15 @@ grub_efi_mm_init (void)
   /* Release the memory maps.  */
   grub_efi_free_pages ((grub_addr_t) memory_map,
 		       2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
+
+  return GRUB_ERR_NONE;
+}
+
+void
+grub_efi_mm_init (void)
+{
+  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE) != GRUB_ERR_NONE)
+    grub_fatal ("%s", grub_errmsg);
 }
 
 #if defined (__aarch64__) || defined (__arm__) || defined (__riscv)
-- 
2.32.0

Re: [PATCH v2 08/15] efi: mm: Extract function to add memory regions

[Daniel Kiper]

On Mon, Mar 28, 2022 at 05:22:33PM +1100, Daniel Axtens wrote:
> From: Patrick Steinhardt <ps@pks.im>
>
> In preparation of support for runtime-allocating additional memory
> region, this patch extracts the function to retrieve the EFI memory map
> and add a subset of it to GRUB's own memory regions.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

[PATCH v2 09/15] efi: mm: Pass up errors from `add_memory_regions ()`

[Daniel Axtens]

From: Patrick Steinhardt <ps@pks.im>

The function `add_memory_regions ()` is currently only called on system
initialization to allocate a fixed amount of pages. As such, it didn't
need to return any errors: in case it failed, we cannot proceed anyway.
This will change with the upcoming support for requesting more memory
from the firmware at runtime, where it doesn't make sense anymore to
fail hard.

Refactor the function to return an error to prepare for this. Note that
this does not change the behaviour when initializing the memory system
because `grub_efi_mm_init ()` knows to call `grub_fatal ()` in case
`grub_efi_mm_add_regions ()` returns an error.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
[dja: clarify error messages]
Signed-off-by: Daniel Axtens <dja@axtens.net>

---

v2: Thank you Glenn and Daniel K for the feedback.

This and all EFI changes were not tested on real hardware.
---
 grub-core/kern/efi/mm.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index c8d3968f7f68..4e594659a1ff 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -478,7 +478,7 @@ filter_memory_map (grub_efi_memory_descriptor_t *memory_map,
 }
 
 /* Add memory regions.  */
-static void
+static grub_err_t
 add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
 		    grub_efi_uintn_t desc_size,
 		    grub_efi_memory_descriptor_t *memory_map_end,
@@ -506,9 +506,9 @@ add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
 					   GRUB_EFI_ALLOCATE_ADDRESS,
 					   GRUB_EFI_LOADER_CODE);
       if (! addr)
-	grub_fatal ("cannot allocate conventional memory %p with %u pages",
-		    (void *) ((grub_addr_t) start),
-		    (unsigned) pages);
+	return grub_error (GRUB_ERR_OUT_OF_MEMORY,
+			    "Memory starting at %p (%u pages) marked as free, but EFI would not allocate",
+			    (void *) ((grub_addr_t) start), (unsigned) pages);
 
       grub_mm_init_region (addr, PAGES_TO_BYTES (pages));
 
@@ -518,7 +518,11 @@ add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
     }
 
   if (required_pages > 0)
-    grub_fatal ("too little memory");
+    return grub_error (GRUB_ERR_OUT_OF_MEMORY,
+                       "could not allocate all requested memory: %" PRIuGRUB_UINT64_T " pages still required after iterating EFI memory map",
+                       required_pages);
+
+  return GRUB_ERR_NONE;
 }
 
 void
@@ -565,6 +569,7 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
   grub_efi_memory_descriptor_t *filtered_memory_map_end;
   grub_efi_uintn_t map_size;
   grub_efi_uintn_t desc_size;
+  grub_err_t err;
   int mm_status;
 
   /* Prepare a memory region to store two memory maps.  */
@@ -609,8 +614,11 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
   sort_memory_map (filtered_memory_map, desc_size, filtered_memory_map_end);
 
   /* Allocate memory regions for GRUB's memory management.  */
-  add_memory_regions (filtered_memory_map, desc_size,
-		      filtered_memory_map_end, BYTES_TO_PAGES (required_bytes));
+  err = add_memory_regions (filtered_memory_map, desc_size,
+			    filtered_memory_map_end,
+			    BYTES_TO_PAGES (required_bytes));
+  if (err != GRUB_ERR_NONE)
+    return err;
 
 #if 0
   /* For debug.  */
-- 
2.32.0

Re: [PATCH v2 09/15] efi: mm: Pass up errors from `add_memory_regions ()`

[Daniel Kiper]

On Mon, Mar 28, 2022 at 05:22:34PM +1100, Daniel Axtens wrote:
> From: Patrick Steinhardt <ps@pks.im>
>
> The function `add_memory_regions ()` is currently only called on system
> initialization to allocate a fixed amount of pages. As such, it didn't
> need to return any errors: in case it failed, we cannot proceed anyway.
> This will change with the upcoming support for requesting more memory
> from the firmware at runtime, where it doesn't make sense anymore to
> fail hard.
>
> Refactor the function to return an error to prepare for this. Note that
> this does not change the behaviour when initializing the memory system
> because `grub_efi_mm_init ()` knows to call `grub_fatal ()` in case
> `grub_efi_mm_add_regions ()` returns an error.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> [dja: clarify error messages]
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

[PATCH v2 10/15] efi: mm: Implement runtime addition of pages

[Daniel Axtens]

From: Patrick Steinhardt <ps@pks.im>

Adjust the interface of `grub_efi_mm_add_regions ()` to take a set of
`GRUB_MM_ADD_REGION_*` flags, which most notably is currently only the
`CONSECUTVE` flag. This allows us to set the function up as callback for
the memory subsystem and have it call out to us in case there's not
enough pages available in the current heap.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 grub-core/kern/efi/mm.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 4e594659a1ff..b5160bdffc67 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -482,7 +482,8 @@ static grub_err_t
 add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
 		    grub_efi_uintn_t desc_size,
 		    grub_efi_memory_descriptor_t *memory_map_end,
-		    grub_efi_uint64_t required_pages)
+		    grub_efi_uint64_t required_pages,
+		    unsigned int flags)
 {
   grub_efi_memory_descriptor_t *desc;
 
@@ -496,6 +497,10 @@ add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
 
       start = desc->physical_start;
       pages = desc->num_pages;
+
+      if (pages < required_pages && (flags & GRUB_MM_ADD_REGION_CONSECUTIVE))
+	continue;
+
       if (pages > required_pages)
 	{
 	  start += PAGES_TO_BYTES (pages - required_pages);
@@ -561,7 +566,7 @@ print_memory_map (grub_efi_memory_descriptor_t *memory_map,
 #endif
 
 static grub_err_t
-grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
+grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes, unsigned int flags)
 {
   grub_efi_memory_descriptor_t *memory_map;
   grub_efi_memory_descriptor_t *memory_map_end;
@@ -616,7 +621,8 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
   /* Allocate memory regions for GRUB's memory management.  */
   err = add_memory_regions (filtered_memory_map, desc_size,
 			    filtered_memory_map_end,
-			    BYTES_TO_PAGES (required_bytes));
+			    BYTES_TO_PAGES (required_bytes),
+			    flags);
   if (err != GRUB_ERR_NONE)
     return err;
 
@@ -643,8 +649,9 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
 void
 grub_efi_mm_init (void)
 {
-  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE) != GRUB_ERR_NONE)
+  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE, GRUB_MM_ADD_REGION_NONE) != GRUB_ERR_NONE)
     grub_fatal ("%s", grub_errmsg);
+  grub_mm_add_region_fn = grub_efi_mm_add_regions;
 }
 
 #if defined (__aarch64__) || defined (__arm__) || defined (__riscv)
-- 
2.32.0

[PATCH v2 11/15] ieee1275: request memory with ibm, client-architecture-support

[Daniel Axtens]

On PowerVM, the first time we boot a Linux partition, we may only get
256MB of real memory area, even if the partition has more memory.

This isn't really enough. Fortunately, the Power Architecture Platform
Reference (PAPR) defines a method we can call to ask for more memory.
This is part of the broad and powerful ibm,client-architecture-support
(CAS) method.

CAS can do an enormous amount of things on a PAPR platform: as well as
asking for memory, you can set the supported processor level, the interrupt
controller, hash vs radix mmu, and so on. We want to touch as little of
this as possible because we don't want to step on the toes of the future OS.

If:

 - we are running under what we think is PowerVM (compatible property of /
   begins with "IBM"), and

 - the full amount of RMA is less than 512MB (as determined by the reg
   property of /memory)

then call CAS as follows: (refer to the Linux on Power Architecture
Reference, LoPAR, which is public, at B.5.2.3):

 - Use the "any" PVR value and supply 2 option vectors.

 - Set option vector 1 (PowerPC Server Processor Architecture Level)
   to "ignore".

 - Set option vector 2 with default or Linux-like options, including a
   min-rma-size of 512MB.

This will cause a CAS reboot and the partition will restart with 512MB
of RMA. Grub will notice the 512MB and not call CAS again.

(A partition can be configured with only 256MB of memory, which would
mean this request couldn't be satisfied, but PFW refuses to load with
only 256MB of memory, so it's a bit moot. SLOF will run fine with 256MB,
but we will never call CAS under qemu/SLOF because /compatible won't
begin with "IBM".)

One of the first things Linux does while still running under OpenFirmware
is to call CAS with a much fuller set of options (including asking for
512MB of memory). This includes a much more restrictive set of PVR values
and processor support levels, and this will induce another reboot. On this
reboot grub will again notice the higher RMA, and not call CAS. We will get
to Linux, Linux will call CAS but because the values are now set for Linux
this will not induce another CAS reboot and we will finally boot.

On all subsequent boots, everything will be configured with 512MB of RMA
and all the settings Linux likes, so there will be no further CAS reboots.

(phyp is super sticky with the RMA size - it persists even on cold boots.
So if you've ever booted Linux in a partition, you'll probably never have
grub call CAS. It'll only ever fire the first time a partition loads grub,
or if you deliberately lower the amount of memory your partition has below
512MB.)

Signed-off-by: Daniel Axtens <dja@axtens.net>

---

v2: reformat
---
 grub-core/kern/ieee1275/cmain.c  |   3 +
 grub-core/kern/ieee1275/init.c   | 144 +++++++++++++++++++++++++++++++
 include/grub/ieee1275/ieee1275.h |   8 ++
 3 files changed, 155 insertions(+)

diff --git a/grub-core/kern/ieee1275/cmain.c b/grub-core/kern/ieee1275/cmain.c
index 4442b6a83193..b707798ec3fb 100644
--- a/grub-core/kern/ieee1275/cmain.c
+++ b/grub-core/kern/ieee1275/cmain.c
@@ -123,6 +123,9 @@ grub_ieee1275_find_options (void)
 	      break;
 	    }
 	}
+
+      if (grub_strncmp (tmp, "IBM,", 4) == 0)
+	grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
     }
 
   if (is_smartfirmware)
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index 2adf4fdfc0e7..d44efd99cfca 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -200,11 +200,155 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
   return 0;
 }
 
+/*
+ * How much memory does OF believe it has? (regardless of whether
+ * it's accessible or not)
+ */
+static grub_err_t
+grub_ieee1275_total_mem (grub_uint64_t *total)
+{
+  grub_ieee1275_phandle_t root;
+  grub_ieee1275_phandle_t memory;
+  grub_uint32_t reg[4];
+  grub_ssize_t reg_size;
+  grub_uint32_t address_cells = 1;
+  grub_uint32_t size_cells = 1;
+  grub_uint64_t size;
+
+  /* If we fail to get to the end, report 0. */
+  *total = 0;
+
+  /* Determine the format of each entry in `reg'.  */
+  grub_ieee1275_finddevice ("/", &root);
+  grub_ieee1275_get_integer_property (root, "#address-cells", &address_cells,
+				      sizeof address_cells, 0);
+  grub_ieee1275_get_integer_property (root, "#size-cells", &size_cells,
+				      sizeof size_cells, 0);
+
+  if (size_cells > address_cells)
+    address_cells = size_cells;
+
+  /* Load `/memory/reg'.  */
+  if (grub_ieee1275_finddevice ("/memory", &memory))
+    return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+		       "couldn't find /memory node");
+  if (grub_ieee1275_get_integer_property (memory, "reg", reg,
+					  sizeof reg, &reg_size))
+    return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+		       "couldn't examine /memory/reg property");
+  if (reg_size < 0 || (grub_size_t) reg_size > sizeof (reg))
+    return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+                       "/memory response buffer exceeded");
+
+  if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_BROKEN_ADDRESS_CELLS))
+    {
+      address_cells = 1;
+      size_cells = 1;
+    }
+
+  /* Decode only the size */
+  size = reg[address_cells];
+  if (size_cells == 2)
+    size = (size << 32) | reg[address_cells + 1];
+
+  *total = size;
+
+  return grub_errno;
+}
+
+/* See PAPR or arch/powerpc/kernel/prom_init.c */
+struct option_vector2 {
+	grub_uint8_t byte1;
+	grub_uint16_t reserved;
+	grub_uint32_t real_base;
+	grub_uint32_t real_size;
+	grub_uint32_t virt_base;
+	grub_uint32_t virt_size;
+	grub_uint32_t load_base;
+	grub_uint32_t min_rma;
+	grub_uint32_t min_load;
+	grub_uint8_t min_rma_percent;
+	grub_uint8_t max_pft_size;
+} __attribute__((packed));
+
+struct pvr_entry {
+	  grub_uint32_t mask;
+	  grub_uint32_t entry;
+};
+
+struct cas_vector {
+    struct {
+      struct pvr_entry terminal;
+    } pvr_list;
+    grub_uint8_t num_vecs;
+    grub_uint8_t vec1_size;
+    grub_uint8_t vec1;
+    grub_uint8_t vec2_size;
+    struct option_vector2 vec2;
+} __attribute__((packed));
+
+/*
+ * Call ibm,client-architecture-support to try to get more RMA.
+ * We ask for 512MB which should be enough to verify a distro kernel.
+ * We ignore most errors: if we don't succeed we'll proceed with whatever
+ * memory we have.
+ */
+static void
+grub_ieee1275_ibm_cas (void)
+{
+  int rc;
+  grub_ieee1275_ihandle_t root;
+  struct cas_args {
+    struct grub_ieee1275_common_hdr common;
+    grub_ieee1275_cell_t method;
+    grub_ieee1275_ihandle_t ihandle;
+    grub_ieee1275_cell_t cas_addr;
+    grub_ieee1275_cell_t result;
+  } args;
+  struct cas_vector vector = {
+    .pvr_list = { { 0x00000000, 0xffffffff } }, /* any processor */
+    .num_vecs = 2 - 1,
+    .vec1_size = 0,
+    .vec1 = 0x80, /* ignore */
+    .vec2_size = 1 + sizeof(struct option_vector2) - 2,
+    .vec2 = {
+      0, 0, -1, -1, -1, -1, -1, 512, -1, 0, 48
+    },
+  };
+
+  INIT_IEEE1275_COMMON (&args.common, "call-method", 3, 2);
+  args.method = (grub_ieee1275_cell_t)"ibm,client-architecture-support";
+  rc = grub_ieee1275_open("/", &root);
+  if (rc) {
+	  grub_error (GRUB_ERR_IO, "could not open root when trying to call CAS");
+	  return;
+  }
+  args.ihandle = root;
+  args.cas_addr = (grub_ieee1275_cell_t)&vector;
+
+  grub_printf("Calling ibm,client-architecture-support...");
+  IEEE1275_CALL_ENTRY_FN (&args);
+  grub_printf("done\n");
+
+  grub_ieee1275_close(root);
+}
+
 static void
 grub_claim_heap (void)
 {
   unsigned long total = 0;
 
+  if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY))
+    {
+      grub_uint64_t rma_size;
+      grub_err_t err;
+
+      err = grub_ieee1275_total_mem (&rma_size);
+      /* if we have an error, don't call CAS, just hope for the best */
+      if (!err && rma_size < (512 * 1024 * 1024))
+	grub_ieee1275_ibm_cas();
+    }
+
   grub_machine_mmap_iterate (heap_init, &total);
 }
 #endif
diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
index f53228703bdb..b5c916d1de58 100644
--- a/include/grub/ieee1275/ieee1275.h
+++ b/include/grub/ieee1275/ieee1275.h
@@ -128,6 +128,14 @@ enum grub_ieee1275_flag
   GRUB_IEEE1275_FLAG_CURSORONOFF_ANSI_BROKEN,
 
   GRUB_IEEE1275_FLAG_RAW_DEVNAMES,
+
+  /*
+   * On PFW, the first time we boot a Linux partition, we may only get 256MB of
+   * real memory area, even if the partition has more memory. Set this flag if
+   * we think we're running under PFW. Then, if this flag is set, and the RMA is
+   * only 256MB in size, try asking for more with CAS.
+   */
+  GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY,
 };
 
 extern int EXPORT_FUNC(grub_ieee1275_test_flag) (enum grub_ieee1275_flag flag);
-- 
2.32.0

[PATCH v2 12/15] ieee1275: drop len -= 1 quirk in heap_init

[Daniel Axtens]

This was apparently 'required by some firmware': commit dc9468500919
("2007-02-12  Hollis Blanchard  <hollis@penguinppc.org>").

It's not clear what firmware that was, and what platform from 14 years ago
which exhibited the bug then is still both in use and buggy now.

It doesn't cause issues on qemu (mac99 or pseries) or under PFW for Power8.

I don't have access to old Mac hardware, but if anyone feels especially
strongly we can put it under some feature flag. I really want to disable
it under pseries because it will mess with region merging.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 grub-core/kern/ieee1275/init.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index d44efd99cfca..eeb11dd40ded 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -166,7 +166,6 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
 	  addr = 0x180000;
 	}
     }
-  len -= 1; /* Required for some firmware.  */
 
   /* Never exceed HEAP_MAX_SIZE  */
   if (*total + len > HEAP_MAX_SIZE)
-- 
2.32.0

[PATCH v2 13/15] ieee1275: support runtime memory claiming

[Daniel Axtens]

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>

---

v2: reformat, rework and explain better, add debug prints with
    grub_dprintf
---
 docs/grub-dev.texi             |   7 +-
 grub-core/kern/ieee1275/init.c | 267 ++++++++++++++++++++++++++++++---
 2 files changed, 254 insertions(+), 20 deletions(-)

diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
index 8a20a9af2f4b..6f3f5fb1fb14 100644
--- a/docs/grub-dev.texi
+++ b/docs/grub-dev.texi
@@ -1059,7 +1059,10 @@ 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 and powerpc-ieee1275, GRUB will allocate 32MiB for its heap on
+startup. It may allocate more at runtime, as long as at least 128MiB remain free
+in OpenFirmware.
 
 On sparc64-ieee1275 stack is 256KiB and heap is 2MiB.
 
@@ -1087,7 +1090,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 available memory - 128MiB
 @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 eeb11dd40ded..7805f6c6fcc5 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -45,13 +45,23 @@
 #include <grub/machine/kernel.h>
 #endif
 
-/* The maximum heap size we're going to claim */
+/* The maximum heap size we're going to claim at boot. Not used by sparc. */
 #ifdef __i386__
 #define HEAP_MAX_SIZE		(unsigned long) (64 * 1024 * 1024)
-#else
+#else // __powerpc__
 #define HEAP_MAX_SIZE		(unsigned long) (32 * 1024 * 1024)
 #endif
 
+/*
+ * The amount of OF space we will not claim here so as to leave space for
+ * the loader and linux to service early allocations.
+ *
+ * In 2021, Daniel Axtens claims that we should leave at least 128MB to
+ * ensure we can load a stock kernel and initrd on a pseries guest with
+ * a 512MB real memory area under PowerVM.
+ */
+#define RUNTIME_MIN_SPACE (128UL * 1024 * 1024)
+
 extern char _start[];
 extern char _end[];
 
@@ -145,16 +155,52 @@ grub_claim_heap (void)
 				 + GRUB_KERNEL_MACHINE_STACK_SIZE), 0x200000);
 }
 #else
-/* Helper for grub_claim_heap.  */
+/* Helpers for mm on powerpc. */
+
+/*
+ * How much memory does OF believe exists in total?
+ *
+ * This isn't necessarily the true total. It can be the total memory
+ * accessible in real mode for a pseries guest, for example.
+ */
+static grub_uint64_t rmo_top;
+
 static int
-heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
-	   void *data)
+count_free (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
+	    void *data)
+{
+  if (type != GRUB_MEMORY_AVAILABLE)
+    return 0;
+
+  /* Do not consider memory beyond 4GB */
+  if (addr > 0xffffffffULL)
+    return 0;
+
+  if (addr + len > 0xffffffffULL)
+    len = 0xffffffffULL - addr;
+
+  *(grub_uint32_t *)data += len;
+
+  return 0;
+}
+
+static int
+regions_claim (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
+	      unsigned int flags, void *data)
 {
-  unsigned long *total = data;
+  grub_uint32_t total = *(grub_uint32_t *)data;
+  grub_uint64_t linux_rmo_save;
 
   if (type != GRUB_MEMORY_AVAILABLE)
     return 0;
 
+  /* Do not consider memory beyond 4GB */
+  if (addr > 0xffffffffULL)
+    return 0;
+
+  if (addr + len > 0xffffffffULL)
+    len = 0xffffffffULL - addr;
+
   if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_NO_PRE1_5M_CLAIM))
     {
       if (addr + len <= 0x180000)
@@ -167,10 +213,6 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
 	}
     }
 
-  /* 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
@@ -182,6 +224,109 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
       len = 0;
     }
 
+  /*
+   * Linux likes to claim memory at min(RMO top, 768MB) and works down
+   * without reference to /memory/available. (See prom_init.c::alloc_down)
+   *
+   * If this block contains min(RMO top, 768MB), do not claim below that for
+   * at least a few MB (this is where RTAS, SML and potentially TCEs live).
+   *
+   * We also need to leave enough space for the DT in the RMA. (See
+   * prom_init.c::alloc_up)
+   *
+   * Finally, we also want to make sure that when grub loads the kernel,
+   * it isn't going to use up all the memory we're trying to reserve! So
+   * enforce our entire RUNTIME_MIN_SPACE here:
+   *
+   * |---------- Top of memory ----------|
+   * |                                   |
+   * |             available             |
+   * |                                   |
+   * |----------     768 MB    ----------|
+   * |                                   |
+   * |              reserved             |
+   * |                                   |
+   * |--- 768 MB - runtime min space  ---|
+   * |                                   |
+   * |             available             |
+   * |                                   |
+   * |----------      0 MB     ----------|
+   *
+   * Edge cases:
+   *
+   * - Total memory less than RUNTIME_MIN_SPACE: only claim up to HEAP_MAX_SIZE.
+   *   (enforced elsewhere)
+   *
+   * - Total memory between RUNTIME_MIN_SPACE and 768MB:
+   *
+   * |---------- Top of memory ----------|
+   * |                                   |
+   * |              reserved             |
+   * |                                   |
+   * |----  top - runtime min space  ----|
+   * |                                   |
+   * |             available             |
+   * |                                   |
+   * |----------      0 MB     ----------|
+   *
+   * This by itself would not leave us with RUNTIME_MIN_SPACE of free bytes: if
+   * rmo_top < 768MB, we will almost certainly have FW claims in the reserved
+   * region. We try to address that elsewhere: grub_ieee1275_mm_add_region will
+   * not call us if the resulting free space would be less than RUNTIME_MIN_SPACE.
+   */
+  linux_rmo_save = grub_min (0x30000000, rmo_top) - RUNTIME_MIN_SPACE;
+  if (rmo_top > RUNTIME_MIN_SPACE)
+    {
+      if (rmo_top <= 0x30000000)
+        {
+          if (addr > linux_rmo_save)
+            {
+              grub_dprintf("ieee1275", "rejecting region in RUNTIME_MIN_SPACE reservation (%llx)\n",
+                           addr);
+              return 0;
+            }
+          else if (addr + len > linux_rmo_save)
+            {
+              grub_dprintf("ieee1275", "capping region: (%llx -> %llx) -> (%llx -> %llx)\n",
+                           addr, addr + len, addr, rmo_top - RUNTIME_MIN_SPACE);
+              len = linux_rmo_save - addr;
+            }
+        }
+      else
+        {
+          /*
+           * we order these cases to prefer higher addresses and avoid some
+           * splitting issues
+           */
+          if (addr < 0x30000000UL && (addr + len) > 0x30000000UL)
+            {
+              grub_dprintf ("ieee1275",
+                            "adjusting region for RUNTIME_MIN_SPACE: (%llx -> %llx) -> (%llx -> %llx)\n",
+                            addr, addr + len, 0x30000000ULL,
+                           addr + len);
+              len = (addr + len) - 0x30000000ULL;
+              addr = 0x30000000;
+            }
+          else if ((addr < linux_rmo_save) && ((addr + len) > linux_rmo_save))
+            {
+              grub_dprintf("ieee1275", "capping region: (%llx -> %llx) -> (%llx -> %llx)\n",
+                           addr, addr + len, addr, linux_rmo_save);
+              len = linux_rmo_save - addr;
+            }
+          else if (addr >= linux_rmo_save && (addr + len) <= 0x30000000UL)
+            {
+              grub_dprintf("ieee1275", "rejecting region in RUNTIME_MIN_SPACE reservation (%llx)\n",
+                           addr);
+              return 0;
+            }
+        }
+    }
+  if (flags & GRUB_MM_ADD_REGION_CONSECUTIVE && len < total)
+    return 0;
+
+  if (len > total)
+    len = total;
+
   if (len)
     {
       grub_err_t err;
@@ -190,15 +335,94 @@ 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;
 }
 
+static int
+heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
+	   void *data)
+{
+  return regions_claim(addr, len, type, GRUB_MM_ADD_REGION_NONE, data);
+}
+
+static int
+region_claim (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
+	   void *data)
+{
+  return regions_claim(addr, len, type, GRUB_MM_ADD_REGION_CONSECUTIVE, data);
+}
+
+
+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);
+      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 */
+      if (total != 0)
+        {
+          total = size + 48;
+          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;
+      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;
+}
+
 /*
  * How much memory does OF believe it has? (regardless of whether
  * it's accessible or not)
@@ -335,16 +559,23 @@ grub_ieee1275_ibm_cas (void)
 static void
 grub_claim_heap (void)
 {
-  unsigned long total = 0;
+  grub_err_t err;
+  grub_uint32_t total = HEAP_MAX_SIZE;
+
+  err = grub_ieee1275_total_mem (&rmo_top);
+
+  /*
+   * If we cannot size the available memory, we can't be sure we're leaving
+   * space for the kernel, initrd and things Linux loads early in boot. So only
+   * allow further allocations from firmware on success
+   */
+  if (err == GRUB_ERR_NONE)
+    grub_mm_add_region_fn = grub_ieee1275_mm_add_region;
 
   if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY))
     {
-      grub_uint64_t rma_size;
-      grub_err_t err;
-
-      err = grub_ieee1275_total_mem (&rma_size);
       /* if we have an error, don't call CAS, just hope for the best */
-      if (!err && rma_size < (512 * 1024 * 1024))
+      if (!err && rmo_top < (512 * 1024 * 1024))
 	grub_ieee1275_ibm_cas();
     }
 
-- 
2.32.0

[PATCH v2 14/15] [RFC] Add memtool module with memory allocation stress-test

[Daniel Axtens]

When working on memory, it's nice to be able to test your work.

Add a memtest module. When compiled with --enable-mm-debug, it exposes
3 commands:

 * lsmem - print all allocations and free space in all regions
 * lsfreemem - print free space in all regions

 * stress_big_allocs - stress test large allocations:
  - how much memory can we allocate in one chunk?
  - how many 1MB chunks can we allocate?
  - check that gap-filling works with a 1MB aligned 900kB alloc + a
     100kB alloc.

Signed-off-by: Daniel Axtens <dja@axtens.net>

---

I haven't addressed most of the change requests yet. This will need lots
of work to get to a mergable state, especially the ability to check
MM_DEBUG in template files so that it can only be built if
--enable-mm-debug is set, ('enable = mm_debug' or something) and that
involves a lot of slow and painful build system hacking. Perhaps later
on. But I have sorted out the copyright assignment so at least that
is clear if anyone else wants to hack on it.
---
 grub-core/Makefile.core.def   |   5 ++
 grub-core/commands/memtools.c | 155 ++++++++++++++++++++++++++++++++++
 grub-core/kern/mm.c           |   4 +
 3 files changed, 164 insertions(+)
 create mode 100644 grub-core/commands/memtools.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index cada67b667ff..ce7d79d38e5b 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2530,3 +2530,8 @@ module = {
   common = commands/i386/wrmsr.c;
   enable = x86;
 };
+
+module = {
+  name = memtools;
+  common = commands/memtools.c;
+};
diff --git a/grub-core/commands/memtools.c b/grub-core/commands/memtools.c
new file mode 100644
index 000000000000..bb4ad359e013
--- /dev/null
+++ b/grub-core/commands/memtools.c
@@ -0,0 +1,155 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2021 Free Software Foundation, Inc.
+ *  Copyright (C) 2021 IBM Corporation
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+#include <grub/dl.h>
+#include <grub/misc.h>
+#include <grub/command.h>
+#include <grub/i18n.h>
+#include <grub/memory.h>
+#include <grub/mm.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#ifdef MM_DEBUG
+
+static grub_err_t
+grub_cmd_lsmem (grub_command_t cmd __attribute__ ((unused)),
+		 int argc __attribute__ ((unused)),
+		 char **args __attribute__ ((unused)))
+
+{
+#ifndef GRUB_MACHINE_EMU
+  grub_mm_dump(0);
+#endif
+
+  return 0;
+}
+
+static grub_err_t
+grub_cmd_lsfreemem (grub_command_t cmd __attribute__ ((unused)),
+		    int argc __attribute__ ((unused)),
+		    char **args __attribute__ ((unused)))
+
+{
+#ifndef GRUB_MACHINE_EMU
+  grub_mm_dump_free();
+#endif
+
+  return 0;
+}
+
+
+static grub_err_t
+grub_cmd_stress_big_allocs (grub_command_t cmd __attribute__ ((unused)),
+			    int argc __attribute__ ((unused)),
+			    char **args __attribute__ ((unused)))
+{
+  int i, max_mb, blocks_alloced;
+  void *mem;
+  void **blocklist;
+
+  grub_printf ("Test 1: increasingly sized allocs to 1GB block\n");
+  for (i = 1; i < 1024; i++) {
+    grub_printf ("%d MB . ", i);
+    mem = grub_malloc (i * 1024 * 1024);
+    if (mem == NULL)
+      {
+	grub_printf ("failed\n");
+	break;
+      }
+    else
+      grub_free (mem);
+
+    if (i % 10 == 0)
+      grub_printf ("\n");
+  }
+
+  max_mb = i - 1;
+  grub_printf ("Max sized allocation we did was %d MB\n", max_mb);
+  
+  grub_printf ("Test 2: 1MB at a time, max 4GB\n");
+  blocklist = grub_calloc (4096, sizeof (void *));
+  for (i = 0; i < 4096; i++)
+    {
+      blocklist[i] = grub_malloc (1024 * 1024);
+      if (!blocklist[i])
+	{
+	  grub_printf ("Ran out of memory at iteration %d\n", i);
+	  break;
+	}
+    }
+  blocks_alloced = i;
+  for (i = 0; i < blocks_alloced; i++)
+    grub_free (blocklist[i]);
+
+  grub_printf ("\nTest 3: 1MB aligned 900kB + 100kB\n");
+  //grub_mm_debug=1;
+  for (i = 0; i < 4096; i += 2)
+    {
+      blocklist[i] = grub_memalign (1024 * 1024, 900 * 1024);
+      if (!blocklist[i])
+	{
+	  grub_printf ("Failed big allocation, iteration %d\n", i);
+	  blocks_alloced = i;
+	  break;
+	}
+
+      blocklist[i + 1] = grub_malloc (100 * 1024);
+      if (!blocklist[i + 1])
+	{
+	  grub_printf ("Failed small allocation, iteration %d\n", i);
+	  blocks_alloced = i + 1;
+	  break;
+	}
+      grub_printf (".");
+    }
+  for (i = 0; i < blocks_alloced; i++)
+    grub_free (blocklist[i]);
+
+  grub_free (blocklist);
+
+  grub_errno = GRUB_ERR_NONE;
+  return GRUB_ERR_NONE;
+}
+
+static grub_command_t cmd_lsmem, cmd_lsfreemem, cmd_sba;
+
+#endif /* MM_DEBUG */
+
+GRUB_MOD_INIT(memtools)
+{
+#ifdef MM_DEBUG
+  cmd_lsmem = grub_register_command ("lsmem", grub_cmd_lsmem,
+				     0, N_("List free and allocated memory blocks."));
+  cmd_lsfreemem = grub_register_command ("lsfreemem", grub_cmd_lsfreemem,
+					 0, N_("List free memory blocks."));
+  cmd_sba = grub_register_command ("stress_big_allocs", grub_cmd_stress_big_allocs,
+  				   0, N_("Stress test large allocations."));
+#endif
+}
+
+GRUB_MOD_FINI(memtools)
+{
+#ifdef MM_DEBUG
+  grub_unregister_command (cmd_lsmem);
+  grub_unregister_command (cmd_lsfreemem);
+  grub_unregister_command (cmd_sba);
+#endif
+}
diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index c3bf4646f55e..1e93d0d6cdb4 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -657,6 +657,8 @@ grub_mm_dump_free (void)
     {
       grub_mm_header_t p;
 
+      grub_printf ("Region %p (size %" PRIuGRUB_SIZE ")\n\n", r, r->size);
+
       /* Follow the free list.  */
       p = r->first;
       do
@@ -684,6 +686,8 @@ grub_mm_dump (unsigned lineno)
     {
       grub_mm_header_t p;
 
+      grub_printf ("Region %p (size %" PRIuGRUB_SIZE ")\n\n", r, r->size);
+
       for (p = (grub_mm_header_t) ALIGN_UP ((grub_addr_t) (r + 1),
 					    GRUB_MM_ALIGN);
 	   (grub_addr_t) p < (grub_addr_t) (r+1) + r->size;
-- 
2.32.0

[PATCH v2 15/15] ibmvtpm: Add support for trusted boot using a vTPM 2.0

[Daniel Axtens]

From: Stefan Berger <stefanb@linux.ibm.com>

Add support for trusted boot using a vTPM 2.0 on the IBM IEEE1275
PowerPC platform. With this patch grub now measures text and binary data
into the TPM's PCRs 8 and 9 in the same way as the x86_64 platform
does.

This patch requires Daniel Axtens's patches for claiming more memory.

For vTPM support to work on PowerVM, system driver levels 1010.30
or 1020.00 are required.

Note: Previous versions of firmware levels with the 2hash-ext-log
API call have a bug that, once this API call is invoked, has the
effect of disabling the vTPM driver under Linux causing an error
message to be displayed in the Linux kernel log. Those users will
have to update their machines to the firmware levels mentioned
above.

Cc: Eric Snowberg <eric.snowberg@oracle.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 docs/grub.texi                        |   3 +-
 grub-core/Makefile.core.def           |   7 ++
 grub-core/commands/ieee1275/ibmvtpm.c | 152 ++++++++++++++++++++++++++
 include/grub/ieee1275/ieee1275.h      |   3 +
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c

diff --git a/docs/grub.texi b/docs/grub.texi
index caba8befb4ef..54063430c717 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -6084,7 +6084,8 @@ tpm module is loaded. As such it is recommended that the tpm module be built
 into @file{core.img} in order to avoid a potential gap in measurement between
 @file{core.img} being loaded and the tpm module being loaded.
 
-Measured boot is currently only supported on EFI platforms.
+Measured boot is currently only supported on EFI and IBM IEEE1275 PowerPC
+platforms.
 
 @node Lockdown
 @section Lockdown when booting on a secure setup
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index ce7d79d38e5b..bbbbeaf43773 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1121,6 +1121,13 @@ module = {
   enable = powerpc_ieee1275;
 };
 
+module = {
+  name = tpm;
+  common = commands/tpm.c;
+  ieee1275 = commands/ieee1275/ibmvtpm.c;
+  enable = powerpc_ieee1275;
+};
+
 module = {
   name = terminal;
   common = commands/terminal.c;
diff --git a/grub-core/commands/ieee1275/ibmvtpm.c b/grub-core/commands/ieee1275/ibmvtpm.c
new file mode 100644
index 000000000000..e68b8448bc00
--- /dev/null
+++ b/grub-core/commands/ieee1275/ibmvtpm.c
@@ -0,0 +1,152 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2021  Free Software Foundation, Inc.
+ *  Copyright (C) 2021  IBM Corporation
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *  IBM vTPM support code.
+ */
+
+#include <grub/err.h>
+#include <grub/types.h>
+#include <grub/tpm.h>
+#include <grub/ieee1275/ieee1275.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+
+static grub_ieee1275_ihandle_t tpm_ihandle;
+static grub_uint8_t tpm_version;
+
+#define IEEE1275_IHANDLE_INVALID ((grub_ieee1275_ihandle_t)0)
+
+static void
+tpm_get_tpm_version (void)
+{
+  grub_ieee1275_phandle_t vtpm;
+  char buffer[20];
+
+  if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm) &&
+      !grub_ieee1275_get_property (vtpm, "compatible", buffer,
+				   sizeof (buffer), NULL) &&
+      !grub_strcmp (buffer, "IBM,vtpm20"))
+    tpm_version = 2;
+}
+
+static grub_err_t
+tpm_init (void)
+{
+  static int init_success = 0;
+
+  if (!init_success)
+    {
+      if (grub_ieee1275_open ("/vdevice/vtpm", &tpm_ihandle) < 0) {
+        tpm_ihandle = IEEE1275_IHANDLE_INVALID;
+        return GRUB_ERR_UNKNOWN_DEVICE;
+      }
+
+      init_success = 1;
+
+      tpm_get_tpm_version ();
+    }
+
+  return GRUB_ERR_NONE;
+}
+
+static int
+ibmvtpm_2hash_ext_log (grub_uint8_t pcrindex,
+		       grub_uint32_t eventtype,
+		       const char *description,
+		       grub_size_t description_size,
+		       void *buf, grub_size_t size)
+{
+  struct tpm_2hash_ext_log
+  {
+    struct grub_ieee1275_common_hdr common;
+    grub_ieee1275_cell_t method;
+    grub_ieee1275_cell_t ihandle;
+    grub_ieee1275_cell_t size;
+    grub_ieee1275_cell_t buf;
+    grub_ieee1275_cell_t description_size;
+    grub_ieee1275_cell_t description;
+    grub_ieee1275_cell_t eventtype;
+    grub_ieee1275_cell_t pcrindex;
+    grub_ieee1275_cell_t catch_result;
+    grub_ieee1275_cell_t rc;
+  }
+  args;
+
+  INIT_IEEE1275_COMMON (&args.common, "call-method", 8, 2);
+  args.method = (grub_ieee1275_cell_t) "2hash-ext-log";
+  args.ihandle = tpm_ihandle;
+  args.pcrindex = pcrindex;
+  args.eventtype = eventtype;
+  args.description = (grub_ieee1275_cell_t) description;
+  args.description_size = description_size;
+  args.buf = (grub_ieee1275_cell_t) buf;
+  args.size = (grub_ieee1275_cell_t) size;
+
+  if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
+    return -1;
+
+  /*
+   * catch_result is set if firmware does not support 2hash-ext-log
+   * rc is GRUB_IEEE1275_CELL_FALSE (0) on failure
+   */
+  if ((args.catch_result) || args.rc == GRUB_IEEE1275_CELL_FALSE)
+    return -1;
+
+  return 0;
+}
+
+static grub_err_t
+tpm2_log_event (unsigned char *buf,
+		grub_size_t size, grub_uint8_t pcr,
+		const char *description)
+{
+  static int error_displayed = 0;
+  int err;
+
+  err = ibmvtpm_2hash_ext_log (pcr, EV_IPL,
+			       description,
+			       grub_strlen(description) + 1,
+			       buf, size);
+  if (err && !error_displayed)
+    {
+      error_displayed++;
+      return grub_error (GRUB_ERR_BAD_DEVICE,
+	      "2HASH-EXT-LOG failed: Firmware is likely too old.\n");
+    }
+
+  return GRUB_ERR_NONE;
+}
+
+grub_err_t
+grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
+		  const char *description)
+{
+  grub_err_t err = tpm_init();
+
+  /* Absence of a TPM isn't a failure. */
+  if (err != GRUB_ERR_NONE)
+    return GRUB_ERR_NONE;
+
+  grub_dprintf ("tpm", "log_event, pcr = %d, size = 0x%" PRIxGRUB_SIZE ", %s\n",
+		pcr, size, description);
+
+  if (tpm_version == 2)
+    return tpm2_log_event (buf, size, pcr, description);
+
+  return GRUB_ERR_NONE;
+}
diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
index b5c916d1de58..5002195fce0d 100644
--- a/include/grub/ieee1275/ieee1275.h
+++ b/include/grub/ieee1275/ieee1275.h
@@ -24,6 +24,9 @@
 #include <grub/types.h>
 #include <grub/machine/ieee1275.h>
 
+#define GRUB_IEEE1275_CELL_FALSE       ((grub_ieee1275_cell_t) 0)
+#define GRUB_IEEE1275_CELL_TRUE        ((grub_ieee1275_cell_t) -1)
+
 struct grub_ieee1275_mem_region
 {
   unsigned int start;
-- 
2.32.0

Re: [PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

On Mon, Mar 28, 2022 at 05:22:25PM +1100, Daniel Axtens wrote:
> Hi all,
>
> This is, at long last, an updated version of my series extending Patrick's
> dynamic memory regions to ieee1275.
>
> Noteworthy changes:
>
>  - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
>    ieee1275 patches.
>
>  - reworked the ieee1275 runtime memory claiming to be more resilient and better
>    documented.
>
>  - fixed comment style and hopefully addressed all other change requests.
>
>  - grub will now try asking for contiguous memory and then, if that fails, for
>    discontiguous memory - in case region merging with the discontiguous memory
>    is sufficient to allow the eventual allocation to succeed.

Patrick, all mm and EFI code got my RB. Could you test it with your
Argon changes? If these changes pass your tests I will merge them.

Daniel

Re: [PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

On Thu, Apr 07, 2022 at 04:41:04PM +0200, Daniel Kiper wrote:
> On Mon, Mar 28, 2022 at 05:22:25PM +1100, Daniel Axtens wrote:
> > Hi all,
> >
> > This is, at long last, an updated version of my series extending Patrick's
> > dynamic memory regions to ieee1275.
> >
> > Noteworthy changes:
> >
> >  - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
> >    ieee1275 patches.
> >
> >  - reworked the ieee1275 runtime memory claiming to be more resilient and better
> >    documented.
> >
> >  - fixed comment style and hopefully addressed all other change requests.
> >
> >  - grub will now try asking for contiguous memory and then, if that fails, for
> >    discontiguous memory - in case region merging with the discontiguous memory
> >    is sufficient to allow the eventual allocation to succeed.
>
> Patrick, all mm and EFI code got my RB. Could you test it with your
> Argon changes? If these changes pass your tests I will merge them.

Patrick, ping?

To be more precise, I am thinking about the patches up to #10.

Daniel

Re: [PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Stefan Berger]

On 4/14/22 11:30, Daniel Kiper wrote:
> On Thu, Apr 07, 2022 at 04:41:04PM +0200, Daniel Kiper wrote:
>> On Mon, Mar 28, 2022 at 05:22:25PM +1100, Daniel Axtens wrote:
>>> Hi all,
>>>
>>> This is, at long last, an updated version of my series extending Patrick's
>>> dynamic memory regions to ieee1275.
>>>
>>> Noteworthy changes:
>>>
>>>   - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
>>>     ieee1275 patches.
>>>
>>>   - reworked the ieee1275 runtime memory claiming to be more resilient and better
>>>     documented.
>>>
>>>   - fixed comment style and hopefully addressed all other change requests.
>>>
>>>   - grub will now try asking for contiguous memory and then, if that fails, for
>>>     discontiguous memory - in case region merging with the discontiguous memory
>>>     is sufficient to allow the eventual allocation to succeed.
>>
>> Patrick, all mm and EFI code got my RB. Could you test it with your
>> Argon changes? If these changes pass your tests I will merge them.
> 
> Patrick, ping?
> 
> To be more precise, I am thinking about the patches up to #10.
> 
> Daniel

Any way we can make progress with this series before it gets all 
forgotten  about?

    Stefan

Re: [PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

On Wed, May 18, 2022 at 11:24:48AM -0400, Stefan Berger wrote:
> On 4/14/22 11:30, Daniel Kiper wrote:
> > On Thu, Apr 07, 2022 at 04:41:04PM +0200, Daniel Kiper wrote:
> > > On Mon, Mar 28, 2022 at 05:22:25PM +1100, Daniel Axtens wrote:
> > > > Hi all,
> > > >
> > > > This is, at long last, an updated version of my series extending Patrick's
> > > > dynamic memory regions to ieee1275.
> > > >
> > > > Noteworthy changes:
> > > >
> > > >   - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
> > > >     ieee1275 patches.
> > > >
> > > >   - reworked the ieee1275 runtime memory claiming to be more resilient and better
> > > >     documented.
> > > >
> > > >   - fixed comment style and hopefully addressed all other change requests.
> > > >
> > > >   - grub will now try asking for contiguous memory and then, if that fails, for
> > > >     discontiguous memory - in case region merging with the discontiguous memory
> > > >     is sufficient to allow the eventual allocation to succeed.
> > >
> > > Patrick, all mm and EFI code got my RB. Could you test it with your
> > > Argon changes? If these changes pass your tests I will merge them.
> >
> > Patrick, ping?
> >
> > To be more precise, I am thinking about the patches up to #10.
> >
> > Daniel
>
> Any way we can make progress with this series before it gets all forgotten
> about?

It is not and it will not be forgotten. I am waiting for some allocator
tests reports. When I get them I will merge allocator stuff and review
the rest of the code from this patch series. Sadly folks who are going
to test the code are busy with other stuff. Though I am pinging them...

Anyway, sorry for delay...

Daniel

Re: [PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2508 bytes --]

On Thu, May 19, 2022 at 06:34:48PM +0200, Daniel Kiper wrote:
> On Wed, May 18, 2022 at 11:24:48AM -0400, Stefan Berger wrote:
> > On 4/14/22 11:30, Daniel Kiper wrote:
> > > On Thu, Apr 07, 2022 at 04:41:04PM +0200, Daniel Kiper wrote:
> > > > On Mon, Mar 28, 2022 at 05:22:25PM +1100, Daniel Axtens wrote:
> > > > > Hi all,
> > > > >
> > > > > This is, at long last, an updated version of my series extending Patrick's
> > > > > dynamic memory regions to ieee1275.
> > > > >
> > > > > Noteworthy changes:
> > > > >
> > > > >   - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
> > > > >     ieee1275 patches.
> > > > >
> > > > >   - reworked the ieee1275 runtime memory claiming to be more resilient and better
> > > > >     documented.
> > > > >
> > > > >   - fixed comment style and hopefully addressed all other change requests.
> > > > >
> > > > >   - grub will now try asking for contiguous memory and then, if that fails, for
> > > > >     discontiguous memory - in case region merging with the discontiguous memory
> > > > >     is sufficient to allow the eventual allocation to succeed.
> > > >
> > > > Patrick, all mm and EFI code got my RB. Could you test it with your
> > > > Argon changes? If these changes pass your tests I will merge them.
> > >
> > > Patrick, ping?
> > >
> > > To be more precise, I am thinking about the patches up to #10.
> > >
> > > Daniel
> >
> > Any way we can make progress with this series before it gets all forgotten
> > about?
> 
> It is not and it will not be forgotten. I am waiting for some allocator
> tests reports. When I get them I will merge allocator stuff and review
> the rest of the code from this patch series. Sadly folks who are going
> to test the code are busy with other stuff. Though I am pinging them...
> 
> Anyway, sorry for delay...
> 
> Daniel

Sorry for the delay here, and thanks for the nudge. I've found a few
quiet moments this morning to rebase my Argon2 patch series for LUKS2 on
top of this series and found everything to work as expected. Decrypting
a volume with a memory hardness of 2GB RAM worked just fine on an EFI
x64 system. So this series is:

    Tested-by: Patrick Steinhardt <ps@pks.im>

I'm going to revive my own patch series and send it in for review
soonish. The nice thing of having waited so long is that Argon2 support
has meanwhile landed in libgcrypt, so that should make it easier to
integrate it into GRUB.

Thanks
Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

Huh! For some reason I missed this email... Sorry folks about that!

On Sun, May 29, 2022 at 07:55:00AM +0200, Patrick Steinhardt wrote:
> On Thu, May 19, 2022 at 06:34:48PM +0200, Daniel Kiper wrote:
> > On Wed, May 18, 2022 at 11:24:48AM -0400, Stefan Berger wrote:
> > > On 4/14/22 11:30, Daniel Kiper wrote:
> > > > On Thu, Apr 07, 2022 at 04:41:04PM +0200, Daniel Kiper wrote:
> > > > > On Mon, Mar 28, 2022 at 05:22:25PM +1100, Daniel Axtens wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > This is, at long last, an updated version of my series extending Patrick's
> > > > > > dynamic memory regions to ieee1275.
> > > > > >
> > > > > > Noteworthy changes:
> > > > > >
> > > > > >   - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
> > > > > >     ieee1275 patches.
> > > > > >
> > > > > >   - reworked the ieee1275 runtime memory claiming to be more resilient and better
> > > > > >     documented.
> > > > > >
> > > > > >   - fixed comment style and hopefully addressed all other change requests.
> > > > > >
> > > > > >   - grub will now try asking for contiguous memory and then, if that fails, for
> > > > > >     discontiguous memory - in case region merging with the discontiguous memory
> > > > > >     is sufficient to allow the eventual allocation to succeed.
> > > > >
> > > > > Patrick, all mm and EFI code got my RB. Could you test it with your
> > > > > Argon changes? If these changes pass your tests I will merge them.
> > > >
> > > > Patrick, ping?
> > > >
> > > > To be more precise, I am thinking about the patches up to #10.
> > > >
> > > > Daniel
> > >
> > > Any way we can make progress with this series before it gets all forgotten
> > > about?
> >
> > It is not and it will not be forgotten. I am waiting for some allocator
> > tests reports. When I get them I will merge allocator stuff and review
> > the rest of the code from this patch series. Sadly folks who are going
> > to test the code are busy with other stuff. Though I am pinging them...
> >
> > Anyway, sorry for delay...
> >
> > Daniel
>
> Sorry for the delay here, and thanks for the nudge. I've found a few
> quiet moments this morning to rebase my Argon2 patch series for LUKS2 on
> top of this series and found everything to work as expected. Decrypting
> a volume with a memory hardness of 2GB RAM worked just fine on an EFI
> x64 system. So this series is:
>
>     Tested-by: Patrick Steinhardt <ps@pks.im>

Patrick, thanks a lot for doing tests!

Here I would like to thank Patrick, Stefan, Daniel, Glenn and other
folks who were working on this feature! It is very important and long
awaited improvement for the GRUB memory manager (everyone who was
looking at it knows it is one of the most complicated parts of the
GRUB). This patch set unblocks and makes possible work on Argon2,
appended signature secure boot support, IBM vTPM, etc.

If I do not hear any objections by the end of this week I will be
pushing into master v3 of this patch set [1] (Patrick, I hope you are OK
with me adding your Tested-by there) together with the other patches
which got my RB up until now.

I will be reviewing appended signature secure boot support, IBM vTPM,
etc. patches in the following weeks.

> I'm going to revive my own patch series and send it in for review
> soonish. The nice thing of having waited so long is that Argon2 support
> has meanwhile landed in libgcrypt, so that should make it easier to
> integrate it into GRUB.

Nice to hear that.

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html

Re: [PATCH v2 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4165 bytes --]

On Thu, Jun 23, 2022 at 07:16:04PM +0200, Daniel Kiper wrote:
> Huh! For some reason I missed this email... Sorry folks about that!
> 
> On Sun, May 29, 2022 at 07:55:00AM +0200, Patrick Steinhardt wrote:
> > On Thu, May 19, 2022 at 06:34:48PM +0200, Daniel Kiper wrote:
> > > On Wed, May 18, 2022 at 11:24:48AM -0400, Stefan Berger wrote:
> > > > On 4/14/22 11:30, Daniel Kiper wrote:
> > > > > On Thu, Apr 07, 2022 at 04:41:04PM +0200, Daniel Kiper wrote:
> > > > > > On Mon, Mar 28, 2022 at 05:22:25PM +1100, Daniel Axtens wrote:
> > > > > > > Hi all,
> > > > > > >
> > > > > > > This is, at long last, an updated version of my series extending Patrick's
> > > > > > > dynamic memory regions to ieee1275.
> > > > > > >
> > > > > > > Noteworthy changes:
> > > > > > >
> > > > > > >   - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
> > > > > > >     ieee1275 patches.
> > > > > > >
> > > > > > >   - reworked the ieee1275 runtime memory claiming to be more resilient and better
> > > > > > >     documented.
> > > > > > >
> > > > > > >   - fixed comment style and hopefully addressed all other change requests.
> > > > > > >
> > > > > > >   - grub will now try asking for contiguous memory and then, if that fails, for
> > > > > > >     discontiguous memory - in case region merging with the discontiguous memory
> > > > > > >     is sufficient to allow the eventual allocation to succeed.
> > > > > >
> > > > > > Patrick, all mm and EFI code got my RB. Could you test it with your
> > > > > > Argon changes? If these changes pass your tests I will merge them.
> > > > >
> > > > > Patrick, ping?
> > > > >
> > > > > To be more precise, I am thinking about the patches up to #10.
> > > > >
> > > > > Daniel
> > > >
> > > > Any way we can make progress with this series before it gets all forgotten
> > > > about?
> > >
> > > It is not and it will not be forgotten. I am waiting for some allocator
> > > tests reports. When I get them I will merge allocator stuff and review
> > > the rest of the code from this patch series. Sadly folks who are going
> > > to test the code are busy with other stuff. Though I am pinging them...
> > >
> > > Anyway, sorry for delay...
> > >
> > > Daniel
> >
> > Sorry for the delay here, and thanks for the nudge. I've found a few
> > quiet moments this morning to rebase my Argon2 patch series for LUKS2 on
> > top of this series and found everything to work as expected. Decrypting
> > a volume with a memory hardness of 2GB RAM worked just fine on an EFI
> > x64 system. So this series is:
> >
> >     Tested-by: Patrick Steinhardt <ps@pks.im>
> 
> Patrick, thanks a lot for doing tests!
> 
> Here I would like to thank Patrick, Stefan, Daniel, Glenn and other
> folks who were working on this feature! It is very important and long
> awaited improvement for the GRUB memory manager (everyone who was
> looking at it knows it is one of the most complicated parts of the
> GRUB). This patch set unblocks and makes possible work on Argon2,
> appended signature secure boot support, IBM vTPM, etc.
> 
> If I do not hear any objections by the end of this week I will be
> pushing into master v3 of this patch set [1] (Patrick, I hope you are OK
> with me adding your Tested-by there) together with the other patches
> which got my RB up until now.

Yes, I am. I noticed at a later point that I accidentally tested v2 and
already re-did the tests on v3 with the same results.

Patrick

> I will be reviewing appended signature secure boot support, IBM vTPM,
> etc. patches in the following weeks.
> 
> > I'm going to revive my own patch series and send it in for review
> > soonish. The nice thing of having waited so long is that Argon2 support
> > has meanwhile landed in libgcrypt, so that should make it easier to
> > integrate it into GRUB.
> 
> Nice to hear that.
> 
> Daniel
> 
> [1] https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]