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>

5 files changed, 127 insertions, 157 deletions

diff --git a/firmware/2lib/2ui.c b/firmware/2lib/2ui.c
index b7a9cad7..c0b527b6 100644
--- a/firmware/2lib/2ui.c
+++ b/firmware/2lib/2ui.c
@@ -71,75 +71,40 @@ vb2_error_t check_shutdown_request(struct vb2_ui_context *ui)
 }
 
 /*****************************************************************************/
-/* Menu navigation actions */
+/* Menu navigation functions */
 
-/**
- * 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)
+vb2_error_t menu_navigation_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;
-}
+	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;
+	}
 
-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);
+	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;
 }
 
-/*****************************************************************************/
-/* 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 vb2_ui_menu_prev(struct vb2_ui_context *ui)
 {
 	int item;
@@ -243,7 +208,6 @@ 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;
 	uint32_t key_flags;
-	vb2_error_t (*action)(struct vb2_ui_context *ui);
 	vb2_error_t rv;
 
 	memset(&ui, 0, sizeof(ui));
@@ -272,37 +236,28 @@ vb2_error_t ui_loop(struct vb2_context *ctx, enum vb2_screen root_screen_id,
 					 ui.state.disabled_item_mask);
 		}
 
-		/* Run screen action. */
-		if (ui.state.screen->action) {
-			rv = ui.state.screen->action(&ui);
-			if (rv != VB2_REQUEST_UI_CONTINUE)
-				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) {
+		rv = check_shutdown_request(&ui);
+		if (rv != VB2_REQUEST_UI_CONTINUE) {
 			VB2_DEBUG("Shutdown requested!\n");
-			return VB2_REQUEST_SHUTDOWN;
+			return rv;
 		}
 
-		/* Run input action function if found. */
-		action = input_action_lookup(ui.key);
-		if (action) {
-			rv = action(&ui);
+		/* Run screen action. */
+		if (ui.state.screen->action) {
+			rv = ui.state.screen->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 ui_loop(struct vb2_context *ctx, enum vb2_screen root_screen_id,
 
 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_broken_recovery_menu(struct vb2_context *ctx)
 
 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 @@ vb2_error_t try_recovery_action(struct vb2_ui_context *ui)
 
 	/* 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_error_t try_recovery_action(struct vb2_ui_context *ui)
 					    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 2c8f57b7..0c9efe1b 100644
--- a/firmware/2lib/2ui_screens.c
+++ b/firmware/2lib/2ui_screens.c
@@ -152,60 +152,64 @@ vb2_error_t recovery_to_dev_init(struct vb2_ui_context *ui)
 	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;
+	VB2_DEBUG("Physical presence confirmed!\n");
+
+	/* Sanity check, should never happen. */
+	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;
+	}
 
-	if (ui->state.screen->id != VB2_SCREEN_RECOVERY_TO_DEV) {
-		VB2_DEBUG("Action needs RECOVERY_TO_DEV screen\n");
+	VB2_DEBUG("Enabling dev mode and rebooting...\n");
+	vb2_enable_developer_mode(ui->ctx);
+	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);
 	}
 
-	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");
+	/* Keyboard physical presence case covered by "Confirm" action. */
+	if (PHYSICAL_PRESENCE_KEYBOARD)
+		return VB2_REQUEST_UI_CONTINUE;
 
-	/* Sanity check, should never happen. */
-	if ((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");
+	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("Enabling dev mode and rebooting...\n");
-	vb2_enable_developer_mode(ui->ctx);
-	return VB2_REQUEST_REBOOT_EC_TO_RO;
+	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 @@ static const struct vb2_screen_info recovery_to_dev_screen = {
 	.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 35716bb1..d4d2ba4b 100644
--- a/firmware/2lib/include/2ui.h
+++ b/firmware/2lib/include/2ui.h
@@ -75,11 +75,6 @@ struct vb2_ui_context {
 	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 96caf649..c1ecd8be 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 3d34d768..ab273da0 100644
--- a/tests/vb2_ui_action_tests.c
+++ b/tests/vb2_ui_action_tests.c
@@ -543,36 +543,37 @@ static void menu_select_tests(void)
 	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 @@ static void try_recovery_action_tests(void)
 	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 @@ static void try_recovery_action_tests(void)
 	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 @@ int main(void)
 	menu_select_tests();
 
 	/* Global actions */
-	try_recovery_action_tests();
+	manual_recovery_action_tests();
 
 	/* Core UI loop */
 	ui_loop_tests();