Attention is currently required from: Michał Żygowski, Maciej Pijanowski, Christian Walter, Julius Werner, Krystian Hebel.
Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68747 )
Change subject: security/tpm: add TPM log format as per 1.2 spec ......................................................................
Patch Set 8:
(6 comments)
File src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h:
https://review.coreboot.org/c/coreboot/+/68747/comment/f685f234_d2c41e7c PS7, Line 143: { CBMEM_ID_TPM_LOG, "TPM STD LOG" }, \
Why do we create a new cbmem ID? We already have IDs TPM logs: […]
Because existing code makes assumptions about format of data based on cbmem IDs. Reusing the ID will break the code, it's not always possible to determine the log format based on data and existing code isn't even trying to. E.g. `CBMEM_ID_TPM2_TCG_LOG` is used by `src/acpi/acpi.c` and is not compatible with a regular TPM log.
File src/security/tpm/Kconfig:
https://review.coreboot.org/c/coreboot/+/68747/comment/be947940_5b53f672 PS5, Line 108: bool "TPM 1.2 format"
Exactly, TPM2 should have different format. […]
Done
File src/security/tpm/Kconfig:
https://review.coreboot.org/c/coreboot/+/68747/comment/9292b49c_2c554eaf PS7, Line 100: default TPM_LOG_TPM1 if TPM1 Well, https://ticket.coreboot.org/issues/422 says:
New entries should default to one of TCG formats, depending on selected TPM family. Choice of other format shouldn't be restricted.
I thought it means log version should be chosen by the user.
File src/security/tpm/tpm1_log_serialized.h:
https://review.coreboot.org/c/coreboot/+/68747/comment/dac58786_b324878e PS7, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
This file could be commonlib with BSD license isn't it? […]
`commonlib/tpm_log_serialized.h` is licensed as `GPL-2.0-only`.
I did try having a single header, it's a bit messy, but the much bigger problem is that it can't work for TPM2 log because it uses `CONFIG()` macro to which `cbmem` has no access.
https://review.coreboot.org/c/coreboot/+/68747/comment/ba692641_9be9d19f PS7, Line 13: #define TPM_1_LOG_VI_MAGIC 0x31544243 /* "CBT1" in LE */
For ACPI table we use "COREv4" as OEM ID and "CORE" as ASL compiler. […]
And use the same ID for TPM1 and TPM2 logs?
File src/security/tpm/tspi/log-tpm1.c:
https://review.coreboot.org/c/coreboot/+/68747/comment/49f6b205_70e6aada PS7, Line 16: void *tpm_log_cbmem_init(void)
If it is source file for TPM1. […]
These functions are called from shared code which doesn't know what log format is used. I can change names only by adding one more function to pick the implementation based on `CONFIG()` options, which can also be done later.