Attention is currently required from: Andrey Pronin, Joel Kitching, Aseda Aboagye, Aaron Durbin. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: vboot/secdata_tpm: Create FWMP space in coreboot ......................................................................
Patch Set 8:
(2 comments)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/52919/comment/8102ad49_bfcc80d3 PS1, Line 420: /* : * Set initial values of secdata_firmware space. : * kernel space is created in _factory_initialize_tpm(). : */ : vb2api_secdata_firmware_create(ctx);
to have some defaults if tlcl_self_test_full() or _factory_initialize_tpm() fail?
Actually, it's better not to. Not having valid data in those spaces is the way we tell vboot that something went wrong reading them. (I guess this isn't really perfect but it should be good enough in practice... with this new "create immediately before writing" scheme, we would probably notice connection problems for one of the other spaces and then not even get to the part for creating the firmware space, and vboot would notice that one quickly.)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/52919/comment/7af247ef_418d49bf PS7, Line 13: <security/tpm/tss/tcg-2.0/tss_structures.h>
curious: how did it work before this? we were already pulling TPMA_NV_* from somewhere... […]
Actually, tss.h already chain-includes these. Not sure if it's intended to also include these directly, but it doesn't hurt. (When to rely on chain includes and when not to is not perfectly well-defined for all cases and also a frequent cause of contention between coreboot developers.)
-
Julius Werner (Code Review)