[coreboot-gerrit] [M] Change in coreboot[master]: security/vboot: Add store/validate methods for VBIOS FMAP cache

[Martin L Roth (Code Review)]

Attention is currently required from: Andrey Pronin, Raul Rangel, Matt DeVillier, Julius Werner, Martin Roth, Yu-Ping Wu, Karthik Ramasubramanian.

Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72401 )

Change subject: security/vboot: Add store/validate methods for VBIOS FMAP cache
......................................................................

Patch Set 4:

(2 comments)

File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/72401/comment/71a3b59f_9766c7b7 
PS4, Line 520: setup_space
> > this "create on 1st write" is useful to deploy the feature on the devices already in the field, wh […]

Karthik, are you saying we should have a Kconfig option for switching between setup_space and disabling it?

The advantage of leaving this in is that it will work on existing boards that weren't initialized in the factory. What's the disadvantage of leaving it?

https://review.coreboot.org/c/coreboot/+/72401/comment/971d3d23_ac1ec7a7 
PS4, Line 520: return
Why would we choose to always return instead of continuing to write the buffer out after setting up the TPM (so long as setup_space works)?

Also, because the code calling this will get the return code from setup_space instead of safe_write, if setup_space works, i think the code will believes that it was correctly written out, even though it wasn't.

So whatever we do, I think returning this way is wrong.  It's not a huge deal - just printing the warning message vs not, but still wrong, (I think).