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: I4d93f48d2d01d75a04550d419e023aa42ca95a7a
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/243671
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
diff --git a/src/drivers/elog/elog.c b/src/drivers/elog/elog.c
index 353406e..6d3c069 100644
--- a/src/drivers/elog/elog.c
+++ b/src/drivers/elog/elog.c
@@ -75,9 +75,13 @@
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 @@
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 @@
*/
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 @@
return -1;
}
- elog_initialized = 1;
-
printk(BIOS_INFO, "ELOG: FLASH @0x%p [SPI 0x%08x]\n",
elog_area, flash_base);
@@ -637,6 +639,8 @@
#endif
#endif
+ elog_initialized = ELOG_INITIALIZED;
+
return 0;
}
