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>
diff --git a/Makefile b/Makefile
index 2b8655c..913f4ac 100644
--- a/Makefile
+++ b/Makefile
@@ -173,13 +173,6 @@
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 d5182b3..1e17358 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 @@
/* 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 e464216..47a9012 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 @@
}
/**
+ * 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 @@
{
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 @@
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 @@
/* 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 @@
/* 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 @@
}
/**
- * 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 @@
* 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 @@
}
/* 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 5411ad1..2df4f2e 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -453,6 +453,11 @@
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 290c7ab..969be2f 100644
--- a/firmware/2lib/include/2api.h
+++ b/firmware/2lib/include/2api.h
@@ -834,6 +834,14 @@
*/
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 622b303..823dec0 100644
--- a/tests/vb2_auxfw_sync_tests.c
+++ b/tests/vb2_auxfw_sync_tests.c
@@ -28,13 +28,11 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
"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 80b4890..ad747e0 100644
--- a/tests/vb2_ec_sync_tests.c
+++ b/tests/vb2_ec_sync_tests.c
@@ -34,9 +34,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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");
+ /* 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");
- 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 - 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();
