From a80a79f9f5ca0250eb31330d9dfdacf3a250efb0 Mon Sep 17 00:00:00 2001
From: Randall Spangler <rspangler@chromium.org>
Date: Mon, 26 Feb 2018 17:01:24 -0800
Subject: 2lib: Add support for 64-byte nvstorage record

The calling firmware can set ctx->flags VB2_CONTEXT_NVDATA_V2 to tell
vboot that nvdata is a 64-byte record instead of a 16-byte record, or
equivalently, set the VBSD_NVDATA_V2 flag if calling the old vboot1
API.

If calling firmware does not (which is the current coreboot and
depthcharge default), then the 16-byte record is used, and V2 fields
return explicit default values.

Added the fw_max_rollforward V2 field, which defaults to 0xfffffffe on
V1.  This will be used by a subsequent CL.

Added unit tests to verify all that.

Added crossystem support, though it will only work with the current
16-byte records until firmware sets the VBSD flag and mosys supports
larger records.

(Note that because coreboot/depthcharge do not yet set the new context
flag, this CL should not change ToT firmware behavior.)

See go/vboot-nvstorage for design doc.

BUG=chromium:789276
BRANCH=none
TEST=make runtests

Change-Id: I43072ef153dfa016c051f560892af1fbb3508e3a
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/942031
---
 firmware/2lib/2nvstorage.c                | 48 +++++++++++++--
 firmware/2lib/include/2api.h              | 21 ++++++-
 firmware/2lib/include/2nvstorage.h        | 28 ++++++++-
 firmware/2lib/include/2nvstorage_fields.h | 44 +++++++++++---
 firmware/include/vboot_struct.h           |  2 +
 firmware/lib/vboot_api_kernel.c           |  8 ++-
 firmware/lib/vboot_display.c              |  2 +-
 host/arch/arm/lib/crossystem_arch.c       |  4 +-
 host/arch/x86/lib/crossystem_arch.c       | 10 ++--
 host/lib/crossystem.c                     | 31 ++++++++--
 tests/vb2_nvstorage_tests.c               | 98 ++++++++++++++++++++++++-------
 tests/vboot_api_kernel4_tests.c           |  4 +-
 tests/vboot_api_kernel5_tests.c           |  2 +-
 13 files changed, 247 insertions(+), 55 deletions(-)

diff --git a/firmware/2lib/2nvstorage.c b/firmware/2lib/2nvstorage.c
index 71a0dbb9..1e24d2d7 100644
--- a/firmware/2lib/2nvstorage.c
+++ b/firmware/2lib/2nvstorage.c
@@ -14,10 +14,19 @@
 
 static void vb2_nv_regen_crc(struct vb2_context *ctx)
 {
-	ctx->nvdata[VB2_NV_OFFS_CRC] = vb2_crc8(ctx->nvdata, VB2_NV_OFFS_CRC);
+	const int offs = ctx->flags & VB2_CONTEXT_NVDATA_V2 ?
+			VB2_NV_OFFS_CRC_V2 : VB2_NV_OFFS_CRC_V1;
+
+	ctx->nvdata[offs] = vb2_crc8(ctx->nvdata, offs);
 	ctx->flags |= VB2_CONTEXT_NVDATA_CHANGED;
 }
 
+int vb2_nv_get_size(const struct vb2_context *ctx)
+{
+	return ctx->flags & VB2_CONTEXT_NVDATA_V2 ?
+			VB2_NVDATA_SIZE_V2 : VB2_NVDATA_SIZE;
+}
+
 /**
  * Check the CRC of the non-volatile storage context.
  *
@@ -32,14 +41,17 @@ static void vb2_nv_regen_crc(struct vb2_context *ctx)
 int vb2_nv_check_crc(const struct vb2_context *ctx)
 {
 	const uint8_t *p = ctx->nvdata;
+	const int offs = ctx->flags & VB2_CONTEXT_NVDATA_V2 ?
+			VB2_NV_OFFS_CRC_V2 : VB2_NV_OFFS_CRC_V1;
+	const int sig = ctx->flags & VB2_CONTEXT_NVDATA_V2 ?
+			VB2_NV_HEADER_SIGNATURE_V2 : VB2_NV_HEADER_SIGNATURE_V1;
 
 	/* Check header */
-	if (VB2_NV_HEADER_SIGNATURE !=
-	    (p[VB2_NV_OFFS_HEADER] & VB2_NV_HEADER_MASK))
+	if (sig != (p[VB2_NV_OFFS_HEADER] & VB2_NV_HEADER_SIGNATURE_MASK))
 		return VB2_ERROR_NV_HEADER;
 
 	/* Check CRC */
-	if (vb2_crc8(p, VB2_NV_OFFS_CRC) != p[VB2_NV_OFFS_CRC])
+	if (vb2_crc8(p, offs) != p[offs])
 		return VB2_ERROR_NV_CRC;
 
 	return VB2_SUCCESS;
@@ -47,14 +59,17 @@ int vb2_nv_check_crc(const struct vb2_context *ctx)
 
 void vb2_nv_init(struct vb2_context *ctx)
 {
+	const int sig = ctx->flags & VB2_CONTEXT_NVDATA_V2 ?
+			VB2_NV_HEADER_SIGNATURE_V2 : VB2_NV_HEADER_SIGNATURE_V1;
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 	uint8_t *p = ctx->nvdata;
 
+
 	/* Check data for consistency */
 	if (vb2_nv_check_crc(ctx) != VB2_SUCCESS) {
 		/* Data is inconsistent (bad CRC or header); reset defaults */
-		memset(p, 0, VB2_NVDATA_SIZE);
-		p[VB2_NV_OFFS_HEADER] = (VB2_NV_HEADER_SIGNATURE |
+		memset(p, 0, VB2_NVDATA_SIZE_V2);
+		p[VB2_NV_OFFS_HEADER] = (sig |
 					 VB2_NV_HEADER_FW_SETTINGS_RESET |
 					 VB2_NV_HEADER_KERNEL_SETTINGS_RESET);
 
@@ -190,6 +205,16 @@ uint32_t vb2_nv_get(struct vb2_context *ctx, enum vb2_nv_param param)
 			| (p[VB2_NV_OFFS_KERNEL_MAX_ROLLFORWARD2] << 8)
 			| (p[VB2_NV_OFFS_KERNEL_MAX_ROLLFORWARD3] << 16)
 			| (p[VB2_NV_OFFS_KERNEL_MAX_ROLLFORWARD4] << 24));
+
+	case VB2_NV_FW_MAX_ROLLFORWARD:
+		/* Field only present in V2 */
+		if (!(ctx->flags & VB2_CONTEXT_NVDATA_V2))
+			return VB2_FW_MAX_ROLLFORWARD_V1_DEFAULT;
+
+		return (p[VB2_NV_OFFS_FW_MAX_ROLLFORWARD1]
+			| (p[VB2_NV_OFFS_FW_MAX_ROLLFORWARD2] << 8)
+			| (p[VB2_NV_OFFS_FW_MAX_ROLLFORWARD3] << 16)
+			| (p[VB2_NV_OFFS_FW_MAX_ROLLFORWARD4] << 24));
 	}
 
 	/*
@@ -376,6 +401,17 @@ void vb2_nv_set(struct vb2_context *ctx,
 		p[VB2_NV_OFFS_KERNEL_MAX_ROLLFORWARD3] = (uint8_t)(value >> 16);
 		p[VB2_NV_OFFS_KERNEL_MAX_ROLLFORWARD4] = (uint8_t)(value >> 24);
 		break;
+
+	case VB2_NV_FW_MAX_ROLLFORWARD:
+		/* Field only present in V2 */
+		if (!(ctx->flags & VB2_CONTEXT_NVDATA_V2))
+			return;
+
+		p[VB2_NV_OFFS_FW_MAX_ROLLFORWARD1] = (uint8_t)(value);
+		p[VB2_NV_OFFS_FW_MAX_ROLLFORWARD2] = (uint8_t)(value >> 8);
+		p[VB2_NV_OFFS_FW_MAX_ROLLFORWARD3] = (uint8_t)(value >> 16);
+		p[VB2_NV_OFFS_FW_MAX_ROLLFORWARD4] = (uint8_t)(value >> 24);
+		break;
 	}
 
 	/*
diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h
index 1e6fada4..f338fa41 100644
--- a/firmware/2lib/include/2api.h
+++ b/firmware/2lib/include/2api.h
@@ -28,8 +28,15 @@
 #include "2recovery_reasons.h"
 #include "2return_codes.h"
 
-/* Size of non-volatile data used by vboot */
+/*
+ * Size of non-volatile data used by vboot.
+ *
+ * If you only support non-volatile data format V1, then use VB2_NVDATA_SIZE.
+ * If you support V2, use VB2_NVDATA_SIZE_V2 and set context flag
+ * VB2_CONTEXT_NVDATA_V2.
+ */
 #define VB2_NVDATA_SIZE 16
+#define VB2_NVDATA_SIZE_V2 64
 
 /* Size of secure data spaces used by vboot */
 #define VB2_SECDATA_SIZE 10
@@ -158,6 +165,16 @@ enum vb2_context_flags {
 	 * software sync.
 	 */
 	VB2_CONTEXT_EC_EFS = (1 << 17),
+
+	/*
+	 * NV storage uses data format V2.  Data is size VB2_NVDATA_SIZE_V2,
+	 * not VB2_NVDATA_SIZE.
+	 *
+	 * Caller must set this flag when initializing the context to use V2.
+	 * (Vboot cannot infer the data size from the data itself, because the
+	 * data provided by the caller could be uninitialized.)
+	 */
+	VB2_CONTEXT_NVDATA_V2 = (1 << 18),
 };
 
 /*
@@ -190,7 +207,7 @@ struct vb2_context {
 	 * vb2api function returns, caller must save the data back to the
 	 * non-volatile location and then clear the flag.
 	 */
-	uint8_t nvdata[VB2_NVDATA_SIZE];
+	uint8_t nvdata[VB2_NVDATA_SIZE_V2];
 
 	/*
 	 * Secure data for firmware verification stage.  Caller must fill this
diff --git a/firmware/2lib/include/2nvstorage.h b/firmware/2lib/include/2nvstorage.h
index 1a654691..654bcab1 100644
--- a/firmware/2lib/include/2nvstorage.h
+++ b/firmware/2lib/include/2nvstorage.h
@@ -105,6 +105,14 @@ enum vb2_nv_param {
         VB2_NV_BATTERY_CUTOFF_REQUEST,
 	/* Maximum kernel version to roll forward to */
 	VB2_NV_KERNEL_MAX_ROLLFORWARD,
+
+	/*** Fields only available in NV storage V2 ***/
+
+	/*
+	 * Maximum firmware version to roll forward to.  Returns
+	 * VB2_MAX_ROLLFORWARD_MAX_V1_DEFAULT for V1.
+	 */
+	VB2_NV_FW_MAX_ROLLFORWARD,
 };
 
 /* Set default boot in developer mode */
@@ -113,7 +121,7 @@ enum vb2_dev_default_boot {
 	VB2_DEV_DEFAULT_BOOT_DISK = 0,
 
 	/* Default to boot from USB */
-	VB2_DEV_DEFAULT_BOOT_USB= 1,
+	VB2_DEV_DEFAULT_BOOT_USB = 1,
 
 	/* Default to boot legacy OS */
 	VB2_DEV_DEFAULT_BOOT_LEGACY = 2,
@@ -135,6 +143,24 @@ enum vb2_fw_result {
 	VB2_FW_RESULT_FAILURE = 3,
 };
 
+/*
+ * Default value for VB2_NV_FIRMWARE_MAX_ROLLFORWARD on V1.  This preserves the
+ * existing behavior that V1 systems will always roll forward the firmware
+ * version when possible.
+ */
+#define VB2_FW_MAX_ROLLFORWARD_V1_DEFAULT 0xfffffffe
+
+/**
+ * Return the size of the non-volatile storage data in the context.
+ *
+ * This may be called before vb2_context_init(), but you must set
+ * VB2_CONTEXT_NVDATA_V2 if you support V2 record size.
+ *
+ * @param ctx		Context pointer
+ * @return Size of the non-volatile storage data in bytes.
+ */
+int vb2_nv_get_size(const struct vb2_context *ctx);
+
 /**
  * Check the CRC of the non-volatile storage context.
  *
diff --git a/firmware/2lib/include/2nvstorage_fields.h b/firmware/2lib/include/2nvstorage_fields.h
index 0ed3325a..d67ad5d4 100644
--- a/firmware/2lib/include/2nvstorage_fields.h
+++ b/firmware/2lib/include/2nvstorage_fields.h
@@ -11,15 +11,16 @@
 /*
  * Constants for NV storage.  We use this rather than structs and bitfields so
  * the data format is consistent across platforms and compilers.  Total NV
- * storage size is VB2_NVDATA_SIZE = 16 bytes.
+ * storage size is:
  *
- * These constants must match the equivalent constants in
- * lib/vboot_nvstorage.c.  (We currently don't share a common header file
- * because we're tring to keep the two libs independent, and we hope to
- * deprecate that one.)
+ * Version 1: VB2_NVDATA_SIZE = 16 bytes
+ * Version 2: VB2_NVDATA_SIZE_V2 = 64 bytes
+ *
+ * Unused bits/bytes must be set to 0.
  */
 
 enum vb2_nv_offset {
+	/*** The following fields are present in all versions ***/
 	VB2_NV_OFFS_HEADER = 0,
 	VB2_NV_OFFS_BOOT = 1,
 	VB2_NV_OFFS_RECOVERY = 2,
@@ -35,16 +36,41 @@ enum vb2_nv_offset {
 	VB2_NV_OFFS_KERNEL2 = 12, /* bits 8-15 of 16 */
 	VB2_NV_OFFS_KERNEL_MAX_ROLLFORWARD3 = 13, /* bits 16-23 of 32 */
 	VB2_NV_OFFS_KERNEL_MAX_ROLLFORWARD4 = 14, /* bits 24-31 of 32 */
+
+	/*
+	 * CRC_V1 must be last field in V1.  This byte can be reused in later
+	 * versions.
+	 */
+	VB2_NV_OFFS_CRC_V1 = 15,
+
+	/* The following fields are only present in V2+ */
+	VB2_NV_OFFS_RESERVED_V2 = 15, /* Was CRC in V1 (unused = 0xff) */
+	VB2_NV_OFFS_FW_MAX_ROLLFORWARD1 = 16, /* bits 0-7 of 32 */
+	VB2_NV_OFFS_FW_MAX_ROLLFORWARD2 = 17, /* bits 8-15 of 32 */
+	VB2_NV_OFFS_FW_MAX_ROLLFORWARD3 = 18, /* bits 16-23 of 32 */
+	VB2_NV_OFFS_FW_MAX_ROLLFORWARD4 = 19, /* bits 24-31 of 32 */
+
 	/* CRC must be last field */
-	VB2_NV_OFFS_CRC = 15
+	VB2_NV_OFFS_CRC_V2 = 63,
  };
 
-/* Fields in VB2_NV_OFFS_HEADER (unused = 0x07) */
+/* Fields in VB2_NV_OFFS_HEADER (unused = 0x04) */
 #define VB2_NV_HEADER_WIPEOUT		       0x08
 #define VB2_NV_HEADER_KERNEL_SETTINGS_RESET    0x10
 #define VB2_NV_HEADER_FW_SETTINGS_RESET        0x20
-#define VB2_NV_HEADER_SIGNATURE                0x40
-#define VB2_NV_HEADER_MASK                     0xc0
+#define VB2_NV_HEADER_SIGNATURE_MASK           0xc3
+
+/*
+ * Valid signature values.  Note that V1 readers only looked at mask 0xc0, and
+ * expect it to have value 0x40.  Versions != V1 must have the top two bits !=
+ * 0x40 so old readers will reject the data.  Using all 1-bits or all 0-bits is
+ * also discouraged, because that is indistinguishable from all-erased data on
+ * some platforms.
+ */
+/* Version 1 = 16-byte record */
+#define VB2_NV_HEADER_SIGNATURE_V1             0x40
+/* Version 2 = 64-byte record */
+#define VB2_NV_HEADER_SIGNATURE_V2             0x03
 
 /* Fields in VB2_NV_OFFS_BOOT */
 #define VB2_NV_BOOT_TRY_COUNT_MASK             0x0f
diff --git a/firmware/include/vboot_struct.h b/firmware/include/vboot_struct.h
index 6036dc8f..fd5777a2 100644
--- a/firmware/include/vboot_struct.h
+++ b/firmware/include/vboot_struct.h
@@ -246,6 +246,8 @@ typedef struct VbKernelPreambleHeader {
 #define VBSD_NOFAIL_BOOT                 0x00040000
 /* VbInit() was told that the EC firmware supports EFS */
 #define VBSD_EC_EFS                      0x00080000
+/* NvStorage uses 64-byte record, not 16-byte */
+#define VBSD_NVDATA_V2                   0x00100000
 
 /*
  * Supported flags by header version.  It's ok to add new flags while keeping
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index f5509892..8016fae0 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -237,9 +237,6 @@ static VbError_t vb2_kernel_setup(VbCommonParams *cparams,
 	 */
 	memset(&ctx, 0, sizeof(ctx));
 
-	VbExNvStorageRead(ctx.nvdata);
-	vb2_nv_init(&ctx);
-
 	/* Translate vboot1 flags back to vboot2 */
 	if (shared->recovery_reason)
 		ctx.flags |= VB2_CONTEXT_RECOVERY_MODE;
@@ -259,6 +256,11 @@ static VbError_t vb2_kernel_setup(VbCommonParams *cparams,
 		ctx.flags |= VB2_CONTEXT_EC_SYNC_SLOW;
 	if (shared->flags & VBSD_EC_EFS)
 		ctx.flags |= VB2_CONTEXT_EC_EFS;
+	if (shared->flags & VBSD_NVDATA_V2)
+		ctx.flags |= VB2_CONTEXT_NVDATA_V2;
+
+	VbExNvStorageRead(ctx.nvdata);
+	vb2_nv_init(&ctx);
 
 	ctx.workbuf_size = VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE +
 			VB2_WORKBUF_ALIGN;
diff --git a/firmware/lib/vboot_display.c b/firmware/lib/vboot_display.c
index ea36c1f1..6d6f7abe 100644
--- a/firmware/lib/vboot_display.c
+++ b/firmware/lib/vboot_display.c
@@ -319,7 +319,7 @@ VbError_t VbDisplayDebugInfo(struct vb2_context *ctx)
 
 	/* Add raw contents of VbNvStorage */
 	used += StrnAppend(buf + used, "\nVbNv.raw:", DEBUG_INFO_SIZE - used);
-	for (i = 0; i < VBNV_BLOCK_SIZE; i++) {
+	for (i = 0; i < vb2_nv_get_size(ctx); i++) {
 		used += StrnAppend(buf + used, " ", DEBUG_INFO_SIZE - used);
 		used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
 				       ctx->nvdata[i], 16, 2);
diff --git a/host/arch/arm/lib/crossystem_arch.c b/host/arch/arm/lib/crossystem_arch.c
index 0a308416..d56f4e2e 100644
--- a/host/arch/arm/lib/crossystem_arch.c
+++ b/host/arch/arm/lib/crossystem_arch.c
@@ -260,7 +260,7 @@ static int vb2_read_nv_storage_disk(struct vb2_context *ctx)
 		return E_FAIL;
 	snprintf(nvctx_path, sizeof(nvctx_path), NVCTX_PATH, emmc_dev);
 
-	if (size != sizeof(ctx->nvdata) || (size + offset > SECTOR_SIZE))
+	if (size != vb2_nv_get_size(ctx) || (size + offset > SECTOR_SIZE))
 		return E_FAIL;
 
 	nvctx_fd = open(nvctx_path, O_RDONLY);
@@ -303,7 +303,7 @@ static int vb2_write_nv_storage_disk(struct vb2_context *ctx)
 		return E_FAIL;
 	snprintf(nvctx_path, sizeof(nvctx_path), NVCTX_PATH, emmc_dev);
 
-	if (size != sizeof(ctx->nvdata) || (size + offset > SECTOR_SIZE))
+	if (size != vb2_nv_get_size(ctx) || (size + offset > SECTOR_SIZE))
 		return E_FAIL;
 
 	do {
diff --git a/host/arch/x86/lib/crossystem_arch.c b/host/arch/x86/lib/crossystem_arch.c
index f026a14f..4921119e 100644
--- a/host/arch/x86/lib/crossystem_arch.c
+++ b/host/arch/x86/lib/crossystem_arch.c
@@ -158,16 +158,17 @@ static int VbCmosWrite(unsigned offs, size_t size, const void *ptr)
 int vb2_read_nv_storage(struct vb2_context *ctx)
 {
 	unsigned offs, blksz;
+	unsigned expectsz = vb2_nv_get_size(ctx);
 
 	/* Get the byte offset from VBNV */
 	if (ReadFileInt(ACPI_VBNV_PATH ".0", &offs) < 0)
 		return -1;
 	if (ReadFileInt(ACPI_VBNV_PATH ".1", &blksz) < 0)
 		return -1;
-	if (VBNV_BLOCK_SIZE > blksz)
+	if (expectsz > blksz)
 		return -1;  /* NV storage block is too small */
 
-	if (0 != VbCmosRead(offs, sizeof(ctx->nvdata), ctx->nvdata))
+	if (0 != VbCmosRead(offs, expectsz, ctx->nvdata))
 		return -1;
 
 	return 0;
@@ -177,6 +178,7 @@ int vb2_read_nv_storage(struct vb2_context *ctx)
 int vb2_write_nv_storage(struct vb2_context *ctx)
 {
 	unsigned offs, blksz;
+	unsigned expectsz = vb2_nv_get_size(ctx);
 
 	if (!(ctx->flags & VB2_CONTEXT_NVDATA_CHANGED))
 		return 0;  /* Nothing changed, so no need to write */
@@ -186,10 +188,10 @@ int vb2_write_nv_storage(struct vb2_context *ctx)
 		return -1;
 	if (ReadFileInt(ACPI_VBNV_PATH ".1", &blksz) < 0)
 		return -1;
-	if (VBNV_BLOCK_SIZE > blksz)
+	if (expectsz > blksz)
 		return -1;  /* NV storage block is too small */
 
-	if (0 != VbCmosWrite(offs, sizeof(ctx->nvdata), ctx->nvdata))
+	if (0 != VbCmosWrite(offs, expectsz, ctx->nvdata))
 		return -1;
 
 	/* Also attempt to write using mosys if using vboot2 */
diff --git a/host/lib/crossystem.c b/host/lib/crossystem.c
index 5a758616..38c9ed6e 100644
--- a/host/lib/crossystem.c
+++ b/host/lib/crossystem.c
@@ -98,11 +98,14 @@ static int vnc_read;
 
 int vb2_get_nv_storage(enum vb2_nv_param param)
 {
+	VbSharedDataHeader* sh = VbSharedDataRead();
 	static struct vb2_context cached_ctx;
 
 	/* TODO: locking around NV access */
 	if (!vnc_read) {
 		memset(&cached_ctx, 0, sizeof(cached_ctx));
+		if (sh->flags & VBSD_NVDATA_V2)
+			cached_ctx.flags |= VB2_CONTEXT_NVDATA_V2;
 		if (0 != vb2_read_nv_storage(&cached_ctx))
 			return -1;
 		vb2_nv_init(&cached_ctx);
@@ -118,10 +121,13 @@ int vb2_get_nv_storage(enum vb2_nv_param param)
 
 int vb2_set_nv_storage(enum vb2_nv_param param, int value)
 {
+	VbSharedDataHeader* sh = VbSharedDataRead();
 	struct vb2_context ctx;
 
 	/* TODO: locking around NV access */
 	memset(&ctx, 0, sizeof(ctx));
+	if (sh->flags & VBSD_NVDATA_V2)
+		ctx.flags |= VB2_CONTEXT_NVDATA_V2;
 	if (0 != vb2_read_nv_storage(&ctx))
 		return -1;
 	vb2_nv_init(&ctx);
@@ -834,18 +840,34 @@ static int ExecuteMosys(char * const argv[], char *buf, size_t bufsize)
 
 int vb2_read_nv_storage_mosys(struct vb2_context *ctx)
 {
-	char hexstring[VBNV_BLOCK_SIZE * 2 + 32];  /* Reserve extra 32 bytes */
+	/* Reserve extra 32 bytes */
+	char hexstring[VB2_NVDATA_SIZE_V2 * 2 + 32];
+	/*
+	 * TODO(rspangler): mosys doesn't know how to read anything but 16-byte
+	 * records yet.  When it grows a command line option to do that, call
+	 * it here when needed.
+	 *
+	 * It's possible mosys won't need that.  For example, if if examines
+	 * the header byte to determine the records size, or if it calls back
+	 * to crossystem to read the VBSD flag.
+	 */
 	char * const argv[] = {
 		InAndroid() ? MOSYS_ANDROID_PATH : MOSYS_CROS_PATH,
 		"nvram", "vboot", "read", NULL
 	};
 	char hexdigit[3];
+	const int nvsize = vb2_nv_get_size(ctx);
 	int i;
 
 	if (ExecuteMosys(argv, hexstring, sizeof(hexstring)))
 		return -1;
+	if (strlen(hexstring) != 2 * nvsize) {
+		fprintf(stderr, "mosys returned hex nvdata size %d"
+			" (expected %d)\n", (int)strlen(hexstring), 2 * nvsize);
+		return -1;
+	}
 	hexdigit[2] = '\0';
-	for (i = 0; i < VBNV_BLOCK_SIZE; i++) {
+	for (i = 0; i < nvsize; i++) {
 		hexdigit[0] = hexstring[i * 2];
 		hexdigit[1] = hexstring[i * 2 + 1];
 		ctx->nvdata[i] = strtol(hexdigit, NULL, 16);
@@ -855,14 +877,15 @@ int vb2_read_nv_storage_mosys(struct vb2_context *ctx)
 
 int vb2_write_nv_storage_mosys(struct vb2_context *ctx)
 {
-	char hexstring[VBNV_BLOCK_SIZE * 2 + 1];
+	char hexstring[VB2_NVDATA_SIZE_V2 * 2 + 1];
 	char * const argv[] = {
 		InAndroid() ? MOSYS_ANDROID_PATH : MOSYS_CROS_PATH,
 		"nvram", "vboot", "write", hexstring, NULL
 	};
+	const int nvsize = vb2_nv_get_size(ctx);
 	int i;
 
-	for (i = 0; i < VBNV_BLOCK_SIZE; i++)
+	for (i = 0; i < nvsize; i++)
 		snprintf(hexstring + i * 2, 3, "%02x", ctx->nvdata[i]);
 	hexstring[sizeof(hexstring) - 1] = '\0';
 	if (ExecuteMosys(argv, NULL, 0))
diff --git a/tests/vb2_nvstorage_tests.c b/tests/vb2_nvstorage_tests.c
index f509b0ab..b4d1d4b6 100644
--- a/tests/vb2_nvstorage_tests.c
+++ b/tests/vb2_nvstorage_tests.c
@@ -19,6 +19,7 @@
 #include "2common.h"
 #include "2misc.h"
 #include "2nvstorage.h"
+#include "2nvstorage_fields.h"
 
 /* Single NV storage field to test */
 struct nv_field {
@@ -63,6 +64,13 @@ static struct nv_field nvfields[] = {
 	{0, 0, 0, 0, NULL}
 };
 
+/* Fields added in v2.  The test_value field is the default returned for V1. */
+static struct nv_field nv2fields[] = {
+	{VB2_NV_FW_MAX_ROLLFORWARD, 0, VB2_FW_MAX_ROLLFORWARD_V1_DEFAULT,
+	 0x87654321, "firmware max rollforward"},
+	{0, 0, 0, 0, NULL}
+};
+
 static void test_changed(struct vb2_context *ctx, int changed, const char *why)
 {
 	if (changed)
@@ -71,58 +79,73 @@ static void test_changed(struct vb2_context *ctx, int changed, const char *why)
 		TEST_EQ(ctx->flags & VB2_CONTEXT_NVDATA_CHANGED, 0, why);
 };
 
-static void nv_storage_test(void)
+static void nv_storage_test(uint32_t ctxflags)
 {
 	struct nv_field *vnf;
 	uint8_t goodcrc;
 	uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
 		__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_context c = {
-		.flags = 0,
+		.flags = ctxflags,
 		.workbuf = workbuf,
 		.workbuf_size = sizeof(workbuf),
 	};
 	struct vb2_shared_data *sd = vb2_get_sd(&c);
 
+	/* Things that change between V1 and V2 */
+	int expect_header = 0x30 | (ctxflags ? VB2_NV_HEADER_SIGNATURE_V2 :
+				    VB2_NV_HEADER_SIGNATURE_V1);
+	int crc_offs = ctxflags ? VB2_NV_OFFS_CRC_V2 : VB2_NV_OFFS_CRC_V1;
+
+	TEST_EQ(vb2_nv_get_size(&c), ctxflags ? VB2_NVDATA_SIZE_V2 :
+				VB2_NVDATA_SIZE, "vb2_nv_get_size()");
+
 	memset(c.nvdata, 0xA6, sizeof(c.nvdata));
 	vb2_init_context(&c);
 
 	/* Init with invalid data should set defaults and regenerate CRC */
 	vb2_nv_init(&c);
-	TEST_EQ(c.nvdata[0], 0x70, "vb2_nv_init() reset header byte");
-	TEST_NEQ(c.nvdata[15], 0, "vb2_nv_init() CRC");
+	TEST_EQ(c.nvdata[VB2_NV_OFFS_HEADER], expect_header,
+		"vb2_nv_init() reset header byte");
+	TEST_NEQ(c.nvdata[crc_offs], 0, "vb2_nv_init() CRC");
 	TEST_EQ(sd->status, VB2_SD_STATUS_NV_INIT | VB2_SD_STATUS_NV_REINIT,
 		"vb2_nv_init() status changed");
 	test_changed(&c, 1, "vb2_nv_init() reset changed");
-	goodcrc = c.nvdata[15];
+	goodcrc = c.nvdata[crc_offs];
 	TEST_SUCC(vb2_nv_check_crc(&c), "vb2_nv_check_crc() good");
 
 	/* Another init should not cause further changes */
-	c.flags = 0;
+	c.flags = ctxflags;
 	sd->status = 0;
 	vb2_nv_init(&c);
 	test_changed(&c, 0, "vb2_nv_init() didn't re-reset");
-	TEST_EQ(c.nvdata[15], goodcrc, "vb2_nv_init() CRC same");
-	TEST_EQ(sd->status, VB2_SD_STATUS_NV_INIT, "vb2_nv_init() status same");
+	TEST_EQ(c.nvdata[crc_offs], goodcrc,
+		"vb2_nv_init() CRC same");
+	TEST_EQ(sd->status, VB2_SD_STATUS_NV_INIT,
+		"vb2_nv_init() status same");
 
-	/* Perturbing the header should force defaults */
-	c.nvdata[0] ^= 0x40;
+	/* Perturbing signature bits in the header should force defaults */
+	c.nvdata[VB2_NV_OFFS_HEADER] ^= 0x40;
 	TEST_EQ(vb2_nv_check_crc(&c),
 		VB2_ERROR_NV_HEADER, "vb2_nv_check_crc() bad header");
 	vb2_nv_init(&c);
-	TEST_EQ(c.nvdata[0], 0x70, "vb2_nv_init() reset header byte again");
+	TEST_EQ(c.nvdata[VB2_NV_OFFS_HEADER], expect_header,
+		"vb2_nv_init() reset header byte again");
 	test_changed(&c, 1, "vb2_nv_init() corrupt changed");
-	TEST_EQ(c.nvdata[15], goodcrc, "vb2_nv_init() CRC same again");
+	TEST_EQ(c.nvdata[crc_offs], goodcrc,
+		"vb2_nv_init() CRC same again");
 
 	/* So should perturbing some other byte */
-	TEST_EQ(c.nvdata[11], 0, "Kernel byte starts at 0");
-	c.nvdata[11] = 12;
+	TEST_EQ(c.nvdata[VB2_NV_OFFS_KERNEL1], 0, "Kernel byte starts at 0");
+	c.nvdata[VB2_NV_OFFS_KERNEL1] = 12;
 	TEST_EQ(vb2_nv_check_crc(&c),
 		VB2_ERROR_NV_CRC, "vb2_nv_check_crc() bad CRC");
 	vb2_nv_init(&c);
-	TEST_EQ(c.nvdata[11], 0, "vb2_nv_init() reset kernel byte");
+	TEST_EQ(c.nvdata[VB2_NV_OFFS_KERNEL1], 0,
+		"vb2_nv_init() reset kernel byte");
 	test_changed(&c, 1, "vb2_nv_init() corrupt elsewhere changed");
-	TEST_EQ(c.nvdata[15], goodcrc, "vb2_nv_init() CRC same again");
+	TEST_EQ(c.nvdata[crc_offs], goodcrc,
+		"vb2_nv_init() CRC same again");
 
 	/* Clear the kernel and firmware flags */
 	vb2_nv_init(&c);
@@ -138,9 +161,11 @@ static void nv_storage_test(void)
 	TEST_EQ(vb2_nv_get(&c, VB2_NV_KERNEL_SETTINGS_RESET),
 		0, "Kernel settings are clear");
 
-	TEST_EQ(c.nvdata[0], 0x40, "Header byte now just has the header bit");
+	TEST_EQ(c.nvdata[VB2_NV_OFFS_HEADER],
+		expect_header & VB2_NV_HEADER_SIGNATURE_MASK,
+		"Header byte now just has the signature");
 	/* That should have changed the CRC */
-	TEST_NEQ(c.nvdata[15], goodcrc,
+	TEST_NEQ(c.nvdata[crc_offs], goodcrc,
 		 "vb2_nv_init() CRC changed due to flags clear");
 
 	/* Test explicitly setting the reset flags again */
@@ -172,6 +197,29 @@ static void nv_storage_test(void)
 			vnf->desc);
 	}
 
+	for (vnf = nv2fields; vnf->desc; vnf++) {
+		if (ctxflags) {
+			TEST_EQ(vb2_nv_get(&c, vnf->param), vnf->default_value,
+				vnf->desc);
+			vb2_nv_set(&c, vnf->param, vnf->test_value);
+			TEST_EQ(vb2_nv_get(&c, vnf->param), vnf->test_value,
+				vnf->desc);
+			vb2_nv_set(&c, vnf->param, vnf->test_value2);
+			TEST_EQ(vb2_nv_get(&c, vnf->param), vnf->test_value2,
+				vnf->desc);
+		} else {
+			/*
+			 * V2 fields always return defaults and can't be set if
+			 * a V1 struct is present.
+			 */
+			TEST_EQ(vb2_nv_get(&c, vnf->param), vnf->test_value,
+				vnf->desc);
+			vb2_nv_set(&c, vnf->param, vnf->test_value2);
+			TEST_EQ(vb2_nv_get(&c, vnf->param), vnf->test_value,
+				vnf->desc);
+		}
+	}
+
 	/* None of those changes should have caused a reset to defaults */
 	vb2_nv_init(&c);
 	TEST_EQ(vb2_nv_get(&c, VB2_NV_FIRMWARE_SETTINGS_RESET),
@@ -180,12 +228,19 @@ static void nv_storage_test(void)
 		0, "Kernel settings are still clear");
 
 	/* Writing identical settings doesn't cause the CRC to regenerate */
-	c.flags = 0;
+	c.flags = ctxflags;
 	vb2_nv_init(&c);
 	test_changed(&c, 0, "No regen CRC on open");
 	for (vnf = nvfields; vnf->desc; vnf++)
 		vb2_nv_set(&c, vnf->param, vnf->test_value2);
 	test_changed(&c, 0, "No regen CRC if data not changed");
+	/*
+	 * If struct is V2, this is the same test.  If struct is V1, this
+	 * verifies that the field couldn't be changed anyway.
+	 */
+	for (vnf = nv2fields; vnf->desc; vnf++)
+		vb2_nv_set(&c, vnf->param, vnf->test_value2);
+	test_changed(&c, 0, "No regen CRC if V2 data not changed");
 
 	/* Test out-of-range fields mapping to defaults or failing */
 	vb2_nv_init(&c);
@@ -215,7 +270,10 @@ static void nv_storage_test(void)
 
 int main(int argc, char* argv[])
 {
-	nv_storage_test();
+	printf("Testing V1\n");
+	nv_storage_test(0);
+	printf("Testing V2\n");
+	nv_storage_test(VB2_CONTEXT_NVDATA_V2);
 
 	return gTestSuccess ? 0 : 255;
 }
diff --git a/tests/vboot_api_kernel4_tests.c b/tests/vboot_api_kernel4_tests.c
index 2e73b8b2..82d6d5db 100644
--- a/tests/vboot_api_kernel4_tests.c
+++ b/tests/vboot_api_kernel4_tests.c
@@ -81,13 +81,13 @@ static void ResetMocks(void)
 
 VbError_t VbExNvStorageRead(uint8_t *buf)
 {
-	memcpy(buf, ctx.nvdata, sizeof(ctx.nvdata));
+	memcpy(buf, ctx.nvdata, vb2_nv_get_size(&ctx));
 	return VBERROR_SUCCESS;
 }
 
 VbError_t VbExNvStorageWrite(const uint8_t *buf)
 {
-	memcpy(ctx.nvdata, buf, sizeof(ctx.nvdata));
+	memcpy(ctx.nvdata, buf, vb2_nv_get_size(&ctx));
 	return VBERROR_SUCCESS;
 }
 
diff --git a/tests/vboot_api_kernel5_tests.c b/tests/vboot_api_kernel5_tests.c
index a831e7f5..537c13b6 100644
--- a/tests/vboot_api_kernel5_tests.c
+++ b/tests/vboot_api_kernel5_tests.c
@@ -174,7 +174,7 @@ int vb2_verify_data(const uint8_t *data,
 
 VbError_t VbExNvStorageRead(uint8_t *buf)
 {
-	memcpy(buf, ctx.nvdata, sizeof(ctx.nvdata));
+	memcpy(buf, ctx.nvdata, vb2_nv_get_size(&ctx));
 	return VBERROR_SUCCESS;
 }
 
-- 
cgit v1.2.1