Clean up calling LoadFirmware() from VbSelectFirmware()

BUG=chromium-os:18383
TEST=compiles for both x86 and ARM; boots on my test system

Change-Id: I6c7dc1dd086fb06e4ad8daa053bccdf61a463240
Reviewed-on: http://gerrit.chromium.org/gerrit/4977
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Stefan Reinauer <reinauer@google.com>
Tested-by: Randall Spangler <rspangler@chromium.org>

6 files changed, 36 insertions, 136 deletions

diff --git a/firmware/include/load_firmware_fw.h b/firmware/include/load_firmware_fw.h
index 6de4c9ff..87af7282 100644
--- a/firmware/include/load_firmware_fw.h
+++ b/firmware/include/load_firmware_fw.h
@@ -10,73 +10,19 @@
 #define VBOOT_REFERENCE_LOAD_FIRMWARE_FW_H_
 
 #include "sysincludes.h"
+#include "vboot_api.h"
 #include "vboot_nvstorage.h"
 #include "vboot_struct.h"
 
-typedef struct LoadFirmwareParams {
-  /* Inputs to LoadFirmware() */
-  void* gbb_data;                /* Pointer to GBB data */
-  uint64_t gbb_size;             /* Size of GBB data in bytes */
-  void* verification_block_0;    /* Key block + preamble for firmware 0 */
-  void* verification_block_1;    /* Key block + preamble for firmware 1 */
-  uint64_t verification_size_0;  /* Verification block 0 size in bytes */
-  uint64_t verification_size_1;  /* Verification block 1 size in bytes */
-
-  /* Shared data blob for data shared between LoadFirmware() and LoadKernel().
-   * This should be at least VB_SHARED_DATA_MIN_SIZE bytes long, and ideally
-   * is VB_SHARED_DATA_REC_SIZE bytes long. */
-  void* shared_data_blob;        /* Shared data blob buffer.  Pass this
-                                  * data to LoadKernel() in
-                                  * LoadKernelParams.shared_data_blob. */
-  uint32_t shared_data_size;     /* On input, set to size of shared data blob
-                                  * buffer, in bytes.  On output, this will
-                                  * contain the actual data size placed into
-                                  * the buffer.  Caller need only pass that
-                                  * much data to LoadKernel().*/
-
-  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;
-
-  /* Internal data for caller / GetFirmwareBody(). */
-  void* caller_internal;
-
-} LoadFirmwareParams;
-
-
-/* Functions provided by wrapper to LoadFirmware() */
-
-/* Get the firmware body data for [firmware_index], which is either
- * 0 (the first firmware image) or 1 (the second firmware image).
- *
- * This function must call UpdateFirmwareBodyHash() before returning,
- * to update the secure hash for the firmware image.  For best
- * performance, the reader should call this function periodically
- * during the read, so that updating the hash can be pipelined with
- * the read.  If the reader cannot update the hash during the read
- * process, it should call UpdateFirmwareBodyHash() on the entire
- * firmeware data after the read, before returning.
- *
- * Returns 0 if successful or non-zero if error. */
-int GetFirmwareBody(LoadFirmwareParams* params, uint64_t firmware_index);
-
-
-/* Functions provided by vboot_firmware to wrapper */
-
 /* Load the rewritable firmware.
  *
+ * Pass the common and firmware params from VbSelectFirmware(), and a
+ * VbNvContext.  Caller is responsible for calling VbNvSetup() and
+ * VbNvTeardown() on the VbNvContext.
+ *
  * Returns VBERROR_SUCCESS if successful.  If unsuccessful, sets a recovery
  * reason via VbNvStorage and returns an error code. */
-int LoadFirmware(LoadFirmwareParams* params);
-
-
-/* Update the data hash for the current firmware image, extending it
- * by [size] bytes stored in [*data].  This function must only be
- * called inside GetFirmwareBody(). */
-void UpdateFirmwareBodyHash(LoadFirmwareParams* params,
-                            uint8_t* data, uint32_t size);
+int LoadFirmware(VbCommonParams* cparams, VbSelectFirmwareParams* fparams,
+                 VbNvContext* vnc);
 
 #endif  /* VBOOT_REFERENCE_LOAD_FIRMWARE_FW_H_ */
diff --git a/firmware/lib/vboot_api_firmware.c b/firmware/lib/vboot_api_firmware.c
index 303845b1..284dff24 100644
--- a/firmware/lib/vboot_api_firmware.c
+++ b/firmware/lib/vboot_api_firmware.c
@@ -25,7 +25,6 @@ static void VbSfRequestRecovery(VbNvContext *vnc, uint32_t recovery_request) {
 VbError_t VbSelectFirmware(VbCommonParams* cparams,
                            VbSelectFirmwareParams* fparams) {
   VbSharedDataHeader* shared = (VbSharedDataHeader*)cparams->shared_data_blob;
-  LoadFirmwareParams p;
   VbNvContext vnc;
   VbError_t retval = VBERROR_UNKNOWN; /* Assume error until proven successful */
   int is_rec = (shared->recovery_reason ? 1 : 0);
@@ -82,30 +81,8 @@ VbError_t VbSelectFirmware(VbCommonParams* cparams,
     fparams->selected_firmware = VB_SELECT_FIRMWARE_RECOVERY;
 
   } else {
-    /* Check the RW firmware */
-    /* Copy parameters from wrapper API structs to old struct */
-    p.gbb_data              = cparams->gbb_data;
-    p.gbb_size              = cparams->gbb_size;
-    p.shared_data_blob      = cparams->shared_data_blob;
-    p.shared_data_size      = cparams->shared_data_size;
-    p.nv_context            = &vnc;
-
-    p.verification_block_0  = fparams->verification_block_A;
-    p.verification_block_1  = fparams->verification_block_B;
-    p.verification_size_0   = fparams->verification_size_A;
-    p.verification_size_1   = fparams->verification_size_B;
-
-    /* Use vboot_context and caller_internal to link our params with
-     * LoadFirmware()'s params. */
-    // TODO: clean up LoadFirmware() to use common params?
-    p.caller_internal = (void*)cparams;
-    cparams->vboot_context = (void*)&p;
-
     /* Chain to LoadFirmware() */
-    retval = LoadFirmware(&p);
-
-    /* Copy amount of used shared data back to the wrapper API struct */
-    cparams->shared_data_size = (uint32_t)p.shared_data_size;
+    retval = LoadFirmware(cparams, fparams, &vnc);
 
     /* Exit if we failed to find an acceptable firmware */
     if (VBERROR_SUCCESS != retval)
@@ -175,27 +152,3 @@ VbSelectFirmware_exit:
 
   return retval;
 }
-
-
-/* TODO: Move this inside vboot_firmware.c; for now this just translates to
- * the original function call. */
-void VbUpdateFirmwareBodyHash(VbCommonParams* cparams, uint8_t* data,
-                              uint32_t size) {
-  LoadFirmwareParams* lfparams = (LoadFirmwareParams*)cparams->vboot_context;
-
-  UpdateFirmwareBodyHash(lfparams, data, size);
-}
-
-
-/* Translation layer from LoadFirmware()'s GetFirmwareBody() to the new
- * wrapper API call.
- *
- * TODO: call directly from LoadFirmware() */
-int GetFirmwareBody(LoadFirmwareParams* lfparams, uint64_t index) {
-  VbCommonParams* cparams = (VbCommonParams*)lfparams->caller_internal;
-  VbError_t rv;
-
-  rv = VbExHashFirmwareBody(cparams, (index ? VB_SELECT_FIRMWARE_B :
-                                      VB_SELECT_FIRMWARE_A));
-  return (VBERROR_SUCCESS == rv ? 0 : 1);
-}
diff --git a/firmware/lib/vboot_firmware.c b/firmware/lib/vboot_firmware.c
index 0d1a307a..a2936394 100644
--- a/firmware/lib/vboot_firmware.c
+++ b/firmware/lib/vboot_firmware.c
@@ -19,26 +19,26 @@
  * good way to pass the params struct back to us. */
 typedef struct VbLoadFirmwareInternal {
   DigestContext body_digest_context;
-  uint64_t body_size_accum;
+  uint32_t body_size_accum;
 } VbLoadFirmwareInternal;
 
 
-void UpdateFirmwareBodyHash(LoadFirmwareParams* params,
-                             uint8_t* data, uint32_t size) {
+void VbUpdateFirmwareBodyHash(VbCommonParams* cparams,
+                              uint8_t* data, uint32_t size) {
   VbLoadFirmwareInternal* lfi =
-      (VbLoadFirmwareInternal*)params->load_firmware_internal;
+      (VbLoadFirmwareInternal*)cparams->vboot_context;
 
   DigestUpdate(&lfi->body_digest_context, data, size);
   lfi->body_size_accum += size;
 }
 
 
-int LoadFirmware(LoadFirmwareParams* params) {
-  VbSharedDataHeader* shared = (VbSharedDataHeader*)params->shared_data_blob;
-  GoogleBinaryBlockHeader* gbb = (GoogleBinaryBlockHeader*)params->gbb_data;
+int LoadFirmware(VbCommonParams* cparams, VbSelectFirmwareParams* fparams,
+                 VbNvContext* vnc) {
+  VbSharedDataHeader* shared = (VbSharedDataHeader*)cparams->shared_data_blob;
+  GoogleBinaryBlockHeader* gbb = (GoogleBinaryBlockHeader*)cparams->gbb_data;
   VbPublicKey* root_key;
   VbLoadFirmwareInternal* lfi;
-  VbNvContext* vnc = params->nv_context;
 
   uint32_t try_b_count;
   uint32_t lowest_version = 0xFFFFFFFF;
@@ -94,12 +94,12 @@ int LoadFirmware(LoadFirmwareParams* params) {
 
   /* Allocate our internal data */
   lfi = (VbLoadFirmwareInternal*)VbExMalloc(sizeof(VbLoadFirmwareInternal));
-  params->load_firmware_internal = (uint8_t*)lfi;
+  cparams->vboot_context = (void*)lfi;
 
   /* Loop over indices */
   for (i = 0; i < 2; i++) {
     VbKeyBlockHeader* key_block;
-    uint64_t vblock_size;
+    uint32_t vblock_size;
     VbFirmwarePreambleHeader* preamble;
     RSAPublicKey* data_key;
     uint64_t key_version;
@@ -110,12 +110,12 @@ int LoadFirmware(LoadFirmwareParams* params) {
     /* If try B count is non-zero try firmware B first */
     index = (try_b_count ? 1 - i : i);
     if (0 == index) {
-      key_block = (VbKeyBlockHeader*)params->verification_block_0;
-      vblock_size = params->verification_size_0;
+      key_block = (VbKeyBlockHeader*)fparams->verification_block_A;
+      vblock_size = fparams->verification_size_A;
       check_result = &shared->check_fw_a_result;
     } else {
-      key_block = (VbKeyBlockHeader*)params->verification_block_1;
-      vblock_size = params->verification_size_1;
+      key_block = (VbKeyBlockHeader*)fparams->verification_block_B;
+      vblock_size = fparams->verification_size_B;
       check_result = &shared->check_fw_b_result;
     }
 
@@ -222,12 +222,16 @@ int LoadFirmware(LoadFirmwareParams* params) {
       shared->flags |= VBSD_LF_USE_RO_NORMAL;
 
     } else {
+      VbError_t rv;
+
       /* Read the firmware data */
       VBPERFSTART("VB_RFD");
       DigestInit(&lfi->body_digest_context, data_key->algorithm);
       lfi->body_size_accum = 0;
-      if (0 != GetFirmwareBody(params, index)) {
-        VBDEBUG(("GetFirmwareBody() failed for index %d\n", index));
+      rv = VbExHashFirmwareBody(cparams, (index ? VB_SELECT_FIRMWARE_B :
+                                          VB_SELECT_FIRMWARE_A));
+      if (VBERROR_SUCCESS != rv) {
+        VBDEBUG(("VbExHashFirmwareBody() failed for index %d\n", index));
         *check_result = VBSD_LF_CHECK_GET_FW_BODY;
         RSAPublicKeyFree(data_key);
         VBPERFEND("VB_RFD");
@@ -290,7 +294,7 @@ int LoadFirmware(LoadFirmwareParams* params) {
 
   /* Free internal data */
   VbExFree(lfi);
-  params->load_firmware_internal = NULL;
+  cparams->vboot_context = NULL;
 
   /* Handle finding good firmware */
   if (good_index >= 0) {
@@ -326,9 +330,5 @@ LoadFirmwareExit:
   VbNvSet(vnc, VBNV_RECOVERY_REQUEST, VBERROR_SUCCESS != retval ?
           recovery : VBNV_RECOVERY_NOT_REQUESTED);
 
-  /* 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
-   * buffer. */
-
   return retval;
 }
diff --git a/firmware/linktest/main.c b/firmware/linktest/main.c
index 71d7ba1b..af5d8d65 100644
--- a/firmware/linktest/main.c
+++ b/firmware/linktest/main.c
@@ -22,8 +22,7 @@ int main(void)
   GptUpdateKernelEntry(0, 0);
 
   /* load_firmware_fw.h */
-  UpdateFirmwareBodyHash(0, 0, 0);
-  LoadFirmware(0);
+  LoadFirmware(0, 0, 0);
 
   /* load_kernel_fw.h */
   LoadKernel(0);
diff --git a/utility/Makefile b/utility/Makefile
index 9ae4d28c..6e7af055 100644
--- a/utility/Makefile
+++ b/utility/Makefile
@@ -21,7 +21,6 @@ TARGET_NAMES = crossystem \
 		dumpRSAPublicKey \
 		dump_kernel_config \
 		gbb_utility \
-		load_firmware_test \
 		load_kernel_test \
 		pad_digest_utility \
 		signature_digest_utility \
@@ -85,8 +84,9 @@ ${BUILD_ROOT}/bmpblk_utility: ${BUILD_ROOT}/bmpblk_utility.o \
 				${BUILD_ROOT}/efidecompress.o
 	$(CXX) -llzma -lyaml $(CFLAGS) $^ -o $@
 
-${BUILD_ROOT}/load_firmware_test: load_firmware_test.c $(LIBS)
-	$(CC) $(CFLAGS) $< -o $@ $(LIBS) -lcrypto
+# TODO: rewrite load_firmware_test to support new wrapper API
+#${BUILD_ROOT}/load_firmware_test: load_firmware_test.c $(LIBS)
+#	$(CC) $(CFLAGS) $< -o $@ $(LIBS) -lcrypto
 
 ${BUILD_ROOT}/load_kernel_test: load_kernel_test.c $(LIBS)
 	$(CC) $(CFLAGS) $< -o $@ $(LIBS) -lcrypto
diff --git a/utility/load_firmware_test.c b/utility/load_firmware_test.c
index 4f698872..62b9c348 100644
--- a/utility/load_firmware_test.c
+++ b/utility/load_firmware_test.c
@@ -37,9 +37,11 @@ int GetFirmwareBody(LoadFirmwareParams* params, uint64_t firmware_index) {
   if (firmware_index != 0 && firmware_index != 1)
     return 1;
 
-  UpdateFirmwareBodyHash(params,
+#if 0
+  VbUpdateFirmwareBodyHash(params,
                          ci->firmware[firmware_index].fw,
                          ci->firmware[firmware_index].size);
+#endif
 
   return 0;
 }