[PATCH v3 10/18] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL

[Simon Glass]

Hi Ilias,

On Thu, 20 Jun 2024 at 23:49, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Fri, 21 Jun 2024 at 08:32, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Fri, 21 Jun 2024 at 02:06, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > It does not make sense to enable all SHA algorithms unless they are
> > > needed. It bloats the code and in this case, causes chromebook_link to
> > > fail to build. That board does use the TPM, but not with measured boot,
> > > nor EFI.
> > >
> > > Since EFI_TCG2_PROTOCOL already selects these options, we just need to
> > > add them to MEASURED_BOOT as well.
> > >
> > > Note that the original commit combines refactoring and new features,
> > > which makes it hard to see what is going on.
> > >
> > > Fixes: 97707f12fda tpm: Support boot measurements
> > > Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > (no changes since v2)
> >
> > There was a discussion in the previous version, bout enabling these on
> > CMD_TPM as they are required.

For reference [1]

>
> Or switch this to an imply instead so you can disable it ?

The thing is, we need to be able to easily disable unwanted features.
My reading of that thread is that U-Boot doesn't use SHA384/512 from
the 'tpm2 pcr_extend' command today. Even if it did, I would think it
better to allow the supported algorithms to be controlled. At worst, I
would expect a separate option to disable that subcommand which would
remove the implies.

Some tpm code recently added puts a table and some functions in
tpm-v2.h - could someone fix that? tpm-common.c would be a better
place. Also we already have a list of hash algorithms in U-Boot
(common/hash.c) so should use that, and respect the fact that certain
algos may not be supported.

If there is a requirement that (for a certain situation / protocol) we
need to support everything, then let's have a Kconfig for that. It is
what my patch was trying to work around, perhaps. For boards which
don't care about extending TPM in this way, or don't use the more
expensive algorithms, it avoids lots of code bloat.

One of the principles of U-Boot is to allow configurability in these
situations. It shouldn't be too hard to have the best of both worlds
(supports all reasonable cases by default / allow unwanted cases to be
dropped).

So I'll await your reply on the above before figuring out where to go
with this. For this release I'd just like to get the board working,
but I understand that the TPM stuff is very-much in flux, perhaps in
-next.

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20240610145920.3302001-3-sjg@chromium.org/