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. > > > > [snip] > > REgards, > Simon >