vb2: parse dev mode in phase1

The rest of the system software expects dev mode to be
orthogonal to recovery mode. However, vb2_fw_phase1() was
only checking recovery mode. The caller would then not
call vb2_fw_phase2() which checked the developer mode settings.
Fix this by doing the necessary steps in vb2_fw_phase1().

BUG=chrome-os-partner:33264
BRANCH=None
TEST=None yet.

Change-Id: I06da388c3ca33754ddb5937a2877fbc287946490
Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://chromium-review.googlesource.com/225906
Tested-by: Furquan Shaikh <furquan@chromium.org>
Commit-Queue: Furquan Shaikh <furquan@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Benson Leung <bleung@chromium.org>
Tested-by: Benson Leung <bleung@chromium.org>

3 files changed, 35 insertions, 32 deletions

diff --git a/firmware/2lib/2api.c b/firmware/2lib/2api.c
index 5070d21f..a01c7f57 100644
--- a/firmware/2lib/2api.c
+++ b/firmware/2lib/2api.c
@@ -35,7 +35,6 @@ void vb2api_fail(struct vb2_context *ctx, uint8_t reason, uint8_t subcode)
 
 int vb2api_fw_phase1(struct vb2_context *ctx)
 {
-	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 	int rv;
 
 	/* Initialize the vboot context if it hasn't been yet */
@@ -47,7 +46,17 @@ int vb2api_fw_phase1(struct vb2_context *ctx)
 	/* Initialize secure data */
 	rv = vb2_secdata_init(ctx);
 	if (rv)
-		sd->recovery_reason = VB2_RECOVERY_SECDATA_INIT;
+		vb2_fail(ctx, VB2_RECOVERY_SECDATA_INIT, rv);
+
+	/* Load and parse the GBB header */
+	rv = vb2_fw_parse_gbb(ctx);
+	if (rv)
+		vb2_fail(ctx, VB2_RECOVERY_GBB_HEADER, rv);
+
+	/* Check for dev switch */
+	rv = vb2_check_dev_switch(ctx);
+	if (rv)
+		vb2_fail(ctx, VB2_RECOVERY_DEV_SWITCH, rv);
 
 	/*
 	 * Check for recovery.  Note that this function returns void, since
@@ -70,20 +79,6 @@ int vb2api_fw_phase2(struct vb2_context *ctx)
 {
 	int rv;
 
-	/* Load and parse the GBB header */
-	rv = vb2_fw_parse_gbb(ctx);
-	if (rv) {
-		vb2_fail(ctx, VB2_RECOVERY_GBB_HEADER, rv);
-		return rv;
-	}
-
-	/* Check for dev switch */
-	rv = vb2_check_dev_switch(ctx);
-	if (rv) {
-		vb2_fail(ctx, VB2_RECOVERY_DEV_SWITCH, rv);
-		return rv;
-	}
-
 	/* Always clear RAM when entering developer mode */
 	if (ctx->flags & VB2_CONTEXT_DEVELOPER_MODE)
 		ctx->flags |= VB2_CONTEXT_CLEAR_RAM;
diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c
index c36372bf..48357bea 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -162,7 +162,7 @@ void vb2_check_recovery(struct vb2_context *ctx)
 	/* If forcing recovery, override recovery reason */
 	if (ctx->flags & VB2_CONTEXT_FORCE_RECOVERY_MODE) {
 		sd->recovery_reason = VB2_RECOVERY_RO_MANUAL;
-		sd->flags = VB2_SD_FLAG_MANUAL_RECOVERY;
+		sd->flags |= VB2_SD_FLAG_MANUAL_RECOVERY;
 	}
 
 	/* If recovery reason is non-zero, tell caller we need recovery mode */
diff --git a/tests/vb2_api_tests.c b/tests/vb2_api_tests.c
index 0d2773a5..cf706d46 100644
--- a/tests/vb2_api_tests.c
+++ b/tests/vb2_api_tests.c
@@ -218,10 +218,30 @@ static void phase1_tests(void)
 	TEST_EQ(cc.flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");
 
 	reset_common_data(FOR_MISC);
+	retval_vb2_fw_parse_gbb = VB2_ERROR_GBB_MAGIC;
+	TEST_EQ(vb2api_fw_phase1(&cc), VB2_ERROR_API_PHASE1_RECOVERY,
+		"phase1 gbb");
+	TEST_EQ(sd->recovery_reason, VB2_RECOVERY_GBB_HEADER,
+		"  recovery reason");
+	TEST_NEQ(cc.flags & VB2_CONTEXT_RECOVERY_MODE, 0, "  recovery flag");
+	TEST_NEQ(cc.flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");
+
+
+	reset_common_data(FOR_MISC);
+	retval_vb2_check_dev_switch = VB2_ERROR_MOCK;
+	TEST_EQ(vb2api_fw_phase1(&cc), VB2_ERROR_API_PHASE1_RECOVERY,
+		"phase1 dev switch");
+	TEST_EQ(sd->recovery_reason, VB2_RECOVERY_DEV_SWITCH,
+		"  recovery reason");
+	TEST_NEQ(cc.flags & VB2_CONTEXT_RECOVERY_MODE, 0, "  recovery flag");
+	TEST_NEQ(cc.flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");
+
+	reset_common_data(FOR_MISC);
 	cc.secdata[0] ^= 0x42;
-	TEST_EQ(vb2api_fw_phase1(&cc),
-		VB2_ERROR_API_PHASE1_RECOVERY, "phase1 secdata");
-	TEST_EQ(sd->recovery_reason, VB2_RECOVERY_SECDATA_INIT, "  recovery");
+	TEST_EQ(vb2api_fw_phase1(&cc), VB2_ERROR_API_PHASE1_RECOVERY,
+		"phase1 secdata");
+	TEST_EQ(sd->recovery_reason, VB2_RECOVERY_SECDATA_INIT,
+		"  recovery reason");
 	TEST_NEQ(cc.flags & VB2_CONTEXT_RECOVERY_MODE, 0, "  recovery flag");
 	TEST_NEQ(cc.flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");
 }
@@ -233,18 +253,6 @@ static void phase2_tests(void)
 	TEST_EQ(cc.flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");
 
 	reset_common_data(FOR_MISC);
-	retval_vb2_fw_parse_gbb = VB2_ERROR_GBB_MAGIC;
-	TEST_EQ(vb2api_fw_phase2(&cc), VB2_ERROR_GBB_MAGIC, "phase2 gbb");
-	TEST_EQ(vb2_nv_get(&cc, VB2_NV_RECOVERY_REQUEST),
-		VB2_RECOVERY_GBB_HEADER, "  recovery reason");
-
-	reset_common_data(FOR_MISC);
-	retval_vb2_check_dev_switch = VB2_ERROR_MOCK;
-	TEST_EQ(vb2api_fw_phase2(&cc), VB2_ERROR_MOCK, "phase2 dev switch");
-	TEST_EQ(vb2_nv_get(&cc, VB2_NV_RECOVERY_REQUEST),
-		VB2_RECOVERY_DEV_SWITCH, "  recovery reason");
-
-	reset_common_data(FOR_MISC);
 	cc.flags |= VB2_CONTEXT_DEVELOPER_MODE;
 	TEST_SUCC(vb2api_fw_phase2(&cc), "phase1 dev");
 	TEST_NEQ(cc.flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");