Hi Ilias,
On Thu, 20 Jun 2024 at 23:49, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Fri, 21 Jun 2024 at 08:32, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
On Fri, 21 Jun 2024 at 02:06, Simon Glass sjg@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@gmx.de Signed-off-by: Simon Glass sjg@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-sj...