[PATCH v2 0/4] Add support for trusted boot on IBM PPC platform

[PATCH v2 0/4] Add support for trusted boot on IBM PPC platform

[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

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 (3):
  ieee1275: drop HEAP_MAX_ADDR, HEAP_MIN_SIZE
  ieee1275: claim more memory
  ieee1275: request memory with ibm,client-architecture-support

Stefan Berger (1):
  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 | 169 ++++++++++++++++++
 grub-core/kern/ieee1275/cmain.c       |   3 +
 grub-core/kern/ieee1275/init.c        | 238 ++++++++++++++++++++++----
 include/grub/ieee1275/ieee1275.h      |   6 +
 7 files changed, 396 insertions(+), 36 deletions(-)
 create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c

-- 
2.25.1

[PATCH v2 1/4] ieee1275: drop HEAP_MAX_ADDR, HEAP_MIN_SIZE

[Stefan Berger]

From: Daniel Axtens <dja@axtens.net>

HEAP_MAX_ADDR is confusing. Currently it is set to 32MB, except
on ieee1275 on x86, where it is 64MB.

There is a comment which purports to explain it:

/* If possible, we will avoid claiming heap above this address, because it
   seems to cause relocation problems with OSes that link at 4 MiB */

This doesn't make a lot of sense when the constants are well above 4MB
already. It was not always this way. Prior to
commit 7b5d0fe4440c ("Increase heap limit") in 2010, HEAP_MAX_SIZE and
HEAP_MAX_ADDR were indeed 4MB. However, when the constants were increased
the comment was left unchanged.

It's been over a decade. It doesn't seem like we have problems with
claims over 4MB on powerpc or x86 ieee1275. (sparc does things completely
differently and never used the constant.)

Drop the constant and the check.

The only use of HEAP_MIN_SIZE was to potentially override the
HEAP_MAX_ADDR check. It is now unused. Remove it.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
---
 grub-core/kern/ieee1275/init.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index d483e35ee..c5d091689 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -45,9 +45,6 @@
 #include <grub/machine/kernel.h>
 #endif
 
-/* The minimal heap size we can live with. */
-#define HEAP_MIN_SIZE		(unsigned long) (2 * 1024 * 1024)
-
 /* The maximum heap size we're going to claim */
 #ifdef __i386__
 #define HEAP_MAX_SIZE		(unsigned long) (64 * 1024 * 1024)
@@ -55,14 +52,6 @@
 #define HEAP_MAX_SIZE		(unsigned long) (32 * 1024 * 1024)
 #endif
 
-/* If possible, we will avoid claiming heap above this address, because it
-   seems to cause relocation problems with OSes that link at 4 MiB */
-#ifdef __i386__
-#define HEAP_MAX_ADDR		(unsigned long) (64 * 1024 * 1024)
-#else
-#define HEAP_MAX_ADDR		(unsigned long) (32 * 1024 * 1024)
-#endif
-
 extern char _start[];
 extern char _end[];
 
@@ -183,12 +172,6 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t type,
   if (*total + len > HEAP_MAX_SIZE)
     len = HEAP_MAX_SIZE - *total;
 
-  /* Avoid claiming anything above HEAP_MAX_ADDR, if possible. */
-  if ((addr < HEAP_MAX_ADDR) &&				/* if it's too late, don't bother */
-      (addr + len > HEAP_MAX_ADDR) &&				/* if it wasn't available anyway, don't bother */
-      (*total + (HEAP_MAX_ADDR - addr) > HEAP_MIN_SIZE))	/* only limit ourselves when we can afford to */
-     len = HEAP_MAX_ADDR - addr;
-
   /* 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
-- 
2.25.1

Re: [PATCH v2 1/4] ieee1275: drop HEAP_MAX_ADDR, HEAP_MIN_SIZE

[Daniel Kiper]

On Tue, Jul 20, 2021 at 05:14:46PM -0400, Stefan Berger wrote:
> From: Daniel Axtens <dja@axtens.net>
>
> HEAP_MAX_ADDR is confusing. Currently it is set to 32MB, except
> on ieee1275 on x86, where it is 64MB.
>
> There is a comment which purports to explain it:
>
> /* If possible, we will avoid claiming heap above this address, because it
>    seems to cause relocation problems with OSes that link at 4 MiB */
>
> This doesn't make a lot of sense when the constants are well above 4MB
> already. It was not always this way. Prior to
> commit 7b5d0fe4440c ("Increase heap limit") in 2010, HEAP_MAX_SIZE and
> HEAP_MAX_ADDR were indeed 4MB. However, when the constants were increased
> the comment was left unchanged.
>
> It's been over a decade. It doesn't seem like we have problems with
> claims over 4MB on powerpc or x86 ieee1275. (sparc does things completely
> differently and never used the constant.)
>
> Drop the constant and the check.
>
> The only use of HEAP_MIN_SIZE was to potentially override the
> HEAP_MAX_ADDR check. It is now unused. Remove it.
>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>

Nit, you should add your Signed-off-by after Daniel's one. I can make it
for you if you do not object.

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

Daniel

Re: [PATCH v2 1/4] ieee1275: drop HEAP_MAX_ADDR, HEAP_MIN_SIZE

[Stefan Berger]

On 7/21/21 10:36 AM, Daniel Kiper wrote:
> On Tue, Jul 20, 2021 at 05:14:46PM -0400, Stefan Berger wrote:
>> From: Daniel Axtens <dja@axtens.net>
>>
>> HEAP_MAX_ADDR is confusing. Currently it is set to 32MB, except
>> on ieee1275 on x86, where it is 64MB.
>>
>> There is a comment which purports to explain it:
>>
>> /* If possible, we will avoid claiming heap above this address, because it
>>     seems to cause relocation problems with OSes that link at 4 MiB */
>>
>> This doesn't make a lot of sense when the constants are well above 4MB
>> already. It was not always this way. Prior to
>> commit 7b5d0fe4440c ("Increase heap limit") in 2010, HEAP_MAX_SIZE and
>> HEAP_MAX_ADDR were indeed 4MB. However, when the constants were increased
>> the comment was left unchanged.
>>
>> It's been over a decade. It doesn't seem like we have problems with
>> claims over 4MB on powerpc or x86 ieee1275. (sparc does things completely
>> differently and never used the constant.)
>>
>> Drop the constant and the check.
>>
>> The only use of HEAP_MIN_SIZE was to potentially override the
>> HEAP_MAX_ADDR check. It is now unused. Remove it.
>>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> Nit, you should add your Signed-off-by after Daniel's one. I can make it
> for you if you do not object.
I do not object.
> Otherwise: Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Thanks.


>
> Daniel

[PATCH v2 2/4] ieee1275: claim more memory

[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

Re: [PATCH v2 2/4] ieee1275: claim more memory

[Daniel Axtens]

Stefan Berger <stefanb@linux.vnet.ibm.com> writes:

> 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;

(looking back over my code to test the runtime claim model on powerpc)
This change accidentally breaks the fixed claim mode. We should keep this change, but ...

> +
>    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);

... here we should initialise total to GRUB_IEEE1275_STATIC_HEAP_LEN first.

I'll do this change when I next do a revision of the series, but Stefan
if you send something with my patches attached before I send the next
revision please include this.

(We should have a discussion elsewhere about dropping some of the
ancient platforms we support here. FORCE_CLAIM is only used for
OpenHackware, which was last modified in 2014 and had qemu support
deprecated in qemu v3.1 and removed in Feb 2020. FORCE_CLAIM requires
special case code in at least 4 places. This seems like poor value for
complexity.)

> -  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

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

[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 73e2f4644..18c479b66 100644
--- a/include/grub/ieee1275/ieee1275.h
+++ b/include/grub/ieee1275/ieee1275.h
@@ -148,6 +148,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

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

[Stefan Berger]

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.

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 | 169 ++++++++++++++++++++++++++
 3 files changed, 178 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..d61cb111a
--- /dev/null
+++ b/grub-core/commands/ieee1275/ibmvtpm.c
@@ -0,0 +1,169 @@
+/*
+ *  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>
+
+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;
+};
+
+#define IEEE1275_CELL_TRUE     ((grub_ieee1275_cell_t) -1)
+
+static grub_ieee1275_ihandle_t tpm_ihandle;
+static grub_uint8_t tpm_version;
+
+static grub_ieee1275_ihandle_t
+tpm_init (void)
+{
+  static int init_called = 0;
+
+  if (!init_called)
+    {
+      init_called = 1;
+      grub_ieee1275_open ("/vdevice/vtpm", &tpm_ihandle);
+    }
+
+  return tpm_ihandle;
+}
+
+static void
+tpm_get_tpm_version (void)
+{
+  static int version_probed = 0;
+  grub_ieee1275_phandle_t vtpm;
+  char buffer[20];
+  grub_ssize_t buffer_size;
+
+  if (!version_probed)
+    {
+      version_probed = 1;
+      if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm) &&
+	  !grub_ieee1275_get_property (vtpm, "compatible", buffer,
+				       sizeof buffer, &buffer_size) &&
+	  !grub_strcmp (buffer, "IBM,vtpm20"))
+	{
+	  tpm_version = 2;
+	}
+    }
+}
+
+static grub_err_t
+tpm_handle_find (grub_ieee1275_ihandle_t *tpm_handle)
+{
+  *tpm_handle = tpm_init ();
+  if (*tpm_handle == 0)
+      return GRUB_ERR_UNKNOWN_DEVICE;
+
+  tpm_get_tpm_version ();
+
+  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 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 != IEEE1275_CELL_TRUE)
+      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;
+  int err;
+
+  err = ibmvtpm_2hash_ext_log (tpm_handle,
+			       pcr, EV_IPL,
+			       description,
+			       grub_strlen(description) + 1,
+			       buf, size);
+  if (err && error_displayed < 1)
+    {
+      error_displayed++;
+      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;
+}
-- 
2.25.1

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

[Daniel Kiper]

On Tue, Jul 20, 2021 at 05:14:49PM -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.
>
> 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 | 169 ++++++++++++++++++++++++++
>  3 files changed, 178 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..d61cb111a
> --- /dev/null
> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
> @@ -0,0 +1,169 @@
> +/*
> + *  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>
> +
> +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;
> +};

Huh! It looks that the struct should be defined in ibmvtpm_2hash_ext_log()
as it is done in the similar GRUB IEEE1275 code. Please move it back there.
Sorry for the confusion.

> +#define IEEE1275_CELL_TRUE     ((grub_ieee1275_cell_t) -1)

This smells like global constant. Does not it? If yes could you define it
in a global header and use it? Maybe even replace existing comparisons
in the IEEE1275 code with IEEE1275_CELL_TRUE. But probably then
s/IEEE1275_CELL_TRUE/GRUB_IEEE1275_CELL_TRUE/...

> +static grub_ieee1275_ihandle_t tpm_ihandle;
> +static grub_uint8_t tpm_version;
> +
> +static grub_ieee1275_ihandle_t
> +tpm_init (void)
> +{
> +  static int init_called = 0;
> +
> +  if (!init_called)
> +    {
> +      init_called = 1;

I would move this behind grub_ieee1275_open() call.

> +      grub_ieee1275_open ("/vdevice/vtpm", &tpm_ihandle);

Why do you ignore status returned by grub_ieee1275_open()?

> +    }
> +
> +  return tpm_ihandle;
> +}
> +
> +static void
> +tpm_get_tpm_version (void)
> +{
> +  static int version_probed = 0;
> +  grub_ieee1275_phandle_t vtpm;
> +  char buffer[20];
> +  grub_ssize_t buffer_size;
> +
> +  if (!version_probed)
> +    {
> +      version_probed = 1;

I would move version_probed behind this if.

> +      if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm) &&
> +	  !grub_ieee1275_get_property (vtpm, "compatible", buffer,
> +				       sizeof buffer, &buffer_size) &&

s/sizeof buffer/sizeof (buffer)/

And you can use NULL instead of buffer_size.

> +	  !grub_strcmp (buffer, "IBM,vtpm20"))
> +	{
> +	  tpm_version = 2;
> +	}

Please drop these curly brackets.

> +    }
> +}
> +
> +static grub_err_t
> +tpm_handle_find (grub_ieee1275_ihandle_t *tpm_handle)
> +{
> +  *tpm_handle = tpm_init ();
> +  if (*tpm_handle == 0)
> +      return GRUB_ERR_UNKNOWN_DEVICE;
> +
> +  tpm_get_tpm_version ();
> +
> +  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 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;

Incorrect alignment of return.

> +  /*
> +   * catch_result is set if firmware does not support 2hash-ext-log
> +   * rc is TRUE (-1) on success
> +   */
> +  if ((args.catch_result) || args.rc != IEEE1275_CELL_TRUE)
> +      return -1;

Ditto.

> +  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;

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 < 1)

s/error_displayed < 1/!error_displayed/

> +    {
> +      error_displayed++;
> +      grub_error (GRUB_ERR_BAD_DEVICE,

s/grub_error/return grub_error/

> +	      "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;

Incorrect alignment of return.

> +  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);

Ditto.

Daniel

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

[Stefan Berger]

On 7/28/21 9:25 AM, Daniel Kiper wrote:
> On Tue, Jul 20, 2021 at 05:14:49PM -0400, Stefan Berger wrote:
>
>> +#define IEEE1275_CELL_TRUE     ((grub_ieee1275_cell_t) -1)
> This smells like global constant. Does not it? If yes could you define it
> in a global header and use it? Maybe even replace existing comparisons
> in the IEEE1275 code with IEEE1275_CELL_TRUE. But probably then
> s/IEEE1275_CELL_TRUE/GRUB_IEEE1275_CELL_TRUE/...

I wasn't sure and also had found local #defines in another .c file.

#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)

https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/ieee1275/ieee1275.c#n24

I haven't seen the usage of -1 as TRUE in other grub files. So I could 
move it to a global header file assuming this is commonly used on this 
platform. I have only seen usage of -1 as TRUE in the SLOF firmware in 
this file here:

https://github.com/aik/SLOF/blob/master/lib/libnvram/libnvram.code#L66

but then of course also here: 
https://github.com/aik/SLOF/blob/master/lib/libtpm/tcgbios.c#L965


    Stefan

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

[Daniel Kiper]

On Thu, Jul 29, 2021 at 09:30:49AM -0400, Stefan Berger wrote:
> On 7/28/21 9:25 AM, Daniel Kiper wrote:
> > On Tue, Jul 20, 2021 at 05:14:49PM -0400, Stefan Berger wrote:
> >
> > > +#define IEEE1275_CELL_TRUE     ((grub_ieee1275_cell_t) -1)
> > This smells like global constant. Does not it? If yes could you define it
> > in a global header and use it? Maybe even replace existing comparisons
> > in the IEEE1275 code with IEEE1275_CELL_TRUE. But probably then
> > s/IEEE1275_CELL_TRUE/GRUB_IEEE1275_CELL_TRUE/...
>
> I wasn't sure and also had found local #defines in another .c file.
>
> #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)
>
> https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/ieee1275/ieee1275.c#n24
>
> I haven't seen the usage of -1 as TRUE in other grub files. So I could move
> it to a global header file assuming this is commonly used on this platform.
> I have only seen usage of -1 as TRUE in the SLOF firmware in this file here:
>
> https://github.com/aik/SLOF/blob/master/lib/libnvram/libnvram.code#L66
>
> but then of course also here:
> https://github.com/aik/SLOF/blob/master/lib/libtpm/tcgbios.c#L965

I think we should use IEEE1275_CELL_INVALID in your code. IMO the TRUE
sounds a bit confusing in this context. So, I would move all these
three constants to a global IEEE1275 header and add "GRUB_" prefix.

Daniel

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

[Stefan Berger]

On 7/30/21 8:44 AM, Daniel Kiper wrote:
> On Thu, Jul 29, 2021 at 09:30:49AM -0400, Stefan Berger wrote:
>> On 7/28/21 9:25 AM, Daniel Kiper wrote:
>>> On Tue, Jul 20, 2021 at 05:14:49PM -0400, Stefan Berger wrote:
>>>
>>>> +#define IEEE1275_CELL_TRUE     ((grub_ieee1275_cell_t) -1)
>>> This smells like global constant. Does not it? If yes could you define it
>>> in a global header and use it? Maybe even replace existing comparisons
>>> in the IEEE1275 code with IEEE1275_CELL_TRUE. But probably then
>>> s/IEEE1275_CELL_TRUE/GRUB_IEEE1275_CELL_TRUE/...
>> I wasn't sure and also had found local #defines in another .c file.
>>
>> #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)
>>
>> https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/ieee1275/ieee1275.c#n24
>>
>> I haven't seen the usage of -1 as TRUE in other grub files. So I could move
>> it to a global header file assuming this is commonly used on this platform.
>> I have only seen usage of -1 as TRUE in the SLOF firmware in this file here:
>>
>> https://github.com/aik/SLOF/blob/master/lib/libnvram/libnvram.code#L66
>>
>> but then of course also here:
>> https://github.com/aik/SLOF/blob/master/lib/libtpm/tcgbios.c#L965
> I think we should use IEEE1275_CELL_INVALID in your code. IMO the TRUE
> sounds a bit confusing in this context. So, I would move all these

TRUE doesn't sound like CELL_INVALID and vice versa. FALSE is defined as 
0, so I think that's a better comparison (and as a value more 'common') 
after introducing GRUB_IEEE1275_CELL_FALSE.

> three constants to a global IEEE1275 header and add "GRUB_" prefix.
I'll move those three in a separate patch.
> Daniel


    stefan