[PATCH 2/2] tpm: get tpm event log from bloblist

[Simon Glass]

Hi Raymond,

On Mon, 16 Dec 2024 at 08:29, Raymond Mao <raymond.mao at linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, 16 Dec 2024 at 10:20, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Raymond,
>>
>> On Mon, 16 Dec 2024 at 07:52, Raymond Mao <raymond.mao at linaro.org> wrote:
>> >
>> > Hi Simon,
>> >
>> > On Sun, 15 Dec 2024 at 19:25, Simon Glass <sjg at chromium.org> wrote:
>> >>
>> >> Hi Raymond,
>> >>
>> >> On Fri, 13 Dec 2024 at 15:56, Raymond Mao <raymond.mao at 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-trini@konsulko.com/
>> >> >
>> >> > Signed-off-by: Raymond Mao <raymond.mao at 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