From 65d69f12712d262724edf1337c8055adc9edaa9d Mon Sep 17 00:00:00 2001 From: Shelley Chen Date: Thu, 11 Jun 2020 22:41:14 -0700 Subject: firmware/2lib: Add visual/audio error handling Adding an enum parameter to vb2ex_display_ui to facilitate printing errors to the screen. Currently, errors are only printed to the serial console. Also adding in beep if an error is displayed. BUG=b:144969091,b:158635317,b:158639298,b:146399181 BRANCH=None TEST=Boot into dev warning screen and try to hit ctrl-u when no USB is plugged in. Ensure error beep occurs. Ensure in dev mode. Boot into recovery and press ctrl-d. Ensure that error message is printed to the screen and beep occurs. make runtests Cq-Depend: chromium:2243513 Change-Id: I548d624532ad8816497c37a726275b33171e28dc Signed-off-by: Shelley Chen Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2243196 Tested-by: Shelley Chen Reviewed-by: Yu-Ping Wu Commit-Queue: Shelley Chen --- firmware/2lib/2ui.c | 41 +++++++++++++++++++++++++++++++++---- firmware/2lib/2ui_screens.c | 12 ++++++++--- firmware/2lib/include/2api.h | 19 +++++++++++++++-- firmware/2lib/include/2ui.h | 3 +++ firmware/2lib/include/2ui_private.h | 1 + tests/vb2_ui_action_tests.c | 3 ++- tests/vb2_ui_tests.c | 3 ++- 7 files changed, 71 insertions(+), 11 deletions(-) diff --git a/firmware/2lib/2ui.c b/firmware/2lib/2ui.c index 96a7b939..4d363f68 100644 --- a/firmware/2lib/2ui.c +++ b/firmware/2lib/2ui.c @@ -71,6 +71,20 @@ vb2_error_t check_shutdown_request(struct vb2_ui_context *ui) return VB2_REQUEST_UI_CONTINUE; } +/*****************************************************************************/ +/* Error action functions */ + +vb2_error_t error_exit_action(struct vb2_ui_context *ui) +{ + /* + * If the only difference is the error message, then just + * redraw the screen without the error string. + */ + if (ui->key && ui->error_code != VB2_UI_ERROR_NONE) + ui->error_code = VB2_UI_ERROR_NONE; + return VB2_REQUEST_UI_CONTINUE; +} + /*****************************************************************************/ /* Menu navigation functions */ @@ -236,6 +250,7 @@ vb2_error_t ui_loop(struct vb2_context *ctx, enum vb2_screen root_screen_id, { struct vb2_ui_context ui; struct vb2_screen_state prev_state; + enum vb2_ui_error prev_error_code; const struct vb2_menu *menu; uint32_t key_flags; vb2_error_t rv; @@ -250,11 +265,13 @@ vb2_error_t ui_loop(struct vb2_context *ctx, enum vb2_screen root_screen_id, if (rv != VB2_REQUEST_UI_CONTINUE) return rv; memset(&prev_state, 0, sizeof(prev_state)); + prev_error_code = VB2_UI_ERROR_NONE; while (1) { /* Draw if there are state changes. */ - if (memcmp(&prev_state, &ui.state, sizeof(ui.state))) { - memcpy(&prev_state, &ui.state, sizeof(ui.state)); + if (memcmp(&prev_state, &ui.state, sizeof(ui.state)) || + /* we want to redraw/beep on a transition */ + prev_error_code != ui.error_code) { menu = get_menu(&ui); VB2_DEBUG("<%s> menu item <%s>\n", @@ -262,10 +279,21 @@ vb2_error_t ui_loop(struct vb2_context *ctx, enum vb2_screen root_screen_id, menu->num_items ? menu->items[ui.state.selected_item].text : "null"); - vb2ex_display_ui(ui.state.screen->id, ui.locale_id, ui.state.selected_item, - ui.state.disabled_item_mask); + ui.state.disabled_item_mask, + ui.error_code); + /* + * Only beep if we're transitioning from no + * error to an error. + */ + if (prev_error_code == VB2_UI_ERROR_NONE && + ui.error_code != VB2_UI_ERROR_NONE) + vb2ex_beep(250, 400); + + /* Update prev variables. */ + memcpy(&prev_state, &ui.state, sizeof(ui.state)); + prev_error_code = ui.error_code; } /* Grab new keyboard input. */ @@ -279,6 +307,11 @@ vb2_error_t ui_loop(struct vb2_context *ctx, enum vb2_screen root_screen_id, return rv; } + /* Check if we need to exit an error box. */ + rv = error_exit_action(&ui); + if (rv != VB2_REQUEST_UI_CONTINUE) + return rv; + /* Run screen action. */ if (ui.state.screen->action) { rv = ui.state.screen->action(&ui); diff --git a/firmware/2lib/2ui_screens.c b/firmware/2lib/2ui_screens.c index 042d6d4c..c43abb8f 100644 --- a/firmware/2lib/2ui_screens.c +++ b/firmware/2lib/2ui_screens.c @@ -249,7 +249,8 @@ static const struct vb2_screen_info recovery_invalid_screen = { vb2_error_t recovery_to_dev_init(struct vb2_ui_context *ui) { if (vb2_get_sd(ui->ctx)->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) { - VB2_DEBUG("Dev mode already enabled?\n"); + /* We're in dev mode, so let user know they can't transition */ + ui->error_code = VB2_UI_ERROR_DEV_MODE_ALREADY_ENABLED; return vb2_ui_change_root(ui); } @@ -489,6 +490,7 @@ vb2_error_t vb2_ui_developer_mode_boot_external_action( !vb2_dev_boot_allowed(ui->ctx) || !vb2_dev_boot_external_allowed(ui->ctx)) { VB2_DEBUG("ERROR: Dev mode external boot not allowed\n"); + ui->error_code = VB2_UI_ERROR_DEV_EXTERNAL_NOT_ALLOWED; return VB2_REQUEST_UI_CONTINUE; } @@ -496,13 +498,17 @@ vb2_error_t vb2_ui_developer_mode_boot_external_action( if (rv == VB2_SUCCESS) { return VB2_SUCCESS; } else if (rv == VB2_ERROR_LK_NO_DISK_FOUND) { - if (ui->state.screen->id != VB2_SCREEN_DEVELOPER_BOOT_EXTERNAL) + if (ui->state.screen->id != VB2_SCREEN_DEVELOPER_BOOT_EXTERNAL) { VB2_DEBUG("No external disk found\n"); + ui->error_code = VB2_UI_ERROR_DEV_EXTERNAL_BOOT_FAILED; + } return vb2_ui_change_screen( ui, VB2_SCREEN_DEVELOPER_BOOT_EXTERNAL); } else { - if (ui->state.screen->id != VB2_SCREEN_DEVELOPER_INVALID_DISK) + if (ui->state.screen->id != VB2_SCREEN_DEVELOPER_INVALID_DISK) { VB2_DEBUG("Invalid external disk: %#x\n", rv); + ui->error_code = VB2_UI_ERROR_DEV_EXTERNAL_BOOT_FAILED; + } return vb2_ui_change_screen( ui, VB2_SCREEN_DEVELOPER_INVALID_DISK); } diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h index 9131dea0..69fd1458 100644 --- a/firmware/2lib/include/2api.h +++ b/firmware/2lib/include/2api.h @@ -1234,21 +1234,36 @@ enum vb2_screen { VB2_SCREEN_DEVELOPER_INVALID_DISK = 0x330, }; +enum vb2_ui_error { + /* No error */ + VB2_UI_ERROR_NONE = 0, + /* Dev mode already enabled */ + VB2_UI_ERROR_DEV_MODE_ALREADY_ENABLED, + /* Dev mode internal boot not allowed */ + VB2_UI_ERROR_DEV_INTERNAL_NOT_ALLOWED, + /* Dev mode external boot not allowed */ + VB2_UI_ERROR_DEV_EXTERNAL_NOT_ALLOWED, + /* Dev mode external boot failed */ + VB2_UI_ERROR_DEV_EXTERNAL_BOOT_FAILED, +}; + /** * Display UI screen. * * @param screen Screen to display. + * @param locale_id Id of current locale. * @param selected_item Index of the selected menu item. If the screen * doesn't have a menu, this value will be ignored. * @param disabled_item_mask Mask for disabled menu items. Bit (1 << idx) * indicates whether item 'idx' is disabled. - * @param locale_id Id of current locale. + * @param error_code Error code if an error occurred. * @return VB2_SUCCESS, or error code on error. */ vb2_error_t vb2ex_display_ui(enum vb2_screen screen, uint32_t locale_id, uint32_t selected_item, - uint32_t disabled_item_mask); + uint32_t disabled_item_mask, + enum vb2_ui_error error_code); /** * Check that physical presence button is currently pressed by the user. diff --git a/firmware/2lib/include/2ui.h b/firmware/2lib/include/2ui.h index 49f51b32..652362dc 100644 --- a/firmware/2lib/include/2ui.h +++ b/firmware/2lib/include/2ui.h @@ -88,6 +88,9 @@ struct vb2_ui_context { /* For language selection screen. */ struct vb2_menu language_menu; + + /* For displaying error messages. */ + enum vb2_ui_error error_code; }; vb2_error_t vb2_ui_developer_mode_boot_internal_action( diff --git a/firmware/2lib/include/2ui_private.h b/firmware/2lib/include/2ui_private.h index effa280d..ab353955 100644 --- a/firmware/2lib/include/2ui_private.h +++ b/firmware/2lib/include/2ui_private.h @@ -13,6 +13,7 @@ /* From 2ui.c */ vb2_error_t check_shutdown_request(struct vb2_ui_context *ui); const struct vb2_menu *get_menu(struct vb2_ui_context *ui); +vb2_error_t error_exit_action(struct vb2_ui_context *ui); vb2_error_t menu_navigation_action(struct vb2_ui_context *ui); vb2_error_t ui_loop(struct vb2_context *ctx, enum vb2_screen root_screen_id, vb2_error_t (*global_action)(struct vb2_ui_context *ui)); diff --git a/tests/vb2_ui_action_tests.c b/tests/vb2_ui_action_tests.c index 13c283a8..a664c191 100644 --- a/tests/vb2_ui_action_tests.c +++ b/tests/vb2_ui_action_tests.c @@ -383,7 +383,8 @@ const struct vb2_screen_info *vb2_get_screen_info(enum vb2_screen screen) vb2_error_t vb2ex_display_ui(enum vb2_screen screen, uint32_t locale_id, uint32_t selected_item, - uint32_t disabled_item_mask) + uint32_t disabled_item_mask, + enum vb2_ui_error error_code) { struct display_call displayed = (struct display_call){ .screen = vb2_get_screen_info(screen), diff --git a/tests/vb2_ui_tests.c b/tests/vb2_ui_tests.c index 407a081f..a56e0a4a 100644 --- a/tests/vb2_ui_tests.c +++ b/tests/vb2_ui_tests.c @@ -325,7 +325,8 @@ struct vb2_gbb_header *vb2_get_gbb(struct vb2_context *c) vb2_error_t vb2ex_display_ui(enum vb2_screen screen, uint32_t locale_id, uint32_t selected_item, - uint32_t disabled_item_mask) + uint32_t disabled_item_mask, + enum vb2_ui_error error_code) { struct display_call displayed = (struct display_call){ .screen = vb2_get_screen_info(screen), -- cgit v1.2.1