On 1/12/23 17:43, Simon Glass wrote:
Hi Eddie,
On Thu, 12 Jan 2023 at 09:16, Eddie James eajames@linux.ibm.com wrote:
Add TPM2 functions to support boot measurement. This includes starting up the TPM, initializing/appending the event log, and measuring the U-Boot version. Much of the code was used in the EFI subsystem, so remove it there and use the common functions.
Signed-off-by: Eddie James eajames@linux.ibm.com
include/efi_tcg2.h | 44 --- include/tpm-v2.h | 211 ++++++++++++ lib/efi_loader/efi_tcg2.c | 362 +------------------ lib/tpm-v2.c | 708 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 938 insertions(+), 387 deletions(-)
[..]
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 697b982e07..00e1b04d74 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -4,13 +4,597 @@
- Author: Miquel Raynal miquel.raynal@bootlin.com
*/
+#include <asm/types.h> +#include <asm/io.h>
Please check header order:
https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-fil...
Sure, however I did have a compile error with sandbox build due to missing phys_addr_t definition in asm/io.h...
#include <common.h> #include <dm.h> +#include <dm/of_access.h> #include <tpm-common.h> #include <tpm-v2.h> #include <linux/bitops.h> +#include <linux/unaligned/be_byteshift.h> +#include <linux/unaligned/generic.h> +#include <linux/unaligned/le_byteshift.h> +#include <u-boot/sha1.h> +#include <u-boot/sha256.h> +#include <u-boot/sha512.h> +#include <version_string.h> #include "tpm-utils.h"
[..]
+static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog) +{
struct tcg_efi_spec_id_event *ev;We cannot add EFI things to generic TPM code.
Ah, this is NOT an EFI thing even though it is named as such. Please see https://trustedcomputinggroup.org/wp-content/uploads/TCG_ServerManagDomainFW... section 9 and https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Specific_P... section 9
Neither of these documents are specific to EFI.
struct tcg_pcr_event *log;u32 event_size;u32 count = 0;u32 log_size;u32 active;u32 mask;size_t i;u16 len;int rc;rc = tcg2_get_active_pcr_banks(dev, &active);if (rc)return rc;event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {mask = tpm2_algorithm_to_mask(tcg2algos[i]);if (!(active & mask))continue;switch (tcg2algos[i]) {case TPM2_ALG_SHA1:case TPM2_ALG_SHA256:case TPM2_ALG_SHA384:case TPM2_ALG_SHA512:count++;break;default:continue;}}event_size += 1 +(sizeof(struct tcg_efi_spec_id_event_algorithm_size) * count);log_size = offsetof(struct tcg_pcr_event, event) + event_size;if (log_size > elog->log_size) {printf("%s: log too large: %u > %u\n", __func__, log_size,elog->log_size);return -ENOBUFS;}log = (struct tcg_pcr_event *)elog->log;put_unaligned_le32(0, &log->pcr_index);put_unaligned_le32(EV_NO_ACTION, &log->event_type);memset(&log->digest, 0, sizeof(log->digest));put_unaligned_le32(event_size, &log->event_size);ev = (struct tcg_efi_spec_id_event *)log->event;strlcpy((char *)ev->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,Same with all of this.
sizeof(ev->signature));put_unaligned_le32(0, &ev->platform_class);ev->spec_version_minor = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;ev->spec_version_major = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;ev->spec_errata = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;I'm not quite sure what is going on here...is this log in a format defined by the EFI spec? What if we are not using EFI? How would a different format be used?
Put another way, people using a TPM should not pull in EFI things just to do that.
Agreed, however the EFI spec is not involved. These specifications and structures are general to any boot measurement. I would guess EFI was the first to do this and therefore defined some structures that the TCG re-used when writing the specs.
I'm just not quite sure of the best approach here...
Regards, Simon