Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )
Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46511/7/src/security/vboot/mrc_cach... File src/security/vboot/mrc_cache_hash_tpm.c:
https://review.coreboot.org/c/coreboot/+/46511/7/src/security/vboot/mrc_cach... PS7, Line 26: uint32_t hash_idx = vboot_recovery_mode_enabled() ? : MRC_REC_HASH_NV_INDEX : MRC_RW_HASH_NV_INDEX; This should happen in mrc_cache.c.
https://review.coreboot.org/c/coreboot/+/46511/8/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/46511/8/src/security/vboot/secdata_... PS8, Line 194: RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data));
First of all, this still says REC_HASH. […]
Agreed. I think it is fine to not create this space until really required which would be in ramstage/romstage in the same boot. My question here https://review.coreboot.org/c/coreboot/+/46432/4/src/security/vboot/secdata_... was just to ensure my understanding matched the intent. We can potentially add a comment in factory_initialize_tpm to indicate why the creation is being skipped here if that is helpful.