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

Thu Dec 19 17:36:04 CET 2024

On Thu, Dec 19, 2024 at 10:37:37AM -0500, Raymond Mao wrote:
> Hi Simon,
> 
> On Thu, 19 Dec 2024 at 10:08, Simon Glass <sjg at chromium.org> wrote:
> 
> > Hi,
> >
> > On Mon, 16 Dec 2024 at 11:30, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Mon, Dec 16, 2024 at 10:37:30AM -0700, Simon Glass wrote:
> > > > Hi Raymond,
> > > >
> > > > On Mon, 16 Dec 2024 at 10:24, Raymond Mao <raymond.mao at linaro.org>
> > wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Mon, 16 Dec 2024 at 10:43, Simon Glass <sjg at chromium.org> wrote:
> > > > >>
> > > > >> 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.
> > > > >>
> > > > >
> > > > > Per my understanding of PRIOR_STAGE, it means we have a bloblist
> > from the previous stage and we want to use it (as a Transfer List).
> > > > > If this is the case, it is not for DT only but can be applied for
> > eventlog or anything for Firmware Handoff in the future.
> > > > > That is the reason I use PRIOR_STAGE here, I don't think we need to
> > add another kconfig.
> > > >
> > > > Who knows what it is supposed to mean...anyway, it should not be used
> > > > like that. The devicetree is special in that we want to get it from
> > > > the bloblist very early, if available. Nothing else (so far) in the
> > > > bloblist has this constraint, certainly not the TPM log.
> > >
> > > Raymond is exactly right, and the device tree is not special.
> >
> > That is quite wrong IMO. Devicetree is special because we need it
> > early and it might come from bloblist before the bloblist has been set
> > up by U-Boot. It also controls the whole operation of U-Boot. There is
> > nothing else in U-Boot quite like the devicetree.
> >
> > > The
> > > specialness is "do we have a valid bloblist upon entry or not". Which
> > > means that also no, this series (since it happens late enough) shouldn't
> > > care about if the previous stage had created it or not, merely as you
> > > said if the tag for the TPM (or other blob) is found.
> >
> > I would feel more comfortable having the discussions about what
> > previous stages need to pass U-Boot a bloblist once we have that
> > situation. This particular patch seems clear enough, and well
> > motivated. It just needs to drop the conditions.
> >
> I think we still need a condition here indicating that the kconfig of
> "handoff via bloblist" is enabled.
> And this "handoff via bloblist" kconfig should mean to handoff everything
> via bloblist, including DT.

There's at least two sets of challenges here. One, being solved by
vexpress64 right now, is that we didn't have CONFIG_BLOBLIST_PASSAGE as
an actual option. And in that case, there's no U-Boot before full U-Boot
and the bloblist exists for us. Two, U-Boot is what is creating the
bloblist. The contentious parts are *when* it's created and *where* it
resides prior to full U-Boot seeing it.

In both cases, full U-Boot will copy the bloblist to where it likes.

And this is separate from "bloblist has device tree or we hang()".

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241219/d75368ec/attachment.sig>