vboot/ui: stop centralizing keyboard input actions
BUG=b:146399181
TEST=make clean && make runtests
BRANCH=none
Change-Id: Ia63537fb13be5f04ae81a6be7e0fac6eaf47cfb7
Signed-off-by: Joel Kitching <kitching@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2210017
Reviewed-by: Hsuan Ting Chen <roccochen@chromium.org>
Reviewed-by: Joel Kitching <kitching@chromium.org>
Tested-by: Joel Kitching <kitching@chromium.org>
Commit-Queue: Yu-Ping Wu <yupingso@chromium.org>
diff --git a/firmware/2lib/2ui.c b/firmware/2lib/2ui.c
index b7a9cad..c0b527b 100644
--- a/firmware/2lib/2ui.c
+++ b/firmware/2lib/2ui.c
@@ -71,75 +71,40 @@
}
/*****************************************************************************/
-/* Menu navigation actions */
-
-/**
- * Context-dependent keyboard shortcut Ctrl+D.
- *
- * - Manual recovery mode: Change to dev mode transition screen.
- * - Developer mode: Boot from internal disk.
- */
-vb2_error_t ctrl_d_action(struct vb2_ui_context *ui)
-{
- if (vb2_allow_recovery(ui->ctx))
- return change_to_dev_screen_action(ui);
- else if (ui->ctx->flags & VB2_CONTEXT_DEVELOPER_MODE)
- return vb2_ui_developer_mode_boot_internal_action(ui);
-
- return VB2_REQUEST_UI_CONTINUE;
-}
-
-vb2_error_t change_to_dev_screen_action(struct vb2_ui_context *ui)
-{
- if (vb2_allow_recovery(ui->ctx))
- return vb2_ui_change_screen(ui, VB2_SCREEN_RECOVERY_TO_DEV);
-
- return VB2_REQUEST_UI_CONTINUE;
-}
-
-/*****************************************************************************/
-/* Action lookup tables */
-
-static struct input_action action_table[] = {
- /* Common navigation (keyboard) */
- { VB_KEY_UP, vb2_ui_menu_prev },
- { VB_KEY_DOWN, vb2_ui_menu_next },
- { VB_KEY_ENTER, vb2_ui_menu_select },
- { VB_KEY_ESC, vb2_ui_change_root },
-
- /* Common navigation (detachable) */
- { VB_BUTTON_VOL_UP_SHORT_PRESS, vb2_ui_menu_prev },
- { VB_BUTTON_VOL_DOWN_SHORT_PRESS, vb2_ui_menu_next },
- { VB_BUTTON_POWER_SHORT_PRESS, vb2_ui_menu_select },
-
- /* Context-specific: developer and recovery */
- { VB_KEY_CTRL('D'), ctrl_d_action },
-
- /* Context-specific: recovery mode */
- { VB_BUTTON_VOL_UP_DOWN_COMBO_PRESS, change_to_dev_screen_action },
- { ' ', vb2_ui_recovery_to_dev_action },
-
- /* Context-specific: developer mode */
- { VB_KEY_CTRL('U'),
- vb2_ui_developer_mode_boot_external_action },
- { VB_BUTTON_VOL_UP_LONG_PRESS,
- vb2_ui_developer_mode_boot_external_action },
- { VB_BUTTON_VOL_DOWN_LONG_PRESS,
- vb2_ui_developer_mode_boot_internal_action },
-};
-
-vb2_error_t (*input_action_lookup(int key))(struct vb2_ui_context *ui)
-{
- int i;
- for (i = 0; i < ARRAY_SIZE(action_table); i++)
- if (action_table[i].key == key)
- return action_table[i].action;
- return NULL;
-}
-
-/*****************************************************************************/
/* Menu navigation functions */
+vb2_error_t menu_navigation_action(struct vb2_ui_context *ui)
+{
+ uint32_t key = ui->key;
+
+ /* Map detachable button presses for simplicity. */
+ if (DETACHABLE) {
+ if (key == VB_BUTTON_VOL_UP_SHORT_PRESS)
+ key = VB_KEY_UP;
+ else if (key == VB_BUTTON_VOL_DOWN_SHORT_PRESS)
+ key = VB_KEY_DOWN;
+ else if (key == VB_BUTTON_POWER_SHORT_PRESS)
+ key = VB_KEY_ENTER;
+ }
+
+ switch (key) {
+ case VB_KEY_UP:
+ return vb2_ui_menu_prev(ui);
+ case VB_KEY_DOWN:
+ return vb2_ui_menu_next(ui);
+ case VB_KEY_ENTER:
+ return vb2_ui_menu_select(ui);
+ case VB_KEY_ESC:
+ return vb2_ui_change_root(ui);
+ default:
+ if (key != 0)
+ VB2_DEBUG("Pressed key %#x, trusted? %d\n",
+ ui->key, ui->key_trusted);
+ }
+
+ return VB2_REQUEST_UI_CONTINUE;
+}
+
vb2_error_t vb2_ui_menu_prev(struct vb2_ui_context *ui)
{
int item;
@@ -243,7 +208,6 @@
struct vb2_ui_context ui;
struct vb2_screen_state prev_state;
uint32_t key_flags;
- vb2_error_t (*action)(struct vb2_ui_context *ui);
vb2_error_t rv;
memset(&ui, 0, sizeof(ui));
@@ -272,6 +236,17 @@
ui.state.disabled_item_mask);
}
+ /* Grab new keyboard input. */
+ ui.key = VbExKeyboardReadWithFlags(&key_flags);
+ ui.key_trusted = !!(key_flags & VB_KEY_FLAG_TRUSTED_KEYBOARD);
+
+ /* Check for shutdown request. */
+ rv = check_shutdown_request(&ui);
+ if (rv != VB2_REQUEST_UI_CONTINUE) {
+ VB2_DEBUG("Shutdown requested!\n");
+ return rv;
+ }
+
/* Run screen action. */
if (ui.state.screen->action) {
rv = ui.state.screen->action(&ui);
@@ -279,30 +254,10 @@
return rv;
}
- /* Grab new keyboard input. */
- ui.key = VbExKeyboardReadWithFlags(&key_flags);
- ui.key_trusted = !!(key_flags & VB_KEY_FLAG_TRUSTED_KEYBOARD);
-
- /* Check for shutdown request. */
- if (check_shutdown_request(&ui) == VB2_REQUEST_SHUTDOWN) {
- VB2_DEBUG("Shutdown requested!\n");
- return VB2_REQUEST_SHUTDOWN;
- }
-
- /* Run input action function if found. */
- action = input_action_lookup(ui.key);
- if (action) {
- rv = action(&ui);
- if (rv != VB2_REQUEST_UI_CONTINUE)
- return rv;
- } else if (ui.key) {
- VB2_DEBUG("Pressed key %#x, trusted? %d\n",
- ui.key, ui.key_trusted);
- }
-
- /* Reset keyboard input. */
- ui.key = 0;
- ui.key_trusted = 0;
+ /* Run menu navigation action. */
+ rv = menu_navigation_action(&ui);
+ if (rv != VB2_REQUEST_UI_CONTINUE)
+ return rv;
/* Run global action function if available. */
if (global_action) {
@@ -323,7 +278,20 @@
vb2_error_t vb2_developer_menu(struct vb2_context *ctx)
{
- return ui_loop(ctx, VB2_SCREEN_DEVELOPER_MODE, NULL);
+ return ui_loop(ctx, VB2_SCREEN_DEVELOPER_MODE, developer_action);
+}
+
+vb2_error_t developer_action(struct vb2_ui_context *ui)
+{
+ /* Developer mode keyboard shortcuts */
+ if (ui->key == VB_KEY_CTRL('U') ||
+ (DETACHABLE && ui->key == VB_BUTTON_VOL_UP_LONG_PRESS))
+ return vb2_ui_developer_mode_boot_external_action(ui);
+ if (ui->key == VB_KEY_CTRL('D') ||
+ (DETACHABLE && ui->key == VB_BUTTON_VOL_DOWN_LONG_PRESS))
+ return vb2_ui_developer_mode_boot_internal_action(ui);
+
+ return VB2_REQUEST_UI_CONTINUE;
}
/*****************************************************************************/
@@ -339,10 +307,10 @@
vb2_error_t vb2_manual_recovery_menu(struct vb2_context *ctx)
{
- return ui_loop(ctx, VB2_SCREEN_RECOVERY_SELECT, try_recovery_action);
+ return ui_loop(ctx, VB2_SCREEN_RECOVERY_SELECT, manual_recovery_action);
}
-vb2_error_t try_recovery_action(struct vb2_ui_context *ui)
+vb2_error_t manual_recovery_action(struct vb2_ui_context *ui)
{
/* See if we have a recovery kernel available yet. */
vb2_error_t rv = VbTryLoadKernel(ui->ctx, VB_DISK_FLAG_REMOVABLE);
@@ -351,6 +319,8 @@
/* If disk validity state changed, switch to appropriate screen. */
if (ui->recovery_rv != rv) {
+ VB2_DEBUG("Recovery VbTryLoadKernel %#x --> %#x\n",
+ ui->recovery_rv, rv);
ui->recovery_rv = rv;
return vb2_ui_change_screen(ui,
rv == VB2_ERROR_LK_NO_DISK_FOUND ?
@@ -358,5 +328,10 @@
VB2_SCREEN_RECOVERY_INVALID);
}
+ /* Manual recovery keyboard shortcuts */
+ if (ui->key == VB_KEY_CTRL('D') ||
+ (DETACHABLE && ui->key == VB_BUTTON_VOL_UP_DOWN_COMBO_PRESS))
+ return vb2_ui_change_screen(ui, VB2_SCREEN_RECOVERY_TO_DEV);
+
return VB2_REQUEST_UI_CONTINUE;
}
diff --git a/firmware/2lib/2ui_screens.c b/firmware/2lib/2ui_screens.c
index 2c8f57b..0c9efe1 100644
--- a/firmware/2lib/2ui_screens.c
+++ b/firmware/2lib/2ui_screens.c
@@ -152,46 +152,13 @@
return VB2_REQUEST_UI_CONTINUE;
}
-vb2_error_t vb2_ui_recovery_to_dev_action(struct vb2_ui_context *ui)
+static vb2_error_t recovery_to_dev_finalize(struct vb2_ui_context *ui)
{
- int pressed;
-
- if (ui->state.screen->id != VB2_SCREEN_RECOVERY_TO_DEV) {
- VB2_DEBUG("Action needs RECOVERY_TO_DEV screen\n");
- return VB2_REQUEST_UI_CONTINUE;
- }
-
- if (ui->key == ' ') {
- VB2_DEBUG("SPACE means cancel dev mode transition\n");
- return vb2_ui_change_root(ui);
- }
-
- if (PHYSICAL_PRESENCE_KEYBOARD) {
- if (ui->key != VB_KEY_ENTER &&
- ui->key != VB_BUTTON_POWER_SHORT_PRESS)
- return VB2_REQUEST_UI_CONTINUE;
- if (!ui->key_trusted) {
- VB2_DEBUG("Reject untrusted %s confirmation\n",
- ui->key == VB_KEY_ENTER ?
- "ENTER" : "POWER");
- return VB2_REQUEST_UI_CONTINUE;
- }
- } else {
- pressed = vb2ex_physical_presence_pressed();
- if (pressed) {
- VB2_DEBUG("Physical presence button pressed, "
- "awaiting release\n");
- ui->physical_presence_button_pressed = 1;
- return VB2_REQUEST_UI_CONTINUE;
- }
- if (!ui->physical_presence_button_pressed)
- return VB2_REQUEST_UI_CONTINUE;
- VB2_DEBUG("Physical presence button released\n");
- }
VB2_DEBUG("Physical presence confirmed!\n");
/* Sanity check, should never happen. */
- if ((vb2_get_sd(ui->ctx)->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) ||
+ if (ui->state.screen->id != VB2_SCREEN_RECOVERY_TO_DEV ||
+ (vb2_get_sd(ui->ctx)->flags & VB2_SD_FLAG_DEV_MODE_ENABLED) ||
!vb2_allow_recovery(ui->ctx)) {
VB2_DEBUG("ERROR: Dev transition sanity check failed\n");
return VB2_REQUEST_UI_CONTINUE;
@@ -202,10 +169,47 @@
return VB2_REQUEST_REBOOT_EC_TO_RO;
}
+vb2_error_t recovery_to_dev_confirm_action(struct vb2_ui_context *ui)
+{
+ if (!ui->key_trusted) {
+ VB2_DEBUG("Reject untrusted %s confirmation\n",
+ ui->key == VB_KEY_ENTER ? "ENTER" : "POWER");
+ return VB2_REQUEST_UI_CONTINUE;
+ }
+ return recovery_to_dev_finalize(ui);
+}
+
+vb2_error_t recovery_to_dev_action(struct vb2_ui_context *ui)
+{
+ int pressed;
+
+ if (ui->key == ' ') {
+ VB2_DEBUG("SPACE means cancel dev mode transition\n");
+ return vb2_ui_change_root(ui);
+ }
+
+ /* Keyboard physical presence case covered by "Confirm" action. */
+ if (PHYSICAL_PRESENCE_KEYBOARD)
+ return VB2_REQUEST_UI_CONTINUE;
+
+ pressed = vb2ex_physical_presence_pressed();
+ if (pressed) {
+ VB2_DEBUG("Physical presence button pressed, "
+ "awaiting release\n");
+ ui->physical_presence_button_pressed = 1;
+ return VB2_REQUEST_UI_CONTINUE;
+ }
+ if (!ui->physical_presence_button_pressed)
+ return VB2_REQUEST_UI_CONTINUE;
+ VB2_DEBUG("Physical presence button released\n");
+
+ return recovery_to_dev_finalize(ui);
+}
+
static const struct vb2_menu_item recovery_to_dev_items[] = {
[RECOVERY_TO_DEV_ITEM_CONFIRM] = {
.text = "Confirm",
- .action = vb2_ui_recovery_to_dev_action,
+ .action = recovery_to_dev_confirm_action,
},
[RECOVERY_TO_DEV_ITEM_CANCEL] = {
.text = "Cancel",
@@ -217,7 +221,7 @@
.id = VB2_SCREEN_RECOVERY_TO_DEV,
.name = "Transition to developer mode",
.init = recovery_to_dev_init,
- .action = vb2_ui_recovery_to_dev_action,
+ .action = recovery_to_dev_action,
MENU_ITEMS(recovery_to_dev_items),
};
diff --git a/firmware/2lib/include/2ui.h b/firmware/2lib/include/2ui.h
index 35716bb..d4d2ba4 100644
--- a/firmware/2lib/include/2ui.h
+++ b/firmware/2lib/include/2ui.h
@@ -75,11 +75,6 @@
int physical_presence_button_pressed;
};
-vb2_error_t vb2_ui_change_screen(struct vb2_ui_context *ui, enum vb2_screen id);
-vb2_error_t vb2_ui_menu_select_action(struct vb2_ui_context *ui);
-vb2_error_t vb2_ui_back_action(struct vb2_ui_context *ui);
-
-vb2_error_t vb2_ui_recovery_to_dev_action(struct vb2_ui_context *ui);
vb2_error_t vb2_ui_developer_mode_boot_internal_action(
struct vb2_ui_context *ui);
vb2_error_t vb2_ui_developer_mode_boot_external_action(
diff --git a/firmware/2lib/include/2ui_private.h b/firmware/2lib/include/2ui_private.h
index 96caf64..c1ecd8b 100644
--- a/firmware/2lib/include/2ui_private.h
+++ b/firmware/2lib/include/2ui_private.h
@@ -10,26 +10,20 @@
#ifndef VBOOT_REFERENCE_2UI_PRIVATE_H_
#define VBOOT_REFERENCE_2UI_PRIVATE_H_
+/* From 2ui.c */
vb2_error_t check_shutdown_request(struct vb2_ui_context *ui);
-
-struct input_action {
- int key;
- vb2_error_t (*action)(struct vb2_ui_context *ui);
-};
-
-vb2_error_t ctrl_d_action(struct vb2_ui_context *ui);
-vb2_error_t change_to_dev_screen_action(struct vb2_ui_context *ui);
-vb2_error_t (*input_action_lookup(int key))(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));
-
-vb2_error_t try_recovery_action(struct vb2_ui_context *ui);
+vb2_error_t developer_action(struct vb2_ui_context *ui);
+vb2_error_t manual_recovery_action(struct vb2_ui_context *ui);
/* From 2ui_screens.c */
vb2_error_t advanced_options_init(struct vb2_ui_context *ui);
vb2_error_t recovery_select_init(struct vb2_ui_context *ui);
vb2_error_t recovery_to_dev_init(struct vb2_ui_context *ui);
+vb2_error_t recovery_to_dev_confirm_action(struct vb2_ui_context *ui);
+vb2_error_t recovery_to_dev_action(struct vb2_ui_context *ui);
vb2_error_t developer_mode_init(struct vb2_ui_context *ui);
vb2_error_t developer_mode_action(struct vb2_ui_context *ui);
vb2_error_t developer_to_norm_action(struct vb2_ui_context *ui);
diff --git a/tests/vb2_ui_action_tests.c b/tests/vb2_ui_action_tests.c
index 3d34d76..ab273da 100644
--- a/tests/vb2_ui_action_tests.c
+++ b/tests/vb2_ui_action_tests.c
@@ -543,36 +543,37 @@
VB2_DEBUG("...done.\n");
}
-static void try_recovery_action_tests(void)
+static void manual_recovery_action_tests(void)
{
- VB2_DEBUG("Testing try recovery action...\n");
+ VB2_DEBUG("Testing manual recovery action...\n");
/* SUCCESS */
reset_common_data();
set_mock_vbtlk(VB2_SUCCESS, VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_SUCCESS,
- "SUCCESS");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_SUCCESS, "SUCCESS");
TEST_EQ(mock_get_screen_info_called, 0, " no change_screen");
/* NO_DISK_FOUND */
reset_common_data();
set_mock_vbtlk(VB2_ERROR_LK_NO_DISK_FOUND, VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_REQUEST_UI_CONTINUE,
- "NO_DISK_FOUND");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_REQUEST_UI_CONTINUE, "NO_DISK_FOUND");
screen_state_eq(mock_state, VB2_SCREEN_RECOVERY_SELECT,
MOCK_IGNORE, MOCK_IGNORE);
/* NO_DISK_FOUND -> INVALID_KERNEL -> SUCCESS */
reset_common_data();
set_mock_vbtlk(VB2_ERROR_LK_NO_DISK_FOUND, VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_REQUEST_UI_CONTINUE,
- "NO_DISK_FOUND");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_REQUEST_UI_CONTINUE, "NO_DISK_FOUND");
set_mock_vbtlk(VB2_ERROR_LK_INVALID_KERNEL_FOUND,
VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_REQUEST_UI_CONTINUE,
- "INVALID_KERNEL");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_REQUEST_UI_CONTINUE, "INVALID_KERNEL");
set_mock_vbtlk(VB2_SUCCESS, VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_SUCCESS, "SUCCESS");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_SUCCESS, "SUCCESS");
screen_state_eq(mock_state, VB2_SCREEN_RECOVERY_INVALID,
MOCK_IGNORE, MOCK_IGNORE);
@@ -580,8 +581,8 @@
reset_common_data();
set_mock_vbtlk(VB2_ERROR_LK_INVALID_KERNEL_FOUND,
VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_REQUEST_UI_CONTINUE,
- "INVALID_KERNEL");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_REQUEST_UI_CONTINUE, "INVALID_KERNEL");
screen_state_eq(mock_state, VB2_SCREEN_RECOVERY_INVALID,
MOCK_IGNORE, MOCK_IGNORE);
@@ -589,13 +590,14 @@
reset_common_data();
set_mock_vbtlk(VB2_ERROR_LK_INVALID_KERNEL_FOUND,
VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_REQUEST_UI_CONTINUE,
- "INVALID_KERNEL");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_REQUEST_UI_CONTINUE, "INVALID_KERNEL");
set_mock_vbtlk(VB2_ERROR_LK_NO_DISK_FOUND, VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_REQUEST_UI_CONTINUE,
- "NO_DISK_FOUND");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_REQUEST_UI_CONTINUE, "NO_DISK_FOUND");
set_mock_vbtlk(VB2_SUCCESS, VB_DISK_FLAG_REMOVABLE);
- TEST_EQ(try_recovery_action(&mock_ui_context), VB2_SUCCESS, "SUCCESS");
+ TEST_EQ(manual_recovery_action(&mock_ui_context),
+ VB2_SUCCESS, "SUCCESS");
screen_state_eq(mock_state, VB2_SCREEN_RECOVERY_SELECT,
MOCK_IGNORE, MOCK_IGNORE);
@@ -712,7 +714,7 @@
menu_select_tests();
/* Global actions */
- try_recovery_action_tests();
+ manual_recovery_action_tests();
/* Core UI loop */
ui_loop_tests();
