Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33534 )
Change subject: vboot: relocate code to log and clear recovery mode switch ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/33534/3/src/security/vboot/vboot_logic.c File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/#/c/33534/3/src/security/vboot/vboot_logic.c@302 PS3, Line 302: ROMSTAGE_CBMEM_INIT_HOOK(vboot_log_and_clear_recovery_mode_switch)
Only x86 devices have elog available in romstage. This needs to be done in ramstage. […]
Hmm... I just re-read how this actually works and that it has always been in romstage. But the whole thing seems like a pretty horrible design with having to plumb it through CBMEM first. Why are we doing all of this in the first place? Is it really just for get_recovery_mode_switch() like that comment says? Nobody other than verstage_main() should ever be calling get_recovery_mode_switch() in the first place! Code running in romstage should just check working data and nothing else.
I guess we can submit this as a clean move without changing behavior for now, but it would be great if you could clean the whole thing up a bit in a follow-up patch.