[coreboot-gerrit] New patch to review for coreboot: 5aa4bf0 elog: Correct behavior when FMAP section doesn't exist on ChromeOS

[Stefan Reinauer (stefan.reinauer@coreboot.org)]

Stefan Reinauer (stefan.reinauer at coreboot.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/9557

-gerrit

commit 5aa4bf02815a5cc6dd404d125b2370877486bd4c
Author: Julius Werner <jwerner at chromium.org>
Date:   Tue Jan 27 15:40:47 2015 -0800

    elog: Correct behavior when FMAP section doesn't exist on ChromeOS
    
    The elog driver has a really stupid bug that checks a result which is
    stored in an unsigned variable for < 0. Surprisingly GCC does not catch
    this nonsense right now, and I spent an hour trying out different
    warning options without finding one that doesn't also bring a load of
    stupid and unavoidable false positives (the biggest offender being
    -Wtype-limits, which does exactly what we'd want except for flagging
    things like if ((u8)var >= CONFIG_VAR_MIN) where the VAR_MIN Kconfig may
    or may not be 0).
    
    So, the only thing we can do is fix this one and wait for the next time
    something like that blows up. -.- Also change some more code to make the
    behavior more explicit (the old code already intended to work this way
    since flash_base is statically initialized to 0, never assigned in the
    error path and checked later in elog_init()... but there was an error
    message that incorrectly claimed a different fallback behavior, and
    explicitly assigning the values makes this easier to see). Finally, add
    another state to the elog_initialized variable to avoid trying to
    reinitialize a broken eventlog on every event (if it doesn't work the
    first time, chances are that it won't work later on during the same boot
    either).
    
    BRANCH=None
    BUG=chrome-os-partner:35940
    TEST=Flashed Jerry with RO 6588.4 and RW 6588.23, observed how it now
    cleanly enters recovery mode without blowing its bootblock away with
    stray eventlog entries.
    
    Change-Id: I0e5348ba961ce4835c30f7108a2453522095f2ee
    Signed-off-by: Stefan Reinauer <reinauer at chromium.org>
    Original-Commit-Id: f9798dbf0c2b2e337062ecd84d0f45434343c0d9
    Original-Change-Id: I4d93f48d2d01d75a04550d419e023aa42ca95a7a
    Original-Signed-off-by: Julius Werner <jwerner at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/243671
    Original-Reviewed-by: Duncan Laurie <dlaurie at chromium.org>
---
 src/drivers/elog/elog.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/drivers/elog/elog.c b/src/drivers/elog/elog.c
index f6b0006..76e8cf7 100644
--- a/src/drivers/elog/elog.c
+++ b/src/drivers/elog/elog.c
@@ -75,9 +75,13 @@ static elog_event_buffer_state event_buffer_state;
 static u16 next_event_offset;
 static u16 event_count;
 
-static int elog_initialized;
 static struct spi_flash *elog_spi;
 
+static enum {
+	ELOG_UNINITIALIZED = 0,
+	ELOG_INITIALIZED,
+	ELOG_BROKEN,
+} elog_initialized = ELOG_UNINITIALIZED;
 
 static inline u32 get_rom_size(void)
 {
@@ -522,25 +526,18 @@ int elog_clear(void)
 
 static void elog_find_flash(void)
 {
-#if CONFIG_CHROMEOS
-	u8 *flash_base_ptr;
-#endif
-
 	elog_debug("elog_find_flash()\n");
 
 #if CONFIG_CHROMEOS
 	/* Find the ELOG base and size in FMAP */
-	total_size = find_fmap_entry("RW_ELOG", (void **)&flash_base_ptr);
-	if (total_size < 0) {
-		printk(BIOS_WARNING, "ELOG: Unable to find RW_ELOG in FMAP, "
-		       "using CONFIG_ELOG_FLASH_BASE instead\n");
-		total_size = CONFIG_ELOG_AREA_SIZE;
+	u8 *flash_base_ptr;
+	int fmap_size = find_fmap_entry("RW_ELOG", (void **)&flash_base_ptr);
+	if (fmap_size < 0) {
+		printk(BIOS_WARNING, "ELOG: Unable to find RW_ELOG in FMAP\n");
+		flash_base = total_size = 0;
 	} else {
 		flash_base = elog_flash_address_to_offset(flash_base_ptr);
-
-		/* Use configured size if smaller than FMAP size */
-		if (total_size > CONFIG_ELOG_AREA_SIZE)
-			total_size = CONFIG_ELOG_AREA_SIZE;
+		total_size = MIN(fmap_size, CONFIG_ELOG_AREA_SIZE);
 	}
 #else
 	flash_base = CONFIG_ELOG_FLASH_BASE;
@@ -554,8 +551,15 @@ static void elog_find_flash(void)
  */
 int elog_init(void)
 {
-	if (elog_initialized)
+	switch (elog_initialized) {
+	case ELOG_UNINITIALIZED:
+		break;
+	case ELOG_INITIALIZED:
 		return 0;
+	case ELOG_BROKEN:
+		return -1;
+	}
+	elog_initialized = ELOG_BROKEN;
 
 	elog_debug("elog_init()\n");
 
@@ -600,8 +604,6 @@ int elog_init(void)
 		return -1;
 	}
 
-	elog_initialized = 1;
-
 	printk(BIOS_INFO, "ELOG: FLASH @0x%p [SPI 0x%08x]\n",
 	       elog_area, flash_base);
 
@@ -637,6 +639,8 @@ int elog_init(void)
 #endif
 #endif
 
+	elog_initialized = ELOG_INITIALIZED;
+
 	return 0;
 }