Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31837 )
Change subject: drivers/tpm: update TPM initialization logic for Intel FSP2.0 ......................................................................
Patch Set 3:
(2 comments)
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
Thanks for letting me know about this -- I had no idea that RECOVERY_MRC_CACHE triggered TPM read/wr […]
I think I would prefer what Furquan said -- just don't support RECOVERY_MRC_CACHE with STARTS_IN_ROMSTAGE. It would add a bunch of complexity and we have no real use case for it. For the purposes of testing STARTS_IN_ROMSTAGE, you can just deactivate RECOVERY_MRC_CACHE on those boards... they should still boot just fine, they'll just take a little longer to do it in recovery mode.
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))
there are 0 platforms that have VBOOT_STARTS_IN_ROMSTAGE and FSP2. […]
True but I'm not sure that needs to be cemented as a hard requirement? I think one of Joel's goals here is to be able to change a newer board to STARTS_IN_ROMSTAGE for development, because we're running out of easily accessible old boards that actually use it and so it never gets tested anymore.
I guess the other option would be to kill the STARTS_IN_ROMSTAGE option entirely, but I'm not sure if we're willing to do that yet.