Hi Heinrich, On Mon, 13 Jan 2025 at 12:52, Heinrich Schuchardt <[email protected]> wrote:
> On 13.01.25 15:43, Raymond Mao wrote: > > Hi Heinrich, > > > > On Fri, 10 Jan 2025 at 19:12, Heinrich Schuchardt <[email protected] > > <mailto:[email protected]>> wrote: > > > > Am 10. Januar 2025 22:56:35 MEZ schrieb Raymond Mao > > <[email protected] <mailto:[email protected]>>: > > >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 <[email protected] > > <mailto:[email protected]>> > > >--- > > >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; > > > } > > > >
