Change in coreboot[master]: [WIP] drivers/mrc_cache: Drop get_write_protect_state() stubs

[Julius Werner (Code Review)]

Attention is currently required from: Jason Glenesk, Marshall Dawson, Tim Wawrzynczak, Kyösti Mälkki, Andrey Petrov, Alexander Couzens, Patrick Rudolph, Yu-Ping Wu, Felix Held.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58968 )

Change subject: [WIP] drivers/mrc_cache: Drop get_write_protect_state() stubs
......................................................................

Patch Set 3:

(2 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/58968/comment/eb10a215_99027297 
PS3, Line 11: from soc/.
I think MRC_SETTINGS_PROTECT is supposed to be a capability Kconfig (i.e. "this platform supports MRC settings protection"), not a choice Kconfig ("I want to protect MRC settings"). Not sure why it is menuconfig visible, that might be a mistake. You can't just remove it from the SoC and turn it on by default for CONFIG_CHROMEOS, because some platforms that use the MRC cache driver (e.g. recent Arm devices) don't support flash-controller-based protection.

I think we just don't have a user choice Kconfig for this because nobody ever needed one. Why do you think it's necessary? There should never really be a reason for the OS to touch the MRC cache anyway so I think the idea was just that nobody would ever have a reason not to want it protected.

File src/security/vboot/bootmode.c:

https://review.coreboot.org/c/coreboot/+/58968/comment/6df03d27_08547714 
PS3, Line 65: int __weak get_write_protect_state(void)
Moving this out of VBOOT_NO_BOARD_SUPPORT makes me uneasy because that makes it easier to accidentally forget to override this function and not notice. There is no other way that would be visible other than the MRC cache being unprotected.

Honestly, I'm not sure there's a reason to explicitly check the hardware WP# line for this anyway. If the flash is unprotected, the status register SRP0 would also be disabled anyway. We used to do that because we had that GPIO available for vboot anyway (in fact I believe it started like that and the extra SRP0 check was added later), but now vboot doesn't need this pin anymore and having to define it for every board just for the MRC cache protection behavior seems a bit cumbersome. Maybe we can just take that part out and decide this based on SRP0 alone?