All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2
@ 2022-12-01 21:11 Stefan Berger
  2022-12-01 21:11 ` [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
                   ` (5 more replies)
  0 siblings, 6 replies; 23+ messages in thread
From: Stefan Berger @ 2022-12-01 21:11 UTC (permalink / raw)
  To: grub-devel, dkiper
  Cc: development, diegodo, dja, sudhakar, nasastry, Stefan Berger

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.

  Stefan

v2:
  - Followed Daniel K.'s list of suggestions checking the list more than
    twice and adjusted formatting, line breaks, etc. in all patches
  - Conditional compilation of ppc64-specific code using #ifdef __powerpc__
  - Fixing of installation of memtool; adding of warning message for
    __powerpc__ since it requires a reboot after running memory stress test
  - Tested it on i386-pc and PowerKVM

Daniel Axtens (4):
  ieee1275: request memory with ibm, client-architecture-support
  ieee1275: drop len -= 1 quirk in heap_init
  ieee1275: support runtime memory claiming
  Add memtool module with memory allocation stress-test

Diego Domingos (1):
  ieee1275: implement vec5 for cas negotiation

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

 configure.ac                          |   1 +
 docs/grub-dev.texi                    |   7 +-
 docs/grub.texi                        |   3 +-
 grub-core/Makefile.core.def           |  13 +
 grub-core/commands/ieee1275/ibmvtpm.c | 151 +++++++++
 grub-core/commands/memtools.c         | 160 ++++++++++
 grub-core/kern/ieee1275/cmain.c       |   3 +
 grub-core/kern/ieee1275/init.c        | 443 +++++++++++++++++++++++++-
 grub-core/kern/mm.c                   |   4 +
 include/grub/ieee1275/ieee1275.h      |  11 +
 10 files changed, 779 insertions(+), 17 deletions(-)
 create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c
 create mode 100644 grub-core/commands/memtools.c

-- 
2.25.1



^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support
  2022-12-01 21:11 [PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2 Stefan Berger
@ 2022-12-01 21:11 ` Stefan Berger
  2022-12-13 15:55   ` Daniel Kiper
  2022-12-13 16:09   ` Vladimir 'phcoder' Serbinenko
  2022-12-01 21:11 ` [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init Stefan Berger
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 23+ messages in thread
From: Stefan Berger @ 2022-12-01 21:11 UTC (permalink / raw)
  To: grub-devel, dkiper
  Cc: development, diegodo, dja, sudhakar, nasastry, Stefan Berger

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>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 grub-core/kern/ieee1275/cmain.c  |   3 +
 grub-core/kern/ieee1275/init.c   | 165 +++++++++++++++++++++++++++++++
 include/grub/ieee1275/ieee1275.h |   8 ++
 3 files changed, 176 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..0bc571e3e 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -200,11 +200,176 @@ 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'.  */
+  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");
+
+  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;
+}
+
+#if defined(__powerpc__)
+
+/* 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;
+} GRUB_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;
+} GRUB_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);
+}
+
+#endif /* __powerpc__ */
+
 static void
 grub_claim_heap (void)
 {
   unsigned long total = 0;
 
+#if defined(__powerpc__)
+  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 == GRUB_ERR_NONE && rma_size < (512 * 1024 * 1024))
+	grub_ieee1275_ibm_cas ();
+    }
+#endif
+
   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.25.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init
  2022-12-01 21:11 [PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2 Stefan Berger
  2022-12-01 21:11 ` [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
@ 2022-12-01 21:11 ` Stefan Berger
  2022-12-13 15:57   ` Daniel Kiper
                     ` (2 more replies)
  2022-12-01 21:11 ` [PATCH v2 3/6] ieee1275: support runtime memory claiming Stefan Berger
                   ` (3 subsequent siblings)
  5 siblings, 3 replies; 23+ messages in thread
From: Stefan Berger @ 2022-12-01 21:11 UTC (permalink / raw)
  To: grub-devel, dkiper; +Cc: development, diegodo, dja, sudhakar, nasastry

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 0bc571e3e..bb234b268 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.25.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 3/6] ieee1275: support runtime memory claiming
  2022-12-01 21:11 [PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2 Stefan Berger
  2022-12-01 21:11 ` [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
  2022-12-01 21:11 ` [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init Stefan Berger
@ 2022-12-01 21:11 ` Stefan Berger
  2022-12-13 16:14   ` Daniel Kiper
  2022-12-01 21:11 ` [PATCH v2 4/6] ieee1275: implement vec5 for cas negotiation Stefan Berger
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Stefan Berger @ 2022-12-01 21:11 UTC (permalink / raw)
  To: grub-devel, dkiper
  Cc: development, diegodo, dja, sudhakar, nasastry, Stefan Berger

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 | 268 ++++++++++++++++++++++++++++++---
 2 files changed, 255 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 bb234b268..e5fdc1fd2 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -45,13 +45,26 @@
 #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
 
+/* RMO max. address at 768 MB */
+#define RMO_ADDR_MAX		(grub_uint64_t) (768 * 1024 * 1024)
+
+/*
+ * 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 +158,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 +216,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 +227,108 @@ 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 (RMO_ADDR_MAX, rmo_top) - RUNTIME_MIN_SPACE;
+  if (rmo_top > RUNTIME_MIN_SPACE)
+    {
+      if (rmo_top <= RMO_ADDR_MAX)
+        {
+          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 < RMO_ADDR_MAX && (addr + len) > RMO_ADDR_MAX)
+            {
+              grub_dprintf ("ieee1275",
+                            "adjusting region for RUNTIME_MIN_SPACE: (%llx -> %llx) -> (%llx -> %llx)\n",
+                            addr, addr + len, RMO_ADDR_MAX, addr + len);
+              len = (addr + len) - RMO_ADDR_MAX;
+              addr = RMO_ADDR_MAX;
+            }
+          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) <= RMO_ADDR_MAX)
+            {
+              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 +337,93 @@ 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)
@@ -354,17 +579,24 @@ 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 defined(__powerpc__)
   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 == GRUB_ERR_NONE && rma_size < (512 * 1024 * 1024))
+      if (err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
 	grub_ieee1275_ibm_cas ();
     }
 #endif
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 4/6] ieee1275: implement vec5 for cas negotiation
  2022-12-01 21:11 [PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2 Stefan Berger
                   ` (2 preceding siblings ...)
  2022-12-01 21:11 ` [PATCH v2 3/6] ieee1275: support runtime memory claiming Stefan Berger
@ 2022-12-01 21:11 ` Stefan Berger
  2022-12-13 16:17   ` Daniel Kiper
  2022-12-01 21:12 ` [PATCH v2 5/6] Add memtool module with memory allocation stress-test Stefan Berger
  2022-12-01 21:12 ` [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0 Stefan Berger
  5 siblings, 1 reply; 23+ messages in thread
From: Stefan Berger @ 2022-12-01 21:11 UTC (permalink / raw)
  To: grub-devel, dkiper
  Cc: development, diegodo, dja, sudhakar, nasastry, Stefan Berger

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

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>
Acked-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Stefan Berger <stefanb@linux.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 e5fdc1fd2..870f89a54 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -498,6 +498,19 @@ struct option_vector2
   grub_uint8_t max_pft_size;
 } GRUB_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;
+} GRUB_PACKED;
+
 struct pvr_entry
 {
   grub_uint32_t mask;
@@ -519,6 +532,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;
 } GRUB_PACKED;
 
 /*
@@ -543,7 +558,7 @@ grub_ieee1275_ibm_cas (void)
   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,
@@ -554,6 +569,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.25.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 5/6] Add memtool module with memory allocation stress-test
  2022-12-01 21:11 [PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2 Stefan Berger
                   ` (3 preceding siblings ...)
  2022-12-01 21:11 ` [PATCH v2 4/6] ieee1275: implement vec5 for cas negotiation Stefan Berger
@ 2022-12-01 21:12 ` Stefan Berger
  2022-12-13 16:24   ` Daniel Kiper
  2022-12-01 21:12 ` [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0 Stefan Berger
  5 siblings, 1 reply; 23+ messages in thread
From: Stefan Berger @ 2022-12-01 21:12 UTC (permalink / raw)
  To: grub-devel, dkiper
  Cc: development, diegodo, dja, sudhakar, nasastry, Stefan Berger

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>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 configure.ac                  |   1 +
 grub-core/Makefile.core.def   |   6 ++
 grub-core/commands/memtools.c | 160 ++++++++++++++++++++++++++++++++++
 grub-core/kern/mm.c           |   4 +
 4 files changed, 171 insertions(+)
 create mode 100644 grub-core/commands/memtools.c

diff --git a/configure.ac b/configure.ac
index 93626b798..ca42ff8f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1520,6 +1520,7 @@ else
   MM_DEBUG=0
 fi
 AC_SUBST([MM_DEBUG])
+AM_CONDITIONAL([COND_MM_DEBUG], [test x$MM_DEBUG = x1])
 
 AC_ARG_ENABLE([cache-stats],
 	      AS_HELP_STRING([--enable-cache-stats],
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 95942fc8c..0ec95997d 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2542,3 +2542,9 @@ module = {
   common = commands/i386/wrmsr.c;
   enable = x86;
 };
+
+module = {
+  name = memtools;
+  common = commands/memtools.c;
+  condition = COND_MM_DEBUG;
+};
diff --git a/grub-core/commands/memtools.c b/grub-core/commands/memtools.c
new file mode 100644
index 000000000..3471e5941
--- /dev/null
+++ b/grub-core/commands/memtools.c
@@ -0,0 +1,160 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2022 Free Software Foundation, Inc.
+ *  Copyright (C) 2022 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 ("%4d MB . ", i);
+      mem = grub_malloc (i * 1024 * 1024);
+      if (mem == NULL)
+	{
+	  grub_printf ("failed\n");
+	  break;
+	}
+      else
+	grub_free (mem);
+
+      if (i % 7 == 0)
+	grub_printf ("\n");
+    }
+
+  max_mb = i - 1;
+  grub_printf ("\nMax sized allocation we did was %d MB\n", max_mb);
+
+  grub_printf ("\nTest 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] == NULL)
+	{
+	  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] == NULL)
+	{
+	  grub_printf ("Failed big allocation, iteration %d\n", i);
+	  blocks_alloced = i;
+	  break;
+	}
+
+      blocklist[i + 1] = grub_malloc (100 * 1024);
+      if (blocklist[i + 1] == NULL)
+	{
+	  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);
+
+#if defined(__powerpc__)
+  grub_printf ("\nA reboot may now be required.\n");
+#endif
+
+  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 ae2279133..319e74a09 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.25.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0
  2022-12-01 21:11 [PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2 Stefan Berger
                   ` (4 preceding siblings ...)
  2022-12-01 21:12 ` [PATCH v2 5/6] Add memtool module with memory allocation stress-test Stefan Berger
@ 2022-12-01 21:12 ` Stefan Berger
  2022-12-13 16:35   ` Daniel Kiper
  5 siblings, 1 reply; 23+ messages in thread
From: Stefan Berger @ 2022-12-01 21:12 UTC (permalink / raw)
  To: grub-devel, dkiper
  Cc: development, diegodo, dja, sudhakar, nasastry, Stefan Berger,
	Eric Snowberg

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 | 151 ++++++++++++++++++++++++++
 include/grub/ieee1275/ieee1275.h      |   3 +
 4 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c

diff --git a/docs/grub.texi b/docs/grub.texi
index 50c811a88..580f2a995 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -6413,7 +6413,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 0ec95997d..ef6964b91 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1133,6 +1133,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..78e002c47
--- /dev/null
+++ b/grub-core/commands/ieee1275/ibmvtpm.c
@@ -0,0 +1,151 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2022  Free Software Foundation, Inc.
+ *  Copyright (C) 2022  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;
+  };
+  struct tpm_2hash_ext_log 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 rc;
+
+  rc = ibmvtpm_2hash_ext_log (pcr, EV_IPL,
+			      description, grub_strlen(description) + 1,
+			      buf, size);
+  if (rc && !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.25.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support
  2022-12-01 21:11 ` [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
@ 2022-12-13 15:55   ` Daniel Kiper
  2022-12-13 16:09   ` Vladimir 'phcoder' Serbinenko
  1 sibling, 0 replies; 23+ messages in thread
From: Daniel Kiper @ 2022-12-13 15:55 UTC (permalink / raw)
  To: Stefan Berger; +Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry

On Thu, Dec 01, 2022 at 04:11:56PM -0500, Stefan Berger wrote:
> 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>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  grub-core/kern/ieee1275/cmain.c  |   3 +
>  grub-core/kern/ieee1275/init.c   | 165 +++++++++++++++++++++++++++++++
>  include/grub/ieee1275/ieee1275.h |   8 ++
>  3 files changed, 176 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 defined(__powerpc__) ?

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

#endif

>      }
>
>    if (is_smartfirmware)
> diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
> index 2adf4fdfc..0bc571e3e 100644
> --- a/grub-core/kern/ieee1275/init.c
> +++ b/grub-core/kern/ieee1275/init.c
> @@ -200,11 +200,176 @@ 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'.  */
> +  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");
> +
> +  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;
> +}
> +
> +#if defined(__powerpc__)
> +
> +/* 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;
> +} GRUB_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;
> +} GRUB_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);
> +}
> +
> +#endif /* __powerpc__ */
> +
>  static void
>  grub_claim_heap (void)
>  {
>    unsigned long total = 0;
>
> +#if defined(__powerpc__)
> +  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 == GRUB_ERR_NONE && rma_size < (512 * 1024 * 1024))
> +	grub_ieee1275_ibm_cas ();
> +    }
> +#endif
> +
>    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.
> +   */

#if defined(__powerpc__) ?

> +  GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY,

#endif

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

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init
  2022-12-01 21:11 ` [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init Stefan Berger
@ 2022-12-13 15:57   ` Daniel Kiper
  2022-12-13 16:07   ` Vladimir 'phcoder' Serbinenko
  2022-12-13 16:11   ` John Paul Adrian Glaubitz
  2 siblings, 0 replies; 23+ messages in thread
From: Daniel Kiper @ 2022-12-13 15:57 UTC (permalink / raw)
  To: Stefan Berger; +Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry

On Thu, Dec 01, 2022 at 04:11:57PM -0500, Stefan Berger wrote:
> 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>

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

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init
  2022-12-01 21:11 ` [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init Stefan Berger
  2022-12-13 15:57   ` Daniel Kiper
@ 2022-12-13 16:07   ` Vladimir 'phcoder' Serbinenko
  2022-12-13 16:11   ` John Paul Adrian Glaubitz
  2 siblings, 0 replies; 23+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2022-12-13 16:07 UTC (permalink / raw)
  To: The development of GNU GRUB

What problem are you trying to solve? Is losing 32 bytes a problem?

On Thu, Dec 1, 2022 at 10:13 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
> 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 0bc571e3e..bb234b268 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.25.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support
  2022-12-01 21:11 ` [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
  2022-12-13 15:55   ` Daniel Kiper
@ 2022-12-13 16:09   ` Vladimir 'phcoder' Serbinenko
  1 sibling, 0 replies; 23+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2022-12-13 16:09 UTC (permalink / raw)
  To: The development of GNU GRUB

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

Does it work on all IBM? IBM has a long history of PPC machines. What
happens when it doesn't?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init
  2022-12-01 21:11 ` [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init Stefan Berger
  2022-12-13 15:57   ` Daniel Kiper
  2022-12-13 16:07   ` Vladimir 'phcoder' Serbinenko
@ 2022-12-13 16:11   ` John Paul Adrian Glaubitz
  2 siblings, 0 replies; 23+ messages in thread
From: John Paul Adrian Glaubitz @ 2022-12-13 16:11 UTC (permalink / raw)
  To: The development of GNU GRUB, Stefan Berger, dkiper
  Cc: development, diegodo, dja, sudhakar, nasastry

On 12/1/22 22:11, Stefan Berger wrote:
> 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.

I can test this on my iBook G4 but not this week as I am currently not at home.

Adrian

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/6] ieee1275: support runtime memory claiming
  2022-12-01 21:11 ` [PATCH v2 3/6] ieee1275: support runtime memory claiming Stefan Berger
@ 2022-12-13 16:14   ` Daniel Kiper
  2022-12-13 19:10     ` Stefan Berger
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Kiper @ 2022-12-13 16:14 UTC (permalink / raw)
  To: Stefan Berger; +Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry

On Thu, Dec 01, 2022 at 04:11:58PM -0500, Stefan Berger wrote:
> From: Daniel Axtens <dja@axtens.net>
>
> On powerpc-ieee1275, we are running out of memory trying to verify
> anything. This is because:
>
>  - we have to load an entire file into memory to verify it. This is
>    difficult to change with appended signatures.
>  - We only have 32MB of heap.
>  - Distro kernels are now often around 30MB.
>
> So we want to be able to claim more memory from OpenFirmware for our heap
> at runtime.
>
> There are some complications:
>
>  - The grub mm code isn't the only thing that will make claims on
>    memory from OpenFirmware:
>
>     * PFW/SLOF will have claimed some for their own use.
>
>     * The ieee1275 loader will try to find other bits of memory that we
>       haven't claimed to place the kernel and initrd when we go to boot.
>
>     * Once we load Linux, it will also try to claim memory. It claims
>       memory without any reference to /memory/available, it just starts
>       at min(top of RMO, 768MB) and works down. So we need to avoid this
>       area. See arch/powerpc/kernel/prom_init.c as of v5.11.
>
>  - The smallest amount of memory a ppc64 KVM guest can have is 256MB.
>    It doesn't work with distro kernels but can work with custom kernels.
>    We should maintain support for that. (ppc32 can boot with even less,
>    and we shouldn't break that either.)
>
>  - Even if a VM has more memory, the memory OpenFirmware makes available
>    as Real Memory Area can be restricted. Even with our CAS work, an LPAR
>    on a PowerVM box is likely to have only 512MB available to OpenFirmware
>    even if it has many gigabytes of memory allocated.
>
> What should we do?
>
> We don't know in advance how big the kernel and initrd are going to be,
> which makes figuring out how much memory we can take a bit tricky.
>
> To figure out how much memory we should leave unused, I looked at:
>
>  - an Ubuntu 20.04.1 ppc64le pseries KVM guest:
>     vmlinux: ~30MB
>     initrd:  ~50MB
>
>  - a RHEL8.2 ppc64le pseries KVM guest:
>     vmlinux: ~30MB
>     initrd:  ~30MB
>
> So to give us a little wriggle room, I think we want to leave at least
> 128MB for the loader to put vmlinux and initrd in memory and leave Linux
> with space to satisfy its early allocations.
>
> Allow other space to be allocated at runtime.
>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

[...]

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

static grub_err_t
grub_ieee1275_mm_add_region (grub_size_t size, unsigned int flags)

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

s/ALIGN_UP(/ALIGN_UP (/

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

This...

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

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

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

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

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

Daniel

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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 4/6] ieee1275: implement vec5 for cas negotiation
  2022-12-01 21:11 ` [PATCH v2 4/6] ieee1275: implement vec5 for cas negotiation Stefan Berger
@ 2022-12-13 16:17   ` Daniel Kiper
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Kiper @ 2022-12-13 16:17 UTC (permalink / raw)
  To: Stefan Berger; +Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry

On Thu, Dec 01, 2022 at 04:11:59PM -0500, Stefan Berger wrote:
> From: Diego Domingos <diegodo@linux.vnet.ibm.com>
>
> 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>
> Acked-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

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

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 5/6] Add memtool module with memory allocation stress-test
  2022-12-01 21:12 ` [PATCH v2 5/6] Add memtool module with memory allocation stress-test Stefan Berger
@ 2022-12-13 16:24   ` Daniel Kiper
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Kiper @ 2022-12-13 16:24 UTC (permalink / raw)
  To: Stefan Berger; +Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry

On Thu, Dec 01, 2022 at 04:12:00PM -0500, Stefan Berger 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:
>
>  * 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>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

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

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0
  2022-12-01 21:12 ` [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0 Stefan Berger
@ 2022-12-13 16:35   ` Daniel Kiper
  2022-12-13 18:18     ` Stefan Berger
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Kiper @ 2022-12-13 16:35 UTC (permalink / raw)
  To: Stefan Berger
  Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry, Eric Snowberg

On Thu, Dec 01, 2022 at 04:12:01PM -0500, Stefan Berger wrote:
> 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 | 151 ++++++++++++++++++++++++++
>  include/grub/ieee1275/ieee1275.h      |   3 +
>  4 files changed, 163 insertions(+), 1 deletion(-)
>  create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 50c811a88..580f2a995 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -6413,7 +6413,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 0ec95997d..ef6964b91 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1133,6 +1133,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..78e002c47
> --- /dev/null
> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
> @@ -0,0 +1,151 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2022  Free Software Foundation, Inc.
> + *  Copyright (C) 2022  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;
> +  };
> +  struct tpm_2hash_ext_log 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 rc;
> +
> +  rc = ibmvtpm_2hash_ext_log (pcr, EV_IPL,
> +			      description, grub_strlen(description) + 1,
> +			      buf, size);
> +  if (rc && !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 ();

This should happen on module load. Then code in tpm_init() and here should
be much simpler.

> +  /* 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;
> +}

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0
  2022-12-13 16:35   ` Daniel Kiper
@ 2022-12-13 18:18     ` Stefan Berger
  2022-12-14 14:20       ` Daniel Kiper
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Berger @ 2022-12-13 18:18 UTC (permalink / raw)
  To: Daniel Kiper
  Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry, Eric Snowberg



On 12/13/22 11:35, Daniel Kiper wrote:
> On Thu, Dec 01, 2022 at 04:12:01PM -0500, Stefan Berger wrote:
>> 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.
>>

>> +}
>> +
>> +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 ();
> 
> This should happen on module load. Then code in tpm_init() and here should
> be much simpler.

I tried moving this into GRUB_MOD_INIT() but at that point it doesn't succeed to find the device it seems. I have to repeat the tpm_init () call then later on in this function here when an actual measurement is to be done and then it is able to find the device. I'd rather leave it as-is now.

It looked like this:

GRUB_MOD_INIT (ibmvtpm)
{
   init_success = tpm_init ();
}

GRUB_MOD_FINI (ibmvtpm)
{
   if (tpm_ihandle != IEEE1275_IHANDLE_INVALID)
     grub_ieee1275_close (tpm_ihandle);
}


    Stefan

> 
>> +  /* 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;
>> +}
> 
> Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/6] ieee1275: support runtime memory claiming
  2022-12-13 16:14   ` Daniel Kiper
@ 2022-12-13 19:10     ` Stefan Berger
  2022-12-14 14:37       ` Daniel Kiper
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Berger @ 2022-12-13 19:10 UTC (permalink / raw)
  To: The development of GNU GRUB, Daniel Kiper
  Cc: development, diegodo, dja, sudhakar, nasastry



On 12/13/22 11:14, Daniel Kiper wrote:
> On Thu, Dec 01, 2022 at 04:11:58PM -0500, Stefan Berger wrote:
>> From: Daniel Axtens <dja@axtens.net>
>>
>> On powerpc-ieee1275, we are running out of memory trying to verify
>> anything. This is because:
>>
>>   - we have to load an entire file into memory to verify it. This is
>>     difficult to change with appended signatures.
>>   - We only have 32MB of heap.
>>   - Distro kernels are now often around 30MB.
>>

>> +
>> +  if (flags & GRUB_MM_ADD_REGION_CONSECUTIVE)
>> +    {
>> +      /* first try rounding up hard for the sake of speed */
>> +      total = grub_max (ALIGN_UP(size, 1024 * 1024) + 1024 * 1024, 32 * 1024 * 1024);
> 
> s/ALIGN_UP(/ALIGN_UP (/
> 
>> +      total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
>> +
>> +      grub_dprintf ("ieee1275", "looking for %x bytes of memory (%x requested)\n", total, size);
>> +
>> +      grub_machine_mmap_iterate (region_claim, &total);
>> +      grub_dprintf ("ieee1275", "get memory from fw %s\n", total == 0 ? "succeeded" : "failed");
>> +
>> +      /* fallback: requested size + padding for a grub_mm_header_t and region */
> 
> This...

With 'this' you mean the comment above or which exact part?

> 
>> +      if (total != 0)
>> +        {
>> +          total = size + 48;
> 
> ... and this should be dropped. I think this patch set, [1], [2], solves

You mean adding 48 to it can be dropped?

> this kinds of problems in general. Please take a look...

I looked at them and I don't know how they are related.


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

Don't we need a parameter to grub_machine_mmap_iterate (region_claim, &total) ?


    Stefan
> 
>> +      total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
>> +      grub_machine_mmap_iterate (heap_init, &total);
>> +      grub_dprintf ("ieee1275", "get noncontig memory from fw %s\n", total == 0 ? "succeeded" : "failed");
>> +    }
>> +
>> +  if (total == 0)
>> +    return GRUB_ERR_NONE;
>> +  else
>> +    return GRUB_ERR_OUT_OF_MEMORY;
>> +}
> 
> Daniel
> 
> [1] https://lists.gnu.org/archive/html/grub-devel/2022-11/msg00147.html
> [2] https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00115.html
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0
  2022-12-13 18:18     ` Stefan Berger
@ 2022-12-14 14:20       ` Daniel Kiper
  2022-12-15  3:05         ` Stefan Berger
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Kiper @ 2022-12-14 14:20 UTC (permalink / raw)
  To: Stefan Berger
  Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry, Eric Snowberg

On Tue, Dec 13, 2022 at 01:18:34PM -0500, Stefan Berger wrote:
> On 12/13/22 11:35, Daniel Kiper wrote:
> > On Thu, Dec 01, 2022 at 04:12:01PM -0500, Stefan Berger wrote:
> > > 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.
> > > +}
> > > +
> > > +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 ();
> >
> > This should happen on module load. Then code in tpm_init() and here should
> > be much simpler.
>
> I tried moving this into GRUB_MOD_INIT() but at that point it doesn't
> succeed to find the device it seems. I have to repeat the tpm_init ()
> call then later on in this function here when an actual measurement is
> to be done and then it is able to find the device. I'd rather leave it
> as-is now.

Hmmm... It looks like a bug in a firmware or initial TPM communication
fails/timeouts for some reason. Do not you loose some initial
measurements this way? Could you check what will happen when you call
tpm_init() from GRUB_MOD_INIT 10, 50, 100 times?

> It looked like this:
>
> GRUB_MOD_INIT (ibmvtpm)
> {
>   init_success = tpm_init ();
> }
>
> GRUB_MOD_FINI (ibmvtpm)
> {
>   if (tpm_ihandle != IEEE1275_IHANDLE_INVALID)
>     grub_ieee1275_close (tpm_ihandle);
> }

LGTM... Weird...

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/6] ieee1275: support runtime memory claiming
  2022-12-13 19:10     ` Stefan Berger
@ 2022-12-14 14:37       ` Daniel Kiper
  2022-12-15 12:09         ` Zhang Boyang
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Kiper @ 2022-12-14 14:37 UTC (permalink / raw)
  To: Stefan Berger
  Cc: The development of GNU GRUB, development, diegodo, dja, sudhakar,
	nasastry, zhangboyang.id

Adding Zhang...

On Tue, Dec 13, 2022 at 02:10:01PM -0500, Stefan Berger wrote:
> On 12/13/22 11:14, Daniel Kiper wrote:
> > On Thu, Dec 01, 2022 at 04:11:58PM -0500, Stefan Berger wrote:
> > > From: Daniel Axtens <dja@axtens.net>
> > >
> > > On powerpc-ieee1275, we are running out of memory trying to verify
> > > anything. This is because:
> > >
> > >   - we have to load an entire file into memory to verify it. This is
> > >     difficult to change with appended signatures.
> > >   - We only have 32MB of heap.
> > >   - Distro kernels are now often around 30MB.
> > >
>
> > > +
> > > +  if (flags & GRUB_MM_ADD_REGION_CONSECUTIVE)
> > > +    {
> > > +      /* first try rounding up hard for the sake of speed */
> > > +      total = grub_max (ALIGN_UP(size, 1024 * 1024) + 1024 * 1024, 32 * 1024 * 1024);
> >
> > s/ALIGN_UP(/ALIGN_UP (/
> >
> > > +      total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
> > > +
> > > +      grub_dprintf ("ieee1275", "looking for %x bytes of memory (%x requested)\n", total, size);
> > > +
> > > +      grub_machine_mmap_iterate (region_claim, &total);
> > > +      grub_dprintf ("ieee1275", "get memory from fw %s\n", total == 0 ? "succeeded" : "failed");
> > > +
> > > +      /* fallback: requested size + padding for a grub_mm_header_t and region */
> >
> > This...
>
> With 'this' you mean the comment above or which exact part?

Yes, exactly...

> > > +      if (total != 0)
> > > +        {
> > > +          total = size + 48;
> >
> > ... and this should be dropped. I think this patch set, [1], [2], solves
>
> You mean adding 48 to it can be dropped?

Yep... Sorry for being imprecise...

> > this kinds of problems in general. Please take a look...
>
> I looked at them and I don't know how they are related.

If I do not miss anything your patch set does not take into account size
needed for *grub_mm_region_t and *grub_mm_header structs. They are
needed for mm mgmt. So, grub_mm_add_region_fn() can be called twice to
fulfill request or memory addition may fail. The "mm: Preallocate some
space when adding new regions" patch extends the region size further to
not call firmware too often to fulfill small allocations. Due to that
I think your series should be merged into master together with Zhang one.
I hope he will repost updated mm series soon.

> > > +          total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
> > > +
> > > +          grub_dprintf ("ieee1275", "fallback for %x bytes of memory (%x requested)\n", total, size);
> > > +
> > > +          grub_machine_mmap_iterate (region_claim, &total);
> > > +          grub_dprintf ("ieee1275", "fallback from fw %s\n", total == 0 ? "succeeded" : "failed");
> > > +        }
> > > +    }
> > > +  else
> > > +    {
> > > +      /* provide padding for a grub_mm_header_t and region */
> > > +      total = size + 48;
> >
> > Ditto... And then probably total can be dropped too...
>
> Don't we need a parameter to grub_machine_mmap_iterate (region_claim, &total) ?

I think you can use size variable instead of total then.

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0
  2022-12-14 14:20       ` Daniel Kiper
@ 2022-12-15  3:05         ` Stefan Berger
  2022-12-15 19:31           ` Daniel Kiper
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Berger @ 2022-12-15  3:05 UTC (permalink / raw)
  To: Daniel Kiper
  Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry, Eric Snowberg



On 12/14/22 09:20, Daniel Kiper wrote:
> On Tue, Dec 13, 2022 at 01:18:34PM -0500, Stefan Berger wrote:
>> On 12/13/22 11:35, Daniel Kiper wrote:
>>> On Thu, Dec 01, 2022 at 04:12:01PM -0500, Stefan Berger wrote:
>>>> 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.
>>>> +}
>>>> +
>>>> +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 ();
>>>
>>> This should happen on module load. Then code in tpm_init() and here should
>>> be much simpler.
>>
>> I tried moving this into GRUB_MOD_INIT() but at that point it doesn't
>> succeed to find the device it seems. I have to repeat the tpm_init ()
>> call then later on in this function here when an actual measurement is
>> to be done and then it is able to find the device. I'd rather leave it
>> as-is now.
> 
> Hmmm... It looks like a bug in a firmware or initial TPM communication
> fails/timeouts for some reason. Do not you loose some initial

tpm_init() , which also calls tpm_get_tpm_version(), don't even talk to the TPM but they only access the OpenFirmware device tree and don't seem to find what they are looking for when run that early.

> measurements this way? Could you check what will happen when you call

No measurements are lost from what I can tell. The late initialization seems to find the OF node it needs.



> tpm_init() from GRUB_MOD_INIT 10, 50, 100 times?

I tried it now with this loop here. Well, not unexpected but the loop doesn't change anything in this case since no timing is involved -- 'found', which I print later on, stays at 0. Something related to the OpenFirmware tree seems to not be initialized at this point.

GRUB_MOD_INIT (ibmvtpm)
{
   grub_ieee1275_phandle_t vtpm;
   int i;

   for (i = 0; i < 100; i++) {
      if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm))
          found++;
   }
}

     Stefan

> 
>> It looked like this:
>>
>> GRUB_MOD_INIT (ibmvtpm)
>> {
>>    init_success = tpm_init ();
>> }
>>
>> GRUB_MOD_FINI (ibmvtpm)
>> {
>>    if (tpm_ihandle != IEEE1275_IHANDLE_INVALID)
>>      grub_ieee1275_close (tpm_ihandle);
>> }
> 
> LGTM... Weird...
> 
> Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 3/6] ieee1275: support runtime memory claiming
  2022-12-14 14:37       ` Daniel Kiper
@ 2022-12-15 12:09         ` Zhang Boyang
  0 siblings, 0 replies; 23+ messages in thread
From: Zhang Boyang @ 2022-12-15 12:09 UTC (permalink / raw)
  To: Daniel Kiper, Stefan Berger
  Cc: The development of GNU GRUB, development, diegodo, dja, sudhakar,
	nasastry

Hi,

On 2022/12/14 22:37, Daniel Kiper wrote:
> Adding Zhang...
> 
> On Tue, Dec 13, 2022 at 02:10:01PM -0500, Stefan Berger wrote:
>> On 12/13/22 11:14, Daniel Kiper wrote:
>>> On Thu, Dec 01, 2022 at 04:11:58PM -0500, Stefan Berger wrote:
>>>> From: Daniel Axtens <dja@axtens.net>
>>>>
>>>> On powerpc-ieee1275, we are running out of memory trying to verify
>>>> anything. This is because:
>>>>
>>>>    - we have to load an entire file into memory to verify it. This is
>>>>      difficult to change with appended signatures.
>>>>    - We only have 32MB of heap.
>>>>    - Distro kernels are now often around 30MB.
>>>>
>>
>>>> +
>>>> +  if (flags & GRUB_MM_ADD_REGION_CONSECUTIVE)
>>>> +    {
>>>> +      /* first try rounding up hard for the sake of speed */
>>>> +      total = grub_max (ALIGN_UP(size, 1024 * 1024) + 1024 * 1024, 32 * 1024 * 1024);
>>>
>>> s/ALIGN_UP(/ALIGN_UP (/
>>>
>>>> +      total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
>>>> +
>>>> +      grub_dprintf ("ieee1275", "looking for %x bytes of memory (%x requested)\n", total, size);
>>>> +
>>>> +      grub_machine_mmap_iterate (region_claim, &total);
>>>> +      grub_dprintf ("ieee1275", "get memory from fw %s\n", total == 0 ? "succeeded" : "failed");
>>>> +
>>>> +      /* fallback: requested size + padding for a grub_mm_header_t and region */
>>>
>>> This...
>>
>> With 'this' you mean the comment above or which exact part?
> 
> Yes, exactly...
> 
>>>> +      if (total != 0)
>>>> +        {
>>>> +          total = size + 48;
>>>
>>> ... and this should be dropped. I think this patch set, [1], [2], solves
>>
>> You mean adding 48 to it can be dropped?
> 
> Yep... Sorry for being imprecise...
> 
>>> this kinds of problems in general. Please take a look...
>>
>> I looked at them and I don't know how they are related.
> 
> If I do not miss anything your patch set does not take into account size
> needed for *grub_mm_region_t and *grub_mm_header structs. They are
> needed for mm mgmt. So, grub_mm_add_region_fn() can be called twice to
> fulfill request or memory addition may fail. The "mm: Preallocate some
> space when adding new regions" patch extends the region size further to
> not call firmware too often to fulfill small allocations. Due to that
> I think your series should be merged into master together with Zhang one.
> I hope he will repost updated mm series soon.
> 

I also think 48 can be removed, if 48 means 
sizeof(grub_mm_region)+sizeof(grub_mm_header). With my patchset, the 
"size" passed to grub_mm_add_region_fn() will include these overhead and 
there is no need to add them in arch specific code.

Best Regards,
Zhang Boyang

>>>> +          total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
>>>> +
>>>> +          grub_dprintf ("ieee1275", "fallback for %x bytes of memory (%x requested)\n", total, size);
>>>> +
>>>> +          grub_machine_mmap_iterate (region_claim, &total);
>>>> +          grub_dprintf ("ieee1275", "fallback from fw %s\n", total == 0 ? "succeeded" : "failed");
>>>> +        }
>>>> +    }
>>>> +  else
>>>> +    {
>>>> +      /* provide padding for a grub_mm_header_t and region */
>>>> +      total = size + 48;
>>>
>>> Ditto... And then probably total can be dropped too...
>>
>> Don't we need a parameter to grub_machine_mmap_iterate (region_claim, &total) ?
> 
> I think you can use size variable instead of total then.
> 
> Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0
  2022-12-15  3:05         ` Stefan Berger
@ 2022-12-15 19:31           ` Daniel Kiper
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Kiper @ 2022-12-15 19:31 UTC (permalink / raw)
  To: Stefan Berger
  Cc: grub-devel, development, diegodo, dja, sudhakar, nasastry, Eric Snowberg

On Wed, Dec 14, 2022 at 10:05:16PM -0500, Stefan Berger wrote:
> On 12/14/22 09:20, Daniel Kiper wrote:
> > On Tue, Dec 13, 2022 at 01:18:34PM -0500, Stefan Berger wrote:
> > > On 12/13/22 11:35, Daniel Kiper wrote:
> > > > On Thu, Dec 01, 2022 at 04:12:01PM -0500, Stefan Berger wrote:
> > > > > 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.
> > > > > +}
> > > > > +
> > > > > +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 ();
> > > >
> > > > This should happen on module load. Then code in tpm_init() and here should
> > > > be much simpler.
> > >
> > > I tried moving this into GRUB_MOD_INIT() but at that point it doesn't
> > > succeed to find the device it seems. I have to repeat the tpm_init ()
> > > call then later on in this function here when an actual measurement is
> > > to be done and then it is able to find the device. I'd rather leave it
> > > as-is now.
> >
> > Hmmm... It looks like a bug in a firmware or initial TPM communication
> > fails/timeouts for some reason. Do not you loose some initial
>
> tpm_init() , which also calls tpm_get_tpm_version(), don't even talk
> to the TPM but they only access the OpenFirmware device tree and don't
> seem to find what they are looking for when run that early.
>
> > measurements this way? Could you check what will happen when you call
>
> No measurements are lost from what I can tell. The late initialization seems to find the OF node it needs.
>
> > tpm_init() from GRUB_MOD_INIT 10, 50, 100 times?
>
> I tried it now with this loop here. Well, not unexpected but the loop
> doesn't change anything in this case since no timing is involved --
> 'found', which I print later on, stays at 0. Something related to the
> OpenFirmware tree seems to not be initialized at this point.
>
> GRUB_MOD_INIT (ibmvtpm)
> {
>   grub_ieee1275_phandle_t vtpm;
>   int i;
>
>   for (i = 0; i < 100; i++) {
>      if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm))
>          found++;
>   }
> }

Huh! Seems like a bug. Of course I would prefer GRUB_MOD_INIT() approach
but as I can see EFI version does not have it either. So, due to the
potential bug in the firmware and to be inline with existing EFI code
let's leave it as is. Though please add a sentence or two to the commit
message and make a comment before tpm_init() why we cannot use
GRUB_MOD_INIT() approach (due to the bug). If you do that feel free to
add my RB to this patch.

Daniel


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2022-12-15 19:31 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 21:11 [PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2 Stefan Berger
2022-12-01 21:11 ` [PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
2022-12-13 15:55   ` Daniel Kiper
2022-12-13 16:09   ` Vladimir 'phcoder' Serbinenko
2022-12-01 21:11 ` [PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init Stefan Berger
2022-12-13 15:57   ` Daniel Kiper
2022-12-13 16:07   ` Vladimir 'phcoder' Serbinenko
2022-12-13 16:11   ` John Paul Adrian Glaubitz
2022-12-01 21:11 ` [PATCH v2 3/6] ieee1275: support runtime memory claiming Stefan Berger
2022-12-13 16:14   ` Daniel Kiper
2022-12-13 19:10     ` Stefan Berger
2022-12-14 14:37       ` Daniel Kiper
2022-12-15 12:09         ` Zhang Boyang
2022-12-01 21:11 ` [PATCH v2 4/6] ieee1275: implement vec5 for cas negotiation Stefan Berger
2022-12-13 16:17   ` Daniel Kiper
2022-12-01 21:12 ` [PATCH v2 5/6] Add memtool module with memory allocation stress-test Stefan Berger
2022-12-13 16:24   ` Daniel Kiper
2022-12-01 21:12 ` [PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0 Stefan Berger
2022-12-13 16:35   ` Daniel Kiper
2022-12-13 18:18     ` Stefan Berger
2022-12-14 14:20       ` Daniel Kiper
2022-12-15  3:05         ` Stefan Berger
2022-12-15 19:31           ` Daniel Kiper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.