Hi Tom, On Mon, 17 Jun 2024 at 11:16, Tom Rini <tr...@konsulko.com> wrote: > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > Hi, > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > <ilias.apalodi...@linaro.org> wrote: > > > > > > Hi Heinrich > > > > > > resending the reply, I accidentally sent half of the message... > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt <xypron.g...@gmx.de> > > > wrote: > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > <xypron.g...@gmx.de> wrote: > > > > >> > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > >>> Hi Simon, > > > > >>> > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass <s...@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 > > > > >>>> Signed-off-by: Simon Glass <s...@chromium.org> > > > > >>>> --- > > > > >>>> > > > > >>>> Changes in v2: > > > > >>>> - Put the conditions under EFI_TCG2_PROTOCOL > > > > >>>> - Consider MEASURED_BOOT too > > > > >>>> > > > > >>>> boot/Kconfig | 4 ++++ > > > > >>>> lib/Kconfig | 4 ---- > > > > >>>> 2 files changed, 4 insertions(+), 4 deletions(-) > > > > >>>> > > > > >>>> diff --git a/boot/Kconfig b/boot/Kconfig > > > > >>>> index 6f3096c15a6..b061891e109 100644 > > > > >>>> --- a/boot/Kconfig > > > > >>>> +++ b/boot/Kconfig > > > > >>>> @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > >>>> config MEASURED_BOOT > > > > >>>> bool "Measure boot images and configuration when booting > > > > >>>> without EFI" > > > > >>>> depends on HASH && TPM_V2 > > > > >>>> + select SHA1 > > > > >>>> + select SHA256 > > > > >>>> + select SHA384 > > > > >>>> + select SHA512 > > > > >>>> help > > > > >>>> This option enables measurement of the boot process > > > > >>>> when booting > > > > >>>> without UEFI . Measurement involves creating > > > > >>>> cryptographic hashes > > > > >>>> diff --git a/lib/Kconfig b/lib/Kconfig > > > > >>>> index 189e6eb31aa..568892fce44 100644 > > > > >>>> --- a/lib/Kconfig > > > > >>>> +++ b/lib/Kconfig > > > > >>>> @@ -438,10 +438,6 @@ config TPM > > > > >>>> bool "Trusted Platform Module (TPM) Support" > > > > >>>> depends on DM > > > > >>>> imply DM_RNG > > > > >>>> - select SHA1 > > > > >>>> - select SHA256 > > > > >>>> - select SHA384 > > > > >>>> - select SHA512 > > > > >>> > > > > >>> I am not sure this is the right way to deal with your problem. > > > > >>> The TPM main functionality is to measure and extend PCRs, so shaXXXX > > > > >>> is really required. To make things even worse, you don't know the > > > > >>> PCR > > > > >>> banks that are enabled beforehand. This is a runtime config of the > > > > >>> TPM. > > > > >> > > > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot > > > > >> cannot extend PCRs. So it seems fine to let these two select the > > > > >> complete set of hashing algorithms. As Simon pointed out for > > > > >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. > > > > > > > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8 > > > > > 0xb0000000 > > > > That's pretty normal for U-Boot though, since we want to avoid lots of > > growth for things people might want control over. We can enable or > > disable the SHA for the board, if this functionality is used outside > > of measured boot and tcg2, but someone is enabling the tpm command. > > > > > > > > > > So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. > > > > > > > > TPM v1 only needs SHA-1. > > > > > > I still prefer to imply all algos. > > > > 'imply' would be OK in this case as I can disable it for that board. I > > don't think it is in the spirit of U-Boot though. > > > > isn't someone checking the growth in U-Boot? Or do so few boards have > > TPMs that it didn't register? The size growth was 3.2KB on > > chromebook_link. > > As always, yes, nearly every PR (I don't check the ones that touch just > a single board for example) gets a world build before/after. In this > case I likely assumed that it was acceptable growth for enabling > features. It sounds like some of the chromebook boards need to be > setting the features to cause link failure if a size is exceeded?
The problem is that some Intel platforms have binary blobs, so the size isn't known unless you have a real blob. Regards, Simon > > -- > Tom
