Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38779 )
Change subject: vboot: push clear recovery mode switch until BS_WRITE_TABLES ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38779/1/src/security/vboot/bootmode... File src/security/vboot/bootmode.c:
https://review.coreboot.org/c/coreboot/+/38779/1/src/security/vboot/bootmode... PS1, Line 44: vboot_log_and_clear_recovery_mode_switch, NULL);
I think this should be combined with google_chromeec_elog_add_recovery_event(). […]
Ah... I did not realize that this was being stowed away in CBMEM as well.
Although now I'm a bit worried I'm not fully understanding why we were clearing the recovery mode switch at ROMSTAGE_CBMEM_INIT_HOOK, rather than just moving it later like we're doing in this CL now? Furquan, do you have any more background on this? Seems like the CLs link to the same bug: https://b/35582650
If we move the logic to google_chromeec_elog_add_recovery_event, then this "clear" step will move from BS_DEV_INIT later on to BS_WRITE_TABLES.
https://review.coreboot.org/c/coreboot/+/38779/1/src/security/vboot/vboot_lo... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/38779/1/src/security/vboot/vboot_lo... PS1, Line 281: x86 systems ignore the saved dram settings : * in the recovery path in order to start from a clean slate. Therefore : * clear the state here since this function is called when memory : * is known to be up. Furquan / Aaron, I'm not sure I fully understand this statement. Please let me know if I'm doing something horribly wrong.