Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46509 )
Change subject: mrc_cache: Add config MRC_SAVE_HASH_IN_TPM ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46509/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46509/2//COMMIT_MSG@13 PS2, Line 13:
Done
Well, technically there are a couple (e.g. Lenovo t530) which do. But I don't think anyone is using vboot with "real" recovery mode on there so I doubt they care.
https://review.coreboot.org/c/coreboot/+/46509/2/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/46509/2/src/drivers/intel/fsp2_0/Kc... PS2, Line 6: select MRC_SAVE_HASH_IN_TPM if HAS_RECOVERY_MRC_CACHE && TPM2 Does this need to have a select here? (The previous version also just used default y, not select, so it was overrideable.)
I think you should just make the new option default y too and then you shouldn't need this line here.
https://review.coreboot.org/c/coreboot/+/46509/2/src/drivers/mrc_cache/Kconf... File src/drivers/mrc_cache/Kconfig:
https://review.coreboot.org/c/coreboot/+/46509/2/src/drivers/mrc_cache/Kconf... PS2, Line 54: VBOOT This should be VBOOT_STARTS_IN_BOOTBLOCK because the mrc_cache.c code is enforcing that.
https://review.coreboot.org/c/coreboot/+/46509/2/src/drivers/mrc_cache/Kconf... PS2, Line 55: n Why not just make this default y? I think we'll probably want this on all future devices.