Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38057 )
Change subject: security: Move TS_{START,END}_TPMINIT out of vboot ......................................................................
security: Move TS_{START,END}_TPMINIT out of vboot
These are generic timestamps, and not vboot specific. Therefore, move them to `tpm_setup()`, so that these timestamps are added in all cases.
(vboot timestamps should be namespaced with VBOOT anyway.)
Change-Id: Ib1048f7b7a5903d186cdd750822b4bc8ea7dc665 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de --- M src/security/tpm/tspi/tspi.c M src/security/vboot/vboot_logic.c 2 files changed, 5 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/38057/1
diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index 5fcf92d..9ca9f98 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -19,6 +19,7 @@ #include <console/console.h> #include <security/tpm/tspi.h> #include <security/tpm/tss.h> +#include <timestamp.h> #if CONFIG(VBOOT) #include <vb2_api.h> #include <vb2_sha.h> @@ -127,6 +128,8 @@ { uint32_t result;
+ timestamp_add_now(TS_START_TPMINIT); + result = tlcl_lib_init(); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Can't initialize.\n"); @@ -174,6 +177,8 @@ result = tpm1_invoke_state_machine(); #endif
+ timestamp_add_now(TS_END_TPMINIT); + return tpm_setup_epilogue(result); }
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 1d17a17..c65ea3a 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -320,10 +320,8 @@ /* Read secdata from TPM. Initialize TPM if secdata not found. We don't * check the return value here because vb2api_fw_phase1 will catch * invalid secdata and tell us what to do (=reboot). */ - timestamp_add_now(TS_START_TPMINIT); if (vboot_setup_tpm(ctx) == TPM_SUCCESS) antirollback_read_space_firmware(ctx); - timestamp_add_now(TS_END_TPMINIT);
/* Enable measured boot mode */ if (CONFIG(VBOOT_MEASURED_BOOT) &&
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38057 )
Change subject: security: Move TS_{START,END}_TPMINIT out of vboot ......................................................................
Patch Set 1:
I guess I should rename the vendor timestamps, so the numbers are kept?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38057 )
Change subject: security: Move TS_{START,END}_TPMINIT out of vboot ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38057/1/src/security/vboot/vboot_lo... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/38057/1/src/security/vboot/vboot_lo... PS1, Line 324: antirollback_read_space_firmware We're losing the time this function takes. Should we rename the #defines and add new ones that are just inside tpm_setup()?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38057 )
Change subject: security: Move TS_{START,END}_TPMINIT out of vboot ......................................................................
Patch Set 1:
Patch Set 1:
I guess I should rename the vendor timestamps, so the numbers are kept?
Which vendor timestamps?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38057 )
Change subject: security: Move TS_{START,END}_TPMINIT out of vboot ......................................................................
Patch Set 1:
(1 comment)
I guess I should rename the vendor timestamps, so the numbers are kept?
We should keep the old numbers (even if these technically don't belong into the vboot block anymore now) so that the cbmem utility keeps working for old builds. Is that what you meant?
https://review.coreboot.org/c/coreboot/+/38057/1/src/security/vboot/vboot_lo... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/38057/1/src/security/vboot/vboot_lo... PS1, Line 324: antirollback_read_space_firmware
We're losing the time this function takes. […]
I'd suggest creating a new TS_VBOOT_TPM_READ_DONE and logging it here, then we can differentiate init and read.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38057 )
Change subject: security: Move TS_{START,END}_TPMINIT out of vboot ......................................................................
Patch Set 1:
Patch Set 1:
(1 comment)
I guess I should rename the vendor timestamps, so the numbers are kept?
We should keep the old numbers (even if these technically don't belong into the vboot block anymore now) so that the cbmem utility keeps working for old builds. Is that what you meant?
Yes, that it what I meant.
I’ll take a stab at that in the next days.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/38057?usp=email )
Change subject: security: Move TS_{START,END}_TPMINIT out of vboot ......................................................................
Abandoned
-
Aaron Durbin (Code Review) -
Julius Werner (Code Review)
-
Paul Menzel (Code Review)
-
Stefan Reinauer (Code Review)