2kernel: Add vb2api_kernel_phase2 and vb2api_normal_bootfirmware-chameleon-14947.Bfirmware-14947.B

Extract the middle part of VbSelectAndLoadKernel as vb2api_kernel_phase2
and call it from VbSelectAndLoadKernel. Also, remove vb2_nv_init in
VbSelectAndLoadKernel. Furthermore, publicize vb2_normal_boot as
vb2api_normal_boot in preparation for moving the content of
VbSelectAndLoadKernel to depthcharge. Besides, when NO_BOOT is set,
manual recovery should be disallowed (unless
VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY is set). Therefore, print the NO_BOOT
debug log only for the broken screen case.

BUG=b:172339016
BRANCH=none
TEST=make runtests

Signed-off-by: Hsin-Te Yuan <yuanhsinte@google.com>
Change-Id: I4dc5ee4fb80ecc8c24a992a489c3bf6fe267046d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3720975
Reviewed-by: Hsuan Ting Chen <roccochen@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>

5 files changed, 117 insertions, 109 deletions

diff --git a/firmware/2lib/2kernel.c b/firmware/2lib/2kernel.c
index 350a1bf6..8d17ff53 100644
--- a/firmware/2lib/2kernel.c
+++ b/firmware/2lib/2kernel.c
@@ -6,7 +6,6 @@
  */
 
 #include "2common.h"
-#include "2kernel.h"
 #include "2misc.h"
 #include "2nvstorage.h"
 #include "2rsa.h"
@@ -38,7 +37,7 @@ static int vb2_reset_nv_requests(struct vb2_context *ctx)
 	return need_reboot;
 }
 
-vb2_error_t vb2_normal_boot(struct vb2_context *ctx)
+vb2_error_t vb2api_normal_boot(struct vb2_context *ctx)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 	uint32_t max_rollforward = vb2_nv_get(ctx,
@@ -198,6 +197,87 @@ vb2_error_t vb2api_kernel_phase1(struct vb2_context *ctx)
 	return VB2_SUCCESS;
 }
 
+static vb2_error_t handle_battery_cutoff(struct vb2_context *ctx)
+{
+	/*
+	 * Check if we need to cut-off battery. This should be done after EC
+	 * FW and auxfw are updated, and before the kernel is started.  This
+	 * is to make sure all firmware is up-to-date before shipping (which
+	 * is the typical use-case for cutoff).
+	 */
+	if (vb2_nv_get(ctx, VB2_NV_BATTERY_CUTOFF_REQUEST)) {
+		VB2_DEBUG("Request to cut-off battery\n");
+		vb2_nv_set(ctx, VB2_NV_BATTERY_CUTOFF_REQUEST, 0);
+
+		/* May lose power immediately, so commit our update now. */
+		VB2_TRY(vb2ex_commit_data(ctx));
+
+		vb2ex_ec_battery_cutoff();
+		return VB2_REQUEST_SHUTDOWN;
+	}
+
+	return VB2_SUCCESS;
+}
+
+vb2_error_t vb2api_kernel_phase2(struct vb2_context *ctx)
+{
+	struct vb2_shared_data *sd = vb2_get_sd(ctx);
+	vb2_gbb_flags_t gbb_flags = vb2api_gbb_get_flags(ctx);
+
+	VB2_DEBUG("GBB flags are %#x\n", gbb_flags);
+
+	/*
+	 * Do EC and auxfw software sync unless we're in recovery mode. This
+	 * has UI but it's just a single non-interactive WAIT screen.
+	 */
+	if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) {
+		VB2_TRY(vb2api_ec_sync(ctx));
+		VB2_TRY(vb2api_auxfw_sync(ctx));
+		VB2_TRY(handle_battery_cutoff(ctx));
+	}
+
+	/*
+	 * If in the broken screen, save the recovery reason as subcode.
+	 * Otherwise, clear any leftover recovery requests or subcodes.
+	 */
+	vb2_clear_recovery(ctx);
+
+	/* Select boot path */
+	switch (ctx->boot_mode) {
+	case VB2_BOOT_MODE_MANUAL_RECOVERY:
+	case VB2_BOOT_MODE_BROKEN_SCREEN:
+		/* If we're in recovery mode just to do memory retraining, all
+		   we need to do is reboot. */
+		if (sd->recovery_reason == VB2_RECOVERY_TRAIN_AND_REBOOT) {
+			VB2_DEBUG("Reboot after retraining in recovery\n");
+			return VB2_REQUEST_REBOOT;
+		}
+
+		/*
+		 * Need to commit nvdata changes immediately, since we will be
+		 * entering either manual recovery UI or BROKEN screen shortly.
+		 */
+		vb2ex_commit_data(ctx);
+		break;
+	case VB2_BOOT_MODE_DIAGNOSTICS:
+		/*
+		 * Need to clear the request flag and commit nvdata changes
+		 * immediately to avoid booting back into diagnostic tool when a
+		 * forced system reset occurs.
+		 */
+		vb2_nv_set(ctx, VB2_NV_DIAG_REQUEST, 0);
+		vb2ex_commit_data(ctx);
+		break;
+	case VB2_BOOT_MODE_DEVELOPER:
+	case VB2_BOOT_MODE_NORMAL:
+		break;
+	default:
+		return VB2_ERROR_ESCAPE_NO_BOOT;
+	}
+
+	return VB2_SUCCESS;
+}
+
 vb2_error_t vb2api_kernel_finalize(struct vb2_context *ctx)
 {
 	vb2_gbb_flags_t gbb_flags = vb2api_gbb_get_flags(ctx);
diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h
index bd0d14ed..6db87b6a 100644
--- a/firmware/2lib/include/2api.h
+++ b/firmware/2lib/include/2api.h
@@ -512,7 +512,8 @@ enum vb2_pcr_digest {
  *	use to verify kernel data - the recovery key from the GBB, or the
  *	kernel subkey from the firmware verification stage.
  *
- *	Kernel phase 2 is finding loading, and verifying the kernel partition.
+ *	Call vb2api_kernel_phase2().  Do EC and auxfw software sync, clear
+ *	recovery and commit nvdata if needed.
  *
  *	Find a boot device (you're on your own here).
  *
@@ -818,6 +819,24 @@ vb2_error_t vb2api_get_pcr_digest(struct vb2_context *ctx,
 vb2_error_t vb2api_kernel_phase1(struct vb2_context *ctx);
 
 /**
+ * Do kernel verification.
+ *
+ * Must be called after vb2api_kernel_phase1.
+ *
+ * @param ctx		Vboot context
+ * @return VB2_SUCCESS, or error code on error.
+ */
+vb2_error_t vb2api_kernel_phase2(struct vb2_context *ctx);
+
+/**
+ * Handle a normal boot.
+ *
+ * @param ctx		Vboot context.
+ * @return VB2_SUCCESS, or error code on error.
+ */
+vb2_error_t vb2api_normal_boot(struct vb2_context *ctx);
+
+/**
  * Finalize for kernel verification stage.
  *
  * Handle NO_BOOT flag.
diff --git a/firmware/2lib/include/2kernel.h b/firmware/2lib/include/2kernel.h
deleted file mode 100644
index bed1a01f..00000000
--- a/firmware/2lib/include/2kernel.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Copyright 2020 The Chromium OS Authors. All rights reserved.
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- *
- * Kernel selection, loading, verification, and booting.
- */
-
-#ifndef VBOOT_REFERENCE_2KERNEL_H_
-#define VBOOT_REFERENCE_2KERNEL_H_
-
-#include "2common.h"
-
-/**
- * Handle a normal boot.
- *
- * @param ctx		Vboot context.
- * @return VB2_SUCCESS, or error code on error.
- */
-vb2_error_t vb2_normal_boot(struct vb2_context *ctx);
-
-#endif  /* VBOOT_REFERENCE_2KERNEL_H_ */
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index a46317e1..f4e5146a 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -7,7 +7,6 @@
 
 #include "2api.h"
 #include "2common.h"
-#include "2kernel.h"
 #include "2misc.h"
 #include "2nvstorage.h"
 #include "2rsa.h"
@@ -29,28 +28,6 @@ struct VbSelectAndLoadKernelParams **VbApiKernelGetParamsPtr(void)
 }
 #endif
 
-static vb2_error_t handle_battery_cutoff(struct vb2_context *ctx)
-{
-	/*
-	 * Check if we need to cut-off battery. This should be done after EC
-	 * FW and auxfw are updated, and before the kernel is started.  This
-	 * is to make sure all firmware is up-to-date before shipping (which
-	 * is the typical use-case for cutoff).
-	 */
-	if (vb2_nv_get(ctx, VB2_NV_BATTERY_CUTOFF_REQUEST)) {
-		VB2_DEBUG("Request to cut-off battery\n");
-		vb2_nv_set(ctx, VB2_NV_BATTERY_CUTOFF_REQUEST, 0);
-
-		/* May lose power immediately, so commit our update now. */
-		VB2_TRY(vb2ex_commit_data(ctx));
-
-		vb2ex_ec_battery_cutoff();
-		return VB2_REQUEST_SHUTDOWN;
-	}
-
-	return VB2_SUCCESS;
-}
-
 static int is_valid_disk(VbDiskInfo *info, uint32_t disk_flags)
 {
 	return info->bytes_per_lba >= 512 &&
@@ -159,54 +136,19 @@ vb2_error_t VbTryLoadMiniOsKernel(struct vb2_context *ctx,
 vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx,
 				  VbSelectAndLoadKernelParams *kparams)
 {
-	struct vb2_shared_data *sd = vb2_get_sd(ctx);
-	vb2_gbb_flags_t gbb_flags = vb2api_gbb_get_flags(ctx);
-
 	/* TODO: Send this argument through subsequent function calls, rather
 	   than relying on a global to pass it to VbTryLoadKernel. */
 	kparams_ptr = kparams;
 
-	/* Init nvstorage space. TODO(kitching): Remove once we add assertions
-	   to vb2_nv_get and vb2_nv_set. */
-	vb2_nv_init(ctx);
-
 	VB2_TRY(vb2api_kernel_phase1(ctx));
+	VB2_TRY(vb2api_kernel_phase2(ctx));
 
-	VB2_DEBUG("GBB flags are %#x\n", gbb_flags);
-
-	/*
-	 * Do EC and auxfw software sync unless we're in recovery mode. This
-	 * has UI but it's just a single non-interactive WAIT screen.
-	 */
-	if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) {
-		VB2_TRY(vb2api_ec_sync(ctx));
-		VB2_TRY(vb2api_auxfw_sync(ctx));
-		VB2_TRY(handle_battery_cutoff(ctx));
-	}
-
-	/*
-	 * If in the broken screen, save the recovery reason as subcode.
-	 * Otherwise, clear any leftover recovery requests or subcodes.
-	 */
-	vb2_clear_recovery(ctx);
-
-	/* Select boot path */
 	switch (ctx->boot_mode) {
 	case VB2_BOOT_MODE_MANUAL_RECOVERY:
+		/* Manual recovery boot.  This has UI. */
+		VB2_TRY(vb2ex_manual_recovery_ui(ctx));
+		break;
 	case VB2_BOOT_MODE_BROKEN_SCREEN:
-		/* If we're in recovery mode just to do memory retraining, all
-		   we need to do is reboot. */
-		if (sd->recovery_reason == VB2_RECOVERY_TRAIN_AND_REBOOT) {
-			VB2_DEBUG("Reboot after retraining in recovery\n");
-			return VB2_REQUEST_REBOOT;
-		}
-
-		/*
-		 * Need to commit nvdata changes immediately, since we will be
-		 * entering either manual recovery UI or BROKEN screen shortly.
-		 */
-		vb2ex_commit_data(ctx);
-
 		/*
 		 * In EFS2, recovery mode can be entered even when battery is
 		 * drained or damaged. EC-RO sets NO_BOOT flag in such case and
@@ -217,21 +159,10 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx,
 		if (ctx->flags & VB2_CONTEXT_NO_BOOT)
 			VB2_DEBUG("NO_BOOT in RECOVERY mode\n");
 
-		/* Recovery boot.  This has UI. */
-		if (ctx->boot_mode == VB2_BOOT_MODE_MANUAL_RECOVERY)
-			VB2_TRY(vb2ex_manual_recovery_ui(ctx));
-		else
-			VB2_TRY(vb2ex_broken_screen_ui(ctx));
+		/* Broken screen.  This has UI. */
+		VB2_TRY(vb2ex_broken_screen_ui(ctx));
 		break;
 	case VB2_BOOT_MODE_DIAGNOSTICS:
-		/*
-		 * Need to clear the request flag and commit nvdata changes
-		 * immediately to avoid booting back into diagnostic tool when a
-		 * forced system reset occurs.
-		 */
-		vb2_nv_set(ctx, VB2_NV_DIAG_REQUEST, 0);
-		vb2ex_commit_data(ctx);
-
 		/* Diagnostic boot.  This has UI. */
 		VB2_TRY(vb2ex_diagnostic_ui(ctx));
 		/*
@@ -245,7 +176,7 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx,
 		break;
 	case VB2_BOOT_MODE_NORMAL:
 		/* Normal boot */
-		VB2_TRY(vb2_normal_boot(ctx));
+		VB2_TRY(vb2api_normal_boot(ctx));
 		break;
 	default:
 		return VB2_ERROR_ESCAPE_NO_BOOT;
diff --git a/tests/vb2_kernel_tests.c b/tests/vb2_kernel_tests.c
index 0b3e94c7..c467fa9d 100644
--- a/tests/vb2_kernel_tests.c
+++ b/tests/vb2_kernel_tests.c
@@ -7,7 +7,6 @@
 
 #include "2api.h"
 #include "2common.h"
-#include "2kernel.h"
 #include "2misc.h"
 #include "2nvstorage.h"
 #include "2rsa.h"
@@ -309,26 +308,26 @@ static void normal_boot_tests(void)
 {
 	reset_common_data(FOR_NORMAL_BOOT);
 	mock_vbtlk_expect_fixed = 1;
-	TEST_EQ(vb2_normal_boot(ctx), VB2_SUCCESS,
-		"vb2_normal_boot() returns VB2_SUCCESS");
+	TEST_EQ(vb2api_normal_boot(ctx), VB2_SUCCESS,
+		"vb2api_normal_boot() returns VB2_SUCCESS");
 
 	reset_common_data(FOR_NORMAL_BOOT);
 	mock_vbtlk_expect_fixed = 1;
 	mock_vbtlk_retval = VB2_ERROR_MOCK;
-	TEST_EQ(vb2_normal_boot(ctx), VB2_ERROR_MOCK,
-		"vb2_normal_boot() returns VB2_ERROR_MOCK");
+	TEST_EQ(vb2api_normal_boot(ctx), VB2_ERROR_MOCK,
+		"vb2api_normal_boot() returns VB2_ERROR_MOCK");
 
 	reset_common_data(FOR_NORMAL_BOOT);
 	vb2_nv_set(ctx, VB2_NV_DISPLAY_REQUEST, 1);
-	TEST_EQ(vb2_normal_boot(ctx), VB2_REQUEST_REBOOT,
-		"vb2_normal_boot() reboot to reset NVRAM display request");
+	TEST_EQ(vb2api_normal_boot(ctx), VB2_REQUEST_REBOOT,
+		"vb2api_normal_boot() reboot to reset NVRAM display request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_DISPLAY_REQUEST), 0,
 		"  display request reset");
 
 	reset_common_data(FOR_NORMAL_BOOT);
 	vb2_nv_set(ctx, VB2_NV_DIAG_REQUEST, 1);
-	TEST_EQ(vb2_normal_boot(ctx), VB2_REQUEST_REBOOT,
-		"vb2_normal_boot() reboot to reset NVRAM diag request");
+	TEST_EQ(vb2api_normal_boot(ctx), VB2_REQUEST_REBOOT,
+		"vb2api_normal_boot() reboot to reset NVRAM diag request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_DIAG_REQUEST), 0,
 		"  diag request reset");}