Attention is currently required from: Andrey Pronin, Julius Werner, Yu-Ping Wu, Karthik Ramasubramanian. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59476 )
Change subject: src/security/vboot: Setup secure counter space in TPM NVRAM ......................................................................
Patch Set 1:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59476/comment/8db1b7b6_4fdc8860 PS1, Line 35: 72057594037927936 Why is the value so high?
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/59476/comment/305807c7_08d6869c PS1, Line 349: SECURE_COUNTER_NAME Is this any arbitrary name?
https://review.coreboot.org/c/coreboot/+/59476/comment/91dc8a2f_dd41dce1 PS1, Line 369: != Should we check for TPM_SUCCESS instead? I'm concerned this will let other errors through.
https://review.coreboot.org/c/coreboot/+/59476/comment/9c4eac4b_6267ce01 PS1, Line 386: * Of all NVRAM spaces defined by this function the firmware space : * must be defined last, because its existence is considered an : * indication that TPM factory initialization was successfully : * completed. Does this comment need updating, or do you need to move setup_secure_counter_spaces before setup_firmware_spaces?
File src/security/vboot/secure_counter.h:
https://review.coreboot.org/c/coreboot/+/59476/comment/5843ced6_60372bbd PS1, Line 7: 8 Would doing a sizeof(uint64_t) make this more clear?
https://review.coreboot.org/c/coreboot/+/59476/comment/96a43a49_9f0ebeed PS1, Line 11: 0x100f Is this a google specific or cr50 specific number? Should we pass these in via a Kconfig instead?