From 4b31e1a9b02cb0eaca01491888cf40b8f7023545 Mon Sep 17 00:00:00 2001 From: Hsuan Ting Chen Date: Fri, 17 Sep 2021 22:09:26 +0800 Subject: 2api: Adjust the order of checking dev switch in vb2api_fw_phase1 This CL is a revert to CL:300621. In that CL, we adjusted the order of checking dev switch (vb2_check_dev_switch) and move it after the check for recovery step (vb2_check_recovery). The reason of that adjustment was because vb2_check_dev_switch might return very early if the TPM is broken and it failed to retrieve the secdata. However, retrieving the secdata becomes a function which cannot fail nowadays and it is not necessary to handle that scenario of the early return in vb2_check_dev_switch. So we adjust them back to the original order. We also remove the related outdated unit tests in vb2_api_tests.c BUG=none BRANCH=none TEST=CC=x86_64-pc-linux-gnu-clang; make clean && make runtests Signed-off-by: Hsuan Ting Chen Change-Id: Iada8092e73ed9282242b1d67d101c836860e13c6 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3168437 Reviewed-by: Yu-Ping Wu Reviewed-by: Julius Werner --- firmware/2lib/2api.c | 19 +++++-------------- tests/vb2_api_tests.c | 14 ++++---------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/firmware/2lib/2api.c b/firmware/2lib/2api.c index aa1d25da..593a12cf 100644 --- a/firmware/2lib/2api.c +++ b/firmware/2lib/2api.c @@ -59,6 +59,11 @@ vb2_error_t vb2api_fw_phase1(struct vb2_context *ctx) if (rv) vb2api_fail(ctx, VB2_RECOVERY_GBB_HEADER, rv); + /* Check for dev switch */ + rv = vb2_check_dev_switch(ctx); + if (rv) + vb2api_fail(ctx, VB2_RECOVERY_DEV_SWITCH, rv); + /* * Check for recovery. Note that this function returns void, since any * errors result in requesting recovery. That's also why we don't @@ -67,20 +72,6 @@ vb2_error_t vb2api_fw_phase1(struct vb2_context *ctx) */ vb2_check_recovery(ctx); - /* Check for dev switch */ - rv = vb2_check_dev_switch(ctx); - if (rv && !(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { - /* - * Error in dev switch processing, and we weren't already - * headed for recovery mode. Reboot into recovery mode, since - * it's too late to handle those errors this boot, and we need - * to take a different path through the dev switch checking - * code in that case. - */ - vb2api_fail(ctx, VB2_RECOVERY_DEV_SWITCH, rv); - return rv; - } - /* * Check for possible reasons to ask the firmware to make display * available. VB2_CONTEXT_RECOVERY_MODE may have been set above by diff --git a/tests/vb2_api_tests.c b/tests/vb2_api_tests.c index 40fdf593..beab239d 100644 --- a/tests/vb2_api_tests.c +++ b/tests/vb2_api_tests.c @@ -384,20 +384,14 @@ static void phase1_tests(void) TEST_NEQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE, 0, " recovery flag"); TEST_NEQ(ctx->flags & VB2_CONTEXT_CLEAR_RAM, 0, " clear ram flag"); - /* Dev switch error in normal mode reboots to recovery */ + /* Dev switch error proceeds to a recovery boot */ reset_common_data(FOR_MISC); retval_vb2_check_dev_switch = VB2_ERROR_MOCK; - TEST_EQ(vb2api_fw_phase1(ctx), VB2_ERROR_MOCK, "phase1 dev switch"); + TEST_EQ(vb2api_fw_phase1(ctx), VB2_ERROR_API_PHASE1_RECOVERY, + "phase1 dev switch error proceeds to recovery"); TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST), VB2_RECOVERY_DEV_SWITCH, " recovery request"); - - /* Dev switch error already in recovery mode just proceeds */ - reset_common_data(FOR_MISC); - vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, VB2_RECOVERY_RO_UNSPECIFIED); - retval_vb2_check_dev_switch = VB2_ERROR_MOCK; - TEST_EQ(vb2api_fw_phase1(ctx), VB2_ERROR_API_PHASE1_RECOVERY, - "phase1 dev switch error in recovery"); - TEST_EQ(sd->recovery_reason, VB2_RECOVERY_RO_UNSPECIFIED, + TEST_EQ(sd->recovery_reason, VB2_RECOVERY_DEV_SWITCH, " recovery reason"); /* Check that DISPLAY_AVAILABLE gets set on recovery mode. */ TEST_NEQ(ctx->flags & VB2_CONTEXT_DISPLAY_INIT, -- cgit v1.2.1