firmware: Refactor and clean up ec_sync

Previously, the EC software sync process called VbDisplayScreen() from
several function calls deep.  Refactor software sync so that the UI
decisions are at a higher level (in ec_sync_all.c) and isolated from
the low-level EC software sync functionality (in ec_sync.c).

This is one in a series of changes which are more clearly separating
out the UI, to make it easier to support multiple UI across a range of
devices.

BUG=chromium:611535
BRANCH=none
TEST=make runtests; build_packages --board=reef chromeos-firmware; boot reef

Change-Id: I40597abeb5b0cc8f5d8fc2098e4acbed4bf59bf6
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/411921
Reviewed-by: Shelley Chen <shchen@chromium.org>

11 files changed, 500 insertions, 299 deletions

diff --git a/Makefile b/Makefile
index dab004ef..316d9acb 100644
--- a/Makefile
+++ b/Makefile
@@ -332,6 +332,7 @@ VBSLK_SRCS = \
 	firmware/lib/cgptlib/cgptlib_internal.c \
 	firmware/lib/cgptlib/crc32.c \
 	firmware/lib/ec_sync.c \
+	firmware/lib/ec_sync_all.c \
 	firmware/lib/gpt_misc.c \
 	firmware/lib/utility_string.c \
 	firmware/lib/vboot_api_kernel.c \
diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h
index 75484783..f754cded 100644
--- a/firmware/2lib/include/2return_codes.h
+++ b/firmware/2lib/include/2return_codes.h
@@ -488,6 +488,15 @@ enum vb2_return_code {
 	/* Unable to verify body in vb2_load_partition() */
 	VB2_ERROR_LOAD_PARTITION_VERIFY_BODY,
 
+	/* Unable to get EC image hash in ec_sync_phase1() */
+	VB2_ERROR_EC_HASH_IMAGE,
+
+	/* Unable to get expected EC image hash in ec_sync_phase1() */
+	VB2_ERROR_EC_HASH_EXPECTED,
+
+	/* Expected and image hashes are different size in ec_sync_phase1() */
+	VB2_ERROR_EC_HASH_SIZE,
+
         /**********************************************************************
 	 * API-level errors
 	 */
diff --git a/firmware/2lib/include/2struct.h b/firmware/2lib/include/2struct.h
index da687b0c..0d1c6ec3 100644
--- a/firmware/2lib/include/2struct.h
+++ b/firmware/2lib/include/2struct.h
@@ -41,6 +41,15 @@ enum vb2_shared_data_flags {
 
 	/* Kernel keyblock was verified by signature (not just hash) */
 	VB2_SD_FLAG_KERNEL_SIGNED = (1 << 2),
+
+	/* Software sync needs to update EC-RO, EC-RW, or PD-RW respectively */
+	VB2_SD_FLAG_ECSYNC_EC_RO = (1 << 3),
+	VB2_SD_FLAG_ECSYNC_EC_RW = (1 << 4),
+	VB2_SD_FLAG_ECSYNC_PD_RW = (1 << 5),
+
+	/* Software sync says EC / PD running RW */
+	VB2_SD_FLAG_ECSYNC_EC_IN_RW = (1 << 6),
+	VB2_SD_FLAG_ECSYNC_PD_IN_RW = (1 << 7),
 };
 
 /* Flags for vb2_shared_data.status */
diff --git a/firmware/include/vboot_struct.h b/firmware/include/vboot_struct.h
index f2704559..6be7256a 100644
--- a/firmware/include/vboot_struct.h
+++ b/firmware/include/vboot_struct.h
@@ -206,7 +206,10 @@ typedef struct VbKernelPreambleHeader {
 #define VBSD_KERNEL_KEY_VERIFIED         0x00000002
 /* LoadFirmware() was told the developer switch was on */
 #define VBSD_LF_DEV_SWITCH_ON            0x00000004
-/* LoadFirmware() is requesting the read only normal/dev code path */
+/*
+ * LoadFirmware() is requesting the read only normal/dev code path.  This is
+ * deprecated and unsupported by current firmware.
+ */
 #define VBSD_LF_USE_RO_NORMAL            0x00000008
 /* Developer switch was enabled at boot time */
 #define VBSD_BOOT_DEV_SWITCH_ON          0x00000010
diff --git a/firmware/lib/ec_sync.c b/firmware/lib/ec_sync.c
index a32c296f..d263c28f 100644
--- a/firmware/lib/ec_sync.c
+++ b/firmware/lib/ec_sync.c
@@ -7,15 +7,29 @@
 
 #include "2sysincludes.h"
 #include "2common.h"
+#include "2misc.h"
 #include "2nvstorage.h"
+
 #include "sysincludes.h"
-#include "utility.h"
-#include "vb2_common.h"
-#include "vboot_api.h"
+#include "ec_sync.h"
+#include "gbb_header.h"
 #include "vboot_common.h"
-#include "vboot_display.h"
 #include "vboot_kernel.h"
 
+#define VB2_SD_FLAG_ECSYNC_RW					\
+	(VB2_SD_FLAG_ECSYNC_EC_RW | VB2_SD_FLAG_ECSYNC_PD_RW)
+#define VB2_SD_FLAG_ECSYNC_ANY					\
+	(VB2_SD_FLAG_ECSYNC_EC_RO | VB2_SD_FLAG_ECSYNC_RW)
+#define VB2_SD_FLAG_ECSYNC_IN_RW					\
+	(VB2_SD_FLAG_ECSYNC_EC_IN_RW | VB2_SD_FLAG_ECSYNC_PD_IN_RW)
+
+#define IN_RW(devidx)							\
+	((devidx) ? VB2_SD_FLAG_ECSYNC_PD_IN_RW : VB2_SD_FLAG_ECSYNC_EC_IN_RW)
+
+#define WHICH_EC(devidx, select) \
+	((select) == VB_SELECT_FIRMWARE_READONLY ? VB2_SD_FLAG_ECSYNC_EC_RO : \
+	 ((devidx) ? VB2_SD_FLAG_ECSYNC_PD_RW : VB2_SD_FLAG_ECSYNC_EC_RW))
+
 static void request_recovery(struct vb2_context *ctx, uint32_t recovery_request)
 {
 	VB2_DEBUG("request_recovery(%u)\n", recovery_request);
@@ -26,133 +40,120 @@ static void request_recovery(struct vb2_context *ctx, uint32_t recovery_request)
 /**
  * Wrapper around VbExEcProtect() which sets recovery reason on error.
  */
-static VbError_t EcProtect(struct vb2_context *ctx, int devidx,
+static VbError_t protect_ec(struct vb2_context *ctx, int devidx,
 			   enum VbSelectFirmware_t select)
 {
 	int rv = VbExEcProtect(devidx, select);
 
 	if (rv == VBERROR_EC_REBOOT_TO_RO_REQUIRED) {
-		VBDEBUG(("VbExEcProtect() needs reboot\n"));
+		VB2_DEBUG("VbExEcProtect() needs reboot\n");
 	} else if (rv != VBERROR_SUCCESS) {
-		VBDEBUG(("VbExEcProtect() returned %d\n", rv));
+		VB2_DEBUG("VbExEcProtect() returned %d\n", rv);
 		request_recovery(ctx, VB2_RECOVERY_EC_PROTECT);
 	}
 	return rv;
 }
 
-static VbError_t EcUpdateImage(struct vb2_context *ctx, int devidx,
-			       VbCommonParams *cparams,
-			       enum VbSelectFirmware_t select,
-			       int *need_update, int in_rw)
+/**
+ * Print a hash to debug output
+ *
+ * @param hash		Pointer to the hash
+ * @param hash_size	Size of the hash in bytes
+ * @param desc		Description of what's being hashed
+ */
+static void print_hash(const uint8_t *hash, uint32_t hash_size,
+		       const char *desc)
 {
-	VbSharedDataHeader *shared =
-		(VbSharedDataHeader *)cparams->shared_data_blob;
-	int rv;
-	int hash_size;
-	int ec_hash_size;
-	const uint8_t *hash = NULL;
-	const uint8_t *expected = NULL;
-	const uint8_t *ec_hash = NULL;
-	int expected_size;
 	int i;
-	int rw_request = select != VB_SELECT_FIRMWARE_READONLY;
 
-	*need_update = 0;
-	VBDEBUG(("EcUpdateImage() - "
-		 "Check for %s update\n", rw_request ? "RW" : "RO"));
+	VB2_DEBUG("%s hash: ", desc);
+	for (i = 0; i < hash_size; i++)
+		VB2_DEBUG("%02x", hash[i]);
+	VB2_DEBUG("\n");
+}
+
+/**
+ * Check if the hash of the EC code matches the expected hash.
+ *
+ * @param ctx		Vboot2 context
+ * @param devidx	Index of EC device to check
+ * @param select	Which firmware image to check
+ * @return VB2_SUCCESS, or non-zero error code.
+ */
+static int check_ec_hash(struct vb2_context *ctx, int devidx,
+			 enum VbSelectFirmware_t select)
+{
+	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 
 	/* Get current EC hash. */
-	rv = VbExEcHashImage(devidx, select, &ec_hash, &ec_hash_size);
+	const uint8_t *ec_hash = NULL;
+	int ec_hash_size;
+	int rv = VbExEcHashImage(devidx, select, &ec_hash, &ec_hash_size);
 	if (rv) {
-		VBDEBUG(("EcUpdateImage() - "
-			 "VbExEcHashImage() returned %d\n", rv));
+		VB2_DEBUG("%s: VbExEcHashImage() returned %d\n", __func__, rv);
 		request_recovery(ctx, VB2_RECOVERY_EC_HASH_FAILED);
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		return VB2_ERROR_EC_HASH_IMAGE;
 	}
-	VBDEBUG(("EC-%s hash: ", rw_request ? "RW" : "RO"));
-	for (i = 0; i < ec_hash_size; i++)
-		VBDEBUG(("%02x",ec_hash[i]));
-	VBDEBUG(("\n"));
+	print_hash(ec_hash, ec_hash_size,
+		   select == VB_SELECT_FIRMWARE_READONLY ? "RO" : "RW");
 
 	/* Get expected EC hash. */
+	const uint8_t *hash = NULL;
+	int hash_size;
 	rv = VbExEcGetExpectedImageHash(devidx, select, &hash, &hash_size);
 	if (rv) {
-		VBDEBUG(("EcUpdateImage() - "
-			 "VbExEcGetExpectedImageHash() returned %d\n", rv));
+		VB2_DEBUG("%s: VbExEcGetExpectedImageHash() returned %d\n",
+			  __func__, rv);
 		request_recovery(ctx, VB2_RECOVERY_EC_EXPECTED_HASH);
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		return VB2_ERROR_EC_HASH_EXPECTED;
 	}
 	if (ec_hash_size != hash_size) {
-		VBDEBUG(("EcUpdateImage() - "
-			 "EC uses %d-byte hash, but AP-RW contains %d bytes\n",
-			 ec_hash_size, hash_size));
+		VB2_DEBUG("%s: EC uses %d-byte hash, but AP-RW contains %d "
+			  "bytes\n", __func__, ec_hash_size, hash_size);
 		request_recovery(ctx, VB2_RECOVERY_EC_HASH_SIZE);
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		return VB2_ERROR_EC_HASH_SIZE;
 	}
 
-	VBDEBUG(("Expected hash: "));
-	for (i = 0; i < hash_size; i++)
-		VBDEBUG(("%02x", hash[i]));
-	VBDEBUG(("\n"));
-	*need_update = vb2_safe_memcmp(ec_hash, hash, hash_size);
-
-	if (!*need_update)
-		return VBERROR_SUCCESS;
-
-	/* Get expected EC image */
-	rv = VbExEcGetExpectedImage(devidx, select, &expected, &expected_size);
-	if (rv) {
-		VBDEBUG(("EcUpdateImage() - "
-			 "VbExEcGetExpectedImage() returned %d\n", rv));
-		request_recovery(ctx, VB2_RECOVERY_EC_EXPECTED_IMAGE);
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-	}
-	VBDEBUG(("EcUpdateImage() - image len = %d\n", expected_size));
-
-	if (in_rw && rw_request) {
-		/*
-		 * Check if BIOS should also load VGA Option ROM when
-		 * rebooting to save another reboot if possible.
-		 */
-		if ((shared->flags & VBSD_EC_SLOW_UPDATE) &&
-		    (shared->flags & VBSD_OPROM_MATTERS) &&
-		    !(shared->flags & VBSD_OPROM_LOADED)) {
-			VBDEBUG(("EcUpdateImage() - Reboot to "
-				 "load VGA Option ROM\n"));
-			vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 1);
-		}
-
-		/*
-		 * EC is running the wrong RW image.  Reboot the EC to
-		 * RO so we can update it on the next boot.
-		 */
-		VBDEBUG(("EcUpdateImage() - "
-			 "in RW, need to update RW, so reboot\n"));
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+	if (vb2_safe_memcmp(ec_hash, hash, hash_size)) {
+		print_hash(hash, hash_size, "Expected");
+		sd->flags |= WHICH_EC(devidx, select);
 	}
 
-	VBDEBUG(("EcUpdateImage() updating EC-%s...\n",
-		 rw_request ? "RW" : "RO"));
+	return VB2_SUCCESS;
+}
 
-	if (shared->flags & VBSD_EC_SLOW_UPDATE) {
-		VBDEBUG(("EcUpdateImage() - EC is slow. Show WAIT screen.\n"));
+/**
+ * Update the specified EC and verify the update succeeded
+ *
+ * @param ctx		Vboot2 context
+ * @param devidx	Index of EC device to check
+ * @param select	Which firmware image to check
+ * @return VBERROR_SUCCESS, or non-zero error code.
+ */
+static VbError_t update_ec(struct vb2_context *ctx, int devidx,
+			   enum VbSelectFirmware_t select)
+{
+	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 
-		/* Ensure the VGA Option ROM is loaded */
-		if ((shared->flags & VBSD_OPROM_MATTERS) &&
-		    !(shared->flags & VBSD_OPROM_LOADED)) {
-			VBDEBUG(("EcUpdateImage() - Reboot to "
-				 "load VGA Option ROM\n"));
-			vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 1);
-			return VBERROR_VGA_OPROM_MISMATCH;
-		}
+	VB2_DEBUG("%s: updating %s...\n", __func__,
+		  select == VB_SELECT_FIRMWARE_READONLY ? "RO" : "RW");
 
-		VbDisplayScreen(ctx, cparams, VB_SCREEN_WAIT, 0);
+	/* Get expected EC image */
+	const uint8_t *want = NULL;
+	int want_size;
+	int rv = VbExEcGetExpectedImage(devidx, select, &want, &want_size);
+	if (rv) {
+		VB2_DEBUG("%s: VbExEcGetExpectedImage() returned %d\n",
+			  __func__, rv);
+		request_recovery(ctx, VB2_RECOVERY_EC_EXPECTED_IMAGE);
+		return rv;
 	}
+	VB2_DEBUG("%s: image len = %d\n", __func__, want_size);
 
-	rv = VbExEcUpdateImage(devidx, select, expected, expected_size);
+	rv = VbExEcUpdateImage(devidx, select, want, want_size);
 	if (rv != VBERROR_SUCCESS) {
-		VBDEBUG(("EcUpdateImage() - "
-			 "VbExEcUpdateImage() returned %d\n", rv));
+		VB2_DEBUG("%s: VbExEcUpdateImage() returned %d\n",
+			  __func__, rv);
 
 		/*
 		 * The EC may know it needs a reboot.  It may need to
@@ -166,33 +167,15 @@ static VbError_t EcUpdateImage(struct vb2_context *ctx, int devidx,
 		if (rv != VBERROR_EC_REBOOT_TO_RO_REQUIRED)
 			request_recovery(ctx, VB2_RECOVERY_EC_UPDATE);
 
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		return rv;
 	}
 
 	/* Verify the EC was updated properly */
-	rv = VbExEcHashImage(devidx, select, &ec_hash, &ec_hash_size);
-	if (rv) {
-		VBDEBUG(("EcUpdateImage() - "
-			 "VbExEcHashImage() returned %d\n", rv));
-		request_recovery(ctx, VB2_RECOVERY_EC_HASH_FAILED);
+	sd->flags &= ~WHICH_EC(devidx, select);
+	if (check_ec_hash(ctx, devidx, select) != VB2_SUCCESS)
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-	}
-	if (hash_size != ec_hash_size) {
-		VBDEBUG(("EcUpdateImage() - "
-			 "VbExEcHashImage() says size %d, not %d\n",
-			 ec_hash_size, hash_size));
-		request_recovery(ctx, VB2_RECOVERY_EC_HASH_SIZE);
-		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-	}
-	VBDEBUG(("Updated EC-%s hash: ", rw_request ? "RW" : "RO"));
-	for (i = 0; i < ec_hash_size; i++)
-		VBDEBUG(("%02x",ec_hash[i]));
-	VBDEBUG(("\n"));
-
-	if (vb2_safe_memcmp(ec_hash, hash, hash_size)){
-		VBDEBUG(("EcUpdateImage() - "
-			 "Failed to update EC-%s\n", rw_request ?
-			 "RW" : "RO"));
+	if (sd->flags & WHICH_EC(devidx, select)) {
+		VB2_DEBUG("%s: Failed to update\n", __func__);
 		request_recovery(ctx, VB2_RECOVERY_EC_UPDATE);
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
@@ -200,28 +183,29 @@ static VbError_t EcUpdateImage(struct vb2_context *ctx, int devidx,
 	return VBERROR_SUCCESS;
 }
 
-VbError_t VbEcSoftwareSync(struct vb2_context *ctx, int devidx,
-			   VbCommonParams *cparams)
+/**
+ * Check if the EC has the correct image active.
+ *
+ * @param ctx		Vboot2 context
+ * @param devidx	Which device (EC=0, PD=1)
+ * @return VBERROR_SUCCESS, or non-zero if error.
+ */
+static VbError_t check_ec_active(struct vb2_context *ctx, int devidx)
 {
-	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 recovery_request;
-	int rv, updated_rw, updated_ro;
-
-	VBDEBUG(("VbEcSoftwareSync(devidx=%d)\n", devidx));
+	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 
 	/* Determine whether the EC is in RO or RW */
-	rv = VbExEcRunningRW(devidx, &in_rw);
+	int in_rw = 0;
+	int rv = VbExEcRunningRW(devidx, &in_rw);
+	if (in_rw) {
+		sd->flags |= IN_RW(devidx);
+	}
 
-	if (shared->recovery_reason) {
-		/* Recovery mode; just verify the EC is in RO code */
+	if (sd->recovery_reason) {
+		/*
+		 * Recovery mode; just verify the EC is in RO code.  Don't do
+		 * software sync, since we don't have a RW image.
+		 */
 		if (rv == VBERROR_SUCCESS && in_rw == 1) {
 			/*
 			 * EC is definitely in RW firmware.  We want it in
@@ -233,13 +217,13 @@ VbError_t VbEcSoftwareSync(struct vb2_context *ctx, int devidx,
 			 * 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"));
-			request_recovery(ctx, shared->recovery_reason);
+			VB2_DEBUG("%s: want recovery but got EC-RW\n",
+				  __func__);
+			request_recovery(ctx, sd->recovery_reason);
 			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 		}
 
-		VBDEBUG(("VbEcSoftwareSync() in recovery; EC-RO\n"));
+		VB2_DEBUG("%s: in recovery; EC-RO\n", __func__);
 		return VBERROR_SUCCESS;
 	}
 
@@ -248,61 +232,54 @@ VbError_t VbEcSoftwareSync(struct vb2_context *ctx, int devidx,
 	 * reboot to recovery.
 	 */
 	if (rv != VBERROR_SUCCESS) {
-		VBDEBUG(("VbEcSoftwareSync() - "
-			 "VbExEcRunningRW() returned %d\n", rv));
+		VB2_DEBUG("%s: VbExEcRunningRW() returned %d\n", __func__, rv);
 		request_recovery(ctx, VB2_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(ctx, devidx, select_rw);
-		if (rv != VBERROR_SUCCESS)
-			return rv;
-
-		rv = VbExEcDisableJump(devidx);
-		if (rv != VBERROR_SUCCESS) {
-			VBDEBUG(("VbEcSoftwareSync() - "
-				 "VbExEcDisableJump() returned %d\n", rv));
-			request_recovery(ctx, VB2_RECOVERY_EC_SOFTWARE_SYNC);
-			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
-		}
+	return VBERROR_SUCCESS;
+}
 
-		VBDEBUG(("VbEcSoftwareSync() in RO-Normal; EC-RO\n"));
-		return VBERROR_SUCCESS;
-	}
+#define RO_RETRIES 2  /* Maximum times to retry flashing RO */
 
-	VBDEBUG(("VbEcSoftwareSync() check for RW update.\n"));
+/**
+ * Sync, jump, and protect one EC device
+ *
+ * @param ctx		Vboot2 context
+ * @param devidx	Which device (EC=0, PD=1)
+ * @return VBERROR_SUCCESS, or non-zero if error.
+ */
+static VbError_t sync_one_ec(struct vb2_context *ctx, int devidx,
+			     VbCommonParams *cparams)
+{
+	VbSharedDataHeader *shared =
+			(VbSharedDataHeader *)cparams->shared_data_blob;
+	struct vb2_shared_data *sd = vb2_get_sd(ctx);
+	const enum VbSelectFirmware_t select_rw =
+		shared->firmware_index ? VB_SELECT_FIRMWARE_B :
+		VB_SELECT_FIRMWARE_A;
+	int rv;
 
-	/* Update the RW Image. */
-	rv = EcUpdateImage(ctx, devidx, cparams, select_rw, &updated_rw, in_rw);
+	VB2_DEBUG("%s: devidx=%d\n", __func__, devidx);
 
-	if (rv != VBERROR_SUCCESS) {
-		VBDEBUG(("VbEcSoftwareSync() - "
-			 "EcUpdateImage() returned %d\n", rv));
-		return rv;
+	/* Update the RW Image */
+	if (sd->flags & VB2_SD_FLAG_ECSYNC_RW) {
+		if (VB2_SUCCESS != update_ec(ctx, devidx, select_rw))
+			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
 
 	/* Tell EC to jump to its RW image */
-	if (!in_rw) {
-		VBDEBUG(("VbEcSoftwareSync() jumping to EC-RW\n"));
+	if (!(sd->flags & IN_RW(devidx))) {
+		VB2_DEBUG("%s: jumping to EC-RW\n", __func__);
 		rv = VbExEcJumpToRW(devidx);
 		if (rv != VBERROR_SUCCESS) {
-			VBDEBUG(("VbEcSoftwareSync() - "
-				 "VbExEcJumpToRW() returned %x\n", rv));
+			VB2_DEBUG("%s: VbExEcJumpToRW() returned %x\n",
+				  __func__, 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.
+			 * If 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.
 			 */
@@ -313,75 +290,170 @@ VbError_t VbEcSoftwareSync(struct vb2_context *ctx, int devidx,
 		}
 	}
 
-	uint32_t try_ro_sync = vb2_nv_get(ctx, VB2_NV_TRY_RO_SYNC);
-	if (!devidx && try_ro_sync &&
-	    !(shared->flags & VBSD_BOOT_FIRMWARE_WP_ENABLED)) {
+	/* Might need to update EC-RO (but not PD-RO) */
+	if (sd->flags & VB2_SD_FLAG_ECSYNC_EC_RO) {
+		VB2_DEBUG("%s: RO Software Sync\n", __func__);
+
 		/* Reset RO Software Sync NV flag */
 		vb2_nv_set(ctx, VB2_NV_TRY_RO_SYNC, 0);
 
-		recovery_request = vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST);
+		/*
+		 * Get the current recovery request (if any).  This gets
+		 * overwritten by a failed try.  If a later try succeeds, we'll
+		 * need to restore this request (or the lack of a request), or
+		 * else we'll end up in recovery mode even though RO software
+		 * sync did eventually succeed.
+		 */
+		uint32_t recovery_request =
+				vb2_nv_get(ctx, VB2_NV_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(ctx, 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)
-					request_recovery(ctx, recovery_request);
+		int num_tries;
+		for (num_tries = 0; num_tries < RO_RETRIES; num_tries++) {
+			if (VB2_SUCCESS ==
+			    update_ec(ctx, devidx, VB_SELECT_FIRMWARE_READONLY))
 				break;
-			} else
-				VBDEBUG(("VbEcSoftwareSync() - "
-					 "EcUpdateImage() returned %d\n", rv));
-
-			num_tries++;
+		}
+		if (num_tries == RO_RETRIES) {
+			/* Ran out of tries */
+			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+		} else if (num_tries) {
+			/*
+			 * Update succeeded after a failure, so we've polluted
+			 * the recovery request.  Restore it.
+			 */
+			request_recovery(ctx, recovery_request);
 		}
 	}
-	if (rv != VBERROR_SUCCESS)
-		return rv;
 
 	/* Protect RO flash */
-	rv = EcProtect(ctx, devidx, select_ro);
+	rv = protect_ec(ctx, devidx, VB_SELECT_FIRMWARE_READONLY);
 	if (rv != VBERROR_SUCCESS)
 		return rv;
 
 	/* Protect RW flash */
-	rv = EcProtect(ctx, devidx, select_rw);
+	rv = protect_ec(ctx, devidx, select_rw);
 	if (rv != VBERROR_SUCCESS)
 		return rv;
 
 	rv = VbExEcDisableJump(devidx);
 	if (rv != VBERROR_SUCCESS) {
-		VBDEBUG(("VbEcSoftwareSync() - "
-			"VbExEcDisableJump() returned %d\n", rv));
+		VB2_DEBUG("%s: VbExEcDisableJump() returned %d\n",
+			  __func__, rv);
 		request_recovery(ctx, VB2_RECOVERY_EC_SOFTWARE_SYNC);
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
 
+	return rv;
+}
+
+VbError_t ec_sync_phase1(struct vb2_context *ctx, VbCommonParams *cparams)
+{
+	VbSharedDataHeader *shared =
+		(VbSharedDataHeader *)cparams->shared_data_blob;
+	struct vb2_shared_data *sd = vb2_get_sd(ctx);
+
+	/* Reasons not to do sync at all */
+	if (!(shared->flags & VBSD_EC_SOFTWARE_SYNC))
+		return VBERROR_SUCCESS;
+	if (cparams->gbb->flags & GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC)
+		return VBERROR_SUCCESS;
+
+#ifdef PD_SYNC
+	const int do_pd_sync = !(cparams->gbb->flags &
+				 GBB_FLAG_DISABLE_PD_SOFTWARE_SYNC);
+#else
+	const int do_pd_sync = 0;
+#endif
+
+	/* Make sure the EC is running the correct image */
+	if (check_ec_active(ctx, 0))
+		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+	if (do_pd_sync && check_ec_active(ctx, 1))
+		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+
+	/*
+	 * In recovery mode; just verify the EC is in RO code.  Don't do
+	 * software sync, since we don't have a RW image.
+	 */
+	if (sd->recovery_reason)
+		return VBERROR_SUCCESS;
+
+	/* See if we need to update RW.  Failures trigger recovery mode. */
+	const enum VbSelectFirmware_t select_rw =
+			shared->firmware_index ? VB_SELECT_FIRMWARE_B :
+			VB_SELECT_FIRMWARE_A;
+	if (check_ec_hash(ctx, 0, select_rw))
+		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+	if (do_pd_sync && check_ec_hash(ctx, 1, select_rw))
+		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+	/*
+	 * See if we need to update EC-RO (devidx=0).
+	 *
+	 * If we want to extend this in the future to update PD-RO, we'll use a
+	 * different NV flag so we can track EC-RO and PD-RO updates
+	 * separately.
+	 */
+	if (vb2_nv_get(ctx, VB2_NV_TRY_RO_SYNC) &&
+	    !(shared->flags & VBSD_BOOT_FIRMWARE_WP_ENABLED) &&
+	    check_ec_hash(ctx, 0, VB_SELECT_FIRMWARE_READONLY)) {
+		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+	}
+
 	/*
-	 * Reboot to unload VGA Option ROM if:
-	 * - RW update was done
-	 * - the system is NOT in developer mode
-	 * - the system has slow EC update flag set
-	 * - the VGA Option ROM was needed and loaded
+	 * If we're in RW, we need to reboot back to RO because RW can't be
+	 * updated while we're running it.
+	 *
+	 * TODO: Technically this isn't true for ECs which don't execute from
+	 * flash.  For example, if the EC loads code from SPI into RAM before
+	 * executing it.
 	 */
-	if (updated_rw &&
-	    !(shared->flags & VBSD_BOOT_DEV_SWITCH_ON) &&
-	    (shared->flags & VBSD_EC_SLOW_UPDATE) &&
-	    (shared->flags & VBSD_OPROM_MATTERS) &&
-	    (shared->flags & VBSD_OPROM_LOADED)) {
-		VBDEBUG(("VbEcSoftwareSync() - Reboot to "
-			 "unload VGA Option ROM\n"));
-		vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 0);
-		return VBERROR_VGA_OPROM_MISMATCH;
+	if ((sd->flags & VB2_SD_FLAG_ECSYNC_RW) &&
+	    (sd->flags & VB2_SD_FLAG_ECSYNC_IN_RW)) {
+		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
 
+	return VBERROR_SUCCESS;
+}
 
-	return rv;
+int ec_will_update_slowly(struct vb2_context *ctx, VbCommonParams *cparams)
+{
+	VbSharedDataHeader *shared =
+		(VbSharedDataHeader *)cparams->shared_data_blob;
+	struct vb2_shared_data *sd = vb2_get_sd(ctx);
+
+	return ((sd->flags & VB2_SD_FLAG_ECSYNC_ANY) &&
+		(shared->flags & VBSD_EC_SLOW_UPDATE));
+}
+
+
+VbError_t ec_sync_phase2(struct vb2_context *ctx, VbCommonParams *cparams)
+{
+	VbSharedDataHeader *shared =
+		(VbSharedDataHeader *)cparams->shared_data_blob;
+	struct vb2_shared_data *sd = vb2_get_sd(ctx);
+
+	/* Reasons not to do sync at all */
+	if (!(shared->flags & VBSD_EC_SOFTWARE_SYNC))
+		return VBERROR_SUCCESS;
+	if (cparams->gbb->flags & GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC)
+		return VBERROR_SUCCESS;
+	if (sd->recovery_reason)
+		return VBERROR_SUCCESS;
+
+	/* Handle updates and jumps for EC */
+	VbError_t retval = sync_one_ec(ctx, 0, cparams);
+	if (retval != VBERROR_SUCCESS)
+		return retval;
+
+#ifdef PD_SYNC
+	/* Handle updates and jumps for PD */
+	if (!(cparams->gbb->flags & GBB_FLAG_DISABLE_PD_SOFTWARE_SYNC)) {
+		retval = sync_one_ec(ctx, 1, cparams);
+		if (retval != VBERROR_SUCCESS)
+			return retval;
+	}
+#endif
+
+	return VBERROR_SUCCESS;
 }
diff --git a/firmware/lib/ec_sync_all.c b/firmware/lib/ec_sync_all.c
new file mode 100644
index 00000000..2ad61035
--- /dev/null
+++ b/firmware/lib/ec_sync_all.c
@@ -0,0 +1,81 @@
+/* Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * EC software sync routines for vboot
+ */
+
+#include "2sysincludes.h"
+#include "2common.h"
+#include "2misc.h"
+#include "2nvstorage.h"
+
+#include "sysincludes.h"
+#include "ec_sync.h"
+#include "vboot_api.h"
+#include "vboot_common.h"
+#include "vboot_display.h"
+#include "vboot_kernel.h"
+
+VbError_t ec_sync_all(struct vb2_context *ctx, struct VbCommonParams *cparams)
+{
+	VbSharedDataHeader *shared =
+		(VbSharedDataHeader *)cparams->shared_data_blob;
+
+	/* Do EC sync phase 1; this determines if we need an update */
+	VbError_t phase1_rv = ec_sync_phase1(ctx, cparams);
+	int need_wait_screen = ec_will_update_slowly(ctx, cparams);
+
+	/*
+	 * Check if we need to reboot to load the VGA Option ROM before we can
+	 * display the WAIT screen.
+	 *
+	 * Do this before we check if ec_sync_phase1() requires a reboot for
+	 * some other reason, since there's no reason to reboot twice.
+	 */
+	int reboot_for_oprom = (need_wait_screen &&
+				shared->flags & VBSD_OPROM_MATTERS &&
+				!(shared->flags & VBSD_OPROM_LOADED));
+	if (reboot_for_oprom) {
+		VB2_DEBUG("%s: Reboot to load VGA Option ROM\n", __func__);
+		vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 1);
+	}
+
+	/* Reboot if phase 1 needed it, or if we need to load VGA Option ROM */
+	if (phase1_rv)
+		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+	if (reboot_for_oprom)
+		return VBERROR_VGA_OPROM_MISMATCH;
+
+	/* Display the wait screen if we need it */
+	if (need_wait_screen) {
+		VB2_DEBUG("%s: EC is slow. Show WAIT screen.\n", __func__);
+		VbDisplayScreen(ctx, cparams, VB_SCREEN_WAIT, 0);
+	}
+
+	/*
+	 * Do EC sync phase 2; this applies the update and/or jumps to the
+	 * correct EC image.
+	 */
+	VbError_t rv = ec_sync_phase2(ctx, cparams);
+	if (rv)
+		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("%s: Reboot to unload VGA Option ROM\n", __func__);
+		vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 0);
+		return VBERROR_VGA_OPROM_MISMATCH;
+	}
+
+	return VBERROR_SUCCESS;
+}
diff --git a/firmware/lib/include/ec_sync.h b/firmware/lib/include/ec_sync.h
new file mode 100644
index 00000000..dd133920
--- /dev/null
+++ b/firmware/lib/include/ec_sync.h
@@ -0,0 +1,76 @@
+/* Copyright 2016 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * EC software sync for verified boot
+ */
+
+#ifndef VBOOT_REFERENCE_EC_SYNC_H_
+#define VBOOT_REFERENCE_EC_SYNC_H_
+
+#include "vboot_api.h"
+
+struct vb2_context;
+struct VbCommonParams;
+
+/**
+ * EC sync, phase 1
+ *
+ * This checks whether the EC is running the correct image to do EC sync, and
+ * whether any updates are necessary.
+ *
+ * @param ctx		Vboot2 context
+ * @param cparams	Vboot common params
+ * @return VBERROR_SUCCESS, VBERROR_EC_REBOOT_TO_RO_REQUIRED if the EC must
+ * reboot back to its RO code to continue EC sync, or other non-zero error
+ * code.
+ */
+VbError_t ec_sync_phase1(struct vb2_context *ctx,
+			 struct VbCommonParams *cparams);
+
+/**
+ * Returns non-zero if the EC will perform a slow update during phase 2.
+ *
+ * This is only valid after calling ec_sync_phase1(), before calling
+ * ec_sync_phase2().
+ *
+ * @param ctx		Vboot2 context
+ * @param cparams	Vboot common params
+ * @return non-zero if a slow update will be done; zero if no update or a
+ * fast update.
+ */
+int ec_will_update_slowly(struct vb2_context *ctx,
+			  struct VbCommonParams *cparams);
+
+/**
+ * EC sync, phase 2
+ *
+ * This updates the EC if necessary, makes sure it has protected its image(s),
+ * and makes sure it has jumped to the correct image.
+ *
+ * If ec_will_update_slowly(), it is suggested that the caller display a
+ * warning screen before calling phase 2.
+ *
+ * @param ctx		Vboot2 context
+ * @param cparams	Vboot common params
+ * @return VBERROR_SUCCESS, VBERROR_EC_REBOOT_TO_RO_REQUIRED if the EC must
+ * reboot back to its RO code to continue EC sync, or other non-zero error
+ * code.
+ */
+VbError_t ec_sync_phase2(struct vb2_context *ctx,
+			 struct VbCommonParams *cparams);
+
+/**
+ * Sync all EC devices to expected versions.
+ *
+ * This is a high-level function which calls the functions above.
+ *
+ * @param ctx		Vboot context
+ * @param devidx	EC device index to sync
+ * @param cparams	Common vboot params
+ * @return VBERROR_SUCCESS, or non-zero if error.
+ */
+VbError_t ec_sync_all(struct vb2_context *ctx, struct
+		      VbCommonParams *cparams);
+
+#endif  /* VBOOT_REFERENCE_EC_SYNC_H_ */
diff --git a/firmware/lib/include/vboot_kernel.h b/firmware/lib/include/vboot_kernel.h
index b6a2a858..1c9dafcb 100644
--- a/firmware/lib/include/vboot_kernel.h
+++ b/firmware/lib/include/vboot_kernel.h
@@ -77,14 +77,4 @@ VbError_t VbBootDeveloper(struct vb2_context *ctx, VbCommonParams *cparams,
 VbError_t VbBootRecovery(struct vb2_context *ctx, VbCommonParams *cparams,
 			 LoadKernelParams *p);
 
-/**
- * Sync EC device <devidx> firmware to expected version.
- *
- * @param ctx		Vboot context
- * @param devidx	EC device index to sync
- * @param cparams	Common vboot params
- */
-VbError_t VbEcSoftwareSync(struct vb2_context *ctx, int devidx,
-			   VbCommonParams *cparams);
-
 #endif  /* VBOOT_REFERENCE_VBOOT_KERNEL_H_ */
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index 597dbc18..7e48daae 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -12,6 +12,7 @@
 #include "2misc.h"
 #include "2nvstorage.h"
 #include "2rsa.h"
+#include "ec_sync.h"
 #include "gbb_access.h"
 #include "gbb_header.h"
 #include "load_kernel_fw.h"
@@ -798,6 +799,9 @@ VbError_t VbSelectAndLoadKernel(VbCommonParams *cparams,
 		return VBERROR_INIT_SHARED_DATA;
 	}
 
+	struct vb2_shared_data *sd = vb2_get_sd(&ctx);
+	sd->recovery_reason = shared->recovery_reason;
+
 	/* Clear output params in case we fail */
 	kparams->disk_handle = NULL;
 	kparams->partition_number = 0;
@@ -813,34 +817,9 @@ VbError_t VbSelectAndLoadKernel(VbCommonParams *cparams,
 		goto VbSelectAndLoadKernel_exit;
 
 	/* Do EC software sync if necessary */
-	if ((shared->flags & VBSD_EC_SOFTWARE_SYNC) &&
-	    !(cparams->gbb->flags & GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC)) {
-		int oprom_mismatch = 0;
-
-		retval = VbEcSoftwareSync(&ctx, 0, cparams);
-		/* Save reboot requested until after possible PD sync */
-		if (retval == VBERROR_VGA_OPROM_MISMATCH)
-			oprom_mismatch = 1;
-		else if (retval != VBERROR_SUCCESS)
-			goto VbSelectAndLoadKernel_exit;
-
-#ifdef PD_SYNC
-		if (!(cparams->gbb->flags &
-		      GBB_FLAG_DISABLE_PD_SOFTWARE_SYNC)) {
-			retval = VbEcSoftwareSync(&ctx, 1, cparams);
-			if (retval == VBERROR_VGA_OPROM_MISMATCH)
-				oprom_mismatch = 1;
-			else if (retval != VBERROR_SUCCESS)
-				goto VbSelectAndLoadKernel_exit;
-		}
-#endif
-
-		/* Request reboot to unload VGA Option ROM */
-		if (oprom_mismatch) {
-			retval = VBERROR_VGA_OPROM_MISMATCH;
-			goto VbSelectAndLoadKernel_exit;
-		}
-	}
+	retval = ec_sync_all(&ctx, cparams);
+	if (retval != VBERROR_SUCCESS)
+		goto VbSelectAndLoadKernel_exit;
 
 	/* EC verification (and possibily updating / jumping) is done */
 	retval = VbExEcVbootDone(!!shared->recovery_reason);
diff --git a/tests/ec_sync_tests.c b/tests/ec_sync_tests.c
index dfdd27bc..b632fb6c 100644
--- a/tests/ec_sync_tests.c
+++ b/tests/ec_sync_tests.c
@@ -13,6 +13,7 @@
 #include "2common.h"
 #include "2misc.h"
 #include "2nvstorage.h"
+#include "ec_sync.h"
 #include "gbb_header.h"
 #include "host_common.h"
 #include "load_kernel_fw.h"
@@ -54,6 +55,7 @@ static uint8_t update_hash;
 static int want_ec_hash_size;
 static struct vb2_context ctx;
 static uint8_t workbuf[VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE];
+static struct vb2_shared_data *sd;
 
 static uint32_t screens_displayed[8];
 static uint32_t screens_count = 0;
@@ -69,16 +71,19 @@ static void ResetMocks(void)
 	memset(&gbb, 0, sizeof(gbb));
 	gbb.major_version = GBB_MAJOR_VER;
 	gbb.minor_version = GBB_MINOR_VER;
-	gbb.flags = 0;
+	gbb.flags = GBB_FLAG_DISABLE_PD_SOFTWARE_SYNC;
+	cparams.gbb = &gbb;
 
 	memset(&ctx, 0, sizeof(ctx));
 	ctx.workbuf = workbuf;
 	ctx.workbuf_size = sizeof(workbuf);
 	vb2_init_context(&ctx);
 	vb2_nv_init(&ctx);
+	sd = vb2_get_sd(&ctx);
 
 	memset(&shared_data, 0, sizeof(shared_data));
 	VbSharedDataInit(shared, sizeof(shared_data));
+	shared->flags = VBSD_EC_SOFTWARE_SYNC;
 
 	trust_ec = 0;
 	mock_in_rw = 0;
@@ -210,7 +215,7 @@ VbError_t VbDisplayScreen(struct vb2_context *ctx, VbCommonParams *cparams,
 
 static void test_ssync(VbError_t retval, int recovery_reason, const char *desc)
 {
-	TEST_EQ(VbEcSoftwareSync(&ctx, 0, &cparams), retval, desc);
+	TEST_EQ(ec_sync_all(&ctx, &cparams), retval, desc);
 	TEST_EQ(vb2_nv_get(&ctx, VB2_NV_RECOVERY_REQUEST),
 		recovery_reason, "  recovery reason");
 }
@@ -221,12 +226,12 @@ static void VbSoftwareSyncTest(void)
 {
 	/* Recovery cases */
 	ResetMocks();
-	shared->recovery_reason = 123;
+	sd->recovery_reason = 123;
 	test_ssync(0, 0, "In recovery, EC-RO");
 	TEST_EQ(ec_rw_protected, 0, "  ec rw protected");
 
 	ResetMocks();
-	shared->recovery_reason = 123;
+	sd->recovery_reason = 123;
 	mock_in_rw = 1;
 	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
 		   123, "Recovery needs EC-RO");
@@ -237,36 +242,6 @@ static void VbSoftwareSyncTest(void)
 	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
 		   VBNV_RECOVERY_EC_UNKNOWN_IMAGE, "Unknown EC image");
 
-	ResetMocks();
-	shared->flags |= VBSD_LF_USE_RO_NORMAL;
-	mock_in_rw = 1;
-	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
-		   0, "AP-RO needs EC-RO");
-
-	ResetMocks();
-	shared->flags |= VBSD_LF_USE_RO_NORMAL;
-	test_ssync(0, 0, "AP-RO, EC-RO");
-	TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
-	TEST_EQ(ec_run_image, 0, "  ec run image");
-
-	ResetMocks();
-	shared->flags |= VBSD_LF_USE_RO_NORMAL;
-	run_retval = VBERROR_SIMULATED;
-	test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
-		   VBNV_RECOVERY_EC_SOFTWARE_SYNC, "Stay in RO fail");
-
-	ResetMocks();
-	shared->flags |= VBSD_LF_USE_RO_NORMAL;
-	protect_retval = VBERROR_SIMULATED;
-	test_ssync(VBERROR_SIMULATED,
-		   VBNV_RECOVERY_EC_PROTECT, "Protect error");
-
-	/* No longer check for shutdown requested */
-	ResetMocks();
-	shared->flags |= VBSD_LF_USE_RO_NORMAL;
-	shutdown_request_calls_left = 0;
-	test_ssync(0, 0, "AP-RO shutdown requested");
-
 	/* Calculate hashes */
 	ResetMocks();
 	mock_ec_rw_hash_size = 0;
diff --git a/tests/vboot_api_kernel4_tests.c b/tests/vboot_api_kernel4_tests.c
index decc7137..0b1223d7 100644
--- a/tests/vboot_api_kernel4_tests.c
+++ b/tests/vboot_api_kernel4_tests.c
@@ -9,6 +9,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include "ec_sync.h"
 #include "gbb_header.h"
 #include "host_common.h"
 #include "load_kernel_fw.h"
@@ -81,8 +82,7 @@ VbError_t VbExNvStorageWrite(const uint8_t *buf)
 	return VBERROR_SUCCESS;
 }
 
-VbError_t VbEcSoftwareSync(struct vb2_context *ctx, int devidx,
-			   VbCommonParams *cparams)
+VbError_t VbExEcRunningRW(int devidx, int *in_rw)
 {
 	return ecsync_retval;
 }
@@ -160,16 +160,22 @@ static void VbSlkTest(void)
 	ResetMocks();
 	test_slk(0, 0, "Normal");
 
-	/* Software sync */
+	/* Mock error early in software sync */
 	ResetMocks();
 	shared->flags |= VBSD_EC_SOFTWARE_SYNC;
 	ecsync_retval = VBERROR_SIMULATED;
-	test_slk(VBERROR_SIMULATED, 0, "EC sync bad");
+	test_slk(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
+		 VBNV_RECOVERY_EC_UNKNOWN_IMAGE, "EC sync bad");
 
+	/*
+	 * If shared->flags doesn't ask for software sync, we won't notice
+	 * that error.
+	 */
 	ResetMocks();
 	ecsync_retval = VBERROR_SIMULATED;
 	test_slk(0, 0, "EC sync not done");
 
+	/* Same if shared->flags asks for sync, but it's overridden by GBB */
 	ResetMocks();
 	shared->flags |= VBSD_EC_SOFTWARE_SYNC;
 	gbb.flags |= GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC;