Attention is currently required from: Andrey Pronin, Julius Werner, Aaron Durbin. Aseda Aboagye has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP ......................................................................
Patch Set 4:
(4 comments)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/52919/comment/33f3e44a_62239458 PS1, Line 111:
I'd put the attribute definition here so we have them all in one place to compare.
Ack
https://review.coreboot.org/c/coreboot/+/52919/comment/fe3c6e98_52aea558 PS1, Line 237: .TPMA_NV_OWNERWRITE = 1,
Should we also add PPWRITE just in case the RW firmware ever needs to make modifications there? (Not […]
I recall reading a comment in a doc that we should consider FWMP as frozen. If we did want to modify the struct, we should create a new space? Andrey may know more here.
I don't see any drawback to adding the attribute though.
https://review.coreboot.org/c/coreboot/+/52919/comment/c8cc88f5_77d35957 PS1, Line 243: rv = tlcl_define_space(FWMP_NV_INDEX, VB2_SECDATA_FWMP_MAX_SIZE,
Why not reuse set_space()? We should initialize the FWMP with valid data too. […]
Per cylai@, cryptohome doesn't really care if the space has been written to or not, so I had elected to not initialize the space.
If we think we still should, I can definitely do so. I did notice that the other spaces are initialized immediately after definition.
https://review.coreboot.org/c/coreboot/+/52919/comment/cce6655e_cf1efe02 PS1, Line 244: pcr0_allowed_policy,
I don't think this has any meaning when POLICY_DELETE isn't set, unless Andrey says otherwise. […]
Ack