All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Add support for trusted boot on IBM PPC platform
@ 2021-07-30 15:45 Stefan Berger
  2021-07-30 15:45 ` [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header Stefan Berger
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Stefan Berger @ 2021-07-30 15:45 UTC (permalink / raw)
  To: grub-devel; +Cc: dkiper, Stefan Berger

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

This series of patches adds support for trusted boot using vTPM on the
IBM IEEE1275 PowerPC platform.

  Stefan

v3:
  - Rebased on latest master
  - Moving #defines from ieee1275.c to ieee1275.h
  - More refactoring in patch 4

v2:
  - Prepended Daniel's patches to claim more memory on IBM platform
  - Added documentation to vTPM patch and major refactoring following
    Daniels' review

Daniel Axtens (2):
  ieee1275: claim more memory
  ieee1275: request memory with ibm,client-architecture-support

Stefan Berger (2):
  ieee1275: Move #defines into common ieee1275.h header
  ibmvtpm: Add support for trusted boot using a vTPM 2.0

 docs/grub-dev.texi                    |   6 +-
 docs/grub.texi                        |   3 +-
 grub-core/Makefile.core.def           |   7 +
 grub-core/commands/ieee1275/ibmvtpm.c | 160 +++++++++++++++++++
 grub-core/kern/ieee1275/cmain.c       |   3 +
 grub-core/kern/ieee1275/ieee1275.c    |  29 ++--
 grub-core/kern/ieee1275/init.c        | 221 ++++++++++++++++++++++++--
 include/grub/ieee1275/ieee1275.h      |  10 ++
 8 files changed, 403 insertions(+), 36 deletions(-)
 create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c

-- 
2.25.1



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

* [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header
  2021-07-30 15:45 [PATCH v3 0/4] Add support for trusted boot on IBM PPC platform Stefan Berger
@ 2021-07-30 15:45 ` Stefan Berger
  2021-08-04 10:42   ` Daniel Kiper
  2021-08-05 22:22   ` Stefan Berger
  2021-07-30 15:45 ` [PATCH v3 2/4] ieee1275: claim more memory Stefan Berger
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 14+ messages in thread
From: Stefan Berger @ 2021-07-30 15:45 UTC (permalink / raw)
  To: grub-devel; +Cc: dkiper, Stefan Berger

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

Move some #defines from ieee1275.c into the common ieee1275.h
header file. Adjust the case used in IHANDLE_INVALID to use
proper ihandle_t.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 grub-core/kern/ieee1275/ieee1275.c | 29 ++++++++++++-----------------
 include/grub/ieee1275/ieee1275.h   |  3 +++
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/grub-core/kern/ieee1275/ieee1275.c b/grub-core/kern/ieee1275/ieee1275.c
index 86f81a3c4..8fe92274d 100644
--- a/grub-core/kern/ieee1275/ieee1275.c
+++ b/grub-core/kern/ieee1275/ieee1275.c
@@ -21,11 +21,6 @@
 #include <grub/types.h>
 #include <grub/misc.h>
 
-#define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_cell_t) -1)
-#define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_cell_t) 0)
-#define IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
-
-\f
 
 int
 grub_ieee1275_finddevice (const char *name, grub_ieee1275_phandle_t *phandlep)
@@ -44,8 +39,7 @@ grub_ieee1275_finddevice (const char *name, grub_ieee1275_phandle_t *phandlep)
   if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
     return -1;
   *phandlep = args.phandle;
-  if (args.phandle == IEEE1275_PHANDLE_INVALID)
-    return -1;
+  if (args.phandle == GRUB_IEEE1275_PHANDLE_INVALID) return -1;
   return 0;
 }
 
@@ -75,7 +69,7 @@ grub_ieee1275_get_property (grub_ieee1275_phandle_t phandle,
     return -1;
   if (actual)
     *actual = (grub_ssize_t) args.size;
-  if (args.size == IEEE1275_CELL_INVALID)
+  if (args.size == GRUB_IEEE1275_CELL_INVALID)
     return -1;
   return 0;
 }
@@ -146,7 +140,7 @@ grub_ieee1275_get_property_length (grub_ieee1275_phandle_t phandle,
   if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
     return -1;
   *length = args.length;
-  if (args.length == IEEE1275_CELL_INVALID)
+  if (args.length == GRUB_IEEE1275_CELL_INVALID)
     return -1;
   return 0;
 }
@@ -169,7 +163,7 @@ grub_ieee1275_instance_to_package (grub_ieee1275_ihandle_t ihandle,
   if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
     return -1;
   *phandlep = args.phandle;
-  if (args.phandle == IEEE1275_PHANDLE_INVALID)
+  if (args.phandle == GRUB_IEEE1275_PHANDLE_INVALID)
     return -1;
   return 0;
 }
@@ -198,7 +192,7 @@ grub_ieee1275_package_to_path (grub_ieee1275_phandle_t phandle,
     return -1;
   if (actual)
     *actual = args.actual;
-  if (args.actual == IEEE1275_CELL_INVALID)
+  if (args.actual == GRUB_IEEE1275_CELL_INVALID)
     return -1;
   return 0;
 }
@@ -227,7 +221,7 @@ grub_ieee1275_instance_to_path (grub_ieee1275_ihandle_t ihandle,
     return -1;
   if (actual)
     *actual = args.actual;
-  if (args.actual == IEEE1275_CELL_INVALID)
+  if (args.actual == GRUB_IEEE1275_CELL_INVALID)
     return -1;
   return 0;
 }
@@ -355,7 +349,7 @@ grub_ieee1275_child (grub_ieee1275_phandle_t node,
 
   INIT_IEEE1275_COMMON (&args.common, "child", 1, 1);
   args.node = node;
-  args.result = IEEE1275_PHANDLE_INVALID;
+  args.result = GRUB_IEEE1275_PHANDLE_INVALID;
 
   if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
     return -1;
@@ -379,7 +373,7 @@ grub_ieee1275_parent (grub_ieee1275_phandle_t node,
 
   INIT_IEEE1275_COMMON (&args.common, "parent", 1, 1);
   args.node = node;
-  args.result = IEEE1275_PHANDLE_INVALID;
+  args.result = GRUB_IEEE1275_PHANDLE_INVALID;
 
   if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
     return -1;
@@ -459,7 +453,7 @@ grub_ieee1275_open (const char *path, grub_ieee1275_ihandle_t *result)
   if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
     return -1;
   *result = args.result;
-  if (args.result == IEEE1275_IHANDLE_INVALID)
+  if (args.result == GRUB_IEEE1275_IHANDLE_INVALID)
     return -1;
   return 0;
 }
@@ -591,7 +585,7 @@ grub_ieee1275_claim (grub_addr_t addr, grub_size_t size, unsigned int align,
     return -1;
   if (result)
     *result = args.base;
-  if (args.base == IEEE1275_CELL_INVALID)
+  if (args.base == GRUB_IEEE1275_CELL_INVALID)
     return -1;
   return 0;
 }
@@ -641,7 +635,8 @@ grub_ieee1275_set_property (grub_ieee1275_phandle_t phandle,
   if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
     return -1;
   *actual = args.actual;
-  if ((args.actual == IEEE1275_CELL_INVALID) || (args.actual != args.size))
+  if ((args.actual == GRUB_IEEE1275_CELL_INVALID) ||
+      (args.actual != args.size))
     return -1;
   return 0;
 }
diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
index 73e2f4644..b963ee830 100644
--- a/include/grub/ieee1275/ieee1275.h
+++ b/include/grub/ieee1275/ieee1275.h
@@ -24,6 +24,8 @@
 #include <grub/types.h>
 #include <grub/machine/ieee1275.h>
 
+#define GRUB_IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
+
 struct grub_ieee1275_mem_region
 {
   unsigned int start;
@@ -58,6 +60,7 @@ typedef grub_uint32_t grub_ieee1275_ihandle_t;
 typedef grub_uint32_t grub_ieee1275_phandle_t;
 
 #define GRUB_IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_phandle_t) -1)
+#define GRUB_IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_ihandle_t) 0)
 
 struct grub_ieee1275_devalias
 {
-- 
2.25.1



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

* [PATCH v3 2/4] ieee1275: claim more memory
  2021-07-30 15:45 [PATCH v3 0/4] Add support for trusted boot on IBM PPC platform Stefan Berger
  2021-07-30 15:45 ` [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header Stefan Berger
@ 2021-07-30 15:45 ` Stefan Berger
  2021-08-04 11:19   ` Daniel Kiper
  2021-07-30 15:45 ` [PATCH v3 3/4] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
  2021-07-30 15:45 ` [PATCH v3 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0 Stefan Berger
  3 siblings, 1 reply; 14+ messages in thread
From: Stefan Berger @ 2021-07-30 15:45 UTC (permalink / raw)
  To: grub-devel; +Cc: dkiper, Daniel Axtens, 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
   extremely difficult to change with appended signatures.
 - We only have 32MB of heap.
 - Distro kernels are now often around 30MB.

So we want to claim more memory from OpenFirmware for our heap.

There are some complications:

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

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

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

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

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

 - Even if a VM has more memory, the memory OpenFirmware makes available
   as Real Memory Area can be restricted. A freshly created LPAR on a
   PowerVM machine is likely to have only 256MB available to OpenFirmware
   even if it has many gigabytes of memory allocated.

EFI systems will attempt to allocate 1/4th of the available memory,
clamped to between 1M and 1600M. That seems like a good sort of
approach, we just need to figure out if 1/4 is the right fraction
for us.

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

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

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

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

Ubuntu VMs struggle to boot with just 256MB under SLOF.
RHEL likewise has a higher minimum supported memory figure.
So lets first consider a distro kernel and 512MB of addressible memory.
(This is the default case for anything booting under PFW.) Say we lose
131MB to PFW (based on some tests). This leaves us 381MB. 1/4 of 381MB
is ~95MB. That should be enough to verify a 30MB vmlinux and should
leave plenty of space to load Linux and the initrd.

If we consider 256MB of RMA under PFW, we have just 125MB remaining. 1/4
of that is a smidge under 32MB, which gives us very poor odds of verifying
a distro-sized kernel. However, if we need 80MB just to put the kernel
and initrd in memory, we can't claim any more than 45MB anyway. So 1/4
will do. We'll come back to this later.

grub is always built as a 32-bit binary, even if it's loading a ppc64
kernel. So we can't address memory beyond 4GB. This gives a natural cap
of 1GB for powerpc-ieee1275.

Also apply this 1/4 approach to i386-ieee1275, but keep the 32MB cap.

make check still works for both i386 and powerpc and I've booted
powerpc grub with this change under SLOF and PFW.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
---
 docs/grub-dev.texi             |  6 ++-
 grub-core/kern/ieee1275/init.c | 81 +++++++++++++++++++++++++++-------
 2 files changed, 69 insertions(+), 18 deletions(-)

diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
index 6c629a23e..c11f1ac46 100644
--- a/docs/grub-dev.texi
+++ b/docs/grub-dev.texi
@@ -1047,7 +1047,9 @@ space is limited to 4GiB. GRUB allocates pages from EFI for its heap, at most
 1.6 GiB.
 
 On i386-ieee1275 and powerpc-ieee1275 GRUB uses same stack as IEEE1275.
-It allocates at most 32MiB for its heap.
+
+On i386-ieee1275, GRUB allocates at most 32MiB for its heap. On
+powerpc-ieee1275, GRUB allocates up to 1GiB.
 
 On sparc64-ieee1275 stack is 256KiB and heap is 2MiB.
 
@@ -1075,7 +1077,7 @@ In short:
 @item i386-qemu               @tab 60 KiB  @tab < 4 GiB
 @item *-efi                   @tab ?       @tab < 1.6 GiB
 @item i386-ieee1275           @tab ?       @tab < 32 MiB
-@item powerpc-ieee1275        @tab ?       @tab < 32 MiB
+@item powerpc-ieee1275        @tab ?       @tab < 1 GiB
 @item sparc64-ieee1275        @tab 256KiB  @tab 2 MiB
 @item arm-uboot               @tab 256KiB  @tab 2 MiB
 @item mips(el)-qemu_mips      @tab 2MiB    @tab 253 MiB
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index c5d091689..4162b5949 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -45,11 +45,12 @@
 #include <grub/machine/kernel.h>
 #endif
 
-/* The maximum heap size we're going to claim */
+/* The maximum heap size we're going to claim. Not used by sparc.
+   We allocate 1/4 of the available memory under 4G, up to this limit. */
 #ifdef __i386__
 #define HEAP_MAX_SIZE		(unsigned long) (64 * 1024 * 1024)
-#else
-#define HEAP_MAX_SIZE		(unsigned long) (32 * 1024 * 1024)
+#else // __powerpc__
+#define HEAP_MAX_SIZE		(unsigned long) (1 * 1024 * 1024 * 1024)
 #endif
 
 extern char _start[];
@@ -145,16 +146,45 @@ grub_claim_heap (void)
 				 + GRUB_KERNEL_MACHINE_STACK_SIZE), 0x200000);
 }
 #else
-/* Helper for grub_claim_heap.  */
+/* Helper for grub_claim_heap on powerpc. */
+static int
+heap_size (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
+	   void *data)
+{
+  grub_uint32_t total = *(grub_uint32_t *)data;
+
+  if (type != GRUB_MEMORY_AVAILABLE)
+    return 0;
+
+  /* Do not consider memory beyond 4GB */
+  if (addr > 0xffffffffUL)
+    return 0;
+
+  if (addr + len > 0xffffffffUL)
+    len = 0xffffffffUL - addr;
+
+  total += len;
+  *(grub_uint32_t *)data = total;
+
+  return 0;
+}
+
 static int
 heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
 	   void *data)
 {
-  unsigned long *total = data;
+  grub_uint32_t total = *(grub_uint32_t *)data;
 
   if (type != GRUB_MEMORY_AVAILABLE)
     return 0;
 
+  /* Do not consider memory beyond 4GB */
+  if (addr > 0xffffffffUL)
+    return 0;
+
+  if (addr + len > 0xffffffffUL)
+    len = 0xffffffffUL - addr;
+
   if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_NO_PRE1_5M_CLAIM))
     {
       if (addr + len <= 0x180000)
@@ -168,10 +198,6 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
     }
   len -= 1; /* Required for some firmware.  */
 
-  /* Never exceed HEAP_MAX_SIZE  */
-  if (*total + len > HEAP_MAX_SIZE)
-    len = HEAP_MAX_SIZE - *total;
-
   /* In theory, firmware should already prevent this from happening by not
      listing our own image in /memory/available.  The check below is intended
      as a safeguard in case that doesn't happen.  However, it doesn't protect
@@ -183,6 +209,18 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
       len = 0;
     }
 
+  /* If this block contains 0x30000000 (768MB), do not claim below that.
+     Linux likes to claim memory at min(RMO top, 768MB) and works down
+     without reference to /memory/available. */
+  if ((addr < 0x30000000) && ((addr + len) > 0x30000000))
+    {
+      len = len - (0x30000000 - addr);
+      addr = 0x30000000;
+    }
+
+  if (len > total)
+    len = total;
+
   if (len)
     {
       grub_err_t err;
@@ -191,10 +229,12 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
       if (err)
 	return err;
       grub_mm_init_region ((void *) (grub_addr_t) addr, len);
+      total -= len;
     }
 
-  *total += len;
-  if (*total >= HEAP_MAX_SIZE)
+  *(grub_uint32_t *)data = total;
+
+  if (total == 0)
     return 1;
 
   return 0;
@@ -203,13 +243,22 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
 static void 
 grub_claim_heap (void)
 {
-  unsigned long total = 0;
+  grub_uint32_t total = 0;
 
   if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_FORCE_CLAIM))
-    heap_init (GRUB_IEEE1275_STATIC_HEAP_START, GRUB_IEEE1275_STATIC_HEAP_LEN,
-	       1, &total);
-  else
-    grub_machine_mmap_iterate (heap_init, &total);
+    {
+      heap_init (GRUB_IEEE1275_STATIC_HEAP_START, GRUB_IEEE1275_STATIC_HEAP_LEN,
+		 1, &total);
+      return;
+    }
+
+  grub_machine_mmap_iterate (heap_size, &total);
+
+  total = total / 4;
+  if (total > HEAP_MAX_SIZE)
+    total = HEAP_MAX_SIZE;
+
+  grub_machine_mmap_iterate (heap_init, &total);
 }
 #endif
 
-- 
2.25.1



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

* [PATCH v3 3/4] ieee1275: request memory with ibm, client-architecture-support
  2021-07-30 15:45 [PATCH v3 0/4] Add support for trusted boot on IBM PPC platform Stefan Berger
  2021-07-30 15:45 ` [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header Stefan Berger
  2021-07-30 15:45 ` [PATCH v3 2/4] ieee1275: claim more memory Stefan Berger
@ 2021-07-30 15:45 ` Stefan Berger
  2021-07-30 15:45 ` [PATCH v3 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0 Stefan Berger
  3 siblings, 0 replies; 14+ messages in thread
From: Stefan Berger @ 2021-07-30 15:45 UTC (permalink / raw)
  To: grub-devel; +Cc: dkiper, Daniel Axtens, 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 really enough. Fortunately, the Power Architecture Platform
Reference (PAPR) defines a method we can call to ask for more memory.
This is part of the broad and powerful ibm,client-architecture-support
(CAS) method.

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

If:

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

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

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

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

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

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

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

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

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

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

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

Signed-off-by: Daniel Axtens <dja@axtens.net>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
---
 grub-core/kern/ieee1275/cmain.c  |   3 +
 grub-core/kern/ieee1275/init.c   | 144 ++++++++++++++++++++++++++++++-
 include/grub/ieee1275/ieee1275.h |   6 ++
 3 files changed, 151 insertions(+), 2 deletions(-)

diff --git a/grub-core/kern/ieee1275/cmain.c b/grub-core/kern/ieee1275/cmain.c
index 20cbbd761..cc98811f4 100644
--- a/grub-core/kern/ieee1275/cmain.c
+++ b/grub-core/kern/ieee1275/cmain.c
@@ -124,6 +124,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 4162b5949..4586bec93 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -240,6 +240,135 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
   return 0;
 }
 
+/* How much memory does OF believe it has? (regardless of whether
+   it's accessible or not) */
+static grub_err_t
+grub_ieee1275_total_mem (grub_uint64_t *total)
+{
+  grub_ieee1275_phandle_t root;
+  grub_ieee1275_phandle_t memory;
+  grub_uint32_t reg[4];
+  grub_ssize_t reg_size;
+  grub_uint32_t address_cells = 1;
+  grub_uint32_t size_cells = 1;
+  grub_uint64_t size;
+
+  /* If we fail to get to the end, report 0. */
+  *total = 0;
+
+  /* Determine the format of each entry in `reg'.  */
+  grub_ieee1275_finddevice ("/", &root);
+  grub_ieee1275_get_integer_property (root, "#address-cells", &address_cells,
+				      sizeof address_cells, 0);
+  grub_ieee1275_get_integer_property (root, "#size-cells", &size_cells,
+				      sizeof size_cells, 0);
+
+  if (size_cells > address_cells)
+    address_cells = size_cells;
+
+  /* Load `/memory/reg'.  */
+  if (grub_ieee1275_finddevice ("/memory", &memory))
+    return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+		       "couldn't find /memory node");
+  if (grub_ieee1275_get_integer_property (memory, "reg", reg,
+					  sizeof reg, &reg_size))
+    return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+		       "couldn't examine /memory/reg property");
+  if (reg_size < 0 || (grub_size_t) reg_size > sizeof (reg))
+    return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+                       "/memory response buffer exceeded");
+
+  if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_BROKEN_ADDRESS_CELLS))
+    {
+      address_cells = 1;
+      size_cells = 1;
+    }
+
+  /* Decode only the size */
+  size = reg[address_cells];
+  if (size_cells == 2)
+    size = (size << 32) | reg[address_cells + 1];
+
+  *total = size;
+
+  return grub_errno;
+}
+
+/* Based on linux - arch/powerpc/kernel/prom_init.c */
+struct option_vector2 {
+	grub_uint8_t byte1;
+	grub_uint16_t reserved;
+	grub_uint32_t real_base;
+	grub_uint32_t real_size;
+	grub_uint32_t virt_base;
+	grub_uint32_t virt_size;
+	grub_uint32_t load_base;
+	grub_uint32_t min_rma;
+	grub_uint32_t min_load;
+	grub_uint8_t min_rma_percent;
+	grub_uint8_t max_pft_size;
+} __attribute__((packed));
+
+struct pvr_entry {
+	  grub_uint32_t mask;
+	  grub_uint32_t entry;
+};
+
+struct cas_vector {
+    struct {
+      struct pvr_entry terminal;
+    } pvr_list;
+    grub_uint8_t num_vecs;
+    grub_uint8_t vec1_size;
+    grub_uint8_t vec1;
+    grub_uint8_t vec2_size;
+    struct option_vector2 vec2;
+} __attribute__((packed));
+
+/* Call ibm,client-architecture-support to try to get more RMA.
+   We ask for 512MB which should be enough to verify a distro kernel.
+   We ignore most errors: if we don't succeed we'll proceed with whatever
+   memory we have. */
+static void
+grub_ieee1275_ibm_cas (void)
+{
+  int rc;
+  grub_ieee1275_ihandle_t root;
+  struct cas_args {
+    struct grub_ieee1275_common_hdr common;
+    grub_ieee1275_cell_t method;
+    grub_ieee1275_ihandle_t ihandle;
+    grub_ieee1275_cell_t cas_addr;
+    grub_ieee1275_cell_t result;
+  } args;
+  struct cas_vector vector = {
+    .pvr_list = { { 0x00000000, 0xffffffff } }, /* any processor */
+    .num_vecs = 2 - 1,
+    .vec1_size = 0,
+    .vec1 = 0x80, /* ignore */
+    .vec2_size = 1 + sizeof(struct option_vector2) - 2,
+    .vec2 = {
+      0, 0, -1, -1, -1, -1, -1, 512, -1, 0, 48
+    },
+  };
+
+  INIT_IEEE1275_COMMON (&args.common, "call-method", 3, 2);
+  args.method = (grub_ieee1275_cell_t)"ibm,client-architecture-support";
+  rc = grub_ieee1275_open("/", &root);
+  if (rc) {
+	  grub_error (GRUB_ERR_IO, "could not open root when trying to call CAS");
+	  return;
+  }
+  args.ihandle = root;
+  args.cas_addr = (grub_ieee1275_cell_t)&vector;
+
+  grub_printf("Calling ibm,client-architecture-support...");
+  IEEE1275_CALL_ENTRY_FN (&args);
+  grub_printf("done\n");
+
+  grub_ieee1275_close(root);
+}
+
 static void 
 grub_claim_heap (void)
 {
@@ -247,11 +376,22 @@ grub_claim_heap (void)
 
   if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_FORCE_CLAIM))
     {
-      heap_init (GRUB_IEEE1275_STATIC_HEAP_START, GRUB_IEEE1275_STATIC_HEAP_LEN,
-		 1, &total);
+      heap_init (GRUB_IEEE1275_STATIC_HEAP_START,
+		 GRUB_IEEE1275_STATIC_HEAP_LEN, 1, &total);
       return;
     }
 
+  if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY))
+    {
+      grub_uint64_t rma_size;
+      grub_err_t err;
+
+      err = grub_ieee1275_total_mem (&rma_size);
+      /* if we have an error, don't call CAS, just hope for the best */
+      if (!err && rma_size < (512 * 1024 * 1024))
+	grub_ieee1275_ibm_cas();
+    }
+
   grub_machine_mmap_iterate (heap_size, &total);
 
   total = total / 4;
diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
index b963ee830..c5402dc1c 100644
--- a/include/grub/ieee1275/ieee1275.h
+++ b/include/grub/ieee1275/ieee1275.h
@@ -151,6 +151,12 @@ 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] 14+ messages in thread

* [PATCH v3 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0
  2021-07-30 15:45 [PATCH v3 0/4] Add support for trusted boot on IBM PPC platform Stefan Berger
                   ` (2 preceding siblings ...)
  2021-07-30 15:45 ` [PATCH v3 3/4] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
@ 2021-07-30 15:45 ` Stefan Berger
  2021-08-04 11:09   ` Daniel Kiper
  3 siblings, 1 reply; 14+ messages in thread
From: Stefan Berger @ 2021-07-30 15:45 UTC (permalink / raw)
  To: grub-devel; +Cc: dkiper, Stefan Berger, Eric Snowberg

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

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

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

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

diff --git a/docs/grub.texi b/docs/grub.texi
index f8b4b3b21..23d2f2d90 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -6023,7 +6023,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 8022e1c0a..e6ac50704 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1118,6 +1118,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..1762f51f2
--- /dev/null
+++ b/grub-core/commands/ieee1275/ibmvtpm.c
@@ -0,0 +1,160 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2021  Free Software Foundation, Inc.
+ *  Copyright (C) 2021  IBM Corporation
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *  IBM vTPM support code.
+ */
+
+#include <grub/err.h>
+#include <grub/types.h>
+#include <grub/tpm.h>
+#include <grub/ieee1275/ieee1275.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+
+static grub_ieee1275_ihandle_t tpm_ihandle;
+static grub_uint8_t tpm_version;
+
+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_ieee1275_ihandle_t
+tpm_init (void)
+{
+  static int init_called = 0;
+
+  if (!init_called)
+    {
+      if (grub_ieee1275_open ("/vdevice/vtpm", &tpm_ihandle) < 0)
+        tpm_ihandle = GRUB_IEEE1275_IHANDLE_INVALID;
+
+      tpm_get_tpm_version ();
+
+      init_called = 1;
+    }
+
+  return tpm_ihandle;
+}
+
+static grub_err_t
+tpm_handle_find (grub_ieee1275_ihandle_t *tpm_handle)
+{
+  *tpm_handle = tpm_init ();
+  if (*tpm_handle == GRUB_IEEE1275_IHANDLE_INVALID)
+    return GRUB_ERR_UNKNOWN_DEVICE;
+
+  return GRUB_ERR_NONE;
+}
+
+static int
+ibmvtpm_2hash_ext_log (grub_uint32_t ihandle,
+		       grub_uint8_t pcrindex,
+		       grub_uint32_t eventtype,
+		       const char *description,
+		       grub_size_t description_size,
+		       void *buf, grub_size_t size)
+{
+  struct tpm_2hash_ext_log
+  {
+    struct grub_ieee1275_common_hdr common;
+    grub_ieee1275_cell_t method;
+    grub_ieee1275_cell_t ihandle;
+    grub_ieee1275_cell_t size;
+    grub_ieee1275_cell_t buf;
+    grub_ieee1275_cell_t description_size;
+    grub_ieee1275_cell_t description;
+    grub_ieee1275_cell_t eventtype;
+    grub_ieee1275_cell_t pcrindex;
+    grub_ieee1275_cell_t catch_result;
+    grub_ieee1275_cell_t rc;
+  }
+  args;
+
+  INIT_IEEE1275_COMMON (&args.common, "call-method", 8, 2);
+  args.method = (grub_ieee1275_cell_t) "2hash-ext-log";
+  args.ihandle = 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 TRUE (-1) on success
+   */
+  if ((args.catch_result) || args.rc == GRUB_IEEE1275_CELL_FALSE)
+    return -1;
+
+  return 0;
+}
+
+static grub_err_t
+tpm2_log_event (grub_ieee1275_ihandle_t tpm_handle, unsigned char *buf,
+		     grub_size_t size, grub_uint8_t pcr,
+		     const char *description)
+{
+  static int error_displayed = 0;
+  int err;
+
+  err = ibmvtpm_2hash_ext_log (tpm_handle,
+			       pcr, EV_IPL,
+			       description,
+			       grub_strlen(description) + 1,
+			       buf, size);
+  if (err && !error_displayed)
+    {
+      error_displayed++;
+      return grub_error (GRUB_ERR_BAD_DEVICE,
+	      "2HASH-EXT-LOG failed: Firmware is likely too old.\n");
+    }
+
+  return GRUB_ERR_NONE;
+}
+
+grub_err_t
+grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
+		  const char *description)
+{
+  grub_ieee1275_ihandle_t tpm_handle;
+
+  /* Absence of a TPM isn't a failure. */
+  if (tpm_handle_find (&tpm_handle) != 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 (tpm_handle, buf, size, pcr, description);
+
+  return GRUB_ERR_NONE;
+}
diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
index c5402dc1c..c81b00bc5 100644
--- a/include/grub/ieee1275/ieee1275.h
+++ b/include/grub/ieee1275/ieee1275.h
@@ -25,6 +25,7 @@
 #include <grub/machine/ieee1275.h>
 
 #define GRUB_IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
+#define GRUB_IEEE1275_CELL_FALSE       ((grub_ieee1275_cell_t) 0)
 
 struct grub_ieee1275_mem_region
 {
-- 
2.25.1



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

* Re: [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header
  2021-07-30 15:45 ` [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header Stefan Berger
@ 2021-08-04 10:42   ` Daniel Kiper
  2021-08-05 22:22   ` Stefan Berger
  1 sibling, 0 replies; 14+ messages in thread
From: Daniel Kiper @ 2021-08-04 10:42 UTC (permalink / raw)
  To: Stefan Berger; +Cc: grub-devel, Stefan Berger

On Fri, Jul 30, 2021 at 11:45:37AM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Move some #defines from ieee1275.c into the common ieee1275.h
> header file. Adjust the case used in IHANDLE_INVALID to use
> proper ihandle_t.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  grub-core/kern/ieee1275/ieee1275.c | 29 ++++++++++++-----------------
>  include/grub/ieee1275/ieee1275.h   |  3 +++
>  2 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/grub-core/kern/ieee1275/ieee1275.c b/grub-core/kern/ieee1275/ieee1275.c
> index 86f81a3c4..8fe92274d 100644
> --- a/grub-core/kern/ieee1275/ieee1275.c
> +++ b/grub-core/kern/ieee1275/ieee1275.c
> @@ -21,11 +21,6 @@
>  #include <grub/types.h>
>  #include <grub/misc.h>
>
> -#define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_cell_t) -1)
> -#define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_cell_t) 0)
> -#define IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
> -
> -\f
>

Please drop this empty line too.

>  int
>  grub_ieee1275_finddevice (const char *name, grub_ieee1275_phandle_t *phandlep)
> @@ -44,8 +39,7 @@ grub_ieee1275_finddevice (const char *name, grub_ieee1275_phandle_t *phandlep)
>    if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>      return -1;
>    *phandlep = args.phandle;
> -  if (args.phandle == IEEE1275_PHANDLE_INVALID)
> -    return -1;
> +  if (args.phandle == GRUB_IEEE1275_PHANDLE_INVALID) return -1;

I am not sure why you move "return -1;". You should not...

Daniel


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

* Re: [PATCH v3 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0
  2021-07-30 15:45 ` [PATCH v3 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0 Stefan Berger
@ 2021-08-04 11:09   ` Daniel Kiper
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Kiper @ 2021-08-04 11:09 UTC (permalink / raw)
  To: Stefan Berger; +Cc: grub-devel, Stefan Berger, Eric Snowberg

On Fri, Jul 30, 2021 at 11:45:40AM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Add support for trusted boot using a vTPM 2.0 on the IBM IEEE1275
> PowerPC platform. With this patch grub now measures text and binary data
> into the TPM's PCRs 8 and 9 in the same way as the x86_64 platform
> does.
>
> This patch requires Daniel Axtens's patches for claiming more memory.
>
> Cc: Eric Snowberg <eric.snowberg@oracle.com>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  docs/grub.texi                        |   3 +-
>  grub-core/Makefile.core.def           |   7 ++
>  grub-core/commands/ieee1275/ibmvtpm.c | 160 ++++++++++++++++++++++++++
>  include/grub/ieee1275/ieee1275.h      |   1 +
>  4 files changed, 170 insertions(+), 1 deletion(-)
>  create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index f8b4b3b21..23d2f2d90 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -6023,7 +6023,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 8022e1c0a..e6ac50704 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1118,6 +1118,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..1762f51f2
> --- /dev/null
> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
> @@ -0,0 +1,160 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2021  Free Software Foundation, Inc.
> + *  Copyright (C) 2021  IBM Corporation
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + *  IBM vTPM support code.
> + */
> +
> +#include <grub/err.h>
> +#include <grub/types.h>
> +#include <grub/tpm.h>
> +#include <grub/ieee1275/ieee1275.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +
> +static grub_ieee1275_ihandle_t tpm_ihandle;
> +static grub_uint8_t tpm_version;
> +
> +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_ieee1275_ihandle_t
> +tpm_init (void)
> +{
> +  static int init_called = 0;
> +
> +  if (!init_called)
> +    {
> +      if (grub_ieee1275_open ("/vdevice/vtpm", &tpm_ihandle) < 0)
> +        tpm_ihandle = GRUB_IEEE1275_IHANDLE_INVALID;
> +

I think "else" is missing here.

Anyway, I would cram tpm_get_tpm_version() code into tpm_init() and drop
tpm_handle_find().

> +      tpm_get_tpm_version ();
> +
> +      init_called = 1;
> +    }
> +
> +  return tpm_ihandle;

You should return grub_err_t here as tpm_handle_find() does.

> +}
> +
> +static grub_err_t
> +tpm_handle_find (grub_ieee1275_ihandle_t *tpm_handle)

Please do not pass tpm_handle in such way if you have tpm_ihandle
available everywhere.

> +{
> +  *tpm_handle = tpm_init ();
> +  if (*tpm_handle == GRUB_IEEE1275_IHANDLE_INVALID)
> +    return GRUB_ERR_UNKNOWN_DEVICE;
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +static int
> +ibmvtpm_2hash_ext_log (grub_uint32_t ihandle,

Please drop ihandle from this function and use tpm_ihandle directly.

> +		       grub_uint8_t pcrindex,
> +		       grub_uint32_t eventtype,
> +		       const char *description,
> +		       grub_size_t description_size,
> +		       void *buf, grub_size_t size)
> +{
> +  struct tpm_2hash_ext_log
> +  {
> +    struct grub_ieee1275_common_hdr common;
> +    grub_ieee1275_cell_t method;
> +    grub_ieee1275_cell_t ihandle;
> +    grub_ieee1275_cell_t size;
> +    grub_ieee1275_cell_t buf;
> +    grub_ieee1275_cell_t description_size;
> +    grub_ieee1275_cell_t description;
> +    grub_ieee1275_cell_t eventtype;
> +    grub_ieee1275_cell_t pcrindex;
> +    grub_ieee1275_cell_t catch_result;
> +    grub_ieee1275_cell_t rc;
> +  }
> +  args;
> +
> +  INIT_IEEE1275_COMMON (&args.common, "call-method", 8, 2);
> +  args.method = (grub_ieee1275_cell_t) "2hash-ext-log";
> +  args.ihandle = 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 TRUE (-1) on success

s/TRUE/GRUB_IEEE1275_CELL_TRUE/ Please look below too...

> +   */
> +  if ((args.catch_result) || args.rc == GRUB_IEEE1275_CELL_FALSE)
> +    return -1;
> +
> +  return 0;
> +}
> +
> +static grub_err_t
> +tpm2_log_event (grub_ieee1275_ihandle_t tpm_handle, unsigned char *buf,

Please drop tpm_handle. You do not need it here.

> +		     grub_size_t size, grub_uint8_t pcr,
> +		     const char *description)
> +{
> +  static int error_displayed = 0;
> +  int err;
> +
> +  err = ibmvtpm_2hash_ext_log (tpm_handle,
> +			       pcr, EV_IPL,
> +			       description,
> +			       grub_strlen(description) + 1,
> +			       buf, size);
> +  if (err && !error_displayed)
> +    {
> +      error_displayed++;
> +      return grub_error (GRUB_ERR_BAD_DEVICE,
> +	      "2HASH-EXT-LOG failed: Firmware is likely too old.\n");

N_("2HASH-EXT-LOG failed: Firmware is likely too old");

> +    }
> +
> +  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_ieee1275_ihandle_t tpm_handle;

Again. you do not need tpm_handle here.

> +  /* Absence of a TPM isn't a failure. */
> +  if (tpm_handle_find (&tpm_handle) != 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 (tpm_handle, buf, size, pcr, description);
> +
> +  return GRUB_ERR_NONE;
> +}
> diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
> index c5402dc1c..c81b00bc5 100644
> --- a/include/grub/ieee1275/ieee1275.h
> +++ b/include/grub/ieee1275/ieee1275.h
> @@ -25,6 +25,7 @@
>  #include <grub/machine/ieee1275.h>
>
>  #define GRUB_IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
> +#define GRUB_IEEE1275_CELL_FALSE       ((grub_ieee1275_cell_t) 0)

I think you should define GRUB_IEEE1275_CELL_TRUE too. Even if you do
not use it.

Daniel


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

* Re: [PATCH v3 2/4] ieee1275: claim more memory
  2021-07-30 15:45 ` [PATCH v3 2/4] ieee1275: claim more memory Stefan Berger
@ 2021-08-04 11:19   ` Daniel Kiper
  2021-08-04 12:40     ` Stefan Berger
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Kiper @ 2021-08-04 11:19 UTC (permalink / raw)
  To: Stefan Berger; +Cc: grub-devel, Daniel Axtens, Stefan Berger, ps

CC-ing Patrick.

On Fri, Jul 30, 2021 at 11:45:38AM -0400, 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
>    extremely difficult to change with appended signatures.
>  - We only have 32MB of heap.
>  - Distro kernels are now often around 30MB.
>
> So we want to claim more memory from OpenFirmware for our heap.

Stefan, Patrick, Daniel, may I ask one of you to work on the [1]
proposal. I want to have this solution implemented.

Daniel

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


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

* Re: [PATCH v3 2/4] ieee1275: claim more memory
  2021-08-04 11:19   ` Daniel Kiper
@ 2021-08-04 12:40     ` Stefan Berger
  2021-08-05 13:15       ` Daniel Kiper
  2021-08-07 16:11       ` Patrick Steinhardt
  0 siblings, 2 replies; 14+ messages in thread
From: Stefan Berger @ 2021-08-04 12:40 UTC (permalink / raw)
  To: The development of GNU GRUB, Daniel Kiper, Stefan Berger
  Cc: Daniel Axtens, ps


On 8/4/21 7:19 AM, Daniel Kiper wrote:
> CC-ing Patrick.
>
> On Fri, Jul 30, 2021 at 11:45:38AM -0400, 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
>>     extremely difficult to change with appended signatures.
>>   - We only have 32MB of heap.
>>   - Distro kernels are now often around 30MB.
>>
>> So we want to claim more memory from OpenFirmware for our heap.
> Stefan, Patrick, Daniel, may I ask one of you to work on the [1]
> proposal. I want to have this solution implemented.

Is the design in [1] correct just that the patches need forward-porting?

   Stefan


>
> Daniel
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2020-06/msg00009.html
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


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

* Re: [PATCH v3 2/4] ieee1275: claim more memory
  2021-08-04 12:40     ` Stefan Berger
@ 2021-08-05 13:15       ` Daniel Kiper
  2021-08-07 16:11       ` Patrick Steinhardt
  1 sibling, 0 replies; 14+ messages in thread
From: Daniel Kiper @ 2021-08-05 13:15 UTC (permalink / raw)
  To: Stefan Berger
  Cc: The development of GNU GRUB, Stefan Berger, Daniel Axtens, ps

On Wed, Aug 04, 2021 at 08:40:01AM -0400, Stefan Berger wrote:
>
> On 8/4/21 7:19 AM, Daniel Kiper wrote:
> > CC-ing Patrick.
> >
> > On Fri, Jul 30, 2021 at 11:45:38AM -0400, 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
> > >     extremely difficult to change with appended signatures.
> > >   - We only have 32MB of heap.
> > >   - Distro kernels are now often around 30MB.
> > >
> > > So we want to claim more memory from OpenFirmware for our heap.
> > Stefan, Patrick, Daniel, may I ask one of you to work on the [1]
> > proposal. I want to have this solution implemented.
>
> Is the design in [1] correct just that the patches need forward-porting?

As I said in the other thread I am OK with general idea. So, I think you
can rabase this patchset, align it with your needs and then we can
continue discussion. Please do not forget to CC at least Patrick.

Daniel


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

* Re: [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header
  2021-07-30 15:45 ` [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header Stefan Berger
  2021-08-04 10:42   ` Daniel Kiper
@ 2021-08-05 22:22   ` Stefan Berger
  2021-08-09 12:17     ` Daniel Kiper
  1 sibling, 1 reply; 14+ messages in thread
From: Stefan Berger @ 2021-08-05 22:22 UTC (permalink / raw)
  To: The development of GNU GRUB, Stefan Berger; +Cc: dkiper, Eric Snowberg


On 7/30/21 11:45 AM, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Move some #defines from ieee1275.c into the common ieee1275.h
> header file. Adjust the case used in IHANDLE_INVALID to use
> proper ihandle_t.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>   grub-core/kern/ieee1275/ieee1275.c | 29 ++++++++++++-----------------
>   include/grub/ieee1275/ieee1275.h   |  3 +++
>   2 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/grub-core/kern/ieee1275/ieee1275.c b/grub-core/kern/ieee1275/ieee1275.c
> index 86f81a3c4..8fe92274d 100644
> --- a/grub-core/kern/ieee1275/ieee1275.c
> +++ b/grub-core/kern/ieee1275/ieee1275.c
> @@ -21,11 +21,6 @@
>   #include <grub/types.h>
>   #include <grub/misc.h>
>   
> -#define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_cell_t) -1)
> -#define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_cell_t) 0)
> -#define IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
> -
> -\f
>   

I think it's safer to keep these here  because of how the sizes for the 
grub_ieee1275_cell_t are defined:

# grep -r grub_ieee1275_cell include/grub/ | grep typedef

include/grub/powerpc/ieee1275/ieee1275.h:typedef grub_uint32_t 
grub_ieee1275_cell_t;
include/grub/sparc64/ieee1275/ieee1275.h:typedef grub_uint64_t 
grub_ieee1275_cell_t;

# grep -r grub_ieee1275_phandle include/grub/ | grep typedef
include/grub/ieee1275/ieee1275.h:typedef grub_uint32_t 
grub_ieee1275_phandle_t;

# grep -r grub_ieee1275_ihandle include/grub/ | grep typedef
include/grub/ieee1275/ieee1275.h:typedef grub_uint32_t 
grub_ieee1275_ihandle_t;

# grep -r GRUB_IEEE1275_PH include/grub/| grep def
include/grub/ieee1275/ieee1275.h:#define GRUB_IEEE1275_PHANDLE_INVALID  
((grub_ieee1275_phandle_t) -1)

Using the global GRUB_IEEE1275_PHANDLE_INVALID here would probably break 
things on sparc64 if we now use 32bit '-1' rather than 64bit '-1'.


     Stefan



>   int
>   grub_ieee1275_finddevice (const char *name, grub_ieee1275_phandle_t *phandlep)
> @@ -44,8 +39,7 @@ grub_ieee1275_finddevice (const char *name, grub_ieee1275_phandle_t *phandlep)
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *phandlep = args.phandle;
> -  if (args.phandle == IEEE1275_PHANDLE_INVALID)
> -    return -1;
> +  if (args.phandle == GRUB_IEEE1275_PHANDLE_INVALID) return -1;
>     return 0;
>   }
>   
> @@ -75,7 +69,7 @@ grub_ieee1275_get_property (grub_ieee1275_phandle_t phandle,
>       return -1;
>     if (actual)
>       *actual = (grub_ssize_t) args.size;
> -  if (args.size == IEEE1275_CELL_INVALID)
> +  if (args.size == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -146,7 +140,7 @@ grub_ieee1275_get_property_length (grub_ieee1275_phandle_t phandle,
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *length = args.length;
> -  if (args.length == IEEE1275_CELL_INVALID)
> +  if (args.length == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -169,7 +163,7 @@ grub_ieee1275_instance_to_package (grub_ieee1275_ihandle_t ihandle,
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *phandlep = args.phandle;
> -  if (args.phandle == IEEE1275_PHANDLE_INVALID)
> +  if (args.phandle == GRUB_IEEE1275_PHANDLE_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -198,7 +192,7 @@ grub_ieee1275_package_to_path (grub_ieee1275_phandle_t phandle,
>       return -1;
>     if (actual)
>       *actual = args.actual;
> -  if (args.actual == IEEE1275_CELL_INVALID)
> +  if (args.actual == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -227,7 +221,7 @@ grub_ieee1275_instance_to_path (grub_ieee1275_ihandle_t ihandle,
>       return -1;
>     if (actual)
>       *actual = args.actual;
> -  if (args.actual == IEEE1275_CELL_INVALID)
> +  if (args.actual == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -355,7 +349,7 @@ grub_ieee1275_child (grub_ieee1275_phandle_t node,
>   
>     INIT_IEEE1275_COMMON (&args.common, "child", 1, 1);
>     args.node = node;
> -  args.result = IEEE1275_PHANDLE_INVALID;
> +  args.result = GRUB_IEEE1275_PHANDLE_INVALID;
>   
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
> @@ -379,7 +373,7 @@ grub_ieee1275_parent (grub_ieee1275_phandle_t node,
>   
>     INIT_IEEE1275_COMMON (&args.common, "parent", 1, 1);
>     args.node = node;
> -  args.result = IEEE1275_PHANDLE_INVALID;
> +  args.result = GRUB_IEEE1275_PHANDLE_INVALID;
>   
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
> @@ -459,7 +453,7 @@ grub_ieee1275_open (const char *path, grub_ieee1275_ihandle_t *result)
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *result = args.result;
> -  if (args.result == IEEE1275_IHANDLE_INVALID)
> +  if (args.result == GRUB_IEEE1275_IHANDLE_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -591,7 +585,7 @@ grub_ieee1275_claim (grub_addr_t addr, grub_size_t size, unsigned int align,
>       return -1;
>     if (result)
>       *result = args.base;
> -  if (args.base == IEEE1275_CELL_INVALID)
> +  if (args.base == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -641,7 +635,8 @@ grub_ieee1275_set_property (grub_ieee1275_phandle_t phandle,
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *actual = args.actual;
> -  if ((args.actual == IEEE1275_CELL_INVALID) || (args.actual != args.size))
> +  if ((args.actual == GRUB_IEEE1275_CELL_INVALID) ||
> +      (args.actual != args.size))
>       return -1;
>     return 0;
>   }
> diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
> index 73e2f4644..b963ee830 100644
> --- a/include/grub/ieee1275/ieee1275.h
> +++ b/include/grub/ieee1275/ieee1275.h
> @@ -24,6 +24,8 @@
>   #include <grub/types.h>
>   #include <grub/machine/ieee1275.h>
>   
> +#define GRUB_IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
> +
>   struct grub_ieee1275_mem_region
>   {
>     unsigned int start;
> @@ -58,6 +60,7 @@ typedef grub_uint32_t grub_ieee1275_ihandle_t;
>   typedef grub_uint32_t grub_ieee1275_phandle_t;
>   
>   #define GRUB_IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_phandle_t) -1)
> +#define GRUB_IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_ihandle_t) 0)
>   
>   struct grub_ieee1275_devalias
>   {


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

* Re: [PATCH v3 2/4] ieee1275: claim more memory
  2021-08-04 12:40     ` Stefan Berger
  2021-08-05 13:15       ` Daniel Kiper
@ 2021-08-07 16:11       ` Patrick Steinhardt
  2021-08-08 12:53         ` Patrick Steinhardt
  1 sibling, 1 reply; 14+ messages in thread
From: Patrick Steinhardt @ 2021-08-07 16:11 UTC (permalink / raw)
  To: Stefan Berger
  Cc: The development of GNU GRUB, Daniel Kiper, Stefan Berger, Daniel Axtens

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

On Wed, Aug 04, 2021 at 08:40:01AM -0400, Stefan Berger wrote:
> 
> On 8/4/21 7:19 AM, Daniel Kiper wrote:
> > CC-ing Patrick.
> >
> > On Fri, Jul 30, 2021 at 11:45:38AM -0400, 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
> >>     extremely difficult to change with appended signatures.
> >>   - We only have 32MB of heap.
> >>   - Distro kernels are now often around 30MB.
> >>
> >> So we want to claim more memory from OpenFirmware for our heap.
> > Stefan, Patrick, Daniel, may I ask one of you to work on the [1]
> > proposal. I want to have this solution implemented.
> 
> Is the design in [1] correct just that the patches need forward-porting?
> 
>    Stefan

Honestly, I don't quite recall the state back when I was working on it.
Forward-porting the patch set is trivial to do, there aren't really any
conflicts. I may try to invest some time into it tomorrow to see whether
it did work as advertised.

Patrick

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

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

* Re: [PATCH v3 2/4] ieee1275: claim more memory
  2021-08-07 16:11       ` Patrick Steinhardt
@ 2021-08-08 12:53         ` Patrick Steinhardt
  0 siblings, 0 replies; 14+ messages in thread
From: Patrick Steinhardt @ 2021-08-08 12:53 UTC (permalink / raw)
  To: The development of GNU GRUB
  Cc: Stefan Berger, Daniel Kiper, Stefan Berger, Daniel Axtens

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

On Sat, Aug 07, 2021 at 06:11:09PM +0200, Patrick Steinhardt wrote:
> On Wed, Aug 04, 2021 at 08:40:01AM -0400, Stefan Berger wrote:
> > 
> > On 8/4/21 7:19 AM, Daniel Kiper wrote:
> > > CC-ing Patrick.
> > >
> > > On Fri, Jul 30, 2021 at 11:45:38AM -0400, 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
> > >>     extremely difficult to change with appended signatures.
> > >>   - We only have 32MB of heap.
> > >>   - Distro kernels are now often around 30MB.
> > >>
> > >> So we want to claim more memory from OpenFirmware for our heap.
> > > Stefan, Patrick, Daniel, may I ask one of you to work on the [1]
> > > proposal. I want to have this solution implemented.
> > 
> > Is the design in [1] correct just that the patches need forward-porting?
> > 
> >    Stefan
> 
> Honestly, I don't quite recall the state back when I was working on it.
> Forward-porting the patch set is trivial to do, there aren't really any
> conflicts. I may try to invest some time into it tomorrow to see whether
> it did work as advertised.
> 
> Patrick

So I've checked with my LUKS2/Argon2 patch set, and it does work as
expected with a memory hardness parameter of 1GB. I'll send out a
revised patch series of those patches for review.

Patrick

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

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

* Re: [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header
  2021-08-05 22:22   ` Stefan Berger
@ 2021-08-09 12:17     ` Daniel Kiper
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Kiper @ 2021-08-09 12:17 UTC (permalink / raw)
  To: Stefan Berger; +Cc: The development of GNU GRUB, Stefan Berger, Eric Snowberg

On Thu, Aug 05, 2021 at 06:22:24PM -0400, Stefan Berger wrote:
> On 7/30/21 11:45 AM, Stefan Berger wrote:
> > From: Stefan Berger <stefanb@linux.ibm.com>
> >
> > Move some #defines from ieee1275.c into the common ieee1275.h
> > header file. Adjust the case used in IHANDLE_INVALID to use
> > proper ihandle_t.
> >
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >   grub-core/kern/ieee1275/ieee1275.c | 29 ++++++++++++-----------------
> >   include/grub/ieee1275/ieee1275.h   |  3 +++
> >   2 files changed, 15 insertions(+), 17 deletions(-)
> >
> > diff --git a/grub-core/kern/ieee1275/ieee1275.c b/grub-core/kern/ieee1275/ieee1275.c
> > index 86f81a3c4..8fe92274d 100644
> > --- a/grub-core/kern/ieee1275/ieee1275.c
> > +++ b/grub-core/kern/ieee1275/ieee1275.c
> > @@ -21,11 +21,6 @@
> >   #include <grub/types.h>
> >   #include <grub/misc.h>
> > -#define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_cell_t) -1)
> > -#define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_cell_t) 0)
> > -#define IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
> > -
> > -\f
>
> I think it's safer to keep these here  because of how the sizes for the
> grub_ieee1275_cell_t are defined:
>
> # grep -r grub_ieee1275_cell include/grub/ | grep typedef
>
> include/grub/powerpc/ieee1275/ieee1275.h:typedef grub_uint32_t
> grub_ieee1275_cell_t;
> include/grub/sparc64/ieee1275/ieee1275.h:typedef grub_uint64_t
> grub_ieee1275_cell_t;
>
> # grep -r grub_ieee1275_phandle include/grub/ | grep typedef
> include/grub/ieee1275/ieee1275.h:typedef grub_uint32_t
> grub_ieee1275_phandle_t;
>
> # grep -r grub_ieee1275_ihandle include/grub/ | grep typedef
> include/grub/ieee1275/ieee1275.h:typedef grub_uint32_t
> grub_ieee1275_ihandle_t;
>
> # grep -r GRUB_IEEE1275_PH include/grub/| grep def
> include/grub/ieee1275/ieee1275.h:#define GRUB_IEEE1275_PHANDLE_INVALID 
> ((grub_ieee1275_phandle_t) -1)
>
> Using the global GRUB_IEEE1275_PHANDLE_INVALID here would probably break
> things on sparc64 if we now use 32bit '-1' rather than 64bit '-1'.

Well, it seems to me at least some code around phandle/ihandle in the
grub-core/kern/ieee1275/ieee1275.c file is buggy. Please take a look,
e.g., at grub_ieee1275_finddevice(). I think args.phandle should be
defined as grub_ieee1275_phandle_t. Otherwise *phandlep = args.phandle
assignment will work by chance only. So, I think your
GRUB_IEEE1275_PHANDLE_INVALID change to grub_ieee1275_phandle_t is
correct. However, IMO the code related to phandle/ihandle in the
grub-core/kern/ieee1275/ieee1275.c file should be fixed first. May I ask
you to do that?

Daniel


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

end of thread, other threads:[~2021-08-09 12:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-30 15:45 [PATCH v3 0/4] Add support for trusted boot on IBM PPC platform Stefan Berger
2021-07-30 15:45 ` [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header Stefan Berger
2021-08-04 10:42   ` Daniel Kiper
2021-08-05 22:22   ` Stefan Berger
2021-08-09 12:17     ` Daniel Kiper
2021-07-30 15:45 ` [PATCH v3 2/4] ieee1275: claim more memory Stefan Berger
2021-08-04 11:19   ` Daniel Kiper
2021-08-04 12:40     ` Stefan Berger
2021-08-05 13:15       ` Daniel Kiper
2021-08-07 16:11       ` Patrick Steinhardt
2021-08-08 12:53         ` Patrick Steinhardt
2021-07-30 15:45 ` [PATCH v3 3/4] ieee1275: request memory with ibm, client-architecture-support Stefan Berger
2021-07-30 15:45 ` [PATCH v3 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0 Stefan Berger
2021-08-04 11:09   ` 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.