Hi Tim,
[...]
Would you agree with removing the requirement for the event log?
No, the log is required, otherwise it's fairly meaningless work. You need the log in your OS to verify the contents of the TPM.
It's the other way around. You trust the TPM and replay the event log in memory to verify it's correct. That being said, I do agree the event log is pretty useful when trying to understand how and what the platform measured. In any case, I'd rather fix any issues rather than sidestep them.
Why do you need a log to verify the contents of the TPM? If the PCR's are not correct you can't get your secrets from the TPM and if they are you can regardless of a log. Where is this log requirement in the TCG specification?
Yes, as I said you *validate* the eventlog by looking at the TPM PCRs, not the other way around. The problem with the TCG spec is
- EFI_TCG2_PROTOCOL.GetEventLog can only returns either EFI_SUCCESS or
EFI_INVALID_PARAMETER. There's no EFI_UNSUPPORTED
- EFI_TCG2_PROTOCOL.HashLogExtendEvent has a flag
(EFI_TCG2_EXTEND_ONLY) which allows you to only extend PCRs and not log them
But I can't find anywhere in the spec a statement that says "the eventlog is optional"
Please see Linux commit 805fa88e0780b ("tpm: Don't make log failures fatal") which has a commit log of:
If a TPM is in disabled state, it's reasonable for it to have an empty log.
Yes, an empty log. Not missing a log overall. Which makes sense if the TPM is disabled.
Bailing out of probe in this case means that the PPI interface isn't available, so there's no way to then enable the TPM from the OS. In general it seems reasonable to ignore log errors - they shouldn't interfere with any other TPM functionality.
That last sentence makes sense to me; Sure the log may be 'useful' to some but I feel like it's not a requirement and it certainly is not a requirement for Linux.
The return value of ofnode_get_addr_size() depends on a couple Kconfig options. Any chance those differ from the ones Eddie is using?
I have the same dt changes. I think this has something to do with the whole are we using of or fdt, and is of live:
$ git diff diff --git a/arch/arm/dts/imx8mm-venice-gw72xx.dtsi b/arch/arm/dts/imx8mm-venice-gw72xx.dtsi index 97ed34a3c586..5752a38c7b4c 100644 --- a/arch/arm/dts/imx8mm-venice-gw72xx.dtsi +++ b/arch/arm/dts/imx8mm-venice-gw72xx.dtsi @@ -14,6 +14,17 @@ usb1 = &usbotg2; };
reserved-memory {#address-cells = <1>;#size-cells = <1>;ranges;event_log: tcg_event_log@40000000 {no-map;reg = <0x40000000 0x2000>;};};led-controller { compatible = "gpio-leds"; pinctrl-names = "default";@@ -91,6 +102,7 @@ tpm@1 { compatible = "tcg,tpm_tis-spi"; reg = <0x1>;
memory-region = <&event_log>; spi-max-frequency = <36000000>; };}; diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 994f8089ba34..0bc08cebed2f 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -672,11 +673,14 @@ __weak int tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size) phys_addr_t a; fdt_size_t s;
+printf("%s %s ofnode=%px valid=%d is_np=%d of_live_active=%d\n", __func__, dev->name, dev_ofnode(dev).np, ofnode_valid(dev_ofnode(de v)), ofnode_is_np(dev_ofnode(dev)), of_live_active()); if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0, 0, &args)) return -ENODEV; +printf("%s %s args:%d\n", __func__, dev->name, args.args_count);
a = ofnode_get_addr_size(args.node, "reg", &s);+printf("%s %s a:%px\n", __func__, dev->name, (void*)a); if (a == FDT_ADDR_T_NONE) return -ENOMEM;
This shows the following: tcg2_platform_get_log tpm@1 ofnode=00000000000054d0 valid=1 is_np=0 of_live_active=0 tcg2_platform_get_log tpm@1 args:0 tcg2_platform_get_log tpm@1 a:ffffffffffffffff ^^^ can't get address of reg
so we have a valid of_node in the dev structure but its not a np?
dev_read_phandle_with_args() is returning success but was not able to parse any args, so we should not use 'args.node' as the arg of ofnode_get_addr_size(args.node, "reg", &s);
I think the mix of dev_read_phandle_with_args and ofnode_get_addr_size is wrong. U-Boot is full of constructs like this that are extremely confusing... missing fdt with of and the concept of if its live or not. I'm hoping Simon can shed some light on this and maybe give or point to a primer on all the of/dt/live stuff?
Thanks that helps. I'll try to reproduce it with sandbox and/or QEMU with OF_LIVE and see if I get something.
Ilias,
I think the following is needed here: diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 994f8089ba34..6b9d587e491c 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -675,8 +675,7 @@ __weak int tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size) if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0, 0, &args)) return -ENODEV;
a = ofnode_get_addr_size(args.node, "reg", &s);
a = ofnode_get_addr_size_index(args.node, 0, &s); if (a == FDT_ADDR_T_NONE) return -ENOMEM;Best Regards,
Looking at the sandbox config file OF_LIVE is enabled. I haven't looked into the differences on ofnode_get_addr_size(), ofnode_get_addr_size_index(), why is the index one needed?
btw, why are you trying to create the eventlog area like this? IIRC linux doesn't support that. Looking at the latest kernel 'tcg_event_log' is only defined in aspeed bmc's on TPMs so I guess they have a special usecase for that, hence the code Eddie carried in U-Boot. For your case, isn't it preferable to add the log area with linux,sml-base, linux,sml-size? Linux will try to read the eventlog in that case.
Thanks /Ilias
Tim