Modify EC software sync to update RO if necessarystabilize-7834.66.Bstabilize-7821.Brelease-R49-7834.B

Allow the AP to sync and verify the EC read only image after updating
the rewritable image.

BUG=chrome-os-partner:48703
BRANCH=none
TEST=manual

1. Update EC to a new version
2. rebuild EC code
3. Update AP firmware
4. Reboot and check that the RO image is updated after the RW image is
updated.

CQ-DEPEND=CL:319213

Change-Id: I774ef25320103f20d8c7d1c180a220dd0819c04d
Signed-off-by: Mary Ruthven <mruthven@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/320614
Reviewed-by: Randall Spangler <rspangler@chromium.org>

3 files changed, 361 insertions, 188 deletions

diff --git a/firmware/include/vboot_api.h b/firmware/include/vboot_api.h
index 4b7f1798..ddc8cc62 100644
--- a/firmware/include/vboot_api.h
+++ b/firmware/include/vboot_api.h
@@ -120,9 +120,11 @@ enum VbErrorPredefined_t {
 	VBERROR_UNSUPPORTED_REGION            = 0x10025,
 	/* No image present (returned from VbGbbReadImage() for missing image) */
 	VBERROR_NO_IMAGE_PRESENT              = 0x10026,
-
 	/* failed to draw screen */
 	VBERROR_SCREEN_DRAW                   = 0x10027,
+	/* failed to jump to RW image */
+	VBERROR_RW_JUMP_FAILED                = 0x10028,
+
 	/* VbExEcGetExpectedRWHash() may return the following codes */
 	/* Compute expected RW hash from the EC image; BIOS doesn't have it */
 	VBERROR_EC_GET_EXPECTED_HASH_FROM_IMAGE = 0x20000,
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index 903055b7..3cc150b4 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -654,162 +654,92 @@ static VbError_t EcProtect(int devidx, enum VbSelectFirmware_t select)
 	return rv;
 }
 
-VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
+static VbError_t EcUpdateImage(int devidx, VbCommonParams *cparams,
+			       enum VbSelectFirmware_t select,
+			       int *need_update, int in_rw)
 {
 	VbSharedDataHeader *shared =
 		(VbSharedDataHeader *)cparams->shared_data_blob;
-	enum VbSelectFirmware_t rw;
-	int in_rw = 0;
 	int rv;
-	const uint8_t *ec_hash = NULL;
-	int ec_hash_size;
-	const uint8_t *rw_hash = NULL;
-	int rw_hash_size;
+	int hash_size;
+	const uint8_t *hash = NULL;
 	const uint8_t *expected = NULL;
+	const uint8_t *ec_hash = NULL;
 	int expected_size;
 	uint8_t expected_hash[SHA256_DIGEST_SIZE];
-	int need_update = 0;
 	int i;
+	int rw_request = select != VB_SELECT_FIRMWARE_READONLY;
 
-	VBDEBUG(("VbEcSoftwareSync(devidx=%d)\n", devidx));
-	rw = shared->firmware_index ? VB_SELECT_FIRMWARE_B :
-	     VB_SELECT_FIRMWARE_A;
-
-	/* Determine whether the EC is in RO or RW */
-	rv = VbExEcRunningRW(devidx, &in_rw);
-
-	if (shared->recovery_reason) {
-		/* Recovery mode; just verify the EC is in RO code */
-		if (rv == VBERROR_SUCCESS && in_rw == 1) {
-			/*
-			 * EC is definitely in RW firmware.  We want it in
-			 * read-only code, so preserve the current recovery
-			 * reason and reboot.
-			 *
-			 * We don't reboot on error or unknown EC code, because
-			 * we could end up in an endless reboot loop.  If we
-			 * had some way to track that we'd already rebooted for
-			 * this reason, we could retry only once.
-			 */
-			VBDEBUG(("VbEcSoftwareSync() - "
-				 "want recovery but got EC-RW\n"));
-			VbSetRecoveryRequest(shared->recovery_reason);
-			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-		}
-
-		VBDEBUG(("VbEcSoftwareSync() in recovery; EC-RO\n"));
-		return VBERROR_SUCCESS;
-	}
-
-	/*
-	 * Not in recovery.  If we couldn't determine where the EC was,
-	 * reboot to recovery.
-	 */
-	if (rv != VBERROR_SUCCESS) {
-		VBDEBUG(("VbEcSoftwareSync() - "
-			 "VbExEcRunningRW() returned %d\n", rv));
-		VbSetRecoveryRequest(VBNV_RECOVERY_EC_UNKNOWN_IMAGE);
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-	}
-
-	/* If AP is read-only normal, EC should be in its RO code also. */
-	if (shared->flags & VBSD_LF_USE_RO_NORMAL) {
-		/* If EC is in RW code, request reboot back to RO */
-		if (in_rw == 1) {
-			VBDEBUG(("VbEcSoftwareSync() - "
-				 "want RO-normal but got EC-RW\n"));
-			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-		}
-
-		/* Protect the RW flash and stay in EC-RO */
-		rv = EcProtect(devidx, rw);
-		if (rv != VBERROR_SUCCESS)
-			return rv;
-
-		rv = VbExEcDisableJump(devidx);
-		if (rv != VBERROR_SUCCESS) {
-			VBDEBUG(("VbEcSoftwareSync() - "
-				 "VbExEcDisableJump() returned %d\n", rv));
-			VbSetRecoveryRequest(VBNV_RECOVERY_EC_SOFTWARE_SYNC);
-			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-		}
+	*need_update = 0;
+	VBDEBUG(("EcUpdateImage() - "
+		 "Check for %s update\n", rw_request ? "RW" : "RO"));
 
-		VBDEBUG(("VbEcSoftwareSync() in RO-Normal; EC-RO\n"));
-		return VBERROR_SUCCESS;
-	}
+	/* Get current EC hash. */
+	rv = VbExEcHashImage(devidx, select, &ec_hash, &hash_size);
 
-	/* Get hash of EC-RW */
-	rv = VbExEcHashImage(devidx, rw, &ec_hash, &ec_hash_size);
 	if (rv) {
-		VBDEBUG(("VbEcSoftwareSync() - "
+		VBDEBUG(("EcUpdateImage() - "
 			 "VbExEcHashImage() returned %d\n", rv));
 		VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_FAILED);
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
-	if (ec_hash_size != SHA256_DIGEST_SIZE) {
-		VBDEBUG(("VbEcSoftwareSync() - "
+	if (hash_size != SHA256_DIGEST_SIZE) {
+		VBDEBUG(("EcUpdateImage() - "
 			 "VbExEcHashImage() says size %d, not %d\n",
-			 ec_hash_size, SHA256_DIGEST_SIZE));
+			 hash_size, SHA256_DIGEST_SIZE));
 		VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_SIZE);
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
-
-	VBDEBUG(("EC hash:"));
+	VBDEBUG(("EC-%s hash:", rw_request ? "RW" : "RO"));
 	for (i = 0; i < SHA256_DIGEST_SIZE; i++)
-		VBDEBUG(("%02x", ec_hash[i]));
+		VBDEBUG(("%02x",ec_hash[i]));
 	VBDEBUG(("\n"));
 
-	/*
-	 * Get expected EC-RW hash. Note that we've already checked for
-	 * RO_NORMAL, so we know that the BIOS must be RW-A or RW-B, and
-	 * therefore the EC must match.
-	 */
-	rv = VbExEcGetExpectedImageHash(devidx, rw, &rw_hash, &rw_hash_size);
+	/* Get expected EC hash. */
+	rv = VbExEcGetExpectedImageHash(devidx, select, &hash, &hash_size);
 
 	if (rv == VBERROR_EC_GET_EXPECTED_HASH_FROM_IMAGE) {
 		/*
 		 * BIOS has verified EC image but doesn't have a precomputed
 		 * hash for it, so we must compute the hash ourselves.
 		 */
-		rw_hash = NULL;
+		hash = NULL;
 	} else if (rv) {
-		VBDEBUG(("VbEcSoftwareSync() - "
-			 "VbExEcGetExpectedRWHash() returned %d\n", rv));
+		VBDEBUG(("EcUpdateImage() - "
+			 "VbExEcGetExpectedImageHash() returned %d\n", rv));
 		VbSetRecoveryRequest(VBNV_RECOVERY_EC_EXPECTED_HASH);
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-	} else if (rw_hash_size != SHA256_DIGEST_SIZE) {
-		VBDEBUG(("VbEcSoftwareSync() - "
-			 "VbExEcGetExpectedRWHash() says size %d, not %d\n",
-			 rw_hash_size, SHA256_DIGEST_SIZE));
+	} else if (hash_size != SHA256_DIGEST_SIZE) {
+		VBDEBUG(("EcUpdateImage() - "
+			 "VbExEcGetExpectedImageHash() says size %d, not %d\n",
+			 hash_size, SHA256_DIGEST_SIZE));
 		VbSetRecoveryRequest(VBNV_RECOVERY_EC_EXPECTED_HASH);
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	} else {
 		VBDEBUG(("Expected hash:"));
 		for (i = 0; i < SHA256_DIGEST_SIZE; i++)
-			VBDEBUG(("%02x", rw_hash[i]));
+			VBDEBUG(("%02x", hash[i]));
 		VBDEBUG(("\n"));
-
-		need_update = SafeMemcmp(ec_hash, rw_hash, SHA256_DIGEST_SIZE);
+		*need_update = SafeMemcmp(ec_hash, hash, SHA256_DIGEST_SIZE);
 	}
 
 	/*
-	 * Get expected EC-RW image if we're sure we need to update (because the
+	 * Get expected EC image if we're sure we need to update (because the
 	 * expected hash didn't match the EC) or we still don't know (because
 	 * there was no expected hash and we need the image to compute one
 	 * ourselves).
 	 */
-	if (need_update || !rw_hash) {
-		/* Get expected EC-RW image */
-		rv = VbExEcGetExpectedImage(devidx, rw, &expected,
+	if (*need_update || !hash) {
+		/* Get expected EC image */
+		rv = VbExEcGetExpectedImage(devidx, select, &expected,
 					    &expected_size);
 		if (rv) {
-			VBDEBUG(("VbEcSoftwareSync() - "
-				 "VbExEcGetExpectedRW() returned %d\n", rv));
+			VBDEBUG(("EcUpdateImage() - "
+				 "VbExEcGetExpectedImage() returned %d\n", rv));
 			VbSetRecoveryRequest(VBNV_RECOVERY_EC_EXPECTED_IMAGE);
 			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 		}
-		VBDEBUG(("VbEcSoftwareSync() - expected len = %d\n",
-			 expected_size));
+		VBDEBUG(("EcUpdateImage() - image len = %d\n", expected_size));
 
 		/* Hash expected image */
 		internal_SHA256(expected, expected_size, expected_hash);
@@ -819,32 +749,27 @@ VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
 		VBDEBUG(("\n"));
 	}
 
-	if (!rw_hash) {
+	if (!hash) {
 		/*
 		 * BIOS didn't have expected EC hash, so check if we need
 		 * update by comparing EC hash to the one we just computed.
 		 */
-		need_update = SafeMemcmp(ec_hash, expected_hash,
-					 SHA256_DIGEST_SIZE);
-	} else if (need_update &&
-		   SafeMemcmp(rw_hash, expected_hash, SHA256_DIGEST_SIZE)) {
+		*need_update = SafeMemcmp(ec_hash, expected_hash,
+					  SHA256_DIGEST_SIZE);
+	} else if (*need_update && SafeMemcmp(hash, expected_hash,
+					      SHA256_DIGEST_SIZE)) {
 		/*
 		 * We need to update, but the expected EC image doesn't match
 		 * the expected EC hash we were given.
 		 */
-		VBDEBUG(("VbEcSoftwareSync() - "
-			 "VbExEcGetExpectedRW() returned %d\n", rv));
+		VBDEBUG(("EcUpdateImage() - "
+			 "VbExEcGetExpectedImage() returned %d\n", rv));
 		VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_MISMATCH);
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
 
-	/*
-	 * TODO: GBB flag to override whether we need update; needed for EC
-	 * development.
-	 */
-
-	if (in_rw) {
-		if (need_update) {
+	if (in_rw && rw_request) {
+		if (*need_update) {
 			/*
 			 * Check if BIOS should also load VGA Option ROM when
 			 * rebooting to save another reboot if possible.
@@ -852,7 +777,7 @@ VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
 			if ((shared->flags & VBSD_EC_SLOW_UPDATE) &&
 			    (shared->flags & VBSD_OPROM_MATTERS) &&
 			    !(shared->flags & VBSD_OPROM_LOADED)) {
-				VBDEBUG(("VbEcSoftwareSync() - Reboot to "
+				VBDEBUG(("EcUpdateImage() - Reboot to "
 					 "load VGA Option ROM\n"));
 				VbNvSet(&vnc, VBNV_OPROM_NEEDED, 1);
 			}
@@ -861,27 +786,29 @@ VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
 			 * EC is running the wrong RW image.  Reboot the EC to
 			 * RO so we can update it on the next boot.
 			 */
-			VBDEBUG(("VbEcSoftwareSync() - "
+			VBDEBUG(("EcUpdateImage() - "
 				 "in RW, need to update RW, so reboot\n"));
 			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 		}
 
-		VBDEBUG(("VbEcSoftwareSync() in EC-RW and it matches\n"));
+		VBDEBUG(("EcUpdateImage() in EC-RW and it matches\n"));
 		return VBERROR_SUCCESS;
 	}
 
 	/* Update EC if necessary */
-	if (need_update) {
-		VBDEBUG(("VbEcSoftwareSync() updating EC-RW...\n"));
+
+	if (*need_update) {
+		VBDEBUG(("EcUpdateImage() updating EC-%s...\n",
+			 rw_request ? "RW" : "RO"));
 
 		if (shared->flags & VBSD_EC_SLOW_UPDATE) {
-			VBDEBUG(("VbEcSoftwareSync() - "
+			VBDEBUG(("EcUpdateImage() - "
 				 "EC is slow. Show WAIT screen.\n"));
 
 			/* Ensure the VGA Option ROM is loaded */
 			if ((shared->flags & VBSD_OPROM_MATTERS) &&
 			    !(shared->flags & VBSD_OPROM_LOADED)) {
-				VBDEBUG(("VbEcSoftwareSync() - Reboot to "
+				VBDEBUG(("EcUpdateImage() - Reboot to "
 					 "load VGA Option ROM\n"));
 				VbNvSet(&vnc, VBNV_OPROM_NEEDED, 1);
 				return VBERROR_VGA_OPROM_MISMATCH;
@@ -890,16 +817,16 @@ VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
 			VbDisplayScreen(cparams, VB_SCREEN_WAIT, 0, &vnc);
 		}
 
-		rv = VbExEcUpdateImage(devidx, rw, expected, expected_size);
+		rv = VbExEcUpdateImage(devidx, select, expected, expected_size);
 
 		if (rv != VBERROR_SUCCESS) {
-			VBDEBUG(("VbEcSoftwareSync() - "
+			VBDEBUG(("EcUpdateImage() - "
 				 "VbExEcUpdateImage() returned %d\n", rv));
 
 			/*
 			 * 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
+			 * unprotect the region before updating, or may need to
+			 * reboot after updating.  Either way, it's not an error
 			 * requiring recovery mode.
 			 *
 			 * If we fail for any other reason, trigger recovery
@@ -911,38 +838,197 @@ VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
 			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 		}
 
-		/*
-		 * TODO: should ask EC to recompute its hash to verify it's
-		 * correct before continuing?
-		 */
 	}
 
-	/* Protect EC-RW flash */
-	rv = EcProtect(devidx, rw);
-	if (rv != VBERROR_SUCCESS)
-		return rv;
+	/* Verify the EC was updated properly */
+	if (*need_update) {
+		/* Get current EC hash. */
+		rv = VbExEcHashImage(devidx, select, &ec_hash, &hash_size);
 
-	/* Tell EC to jump to its RW image */
-	VBDEBUG(("VbEcSoftwareSync() jumping to EC-RW\n"));
-	rv = VbExEcJumpToRW(devidx);
+		if (rv) {
+			VBDEBUG(("EcUpdateImage() - "
+				 "VbExEcHashImage() returned %d\n", rv));
+			VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_FAILED);
+			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		}
+		if (hash_size != SHA256_DIGEST_SIZE) {
+			VBDEBUG(("EcUpdateImage() - "
+				 "VbExEcHashImage() says size %d, not %d\n",
+				 hash_size, SHA256_DIGEST_SIZE));
+			VbSetRecoveryRequest(VBNV_RECOVERY_EC_HASH_SIZE);
+			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		}
+		VBDEBUG(("Updated EC-%s hash:", rw_request ? "RW" : "RO"));
+		for (i = 0; i < SHA256_DIGEST_SIZE; i++)
+			VBDEBUG(("%02x",ec_hash[i]));
+		VBDEBUG(("\n"));
+
+		if (SafeMemcmp(ec_hash, hash, SHA256_DIGEST_SIZE)){
+			VBDEBUG(("EcUpdateImage() - "
+				 "Failed to update EC-%s\n", rw_request ?
+				 "RW" : "RO"));
+			VbSetRecoveryRequest(VBNV_RECOVERY_EC_UPDATE);
+			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		}
+	}
+	return VBERROR_SUCCESS;
+}
+
+VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
+{
+	VbSharedDataHeader *shared =
+		(VbSharedDataHeader *)cparams->shared_data_blob;
+	enum VbSelectFirmware_t select_rw =
+		shared->firmware_index ? VB_SELECT_FIRMWARE_B :
+		VB_SELECT_FIRMWARE_A;
+	enum VbSelectFirmware_t select_ro = VB_SELECT_FIRMWARE_READONLY;
+	int in_rw = 0;
+	int ro_try_count = 2;
+	int num_tries = 0;
+	uint32_t try_ro_sync, recovery_request;
+	int rv, updated_rw, updated_ro;
+
+	VBDEBUG(("VbEcSoftwareSync(devidx=%d)\n", devidx));
+
+	/* Determine whether the EC is in RO or RW */
+	rv = VbExEcRunningRW(devidx, &in_rw);
+
+	if (shared->recovery_reason) {
+		/* Recovery mode; just verify the EC is in RO code */
+		if (rv == VBERROR_SUCCESS && in_rw == 1) {
+			/*
+			 * EC is definitely in RW firmware.  We want it in
+			 * read-only code, so preserve the current recovery
+			 * reason and reboot.
+			 *
+			 * We don't reboot on error or unknown EC code, because
+			 * we could end up in an endless reboot loop.  If we
+			 * had some way to track that we'd already rebooted for
+			 * this reason, we could retry only once.
+			 */
+			VBDEBUG(("VbEcSoftwareSync() - "
+				 "want recovery but got EC-RW\n"));
+			VbSetRecoveryRequest(shared->recovery_reason);
+			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		}
+
+		VBDEBUG(("VbEcSoftwareSync() in recovery; EC-RO\n"));
+		return VBERROR_SUCCESS;
+	}
+
+	/*
+	 * Not in recovery.  If we couldn't determine where the EC was,
+	 * reboot to recovery.
+	 */
 	if (rv != VBERROR_SUCCESS) {
 		VBDEBUG(("VbEcSoftwareSync() - "
-			 "VbExEcJumpToRW() returned %d\n", rv));
+			 "VbExEcRunningRW() returned %d\n", rv));
+		VbSetRecoveryRequest(VBNV_RECOVERY_EC_UNKNOWN_IMAGE);
+		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+	}
 
-		/*
-		 * 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);
+	/* If AP is read-only normal, EC should be in its RO code also. */
+	if (shared->flags & VBSD_LF_USE_RO_NORMAL) {
+		/* If EC is in RW code, request reboot back to RO */
+		if (in_rw == 1) {
+			VBDEBUG(("VbEcSoftwareSync() - "
+				 "want RO-normal but got EC-RW\n"));
+			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		}
 
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		/* Protect the RW flash and stay in EC-RO */
+		rv = EcProtect(devidx, select_rw);
+		if (rv != VBERROR_SUCCESS)
+			return rv;
+
+		rv = VbExEcDisableJump(devidx);
+		if (rv != VBERROR_SUCCESS) {
+			VBDEBUG(("VbEcSoftwareSync() - "
+				 "VbExEcDisableJump() returned %d\n", rv));
+			VbSetRecoveryRequest(VBNV_RECOVERY_EC_SOFTWARE_SYNC);
+			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		}
+
+		VBDEBUG(("VbEcSoftwareSync() in RO-Normal; EC-RO\n"));
+		return VBERROR_SUCCESS;
+	}
+
+	VBDEBUG(("VbEcSoftwareSync() check for RW update.\n"));
+
+	/* Update the RW Image. */
+	rv = EcUpdateImage(devidx, cparams, select_rw, &updated_rw, in_rw);
+
+	if (rv != VBERROR_SUCCESS) {
+		VBDEBUG(("VbEcSoftwareSync() - "
+			 "EcUpdateImage() returned %d\n", rv));
+		return rv;
+	}
+
+	/* Tell EC to jump to its RW image */
+	if (!in_rw) {
+		VBDEBUG(("VbEcSoftwareSync() jumping to EC-RW\n"));
+		rv = VbExEcJumpToRW(devidx);
+		if (rv != VBERROR_SUCCESS) {
+			VBDEBUG(("VbEcSoftwareSync() - "
+				 "VbExEcJumpToRW() returned %x\n", rv));
+
+			/*
+			 * 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;
+		}
+	}
+
+	VbNvGet(&vnc, VBNV_TRY_RO_SYNC, &try_ro_sync);
+
+	if (!devidx && try_ro_sync &&
+	    !(shared->flags & VBSD_BOOT_FIRMWARE_WP_ENABLED)) {
+		/* Reset RO Software Sync NV flag */
+		VbNvSet(&vnc, VBNV_TRY_RO_SYNC, 0);
+
+		VbNvGet(&vnc, VBNV_RECOVERY_REQUEST, &recovery_request);
+
+		/* Update the RO Image. */
+		while (num_tries < ro_try_count) {
+			VBDEBUG(("VbEcSoftwareSync() RO Software Sync\n"));
+
+			/* Get expected EC-RO Image. */
+			rv = EcUpdateImage(devidx, cparams, select_ro,
+					   &updated_ro, in_rw);
+			if (rv == VBERROR_SUCCESS) {
+				/*
+				 * If the RO update had failed, reset the
+				 * recovery request.
+				 */
+				if (num_tries)
+					VbSetRecoveryRequest(recovery_request);
+				break;
+			} else
+				VBDEBUG(("VbEcSoftwareSync() - "
+					 "EcUpdateImage() returned %d\n", rv));
+
+			num_tries++;
+		}
 	}
+	if (rv != VBERROR_SUCCESS)
+		return rv;
 
-	VBDEBUG(("VbEcSoftwareSync() jumped to EC-RW\n"));
+	/* Protect RO flash */
+	rv = EcProtect(devidx, select_ro);
+	if (rv != VBERROR_SUCCESS)
+		return rv;
+
+	/* Protect RW flash */
+	rv = EcProtect(devidx, select_rw);
+	if (rv != VBERROR_SUCCESS)
+		return rv;
 
 	rv = VbExEcDisableJump(devidx);
 	if (rv != VBERROR_SUCCESS) {
@@ -959,7 +1045,7 @@ VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
 	 * - the system has slow EC update flag set
 	 * - the VGA Option ROM was needed and loaded
 	 */
-	if (need_update &&
+	if (updated_rw &&
 	    !(shared->flags & VBSD_BOOT_DEV_SWITCH_ON) &&
 	    (shared->flags & VBSD_EC_SLOW_UPDATE) &&
 	    (shared->flags & VBSD_OPROM_MATTERS) &&
@@ -969,8 +1055,8 @@ VbError_t VbEcSoftwareSync(int devidx, VbCommonParams *cparams)
 		return VBERROR_VGA_OPROM_MISMATCH;
 	}
 
-	VBDEBUG(("VbEcSoftwareSync() in RW; done\n"));
-	return VBERROR_SUCCESS;
+
+	return rv;
 }
 
 /* This function is also used by tests */
diff --git a/tests/vboot_api_kernel3_tests.c b/tests/vboot_api_kernel3_tests.c
index a4c5ea77..d6f740b6 100644
--- a/tests/vboot_api_kernel3_tests.c
+++ b/tests/vboot_api_kernel3_tests.c
@@ -30,17 +30,22 @@ static int trust_ec;
 static int mock_in_rw;
 static VbError_t in_rw_retval;
 static int protect_retval;
-static int ec_protected;
+static int ec_ro_protected;
+static int ec_rw_protected;
 static int run_retval;
 static int ec_run_image;
 static int update_retval;
-static int ec_updated;
+static int ec_ro_updated;
+static int ec_rw_updated;
 static int get_expected_retval;
 static int shutdown_request_calls_left;
 
-static uint8_t mock_ec_hash[32];
-static int mock_ec_hash_size;
+static uint8_t mock_ec_ro_hash[32];
+static uint8_t mock_ec_rw_hash[32];
+static int mock_ec_ro_hash_size;
+static int mock_ec_rw_hash_size;
 static uint8_t want_ec_hash[32];
+static uint8_t update_hash;
 static int want_ec_hash_size;
 static uint8_t mock_sha[32];
 
@@ -73,9 +78,11 @@ static void ResetMocks(void)
 
 	trust_ec = 0;
 	mock_in_rw = 0;
-	ec_protected = 0;
+	ec_ro_protected = 0;
+	ec_rw_protected = 0;
 	ec_run_image = 0;   /* 0 = RO, 1 = RW */
-	ec_updated = 0;
+	ec_ro_updated = 0;
+	ec_rw_updated = 0;
 	in_rw_retval = VBERROR_SUCCESS;
 	protect_retval = VBERROR_SUCCESS;
 	update_retval = VBERROR_SUCCESS;
@@ -83,14 +90,20 @@ static void ResetMocks(void)
 	get_expected_retval = VBERROR_SUCCESS;
 	shutdown_request_calls_left = -1;
 
-	Memset(mock_ec_hash, 0, sizeof(mock_ec_hash));
-	mock_ec_hash[0] = 42;
-	mock_ec_hash_size = sizeof(mock_ec_hash);
+	Memset(mock_ec_ro_hash, 0, sizeof(mock_ec_ro_hash));
+	mock_ec_ro_hash[0] = 42;
+	mock_ec_ro_hash_size = sizeof(mock_ec_ro_hash);
+
+	Memset(mock_ec_rw_hash, 0, sizeof(mock_ec_rw_hash));
+	mock_ec_rw_hash[0] = 42;
+	mock_ec_rw_hash_size = sizeof(mock_ec_rw_hash);
 
 	Memset(want_ec_hash, 0, sizeof(want_ec_hash));
 	want_ec_hash[0] = 42;
 	want_ec_hash_size = sizeof(want_ec_hash);
 
+	update_hash = 42;
+
 	Memset(mock_sha, 0, sizeof(want_ec_hash));
 	mock_sha[0] = 42;
 
@@ -125,7 +138,10 @@ VbError_t VbExEcRunningRW(int devidx, int *in_rw)
 
 VbError_t VbExEcProtect(int devidx, enum VbSelectFirmware_t select)
 {
-	ec_protected = 1;
+	if (select == VB_SELECT_FIRMWARE_READONLY)
+		ec_ro_protected = 1;
+	else
+		ec_rw_protected = 1;
 	return protect_retval;
 }
 
@@ -137,15 +153,18 @@ VbError_t VbExEcDisableJump(int devidx)
 VbError_t VbExEcJumpToRW(int devidx)
 {
 	ec_run_image = 1;
+	mock_in_rw = 1;
 	return run_retval;
 }
 
 VbError_t VbExEcHashImage(int devidx, enum VbSelectFirmware_t select,
 			  const uint8_t **hash, int *hash_size)
 {
-	*hash = mock_ec_hash;
-	*hash_size = mock_ec_hash_size;
-	return mock_ec_hash_size ? VBERROR_SUCCESS : VBERROR_SIMULATED;
+	*hash = select == VB_SELECT_FIRMWARE_READONLY ?
+		mock_ec_ro_hash : mock_ec_rw_hash;
+	*hash_size = select == VB_SELECT_FIRMWARE_READONLY ?
+		     mock_ec_ro_hash_size : mock_ec_rw_hash_size;
+	return *hash_size ? VBERROR_SUCCESS : VBERROR_SIMULATED;
 }
 
 VbError_t VbExEcGetExpectedImage(int devidx, enum VbSelectFirmware_t select,
@@ -178,7 +197,13 @@ uint8_t *internal_SHA256(const uint8_t *data, uint64_t len, uint8_t *digest)
 VbError_t VbExEcUpdateImage(int devidx, enum VbSelectFirmware_t select,
 			    const uint8_t *image, int image_size)
 {
-	ec_updated = 1;
+	if (select == VB_SELECT_FIRMWARE_READONLY) {
+		ec_ro_updated = 1;
+		mock_ec_ro_hash[0] = update_hash;
+	 } else {
+		ec_rw_updated = 1;
+		mock_ec_rw_hash[0] = update_hash;
+	}
 	return update_retval;
 }
 
@@ -208,7 +233,7 @@ static void VbSoftwareSyncTest(void)
 	ResetMocks();
 	shared->recovery_reason = 123;
 	test_ssync(0, 0, "In recovery, EC-RO");
-	TEST_EQ(ec_protected, 0, "  ec protected");
+	TEST_EQ(ec_rw_protected, 0, "  ec rw protected");
 
 	ResetMocks();
 	shared->recovery_reason = 123;
@@ -231,7 +256,7 @@ static void VbSoftwareSyncTest(void)
 	ResetMocks();
 	shared->flags |= VBSD_LF_USE_RO_NORMAL;
 	test_ssync(0, 0, "AP-RO, EC-RO");
-	TEST_EQ(ec_protected, 1, "  ec protected");
+	TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
 	TEST_EQ(ec_run_image, 0, "  ec run image");
 
 	ResetMocks();
@@ -254,12 +279,12 @@ static void VbSoftwareSyncTest(void)
 
 	/* Calculate hashes */
 	ResetMocks();
-	mock_ec_hash_size = 0;
+	mock_ec_rw_hash_size = 0;
 	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
 		   VBNV_RECOVERY_EC_HASH_FAILED, "Bad EC hash");
 
 	ResetMocks();
-	mock_ec_hash_size = 16;
+	mock_ec_rw_hash_size = 16;
 	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
 		   VBNV_RECOVERY_EC_HASH_SIZE, "Bad EC hash size");
 
@@ -295,32 +320,90 @@ static void VbSoftwareSyncTest(void)
 
 	ResetMocks();
 	mock_in_rw = 1;
-	mock_ec_hash[0]++;
+	mock_ec_rw_hash[0]++;
 	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
 		   0, "Pending update needs reboot");
 
 	ResetMocks();
-	mock_ec_hash[0]++;
-	test_ssync(0, 0, "Update without reboot");
-	TEST_EQ(ec_protected, 1, "  ec protected");
+	mock_ec_rw_hash[0]++;
+	VbNvSet(VbApiKernelGetVnc(), VBNV_TRY_RO_SYNC, 1);
+	test_ssync(0, 0, "Update rw without reboot");
+	TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
+	TEST_EQ(ec_run_image, 1, "  ec run image");
+	TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
+	TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
+	TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
+
+	ResetMocks();
+	mock_ec_rw_hash[0]++;
+	mock_ec_ro_hash[0]++;
+	VbNvSet(VbApiKernelGetVnc(), VBNV_TRY_RO_SYNC, 1);
+	test_ssync(0, 0, "Update rw and ro images without reboot");
+	TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
+	TEST_EQ(ec_run_image, 1, "  ec run image");
+	TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
+	TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
+	TEST_EQ(ec_ro_updated, 1, "  ec ro updated");
+
+	ResetMocks();
+	shared->flags |= VBSD_BOOT_FIRMWARE_WP_ENABLED;
+	VbNvSet(VbApiKernelGetVnc(), VBNV_TRY_RO_SYNC, 1);
+	mock_ec_rw_hash[0]++;
+	mock_ec_ro_hash[0]++;
+	test_ssync(0, 0, "WP enabled");
+	TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
 	TEST_EQ(ec_run_image, 1, "  ec run image");
-	TEST_EQ(ec_updated, 1, "  ec updated");
+	TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
+	TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
+	TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
+
+	ResetMocks();
+	VbNvSet(VbApiKernelGetVnc(), VBNV_TRY_RO_SYNC, 1);
+	mock_ec_ro_hash[0]++;
+	test_ssync(0, 0, "rw update not needed");
+	TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
+	TEST_EQ(ec_run_image, 1, "  ec run image");
+	TEST_EQ(ec_rw_updated, 0, "  ec rw not updated");
+	TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
+	TEST_EQ(ec_ro_updated, 1, "  ec ro updated");
+
+	ResetMocks();
+	mock_ec_rw_hash[0]++;
+	mock_ec_ro_hash[0]++;
+	test_ssync(0, 0, "ro update not requested");
+	TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
+	TEST_EQ(ec_run_image, 1, "  ec run image");
+	TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
+	TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
+	TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
+
+	ResetMocks();
+	mock_ec_rw_hash[0]++;
+	update_hash++;
+	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
+		   VBNV_RECOVERY_EC_UPDATE, "updated hash mismatch");
+	TEST_EQ(ec_rw_protected, 0, "  ec rw protected");
+	TEST_EQ(ec_run_image, 0, "  ec run image");
+	TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
+	TEST_EQ(ec_ro_protected, 0, "  ec ro protected");
+	TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
 
 	ResetMocks();
-	mock_ec_hash[0]++;
+	mock_ec_rw_hash[0]++;
 	update_retval = VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
-		   0, "Reboot after update");
-	TEST_EQ(ec_updated, 1, "  ec updated");
+		   0, "Reboot after rw update");
+	TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
+	TEST_EQ(ec_ro_updated, 0, "  ec rw updated");
 
 	ResetMocks();
-	mock_ec_hash[0]++;
+	mock_ec_rw_hash[0]++;
 	update_retval = VBERROR_SIMULATED;
 	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
 		   VBNV_RECOVERY_EC_UPDATE, "Update failed");
 
 	ResetMocks();
-	mock_ec_hash[0]++;
+	mock_ec_rw_hash[0]++;
 	shared->flags |= VBSD_EC_SLOW_UPDATE;
 	test_ssync(0, 0, "Slow update");
 	TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT, "  wait screen");
@@ -332,9 +415,11 @@ static void VbSoftwareSyncTest(void)
 
 	ResetMocks();
 	test_ssync(0, 0, "AP-RW, EC-RO -> EC-RW");
-	TEST_EQ(ec_protected, 1, "  ec protected");
+	TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
 	TEST_EQ(ec_run_image, 1, "  ec run image");
-	TEST_EQ(ec_updated, 0, "  ec updated");
+	TEST_EQ(ec_rw_updated, 0, "  ec rw updated");
+	TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
+	TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
 
 	ResetMocks();
 	run_retval = VBERROR_SIMULATED;