
Hi Heinrich,
On Mon, 13 Jan 2025 at 12:52, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 13.01.25 15:43, Raymond Mao wrote:
Hi Heinrich,
On Fri, 10 Jan 2025 at 19:12, Heinrich Schuchardt <xypron.glpk@gmx.de mailto:xypron.glpk@gmx.de> wrote:
Am 10. Januar 2025 22:56:35 MEZ schrieb Raymond Mao <raymond.mao@linaro.org <mailto:raymond.mao@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@linaro.org <mailto:raymond.mao@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?
By the inline function, we call bloblist_get_blob and we can get the result regardless whether BLOBLIST is selected or not. What I mean above is, "else if (CONFIG_IS_ENABLED(BLOBLIST))" is a temporary solution before we solve it with a new kconfig for "handoff from the previous boot stage is mandatory". Finally it will be replaced by "else if (CONFIG_IS_ENABLED(<NEW_KCONFIG>))" here, which is not directly related to BLOBLIST. Aka, enabling BLOBLIST does not mean we force "handoff from the previous boot stage using bloblist", since a bloblist can be generated natively inside U-Boot instead of passing from a previous boot stage.
I can add a TODO comment into this line to address the potential confusions.
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; > }