Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37471 )
Change subject: vboot: Clear secdata change flags after factory init ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37471/3/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/37471/3/src/security/vboot/secdata_... PS3, Line 96: int attempts = 3; We got rid of retries in depthcharge because we decided that our communication with Cr50 is reliable. Is that not the case here? (Or are we not making the assumption of using Cr50 here?)
https://review.coreboot.org/c/coreboot/+/37471/3/src/security/vboot/secdata_... PS3, Line 418: /* : * This seems the first time we've run. Initialize the TPM. : */ Or perhaps also making this shorter than three lines?
https://review.coreboot.org/c/coreboot/+/37471/3/src/security/vboot/secdata_... PS3, Line 425: //RETURN_ON_FAILURE(factory_initialize_tpm(ctx)); If we're fixing any other random stuff in this CL, should we think about removing this line?
https://review.coreboot.org/c/coreboot/+/37471/3/src/security/vboot/vboot_lo... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/37471/3/src/security/vboot/vboot_lo... PS3, Line 268: VB2_CONTEXT_SECDATA_FIRMWARE_CHANGED Good catch, thanks.