Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38780 )
Change subject: vboot: remove VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT option ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38780/2/src/security/vboot/bootmode... File src/security/vboot/bootmode.c:
https://review.coreboot.org/c/coreboot/+/38780/2/src/security/vboot/bootmode... PS2, Line 46: static int vboot_get_recovery_reason_shared_data(void)
Since now this function is used only in one place, should we consider moving the 3-line code into vb […]
Done
https://review.coreboot.org/c/coreboot/+/38780/2/src/security/vboot/bootmode... PS2, Line 65: if (get_recovery_mode_switch())
BTW, this is slightly unrelated but for further improvement of this function I would consider moving […]
If we are going to rely on the assertion in vboot_get_context(), we may as well do the same for recovery_mode_enabled() and developer_mode_enabled(). That means these three functions should only ever be called *after* verstage has run.
vboot_check_recovery_request() is used for (1) deciding whether to retrain memory (in mt8183 from CB:36251), and (2) to log the recovery reason into elog.
vboot_recovery_mode_enabled() is used to decide which memory cache to use (normal or memory) as part of memory initialization. Also used in veyron_rialto/mainboard.c (CB:12135), and in disallowing Cr50 updates in recovery mode (cr50_enable_update.c). Not totally sure if those last two cases are guaranteed to be after verstage?
vboot_developer_mode_enabled() is used to decide whether or not to enable UDC (vboot_can_enable_udc), and is also saved to elog. Not totally sure about the UDC case.