Attention is currently required from: Christian Walter, Filip Lewiński, Michał Żygowski, Yu-Ping Wu.
Julius Werner has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/82695?usp=email )
Change subject: security: Allow vboot when INTEL_TXT enabled ......................................................................
Patch Set 9:
(3 comments)
File src/lib/bootblock.c:
https://review.coreboot.org/c/coreboot/+/82695/comment/c7135452_00bcbed2?usp... : PS5, Line 62: if (CONFIG(TPM_MEASURED_BOOT_INIT_BOOTBLOCK) && !CONFIG(VBOOT_STARTS_IN_BOOTBLOCK)) {
@jwerner@chromium. […]
Sorry, I must have missed the last message.
So it sounds like we're going with the "always have MEASURED_BOOT initialize the TPM and then just have vboot assume it is already initialized" approach now, right? Then I think most of this is fine, I'm just not sure why you are adding the extra !STARTS_IN_BOOTBLOCK check here. Even in the STARTS_IN_BOOTBLOCK case, vboot runs after this (as part of the run_romstage() below), so you should still initialize the TPM here, do the measurement stuff, and then have vboot assume it was already initialized later, right?
File src/security/tpm/Kconfig:
https://review.coreboot.org/c/coreboot/+/82695/comment/02680623_c001be05?usp... : PS9, Line 125: depends on TPM_MEASURED_BOOT This is still going to clash with the "vboot in PSP" thing the AMD CPUs are doing, so since you probably don't care about that I'd just add `&& !VBOOT_STARTS_BEFORE_BOOTBLOCK` here.
File src/security/vboot/tpm_common.c:
https://review.coreboot.org/c/coreboot/+/82695/comment/c68561d9_2e654ea5?usp... : PS9, Line 24: && !CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) I don't really understand the second part here? Even in the VBOOT_STARTS_IN_BOOTBLOCK case, the MEASURED_BOOT tpm_setup() runs before vboot, doesn't it?
-
Julius Werner (Code Review)