[coreboot-gerrit] Patch set updated for coreboot: vboot: Clear battery cutoff flags when vbnv_cmos loads backup VBNV.

Martin Roth (martinroth@google.com) gerrit at coreboot.org
Tue Jan 3 19:02:46 CET 2017


Martin Roth (martinroth at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/18008

-gerrit

commit 468e977404f63dc117f2e5f46d2eb77c8d377c38
Author: Hung-Te Lin <hungte at chromium.org>
Date:   Thu Dec 29 20:59:37 2016 +0800

    vboot: Clear battery cutoff flags when vbnv_cmos loads backup VBNV.
    
    When CONFIG_VBOOT_VBNV_CMOS_BACKUP_TO_FLASH is set, vbnv_cmos will try
    to load VBNV from flash if the VBNV in CMOS is invalid. This is usually
    correct, except the case of battery cut-off.
    
    CMOS will always be invalid after battery cut-off if there is no RTC
    battery (or if that is dead). However, in current implementation the
    backup in flash is only updated in coreboot, while the real battery
    cutoff (and the clearing of cutoff flags in VBNV) is done in payload
    (Depthcharge) stage. This will create an endless reboot loop that:
    
     1. crossystem sets battery cutoff flag in VBNV_CMOS then reboot.
     2. coreboot backs-up VBNV_CMOS to VBNV_flash.
     3. Depthcharge sees cutoff flag in VBNV_CMOS.
     4. Depthcharge clears cutoff flag in VBNV_CMOS.
     5. Depthcharge performs battery cutoff (CMOS data is lost).
     6. (Plug AC adapter) Reboot.
     7. Coreboot sees invalid VBNV_CMOS, load backup from VBNV_flash.
     8. Jump to 3.
    
    As a result, we should always clear battery cutoff flags when loading
    backups from VBNV_flash.
    
    BRANCH=glados,reef
    BUG=chrome-os-partner:61365,chrome-os-partner:59615
    TEST=emerge-reef coreboot bootimage;
    
    Change-Id: I3250a3a179a7b0de9c6e401e4a94dcd23920e473
    Signed-off-by: Hung-Te Lin <hungte at chromium.org>
    Reviewed-on: https://chromium-review.googlesource.com/423460
    Reviewed-by: Duncan Laurie <dlaurie at google.com>
---
 src/vboot/vbnv.c        |  6 ++++++
 src/vboot/vbnv.h        |  1 +
 src/vboot/vbnv_cmos.c   | 17 +++++++++++++++++
 src/vboot/vbnv_layout.h |  3 +++
 4 files changed, 27 insertions(+)

diff --git a/src/vboot/vbnv.c b/src/vboot/vbnv.c
index ce64928..6537bf0 100644
--- a/src/vboot/vbnv.c
+++ b/src/vboot/vbnv.c
@@ -79,6 +79,12 @@ int verify_vbnv(uint8_t *vbnv_copy)
 		(crc8_vbnv(vbnv_copy, CRC_OFFSET) == vbnv_copy[CRC_OFFSET]);
 }
 
+/* Re-generate VBNV checksum. */
+void regen_vbnv_crc(uint8_t *vbnv_copy)
+{
+	vbnv_copy[CRC_OFFSET] = crc8_vbnv(vbnv_copy, CRC_OFFSET);
+}
+
 /*
  * Read VBNV data from configured storage backend.
  * If VBNV verification fails, reset the vbnv copy.
diff --git a/src/vboot/vbnv.h b/src/vboot/vbnv.h
index 78ca8f6..30da6a5 100644
--- a/src/vboot/vbnv.h
+++ b/src/vboot/vbnv.h
@@ -22,6 +22,7 @@
 void read_vbnv(uint8_t *vbnv_copy);
 void save_vbnv(const uint8_t *vbnv_copy);
 int verify_vbnv(uint8_t *vbnv_copy);
+void regen_vbnv_crc(uint8_t *vbnv_copy);
 int get_recovery_mode_from_vbnv(void);
 void set_recovery_mode_into_vbnv(int recovery_reason);
 int vboot_wants_oprom(void);
diff --git a/src/vboot/vbnv_cmos.c b/src/vboot/vbnv_cmos.c
index 5eda8e6..b7ef3e7 100644
--- a/src/vboot/vbnv_cmos.c
+++ b/src/vboot/vbnv_cmos.c
@@ -20,6 +20,22 @@
 #include <vboot/vbnv.h>
 #include <vboot/vbnv_layout.h>
 
+static void clear_vbnv_battery_cutoff_flag(uint8_t *vbnv_copy)
+{
+	/*
+	 * Currently battery cutoff is done in payload stage, which does not
+	 * update backup VBNV. And doing battery cutoff will invalidate CMOS.
+	 * This means for every reboot after cutoff, read_vbnv_cmos will reload
+	 * backup VBNV and try to cutoff again, causing endless reboot loop.
+	 * So we should always clear battery cutoff flag from loaded backup.
+	 */
+	if (vbnv_copy[MISC_FLAGS_OFFSET] & MISC_FLAGS_BATTERY_CUTOFF_MASK) {
+		printk(BIOS_INFO, "VBNV: Remove battery cut-off request.\n");
+		vbnv_copy[MISC_FLAGS_OFFSET] &= ~MISC_FLAGS_BATTERY_CUTOFF_MASK;
+		regen_vbnv_crc(vbnv_copy);
+	}
+}
+
 void read_vbnv_cmos(uint8_t *vbnv_copy)
 {
 	int i;
@@ -35,6 +51,7 @@ void read_vbnv_cmos(uint8_t *vbnv_copy)
 		read_vbnv_flash(vbnv_copy);
 
 		if (verify_vbnv(vbnv_copy)) {
+			clear_vbnv_battery_cutoff_flag(vbnv_copy);
 			save_vbnv_cmos(vbnv_copy);
 			printk(BIOS_INFO, "VBNV: Flash backup restored\n");
 		} else {
diff --git a/src/vboot/vbnv_layout.h b/src/vboot/vbnv_layout.h
index 59acd0c..1dc01c9 100644
--- a/src/vboot/vbnv_layout.h
+++ b/src/vboot/vbnv_layout.h
@@ -41,6 +41,9 @@
 #define DEV_BOOT_USB_MASK               0x01
 #define DEV_BOOT_SIGNED_ONLY_MASK       0x02
 
+#define MISC_FLAGS_OFFSET            8
+#define MISC_FLAGS_BATTERY_CUTOFF_MASK  0x08
+
 #define KERNEL_FIELD_OFFSET         11
 #define CRC_OFFSET                  15
 



More information about the coreboot-gerrit mailing list