cr50: complete support of the new NVMEM structure

This patch eliminates unnecessary legacy nvmem.c and nvmem_vars.c code
and brings the code base to the state where the new NVMEM layout is
fully functional.

BRANCH=cr50, cr50-mp
BUG=b:69907320, b:129710256
CQ-DEPEND=CL:1450278
TEST=the following tests pass:
   - test cases in ./test/nvmem.c

   - TCG suite (passes on par with the existing Cr50 code with the
     reduced code footprint TPM2 library)

   - Chrome OS device migrates from legacy to new implementation with
     user account maintained.

   - Chrome OS user account is maintained over AP and H1 reboots and
     deep sleep cycles.

Change-Id: If4bc2dd125873a79dbe0e268eb32100a8b8b352d
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1496607
Reviewed-by: Andrey Pronin <apronin@chromium.org>

8 files changed, 124 insertions, 570 deletions

diff --git a/common/build.mk b/common/build.mk
index 7d5cd9dad0..5ba841734e 100644
--- a/common/build.mk
+++ b/common/build.mk
@@ -66,6 +66,7 @@ common-$(CONFIG_FLASH)+=flash.o
 common-$(CONFIG_FLASH_LOG)+=flash_log.o flash_log_vc.o
 common-$(CONFIG_FLASH_NVCOUNTER)+=nvcounter.o
 common-$(CONFIG_FLASH_NVMEM)+=nvmem.o
+common-$(CONFIG_FLASH_NVMEM)+=new_nvmem.o
 common-$(CONFIG_FLASH_NVMEM_VARS)+=nvmem_vars.o
 common-$(CONFIG_FMAP)+=fmap.o
 common-$(CONFIG_GESTURE_SW_DETECTION)+=gesture.o
diff --git a/common/ccd_config.c b/common/ccd_config.c
index c2a92aeb30..62f97892f4 100644
--- a/common/ccd_config.c
+++ b/common/ccd_config.c
@@ -389,6 +389,8 @@ static void ccd_load_config(void)
 		ccd_reset_config(t->val_len < 2 ? CCD_RESET_TEST_LAB : 0);
 	}
 
+	freevar(t);
+
 ccd_is_loaded:
 	ccd_config_loaded = 1;
 
@@ -410,8 +412,6 @@ static int ccd_save_config(void)
 	if (rv)
 		return rv;
 
-	rv = writevars();
-
 	/*
 	 * Notify CCD users of configuration change.
 	 * Protect this notify with the ccd_config_loaded flag so recipients of
diff --git a/common/new_nvmem.c b/common/new_nvmem.c
index 50433feb4c..9ec6e3c885 100644
--- a/common/new_nvmem.c
+++ b/common/new_nvmem.c
@@ -1379,7 +1379,7 @@ static enum ec_error_list migrate_vars(struct nn_container *ch)
 	var = NULL;
 	total_var_space = 0;
 
-	while ((var = getnextvar(var)) != NULL)
+	while ((var = legacy_getnextvar(var)) != NULL)
 		save_var(var->data_, var->key_len, var->data_ + var->key_len,
 			 var->val_len, (struct max_var_container *)ch);
 
@@ -2607,9 +2607,9 @@ static struct max_var_container *find_var(const uint8_t *key, size_t key_len,
 	return NULL;
 }
 
-struct tuple *getvar(const uint8_t *key, uint8_t key_len)
+const struct tuple *getvar(const uint8_t *key, uint8_t key_len)
 {
-	struct max_var_container *vc;
+	const struct max_var_container *vc;
 	struct access_tracker at = {};
 
 	if (!key || !key_len)
@@ -2623,14 +2623,15 @@ struct tuple *getvar(const uint8_t *key, uint8_t key_len)
 	return NULL;
 }
 
-int freevar(struct tuple *var)
+void freevar(const struct tuple *var)
 {
 	void *vc;
 
+	if (!var)
+		return;
+
 	vc = (uint8_t *)var - offsetof(struct max_var_container, t_header);
 	shared_mem_release(vc);
-
-	return EC_SUCCESS; /* Could verify var first before releasing. */
 }
 
 static enum ec_error_list save_container(struct nn_container *nc)
diff --git a/common/nvmem.c b/common/nvmem.c
index de6fba47b5..577318af2f 100644
--- a/common/nvmem.c
+++ b/common/nvmem.c
@@ -8,14 +8,13 @@
 #include "dcrypto.h"
 #include "flash.h"
 #include "nvmem.h"
+#include "new_nvmem.h"
 #include "task.h"
 #include "timer.h"
 #include "util.h"
 
-#define CPRINTF(format, args...) cprintf(CC_COMMAND, format, ## args)
-#define CPRINTS(format, args...) cprints(CC_COMMAND, format, ## args)
-
-#define NVMEM_NOT_INITIALIZED (-1)
+#define CPRINTF(format, args...) cprintf(CC_COMMAND, format, ##args)
+#define CPRINTS(format, args...) cprints(CC_COMMAND, format, ##args)
 
 /*
  * The NVMEM contents are stored in flash memory. At run time there is an SRAM
@@ -103,74 +102,13 @@ static void nvmem_compute_sha(struct nvmem_tag *tag, void *sha_buf)
 
 static int nvmem_save(void)
 {
-	struct nvmem_partition *part;
-	size_t nvmem_offset;
-	int dest_partition;
-	uint8_t sha_comp[NVMEM_SHA_SIZE];
-	int rv = EC_SUCCESS;
-
-	if (!DCRYPTO_ladder_is_enabled()) {
-		CPRINTF("%s: Key ladder is disabled. Skipping flash write\n",
-			__func__);
-		goto release_cache;
-	}
-
-	part = (struct nvmem_partition *)nvmem_cache;
-
-	/* Has anything changed in the cache? */
-	nvmem_compute_sha(&part->tag, sha_comp);
-
-	if (!memcmp(part->tag.sha, sha_comp, sizeof(part->tag.sha))) {
-		CPRINTF("%s: Nothing changed, skipping flash write\n",
-			__func__);
-		goto release_cache;
-	}
+	enum ec_error_list rv;
 
-	/* Get flash offset of the partition to save to. */
-	dest_partition = (nvmem_act_partition + 1) % NVMEM_NUM_PARTITIONS;
-	nvmem_offset = nvmem_base_addr[dest_partition] -
-		CONFIG_PROGRAM_MEMORY_BASE;
+	rv = new_nvmem_save();
 
-	/* Erase partition */
-	rv = flash_physical_erase(nvmem_offset, NVMEM_PARTITION_SIZE);
-	if (rv != EC_SUCCESS) {
-		CPRINTF("%s flash erase failed\n", __func__);
-		goto release_cache;
-	}
-
-	part->tag.layout_version = NVMEM_LAYOUT_VERSION;
-	part->tag.generation++;
-
-	/* Calculate sha of the whole thing. */
-	nvmem_compute_sha(&part->tag, part->tag.sha);
-
-	/* Encrypt actual payload. */
-	if (!app_cipher(part->tag.sha, part->buffer, part->buffer,
-			sizeof(part->buffer))) {
-		CPRINTF("%s encryption failed\n", __func__);
-		rv = EC_ERROR_UNKNOWN;
-		goto release_cache;
-	}
-
-	rv = flash_physical_write(nvmem_offset,
-				  NVMEM_PARTITION_SIZE,
-				  nvmem_cache);
-	if (rv != EC_SUCCESS) {
-		CPRINTF("%s flash write failed\n", __func__);
-		goto release_cache;
-	}
+	if (rv == EC_SUCCESS)
+		nvmem_act_partition = NVMEM_NOT_INITIALIZED;
 
-	/* Restore payload. */
-	if (!app_cipher(part->tag.sha, part->buffer, part->buffer,
-			sizeof(part->buffer))) {
-		CPRINTF("%s decryption failed\n", __func__);
-		rv = EC_ERROR_UNKNOWN;
-		goto release_cache;
-	}
-
-	nvmem_act_partition = dest_partition;
-
- release_cache:
 	nvmem_mutex.write_in_progress = 0;
 	nvmem_release_cache();
 	return rv;
@@ -245,21 +183,6 @@ static void nvmem_release_cache(void)
 	mutex_unlock(&nvmem_mutex.mtx);
 }
 
-static int nvmem_reinitialize(void)
-{
-	nvmem_lock_cache();  /* Unlocked by nvmem_save() below. */
-	/*
-	 * NvMem is not properly initialized. Let's just erase everything and
-	 * start over, so that at least 1 partition is ready to be used.
-	 */
-	nvmem_act_partition = 0;
-
-	memset(nvmem_cache, 0xff, NVMEM_PARTITION_SIZE);
-
-	/* Start with generation zero in the current active partition. */
-	return nvmem_save();
-}
-
 static int nvmem_compare_generation(void)
 {
 	struct nvmem_partition *p_part;
@@ -301,10 +224,10 @@ static int nvmem_find_partition(void)
 
 		if (nvmem_partition_read_verify(check_part) == EC_SUCCESS) {
 			nvmem_act_partition = check_part;
+			ccprintf("%s:%d found legacy partition %d\n", __func__,
+				 __LINE__, check_part);
 			return EC_SUCCESS;
 		}
-		ccprintf("%s:%d partiton %d verification FAILED\n",
-			 __func__, __LINE__, check_part);
 	}
 
 	/*
@@ -312,14 +235,8 @@ static int nvmem_find_partition(void)
 	 * is valid. Let's reinitialize the NVMEM - there is nothing else we
 	 * can do.
 	 */
-	CPRINTS("%s: No Valid Partition found, will reinitialize!", __func__);
-
-	if (nvmem_reinitialize() != EC_SUCCESS) {
-		CPRINTS("%s: Reinitialization failed!!");
-		return EC_ERROR_UNKNOWN;
-	}
-
-	return EC_SUCCESS;
+	CPRINTS("%s: No Legacy Partitions found.", __func__);
+	return EC_ERROR_INVALID_CONFIG;
 }
 
 static int nvmem_generate_offset_table(void)
@@ -342,8 +259,17 @@ static int nvmem_generate_offset_table(void)
 
 	return EC_SUCCESS;
 }
-static int nvmem_get_partition_off(int user, uint32_t offset,
-				   uint32_t len, uint32_t *p_buf_offset)
+
+void *nvmem_cache_base(enum nvmem_users user)
+{
+	if ((user < 0) || (user >= NVMEM_NUM_USERS))
+		return NULL;
+
+	return nvmem_cache + nvmem_user_start_offset[user];
+}
+
+static int nvmem_get_partition_off(int user, uint32_t offset, uint32_t len,
+				   uint32_t *p_buf_offset)
 {
 	uint32_t start_offset;
 
@@ -365,48 +291,6 @@ static int nvmem_get_partition_off(int user, uint32_t offset,
 	return EC_SUCCESS;
 }
 
-int nvmem_erase_user_data(enum nvmem_users user)
-{
-	int part;
-	int ret;
-	uint32_t user_offset, user_size;
-
-	if (user >= NVMEM_NUM_USERS)
-		return EC_ERROR_INVAL;
-
-	CPRINTS("Erasing NVMEM Flash Partition user: %d", user);
-
-	ret = EC_SUCCESS;
-
-	/* Find offset within cache. */
-	user_offset = nvmem_user_start_offset[user];
-	user_size = nvmem_user_sizes[user];
-
-	for (part = 0; part < NVMEM_NUM_PARTITIONS; part++) {
-		int rv;
-
-		/* Lock the cache buffer. */
-		nvmem_lock_cache();
-		/* Erase the user's data. */
-		memset(nvmem_cache + user_offset, 0xFF, user_size);
-
-		/*
-		 * Make sure the contents change between runs of
-		 * nvmem_save() so that all flash partitions are
-		 * written with empty contents and different
-		 * generation numbers.
-		 */
-		((struct nvmem_partition *)nvmem_cache)->tag.generation = part;
-
-		/* Make a best effort to clear each partition. */
-		rv = nvmem_save();
-		if (rv != EC_SUCCESS)
-			ret = rv;
-	}
-
-	return ret;
-}
-
 int nvmem_init(void)
 {
 	int ret;
@@ -417,8 +301,6 @@ int nvmem_init(void)
 		CPRINTF("%s:%d\n", __func__, __LINE__);
 		return ret;
 	}
-	/* Initialize error state, assume everything is good */
-	nvmem_error_state = EC_SUCCESS;
 	nvmem_write_error = 0;
 
 	/*
@@ -426,24 +308,27 @@ int nvmem_init(void)
 	 * succeeds to bootstrap the nvmem area.
 	 */
 	commits_enabled = 1;
-	ret = nvmem_find_partition();
+
+	/*
+	 * Try discovering legacy partition(s). If even one is present, need
+	 * to migrate to the new nvmem storage scheme.
+	 */
+	if (nvmem_find_partition() == EC_SUCCESS)
+		ret = new_nvmem_migrate(nvmem_act_partition);
+	else
+		ret = new_nvmem_init();
+
+	nvmem_error_state = ret;
 
 	if (ret != EC_SUCCESS) {
-		/* Change error state to non-zero */
-		nvmem_error_state = ret;
-		CPRINTF("%s:%d\n", __func__, __LINE__);
+		CPRINTF("%s:%d error %d!\n", __func__, __LINE__, ret);
 		return ret;
 	}
 
-	CPRINTS("Active Nvmem partition set to %d", nvmem_act_partition);
-
 	return EC_SUCCESS;
 }
 
-int nvmem_get_error_state(void)
-{
-	return nvmem_error_state;
-}
+int nvmem_get_error_state(void) { return nvmem_error_state; }
 
 int nvmem_is_different(uint32_t offset, uint32_t size, void *data,
 		       enum nvmem_users user)
diff --git a/common/nvmem_vars.c b/common/nvmem_vars.c
index d90fe37f9e..2b60dbed0c 100644
--- a/common/nvmem_vars.c
+++ b/common/nvmem_vars.c
@@ -7,7 +7,7 @@
 #include "common.h"
 #include "console.h"
 #include "nvmem.h"
-#include "nvmem_vars.h"
+#include "new_nvmem.h"
 #include "printf.h"
 #include "shared_mem.h"
 #include "util.h"
@@ -17,33 +17,14 @@
 
 test_mockable_static uint8_t *rbuf;
 
-test_mockable_static
-void release_local_copy(void)
+int set_local_copy(void)
 {
 	if (rbuf)
-		shared_mem_release(rbuf);
-	rbuf = NULL;
-}
-
-test_mockable_static
-int get_local_copy(void)
-{
-	int rv;
-
-	if (rbuf)
-		return EC_SUCCESS;
-
-	rv = SHARED_MEM_ACQUIRE_CHECK(CONFIG_FLASH_NVMEM_VARS_USER_SIZE,
-				(char **)&rbuf);
+		return EC_ERROR_UNKNOWN;
 
-	if (rv == EC_SUCCESS) {
-		rv = nvmem_read(0, CONFIG_FLASH_NVMEM_VARS_USER_SIZE,
-				rbuf, CONFIG_FLASH_NVMEM_VARS_USER_NUM);
-		if (rv != EC_SUCCESS)
-			release_local_copy();
-	}
+	rbuf = nvmem_cache_base(NVMEM_CR50);
 
-	return rv;
+	return EC_SUCCESS;
 }
 
 /****************************************************************************/
@@ -88,261 +69,42 @@ int get_local_copy(void)
  */
 
 /****************************************************************************/
-/* Helper functions */
-
-/* Return true if the tuple at rbuf+idx matches the key */
-static int match_key_at(uint32_t idx, const uint8_t *key, uint8_t key_len)
-{
-	struct tuple *tuple = (struct tuple *)(rbuf + idx);
-	uint32_t i, max_len;
-	uint8_t diffs;
-
-	/* Don't try to look past the 0 at the end of the user region */
-	max_len = MIN(key_len, CONFIG_FLASH_NVMEM_VARS_USER_SIZE - idx - 1);
-
-	/* Do constant-time comparision, since AP sets key_len to look for */
-	diffs = max_len ^ key_len;
-	diffs |= tuple->key_len ^ key_len;
-	for (i = 0; i < max_len; i++)
-		diffs |=  tuple->data_[i] ^ key[i];
-
-	return !diffs;
-}
-
-/*
- * Find the start of the next tuple in rbuf. Return false if there isn't one.
- * The idx arg tracks where to start looking and where the next tuple was
- * expected to be found.
- */
-static int next_tuple(uint32_t *idx)
-{
-	struct tuple *tuple = (struct tuple *)(rbuf + *idx);
-
-	/* Not at a valid tuple now, so there aren't any more */
-	if (!tuple->key_len)
-		return 0;
-
-	/* Advance to the next one */
-	*idx += sizeof(struct tuple) + tuple->key_len + tuple->val_len;
-	tuple = (struct tuple *)(rbuf + *idx);
-
-	/* Do we have one or not? */
-	return tuple->key_len;
-}
-
-/*
- * Look for the key in rbuf. If a match is found, set the index to the start of
- * the tuple and return true. If the key is not found, set the index to the
- * location where a new tuple should be added (0 if no tuples exist at all,
- * else at the '\0' at the end of the tuples) and return false.
- */
-test_mockable_static
-int getvar_idx(uint32_t *idx, const uint8_t *key, uint8_t key_len)
-{
-	uint32_t i = *idx;
-
-	do {
-		if (match_key_at(i, key, key_len)) {
-			*idx = i;
-			return 1;
-		}
-	} while (next_tuple(&i));
-
-	*idx = i;
-	return 0;
-}
-
-static inline int bogus_blob(const uint8_t *blob, uint8_t blob_len)
-{
-	return !blob || !blob_len;
-}
-
-/****************************************************************************/
 /* API functions */
 
-/* This MUST be called first. The internal functions assume valid entries */
-int initvars(void)
+const struct tuple *legacy_getnextvar(const struct tuple *prev_var)
 {
-	struct tuple *tuple;
-	int rv, i, len;
-
-	rv = get_local_copy();
-	if (rv != EC_SUCCESS)
-		return rv;
-
-	for (i = len = 0; /* FOREVER */ 1; i += len) {
-		/* Zero byte (i.e. key_len == 0) indicates end of tuples. */
-		if (rbuf[i] == 0)
-			break;
-
-		tuple = (struct tuple *)(rbuf + i);
-		len = sizeof(struct tuple);
-
-		/* Make sure the tuple struct is within bounds. */
-		if (i + len > CONFIG_FLASH_NVMEM_VARS_USER_SIZE)
-			goto fixit;
-
-		/* Empty values are not allowed */
-		if (!tuple->val_len)
-			goto fixit;
-
-		/* See how big the tuple is */
-		len += tuple->key_len + tuple->val_len;
+	const struct tuple *var;
+	uintptr_t idx;
 
-		/* Oops, it's off the end (leave one byte for final 0) */
-		if (i + len >= CONFIG_FLASH_NVMEM_VARS_USER_SIZE)
-			goto fixit;
+	if (!prev_var) {
+		/*
+		 * The caller is just starting, let's get the first var, if
+		 * any.
+		 */
+		if (!rbuf[0])
+			return NULL;
+		return (const struct tuple *)rbuf;
 	}
 
-	/* Found the end of variables. Now make sure the rest is all 0xff. */
-	for (i++ ; i < CONFIG_FLASH_NVMEM_VARS_USER_SIZE; i++)
-		if (rbuf[i] != 0xff)
-			goto fixit;
+	/* Let's try to get the next one. */
+	idx = (uintptr_t)prev_var;
+	idx += prev_var->key_len + prev_var->val_len + sizeof(struct tuple);
 
-	release_local_copy();
-	return EC_SUCCESS;
+	var = (const struct tuple *)idx;
 
-fixit:
-	/* No variables */
-	rbuf[0] = 0;
-	/* Everything else is 0xff */
-	memset(rbuf + 1, 0xff, CONFIG_FLASH_NVMEM_VARS_USER_SIZE - 1);
+	if (var->key_len)
+		return var;
 
-	return writevars();
+	return NULL;
 }
 
-const struct tuple *getvar(const uint8_t *key, uint8_t key_len)
-{
-	uint32_t i = 0;
-
-	if (bogus_blob(key, key_len))
-		return 0;
-
-	if (get_local_copy() != EC_SUCCESS)
-		return 0;
-
-	if (!getvar_idx(&i, key, key_len))
-		return 0;
-
-	return (const struct tuple *)(rbuf + i);
-}
-
-const uint8_t *tuple_key(const struct tuple *t)
-{
-	return t->data_;
-}
+const uint8_t *tuple_key(const struct tuple *t) { return t->data_; }
 
 const uint8_t *tuple_val(const struct tuple *t)
 {
 	return t->data_ + t->key_len;
 }
 
-int setvar(const uint8_t *key, uint8_t key_len,
-	   const uint8_t *val, uint8_t val_len)
-{
-	struct tuple *tuple;
-	int rv, i, j;
-
-	if (bogus_blob(key, key_len))
-		return EC_ERROR_INVAL;
-
-	rv = get_local_copy();
-	if (rv != EC_SUCCESS)
-		return rv;
-
-	i = 0;
-	if (getvar_idx(&i, key, key_len)) {
-		/* Found the match at position i */
-		j = i;
-		if (next_tuple(&j)) {
-			/*
-			 * Now j is the start of the tuple after ours. Delete
-			 * our entry by shifting left from there to the end of
-			 * rbuf, so that it covers ours up.
-			 *
-			 * Before:
-			 *            i        j
-			 *   <foo,bar><KEY,VAL><hey,splat>0
-			 *
-			 * After:
-			 *            i
-			 *   <foo,bar><hey,splat>0...
-			 */
-			memmove(rbuf + i, rbuf + j,
-				CONFIG_FLASH_NVMEM_VARS_USER_SIZE - j);
-			/* Advance i to point to the end of all tuples */
-			while (next_tuple(&i))
-				;
-		}
-		/* Whether we found a match or not, it's not there now */
-	}
-	/*
-	 * Now i is where the new tuple should be written.
-	 *
-	 * Either this:
-	 *                       i
-	 *   <foo,bar><hey,splat>0
-	 *
-	 * or there are no tuples at all and i == 0
-	 *
-	 */
-
-	/* If there's no value to save, we're done. */
-	if (bogus_blob(val, val_len))
-		goto done;
-
-	/*
-	 * We'll always write the updated entry at the end of any existing
-	 * tuples, and we mark the end with an additional 0. Make sure all that
-	 * will all fit. If it doesn't, we've already removed the previous
-	 * entry but we still need to mark the end.
-	 */
-	if (i + sizeof(struct tuple) + key_len + val_len + 1 >
-	    CONFIG_FLASH_NVMEM_VARS_USER_SIZE) {
-		rv = EC_ERROR_OVERFLOW;
-		goto done;
-	}
-
-	/* write the tuple */
-	tuple = (struct tuple *)(rbuf + i);
-	tuple->key_len = key_len;
-	tuple->val_len = val_len;
-	tuple->flags = 0;			/* UNUSED, set to zero */
-	memcpy(tuple->data_, key, key_len);
-	memcpy(tuple->data_ + key_len, val, val_len);
-	/* move past it */
-	next_tuple(&i);
-
-done:
-	/* mark the end */
-	rbuf[i++] = 0;
-	/* erase the rest */
-	memset(rbuf + i, 0xff, CONFIG_FLASH_NVMEM_VARS_USER_SIZE - i);
-
-	return rv;
-}
-
-int writevars(void)
-{
-	int rv;
-
-	if (!rbuf)
-		return EC_SUCCESS;
-
-	rv = nvmem_write(0, CONFIG_FLASH_NVMEM_VARS_USER_SIZE,
-			 rbuf, CONFIG_FLASH_NVMEM_VARS_USER_NUM);
-	if (rv != EC_SUCCESS)
-		return rv;
-
-	rv = nvmem_commit();
-	if (rv != EC_SUCCESS)
-		return rv;
-
-	release_local_copy();
-
-	return rv;
-}
-
 /****************************************************************************/
 #if defined(TEST_BUILD) && !defined(TEST_FUZZ)
 #include "console.h"
@@ -382,133 +144,28 @@ static int command_set(int argc, char **argv)
 		return EC_ERROR_PARAM_COUNT;
 
 	if (argc == 2)
-		rc =  setvar(argv[1], strlen(argv[1]), 0, 0);
+		rc = setvar(argv[1], strlen(argv[1]), 0, 0);
 	else
-		rc =  setvar(argv[1], strlen(argv[1]),
-			     argv[2], strlen(argv[2]));
-	if (rc)
-		return rc;
+		rc = setvar(argv[1], strlen(argv[1]), argv[2], strlen(argv[2]));
 
-	return writevars();
+	return rc;
 }
-DECLARE_CONSOLE_COMMAND(set, command_set,
-			"VARIABLE [VALUE]",
+DECLARE_CONSOLE_COMMAND(set, command_set, "VARIABLE [VALUE]",
 			"Set/clear the value of the specified variable");
 
 static int command_print(int argc, char **argv)
 {
-	const struct tuple *tuple;
-	int rv, i = 0;
-
-	rv = get_local_copy();
-	if (rv)
-		return rv;
-
-	tuple = (const struct tuple *)(rbuf + i);
-	if (!tuple->key_len)
-		return EC_SUCCESS;
-
-	do {
-		tuple = (const struct tuple *)(rbuf + i);
-		print_blob(tuple_key(tuple), tuple->key_len);
-		ccprintf("=");
-		print_blob(tuple_val(tuple), tuple->val_len);
-		ccprintf("\n");
-	} while (next_tuple(&i));
-
-	return EC_SUCCESS;
+	ccprintf("Print all vars is not yet implemented\n");
+	return EC_ERROR_INVAL;
 }
-DECLARE_CONSOLE_COMMAND(print, command_print,
-			"",
+DECLARE_CONSOLE_COMMAND(print, command_print, "",
 			"Print all defined variables");
 
-static int command_dump(int argc, char **argv)
-{
-	int i, rv;
-
-	rv = get_local_copy();
-	if (rv)
-		return rv;
-
-	for (i = 0; i < CONFIG_FLASH_NVMEM_VARS_USER_SIZE; i++)
-		ccprintf(" %02x", rbuf[i]);
-	ccprintf("\n");
-	release_local_copy();
-
-	return EC_SUCCESS;
-}
-DECLARE_CONSOLE_COMMAND(dump, command_dump,
-			"",
-			"Dump the variable memory");
-
 static int command_clear_nvmem_vars(int argc, char **argv)
 {
-	int rv;
-
-	rv = nvmem_erase_user_data(CONFIG_FLASH_NVMEM_VARS_USER_NUM);
-	if (rv)
-		ccprintf("Error clearing nvmem vars! (rv: %d)\n", rv);
-	else
-		ccprintf("NvMem vars cleared successfully.\n");
-
-	/*
-	 * Invalidate the cache buffer since we just erased the backing
-	 * store.
-	 */
-	writevars();
-
-	/*
-	 * Re-initialize the NvMem vars space so that it's ready for
-	 * immediate use.
-	 */
-	initvars();
-
-	/*
-	 * TODO(aaboagye): For "V1", this is where you might want to call and
-	 * reset the defaults.
-	 */
-
-	return rv;
+	ccprintf("Nvmem clear vars has not yet been implemented\n");
+	return EC_ERROR_INVAL;
 }
-DECLARE_CONSOLE_COMMAND(clr_nvmem_vars, command_clear_nvmem_vars,
-			"",
+DECLARE_CONSOLE_COMMAND(clr_nvmem_vars, command_clear_nvmem_vars, "",
 			"Clear the NvMem variables.");
-
-static int command_nv_test_var(int argc, char **argv)
-{
-	const struct tuple *t;
-	uint8_t key;
-	uint8_t val;
-	int rv;
-
-	key = NVMEM_VAR_TEST_VAR;
-
-	if (argc > 1) {
-		val = (uint8_t)atoi(argv[1]);
-		rv = setvar(&key, 1, &val, 1);
-		if (rv)
-			ccprintf("setvar err %d", rv);
-
-		rv = writevars();
-		if (rv)
-			ccprintf("writevar err %d", rv);
-	}
-
-	t = getvar(&key, 1);
-	if (t) {
-		val = *tuple_val(t);
-	} else {
-		ccprintf("No value set.\n");
-		return EC_SUCCESS;
-	}
-
-	/* Invalidate RAM buffer. */
-	writevars();
-	ccprintf("test_var: %d\n", val);
-
-	return EC_SUCCESS;
-}
-DECLARE_SAFE_CONSOLE_COMMAND(nvtestvar, command_nv_test_var,
-			     "[0-255]",
-			     "Get/Set an NvMem test variable.");
 #endif
diff --git a/common/pinweaver.c b/common/pinweaver.c
index 1777a4c28b..d65e1bad0c 100644
--- a/common/pinweaver.c
+++ b/common/pinweaver.c
@@ -656,6 +656,7 @@ static int load_log_data(struct pw_log_storage_t *log)
 {
 	const struct tuple *ptr;
 	const struct pw_log_storage_t *view;
+	int rv = EC_SUCCESS;
 
 	ptr = getvar(PW_LOG_VAR0, sizeof(PW_LOG_VAR0) - 1);
 	if (ptr == NULL)
@@ -663,24 +664,21 @@ static int load_log_data(struct pw_log_storage_t *log)
 
 	view = (void *)tuple_val(ptr);
 	if (ptr->val_len != sizeof(struct pw_log_storage_t))
-		return PW_ERR_NV_LENGTH_MISMATCH;
-	if (view->storage_version != PW_STORAGE_VERSION)
-		return PW_ERR_NV_VERSION_MISMATCH;
+		rv = PW_ERR_NV_LENGTH_MISMATCH;
+	else if (view->storage_version != PW_STORAGE_VERSION)
+		rv = PW_ERR_NV_VERSION_MISMATCH;
+	else
+		memcpy(log, view, ptr->val_len);
 
-	memcpy(log, view, ptr->val_len);
-	return EC_SUCCESS;
+	freevar(ptr);
+
+	return rv;
 }
 
 int store_log_data(const struct pw_log_storage_t *log)
 {
-	int ret;
-
-	ret = setvar(PW_LOG_VAR0, sizeof(PW_LOG_VAR0) - 1, (uint8_t *)log,
-		     sizeof(struct pw_log_storage_t));
-	if (ret != EC_SUCCESS)
-		return ret;
-
-	return writevars();
+	return setvar(PW_LOG_VAR0, sizeof(PW_LOG_VAR0) - 1, (uint8_t *)log,
+		      sizeof(struct pw_log_storage_t));
 }
 
 static int load_merkle_tree(struct merkle_tree_t *merkle_tree)
@@ -695,15 +693,19 @@ static int load_merkle_tree(struct merkle_tree_t *merkle_tree)
 		const struct pw_long_term_storage_t *tree;
 
 		ptr = getvar(PW_TREE_VAR, sizeof(PW_TREE_VAR) - 1);
-		if (ptr == NULL)
+		if (!ptr)
 			return PW_ERR_NV_EMPTY;
 
 		tree = (void *)tuple_val(ptr);
 		/* Add storage format updates here. */
-		if (ptr->val_len != sizeof(*tree))
+		if (ptr->val_len != sizeof(*tree)) {
+			freevar(ptr);
 			return PW_ERR_NV_LENGTH_MISMATCH;
-		if (tree->storage_version != PW_STORAGE_VERSION)
+		}
+		if (tree->storage_version != PW_STORAGE_VERSION) {
+			freevar(ptr);
 			return PW_ERR_NV_VERSION_MISMATCH;
+		}
 
 		merkle_tree->bits_per_level = tree->bits_per_level;
 		merkle_tree->height = tree->height;
@@ -711,8 +713,10 @@ static int load_merkle_tree(struct merkle_tree_t *merkle_tree)
 		       tree->key_derivation_nonce,
 		       sizeof(tree->key_derivation_nonce));
 		ret = derive_keys(merkle_tree);
-		if (ret != EC_SUCCESS)
+		if (ret != EC_SUCCESS) {
+			freevar(ptr);
 			return ret;
+		}
 	}
 
 	/* Handle the root hash. */
@@ -720,15 +724,19 @@ static int load_merkle_tree(struct merkle_tree_t *merkle_tree)
 		struct pw_log_storage_t *log;
 
 		ptr = getvar(PW_LOG_VAR0, sizeof(PW_LOG_VAR0) - 1);
-		if (ptr == NULL)
+		if (!ptr)
 			return PW_ERR_NV_EMPTY;
 
 		log = (void *)tuple_val(ptr);
 		/* Add storage format updates here. */
-		if (ptr->val_len != sizeof(struct pw_log_storage_t))
+		if (ptr->val_len != sizeof(struct pw_log_storage_t)) {
+			freevar(ptr);
 			return PW_ERR_NV_LENGTH_MISMATCH;
-		if (log->storage_version != PW_STORAGE_VERSION)
+		}
+		if (log->storage_version != PW_STORAGE_VERSION) {
+			freevar(ptr);
 			return PW_ERR_NV_VERSION_MISMATCH;
+		}
 
 		memcpy(merkle_tree->root, log->entries[0].root,
 		       sizeof(merkle_tree->root));
@@ -747,15 +755,15 @@ static int load_merkle_tree(struct merkle_tree_t *merkle_tree)
 			ret = setvar(PW_LOG_VAR0, sizeof(PW_LOG_VAR0) - 1,
 				     (uint8_t *)log,
 				     sizeof(struct pw_log_storage_t));
-			if (ret != EC_SUCCESS)
-				return ret;
-			ret = writevars();
-			if (ret != EC_SUCCESS)
+			if (ret != EC_SUCCESS) {
+				freevar(ptr);
 				return ret;
+			}
 		}
 		pw_restart_count = log->restart_count;
 	}
 
+	freevar(ptr);
 	cprints(CC_TASK, "PinWeaver: Loaded Tree. restart_count = %d",
 		pw_restart_count);
 
diff --git a/common/shmalloc.c b/common/shmalloc.c
index c1cebaaf7c..8c66a63eb8 100644
--- a/common/shmalloc.c
+++ b/common/shmalloc.c
@@ -310,6 +310,8 @@ int shared_mem_acquire(int size, char **dest_ptr)
 	int rv;
 	struct shm_buffer *new_buf;
 
+	*dest_ptr = NULL;
+
 	if (in_interrupt_context())
 		return EC_ERROR_INVAL;
 
diff --git a/common/tpm_registers.c b/common/tpm_registers.c
index 142261ecf3..947a3b089a 100644
--- a/common/tpm_registers.c
+++ b/common/tpm_registers.c
@@ -13,7 +13,7 @@
 #include "console.h"
 #include "extension.h"
 #include "link_defs.h"
-#include "nvmem.h"
+#include "new_nvmem.h"
 #include "printf.h"
 #include "signed_header.h"
 #include "sps.h"
@@ -828,20 +828,11 @@ static void tpm_reset_now(int wipe_first)
 
 	if (wipe_first)
 		/* Now wipe the TPM's nvmem */
-		wipe_result = nvmem_erase_user_data(NVMEM_TPM);
+		wipe_result = nvmem_erase_tpm_data();
 	else
 		wipe_result = EC_SUCCESS;
 
 	/*
-	 * Clear the TPM library's zero-init data.  Note that the linker script
-	 * includes this file's .bss in the same section, so it will be cleared
-	 * at the same time.
-	 */
-	memset(&__bss_libtpm2_start, 0,
-	       (uintptr_t)(&__bss_libtpm2_end) -
-	       (uintptr_t)(&__bss_libtpm2_start));
-
-	/*
 	 * NOTE: If any __initialized variables need reinitializing after
 	 * reset, this is the place to do it.
 	 */
@@ -853,6 +844,15 @@ static void tpm_reset_now(int wipe_first)
 	nvmem_enable_commits();
 
 	/*
+	 * Clear the TPM library's zero-init data.  Note that the linker script
+	 * includes this file's .bss in the same section, so it will be cleared
+	 * at the same time.
+	 */
+	memset(&__bss_libtpm2_start, 0,
+	       (uintptr_t)(&__bss_libtpm2_end) -
+		       (uintptr_t)(&__bss_libtpm2_start));
+
+	/*
 	 * Prevent NVRAM commits until further notice, unless running in
 	 * factory mode.
 	 */