From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ilias Apalodimas Date: Sun, 29 Nov 2020 15:27:43 +0200 Subject: [PATCH 2/3] efi_loader: Introduce eventlog support for TCG2_PROTOCOL In-Reply-To: References: <20201127162932.1965323-1-ilias.apalodimas@linaro.org> <20201127162932.1965323-3-ilias.apalodimas@linaro.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sun, Nov 29, 2020 at 07:02:39AM +0100, Heinrich Schuchardt wrote: > On 11/27/20 5:29 PM, Ilias Apalodimas wrote: > > In the previous patches we only introduced a minimal subset of the > > [...] > > +#define TPM2_SHA1_DIGEST_SIZE 20 > > +#define TPM2_SHA256_DIGEST_SIZE 32 > > +#define TPM2_SHA384_DIGEST_SIZE 48 > > +#define TPM2_SHA512_DIGEST_SIZE 64 > > lib/efi_loader/efi_tcg2.c already includes tpm-v2.h. > > Why should we define the same constant twice? > We don't. That was probably a c/p I forgot to fix when I creted the patches. I'll only keep the declarations in tpm-v2.h. > Best regards > > Heinrich > > > + > > +#define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1 > > + > > +#define TPM2_EVENT_LOG_SIZE 4096 > > What does this size derive from? A comment describing the constant could > help. > There's no guidance for this. This is the size of the eventlog and it's up to us to define something that makes sense. It obviously depends on: - Number of supported algoriths - Number of events We could move it to a Kconfig? > > + > > typedef u32 efi_tcg_event_log_bitmap; > > typedef u32 efi_tcg_event_log_format; > > typedef u32 efi_tcg_event_algorithm_bitmap; > > @@ -65,6 +76,40 @@ struct efi_tcg2_boot_service_capability { > > sizeof(struct efi_tcg2_boot_service_capability) - \ > > offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks) > > > > +#define tcg_efi_spec_id_event_signature_03 "Spec ID Event03" > > + > > +#define tcg_efi_spec_id_event_spec_version_major_tpm2 2 > > +#define tcg_efi_spec_id_event_spec_version_minor_tpm2 0 > > +#define tcg_efi_spec_id_event_spec_version_errata_tpm2 0 > > Constants should be capitalized. > Ok > > + > > Please, provide Sphinx style comments for structures. Cf. > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation > Ok > > +struct tcg_efi_spec_id_event_algorithm_size { > > + u16 algorithm_id; > > + u16 digest_size; > > +} __packed; > > + > > +struct tcg_efi_spec_id_event { > > + u8 signature[16]; > > + u32 platform_class; > > + u8 spec_version_minor; > > + u8 spec_version_major; > > + u8 spec_errata; > > + u8 uintn_size; > > + u32 number_of_algorithms; > > + struct tcg_efi_spec_id_event_algorithm_size digest_sizes[TPM2_NUM_PCR_BANKS]; > > + u8 vendor_info_size; > > + /* > > + * filled in with vendor_info_size > > + * currently vendor_info_size = 0 > > %s/vendor_info_size = 0/U-Boot does not provide any vendor info/ > Ok [...] > > + /* Setup specID event data */ > > + spec_event = (struct tcg_efi_spec_id_event *)buffer; > > + memcpy(spec_event->signature, tcg_efi_spec_id_event_signature_03, > > + sizeof(spec_event->signature)); > > + put_unaligned_le32(0, &spec_event->platform_class); /* type client */ > > + __put_unaligned_le(tcg_efi_spec_id_event_spec_version_minor_tpm2, > > + &spec_event->spec_version_minor); > > + __put_unaligned_le(tcg_efi_spec_id_event_spec_version_major_tpm2, > > + &spec_event->spec_version_major); > > + __put_unaligned_le(tcg_efi_spec_id_event_spec_version_errata_tpm2, > > + &spec_event->spec_errata); > > + __put_unaligned_le(sizeof(efi_uintn_t) / sizeof(u32), > > + &spec_event->uintn_size); Any preference on this? The put_unaligned here is not strictly needed since it's u8 values. It just seemed 'easier' to read since all the other additions to the log are done with put_unaligned16/32. [...] cheerrs /Ilias