vboot: decouple EC/AUXFW sync from UI

Since we don't always want to show a UI on EC sync (for example, in
coreboot, where display hasn't been initialized), decouple
vb2api_ec_sync() from VbDisplayScreen() by leaving screen display out of
vboot and letting the caller (such as depthcharge) handle it. Similarly,
stop calling screen display function from vb2api_auxfw_sync().

In order to display screen from depthcharge, it needs to know the
locale. Therefore, add vb2api_get_locale() to vboot API, which returns
the locale from nvdata.

After this change, the constant EC_SLOW_UPDATE is no longer used, so
remove it from Makefile.

BRANCH=none
BUG=chromium:1055125
TEST=make runtests

Cq-Depend: chromium:2117776
Change-Id: I0e2e8ebdd26d48a2e94d36495c2e45a5734cdc5d
Signed-off-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2087016
Reviewed-by: Joel Kitching <kitching@chromium.org>

7 files changed, 81 insertions, 176 deletions

diff --git a/Makefile b/Makefile
index 2b8655c1..913f4ac2 100644
--- a/Makefile
+++ b/Makefile
@@ -173,13 +173,6 @@ else
 CFLAGS += -DUSB_BOOT_ON_DEV=0
 endif
 
-# EC software sync is slow to update. Enable warning screen display.
-ifneq ($(filter-out 0,${EC_SLOW_UPDATE}),)
-CFLAGS += -DEC_SLOW_UPDATE=1
-else
-CFLAGS += -DEC_SLOW_UPDATE=0
-endif
-
 # Enable EC early firmware selection.
 ifneq ($(filter-out 0,${EC_EFS}),)
 CFLAGS += -DEC_EFS=1
diff --git a/firmware/2lib/2auxfw_sync.c b/firmware/2lib/2auxfw_sync.c
index d5182b36..1e173582 100644
--- a/firmware/2lib/2auxfw_sync.c
+++ b/firmware/2lib/2auxfw_sync.c
@@ -14,15 +14,6 @@
 #include "vboot_display.h"
 
 /**
- * Display the WAIT screen
- */
-static void display_wait_screen(struct vb2_context *ctx)
-{
-	VB2_DEBUG("AUX FW update is slow. Show WAIT screen.\n");
-	VbDisplayScreen(ctx, VB_SCREEN_WAIT, 0, NULL);
-}
-
-/**
  * Determine if we are allowed to update auxfw
  *
  * @param ctx		Vboot2 context
@@ -102,14 +93,6 @@ vb2_error_t vb2api_auxfw_sync(struct vb2_context *ctx)
 	/* Check for update severity */
 	VB2_TRY(auxfw_sync_check_update(ctx, &fw_update));
 
-	/* If AUX FW update is slow display the wait screen */
-	if (fw_update == VB_AUX_FW_SLOW_UPDATE) {
-		/* Display should be available, but better check again */
-		if (vb2api_need_reboot_for_display(ctx))
-			return VBERROR_REBOOT_REQUIRED;
-		display_wait_screen(ctx);
-	}
-
 	if (fw_update > VB_AUX_FW_NO_UPDATE) {
 		VB2_TRY(update_auxfw(ctx));
 		/*
diff --git a/firmware/2lib/2ec_sync.c b/firmware/2lib/2ec_sync.c
index e4642164..47a90125 100644
--- a/firmware/2lib/2ec_sync.c
+++ b/firmware/2lib/2ec_sync.c
@@ -21,15 +21,6 @@
 	 VB2_SD_FLAG_ECSYNC_EC_RO : VB2_SD_FLAG_ECSYNC_EC_RW)
 
 /**
- * Display the WAIT screen
- */
-static void display_wait_screen(struct vb2_context *ctx)
-{
-	VB2_DEBUG("EC FW update is slow. Show WAIT screen.\n");
-	VbDisplayScreen(ctx, VB_SCREEN_WAIT, 0, NULL);
-}
-
-/**
  * Set the RECOVERY_REQUEST flag in NV space
  */
 static void request_recovery(struct vb2_context *ctx, uint32_t recovery_request)
@@ -40,6 +31,26 @@ static void request_recovery(struct vb2_context *ctx, uint32_t recovery_request)
 }
 
 /**
+ * Whether a reboot is requested.
+ *
+ * When this function returns 1, rv isn't considered an error and hence
+ * recovery mode shouldn't be triggered.  This includes the following
+ * situations:
+ *
+ * VBERROR_REBOOT_REQUIRED: When we need to display firmware sync screen, but
+ * the display hasn't been initialized, a reboot will be required.
+ *
+ * VBERROR_EC_REBOOT_TO_RO_REQUIRED: The EC may know it needs a reboot.  It may
+ * need to reboot to unprotect the region before updating, or may need to reboot
+ * after updating.  When EC update fails, it also needs to reboot.
+ */
+static inline int reboot_requested(vb2_error_t rv)
+{
+	return rv == VBERROR_REBOOT_REQUIRED ||
+	       rv == VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+}
+
+/**
  * Wrapper around vb2ex_ec_protect() which sets recovery reason on error.
  */
 static vb2_error_t protect_ec(struct vb2_context *ctx,
@@ -47,8 +58,8 @@ static vb2_error_t protect_ec(struct vb2_context *ctx,
 {
 	vb2_error_t rv = vb2ex_ec_protect(select);
 
-	if (rv == VBERROR_EC_REBOOT_TO_RO_REQUIRED) {
-		VB2_DEBUG("vb2ex_ec_protect() needs reboot\n");
+	if (reboot_requested(rv)) {
+		VB2_DEBUG("vb2ex_ec_protect() needs reboot: %#x\n", rv);
 	} else if (rv != VB2_SUCCESS) {
 		VB2_DEBUG("vb2ex_ec_protect() returned %#x\n", rv);
 		request_recovery(ctx, VB2_RECOVERY_EC_PROTECT);
@@ -185,19 +196,8 @@ static vb2_error_t update_ec(struct vb2_context *ctx,
 	rv = vb2ex_ec_update_image(select);
 	if (rv != VB2_SUCCESS) {
 		VB2_DEBUG("vb2ex_ec_update_image() returned %#x\n", rv);
-
-		/*
-		 * The EC may know it needs a reboot.  It may need to
-		 * 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
-		 * mode.
-		 */
-		if (rv != VBERROR_EC_REBOOT_TO_RO_REQUIRED)
+		if (!reboot_requested(rv))
 			request_recovery(ctx, VB2_RECOVERY_EC_UPDATE);
-
 		return rv;
 	}
 
@@ -266,7 +266,10 @@ static vb2_error_t sync_ec(struct vb2_context *ctx)
 
 	/* Update the RW Image */
 	if (sd->flags & SYNC_FLAG(select_rw)) {
-		if (VB2_SUCCESS != update_ec(ctx, select_rw))
+		rv = update_ec(ctx, select_rw);
+		if (reboot_requested(rv))
+			return rv;
+		else if (rv)
 			return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 		/* Updated successfully. Cold reboot to switch to the new RW. */
 		if (ctx->flags & VB2_CONTEXT_NO_BOOT) {
@@ -328,9 +331,11 @@ static vb2_error_t sync_ec(struct vb2_context *ctx)
 		/* Update the RO Image. */
 		int num_tries;
 		for (num_tries = 0; num_tries < RO_RETRIES; num_tries++) {
-			if (VB2_SUCCESS ==
-			    update_ec(ctx, VB_SELECT_FIRMWARE_READONLY))
+			rv = update_ec(ctx, VB_SELECT_FIRMWARE_READONLY);
+			if (rv == VB2_SUCCESS)
 				break;
+			if (reboot_requested(rv))
+				return rv;
 		}
 		if (num_tries == RO_RETRIES) {
 			/* Ran out of tries */
@@ -417,25 +422,6 @@ static vb2_error_t ec_sync_phase1(struct vb2_context *ctx)
 }
 
 /**
- * Returns non-zero if the EC will perform a slow update.
- *
- * This is only valid after calling ec_sync_phase1(), before calling
- * sync_ec().
- *
- * @param ctx		Vboot2 context
- * @return non-zero if a slow update will be done; zero if no update or a
- * fast update.
- */
-static int ec_will_update_slowly(struct vb2_context *ctx)
-{
-	struct vb2_shared_data *sd = vb2_get_sd(ctx);
-
-	return (((sd->flags & SYNC_FLAG(VB_SELECT_FIRMWARE_READONLY)) ||
-		 (sd->flags & SYNC_FLAG(VB_SELECT_FIRMWARE_EC_ACTIVE)))
-			&& EC_SLOW_UPDATE);
-}
-
-/**
  * determine if we can update the EC
  *
  * @param ctx		Vboot2 context
@@ -462,9 +448,6 @@ static int ec_sync_allowed(struct vb2_context *ctx)
  * 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
  * @return VB2_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
@@ -502,17 +485,7 @@ vb2_error_t vb2api_ec_sync(struct vb2_context *ctx)
 	}
 
 	/* Phase 1; this determines if we need an update */
-	vb2_error_t phase1_rv = ec_sync_phase1(ctx);
-	int need_wait_screen = ec_will_update_slowly(ctx);
-
-	if (need_wait_screen && vb2api_need_reboot_for_display(ctx))
-		return VBERROR_REBOOT_REQUIRED;
-
-	if (phase1_rv)
-		return phase1_rv;
-
-	if (need_wait_screen)
-		display_wait_screen(ctx);
+	VB2_TRY(ec_sync_phase1(ctx));
 
 	/* Phase 2; Applies update and/or jumps to the correct EC image */
 	VB2_TRY(ec_sync_phase2(ctx));
diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c
index 5411ad1e..2df4f2e1 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -453,6 +453,11 @@ uint32_t vb2api_get_recovery_reason(struct vb2_context *ctx)
 	return vb2_get_sd(ctx)->recovery_reason;
 }
 
+uint32_t vb2api_get_locale_id(struct vb2_context *ctx)
+{
+	return vb2_nv_get(ctx, VB2_NV_LOCALIZATION_INDEX);
+}
+
 void vb2api_export_vbsd(struct vb2_context *ctx, void *dest)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h
index 290c7ab1..969be2fa 100644
--- a/firmware/2lib/include/2api.h
+++ b/firmware/2lib/include/2api.h
@@ -834,6 +834,14 @@ int vb2api_need_reboot_for_display(struct vb2_context *ctx);
  */
 uint32_t vb2api_get_recovery_reason(struct vb2_context *ctx);
 
+/**
+ * Get the current locale id from nvdata.
+ *
+ * @param ctx		Vboot context
+ * @return Current locale id.
+ */
+uint32_t vb2api_get_locale_id(struct vb2_context *ctx);
+
 /*****************************************************************************/
 /* APIs provided by the caller to verified boot */
 
diff --git a/tests/vb2_auxfw_sync_tests.c b/tests/vb2_auxfw_sync_tests.c
index 622b3030..823dec0b 100644
--- a/tests/vb2_auxfw_sync_tests.c
+++ b/tests/vb2_auxfw_sync_tests.c
@@ -28,13 +28,11 @@ static uint8_t workbuf[VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE]
 static struct vb2_shared_data *sd;
 static struct vb2_gbb_header gbb;
 
-static uint32_t screens_displayed[8];
-static uint32_t screens_count = 0;
-
 static vb2_error_t auxfw_retval;
 static int auxfw_update_req;
 static enum vb2_auxfw_update_severity auxfw_mock_severity;
 static enum vb2_auxfw_update_severity auxfw_update_severity;
+static int auxfw_mock_display_available;
 static int auxfw_protected;
 static vb2_error_t auxfw_done_retval;
 
@@ -48,16 +46,13 @@ static void ResetMocks(void)
 	vb2_nv_init(ctx);
 
 	sd = vb2_get_sd(ctx);
-	sd->flags |= VB2_SD_FLAG_DISPLAY_AVAILABLE;
 
 	memset(&gbb, 0, sizeof(gbb));
 
-	memset(screens_displayed, 0, sizeof(screens_displayed));
-	screens_count = 0;
-
 	auxfw_retval = VB2_SUCCESS;
 	auxfw_mock_severity = VB_AUX_FW_NO_UPDATE;
 	auxfw_update_severity = VB_AUX_FW_NO_UPDATE;
+	auxfw_mock_display_available = 1;
 	auxfw_update_req = 0;
 	auxfw_protected = 0;
 	auxfw_done_retval = VB2_SUCCESS;
@@ -69,19 +64,13 @@ struct vb2_gbb_header *vb2_get_gbb(struct vb2_context *c)
 	return &gbb;
 }
 
-vb2_error_t VbDisplayScreen(struct vb2_context *c, uint32_t screen, int force,
-			    const VbScreenData *data)
-{
-	if (screens_count < ARRAY_SIZE(screens_displayed))
-		screens_displayed[screens_count++] = screen;
-
-	return VB2_SUCCESS;
-}
-
 vb2_error_t vb2ex_auxfw_check(enum vb2_auxfw_update_severity *severity)
 {
 	*severity = auxfw_mock_severity;
 	auxfw_update_severity = auxfw_mock_severity;
+	if (*severity == VB_AUX_FW_SLOW_UPDATE)
+		if (!auxfw_mock_display_available)
+			return VBERROR_REBOOT_REQUIRED;
 	return VB2_SUCCESS;
 }
 
@@ -133,8 +122,6 @@ static void VbSoftwareSyncTest(void)
 	auxfw_mock_severity = VB_AUX_FW_NO_DEVICE;
 	test_auxsync(VB2_SUCCESS, 0,
 		     "No auxiliary FW update needed");
-	TEST_EQ(screens_count, 0,
-		"  wait screen skipped");
 	TEST_EQ(auxfw_update_req, 0, "  no aux fw update requested");
 	TEST_EQ(auxfw_protected, 0, "  no aux fw protected");
 
@@ -142,8 +129,6 @@ static void VbSoftwareSyncTest(void)
 	auxfw_mock_severity = VB_AUX_FW_NO_UPDATE;
 	test_auxsync(VB2_SUCCESS, 0,
 		"No auxiliary FW update needed");
-	TEST_EQ(screens_count, 0,
-		"  wait screen skipped");
 	TEST_EQ(auxfw_update_req, 0, "  no aux fw update requested");
 	TEST_EQ(auxfw_protected, 1, "  aux fw protected");
 
@@ -151,14 +136,12 @@ static void VbSoftwareSyncTest(void)
 	auxfw_mock_severity = VB_AUX_FW_FAST_UPDATE;
 	test_auxsync(VBERROR_EC_REBOOT_TO_RO_REQUIRED, 0,
 		     "Fast auxiliary FW update needed");
-	TEST_EQ(screens_count, 0,
-		"  wait screen skipped");
 	TEST_EQ(auxfw_update_req, 1, "  aux fw update requested");
 	TEST_EQ(auxfw_protected, 0, "  aux fw protected");
 
 	ResetMocks();
 	auxfw_mock_severity = VB_AUX_FW_SLOW_UPDATE;
-	sd->flags &= ~VB2_SD_FLAG_DISPLAY_AVAILABLE;
+	auxfw_mock_display_available = 0;
 	test_auxsync(VBERROR_REBOOT_REQUIRED, 0,
 		     "Slow auxiliary FW update needed - reboot for display");
 
@@ -168,8 +151,6 @@ static void VbSoftwareSyncTest(void)
 		     "Slow auxiliary FW update needed");
 	TEST_EQ(auxfw_update_req, 1, "  aux fw update requested");
 	TEST_EQ(auxfw_protected, 0, "  aux fw protected");
-	TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT,
-		"  wait screen forced");
 
 	ResetMocks();
 	auxfw_mock_severity = VB_AUX_FW_FAST_UPDATE;
diff --git a/tests/vb2_ec_sync_tests.c b/tests/vb2_ec_sync_tests.c
index 80b48908..ad747e09 100644
--- a/tests/vb2_ec_sync_tests.c
+++ b/tests/vb2_ec_sync_tests.c
@@ -34,9 +34,7 @@ static int shutdown_request_calls_left;
 static vb2_error_t ec_vboot_done_retval;
 static int ec_vboot_done_calls;
 
-static uint32_t screens_displayed[8];
-static uint32_t screens_count = 0;
-
+static int mock_display_available;
 static uint8_t mock_ec_ro_hash[32];
 static uint8_t mock_ec_rw_hash[32];
 static uint8_t hmir[32];
@@ -61,10 +59,11 @@ static void ResetMocks(void)
 	vb2_nv_init(ctx);
 
 	sd = vb2_get_sd(ctx);
-	sd->flags |= VB2_SD_FLAG_DISPLAY_AVAILABLE;
 
 	memset(&gbb, 0, sizeof(gbb));
 
+	mock_display_available = 1;
+
 	ec_ro_updated = 0;
 	ec_rw_updated = 0;
 	ec_ro_protected = 0;
@@ -94,9 +93,6 @@ static void ResetMocks(void)
 
 	update_hash = 42;
 
-	memset(screens_displayed, 0, sizeof(screens_displayed));
-	screens_count = 0;
-
 	vb2api_secdata_kernel_create(ctx);
 	vb2_secdata_kernel_init(ctx);
 
@@ -182,6 +178,9 @@ vb2_error_t vb2ex_ec_update_image(enum vb2_firmware_selection select)
 	if (update_retval)
 		return update_retval;
 
+	if (!mock_display_available)
+		return VBERROR_REBOOT_REQUIRED;
+
 	if (select == VB_SELECT_FIRMWARE_READONLY) {
 		ec_ro_updated = 1;
 		mock_ec_ro_hash[0] = update_hash;
@@ -192,15 +191,6 @@ vb2_error_t vb2ex_ec_update_image(enum vb2_firmware_selection select)
 	return VB2_SUCCESS;
 }
 
-vb2_error_t VbDisplayScreen(struct vb2_context *c, uint32_t screen, int force,
-			    const VbScreenData *data)
-{
-	if (screens_count < ARRAY_SIZE(screens_displayed))
-		screens_displayed[screens_count++] = screen;
-
-	return VB2_SUCCESS;
-}
-
 vb2_error_t vb2ex_ec_vboot_done(struct vb2_context *c)
 {
 	ec_vboot_done_calls++;
@@ -494,58 +484,30 @@ static void VbSoftwareSyncTest(void)
 	TEST_EQ(ec_rw_protected, 0, "  ec rw protected");
 	TEST_EQ(ec_run_image, 0, "  ec run image");
 
-	/* Tests related to slow update wait screen */
-	if (EC_SLOW_UPDATE) {
-		ResetMocks();
-		mock_ec_rw_hash[0]++;
-		test_ssync(0, 0, "Slow update");
-		TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
-		TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
-		TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
-		TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
-		TEST_EQ(ec_run_image, 1, "  ec run image");
-		TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT, "  wait screen");
-
-		ResetMocks();
-		mock_ec_rw_hash[0]++;
-		sd->flags &= ~VB2_SD_FLAG_DISPLAY_AVAILABLE;
-		test_ssync(VBERROR_REBOOT_REQUIRED, 0,
-			   "Slow update - reboot for display");
-		TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
-		TEST_EQ(ec_rw_updated, 0, "  ec rw updated");
-		TEST_EQ(ec_ro_protected, 0, "  ec ro protected");
-		TEST_EQ(ec_rw_protected, 0, "  ec rw protected");
-		TEST_EQ(ec_run_image, 0, "  ec run image");
-
-		ResetMocks();
-		mock_ec_rw_hash[0]++;
-		vb2_nv_set(ctx, VB2_NV_DISPLAY_REQUEST, 1);
-		test_ssync(VB2_SUCCESS, 0,
-			   "Slow update with display request");
-		TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
-		TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
-		TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
-		TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
-		TEST_EQ(ec_run_image, 1, "  ec run image");
-		TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT, "  wait screen");
-		TEST_EQ(vb2_nv_get(ctx, VB2_NV_DISPLAY_REQUEST), 1,
-			"  DISPLAY_REQUEST left untouched");
-
-		ResetMocks();
-		mock_ec_rw_hash[0]++;
-		vb2_nv_set(ctx, VB2_NV_DISPLAY_REQUEST, 0);
-		test_ssync(VB2_SUCCESS, 0,
-			   "Slow update without display request "
-			   "(no reboot needed)");
-		TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
-		TEST_EQ(ec_rw_updated, 1, "  ec rw updated");
-		TEST_EQ(ec_ro_protected, 1, "  ec ro protected");
-		TEST_EQ(ec_rw_protected, 1, "  ec rw protected");
-		TEST_EQ(ec_run_image, 1, "  ec run image");
-		TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT, "  wait screen");
-		TEST_EQ(vb2_nv_get(ctx, VB2_NV_DISPLAY_REQUEST), 0,
-			"  DISPLAY_REQUEST left untouched");
-	}
+	/* Display not available - RW */
+	ResetMocks();
+	mock_ec_rw_hash[0]++;
+	mock_display_available = 0;
+	test_ssync(VBERROR_REBOOT_REQUIRED, 0,
+		   "Reboot for display - ec rw");
+	TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
+	TEST_EQ(ec_rw_updated, 0, "  ec rw updated");
+	TEST_EQ(ec_ro_protected, 0, "  ec ro protected");
+	TEST_EQ(ec_rw_protected, 0, "  ec rw protected");
+	TEST_EQ(ec_run_image, 0, "  ec run image");
+
+	/* Display not available - RO */
+	ResetMocks();
+	vb2_nv_set(ctx, VB2_NV_TRY_RO_SYNC, 1);
+	mock_ec_ro_hash[0]++;
+	mock_display_available = 0;
+	test_ssync(VBERROR_REBOOT_REQUIRED, 0,
+		   "Reboot for display - ec ro");
+	TEST_EQ(ec_ro_updated, 0, "  ec ro updated");
+	TEST_EQ(ec_rw_updated, 0, "  ec rw updated");
+	TEST_EQ(ec_ro_protected, 0, "  ec ro protected");
+	TEST_EQ(ec_rw_protected, 0, "  ec rw protected");
+	TEST_EQ(ec_run_image, 1, "  ec run image");
 
 	/* RW cases, no update */
 	ResetMocks();