ec_sync: Go to recovery on aux fw update failure

If an aux firmware update fails enter recovery with a specific
reason code so we can identify systems that fail.

Also handle the case where the update succeeds and requests a
cold reset of the EC, first clearing the oprom flag if
necessary in order to prevent a second reset.

Unit test was added to check recovery reason for aux firmware
update failure.

BUG=b:74336712
BRANCH=eve
TEST=manual: force update to fail and ensure it goes to recovery
mode, and after successful update check that the option rom flag
is cleared before the EC reset happens.
Unit tests udpated and 'make runtests' passes.

Change-Id: I35a93892a0f8bb16eac0925ada5dfbc5c3144f8d
Signed-off-by: Duncan Laurie <dlaurie@google.com>
Reviewed-on: https://chromium-review.googlesource.com/959671
Reviewed-by: Caveh Jalali <caveh@google.com>
Reviewed-by: Furquan Shaikh <furquan@chromium.org>

4 files changed, 63 insertions, 19 deletions

diff --git a/firmware/lib/ec_sync.c b/firmware/lib/ec_sync.c
index c19e5d20..3c70ae15 100644
--- a/firmware/lib/ec_sync.c
+++ b/firmware/lib/ec_sync.c
@@ -438,6 +438,20 @@ VbError_t ec_sync_check_aux_fw(struct vb2_context *ctx,
 	return VbExCheckAuxFw(severity);
 }
 
+VbError_t ec_sync_update_aux_fw(struct vb2_context *ctx)
+{
+	VbError_t rv = VbExUpdateAuxFw();
+	if (rv) {
+		if (rv == VBERROR_EC_REBOOT_TO_RO_REQUIRED) {
+			VB2_DEBUG("AUX firmware update requires RO reboot.\n");
+		} else {
+			VB2_DEBUG("AUX firmware update/protect failed.\n");
+			request_recovery(ctx, VB2_RECOVERY_AUX_FW_UPDATE);
+		}
+	}
+	return rv;
+}
+
 VbError_t ec_sync_phase2(struct vb2_context *ctx)
 {
 	if (!ec_sync_allowed(ctx))
diff --git a/firmware/lib/ec_sync_all.c b/firmware/lib/ec_sync_all.c
index 68e0a9af..36d9e727 100644
--- a/firmware/lib/ec_sync_all.c
+++ b/firmware/lib/ec_sync_all.c
@@ -17,6 +17,28 @@
 #include "vboot_display.h"
 #include "vboot_kernel.h"
 
+static VbError_t ec_sync_unload_oprom(struct vb2_context *ctx,
+				      VbSharedDataHeader *shared,
+				      int need_wait_screen)
+{
+	/*
+	 * Reboot to unload VGA Option ROM if:
+	 * - we displayed the wait screen
+	 * - the system has slow EC update flag set
+	 * - the VGA Option ROM was needed and loaded
+	 * - the system is NOT in developer mode (that'll also need the ROM)
+	 */
+	if (need_wait_screen &&
+	    (shared->flags & VBSD_OPROM_MATTERS) &&
+	    (shared->flags & VBSD_OPROM_LOADED) &&
+	    !(shared->flags & VBSD_BOOT_DEV_SWITCH_ON)) {
+		VB2_DEBUG("Reboot to unload VGA Option ROM\n");
+		vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 0);
+		return VBERROR_VGA_OPROM_MISMATCH;
+	}
+	return VBERROR_SUCCESS;
+}
+
 VbError_t ec_sync_all(struct vb2_context *ctx)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
@@ -66,28 +88,21 @@ VbError_t ec_sync_all(struct vb2_context *ctx)
 		return rv;
 
 	/*
-	 * Do software sync for devices tunneled through the EC.
+	 * Do Aux FW software sync and protect devices tunneled through the EC.
+	 * Aux FW update may request RO reboot to force EC cold reset so also
+	 * unload the option ROM if needed to prevent a second reboot.
 	 */
-	rv = VbExUpdateAuxFw();
-	if (rv)
+	rv = ec_sync_update_aux_fw(ctx);
+	if (rv) {
+		ec_sync_unload_oprom(ctx, shared, need_wait_screen);
 		return rv;
-
-	/*
-	 * Reboot to unload VGA Option ROM if:
-	 * - we displayed the wait screen
-	 * - the system has slow EC update flag set
-	 * - the VGA Option ROM was needed and loaded
-	 * - the system is NOT in developer mode (that'll also need the ROM)
-	 */
-	if (need_wait_screen &&
-	    (shared->flags & VBSD_OPROM_MATTERS) &&
-	    (shared->flags & VBSD_OPROM_LOADED) &&
-	    !(shared->flags & VBSD_BOOT_DEV_SWITCH_ON)) {
-		VB2_DEBUG("Reboot to unload VGA Option ROM\n");
-		vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 0);
-		return VBERROR_VGA_OPROM_MISMATCH;
 	}
 
+	/* Reboot to unload VGA Option ROM if needed */
+	rv = ec_sync_unload_oprom(ctx, shared, need_wait_screen);
+	if (rv)
+		return rv;
+
 	/* Phase 3; Completes sync and handles battery cutoff */
 	rv = ec_sync_phase3(ctx);
 	if (rv)
diff --git a/firmware/lib/include/ec_sync.h b/firmware/lib/include/ec_sync.h
index 573c94cb..dfb8ec9c 100644
--- a/firmware/lib/include/ec_sync.h
+++ b/firmware/lib/include/ec_sync.h
@@ -49,6 +49,14 @@ VbError_t ec_sync_check_aux_fw(struct vb2_context *ctx,
 			       VbAuxFwUpdateSeverity_t *severity);
 
 /**
+ * Update and protect auxiliary firmware.
+ *
+ * @param ctx          Vboot2 context
+ * @return VBERROR_SUCCESS or non-zero error code.
+ */
+VbError_t ec_sync_update_aux_fw(struct vb2_context *ctx);
+
+/**
  * EC sync, phase 2
  *
  * This updates the EC if necessary, makes sure it has protected its image(s),
diff --git a/tests/ec_sync_tests.c b/tests/ec_sync_tests.c
index eceae78f..4c1775f2 100644
--- a/tests/ec_sync_tests.c
+++ b/tests/ec_sync_tests.c
@@ -56,6 +56,7 @@ static struct vb2_shared_data *sd;
 static uint32_t screens_displayed[8];
 static uint32_t screens_count = 0;
 
+static VbError_t ec_aux_fw_retval;
 static int ec_aux_fw_update_req;
 static VbAuxFwUpdateSeverity_t ec_aux_fw_mock_severity;
 static VbAuxFwUpdateSeverity_t ec_aux_fw_update_severity;
@@ -109,6 +110,7 @@ static void ResetMocks(void)
 	memset(screens_displayed, 0, sizeof(screens_displayed));
 	screens_count = 0;
 
+	ec_aux_fw_retval = VBERROR_SUCCESS;
 	ec_aux_fw_mock_severity = VB_AUX_FW_NO_UPDATE;
 	ec_aux_fw_update_severity = VB_AUX_FW_NO_UPDATE;
 	ec_aux_fw_update_req = 0;
@@ -219,7 +221,7 @@ VbError_t VbExUpdateAuxFw()
 {
 	ec_aux_fw_update_req = ec_aux_fw_update_severity != VB_AUX_FW_NO_UPDATE;
 	ec_aux_fw_protected = 1;
-	return VBERROR_SUCCESS;
+	return ec_aux_fw_retval;
 }
 
 static void test_ssync(VbError_t retval, int recovery_reason, const char *desc)
@@ -428,6 +430,11 @@ static void VbSoftwareSyncTest(void)
 	TEST_EQ(ec_aux_fw_protected, 1, "  aux fw protected");
 	TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT,
 		"  wait screen forced");
+
+	ResetMocks();
+	ec_aux_fw_retval = VBERROR_UNKNOWN;
+	test_ssync(VBERROR_UNKNOWN, VB2_RECOVERY_AUX_FW_UPDATE,
+		   "Error updating AUX firmware");
 }
 
 int main(void)