2api: Adjust the order of checking dev switch in vb2api_fw_phase1stabilize-14238.B

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 <roccochen@chromium.org>
Change-Id: Iada8092e73ed9282242b1d67d101c836860e13c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3168437
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>

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,