Fix jumping EC from RO to RW if disable-jump was called on previous boot

It's possible for the AP to get updated and remove the RO-normal flag
without needing to update EC-RW firmware - for example, if it only
needs to update the BIOS.  In this case, the EC doesn't need update,
but does need to jump to its RW firmware.  But if the EC is already
booted RO-normal with jump disabled, it will refuse that request and
go to recovery mode.

The fix is simply to check if the request to jump to RW requires the
EC to cold-boot first, and pass through that error code to the caller.

BUG=chrome-os-partner:22617
BRANCH=none (affects all platforms, but only in this odd case, and this
       is a change to the RW portion of the code)
TEST=pass new unit test which triggers this condition

Change-Id: Ia8d64dff784a9135ef23f6eb26bbca4ad9df57c3
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/170168
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

2 files changed, 32 insertions, 12 deletions

diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index 7f85dc75..e137e6d4 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -827,20 +827,23 @@ VbError_t VbEcSoftwareSync(VbCommonParams *cparams)
 		}
 
 		rv = VbExEcUpdateRW(expected, expected_size);
-		if (rv == VBERROR_EC_REBOOT_TO_RO_REQUIRED) {
+
+		if (rv != VBERROR_SUCCESS) {
+			VBDEBUG(("VbEcSoftwareSync() - "
+				 "VbExEcUpdateRW() returned %d\n", rv));
+
 			/*
-			 * Reboot required.  May need to unprotect RW before
-			 * updating, or may need to reboot after RW updated.
-			 * Either way, it's not an error requiring recovery
+			 * The EC may know it needs a reboot.  It may need to
+			 * unprotect RW before updating, or may need to reboot
+			 * after RW updated.  Either way, it's not an error
+			 * requiring recovery mode.
+			 *
+			 * If we fail for any other reason, trigger recovery
 			 * mode.
 			 */
-			VBDEBUG(("VbEcSoftwareSync() - "
-				 "VbExEcUpdateRW() needs reboot\n"));
-			return rv;
-		} else if (rv != VBERROR_SUCCESS) {
-			VBDEBUG(("VbEcSoftwareSync() - "
-				 "VbExEcUpdateRW() returned %d\n", rv));
-			VbSetRecoveryRequest(VBNV_RECOVERY_EC_UPDATE);
+			if (rv != VBERROR_EC_REBOOT_TO_RO_REQUIRED)
+				VbSetRecoveryRequest(VBNV_RECOVERY_EC_UPDATE);
+
 			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 		}
 
@@ -858,12 +861,24 @@ VbError_t VbEcSoftwareSync(VbCommonParams *cparams)
 	/* Tell EC to jump to its RW image */
 	VBDEBUG(("VbEcSoftwareSync() jumping to EC-RW\n"));
 	rv = VbExEcJumpToRW();
+
 	if (rv != VBERROR_SUCCESS) {
 		VBDEBUG(("VbEcSoftwareSync() - "
 			 "VbExEcJumpToRW() returned %d\n", rv));
-		VbSetRecoveryRequest(VBNV_RECOVERY_EC_JUMP_RW);
+
+		/*
+		 * If the EC booted RO-normal and a previous AP boot has called
+		 * VbExEcStayInRO(), we need to reboot the EC to unlock the
+		 * ability to jump to the RW firmware.
+		 *
+		 * All other errors trigger recovery mode.
+		 */
+		if (rv != VBERROR_EC_REBOOT_TO_RO_REQUIRED)
+			VbSetRecoveryRequest(VBNV_RECOVERY_EC_JUMP_RW);
+
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
+
 	VBDEBUG(("VbEcSoftwareSync() jumped to EC-RW\n"));
 
 	rv = VbExEcDisableJump();
diff --git a/tests/vboot_api_kernel3_tests.c b/tests/vboot_api_kernel3_tests.c
index cefbf416..49bc7aad 100644
--- a/tests/vboot_api_kernel3_tests.c
+++ b/tests/vboot_api_kernel3_tests.c
@@ -340,6 +340,11 @@ static void VbSoftwareSyncTest(void)
 		   VBNV_RECOVERY_EC_JUMP_RW, "Jump to RW fail");
 
 	ResetMocks();
+	run_retval = VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
+		   0, "Jump to RW fail because locked");
+
+	ResetMocks();
 	protect_retval = VBERROR_SIMULATED;
 	test_ssync(VBERROR_SIMULATED,
 		   VBNV_RECOVERY_EC_PROTECT, "Protect error");