vboot: push clear recovery mode switch until BS_WRITE_TABLES

Serves two purposes:

(1) On some platforms, FSP initialization may cause a reboot.
Push clearing the recovery mode switch until after FSP code runs,
so that a manual recovery request (three-finger salute) will
function correctly under this condition.

(2) The recovery mode switch value is needed at BS_WRITE_TABLES
for adding an event to elog.  (Previously this was done by
stashing the value in CBMEM_ID_EC_HOSTEVENT.)

BUG=b:124141368, b:35576380
TEST=make clean && make test-abuild
BRANCH=none

Change-Id: I30c02787c620b937e5a50a5ed94ac906e3112dad
Signed-off-by: Joel Kitching <kitching@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38779
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>

2 files changed, 16 insertions, 28 deletions

diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c
index 83baa815c7..2a911cbf10 100644
--- a/src/security/vboot/bootmode.c
+++ b/src/security/vboot/bootmode.c
@@ -101,14 +101,27 @@ int vboot_recovery_mode_enabled(void)
 
 int __weak clear_recovery_mode_switch(void)
 {
-	// Weak implementation. Nothing to do.
 	return 0;
 }
 
-void __weak log_recovery_mode_switch(void)
+static void do_clear_recovery_mode_switch(void *unused)
 {
-	// Weak implementation. Nothing to do.
+	if (vboot_get_context()->flags & VB2_CONTEXT_FORCE_RECOVERY_MODE)
+		clear_recovery_mode_switch();
 }
+/*
+ * The recovery mode switch (typically backed by EC) is not cleared until
+ * BS_WRITE_TABLES for two reasons:
+ *
+ * (1) On some platforms, FSP initialization may cause a reboot.  Push clearing
+ * the recovery mode switch until after FSP code runs, so that a manual recovery
+ * request (three-finger salute) will function correctly under this condition.
+ *
+ * (2) To give the implementation of clear_recovery_mode_switch a chance to
+ * add an event to elog.  See the function in chromeec/switches.c.
+ */
+BOOT_STATE_INIT_ENTRY(BS_WRITE_TABLES, BS_ON_ENTRY,
+		      do_clear_recovery_mode_switch, NULL);
 
 int __weak get_recovery_mode_retrain_switch(void)
 {
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index 182128c547..18c96d77ff 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -248,26 +248,6 @@ static uint32_t extend_pcrs(struct vb2_context *ctx)
 		   vboot_extend_pcr(ctx, 1, HWID_DIGEST_PCR);
 }
 
-static void vboot_log_and_clear_recovery_mode_switch(int unused)
-{
-	/* Log the recovery mode switches if required, before clearing them. */
-	log_recovery_mode_switch();
-
-	/*
-	 * The recovery mode switch is cleared (typically backed by EC) here
-	 * to allow multiple queries to get_recovery_mode_switch() and have
-	 * them return consistent results during the verified boot path as well
-	 * as dram initialization. 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.
-	 */
-	clear_recovery_mode_switch();
-}
-#if !CONFIG(VBOOT_STARTS_IN_ROMSTAGE)
-ROMSTAGE_CBMEM_INIT_HOOK(vboot_log_and_clear_recovery_mode_switch)
-#endif
-
 /**
  * Verify and select the firmware in the RW image
  *
@@ -428,11 +408,6 @@ void verstage_main(void)
 	       vboot_is_firmware_slot_a(ctx) ? 'A' : 'B');
 
  verstage_main_exit:
-	/* If CBMEM is not up yet, let the ROMSTAGE_CBMEM_INIT_HOOK take care
-	   of running this function. */
-	if (ENV_ROMSTAGE && CONFIG(VBOOT_STARTS_IN_ROMSTAGE))
-		vboot_log_and_clear_recovery_mode_switch(0);
-
 	/* Save recovery reason in case of unexpected reboots on x86. */
 	vboot_save_recovery_reason_vbnv();