[coreboot-gerrit] New patch to review for coreboot: vbnv: check alignment of nvram in advance

[Patrick Georgi (pgeorgi@google.com)]

Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/11654

-gerrit

commit cf784386b64bb61fd46079ec241b481c4f01f388
Author: Daisuke Nojiri <dnojiri at chromium.org>
Date:   Fri Sep 4 14:32:49 2015 -0700

    vbnv: check alignment of nvram in advance
    
    Currently, erase operation only works if the region is sector-aligned.
    These asserts ensure we can erase the region when it's all used up.
    
    Erase operation can be updated to handle unaligned erases by read,
    update, write-back cycle. However, these asserts will still remain useful
    in case the adjacent region contains critical data and mis-updating it
    can cause a critical failure.
    
    Additionaly we should write a FAFT test but it's more reliable to catch
    it here since FAFT can fail in many ways.
    
    BUG=none
    BRANCH=master
    TEST=tested on samus using misaligned nvram region
    
    Change-Id: I3add4671ed354d9763e21bf96616c8aeca0cb777
    Signed-off-by: Patrick Georgi <pgeorgi at chromium.org>
    Original-Commit-Id: fc001a4d3446cf96b76367dde492c3453aa948c6
    Original-Change-Id: Ib4df8f620bf7531b345364fa4c3e274aba09f677
    Original-Signed-off-by: Daisuke Nojiri <dnojiri at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/297801
---
 src/vendorcode/google/chromeos/vbnv_flash.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/vendorcode/google/chromeos/vbnv_flash.c b/src/vendorcode/google/chromeos/vbnv_flash.c
index 656c3ea..0ee9eaa 100644
--- a/src/vendorcode/google/chromeos/vbnv_flash.c
+++ b/src/vendorcode/google/chromeos/vbnv_flash.c
@@ -19,6 +19,7 @@
  * TODO: Make this CAR-friendly in case we use it on x86 some day.
  */
 
+#include <assert.h>
 #include <console/console.h>
 #include <spi_flash.h>
 #include <string.h>
@@ -111,6 +112,23 @@ static int init_vbnv(void)
 	return 0;
 }
 
+static void vbnv_is_erasable(void)
+{
+	/*
+	 * We check whether the region is aligned or not in advance to ensure
+	 * we can erase the region when it's all used up.
+	 *
+	 * The region offset & size are determined by fmap.dts yet the check can
+	 * be confidently done only by the spi flash driver. We use the same
+	 * check as the one used by spi_flash_cmd_erase, which happens to be
+	 * common to all the spi flash parts we support.
+	 *
+	 * TODO: Check by calling can_erase implemented by each spi flash driver
+	 */
+	assert(!(region_device_offset(&nvram_region) % spi_flash->sector_size));
+	assert(!(region_device_sz(&nvram_region) % spi_flash->sector_size));
+}
+
 static int vbnv_flash_probe(void)
 {
 	if (!spi_flash) {
@@ -119,6 +137,11 @@ static int vbnv_flash_probe(void)
 			printk(BIOS_ERR, "failed to probe spi flash\n");
 			return 1;
 		}
+		/*
+		 * Called here instead of init_vbnv to reduce impact on boot
+		 * speed.
+		 */
+		vbnv_is_erasable();
 	}
 	return 0;
 }