vboot: clear recovery request in all boot modes
Previously, recovery requests are only cleared when user
initiates a manual recovery. This causes problems with
two cases specifically:
* Transient failures - The recovery request remains in the
subcode field for some unknown period of time, and then
erroneously gets promoted to the "recovery reason" the
next time the user initiates a manual recovery request.
* TRAIN_AND_REBOOT - The recovery request remains in the
subcode field after training has completed. The next
time a manual recovery request is initiated, the subcode
is promoted and training occurs yet again. When finished,
a reboot occurs and the user ends up back in the OS.
Make two changes to deal with these cases:
* Clear recovery request (including subcode) unconditionally
for non-recovery boot modes.
* Stop promoting TRAIN_AND_REBOOT subcodes.
BUG=b:153157134, b:35576380
TEST=make clean && make runtests
BRANCH=none
Change-Id: I79f8fbed72a9d052b5ed5f70e9a2515136b6ef10
Signed-off-by: Joel Kitching <kitching@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2139335
Tested-by: Joel Kitching <kitching@chromium.org>
Tested-by: Frank Wu <frank_wu@compal.corp-partner.google.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>
diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c
index 30d6011..de4b424 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -144,7 +144,8 @@
if (ctx->flags & VB2_CONTEXT_FORCE_RECOVERY_MODE) {
VB2_DEBUG("Recovery was requested manually\n");
- if (subcode && !sd->recovery_reason)
+ if (subcode && !sd->recovery_reason &&
+ subcode != VB2_RECOVERY_TRAIN_AND_REBOOT)
/*
* Recovery was requested at 'broken' screen.
* Promote subcode to reason.
@@ -416,16 +417,20 @@
void vb2_clear_recovery(struct vb2_context *ctx)
{
struct vb2_shared_data *sd = vb2_get_sd(ctx);
+ uint32_t reason = vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST);
+ uint32_t subcode = vb2_nv_get(ctx, VB2_NV_RECOVERY_SUBCODE);
- VB2_DEBUG("Clearing recovery request: %#x / %#x\n",
- vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
- vb2_nv_get(ctx, VB2_NV_RECOVERY_SUBCODE));
+ if (reason || subcode)
+ VB2_DEBUG("Clearing recovery request: %#x / %#x\n",
+ reason, subcode);
- /* Clear recovery request for both cases. */
+ /* Clear recovery request for both manual and non-manual. */
vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, VB2_RECOVERY_NOT_REQUESTED);
- vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, VB2_RECOVERY_NOT_REQUESTED);
+ vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 0);
- if (!vb2_allow_recovery(ctx)) {
+ /* But stow recovery reason as subcode for non-manual recovery. */
+ if ((ctx->flags & VB2_CONTEXT_RECOVERY_MODE) &&
+ !vb2_allow_recovery(ctx)) {
VB2_DEBUG("Stow recovery reason as subcode (%#x)\n",
sd->recovery_reason);
vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, sd->recovery_reason);
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index ff13a25..14f70c7 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -198,10 +198,14 @@
VB2_TRY(handle_battery_cutoff(ctx));
}
+ /*
+ * If in non-manual recovery mode, save the recovery reason as subcode.
+ * Otherwise, clear any leftover recovery requests or subcodes.
+ */
+ vb2_clear_recovery(ctx);
+
/* Select boot path */
if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) {
- vb2_clear_recovery(ctx);
-
/* 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) {
@@ -214,6 +218,7 @@
* 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
diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c
index 1d8b42e..493aab4 100644
--- a/tests/vb2_misc_tests.c
+++ b/tests/vb2_misc_tests.c
@@ -458,13 +458,23 @@
TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
0, "SD flag set");
- /* Override at broken screen */
+ /* Override subcode TRAIN_AND_REBOOT */
+ reset_common_data();
+ vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, VB2_RECOVERY_TRAIN_AND_REBOOT);
+ ctx->flags |= VB2_CONTEXT_FORCE_RECOVERY_MODE;
+ vb2_check_recovery(ctx);
+ TEST_EQ(sd->recovery_reason, VB2_RECOVERY_RO_MANUAL,
+ "Recovery reason forced");
+ TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
+ 0, "SD flag set");
+
+ /* Promote subcode from BROKEN screen*/
reset_common_data();
vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, VB2_RECOVERY_US_TEST);
ctx->flags |= VB2_CONTEXT_FORCE_RECOVERY_MODE;
vb2_check_recovery(ctx);
TEST_EQ(sd->recovery_reason, VB2_RECOVERY_US_TEST,
- "Recovery reason forced from broken");
+ "Recovery reason forced from BROKEN");
TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
0, "SD flag set");
}
@@ -768,6 +778,7 @@
reset_common_data();
allow_recovery_retval = 1;
sd->recovery_reason = 4;
+ ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, 5);
vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 13);
vb2_clear_recovery(ctx);
@@ -780,6 +791,7 @@
reset_common_data();
allow_recovery_retval = 0;
sd->recovery_reason = 4;
+ ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, 5);
vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 13);
vb2_clear_recovery(ctx);
