Attention is currently required from: Andrey Pronin, Raul Rangel, Julius Werner, Yu-Ping Wu. Karthik Ramasubramanian 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:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59476/comment/27f90f29_de569d3e PS1, Line 35: 72057594037927936
Why is the value so high?
I think because of endianness reasons - The value is 0x1000000000000000.
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/59476/comment/7b2e1808_8b8b2cc6 PS1, Line 349: SECURE_COUNTER_NAME
Is this any arbitrary name?
This name is all local to coreboot. I dont think it is passed to TPM. Hence chosen a generic name.
https://review.coreboot.org/c/coreboot/+/59476/comment/e6fd9301_750c1a0a PS1, Line 368: rv = tlcl_read(index, &value, SECURE_COUNTER_SIZE);
But that is true for all our TPM spaces. […]
Does TPM return TPM2_RC_NV_DEFINED if the NV index already exists? If so, I can treat it similar to TPM_SUCCESS and continue with defining the rest of the counters.
If any other response code is returned, abort defining the counters.
https://review.coreboot.org/c/coreboot/+/59476/comment/bbccaaf4_de8387f5 PS1, Line 369: !=
!= TPM_E_BADINDEX also makes sure that the index is created and written. […]
I will handle the error accordingly based on the discussion in line 368.
https://review.coreboot.org/c/coreboot/+/59476/comment/e4aaaa6a_3d13844a 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_fi […]
Julius/Andrey confirmed that firmware_space should be the last and hence asked to move counter space further up.