[PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Diego Domingos]

Hello,

This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).

Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation
However, during some tests, we found this can be a problem if:

- we have more than 64 CPUs
- Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs)
- Grub needs to request memory.

If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum 
number of CPUs supported by the OS, causing the machine to fail to init.
Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value.

The patches 11-15 aren't merged to the grub tree yet, so I'm sending those patches again together with my patch to implement vector 5
on top of them.

The patches 11-15 contains the following:

Daniel Axtens (4):
  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

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

[PATCH 1/6] ieee1275: request memory with ibm, client-architecture-support

[Diego Domingos]

From: Daniel Axtens <dja@axtens.net>

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 enough to reliably verify a kernel. Fortunately, the Power
Architecture Platform Reference (PAPR) defines a method we can call to ask
for more memory: 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.

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.

 - Set option vector 3 to request Floating Point, VMX and Decimal Floating
   point, but don't abort the boot if we can't get them.

 - Set option vector 4 to request a minimum VP percentage to 1%, which is
   what Linux requests, and is below the default of 10%. Without this,
   some systems with very large or very small configurations fail to boot.

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

Notes about the choices of parameters:

 - 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".)

 - unspecified CAS vectors take on default values. Some of these values
   might restrict the ability of certain hardware configurations to boot.
   This is why we need to specify the VP percentage in vector 4, which is
   in turn why we need to specify vector 3.

Finally, we should have enough memory to verify a kernel, and we will
reach Linux. 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). Linux includes a much more restrictive set of
PVR values and processor support levels, and this CAS invocation will likely
induce another reboot. On this reboot grub will again notice the higher RMA,
and not call CAS. We will get to Linux again, Linux will call CAS again, but
because the values are now set for Linux this will not induce another CAS
reboot and we will finally boot all the way to userspace.

On all subsequent boots, everything will be configured with 512MB of RMA,
so there will be no further CAS reboots from grub. (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>
---
 grub-core/kern/ieee1275/cmain.c  |   3 +
 grub-core/kern/ieee1275/init.c   | 152 +++++++++++++++++++++++++++++++
 include/grub/ieee1275/ieee1275.h |   8 ++
 3 files changed, 163 insertions(+)

diff --git a/grub-core/kern/ieee1275/cmain.c b/grub-core/kern/ieee1275/cmain.c
index 4442b6a83..b707798ec 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 2adf4fdfc..cf4bcf2cf 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -200,11 +200,163 @@ 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;
+  grub_uint8_t vec3_size;
+  grub_uint16_t vec3;
+  grub_uint8_t vec4_size;
+  grub_uint16_t vec4;
+} __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 = 4 - 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
+    },
+    .vec3_size = 2 - 1,
+    .vec3 = 0x00e0, /* ask for FP + VMX + DFP but don't halt if unsatisfied */
+    .vec4_size = 2 - 1,
+    .vec4 = 0x0001, /* set required minimum capacity % to the lowest value */
+  };
+
+  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 from grub...");
+  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 f53228703..b5c916d1d 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.31.1

[PATCH 2/6] ieee1275: drop len -= 1 quirk in heap_init

[Diego Domingos]

From: Daniel Axtens <dja@axtens.net>

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 cf4bcf2cf..60a49301b 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.31.1

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

[Diego Domingos]

From: Daniel Axtens <dja@axtens.net>

On powerpc-ieee1275, we are running out of memory trying to verify
anything. This is because:

 - we have to load an entire file into memory to verify it. This is
   difficult to change with appended signatures.
 - We only have 32MB of heap.
 - Distro kernels are now often around 30MB.

So we want to be able to claim more memory from OpenFirmware for our heap
at runtime.

There are some complications:

 - The grub mm code isn't the only thing that will make claims on
   memory from OpenFirmware:

    * PFW/SLOF will have claimed some for their own use.

    * The ieee1275 loader will try to find other bits of memory that we
      haven't claimed to place the kernel and initrd when we go to boot.

    * Once we load Linux, it will also try to claim memory. It claims
      memory without any reference to /memory/available, it just starts
      at min(top of RMO, 768MB) and works down. So we need to avoid this
      area. See arch/powerpc/kernel/prom_init.c as of v5.11.

 - The smallest amount of memory a ppc64 KVM guest can have is 256MB.
   It doesn't work with distro kernels but can work with custom kernels.
   We should maintain support for that. (ppc32 can boot with even less,
   and we shouldn't break that either.)

 - Even if a VM has more memory, the memory OpenFirmware makes available
   as Real Memory Area can be restricted. Even with our CAS work, an LPAR
   on a PowerVM box is likely to have only 512MB available to OpenFirmware
   even if it has many gigabytes of memory allocated.

What should we do?

We don't know in advance how big the kernel and initrd are going to be,
which makes figuring out how much memory we can take a bit tricky.

To figure out how much memory we should leave unused, I looked at:

 - an Ubuntu 20.04.1 ppc64le pseries KVM guest:
    vmlinux: ~30MB
    initrd:  ~50MB

 - a RHEL8.2 ppc64le pseries KVM guest:
    vmlinux: ~30MB
    initrd:  ~30MB

So to give us a little wriggle room, I think we want to leave at least
128MB for the loader to put vmlinux and initrd in memory and leave Linux
with space to satisfy its early allocations.

Allow other space to be allocated at runtime.

Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 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 f76fc658b..31eb99ea2 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 60a49301b..ec591e6e1 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)
@@ -343,16 +567,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.31.1

[PATCH 4/6] Add memtool module with memory allocation stress-test

[Diego Domingos]

From: Daniel Axtens <dja@axtens.net>

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>
---
 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 5212dfab1..485169d53 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2538,3 +2538,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 000000000..bb4ad359e
--- /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 75f6eacbe..c341d4f60 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -664,6 +664,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
@@ -691,6 +693,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.31.1

[PATCH 5/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0

[Diego Domingos]

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 b848d0928..ac2d40579 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -6359,7 +6359,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 485169d53..65bc902d4 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1129,6 +1129,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 000000000..e68b8448b
--- /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 b5c916d1d..5002195fc 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.31.1

[PATCH 6/6] ieee1275: implement vec5 for cas negotiation

[Diego Domingos]

As a legacy support, if the vector 5 is not implemented, Power Hypervisor will
consider the max CPUs as 64 instead 256 currently supported during
client-architecture-support negotiation.

This patch implements the vector 5 and set the MAX CPUs to 256 while setting the
others values to 0 (default).

Signed-off-by: Diego Domingos <diegodo@linux.vnet.ibm.com>

---
 grub-core/kern/ieee1275/init.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index ec591e6e1..26d3c7a33 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -494,6 +494,19 @@ struct option_vector2 {
   grub_uint8_t max_pft_size;
 } __attribute__((packed));
 
+struct option_vector5 {
+        grub_uint8_t byte1;
+        grub_uint8_t byte2;
+        grub_uint8_t byte3;
+        grub_uint8_t cmo;
+        grub_uint8_t associativity;
+        grub_uint8_t bin_opts;
+        grub_uint8_t micro_checkpoint;
+        grub_uint8_t reserved0;
+        grub_uint32_t max_cpus;
+} __attribute__((packed));
+
+
 struct pvr_entry {
   grub_uint32_t mask;
   grub_uint32_t entry;
@@ -512,6 +525,8 @@ struct cas_vector {
   grub_uint16_t vec3;
   grub_uint8_t vec4_size;
   grub_uint16_t vec4;
+  grub_uint8_t vec5_size;
+  struct option_vector5 vec5;
 } __attribute__((packed));
 
 /*
@@ -534,7 +549,7 @@ grub_ieee1275_ibm_cas (void)
   } args;
   struct cas_vector vector = {
     .pvr_list = { { 0x00000000, 0xffffffff } }, /* any processor */
-    .num_vecs = 4 - 1,
+    .num_vecs = 5 - 1,
     .vec1_size = 0,
     .vec1 = 0x80, /* ignore */
     .vec2_size = 1 + sizeof(struct option_vector2) - 2,
@@ -545,6 +560,10 @@ grub_ieee1275_ibm_cas (void)
     .vec3 = 0x00e0, /* ask for FP + VMX + DFP but don't halt if unsatisfied */
     .vec4_size = 2 - 1,
     .vec4 = 0x0001, /* set required minimum capacity % to the lowest value */
+    .vec5_size = 1 + sizeof(struct option_vector5) - 2,
+    .vec5 = {
+	0, 0, 0, 0, 0, 0, 0, 0, 256
+    }
   };
 
   INIT_IEEE1275_COMMON (&args.common, "call-method", 3, 2);
-- 
2.31.1

Re: [PATCH 4/6] Add memtool module with memory allocation stress-test

[Glenn Washburn]

On Thu, 11 Aug 2022 14:41:02 -0300
Diego Domingos <diegodo@linux.vnet.ibm.com> wrote:

> From: Daniel Axtens <dja@axtens.net>
> 
> 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:

Does this build without giving --enable-mm-debug to configure? Last
time I checked it did not. It would be nice to include this patch if it
allows building without --enable-mm-debug.

Glenn

> 
>  * 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>
> ---
>  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 5212dfab1..485169d53 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2538,3 +2538,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 000000000..bb4ad359e
> --- /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 75f6eacbe..c341d4f60 100644
> --- a/grub-core/kern/mm.c
> +++ b/grub-core/kern/mm.c
> @@ -664,6 +664,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
> @@ -691,6 +693,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;

Re: [PATCH 6/6] ieee1275: implement vec5 for cas negotiation

[Daniel Axtens]

Diego Domingos <diegodo@linux.vnet.ibm.com> writes:

> As a legacy support, if the vector 5 is not implemented, Power Hypervisor will
> consider the max CPUs as 64 instead 256 currently supported during
> client-architecture-support negotiation.
>
> This patch implements the vector 5 and set the MAX CPUs to 256 while setting the
> others values to 0 (default).

Ergh, CAS. I'm sorry I didn't check the defaults more carefully when I
was at IBM!

Anyway, this looks sane to me. I'm not in a position to test it any
more, but it certainly follows the pattern I'd expect. And it is only
likely to affect IBM machines anyway, so I think it's safe to add.


> +struct option_vector5 {
> +        grub_uint8_t byte1;
> +        grub_uint8_t byte2;
> +        grub_uint8_t byte3;
> +        grub_uint8_t cmo;
> +        grub_uint8_t associativity;
> +        grub_uint8_t bin_opts;
> +        grub_uint8_t micro_checkpoint;
> +        grub_uint8_t reserved0;
> +        grub_uint32_t max_cpus;
> +} __attribute__((packed));
> +
I think the indent here should be 2 spaces?

> +
>  struct pvr_entry {
>    grub_uint32_t mask;
>    grub_uint32_t entry;
> @@ -512,6 +525,8 @@ struct cas_vector {
>    grub_uint16_t vec3;
>    grub_uint8_t vec4_size;
>    grub_uint16_t vec4;
> +  grub_uint8_t vec5_size;
> +  struct option_vector5 vec5;
>  } __attribute__((packed));
>  
>  /*
> @@ -534,7 +549,7 @@ grub_ieee1275_ibm_cas (void)
>    } args;
>    struct cas_vector vector = {
>      .pvr_list = { { 0x00000000, 0xffffffff } }, /* any processor */
> -    .num_vecs = 4 - 1,
> +    .num_vecs = 5 - 1,
>      .vec1_size = 0,
>      .vec1 = 0x80, /* ignore */
>      .vec2_size = 1 + sizeof(struct option_vector2) - 2,
> @@ -545,6 +560,10 @@ grub_ieee1275_ibm_cas (void)
>      .vec3 = 0x00e0, /* ask for FP + VMX + DFP but don't halt if unsatisfied */
>      .vec4_size = 2 - 1,
>      .vec4 = 0x0001, /* set required minimum capacity % to the lowest value */
> +    .vec5_size = 1 + sizeof(struct option_vector5) - 2,
> +    .vec5 = {
> +	0, 0, 0, 0, 0, 0, 0, 0, 256
This maybe should be indented 6 spaces instead of 1 tab? But I wouldn't
do a whole new revision just for this.

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

Kind regards,
Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

Hi,

Adding Sudhakar and Glenn...

On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> Hello,
>
> This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
>
> Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation
> However, during some tests, we found this can be a problem if:
>
> - we have more than 64 CPUs
> - Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs)
> - Grub needs to request memory.
>
> If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum
> number of CPUs supported by the OS, causing the machine to fail to init.
> Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value.
>
> The patches 11-15 aren't merged to the grub tree yet, so I'm sending those patches again together with my patch to implement vector 5
> on top of them.
>
> The patches 11-15 contains the following:
>
> Daniel Axtens (4):
>   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
>
> Stefan Berger (1):
>   ibmvtpm: Add support for trusted boot using a vTPM 2.0

I went through all patches and cannot see major problems with them.
Though there are a lot of minor things which have to be fixed. Sadly due
to number of them I cannot simply ignore that.

Here is the list of the issues:
  - functions calls/sizeof(): e.g. "grub_printf()" should be replaced with "grub_printf ()",
    add space before "(", in the code; though I am OK with the former in comments and
    commit messages,
  - casts: e.g. "*(grub_uint32_t *)data" should be replaced with "*(grub_uint32_t *) data",
    add space between ")" and "data",
  - s/__attribute__((packed))/GRUB_PACKED/
  - if you use grub_err_t type please test for GRUB_ERR_NONE instead of !err or err;
    please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
  - if you test pointers for NULL please test using NULL constant instead of e.g. !ptr
  - if you use a value often please define constant for it; good candidate for such
    change is at least 0x30000000 in the patch #3; if constant definition is an overkill
    please comment what given numbers/strings mean or at least where they come from,
  - please do not use "//" for comments,
  - I am OK with lines a bit longer than 80; so, please do not wrap
    lines too early,
  - year in the copyright should be 2022.

The GRUB coding style is described here [1] and you can find good
example of coding style in the grub-core/kern/efi/sb.c file.

Please take into account latest comments from Daniel A. and Glenn too.

If something is not clear please drop me a line.

Last but not least, sorry for huge delay. I hope I will be able to
review much faster next version of this patch set.

Daniel

[1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Coding-style

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Stefan Berger]

On 11/24/22 12:56, Daniel Kiper wrote:
> Hi,
> 
> Adding Sudhakar and Glenn...
> 
> On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
>> Hello,
>>
>> This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
>>
>> Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation
>> However, during some tests, we found this can be a problem if:
>>
>> - we have more than 64 CPUs
>> - Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs)
>> - Grub needs to request memory.
>>
>> If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum
>> number of CPUs supported by the OS, causing the machine to fail to init.
>> Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value.
>>
>> The patches 11-15 aren't merged to the grub tree yet, so I'm sending those patches again together with my patch to implement vector 5
>> on top of them.
>>
>> The patches 11-15 contains the following:
>>
>> Daniel Axtens (4):
>>    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
>>
>> Stefan Berger (1):
>>    ibmvtpm: Add support for trusted boot using a vTPM 2.0
> 
> I went through all patches and cannot see major problems with them.
> Though there are a lot of minor things which have to be fixed. Sadly due
> to number of them I cannot simply ignore that.
> 
> Here is the list of the issues:
>    - functions calls/sizeof(): e.g. "grub_printf()" should be replaced with "grub_printf ()",
>      add space before "(", in the code; though I am OK with the former in comments and
>      commit messages,
>    - casts: e.g. "*(grub_uint32_t *)data" should be replaced with "*(grub_uint32_t *) data",
>      add space between ")" and "data",
>    - s/__attribute__((packed))/GRUB_PACKED/
>    - if you use grub_err_t type please test for GRUB_ERR_NONE instead of !err or err;
>      please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
>    - if you test pointers for NULL please test using NULL constant instead of e.g. !ptr
>    - if you use a value often please define constant for it; good candidate for such
>      change is at least 0x30000000 in the patch #3; if constant definition is an overkill
>      please comment what given numbers/strings mean or at least where they come from,
>    - please do not use "//" for comments,
>    - I am OK with lines a bit longer than 80; so, please do not wrap
>      lines too early,

This is a bit vague but I think I addressed them now.

>    - year in the copyright should be 2022.
> 
> The GRUB coding style is described here [1] and you can find good
> example of coding style in the grub-core/kern/efi/sb.c file.
> 
> Please take into account latest comments from Daniel A. and Glenn too.

I don't know how to support the memtool without --enable-mm-debug at the same time since the module seems to be missing then but the build system still expects it on 'make install'. Unless there's an existing example of how to do it I would not post with this patch.

I can get it to create an empty module with this trick here but don't know whether this helps the cause.

GRUB_MOD_FINI (memtools)
{
#ifdef MM_DEBUG
   grub_unregister_command (cmd_lsmem);
   grub_unregister_command (cmd_lsfreemem);
   grub_unregister_command (cmd_sba);
#else
   (void) grub_unregister_command;
#endif
}



    stefan

> 
> If something is not clear please drop me a line.
> 
> Last but not least, sorry for huge delay. I hope I will be able to
> review much faster next version of this patch set.
> 
> Daniel
> 
> [1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Coding-style

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Stefan Berger]

On 11/30/22 14:47, Stefan Berger wrote:
> 
> 
> On 11/24/22 12:56, Daniel Kiper wrote:
>> Hi,
>>
>> Adding Sudhakar and Glenn...
>>
>> On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
>>> Hello,
>>>
>>> This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
>>>
>>> Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation
>>> However, during some tests, we found this can be a problem if:
>>>
>>> - we have more than 64 CPUs
>>> - Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs)
>>> - Grub needs to request memory.
>>>
>>> If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum
>>> number of CPUs supported by the OS, causing the machine to fail to init.
>>> Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value.
>>>
>>> The patches 11-15 aren't merged to the grub tree yet, so I'm sending those patches again together with my patch to implement vector 5
>>> on top of them.
>>>
>>> The patches 11-15 contains the following:
>>>
>>> Daniel Axtens (4):
>>>    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
>>>
>>> Stefan Berger (1):
>>>    ibmvtpm: Add support for trusted boot using a vTPM 2.0
>>
>> I went through all patches and cannot see major problems with them.
>> Though there are a lot of minor things which have to be fixed. Sadly due
>> to number of them I cannot simply ignore that.
>>
>> Here is the list of the issues:
>>    - functions calls/sizeof(): e.g. "grub_printf()" should be replaced with "grub_printf ()",
>>      add space before "(", in the code; though I am OK with the former in comments and
>>      commit messages,
>>    - casts: e.g. "*(grub_uint32_t *)data" should be replaced with "*(grub_uint32_t *) data",
>>      add space between ")" and "data",
>>    - s/__attribute__((packed))/GRUB_PACKED/
>>    - if you use grub_err_t type please test for GRUB_ERR_NONE instead of !err or err;
>>      please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
>>    - if you test pointers for NULL please test using NULL constant instead of e.g. !ptr
>>    - if you use a value often please define constant for it; good candidate for such
>>      change is at least 0x30000000 in the patch #3; if constant definition is an overkill
>>      please comment what given numbers/strings mean or at least where they come from,
>>    - please do not use "//" for comments,
>>    - I am OK with lines a bit longer than 80; so, please do not wrap
>>      lines too early,
> 
> This is a bit vague but I think I addressed them now.
> 
>>    - year in the copyright should be 2022.
>>
>> The GRUB coding style is described here [1] and you can find good
>> example of coding style in the grub-core/kern/efi/sb.c file.
>>
>> Please take into account latest comments from Daniel A. and Glenn too.
> 
> I don't know how to support the memtool without --enable-mm-debug at the same time since the module seems to be missing then but the build system still expects it on 'make install'. Unless there's an existing example of how to do it I would not post with this patch.
> 
> I can get it to create an empty module with this trick here but don't know whether this helps the cause.
> 
> GRUB_MOD_FINI (memtools)
> {
> #ifdef MM_DEBUG
>    grub_unregister_command (cmd_lsmem);
>    grub_unregister_command (cmd_lsfreemem);
>    grub_unregister_command (cmd_sba);
> #else
>    (void) grub_unregister_command;
> #endif
> }
> 
> 
In 1/6 we have this here. Is this sufficiently gating the usage of the code or do we need to use '#if defined(__powerpc__)' to only compile code newly added powerpc-specific code  used due to this flag being set?

+
+      if (grub_strncmp (tmp, "IBM,", 4) == 0)
+       grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);


> 
>     stefan
> 
>>
>> If something is not clear please drop me a line.
>>
>> Last but not least, sorry for huge delay. I hope I will be able to
>> review much faster next version of this patch set.
>>
>> Daniel
>>
>> [1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Coding-style

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Stefan Berger]

On 11/30/22 16:24, Stefan Berger wrote:
> 
> 
> On 11/30/22 14:47, Stefan Berger wrote:
>>
>>
>> On 11/24/22 12:56, Daniel Kiper wrote:
>>> Hi,
>>>
>>> Adding Sudhakar and Glenn...
>>>
>>> On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
>>>> Hello,
>>>>
>>>> This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
>>>>
>>>> Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation
>>>> However, during some tests, we found this can be a problem if:
>>>>
>>>> - we have more than 64 CPUs
>>>> - Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs)
>>>> - Grub needs to request memory.
>>>>
>>>> If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum
>>>> number of CPUs supported by the OS, causing the machine to fail to init.
>>>> Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value.
>>>>
>>>> The patches 11-15 aren't merged to the grub tree yet, so I'm sending those patches again together with my patch to implement vector 5
>>>> on top of them.
>>>>
>>>> The patches 11-15 contains the following:
>>>>
>>>> Daniel Axtens (4):
>>>>    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
>>>>
>>>> Stefan Berger (1):
>>>>    ibmvtpm: Add support for trusted boot using a vTPM 2.0
>>>
>>> I went through all patches and cannot see major problems with them.
>>> Though there are a lot of minor things which have to be fixed. Sadly due
>>> to number of them I cannot simply ignore that.
>>>
>>> Here is the list of the issues:
>>>    - functions calls/sizeof(): e.g. "grub_printf()" should be replaced with "grub_printf ()",
>>>      add space before "(", in the code; though I am OK with the former in comments and
>>>      commit messages,
>>>    - casts: e.g. "*(grub_uint32_t *)data" should be replaced with "*(grub_uint32_t *) data",
>>>      add space between ")" and "data",
>>>    - s/__attribute__((packed))/GRUB_PACKED/
>>>    - if you use grub_err_t type please test for GRUB_ERR_NONE instead of !err or err;
>>>      please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
>>>    - if you test pointers for NULL please test using NULL constant instead of e.g. !ptr
>>>    - if you use a value often please define constant for it; good candidate for such
>>>      change is at least 0x30000000 in the patch #3; if constant definition is an overkill
>>>      please comment what given numbers/strings mean or at least where they come from,
>>>    - please do not use "//" for comments,
>>>    - I am OK with lines a bit longer than 80; so, please do not wrap
>>>      lines too early,
>>
>> This is a bit vague but I think I addressed them now.
>>
>>>    - year in the copyright should be 2022.
>>>
>>> The GRUB coding style is described here [1] and you can find good
>>> example of coding style in the grub-core/kern/efi/sb.c file.
>>>
>>> Please take into account latest comments from Daniel A. and Glenn too.
>>
>> I don't know how to support the memtool without --enable-mm-debug at the same time since the module seems to be missing then but the build system still expects it on 'make install'. Unless there's an existing example of how to do it I would not post with this patch.
>>
>> I can get it to create an empty module with this trick here but don't know whether this helps the cause.
>>
>> GRUB_MOD_FINI (memtools)
>> {
>> #ifdef MM_DEBUG
>>    grub_unregister_command (cmd_lsmem);
>>    grub_unregister_command (cmd_lsfreemem);
>>    grub_unregister_command (cmd_sba);
>> #else
>>    (void) grub_unregister_command;
>> #endif
>> }
>>
>>
> In 1/6 we have this here. Is this sufficiently gating the usage of the code or do we need to use '#if defined(__powerpc__)' to only compile code newly added powerpc-specific code  used due to this flag being set?
> 
> +
> +      if (grub_strncmp (tmp, "IBM,", 4) == 0)
> +       grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
> 
> 

And yet another question: Is __i386__ actually using grub-core/kern/ieee1275/init.c ? I don't see it compiling this file but there's a #ifdef __i386__ in this file.

>>
>>     stefan
>>
>>>
>>> If something is not clear please drop me a line.
>>>
>>> Last but not least, sorry for huge delay. I hope I will be able to
>>> review much faster next version of this patch set.
>>>
>>> Daniel
>>>
>>> [1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Coding-style
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Glenn Washburn]

On Wed, 30 Nov 2022 17:42:40 -0500
Stefan Berger <stefanb@linux.ibm.com> wrote:

> 
> 
> On 11/30/22 16:24, Stefan Berger wrote:
> > 
> > 
> > On 11/30/22 14:47, Stefan Berger wrote:
> >>
> >>
> >> On 11/24/22 12:56, Daniel Kiper wrote:
> >>> Hi,
> >>>
> >>> Adding Sudhakar and Glenn...
> >>>
> >>> On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> >>>> Hello,
> >>>>
> >>>> This is an addition to the series sent from Daniel Axtens
> >>>> (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
> >>>>
> >>>> Patch 'ieee1275: request memory with
> >>>> ibm,client-architecture-support' implements vectors 1-4 of
> >>>> client-architecture-support negotiation However, during some
> >>>> tests, we found this can be a problem if:
> >>>>
> >>>> - we have more than 64 CPUs
> >>>> - Hardware Management Console (HMC) is configured to minimum of
> >>>> CPUs >64 (for example, min of 200 CPUs)
> >>>> - Grub needs to request memory.
> >>>>
> >>>> If vector 5 is not implemented, Power Hypervisor will consider
> >>>> the default value for vector 5 and 64 will bet set as the
> >>>> maximum number of CPUs supported by the OS, causing the machine
> >>>> to fail to init. Today we support 256 CPUs (max) on Power, so we
> >>>> need to implement vector 5 and set the MAX CPUs bits to this
> >>>> value.
> >>>>
> >>>> The patches 11-15 aren't merged to the grub tree yet, so I'm
> >>>> sending those patches again together with my patch to implement
> >>>> vector 5 on top of them.
> >>>>
> >>>> The patches 11-15 contains the following:
> >>>>
> >>>> Daniel Axtens (4):
> >>>>    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
> >>>>
> >>>> Stefan Berger (1):
> >>>>    ibmvtpm: Add support for trusted boot using a vTPM 2.0
> >>>
> >>> I went through all patches and cannot see major problems with
> >>> them. Though there are a lot of minor things which have to be
> >>> fixed. Sadly due to number of them I cannot simply ignore that.
> >>>
> >>> Here is the list of the issues:
> >>>    - functions calls/sizeof(): e.g. "grub_printf()" should be
> >>> replaced with "grub_printf ()", add space before "(", in the
> >>> code; though I am OK with the former in comments and commit
> >>> messages,
> >>>    - casts: e.g. "*(grub_uint32_t *)data" should be replaced with
> >>> "*(grub_uint32_t *) data", add space between ")" and "data",
> >>>    - s/__attribute__((packed))/GRUB_PACKED/
> >>>    - if you use grub_err_t type please test for GRUB_ERR_NONE
> >>> instead of !err or err; please do not use plain numbers, e.g. 0
> >>> to substitute GRUB_ERR_NONE,
> >>>    - if you test pointers for NULL please test using NULL
> >>> constant instead of e.g. !ptr
> >>>    - if you use a value often please define constant for it; good
> >>> candidate for such change is at least 0x30000000 in the patch #3;
> >>> if constant definition is an overkill please comment what given
> >>> numbers/strings mean or at least where they come from,
> >>>    - please do not use "//" for comments,
> >>>    - I am OK with lines a bit longer than 80; so, please do not
> >>> wrap lines too early,
> >>
> >> This is a bit vague but I think I addressed them now.
> >>
> >>>    - year in the copyright should be 2022.
> >>>
> >>> The GRUB coding style is described here [1] and you can find good
> >>> example of coding style in the grub-core/kern/efi/sb.c file.
> >>>
> >>> Please take into account latest comments from Daniel A. and Glenn
> >>> too.
> >>
> >> I don't know how to support the memtool without --enable-mm-debug
> >> at the same time since the module seems to be missing then but the
> >> build system still expects it on 'make install'. Unless there's an
> >> existing example of how to do it I would not post with this patch.
> >>
> >> I can get it to create an empty module with this trick here but
> >> don't know whether this helps the cause.
> >>
> >> GRUB_MOD_FINI (memtools)
> >> {
> >> #ifdef MM_DEBUG
> >>    grub_unregister_command (cmd_lsmem);
> >>    grub_unregister_command (cmd_lsfreemem);
> >>    grub_unregister_command (cmd_sba);
> >> #else
> >>    (void) grub_unregister_command;
> >> #endif
> >> }
> >>
> >>
> > In 1/6 we have this here. Is this sufficiently gating the usage of
> > the code or do we need to use '#if defined(__powerpc__)' to only
> > compile code newly added powerpc-specific code  used due to this
> > flag being set?
> > 
> > +
> > +      if (grub_strncmp (tmp, "IBM,", 4) == 0)
> > +       grub_ieee1275_set_flag
> > (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
> > 
> > 
> 
> And yet another question: Is __i386__ actually using
> grub-core/kern/ieee1275/init.c ? I don't see it compiling this file
> but there's a #ifdef __i386__ in this file.

Yes, there is a i386-ieee1275 target. It builds and the tests run
successfully, iirc.

Glenn

> 
> >>
> >>     stefan
> >>
> >>>
> >>> If something is not clear please drop me a line.
> >>>
> >>> Last but not least, sorry for huge delay. I hope I will be able to
> >>> review much faster next version of this patch set.
> >>>
> >>> Daniel
> >>>
> >>> [1]
> >>> https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Coding-style
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Stefan Berger]

On 12/1/22 00:19, Glenn Washburn wrote:
> On Wed, 30 Nov 2022 17:42:40 -0500
> Stefan Berger <stefanb@linux.ibm.com> wrote:
> 
>>
>>
>> On 11/30/22 16:24, Stefan Berger wrote:
>>>
>>>
>>> On 11/30/22 14:47, Stefan Berger wrote:
>>>>
>>>>
>>>> On 11/24/22 12:56, Daniel Kiper wrote:
>>>>> Hi,
>>>>>
>>>>> Adding Sudhakar and Glenn...
>>>>>
>>>>> On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
>>>>>> Hello,
>>>>>>
>>>>>> This is an addition to the series sent from Daniel Axtens
>>>>>> (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
>>>>>>
>>>>>> Patch 'ieee1275: request memory with
>>>>>> ibm,client-architecture-support' implements vectors 1-4 of
>>>>>> client-architecture-support negotiation However, during some
>>>>>> tests, we found this can be a problem if:
>>>>>>
>>>>>> - we have more than 64 CPUs
>>>>>> - Hardware Management Console (HMC) is configured to minimum of
>>>>>> CPUs >64 (for example, min of 200 CPUs)
>>>>>> - Grub needs to request memory.
>>>>>>
>>>>>> If vector 5 is not implemented, Power Hypervisor will consider
>>>>>> the default value for vector 5 and 64 will bet set as the
>>>>>> maximum number of CPUs supported by the OS, causing the machine
>>>>>> to fail to init. Today we support 256 CPUs (max) on Power, so we
>>>>>> need to implement vector 5 and set the MAX CPUs bits to this
>>>>>> value.
>>>>>>
>>>>>> The patches 11-15 aren't merged to the grub tree yet, so I'm
>>>>>> sending those patches again together with my patch to implement
>>>>>> vector 5 on top of them.
>>>>>>
>>>>>> The patches 11-15 contains the following:
>>>>>>
>>>>>> Daniel Axtens (4):
>>>>>>     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
>>>>>>
>>>>>> Stefan Berger (1):
>>>>>>     ibmvtpm: Add support for trusted boot using a vTPM 2.0
>>>>>
>>>>> I went through all patches and cannot see major problems with
>>>>> them. Though there are a lot of minor things which have to be
>>>>> fixed. Sadly due to number of them I cannot simply ignore that.
>>>>>
>>>>> Here is the list of the issues:
>>>>>     - functions calls/sizeof(): e.g. "grub_printf()" should be
>>>>> replaced with "grub_printf ()", add space before "(", in the
>>>>> code; though I am OK with the former in comments and commit
>>>>> messages,
>>>>>     - casts: e.g. "*(grub_uint32_t *)data" should be replaced with
>>>>> "*(grub_uint32_t *) data", add space between ")" and "data",
>>>>>     - s/__attribute__((packed))/GRUB_PACKED/
>>>>>     - if you use grub_err_t type please test for GRUB_ERR_NONE
>>>>> instead of !err or err; please do not use plain numbers, e.g. 0
>>>>> to substitute GRUB_ERR_NONE,
>>>>>     - if you test pointers for NULL please test using NULL
>>>>> constant instead of e.g. !ptr
>>>>>     - if you use a value often please define constant for it; good
>>>>> candidate for such change is at least 0x30000000 in the patch #3;
>>>>> if constant definition is an overkill please comment what given
>>>>> numbers/strings mean or at least where they come from,
>>>>>     - please do not use "//" for comments,
>>>>>     - I am OK with lines a bit longer than 80; so, please do not
>>>>> wrap lines too early,
>>>>
>>>> This is a bit vague but I think I addressed them now.
>>>>
>>>>>     - year in the copyright should be 2022.
>>>>>
>>>>> The GRUB coding style is described here [1] and you can find good
>>>>> example of coding style in the grub-core/kern/efi/sb.c file.
>>>>>
>>>>> Please take into account latest comments from Daniel A. and Glenn
>>>>> too.
>>>>
>>>> I don't know how to support the memtool without --enable-mm-debug
>>>> at the same time since the module seems to be missing then but the
>>>> build system still expects it on 'make install'. Unless there's an
>>>> existing example of how to do it I would not post with this patch.
>>>>
>>>> I can get it to create an empty module with this trick here but
>>>> don't know whether this helps the cause.
>>>>
>>>> GRUB_MOD_FINI (memtools)
>>>> {
>>>> #ifdef MM_DEBUG
>>>>     grub_unregister_command (cmd_lsmem);
>>>>     grub_unregister_command (cmd_lsfreemem);
>>>>     grub_unregister_command (cmd_sba);
>>>> #else
>>>>     (void) grub_unregister_command;
>>>> #endif
>>>> }
>>>>
>>>>
>>> In 1/6 we have this here. Is this sufficiently gating the usage of
>>> the code or do we need to use '#if defined(__powerpc__)' to only
>>> compile code newly added powerpc-specific code  used due to this
>>> flag being set?
>>>
>>> +
>>> +      if (grub_strncmp (tmp, "IBM,", 4) == 0)
>>> +       grub_ieee1275_set_flag
>>> (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
>>>
>>>
>>
>> And yet another question: Is __i386__ actually using
>> grub-core/kern/ieee1275/init.c ? I don't see it compiling this file
>> but there's a #ifdef __i386__ in this file.
> 
> Yes, there is a i386-ieee1275 target. It builds and the tests run
> successfully, iirc.

How is this target enabled? Just configuring on an i386 host doesn't seem to do it and I don't see an obvious configure option to build for it, either.

    Stefan

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

Adding Vladimir...

On Wed, Nov 30, 2022 at 02:47:41PM -0500, Stefan Berger wrote:
> On 11/24/22 12:56, Daniel Kiper wrote:
> > Hi,
> >
> > Adding Sudhakar and Glenn...
> >
> > On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> > > Hello,
> > >
> > > This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
> > >
> > > Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation
> > > However, during some tests, we found this can be a problem if:
> > >
> > > - we have more than 64 CPUs
> > > - Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs)
> > > - Grub needs to request memory.
> > >
> > > If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum
> > > number of CPUs supported by the OS, causing the machine to fail to init.
> > > Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value.
> > >
> > > The patches 11-15 aren't merged to the grub tree yet, so I'm sending those patches again together with my patch to implement vector 5
> > > on top of them.
> > >
> > > The patches 11-15 contains the following:
> > >
> > > Daniel Axtens (4):
> > >    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
> > >
> > > Stefan Berger (1):
> > >    ibmvtpm: Add support for trusted boot using a vTPM 2.0
> >
> > I went through all patches and cannot see major problems with them.
> > Though there are a lot of minor things which have to be fixed. Sadly due
> > to number of them I cannot simply ignore that.
> >
> > Here is the list of the issues:
> >    - functions calls/sizeof(): e.g. "grub_printf()" should be replaced with "grub_printf ()",
> >      add space before "(", in the code; though I am OK with the former in comments and
> >      commit messages,
> >    - casts: e.g. "*(grub_uint32_t *)data" should be replaced with "*(grub_uint32_t *) data",
> >      add space between ")" and "data",
> >    - s/__attribute__((packed))/GRUB_PACKED/
> >    - if you use grub_err_t type please test for GRUB_ERR_NONE instead of !err or err;
> >      please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
> >    - if you test pointers for NULL please test using NULL constant instead of e.g. !ptr
> >    - if you use a value often please define constant for it; good candidate for such
> >      change is at least 0x30000000 in the patch #3; if constant definition is an overkill
> >      please comment what given numbers/strings mean or at least where they come from,
> >    - please do not use "//" for comments,
> >    - I am OK with lines a bit longer than 80; so, please do not wrap
> >      lines too early,
>
> This is a bit vague but I think I addressed them now.

Cool! Thanks!

> >    - year in the copyright should be 2022.
> >
> > The GRUB coding style is described here [1] and you can find good
> > example of coding style in the grub-core/kern/efi/sb.c file.
> >
> > Please take into account latest comments from Daniel A. and Glenn too.
>
> I don't know how to support the memtool without --enable-mm-debug at the same time since the module seems to be missing then but the build system still expects it on 'make install'. Unless there's an existing example of how to do it I would not post with this patch.
>
> I can get it to create an empty module with this trick here but don't know whether this helps the cause.
>
> GRUB_MOD_FINI (memtools)
> {
> #ifdef MM_DEBUG
>   grub_unregister_command (cmd_lsmem);
>   grub_unregister_command (cmd_lsfreemem);
>   grub_unregister_command (cmd_sba);
> #else
>   (void) grub_unregister_command;
> #endif
> }

I think it would be better if you fix Makefile to not install non-existing
modules than use trick mentioned above. And it seems to me we should have
similar case somewhere to steal idea from. Or maybe Vladimir will have an
idea how to easily/nicely fix this issue. Vladimir?

Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

On Wed, Nov 30, 2022 at 04:24:59PM -0500, Stefan Berger wrote:
> On 11/30/22 14:47, Stefan Berger wrote:
> > On 11/24/22 12:56, Daniel Kiper wrote:
> > > Hi,
> > >
> > > Adding Sudhakar and Glenn...
> > >
> > > On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> > > > Hello,
> > > >
> > > > This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
> > > >
> > > > Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation
> > > > However, during some tests, we found this can be a problem if:
> > > >
> > > > - we have more than 64 CPUs
> > > > - Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs)
> > > > - Grub needs to request memory.
> > > >
> > > > If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum
> > > > number of CPUs supported by the OS, causing the machine to fail to init.
> > > > Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value.
> > > >
> > > > The patches 11-15 aren't merged to the grub tree yet, so I'm sending those patches again together with my patch to implement vector 5
> > > > on top of them.
> > > >
> > > > The patches 11-15 contains the following:
> > > >
> > > > Daniel Axtens (4):
> > > >    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
> > > >
> > > > Stefan Berger (1):
> > > >    ibmvtpm: Add support for trusted boot using a vTPM 2.0
> > >
> > > I went through all patches and cannot see major problems with them.
> > > Though there are a lot of minor things which have to be fixed. Sadly due
> > > to number of them I cannot simply ignore that.
> > >
> > > Here is the list of the issues:
> > >    - functions calls/sizeof(): e.g. "grub_printf()" should be replaced with "grub_printf ()",
> > >      add space before "(", in the code; though I am OK with the former in comments and
> > >      commit messages,
> > >    - casts: e.g. "*(grub_uint32_t *)data" should be replaced with "*(grub_uint32_t *) data",
> > >      add space between ")" and "data",
> > >    - s/__attribute__((packed))/GRUB_PACKED/
> > >    - if you use grub_err_t type please test for GRUB_ERR_NONE instead of !err or err;
> > >      please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
> > >    - if you test pointers for NULL please test using NULL constant instead of e.g. !ptr
> > >    - if you use a value often please define constant for it; good candidate for such
> > >      change is at least 0x30000000 in the patch #3; if constant definition is an overkill
> > >      please comment what given numbers/strings mean or at least where they come from,
> > >    - please do not use "//" for comments,
> > >    - I am OK with lines a bit longer than 80; so, please do not wrap
> > >      lines too early,
> >
> > This is a bit vague but I think I addressed them now.
> >
> > >    - year in the copyright should be 2022.
> > >
> > > The GRUB coding style is described here [1] and you can find good
> > > example of coding style in the grub-core/kern/efi/sb.c file.
> > >
> > > Please take into account latest comments from Daniel A. and Glenn too.
> >
> > I don't know how to support the memtool without --enable-mm-debug at the same time since the module seems to be missing then but the build system still expects it on 'make install'. Unless there's an existing example of how to do it I would not post with this patch.
> >
> > I can get it to create an empty module with this trick here but don't know whether this helps the cause.
> >
> > GRUB_MOD_FINI (memtools)
> > {
> > #ifdef MM_DEBUG
> >    grub_unregister_command (cmd_lsmem);
> >    grub_unregister_command (cmd_lsfreemem);
> >    grub_unregister_command (cmd_sba);
> > #else
> >    (void) grub_unregister_command;
> > #endif
> > }
> >
> >
> In 1/6 we have this here. Is this sufficiently gating the usage of the
> code or do we need to use '#if defined(__powerpc__)' to only compile
> code newly added powerpc-specific code  used due to this flag being
> set?
>
> +
> +      if (grub_strncmp (tmp, "IBM,", 4) == 0)
> +       grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);

Good point! I think we should use "#if defined(__powerpc__)" if it is
powerpc-specific code.

Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

On Thu, Dec 01, 2022 at 08:43:56AM -0500, Stefan Berger wrote:
> On 12/1/22 00:19, Glenn Washburn wrote:
> > On Wed, 30 Nov 2022 17:42:40 -0500
> > Stefan Berger <stefanb@linux.ibm.com> wrote:
> > > On 11/30/22 16:24, Stefan Berger wrote:
> > > > On 11/30/22 14:47, Stefan Berger wrote:
> > > > > On 11/24/22 12:56, Daniel Kiper wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Adding Sudhakar and Glenn...
> > > > > >
> > > > > > On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> > > > > > > Hello,
> > > > > > >
> > > > > > > This is an addition to the series sent from Daniel Axtens
> > > > > > > (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
> > > > > > >
> > > > > > > Patch 'ieee1275: request memory with
> > > > > > > ibm,client-architecture-support' implements vectors 1-4 of
> > > > > > > client-architecture-support negotiation However, during some
> > > > > > > tests, we found this can be a problem if:
> > > > > > >
> > > > > > > - we have more than 64 CPUs
> > > > > > > - Hardware Management Console (HMC) is configured to minimum of
> > > > > > > CPUs >64 (for example, min of 200 CPUs)
> > > > > > > - Grub needs to request memory.
> > > > > > >
> > > > > > > If vector 5 is not implemented, Power Hypervisor will consider
> > > > > > > the default value for vector 5 and 64 will bet set as the
> > > > > > > maximum number of CPUs supported by the OS, causing the machine
> > > > > > > to fail to init. Today we support 256 CPUs (max) on Power, so we
> > > > > > > need to implement vector 5 and set the MAX CPUs bits to this
> > > > > > > value.
> > > > > > >
> > > > > > > The patches 11-15 aren't merged to the grub tree yet, so I'm
> > > > > > > sending those patches again together with my patch to implement
> > > > > > > vector 5 on top of them.
> > > > > > >
> > > > > > > The patches 11-15 contains the following:
> > > > > > >
> > > > > > > Daniel Axtens (4):
> > > > > > >     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
> > > > > > >
> > > > > > > Stefan Berger (1):
> > > > > > >     ibmvtpm: Add support for trusted boot using a vTPM 2.0
> > > > > >
> > > > > > I went through all patches and cannot see major problems with
> > > > > > them. Though there are a lot of minor things which have to be
> > > > > > fixed. Sadly due to number of them I cannot simply ignore that.
> > > > > >
> > > > > > Here is the list of the issues:
> > > > > >     - functions calls/sizeof(): e.g. "grub_printf()" should be
> > > > > > replaced with "grub_printf ()", add space before "(", in the
> > > > > > code; though I am OK with the former in comments and commit
> > > > > > messages,
> > > > > >     - casts: e.g. "*(grub_uint32_t *)data" should be replaced with
> > > > > > "*(grub_uint32_t *) data", add space between ")" and "data",
> > > > > >     - s/__attribute__((packed))/GRUB_PACKED/
> > > > > >     - if you use grub_err_t type please test for GRUB_ERR_NONE
> > > > > > instead of !err or err; please do not use plain numbers, e.g. 0
> > > > > > to substitute GRUB_ERR_NONE,
> > > > > >     - if you test pointers for NULL please test using NULL
> > > > > > constant instead of e.g. !ptr
> > > > > >     - if you use a value often please define constant for it; good
> > > > > > candidate for such change is at least 0x30000000 in the patch #3;
> > > > > > if constant definition is an overkill please comment what given
> > > > > > numbers/strings mean or at least where they come from,
> > > > > >     - please do not use "//" for comments,
> > > > > >     - I am OK with lines a bit longer than 80; so, please do not
> > > > > > wrap lines too early,
> > > > >
> > > > > This is a bit vague but I think I addressed them now.
> > > > >
> > > > > >     - year in the copyright should be 2022.
> > > > > >
> > > > > > The GRUB coding style is described here [1] and you can find good
> > > > > > example of coding style in the grub-core/kern/efi/sb.c file.
> > > > > >
> > > > > > Please take into account latest comments from Daniel A. and Glenn
> > > > > > too.
> > > > >
> > > > > I don't know how to support the memtool without --enable-mm-debug
> > > > > at the same time since the module seems to be missing then but the
> > > > > build system still expects it on 'make install'. Unless there's an
> > > > > existing example of how to do it I would not post with this patch.
> > > > >
> > > > > I can get it to create an empty module with this trick here but
> > > > > don't know whether this helps the cause.
> > > > >
> > > > > GRUB_MOD_FINI (memtools)
> > > > > {
> > > > > #ifdef MM_DEBUG
> > > > >     grub_unregister_command (cmd_lsmem);
> > > > >     grub_unregister_command (cmd_lsfreemem);
> > > > >     grub_unregister_command (cmd_sba);
> > > > > #else
> > > > >     (void) grub_unregister_command;
> > > > > #endif
> > > > > }
> > > > >
> > > > >
> > > > In 1/6 we have this here. Is this sufficiently gating the usage of
> > > > the code or do we need to use '#if defined(__powerpc__)' to only
> > > > compile code newly added powerpc-specific code  used due to this
> > > > flag being set?
> > > >
> > > > +
> > > > +      if (grub_strncmp (tmp, "IBM,", 4) == 0)
> > > > +       grub_ieee1275_set_flag
> > > > (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
> > >
> > > And yet another question: Is __i386__ actually using
> > > grub-core/kern/ieee1275/init.c ? I don't see it compiling this file
> > > but there's a #ifdef __i386__ in this file.
> >
> > Yes, there is a i386-ieee1275 target. It builds and the tests run
> > successfully, iirc.
>
> How is this target enabled? Just configuring on an i386 host doesn't
> seem to do it and I don't see an obvious configure option to build for
> it, either.

./configure --target=i386 --with-platform=ieee1275 ...

Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Stefan Berger]

On 12/1/22 09:02, Daniel Kiper wrote:
> On Thu, Dec 01, 2022 at 08:43:56AM -0500, Stefan Berger wrote:
>> On 12/1/22 00:19, Glenn Washburn wrote:
>>> On Wed, 30 Nov 2022 17:42:40 -0500
>>> Stefan Berger <stefanb@linux.ibm.com> wrote:
>>>> On 11/30/22 16:24, Stefan Berger wrote:
>>>>> On 11/30/22 14:47, Stefan Berger wrote:
>>>>>> On 11/24/22 12:56, Daniel Kiper wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Adding Sudhakar and Glenn...
>>>>>>>
>>>>>>> On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> This is an addition to the series sent from Daniel Axtens
>>>>>>>> (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
>>>>>>>>
>>>>>>>> Patch 'ieee1275: request memory with
>>>>>>>> ibm,client-architecture-support' implements vectors 1-4 of
>>>>>>>> client-architecture-support negotiation However, during some
>>>>>>>> tests, we found this can be a problem if:
>>>>>>>>
>>>>>>>> - we have more than 64 CPUs
>>>>>>>> - Hardware Management Console (HMC) is configured to minimum of
>>>>>>>> CPUs >64 (for example, min of 200 CPUs)
>>>>>>>> - Grub needs to request memory.
>>>>>>>>
>>>>>>>> If vector 5 is not implemented, Power Hypervisor will consider
>>>>>>>> the default value for vector 5 and 64 will bet set as the
>>>>>>>> maximum number of CPUs supported by the OS, causing the machine
>>>>>>>> to fail to init. Today we support 256 CPUs (max) on Power, so we
>>>>>>>> need to implement vector 5 and set the MAX CPUs bits to this
>>>>>>>> value.
>>>>>>>>
>>>>>>>> The patches 11-15 aren't merged to the grub tree yet, so I'm
>>>>>>>> sending those patches again together with my patch to implement
>>>>>>>> vector 5 on top of them.
>>>>>>>>
>>>>>>>> The patches 11-15 contains the following:
>>>>>>>>
>>>>>>>> Daniel Axtens (4):
>>>>>>>>      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
>>>>>>>>
>>>>>>>> Stefan Berger (1):
>>>>>>>>      ibmvtpm: Add support for trusted boot using a vTPM 2.0
>>>>>>>
>>>>>>> I went through all patches and cannot see major problems with
>>>>>>> them. Though there are a lot of minor things which have to be
>>>>>>> fixed. Sadly due to number of them I cannot simply ignore that.
>>>>>>>
>>>>>>> Here is the list of the issues:
>>>>>>>      - functions calls/sizeof(): e.g. "grub_printf()" should be
>>>>>>> replaced with "grub_printf ()", add space before "(", in the
>>>>>>> code; though I am OK with the former in comments and commit
>>>>>>> messages,
>>>>>>>      - casts: e.g. "*(grub_uint32_t *)data" should be replaced with
>>>>>>> "*(grub_uint32_t *) data", add space between ")" and "data",
>>>>>>>      - s/__attribute__((packed))/GRUB_PACKED/
>>>>>>>      - if you use grub_err_t type please test for GRUB_ERR_NONE
>>>>>>> instead of !err or err; please do not use plain numbers, e.g. 0
>>>>>>> to substitute GRUB_ERR_NONE,
>>>>>>>      - if you test pointers for NULL please test using NULL
>>>>>>> constant instead of e.g. !ptr
>>>>>>>      - if you use a value often please define constant for it; good
>>>>>>> candidate for such change is at least 0x30000000 in the patch #3;
>>>>>>> if constant definition is an overkill please comment what given
>>>>>>> numbers/strings mean or at least where they come from,
>>>>>>>      - please do not use "//" for comments,
>>>>>>>      - I am OK with lines a bit longer than 80; so, please do not
>>>>>>> wrap lines too early,
>>>>>>
>>>>>> This is a bit vague but I think I addressed them now.
>>>>>>
>>>>>>>      - year in the copyright should be 2022.
>>>>>>>
>>>>>>> The GRUB coding style is described here [1] and you can find good
>>>>>>> example of coding style in the grub-core/kern/efi/sb.c file.
>>>>>>>
>>>>>>> Please take into account latest comments from Daniel A. and Glenn
>>>>>>> too.
>>>>>>
>>>>>> I don't know how to support the memtool without --enable-mm-debug
>>>>>> at the same time since the module seems to be missing then but the
>>>>>> build system still expects it on 'make install'. Unless there's an
>>>>>> existing example of how to do it I would not post with this patch.
>>>>>>
>>>>>> I can get it to create an empty module with this trick here but
>>>>>> don't know whether this helps the cause.
>>>>>>
>>>>>> GRUB_MOD_FINI (memtools)
>>>>>> {
>>>>>> #ifdef MM_DEBUG
>>>>>>      grub_unregister_command (cmd_lsmem);
>>>>>>      grub_unregister_command (cmd_lsfreemem);
>>>>>>      grub_unregister_command (cmd_sba);
>>>>>> #else
>>>>>>      (void) grub_unregister_command;
>>>>>> #endif
>>>>>> }
>>>>>>
>>>>>>
>>>>> In 1/6 we have this here. Is this sufficiently gating the usage of
>>>>> the code or do we need to use '#if defined(__powerpc__)' to only
>>>>> compile code newly added powerpc-specific code  used due to this
>>>>> flag being set?
>>>>>
>>>>> +
>>>>> +      if (grub_strncmp (tmp, "IBM,", 4) == 0)
>>>>> +       grub_ieee1275_set_flag
>>>>> (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
>>>>
>>>> And yet another question: Is __i386__ actually using
>>>> grub-core/kern/ieee1275/init.c ? I don't see it compiling this file
>>>> but there's a #ifdef __i386__ in this file.
>>>
>>> Yes, there is a i386-ieee1275 target. It builds and the tests run
>>> successfully, iirc.
>>
>> How is this target enabled? Just configuring on an i386 host doesn't
>> seem to do it and I don't see an obvious configure option to build for
>> it, either.
> 
> ./configure --target=i386 --with-platform=ieee1275 ...

I had to adjust the created symlist.h like this to make it compile at least:

//#include <../include/grub/machine/pxe.h>
//#include <../include/grub/machine/int.h>

When trying to install this on my i386 VM I get this here:

sudo grub-install /dev/vda --target=i386-ieee1275
Installing for i386-ieee1275 platform.
grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
grub-install: warning: unknown device type vda1.
grub-install: error: ofpathname: not found.


Would this type of target produce a grub version that works on a VM that would otherwise work with i386-pc?
And is this still a supported target?

    Stefan


> 
> Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote:
> On 12/1/22 09:02, Daniel Kiper wrote:

[...]

> > ./configure --target=i386 --with-platform=ieee1275 ...
>
> I had to adjust the created symlist.h like this to make it compile at least:
>
> //#include <../include/grub/machine/pxe.h>
> //#include <../include/grub/machine/int.h>

Hmmm... Strange... It builds for me without any issues. I use latest
Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with
gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler
do you use?

> When trying to install this on my i386 VM I get this here:
>
> sudo grub-install /dev/vda --target=i386-ieee1275
> Installing for i386-ieee1275 platform.
> grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
> grub-install: warning: unknown device type vda1.
> grub-install: error: ofpathname: not found.
>
> Would this type of target produce a grub version that works on a VM
> that would otherwise work with i386-pc?

Nope...

> And is this still a supported target?

What do you mean by "supported"? It is build tested and, AIUI, all "make check"
tests pass. Though I am not sure anybody uses this kinda weird target. Well,
I think I knew but I forgot... :-)

Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Stefan Berger]

On 12/1/22 09:47, Daniel Kiper wrote:
> On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote:
>> On 12/1/22 09:02, Daniel Kiper wrote:
> 
> [...]
> 
>>> ./configure --target=i386 --with-platform=ieee1275 ...
>>
>> I had to adjust the created symlist.h like this to make it compile at least:
>>
>> //#include <../include/grub/machine/pxe.h>
>> //#include <../include/grub/machine/int.h>
> 
> Hmmm... Strange... It builds for me without any issues. I use latest
> Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with
> gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler
> do you use?

$ cat /etc/debian_version
11.5
$ gcc --version
gcc (Debian 10.2.1-6) 10.2.1 20210110
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.



> 
>> When trying to install this on my i386 VM I get this here:
>>
>> sudo grub-install /dev/vda --target=i386-ieee1275
>> Installing for i386-ieee1275 platform.
>> grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
>> grub-install: warning: unknown device type vda1.
>> grub-install: error: ofpathname: not found.
>>
>> Would this type of target produce a grub version that works on a VM
>> that would otherwise work with i386-pc?
> 
> Nope...

Well, not good...

> 
>> And is this still a supported target?
> 
> What do you mean by "supported"? It is build tested and, AIUI, all "make check"
> tests pass. Though I am not sure anybody uses this kinda weird target. Well,
> I think I knew but I forgot... :-)


The problem is I don't know much about the target other than ieee1275 being OpenFirmare or so. So we (I) could easily break someone's favorite target here ...

The following function from this series would probably run on i386-ieee1275 as well since these nodes are probably so fundamental to OpenFirmware that there's no point in surrounding it with #ifdef __powerpc__ ?

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'.  */
   if (grub_ieee1275_finddevice ("/", &root))
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't find / node");
   if (grub_ieee1275_get_integer_property (root, "#address-cells", &address_cells,
                                           sizeof (address_cells), 0))
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine #address-cells");
   if (grub_ieee1275_get_integer_property (root, "#size-cells", &size_cells,
                                           sizeof (size_cells), 0))
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine #size-cells");


I added the if's here just to be 'safe'.

In the end i386-ieee1275 shouldn't hold up progress on powerpc ieee1275...

    Stefan


> 
> Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

On Thu, Dec 01, 2022 at 09:58:45AM -0500, Stefan Berger wrote:
> On 12/1/22 09:47, Daniel Kiper wrote:
> > On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote:
> > > On 12/1/22 09:02, Daniel Kiper wrote:
> >
> > [...]
> >
> > > > ./configure --target=i386 --with-platform=ieee1275 ...
> > >
> > > I had to adjust the created symlist.h like this to make it compile at least:
> > >
> > > //#include <../include/grub/machine/pxe.h>
> > > //#include <../include/grub/machine/int.h>
> >
> > Hmmm... Strange... It builds for me without any issues. I use latest
> > Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with
> > gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler
> > do you use?
>
> $ cat /etc/debian_version
> 11.5
> $ gcc --version
> gcc (Debian 10.2.1-6) 10.2.1 20210110
> Copyright (C) 2020 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Hmmm... It should work...

> > > When trying to install this on my i386 VM I get this here:
> > >
> > > sudo grub-install /dev/vda --target=i386-ieee1275
> > > Installing for i386-ieee1275 platform.
> > > grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
> > > grub-install: warning: unknown device type vda1.
> > > grub-install: error: ofpathname: not found.
> > >
> > > Would this type of target produce a grub version that works on a VM
> > > that would otherwise work with i386-pc?
> >
> > Nope...
>
> Well, not good...
>
> >
> > > And is this still a supported target?
> >
> > What do you mean by "supported"? It is build tested and, AIUI, all "make check"
> > tests pass. Though I am not sure anybody uses this kinda weird target. Well,
> > I think I knew but I forgot... :-)
>
> The problem is I don't know much about the target other than ieee1275
> being OpenFirmare or so. So we (I) could easily break someone's
> favorite target here ...

:-) I am happy you care about that. I think Glenn could tell you how to
run "make check" for i386-ieee1275 target. Glenn?

And I think you can find some hints how to run "make check" in the
INSTALL file too...

> The following function from this series would probably run on
> i386-ieee1275 as well since these nodes are probably so fundamental to
> OpenFirmware that there's no point in surrounding it with #ifdef
> __powerpc__ ?

Yeah, I think you are right...

> 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'.  */
>   if (grub_ieee1275_finddevice ("/", &root))
>     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't find / node");
>   if (grub_ieee1275_get_integer_property (root, "#address-cells", &address_cells,
>                                           sizeof (address_cells), 0))
>     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine #address-cells");
>   if (grub_ieee1275_get_integer_property (root, "#size-cells", &size_cells,
>                                           sizeof (size_cells), 0))
>     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine #size-cells");
>
> I added the if's here just to be 'safe'.

This should be enough. Though please double check if other parts of the
code fail safely if grub_ieee1275_total_mem() returns an error.

> In the end i386-ieee1275 shouldn't hold up progress on powerpc ieee1275...

Yeah, I concur...

Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Stefan Berger]

On 12/1/22 10:51, Daniel Kiper wrote:
> On Thu, Dec 01, 2022 at 09:58:45AM -0500, Stefan Berger wrote:
>> On 12/1/22 09:47, Daniel Kiper wrote:
>>> On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote:
>>>> On 12/1/22 09:02, Daniel Kiper wrote:
>>>
>>> [...]
>>>
>>>>> ./configure --target=i386 --with-platform=ieee1275 ...
>>>>
>>>> I had to adjust the created symlist.h like this to make it compile at least:
>>>>
>>>> //#include <../include/grub/machine/pxe.h>
>>>> //#include <../include/grub/machine/int.h>
>>>
>>> Hmmm... Strange... It builds for me without any issues. I use latest
>>> Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with
>>> gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler
>>> do you use?
>>
>> $ cat /etc/debian_version
>> 11.5
>> $ gcc --version
>> gcc (Debian 10.2.1-6) 10.2.1 20210110
>> Copyright (C) 2020 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions.  There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> Hmmm... It should work...

Not typically using Debian but Debian 11.5 was the i386 iso I could find. Well, it works fine for i386-pc.

> 
>>>> When trying to install this on my i386 VM I get this here:
>>>>
>>>> sudo grub-install /dev/vda --target=i386-ieee1275
>>>> Installing for i386-ieee1275 platform.
>>>> grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
>>>> grub-install: warning: unknown device type vda1.
>>>> grub-install: error: ofpathname: not found.
>>>>
>>>> Would this type of target produce a grub version that works on a VM
>>>> that would otherwise work with i386-pc?
>>>
>>> Nope...
>>
>> Well, not good...
>>
>>>
>>>> And is this still a supported target?
>>>
>>> What do you mean by "supported"? It is build tested and, AIUI, all "make check"
>>> tests pass. Though I am not sure anybody uses this kinda weird target. Well,
>>> I think I knew but I forgot... :-)
>>
>> The problem is I don't know much about the target other than ieee1275
>> being OpenFirmare or so. So we (I) could easily break someone's
>> favorite target here ...
> 
> :-) I am happy you care about that. I think Glenn could tell you how to

The more esoteric the target the more ... worrisome ? :-)

> run "make check" for i386-ieee1275 target. Glenn?

I just ran 'make check' (as user) for i386-pc on current master (and with the patches applied). The fallout was this:

FAIL: squashfs_test    [blkid missing]
FAIL: help_test        no particular helpful message
FAIL: grub_func_test   only this type of error: tests/video_checksum.c:checksum:615: assert failed: 0 Checksum cmdline_cat_2560x1440xrgba8888:44 failed: 0x62031fea vs 0x8071678a


Now on the same i386 machine with i386-ieee1275 (master): [after installing some more packages to be able to run the tests I can now build it fine]

FAIL: squashfs_test
FAIL: ahci_test
FAIL: grub_script_eval
FAIL: pata_test
FAIL: example_grub_script_test
FAIL: grub_script_leading_whitespace
FAIL: ehci_test
FAIL: ohci_test
FAIL: grub_script_test
FAIL: grub_script_echo1
FAIL: uhci_test
FAIL: grub_script_echo_keywords
FAIL: grub_script_vars1
FAIL: grub_script_for1
FAIL: grub_script_while1
FAIL: grub_script_if
FAIL: grub_script_comments
FAIL: grub_script_break
FAIL: grub_script_return
FAIL: grub_script_shift
FAIL: grub_cmd_regexp
FAIL: grub_script_functions
FAIL: grub_script_blockarg
FAIL: grub_script_continue
FAIL: grub_script_setparams
FAIL: grub_cmd_set_date
FAIL: grub_cmd_date
FAIL: grub_func_test
FAIL: grub_cmd_sleep
FAIL: grub_script_not
FAIL: hddboot_test
FAIL: fddboot_test
FAIL: xzcompress_test
FAIL: help_test
FAIL: test_unset
FAIL: grub_cmd_echo
FAIL: grub_script_gettext
FAIL: grub_script_escape_comma
FAIL: test_sha512sum
FAIL: grub_script_strcmp
FAIL: grub_cmd_tr
FAIL: file_filter_test
FAIL: gzcompress_test
FAIL: lzocompress_test

So I take this as the baseline for me now -- interpretation is: i386-ieee1275 is borked.



> 
> And I think you can find some hints how to run "make check" in the
> INSTALL file too...

I wished there was a line apt-get to install them all. Something like this seems a good base but still need qemu:

sudo apt install dosfstools gzip lzop xz-utils attr cpio g++ gawk parted recode tar util-linux btrfs-progs dosfstools e2fsprogs f2fs-tools genromfs hfsprogs jfsutils nilfs-tools ntfs-3g reiserfsprogs squashfs-tools reiserfsprogs udftools xfsprogs zfs-fuse exfat-fuse

> 
>> The following function from this series would probably run on
>> i386-ieee1275 as well since these nodes are probably so fundamental to
>> OpenFirmware that there's no point in surrounding it with #ifdef
>> __powerpc__ ?
> 
> Yeah, I think you are right...
> 
>> 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'.  */
>>    if (grub_ieee1275_finddevice ("/", &root))
>>      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't find / node");
>>    if (grub_ieee1275_get_integer_property (root, "#address-cells", &address_cells,
>>                                            sizeof (address_cells), 0))
>>      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine #address-cells");
>>    if (grub_ieee1275_get_integer_property (root, "#size-cells", &size_cells,
>>                                            sizeof (size_cells), 0))
>>      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine #size-cells");
>>
>> I added the if's here just to be 'safe'.
> 
> This should be enough. Though please double check if other parts of the
> code fail safely if grub_ieee1275_total_mem() returns an error.
> 
>> In the end i386-ieee1275 shouldn't hold up progress on powerpc ieee1275...
> 
> Yeah, I concur...


Alright then let's proceed .

    Stefan

> 
> Daniel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Kiper]

On Thu, Dec 01, 2022 at 11:44:02AM -0500, Stefan Berger wrote:
> On 12/1/22 10:51, Daniel Kiper wrote:
> > On Thu, Dec 01, 2022 at 09:58:45AM -0500, Stefan Berger wrote:
> > > On 12/1/22 09:47, Daniel Kiper wrote:
> > > > On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote:
> > > > > On 12/1/22 09:02, Daniel Kiper wrote:
> > > >
> > > > [...]
> > > >
> > > > > > ./configure --target=i386 --with-platform=ieee1275 ...
> > > > >
> > > > > I had to adjust the created symlist.h like this to make it compile at least:
> > > > >
> > > > > //#include <../include/grub/machine/pxe.h>
> > > > > //#include <../include/grub/machine/int.h>
> > > >
> > > > Hmmm... Strange... It builds for me without any issues. I use latest
> > > > Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with
> > > > gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler
> > > > do you use?
> > >
> > > $ cat /etc/debian_version
> > > 11.5
> > > $ gcc --version
> > > gcc (Debian 10.2.1-6) 10.2.1 20210110
> > > Copyright (C) 2020 Free Software Foundation, Inc.
> > > This is free software; see the source for copying conditions.  There is NO
> > > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> >
> > Hmmm... It should work...
>
> Not typically using Debian but Debian 11.5 was the i386 iso I could find. Well, it works fine for i386-pc.
>
> > > > > When trying to install this on my i386 VM I get this here:
> > > > >
> > > > > sudo grub-install /dev/vda --target=i386-ieee1275
> > > > > Installing for i386-ieee1275 platform.
> > > > > grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
> > > > > grub-install: warning: unknown device type vda1.
> > > > > grub-install: error: ofpathname: not found.
> > > > >
> > > > > Would this type of target produce a grub version that works on a VM
> > > > > that would otherwise work with i386-pc?
> > > >
> > > > Nope...
> > >
> > > Well, not good...
> > >
> > > >
> > > > > And is this still a supported target?
> > > >
> > > > What do you mean by "supported"? It is build tested and, AIUI, all "make check"
> > > > tests pass. Though I am not sure anybody uses this kinda weird target. Well,
> > > > I think I knew but I forgot... :-)
> > >
> > > The problem is I don't know much about the target other than ieee1275
> > > being OpenFirmare or so. So we (I) could easily break someone's
> > > favorite target here ...
> >
> > :-) I am happy you care about that. I think Glenn could tell you how to
>
> The more esoteric the target the more ... worrisome ? :-)

:-)

> > run "make check" for i386-ieee1275 target. Glenn?
>
> I just ran 'make check' (as user) for i386-pc on current master (and with the patches applied). The fallout was this:
>
> FAIL: squashfs_test    [blkid missing]
> FAIL: help_test        no particular helpful message
> FAIL: grub_func_test   only this type of error: tests/video_checksum.c:checksum:615: assert failed: 0 Checksum cmdline_cat_2560x1440xrgba8888:44 failed: 0x62031fea vs 0x8071678a
>
> Now on the same i386 machine with i386-ieee1275 (master): [after installing some more packages to be able to run the tests I can now build it fine]
>
> FAIL: squashfs_test
> FAIL: ahci_test
> FAIL: grub_script_eval
> FAIL: pata_test
> FAIL: example_grub_script_test
> FAIL: grub_script_leading_whitespace
> FAIL: ehci_test
> FAIL: ohci_test
> FAIL: grub_script_test
> FAIL: grub_script_echo1
> FAIL: uhci_test
> FAIL: grub_script_echo_keywords
> FAIL: grub_script_vars1
> FAIL: grub_script_for1
> FAIL: grub_script_while1
> FAIL: grub_script_if
> FAIL: grub_script_comments
> FAIL: grub_script_break
> FAIL: grub_script_return
> FAIL: grub_script_shift
> FAIL: grub_cmd_regexp
> FAIL: grub_script_functions
> FAIL: grub_script_blockarg
> FAIL: grub_script_continue
> FAIL: grub_script_setparams
> FAIL: grub_cmd_set_date
> FAIL: grub_cmd_date
> FAIL: grub_func_test
> FAIL: grub_cmd_sleep
> FAIL: grub_script_not
> FAIL: hddboot_test
> FAIL: fddboot_test
> FAIL: xzcompress_test
> FAIL: help_test
> FAIL: test_unset
> FAIL: grub_cmd_echo
> FAIL: grub_script_gettext
> FAIL: grub_script_escape_comma
> FAIL: test_sha512sum
> FAIL: grub_script_strcmp
> FAIL: grub_cmd_tr
> FAIL: file_filter_test
> FAIL: gzcompress_test
> FAIL: lzocompress_test
>
> So I take this as the baseline for me now -- interpretation is: i386-ieee1275 is borked.

Glenn? Any idea?

> > And I think you can find some hints how to run "make check" in the
> > INSTALL file too...
>
> I wished there was a line apt-get to install them all. Something like this seems a good base but still need qemu:
>
> sudo apt install dosfstools gzip lzop xz-utils attr cpio g++ gawk parted recode tar util-linux btrfs-progs dosfstools e2fsprogs f2fs-tools genromfs hfsprogs jfsutils nilfs-tools ntfs-3g reiserfsprogs squashfs-tools reiserfsprogs udftools xfsprogs zfs-fuse exfat-fuse

Feel free to add it to INSTALL file. Though before doing that please
drop "sudo", remove duplicates and sort the list alphabetically.

Daniel

Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Lennart Sorensen]

On Thu, Dec 01, 2022 at 03:47:30PM +0100, Daniel Kiper wrote:
> What do you mean by "supported"? It is build tested and, AIUI, all "make check"
> tests pass. Though I am not sure anybody uses this kinda weird target. Well,
> I think I knew but I forgot... :-)

I thought OLPC used openfirware on i386.  Are those things still around?

-- 
Len Sorensen

[PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2

[Diego Domingos]

Hello,

This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).

Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation
However, during some tests, we found this can be a problem if:

- we have more than 64 CPUs
- Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs)
- Grub needs to request memory.

If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum 
number of CPUs supported by the OS, causing the machine to fail to init.
Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value.

The patches 11-15 aren't merged to the grub tree yet, so I'm sending those patches again together with my patch to implement vector 5
on top of them.

The patches 11-15 contains the following:

Daniel Axtens (4):
  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

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