[coreboot-gerrit] New patch to review for coreboot: vbnv: Do not initialize vbnv_copy in vbnv layer

[Furquan Shaikh (furquan@google.com)]

Furquan Shaikh (furquan at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15498

-gerrit

commit 3afe52e05ed70baf971ccc98a52df8b8bde74b64
Author: Furquan Shaikh <furquan at google.com>
Date:   Wed Jun 29 11:26:27 2016 -0700

    vbnv: Do not initialize vbnv_copy in vbnv layer
    
    If read_vbnv finds that the vbnv_copy is not valid, it initializes it
    with the correct HEADER_SIGNATURE and other attributes. However, the
    vbnv copy is checked for validity and initialized at the vboot layer as
    well. Since, vboot is the owner of this data, it should be the one
    initializing it. Thus, if read_vbnv sees that the data is not valid,
    simply reset it to all 0s and let vboot layer take care of it. This also
    removes the need for additional checks to ensure that the dirty vbnv
    copy is properly updated on storage.
    
    Change-Id: I6101ac41f31f720a6e357c9c56e571d62e0f2f47
    Signed-off-by: Furquan Shaikh <furquan at google.com>
---
 src/vendorcode/google/chromeos/vbnv.c               | 14 +++-----------
 src/vendorcode/google/chromeos/vbnv.h               |  7 +------
 src/vendorcode/google/chromeos/vboot2/vboot_logic.c |  8 ++------
 3 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/src/vendorcode/google/chromeos/vbnv.c b/src/vendorcode/google/chromeos/vbnv.c
index baccb23..d900775 100644
--- a/src/vendorcode/google/chromeos/vbnv.c
+++ b/src/vendorcode/google/chromeos/vbnv.c
@@ -63,10 +63,6 @@ static uint8_t crc8_vbnv(const uint8_t *data, int len)
 static void reset_vbnv(uint8_t *vbnv_copy)
 {
 	memset(vbnv_copy, 0, VBNV_BLOCK_SIZE);
-	vbnv_copy[HEADER_OFFSET] = HEADER_SIGNATURE |
-				   HEADER_FIRMWARE_SETTINGS_RESET |
-				   HEADER_KERNEL_SETTINGS_RESET;
-	vbnv_copy[CRC_OFFSET] = crc8_vbnv(vbnv_copy, CRC_OFFSET);
 }
 
 /* Read VBNV data into cache. */
@@ -88,9 +84,8 @@ int verify_vbnv(uint8_t *vbnv_copy)
 /*
  * Read VBNV data from configured storage backend.
  * If VBNV verification fails, reset the vbnv copy.
- * Returns 1 if write-back of vbnv copy is required. Else, returns 0.
  */
-int read_vbnv(uint8_t *vbnv_copy)
+void read_vbnv(uint8_t *vbnv_copy)
 {
 	if (IS_ENABLED(CONFIG_CHROMEOS_VBNV_CMOS))
 		read_vbnv_cmos(vbnv_copy);
@@ -100,11 +95,8 @@ int read_vbnv(uint8_t *vbnv_copy)
 		read_vbnv_flash(vbnv_copy);
 
 	/* Check data for consistency */
-	if (verify_vbnv(vbnv_copy))
-		return 0;
-
-	reset_vbnv(vbnv_copy);
-	return 1;
+	if (!verify_vbnv(vbnv_copy))
+		reset_vbnv(vbnv_copy);
 }
 
 /*
diff --git a/src/vendorcode/google/chromeos/vbnv.h b/src/vendorcode/google/chromeos/vbnv.h
index a66d687..5d21cc8 100644
--- a/src/vendorcode/google/chromeos/vbnv.h
+++ b/src/vendorcode/google/chromeos/vbnv.h
@@ -19,12 +19,7 @@
 #include <types.h>
 
 /* Generic functions */
-/*
- * Return value for read_vbnv:
- * 1 = write-back of vbnv copy is required.
- * 0 = otherwise
- */
-int read_vbnv(uint8_t *vbnv_copy);
+void read_vbnv(uint8_t *vbnv_copy);
 void save_vbnv(const uint8_t *vbnv_copy);
 int verify_vbnv(uint8_t *vbnv_copy);
 int get_recovery_mode_from_vbnv(void);
diff --git a/src/vendorcode/google/chromeos/vboot2/vboot_logic.c b/src/vendorcode/google/chromeos/vboot2/vboot_logic.c
index 116c949..4c799c9 100644
--- a/src/vendorcode/google/chromeos/vboot2/vboot_logic.c
+++ b/src/vendorcode/google/chromeos/vboot2/vboot_logic.c
@@ -301,12 +301,8 @@ void verstage_main(void)
 	/* Set up context and work buffer */
 	vb2_init_work_context(&ctx);
 
-	/*
-	 * Read nvdata from a non-volatile storage and mark data as changed
-	 * if instructed.
-	 */
-	if (read_vbnv(ctx.nvdata))
-		ctx.flags |= VB2_CONTEXT_NVDATA_CHANGED;
+	/* Read nvdata from a non-volatile storage. */
+	read_vbnv(ctx.nvdata);
 
 	/* Set S3 resume flag if vboot should behave differently when selecting
 	 * which slot to boot.  This is only relevant to vboot if the platform