Hi Raymond, On Mon, 16 Dec 2024 at 08:29, Raymond Mao <raymond....@linaro.org> wrote: > > Hi Simon, > > On Mon, 16 Dec 2024 at 10:20, Simon Glass <s...@chromium.org> wrote: >> >> Hi Raymond, >> >> On Mon, 16 Dec 2024 at 07:52, Raymond Mao <raymond....@linaro.org> wrote: >> > >> > Hi Simon, >> > >> > On Sun, 15 Dec 2024 at 19:25, Simon Glass <s...@chromium.org> wrote: >> >> >> >> Hi Raymond, >> >> >> >> On Fri, 13 Dec 2024 at 15:56, Raymond Mao <raymond....@linaro.org> wrote: >> >> > >> >> > Get tpm event log from bloblist instead of FDT when bloblist is >> >> > enabled and valid from previous boot stage. >> >> > >> >> > Note: >> >> > This patch depends on: >> >> > [PATCH 1/2] bloblist: Introduce BLOBLIST_PRIOR_STAGE options >> >> > https://lore.kernel.org/u-boot/20241212151142.1562825-1-tr...@konsulko.com/ >> >> > >> >> > Signed-off-by: Raymond Mao <raymond....@linaro.org> >> >> > --- >> >> > lib/tpm_tcg2.c | 7 +++++++ >> >> > 1 file changed, 7 insertions(+) >> >> > >> >> > diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c >> >> > index 7f868cc883..acaf0acb88 100644 >> >> > --- a/lib/tpm_tcg2.c >> >> > +++ b/lib/tpm_tcg2.c >> >> > @@ -19,6 +19,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) >> >> > @@ -672,6 +673,12 @@ __weak int tcg2_platform_get_log(struct udevice >> >> > *dev, void **addr, u32 *size) >> >> > *addr = NULL; >> >> > *size = 0; >> >> > >> >> > + if (CONFIG_IS_ENABLED(BLOBLIST_PRIOR_STAGE) && >> >> > !bloblist_maybe_init()) { >> >> >> >> Please drop the second term...the bloblist is inited by U-Boot already. >> >> >> > >> > As an independent library, from the tpm library's angle, whether the >> > bloblist init was already called or not is agnostic. >> > I think this is the purpose of the "maybe" function. >> >> No. U-Boot does init at the start. We don't want each subsystem >> 'maybe' initing each other subsystem that it uses. If you want to >> check whether bloblist is available, you can use the GD flag. >> > > OK. I can use the gd instead. > >> >> > >> >> >> >> Also please drop the first term. If you find it in the bloblist, just use >> >> it. >> >> >> > >> > I think here we should have the same logic as the existing one in fdtdec: >> > If bloblist is not enabled or errors observed, do a fallback to the >> > original process. >> > At least I want to keep the same logic in different places before >> > everything is ready from TF-A to U-boot. >> >> Yes, your second line is right. But PRIOR_STAGE doesn't exist, nor >> should it. Nor do you need it, since you can just check the bloblist >> for what you want. If you don't find it, fall back. >> > > Since Firmware Handoff is still an experimental feature and not mandatory yet > (at least now), > I put everything under PRIOR_STAGE to allow users to disable it by their own > decisions. > I saw PRIOR_STAGE in one of Tom's recent patches so I have added a dependency > claim in my commit message.
Yes, but even if Tom does pick his patch, which I object to, by the time your code is called, there will be a bloblist. So the end result of your test is to allow the user to ignore any TPM log in the bloblist. If that's what you want (which seems fine to me), add a Kconfig for it. This is the sort of confusion that I find very frustrating. At least with OF_BLOBLIST it was clear that it related to devicetree only... Regards, Simon
