Julius Werner 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:
(3 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 […]
Other upstream coreboot users are still using different TPMs. Those are probably reliable too, idk... but not really something I want to explore that much in this CL, I just wanted to remove this inconsistency (because everything else in this file uses this helper instead of safe_write() directly).
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?
Ack
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?
Sure.