Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32704 )
Change subject: security: Add common boot media write protection ......................................................................
Patch Set 11:
(3 comments)
But the MRC cache is also common code? So just pick the latest it can happen from there (looks like that's OS_RESUME_CHECK)? Sounds like this is something we should try to standardize now if it hasn't been before.
that doesn't work because on Intel the spibar is locked before DEV_INIT.
I'm a bit confused, you say it doesn't work but it looks like you now changed the code to do that after all?
https://review.coreboot.org/c/coreboot/+/32704/11/src/security/lockdown/Kcon... File src/security/lockdown/Kconfig:
https://review.coreboot.org/c/coreboot/+/32704/11/src/security/lockdown/Kcon... PS11, Line 14: On Intel platforms for e.g. this will make use of the SPIBAR PRRs. nit: mention that only some platforms support this?
https://review.coreboot.org/c/coreboot/+/32704/11/src/security/lockdown/Kcon... PS11, Line 21: protection bits. nit: Same... only some chips. (I think we can assume that all chips supporting this would use flash protection bits in some arrangement, so I don't think you need to call Winbond out like that, but mention that protection bits aren't supported on all chips.)
https://review.coreboot.org/c/coreboot/+/32704/11/src/security/lockdown/Kcon... PS11, Line 33: Select this if you want to write-protect the whole firmware boot : medium using the controller. The locking will take place during the : chipset lockdown, which is either triggered by coreboot : (when INTEL_CHIPSET_LOCKDOWN is set) or has to be triggered later : (e.g. by the payload or the OS). Some of this text needs moving/updating.