tcg2_platform_get_log failing to read address and size of memory-region via ofnode_get_addr_size
Tim Harvey
tharvey at gateworks.com
Tue Mar 26 17:15:29 CET 2024
On Tue, Mar 26, 2024 at 2:24 AM Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Tim,
>
> On Tue, 26 Mar 2024 at 03:15, Tim Harvey <tharvey at gateworks.com> wrote:
> >
> > Greetings,
> >
> > I'm unable to understand why tcg2_platform_get_log is failing to read
> > a memory region.
> >
> > For example the following diffs:
>
> I am not really sure what those nodes are supposed to do in sandbox.
> Pehaps Eddie remembers.
> What exactly are you trying to achieve here? Read the eventlog from TF-A?
>
Hi Ilias,
I was trying to get measured boot (CONFIG_MEASURED_BOOT=y) working on
a tpm on my board but ran into an issue when I couldn't get the
memory-region I added for testing to be recognized with the current
code in tcg2_platform_get_log().
I wonder if an event log should be required for measured boot - it
sounds like that was something required for EFI, so I was thinking of
submitting the following:
commit b3f336c2f863168219a93cd1c7ac922396e0fad5 (HEAD -> master-venice)
Author: Tim Harvey <tharvey at gateworks.com>
Date: Tue Mar 26 08:49:07 2024 -0700
tpm: allow measured boot without an event log
Currently an event log is required for measured boot. Remove this
requirement.
Signed-off-by: Tim Harvey <tharvey at gateworks.com>
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 68eaaa639f89..994f8089ba34 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -175,17 +175,19 @@ static int tcg2_log_append_check(struct
tcg2_event_log *elog, u32 pcr_index,
u32 event_size;
u8 *log;
- event_size = size + tcg2_event_get_size(digest_list);
- if (elog->log_position + event_size > elog->log_size) {
- printf("%s: log too large: %u + %u > %u\n", __func__,
- elog->log_position, event_size, elog->log_size);
- return -ENOBUFS;
- }
+ if (elog->log_size) {
+ event_size = size + tcg2_event_get_size(digest_list);
+ if (elog->log_position + event_size > elog->log_size) {
+ printf("%s: log too large: %u + %u > %u\n", __func__,
+ elog->log_position, event_size, elog->log_size);
+ return -ENOBUFS;
+ }
- log = elog->log + elog->log_position;
- elog->log_position += event_size;
+ log = elog->log + elog->log_position;
+ elog->log_position += event_size;
- tcg2_log_append(pcr_index, event_type, digest_list, size, event, log);
+ tcg2_log_append(pcr_index, event_type, digest_list,
size, event, log);
+ }
return 0;
}
@@ -613,10 +615,8 @@ int tcg2_measurement_init(struct udevice **dev,
struct tcg2_event_log *elog,
return rc;
rc = tcg2_log_prepare_buffer(*dev, elog, ignore_existing_log);
- if (rc) {
+ 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,
Would you agree with removing the requirement for the event log?
I have another question that perhaps you may have some feedback on.
The tpm commands such as pcr_extend, pcr_read currently require a
32-byte SHA256 digest and I wish to extend that as my TPM supports
only SHA1. The tpm2_pcr_extend and tpm2_pcr_read functions were
extended to function to allow the digest type and length to be passed
in and I'm wondering what the best way to extend the tpm extend/read
commands would be to support that.
The tcg2_create_digest function creates a digest based on the
capabilities of the tpm and the tcg2_pcr_extend loops over those
calling tpm2_pcr_extend for each digtest supported (and same for
tcg2_pcr_read looping over tpm2_pcr_read) and I'm assuming TPM's can
support multiple algos so I suppose a parameter needs to be added to
the pcr_read and pcr_extend commands. Would you agree with that?
Best Regards,
Tim
> Thanks
> /Ilias
> >
> > diff --git a/arch/arm/dts/imx8mm-venice-gw73xx.dtsi
> > b/arch/arm/dts/imx8mm-venice-gw73xx.dtsi
> > index 7b2130dbdb21..57b3c227ceaf 100644
> > --- a/arch/arm/dts/imx8mm-venice-gw73xx.dtsi
> > +++ b/arch/arm/dts/imx8mm-venice-gw73xx.dtsi
> > @@ -112,6 +112,7 @@
> > compatible = "tcg,tpm_tis-spi";
> > reg = <0x1>;
> > spi-max-frequency = <36000000>;
> > + memory-region = <&event_log>;
> > };
> > };
> > diff --git a/arch/arm/dts/imx8mm-venice-gw700x.dtsi
> > b/arch/arm/dts/imx8mm-venice-gw700x.dtsi
> > index c305e325d007..697fd1148785 100644
> > --- a/arch/arm/dts/imx8mm-venice-gw700x.dtsi
> > +++ b/arch/arm/dts/imx8mm-venice-gw700x.dtsi
> > @@ -13,6 +13,17 @@
> > reg = <0x0 0x40000000 0 0x80000000>;
> > };
> >
> > + reserved-memory {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + event_log: tcg_event_log {
> > + no-map;
> > + reg = <0 0x40000000 0x2000>;
> > + };
> > + };
> > +
> > gpio-keys {
> > compatible = "gpio-keys";
> >
> > And at runtime:
> > u-boot=> fdt addr $fdtcontroladdr
> > u-boot=> fdt list /soc at 0/bus at 30800000/spba-bus at 30800000/spi at 30830000/tpm at 1/
> > tpm at 1 {
> > compatible = "tcg,tpm_tis-spi";
> > reg = <0x00000001>;
> > spi-max-frequency = <0x02255100>;
> > memory-region = <0x00000025>;
> > };
> > u-boot=> fdt list /reserved-memory/
> > reserved-memory {
> > #address-cells = <0x00000002>;
> > #size-cells = <0x00000002>;
> > ranges;
> > tcg_event_log {
> > };
> > };
> > u-boot=> fdt list /reserved-memory/tcg_event_log
> > tcg_event_log {
> > no-map;
> > reg = <0x00000000 0x40000000 0x00002000>;
> > phandle = <0x00000025>;
> > };
> >
> > So why does the following code in tcg2_platform_get_log() return -ENOMEM?
> >
> > 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;
> >
> > debugging shows that dev_read_phandle_with_args returns non-zero but
> > args.args_count is 0.
> >
> > I feel like the construct of using dev_read_phandle_with_args followed
> > by the ofnode_get_addr_size is just wrong but I don't understand why
> > nor do I understand how my dt changes differ from what is in
> > arch/sandbox/dts/test.dts (other than its using address-size=1 which
> > doesn't appear to be the issue in my testing). The abstraction of the
> > ofnode and fdt stuff always trip me up... very confusing.
> >
> > Can anyone explain the issue here?
> >
> > Best Regards,
> >
> > Tim
More information about the U-Boot
mailing list