diff options
author | Joel Kitching <kitching@google.com> | 2020-04-07 17:22:15 +0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-04-15 03:32:21 +0000 |
commit | 63ef6156fac191eac5e22b40b40d17c53510c9d2 (patch) | |
tree | fb5c71f0af7aa292dd49f3b5cf2b7d8541ce008a | |
parent | cfe98a1b6284c71cc193abba66940413899e25e6 (diff) | |
download | vboot-63ef6156fac191eac5e22b40b40d17c53510c9d2.tar.gz |
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>
Signed-off-by: Frank Wu <frank_wu@compal.corp-partner.google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2148571
Reviewed-by: Joel Kitching <kitching@chromium.org>
-rw-r--r-- | firmware/2lib/2misc.c | 19 | ||||
-rw-r--r-- | firmware/lib/vboot_api_kernel.c | 9 | ||||
-rw-r--r-- | tests/vb2_misc_tests.c | 16 |
3 files changed, 33 insertions, 11 deletions
diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c index 54ddb900..b23c1980 100644 --- a/firmware/2lib/2misc.c +++ b/firmware/2lib/2misc.c @@ -148,7 +148,8 @@ void vb2_check_recovery(struct vb2_context *ctx) 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. @@ -419,16 +420,20 @@ int vb2_allow_recovery(struct vb2_context *ctx) 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 72224e19..72b2a3c6 100644 --- a/firmware/lib/vboot_api_kernel.c +++ b/firmware/lib/vboot_api_kernel.c @@ -321,10 +321,14 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, return rv; } + /* + * 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) { @@ -338,6 +342,7 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, */ vb2ex_commit_data(ctx); + /* Recovery boot. This has UI. */ if (LEGACY_MENU_UI) rv = VbBootRecoveryLegacyMenu(ctx); diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c index 89f534d7..a0483a50 100644 --- a/tests/vb2_misc_tests.c +++ b/tests/vb2_misc_tests.c @@ -454,13 +454,23 @@ static void recovery_tests(void) 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"); } @@ -764,6 +774,7 @@ static void clear_recovery_tests(void) 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); @@ -776,6 +787,7 @@ static void clear_recovery_tests(void) 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); |