[PATCH v4 3/3] tpm: get tpm event log from bloblist

[Heinrich Schuchardt]

On 13.01.25 15:43, Raymond Mao wrote:
> Hi Heinrich,
>
> On Fri, 10 Jan 2025 at 19:12, Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at gmx.de>> wrote:
>
>     Am 10. Januar 2025 22:56:35 MEZ schrieb Raymond Mao
>     <raymond.mao at linaro.org <mailto:raymond.mao at linaro.org>>:
>      >Get tpm event log from bloblist instead of FDT when bloblist is
>      >enabled and valid from previous boot stage.
>      >
>      >As a fallback, when no event log from previous stage is observed
>      >and no user buffer is passed, malloc a default buffer to initialize
>      >the event log.
>      >
>      >Signed-off-by: Raymond Mao <raymond.mao at linaro.org
>     <mailto:raymond.mao at linaro.org>>
>      >---
>      >Changes in v2
>      >- Remove patch dependency.
>      >- Remove the fallback to FDT when BLOBLIST is selected.
>      >Changes in v3
>      >- Malloc an 8KB buffer when user eventlog buffer does not exist.
>      >Changes in v4
>      >- Replace the default eventlog size with TPM2_EVENT_LOG_SIZE.
>      >
>      > lib/tpm_tcg2.c | 55 ++++++++++++++++++++++++++++++++
>     +-----------------
>      > 1 file changed, 36 insertions(+), 19 deletions(-)
>      >
>      >diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c
>      >index 7f868cc883..685699688b 100644
>      >--- a/lib/tpm_tcg2.c
>      >+++ b/lib/tpm_tcg2.c
>      >@@ -5,6 +5,7 @@
>      >
>      > #include <dm.h>
>      > #include <dm/of_access.h>
>      >+#include <malloc.h>
>      > #include <tpm_api.h>
>      > #include <tpm-common.h>
>      > #include <tpm-v2.h>
>      >@@ -19,6 +20,7 @@
>      > #include <linux/unaligned/generic.h>
>      > #include <linux/unaligned/le_byteshift.h>
>      > #include "tpm-utils.h"
>      >+#include <bloblist.h>
>      >
>      > int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_pcr,
>     u32 *active_pcr,
>      >                     u32 *pcr_banks)
>      >@@ -607,15 +609,24 @@ int tcg2_log_prepare_buffer(struct udevice
>     *dev, struct tcg2_event_log *elog,
>      >               elog->found = log.found;
>      >       }
>      >
>      >+      if (elog->found)
>      >+              return 0;
>      >+
>      >       /*
>      >-       * Initialize the log buffer if no log was discovered and
>     the buffer is
>      >-       * valid. User's can pass in their own buffer as a fallback
>     if no
>      >-       * memory region is found.
>      >+       * Initialize the log buffer if no log was discovered.
>      >+       * User can pass in their own buffer as a fallback if no
>     memory region
>      >+       * is found, else malloc a buffer if it does not exist.
>      >        */
>      >-      if (!elog->found && elog->log_size)
>      >-              rc = tcg2_log_init(dev, elog);
>      >+      if (!elog->log_size) {
>      >+              elog->log = malloc(TPM2_EVENT_LOG_SIZE);
>      >+              if (!elog->log)
>      >+                      return -ENOMEM;
>      >+
>      >+              memset(elog->log, 0, TPM2_EVENT_LOG_SIZE);
>      >+              elog->log_size = TPM2_EVENT_LOG_SIZE;
>      >+      }
>      >
>      >-      return rc;
>      >+      return tcg2_log_init(dev, elog);
>      > }
>      >
>      > int tcg2_measurement_init(struct udevice **dev, struct
>     tcg2_event_log *elog,
>      >@@ -668,10 +679,19 @@ __weak int tcg2_platform_get_log(struct
>     udevice *dev, void **addr, u32 *size)
>      >       const __be32 *size_prop;
>      >       int asize;
>      >       int ssize;
>      >+      struct ofnode_phandle_args args;
>      >+      phys_addr_t a;
>      >+      fdt_size_t s;
>      >
>      >       *addr = NULL;
>      >       *size = 0;
>      >
>      >+      *addr = bloblist_get_blob(BLOBLISTT_TPM_EVLOG, size);
>      >+      if (*addr && *size)
>      >+              return 0;
>      >+      else if (CONFIG_IS_ENABLED(BLOBLIST))
>      >+              return -ENODEV;
>      >+
>
>     You are querying the CONFIG value. Why call function
>     bloblist_get_blob if blobs are not supported? Please, simply skip in
>     this case.
>
> Actually BLOBLIST is not required to call bloblist_get_blob here since I
> have added the inline function, but we need a kconfig to separate the
> user's preference on "fallback to DT" or not.
> BLOBLIST is not a perfect idea but a temporary solution before we have a
> concrete idea to define one new kconfig for "blobs handoff from previous
> boot stage is mandatory".
> For more information, please see the previous discussion between me, Tom
> and Simon in v1 of this patch.

Why should we call the empty inline function, if we already check the
CONFIG value?

Best regards

Heinrich

>
> Regards,
> Raymond
>
>
>
>      >       addr_prop = dev_read_prop(dev, "tpm_event_log_addr", &asize);
>      >       if (!addr_prop)
>      >               addr_prop = dev_read_prop(dev, "linux,sml-base",
>     &asize);
>      >@@ -686,22 +706,19 @@ __weak int tcg2_platform_get_log(struct
>     udevice *dev, void **addr, u32 *size)
>      >
>      >               *addr = map_physmem(a, s, MAP_NOCACHE);
>      >               *size = (u32)s;
>      >-      } else {
>      >-              struct ofnode_phandle_args args;
>      >-              phys_addr_t a;
>      >-              fdt_size_t s;
>      >
>      >-              if (dev_read_phandle_with_args(dev, "memory-
>     region", NULL, 0,
>      >-                                             0, &args))
>      >-                      return -ENODEV;
>      >+              return 0;
>      >+      }
>      >
>      >-              a = ofnode_get_addr_size(args.node, "reg", &s);
>      >-              if (a == FDT_ADDR_T_NONE)
>      >-                      return -ENOMEM;
>      >+      if (dev_read_phandle_with_args(dev, "memory-region", NULL,
>     0, 0, &args))
>      >+              return -ENODEV;
>      >
>      >-              *addr = map_physmem(a, s, MAP_NOCACHE);
>      >-              *size = (u32)s;
>      >-      }
>      >+      a = ofnode_get_addr_size(args.node, "reg", &s);
>      >+      if (a == FDT_ADDR_T_NONE)
>      >+              return -ENOMEM;
>      >+
>      >+      *addr = map_physmem(a, s, MAP_NOCACHE);
>      >+      *size = (u32)s;
>      >
>      >       return 0;
>      > }
>