Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31837 )
Change subject: drivers/intel/fsp2_0: fix TPM setup and MRC cache hash logic ......................................................................
Patch Set 4:
(2 comments)
We actually have two options here:
* When VBOOT_STARTS_IN_ROMSTAGE is enabled, disallow USE_RECOVERY_MRC_CACHE. AFAIK this would regenerate the recovery MRC training data on each recovery mode boot. Pro: slow. Con: safe.
* When VBOOT_STARTS_IN_ROMSTAGE is enabled, disallow FSP2_0_USES_TPM_MRC_HASH. This would use the recovery MRC cache as normal, without the save-hash-in-TPM functionality. Pro: fast. Con: less safe.
https://review.coreboot.org/#/c/31837/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31837/2//COMMIT_MSG@14 PS2, Line 14: TPM will be initialized whenever : verstage is executed, depending on how the device is configured
Correct. Even without any change, currently, it would do the memory retraining in recovery mode. […]
OK, let's just disable the combination of Kconfig options as suggested.
https://review.coreboot.org/#/c/31837/3/src/security/vboot/secdata_tpm.c File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/#/c/31837/3/src/security/vboot/secdata_tpm.c@453 PS3, Line 453: CONFIG(VBOOT_STARTS_IN_ROMSTAGE))
I wouldn't recommend killing the support which is used by multiple platforms btw
I think we still need to consider killing VBOOT_STARTS_IN_ROMSTAGE. It causes many edge cases that are a hassle to handle, given that this option is not used for any of our modern boards.