diff options
author | Yu-Ping Wu <yupingso@chromium.org> | 2020-03-24 11:54:56 +0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-04-17 15:19:16 +0000 |
commit | 785cc5e9a84142fe84d0410e2f56f4aee65fbe65 (patch) | |
tree | 442cee07db8c31820ae90a25c689d90a19f331a7 | |
parent | 297ea05cf12ce31156f8648983431332b50c995c (diff) | |
download | vboot-785cc5e9a84142fe84d0410e2f56f4aee65fbe65.tar.gz |
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>
-rw-r--r-- | Makefile | 7 | ||||
-rw-r--r-- | firmware/2lib/2auxfw_sync.c | 17 | ||||
-rw-r--r-- | firmware/2lib/2ec_sync.c | 91 | ||||
-rw-r--r-- | firmware/2lib/2misc.c | 5 | ||||
-rw-r--r-- | firmware/2lib/include/2api.h | 8 | ||||
-rw-r--r-- | tests/vb2_auxfw_sync_tests.c | 31 | ||||
-rw-r--r-- | tests/vb2_ec_sync_tests.c | 98 |
7 files changed, 81 insertions, 176 deletions
@@ -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(); |