Move VbNvContext setup/teardown to vboot wrapper funcs

BUG=chromium-os:18226
TEST=make && make runtests

Change-Id: Ie22e99a9796866e646ed92158410bc7a2fdf90f6
Reviewed-on: http://gerrit.chromium.org/gerrit/4754
Reviewed-by: Stefan Reinauer <reinauer@google.com>
Tested-by: Randall Spangler <rspangler@chromium.org>

7 files changed, 21 insertions, 52 deletions

diff --git a/firmware/include/load_firmware_fw.h b/firmware/include/load_firmware_fw.h
index 1189654f..6de4c9ff 100644
--- a/firmware/include/load_firmware_fw.h
+++ b/firmware/include/load_firmware_fw.h
@@ -34,12 +34,9 @@ typedef struct LoadFirmwareParams {
                                   * the buffer.  Caller need only pass that
                                   * much data to LoadKernel().*/
 
-  VbNvContext* nv_context;       /* Context for NV storage.  nv_context->raw
-                                  * must be filled before calling
-                                  * LoadFirmware().  On output, check
-                                  * nv_context->raw_changed to see if
-                                  * nv_context->raw has been modified and
-                                  * needs saving. */
+  VbNvContext* nv_context;       /* Context for NV storage.  Caller is
+                                  * responsible for calling VbNvSetup() and
+                                  * VbNvTeardown() on the context. */
 
   /* Internal data for LoadFirmware() / UpdateFirmwareBodyHash(). */
   void* load_firmware_internal;
diff --git a/firmware/include/load_kernel_fw.h b/firmware/include/load_kernel_fw.h
index dfbd4e89..406691ff 100644
--- a/firmware/include/load_kernel_fw.h
+++ b/firmware/include/load_kernel_fw.h
@@ -44,12 +44,9 @@ typedef struct LoadKernelParams {
                                  * (normally at 0x100000) */
   uint64_t kernel_buffer_size;  /* Size of kernel buffer in bytes */
   uint64_t boot_flags;          /* Boot flags */
-  VbNvContext* nv_context;       /* Context for NV storage.  nv_context->raw
-                                  * must be filled before calling
-                                  * LoadKernel().  On output, check
-                                  * nv_context->raw_changed to see if
-                                  * nv_context->raw has been modified and
-                                  * needs saving. */
+  VbNvContext* nv_context;      /* Context for NV storage.  Caller is
+                                 * responsible for calling VbNvSetup() and
+                                 * VbNvTeardown() on the context. */
 
   /* Outputs from LoadKernel(); valid only if LoadKernel() returns
    * LOAD_KERNEL_SUCCESS */
diff --git a/firmware/lib/vboot_api_firmware.c b/firmware/lib/vboot_api_firmware.c
index 6a3f89e8..303845b1 100644
--- a/firmware/lib/vboot_api_firmware.c
+++ b/firmware/lib/vboot_api_firmware.c
@@ -18,11 +18,7 @@
 /* Set recovery request */
 static void VbSfRequestRecovery(VbNvContext *vnc, uint32_t recovery_request) {
   VBDEBUG(("VbSfRequestRecovery(%d)\n", (int)recovery_request));
-  VbNvSetup(vnc);
   VbNvSet(vnc, VBNV_RECOVERY_REQUEST, recovery_request);
-  VbNvTeardown(vnc);
-  if (vnc->raw_changed)
-    VbExNvStorageWrite(vnc->raw);
 }
 
 
@@ -42,7 +38,7 @@ VbError_t VbSelectFirmware(VbCommonParams* cparams,
 
   /* Load NV storage */
   VbExNvStorageRead(vnc.raw);
-  vnc.raw_changed = 0;
+  VbNvSetup(&vnc);
 
   /* Initialize the TPM */
   VBPERFSTART("VB_TPMI");
@@ -108,10 +104,6 @@ VbError_t VbSelectFirmware(VbCommonParams* cparams,
     /* Chain to LoadFirmware() */
     retval = LoadFirmware(&p);
 
-    /* Save NV storage, if necessary */
-    if (vnc.raw_changed)
-      VbExNvStorageWrite(vnc.raw);
-
     /* Copy amount of used shared data back to the wrapper API struct */
     cparams->shared_data_size = (uint32_t)p.shared_data_size;
 
@@ -170,6 +162,11 @@ VbError_t VbSelectFirmware(VbCommonParams* cparams,
 
 VbSelectFirmware_exit:
 
+  /* Save NV storage */
+  VbNvTeardown(&vnc);
+  if (vnc.raw_changed)
+    VbExNvStorageWrite(vnc.raw);
+
   /* Stop timer */
   shared->timer_vb_select_firmware_exit = VbExGetTimer();
 
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index 22bddf9f..692e3b08 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -31,12 +31,7 @@ VbNvContext* VbApiKernelGetVnc(void) {
 /* Set recovery request */
 static void VbSetRecoveryRequest(uint32_t recovery_request) {
   VBDEBUG(("VbSetRecoveryRequest(%d)\n", (int)recovery_request));
-
-  VbNvSetup(&vnc);
   VbNvSet(&vnc, VBNV_RECOVERY_REQUEST, recovery_request);
-  VbNvTeardown(&vnc);
-  if (vnc.raw_changed)
-    VbExNvStorageWrite(vnc.raw);
 }
 
 
@@ -144,15 +139,11 @@ static VbError_t VbDisplayScreenFromGBB(VbCommonParams* cparams,
   }
 
   /* Clip localization to the number of localizations present in the GBB */
-  VbNvSetup(&vnc);
   VbNvGet(&vnc, VBNV_LOCALIZATION_INDEX, &localization);
   if (localization >= hdr->number_of_localizations) {
     localization = 0;
     VbNvSet(&vnc, VBNV_LOCALIZATION_INDEX, localization);
   }
-  VbNvTeardown(&vnc);
-  if (vnc.raw_changed)
-    VbExNvStorageWrite(vnc.raw);
 
   /* Calculate offset of screen layout = start of screen stuff +
    * correct locale + correct screen. */
@@ -309,21 +300,15 @@ static VbError_t VbCheckDisplayKey(VbCommonParams* cparams, uint32_t key) {
     uint32_t loc = 0;
     uint32_t count = 0;
 
-    /* Get localization count */
-    VbGetLocalizationCount(cparams, &count);
-
-    /* Change localization */
-    VbNvSetup(&vnc);
     VbNvGet(&vnc, VBNV_LOCALIZATION_INDEX, &loc);
-    if (VB_KEY_RIGHT == key)
+    if (VBERROR_SUCCESS != VbGetLocalizationCount(cparams, &count))
+      loc = 0;  /* No localization count (bad GBB?), so set to 0 (default) */
+    else if (VB_KEY_RIGHT == key)
       loc = (loc < count - 1 ? loc + 1 : 0);
     else
       loc = (loc > 0 ? loc - 1 : count - 1);
     VBDEBUG(("VbCheckDisplayKey() - change localization to %d\n", (int)loc));
     VbNvSet(&vnc, VBNV_LOCALIZATION_INDEX, loc);
-    VbNvTeardown(&vnc);
-    if (vnc.raw_changed)
-      VbExNvStorageWrite(vnc.raw);
 
     /* Force redraw of current screen */
     return VbDisplayScreen(cparams, disp_current_screen, 1);
@@ -342,13 +327,13 @@ static VbError_t VbCheckDisplayKey(VbCommonParams* cparams, uint32_t key) {
  * May return other VBERROR_ codes for other failures. */
 uint32_t VbTryLoadKernel(VbCommonParams* cparams, LoadKernelParams* p,
                          uint32_t get_info_flags) {
-  int retval = VBERROR_UNKNOWN;
+  VbError_t retval = VBERROR_UNKNOWN;
   VbDiskInfo* disk_info = NULL;
   uint32_t disk_count = 0;
   uint32_t i;
 
   VBDEBUG(("VbTryLoadKernel() start, get_info_flags=0x%x\n",
-          (int)get_info_flags));
+          (unsigned)get_info_flags));
 
   p->disk_handle = NULL;
 
@@ -571,7 +556,7 @@ VbError_t VbSelectAndLoadKernel(VbCommonParams* cparams,
   shared->timer_vb_select_and_load_kernel_enter = VbExGetTimer();
 
   VbExNvStorageRead(vnc.raw);
-  vnc.raw_changed = 0;
+  VbNvSetup(&vnc);
 
   /* Clear output params in case we fail */
   kparams->disk_handle = NULL;
diff --git a/firmware/lib/vboot_firmware.c b/firmware/lib/vboot_firmware.c
index ce6206c2..0d1a307a 100644
--- a/firmware/lib/vboot_firmware.c
+++ b/firmware/lib/vboot_firmware.c
@@ -56,9 +56,6 @@ int LoadFirmware(LoadFirmwareParams* params) {
 
   VBDEBUG(("LoadFirmware started...\n"));
 
-  /* Setup NV storage */
-  VbNvSetup(vnc);
-
   /* Handle test errors */
   VbNvGet(vnc, VBNV_TEST_ERROR_FUNC, &test_err);
   if (VBNV_TEST_ERROR_LOAD_FIRMWARE == test_err) {
@@ -325,10 +322,9 @@ int LoadFirmware(LoadFirmwareParams* params) {
   }
 
 LoadFirmwareExit:
-  /* Store recovery request, if any, then tear down non-volatile storage */
+  /* Store recovery request, if any */
   VbNvSet(vnc, VBNV_RECOVERY_REQUEST, VBERROR_SUCCESS != retval ?
           recovery : VBNV_RECOVERY_NOT_REQUESTED);
-  VbNvTeardown(vnc);
 
   /* Note that we don't reduce params->shared_data_size to shared->data_used,
    * since we want to leave space for LoadKernel() to add to the shared data
diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c
index 64c5cc58..6f8029a9 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -143,9 +143,6 @@ VbError_t LoadKernel(LoadKernelParams* params) {
   VbError_t retval = VBERROR_UNKNOWN;
   int recovery = VBNV_RECOVERY_RO_UNSPECIFIED;
 
-  /* Setup NV storage */
-  VbNvSetup(vnc);
-
   /* Sanity Checks */
   if (!params ||
       !params->bytes_per_lba ||
@@ -537,10 +534,9 @@ VbError_t LoadKernel(LoadKernelParams* params) {
 
 LoadKernelExit:
 
-  /* Store recovery request, if any, then tear down non-volatile storage */
+  /* Store recovery request, if any */
   VbNvSet(vnc, VBNV_RECOVERY_REQUEST, VBERROR_SUCCESS != retval ?
           recovery : VBNV_RECOVERY_NOT_REQUESTED);
-  VbNvTeardown(vnc);
 
   shcall->return_code = (uint8_t)retval;
 
diff --git a/utility/load_kernel_test.c b/utility/load_kernel_test.c
index 1f8cc2a5..c36c3357 100644
--- a/utility/load_kernel_test.c
+++ b/utility/load_kernel_test.c
@@ -92,6 +92,7 @@ int main(int argc, char* argv[]) {
   lkp.bytes_per_lba = LBA_BYTES;
   lkp.boot_flags = BOOT_FLAG_RECOVERY;
   Memset(&vnc, 0, sizeof(VbNvContext));
+  VbNvSetup(&vnc);
   lkp.nv_context = &vnc;
 
   /* Parse options */