factory_mode: refactor factory_enable_failed

Refactor factory_enable_failed, so cr50 always resets if a reset is
requested. This change also renames factory_enable_failed to be more
specific. It renames ccd_hook_active to wait_for_factory_ccd_change so
it's obvious what the variable is doing. It's waiting for the ccd_config
change after we enable factory mode.

Enabling factory mode can fail in a lot of ways, but by the time we
called factory_enable_failed, the failure is specifically about saving
the config. This change renames the function, so the failure is a bit
more specific.

If a reset is required, always reset the system even if saving the
factory config failed. ccd_reset_factory_failed is triggered if the ccd
changed hook isn't triggered quickly enough or if cr50 fails to save the
ccd config. Cr50 has already wiped the TPM and has most likely saved
some if not all of the factory mode state. Cr50 should still reset even
if the config isn't saved to be safe.

enable_ccd_factory_mode isn't used in the process to enable factory mode
during init, so this change won't cause a cr50 reboot loop from cr50
trying and failing to enable factory mode during init. This only affects
the RMA and factory mode enable vendor commands.

BUG=b:129956462
BRANCH=cr50
TEST=Use rma and factory mode vendor commands to enable factory mode.

Change-Id: Ib8a502297040296fb0a2250a9e8945af330d4334
Signed-off-by: Mary Ruthven <mruthven@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1572450
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Keith Short <keithshort@chromium.org>

1 files changed, 22 insertions, 17 deletions

diff --git a/common/factory_mode.c b/common/factory_mode.c
index f2ed77cae6..be6c532304 100644
--- a/common/factory_mode.c
+++ b/common/factory_mode.c
@@ -15,34 +15,38 @@
 
 #define CPRINTS(format, args...) cprints(CC_CCD, format, ## args)
 
-static uint8_t ccd_hook_active;
+static uint8_t wait_for_factory_ccd_change;
 static uint8_t reset_required_;
 
-static void ccd_config_changed(void)
+static void factory_config_saved(int saved)
 {
-	if (!ccd_hook_active)
-		return;
+	wait_for_factory_ccd_change = 0;
 
-	ccd_hook_active = 0;
+	CPRINTS("%s: %s%s", __func__, saved ? "done" : "failed",
+		reset_required_ ? ", rebooting" : "");
 
 	if (!reset_required_)
 		return;
 
-	CPRINTS("%s: saved, rebooting\n", __func__);
 	cflush();
 	system_reset(SYSTEM_RESET_HARD);
 }
-DECLARE_HOOK(HOOK_CCD_CHANGE, ccd_config_changed, HOOK_PRIO_LAST);
 
-static void factory_enable_failed(void)
+static void ccd_config_changed(void)
 {
-	ccd_hook_active = 0;
-	CPRINTS("factory enable failed");
+	if (!wait_for_factory_ccd_change)
+		return;
 
-	if (reset_required_)
-		reset_required_ = 0;
+	factory_config_saved(1);
+}
+DECLARE_HOOK(HOOK_CCD_CHANGE, ccd_config_changed, HOOK_PRIO_LAST);
+
+static void force_system_reset(void)
+{
+	CPRINTS("%s: ccd hook didn't reset the system");
+	factory_config_saved(0);
 }
-DECLARE_DEFERRED(factory_enable_failed);
+DECLARE_DEFERRED(force_system_reset);
 
 /* The below time constants are way longer than should be required in practice:
  *
@@ -68,17 +72,18 @@ static void factory_enable_deferred(void)
 
 	CPRINTS("%s: TPM reset done, enabling factory mode", __func__);
 
-	ccd_hook_active = 1;
+	wait_for_factory_ccd_change = 1;
 	rv = ccd_reset_config(CCD_RESET_FACTORY);
 	if (rv != EC_SUCCESS)
-		factory_enable_failed();
+		factory_config_saved(0);
 
 	if (reset_required_) {
 		/*
 		 * Cr50 will reset once factory mode is enabled. If it hasn't in
-		 * TPM_RESET_TIME, declare factory enable failed.
+		 * TPM_RESET_TIME, declare factory enable failed and force the
+		 * reset.
 		 */
-		hook_call_deferred(&factory_enable_failed_data, TPM_RESET_TIME);
+		hook_call_deferred(&force_system_reset_data, TPM_RESET_TIME);
 	}
 }
 DECLARE_DEFERRED(factory_enable_deferred);