From ffa02e80c8cec862106607ffd0333258bb1ed62e Mon Sep 17 00:00:00 2001 From: Joel Kitching Date: Fri, 4 Oct 2019 16:18:00 +0800 Subject: vboot: make BROKEN screen code for saving nvdata more consistent Remove SAVE_LOCALE_IMMEDIATELY. Check for VB2_CONTEXT_RECOVERY_MODE and !vb2_allow_recovery() before committing nvdata. Ensure comments are consistent. BUG=b:124141368, chromium:1006689 TEST=make clean && make runtests BRANCH=none Change-Id: I6919fb858f999c6d8b81a090dc1f271756bc7dc4 Signed-off-by: Joel Kitching Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1840192 Tested-by: Joel Kitching Commit-Queue: Joel Kitching Reviewed-by: Julius Werner --- Makefile | 6 ------ firmware/lib/vboot_display.c | 17 +++++++---------- firmware/lib/vboot_ui.c | 6 ++++-- firmware/lib/vboot_ui_menu.c | 10 +++++----- 4 files changed, 16 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index 0bb8de90..8a2ec5a3 100644 --- a/Makefile +++ b/Makefile @@ -860,12 +860,6 @@ ${FWLIB_OBJS}: CFLAGS += -DUNROLL_LOOPS ${FWLIB2X_OBJS}: CFLAGS += -DUNROLL_LOOPS ${FWLIB20_OBJS}: CFLAGS += -DUNROLL_LOOPS ${FWLIB21_OBJS}: CFLAGS += -DUNROLL_LOOPS - -# Workaround for coreboot on x86, which will power off asynchronously -# without giving us a chance to react. This is not an example of the Right -# Way to do things. See chrome-os-partner:7689, and the commit message -# that made this change. -${FWLIB_OBJS}: CFLAGS += -DSAVE_LOCALE_IMMEDIATELY endif ${FWLIB21_OBJS}: INCLUDES += -Ifirmware/lib21/include diff --git a/firmware/lib/vboot_display.c b/firmware/lib/vboot_display.c index 218d66f7..13107d37 100644 --- a/firmware/lib/vboot_display.c +++ b/firmware/lib/vboot_display.c @@ -14,6 +14,7 @@ #include "vboot_api.h" #include "vboot_common.h" #include "vboot_display.h" +#include "vboot_kernel.h" static uint32_t disp_current_screen = VB_SCREEN_BLANK; static uint32_t disp_current_index = 0; @@ -382,18 +383,14 @@ vb2_error_t VbCheckDisplayKey(struct vb2_context *ctx, uint32_t key, vb2_nv_set(ctx, VB2_NV_LOCALIZATION_INDEX, loc); vb2_nv_set(ctx, VB2_NV_BACKUP_NVRAM_REQUEST, 1); -#ifdef SAVE_LOCALE_IMMEDIATELY /* - * This is a workaround for coreboot on x86, which will power - * off asynchronously without giving us a chance to react. - * This is not an example of the Right Way to do things. See - * chrome-os-partner:7689. + * Non-manual recovery mode is meant to be left via three-finger + * salute (into manual recovery mode). Need to commit nvdata + * changes immediately. */ - if (ctx->flags & VB2_CONTEXT_NVDATA_CHANGED) { - VbExNvStorageWrite(ctx.nvdata); - ctx.flags &= ~VB2_CONTEXT_NVDATA_CHANGED; - } -#endif + if ((ctx->flags & VB2_CONTEXT_RECOVERY_MODE) && + !vb2_allow_recovery(ctx)) + vb2_nv_commit(ctx); /* Force redraw of current screen */ return VbDisplayScreen(ctx, disp_current_screen, 1, data); diff --git a/firmware/lib/vboot_ui.c b/firmware/lib/vboot_ui.c index fd61503f..362aa38e 100644 --- a/firmware/lib/vboot_ui.c +++ b/firmware/lib/vboot_ui.c @@ -809,9 +809,11 @@ static vb2_error_t recovery_ui(struct vb2_context *ctx) shared->recovery_reason); vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, shared->recovery_reason); + /* - * Commit NV now, because it won't get saved if the user forces - * manual recovery via the three-finger salute. + * Non-manual recovery mode is meant to be left via three-finger + * salute (into manual recovery mode). Need to commit nvdata + * changes immediately. */ vb2_nv_commit(ctx); diff --git a/firmware/lib/vboot_ui_menu.c b/firmware/lib/vboot_ui_menu.c index b932ce9e..81babf02 100644 --- a/firmware/lib/vboot_ui_menu.c +++ b/firmware/lib/vboot_ui_menu.c @@ -308,16 +308,16 @@ static vb2_error_t debug_info_action(struct vb2_context *ctx) /* Action when selecting a language entry in the language menu. */ static vb2_error_t language_action(struct vb2_context *ctx) { - VbSharedDataHeader *vbsd = vb2_get_sd(ctx)->vbsd; - /* Write selected language ID back to NVRAM. */ vb2_nv_set(ctx, VB2_NV_LOCALIZATION_INDEX, current_menu_idx); /* - * Non-manual recovery mode is meant to be left via hard reset (into - * manual recovery mode). Need to commit NVRAM changes immediately. + * Non-manual recovery mode is meant to be left via three-finger + * salute (into manual recovery mode). Need to commit nvdata + * changes immediately. */ - if (vbsd->recovery_reason && !vb2_allow_recovery(ctx)) + if ((ctx->flags & VB2_CONTEXT_RECOVERY_MODE) && + !vb2_allow_recovery(ctx)) vb2_nv_commit(ctx); /* Return to previous menu. */ -- cgit v1.2.1