Hi Eddie,
Thanks for the cleanup! Unfortunately this doesn't compile with EFI selected, but in general it looks pretty good.
On Wed, Jan 25, 2023 at 11:18:06AM -0600, Eddie James 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 | 254 ++++++++++ lib/efi_loader/efi_tcg2.c | 975 +++----------------------------------- lib/tpm-v2.c | 799 +++++++++++++++++++++++++++++++ 4 files changed, 1129 insertions(+), 943 deletions(-)
HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, };
[...]
}
+}
+int tcg2_measure_data(struct udevice *dev, struct tcg2_event_log *elog,
u32 pcr_index, u32 size, const u8 *data, u32 event_type,u32 event_size, const u8 *event)+{
- struct tpml_digest_values digest_list;
- int rc;
- rc = tcg2_create_digest(dev, data, size, &digest_list);
- if (rc)
return rc;- rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
- if (rc)
return rc;- return tcg2_log_append_check(elog, pcr_index, event_type, &digest_list,
event_size, event);+}
+int tcg2_measure_event(struct udevice *dev, struct tcg2_event_log *elog,
u32 pcr_index, u32 event_type, u32 size,const u8 *event)+{
- struct tpml_digest_values digest_list;
- int rc;
- rc = tcg2_create_digest(dev, event, size, &digest_list);
- if (rc)
return rc;- rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
- if (rc)
return rc;- return tcg2_log_append_check(elog, pcr_index, event_type, &digest_list,
size, event);+}
The only difference between these 2 is the measured data. Can't we make one function? If data = NULL we could just measure the event
+int tcg2_init_log(struct udevice *dev, struct tcg2_event_log *elog) +{
- int rc;
- rc = tcg2_platform_get_log(dev, (void **)&elog->log, &elog->log_size);
- if (rc)
return rc;- elog->log_position = 0;
- elog->found = false;
- rc = tcg2_log_parse(dev, elog);
- if (rc)
return rc;- if (!elog->found) {
rc = tcg2_log_init(dev, elog);if (rc)return rc;
You can get rid of this if and just return rc on the function
- }
- return 0;
+}
+int tcg2_measurement_init(struct udevice **dev, struct tcg2_event_log *elog) +{
- int rc;
- rc = tcg2_platform_get_tpm2(dev);
- if (rc)
return rc;- rc = tpm_init(*dev);
- if (rc)
return rc;- rc = tpm2_startup(*dev, TPM2_SU_CLEAR);
- if (rc) {
tcg2_platform_startup_error(*dev, rc);return rc;- }
- rc = tpm2_self_test(*dev, TPMI_YES);
- if (rc)
printf("%s: self test error, continuing.\n", __func__);
This is the correct init sequence of the TPM. I was trying to solve something similar a few days ago [0]. I haven't merged that patch yet, but I will send a new version that also calls tpm_init() and doesn't exit on -EBUSY. Can you use that patch instead of open coding the init sequence? Also we should be bailing out on selftest errors?
The problem here is that with U-Boot's lazy binding we might need to init the TPM in other subsystems and at the very least we can have a function doing that for us reliably.
- rc = tcg2_init_log(*dev, elog);
- if (rc) {
tcg2_measurement_term(*dev, elog, true);return rc;- }
- rc = tcg2_measure_event(*dev, elog, 0, EV_S_CRTM_VERSION,
strlen(version_string) + 1,(u8 *)version_string);- if (rc) {
tcg2_measurement_term(*dev, elog, true);return rc;- }
- return 0;
+}
+void tcg2_measurement_term(struct udevice *dev, struct tcg2_event_log *elog,
bool error)+{
- u32 event = error ? 0x1 : 0xffffffff;
- int i;
- for (i = 0; i < 8; ++i)
tcg2_measure_event(dev, elog, i, EV_SEPARATOR, sizeof(event),(const u8 *)&event);
Is the separator event at the end of the eventlog described in the spec?
- if (elog->log)
unmap_physmem(elog->log, MAP_NOCACHE);+}
+__weak int tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size) +{
- const __be32 *addr_prop;
- const __be32 *size_prop;
- int asize;
- int ssize;
- *addr = NULL;
- *size = 0;
- addr_prop = dev_read_prop(dev, "linux,sml-base", &asize);
- if (!addr_prop)
addr_prop = dev_read_prop(dev, "tpm_event_log_addr", &asize);- size_prop = dev_read_prop(dev, "linux,sml-size", &ssize);
- if (!size_prop)
size_prop = dev_read_prop(dev, "tpm_event_log_size", &ssize);- if (addr_prop && size_prop) {
u64 a = of_read_number(addr_prop, asize / sizeof(__be32));u64 s = of_read_number(size_prop, ssize / sizeof(__be32));*addr = map_physmem(a, s, MAP_NOCACHE);*size = (u32)s;- } else {
struct ofnode_phandle_args args;phys_addr_t a;phys_size_t s;if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0,0, &args))return -ENODEV;a = ofnode_get_addr_size(args.node, "reg", &s);if (a == FDT_ADDR_T_NONE)return -ENOMEM;*addr = map_physmem(a, s, MAP_NOCACHE);*size = (u32)s;- }
- return 0;
+}
+__weak int tcg2_platform_get_tpm2(struct udevice **dev) +{
- for_each_tpm_device(*dev) {
if (tpm_get_version(*dev) == TPM_V2)return 0;- }
- return -ENODEV;
+}
+__weak void tcg2_platform_startup_error(struct udevice *dev, int rc) {}
u32 tpm2_startup(struct udevice *dev, enum tpm2_startup_types mode) { const u8 command_v2[12] = { @@ -342,6 +1016,131 @@ u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property, return 0; }
+static int tpm2_get_num_pcr(struct udevice *dev, u32 *num_pcr) +{
- u8 response[(sizeof(struct tpms_capability_data) -
offsetof(struct tpms_capability_data, data))];- u32 properties_offset =
offsetof(struct tpml_tagged_tpm_property, tpm_property) +offsetof(struct tpms_tagged_property, value);- u32 ret;
- memset(response, 0, sizeof(response));
- ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
TPM2_PT_PCR_COUNT, response, 1);- if (ret)
return ret;- *num_pcr = get_unaligned_be32(response + properties_offset);
- if (*num_pcr > TPM2_MAX_PCRS) {
printf("%s: too many pcrs: %u\n", __func__, *num_pcr);return -E2BIG;- }
- return 0;
+}
[...]
[0] https://lore.kernel.org/u-boot/20230125144851.532154-1-ilias.apalodimas@lina...
Thanks /Ilias