Refactor LoadFirmware() and LoadKernel() to return VbError_t

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

Change-Id: I1f60654fef84e26ee15fa8bdaacdb87fb1ddf69e
Reviewed-on: http://gerrit.chromium.org/gerrit/4676
Reviewed-by: Stefan Reinauer <reinauer@chromium.org>
Tested-by: Randall Spangler <rspangler@chromium.org>

9 files changed, 58 insertions, 117 deletions

diff --git a/firmware/include/load_firmware_fw.h b/firmware/include/load_firmware_fw.h
index c8034c14..1189654f 100644
--- a/firmware/include/load_firmware_fw.h
+++ b/firmware/include/load_firmware_fw.h
@@ -13,12 +13,6 @@
 #include "vboot_nvstorage.h"
 #include "vboot_struct.h"
 
-/* Return codes for LoadFirmware() and S3Resume(). */
-#define LOAD_FIRMWARE_SUCCESS 0   /* Success */
-#define LOAD_FIRMWARE_RECOVERY 1  /* Reboot to recovery mode.  The specific
-                                   * recovery reason has been set in
-                                   * VbNvContext (VBNV_RECOVERY_REQUEST). */
-
 typedef struct LoadFirmwareParams {
   /* Inputs to LoadFirmware() */
   void* gbb_data;                /* Pointer to GBB data */
@@ -56,7 +50,7 @@ typedef struct LoadFirmwareParams {
 } LoadFirmwareParams;
 
 
-/* Functions provided by PEI to LoadFirmware() */
+/* 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).
@@ -73,17 +67,12 @@ typedef struct LoadFirmwareParams {
 int GetFirmwareBody(LoadFirmwareParams* params, uint64_t firmware_index);
 
 
-/* Functions provided by verified boot library to PEI */
-
-/* Early setup for LoadFirmware().  This should be called as soon as the TPM
- * is available in the boot process.
- *
- * Returns LOAD_FIRMWARE_SUCCESS if successful, error code on failure. */
-int LoadFirmwareSetup(void);
+/* Functions provided by vboot_firmware to wrapper */
 
-/* Attempts to load the rewritable firmware.
+/* Load the rewritable firmware.
  *
- * Returns LOAD_FIRMWARE_SUCCESS if successful, error code on failure. */
+ * Returns VBERROR_SUCCESS if successful.  If unsuccessful, sets a recovery
+ * reason via VbNvStorage and returns an error code. */
 int LoadFirmware(LoadFirmwareParams* params);
 
 
@@ -93,9 +82,4 @@ int LoadFirmware(LoadFirmwareParams* params);
 void UpdateFirmwareBodyHash(LoadFirmwareParams* params,
                             uint8_t* data, uint32_t size);
 
-/* Handle S3 resume.
- *
- * Returns LOAD_FIRMWARE_SUCCESS if successful, error code on failure. */
-int S3Resume(void);
-
 #endif  /* VBOOT_REFERENCE_LOAD_FIRMWARE_FW_H_ */
diff --git a/firmware/include/load_kernel_fw.h b/firmware/include/load_kernel_fw.h
index 96a78d94..dfbd4e89 100644
--- a/firmware/include/load_kernel_fw.h
+++ b/firmware/include/load_kernel_fw.h
@@ -15,13 +15,6 @@
 
 /* Interface provided by verified boot library to BDS */
 
-/* Return codes for LoadKernel() */
-#define LOAD_KERNEL_SUCCESS 0    /* Success; good kernel found on device */
-#define LOAD_KERNEL_NOT_FOUND 1  /* No kernel found on device */
-#define LOAD_KERNEL_INVALID 2    /* Only invalid kernels found on device */
-#define LOAD_KERNEL_RECOVERY 3   /* Internal error; reboot to recovery mode */
-
-
 /* Boot flags for LoadKernel().boot_flags */
 /* Developer switch is on */
 #define BOOT_FLAG_DEVELOPER UINT64_C(0x01)
@@ -67,10 +60,11 @@ typedef struct LoadKernelParams {
   uint8_t  partition_guid[16];  /* UniquePartitionGuid for boot partition */
 } LoadKernelParams;
 
-int LoadKernel(LoadKernelParams* params);
+VbError_t LoadKernel(LoadKernelParams* params);
 /* Attempts to load the kernel from the current device.
  *
- * Returns LOAD_KERNEL_SUCCESS if successful, error code on failure. */
+ * Returns VBERROR_SUCCESS if successful.  If unsuccessful, sets a recovery
+ * reason via VbNvStorage and returns an error code. */
 
 
 typedef struct KernelBootloaderOptions {
diff --git a/firmware/include/vboot_api.h b/firmware/include/vboot_api.h
index e61340ef..79a461bf 100644
--- a/firmware/include/vboot_api.h
+++ b/firmware/include/vboot_api.h
@@ -90,7 +90,11 @@ enum VbErrorPredefined_t {
   /* Invalid bitmap volume */
   VBERROR_INVALID_BMPFV                 = 0x10014,
   /* Invalid screen index */
-  VBERROR_INVALID_SCREEN_INDEX          = 0x10015
+  VBERROR_INVALID_SCREEN_INDEX          = 0x10015,
+  /* Simulated (test) error */
+  VBERROR_SIMULATED                     = 0x10016,
+  /* Invalid parameter */
+  VBERROR_INVALID_PARAMETER             = 0x10017
 };
 
 
diff --git a/firmware/lib/vboot_api_firmware.c b/firmware/lib/vboot_api_firmware.c
index 6ffca2d0..6a3f89e8 100644
--- a/firmware/lib/vboot_api_firmware.c
+++ b/firmware/lib/vboot_api_firmware.c
@@ -36,7 +36,6 @@ VbError_t VbSelectFirmware(VbCommonParams* cparams,
   int is_dev = (shared->flags & VBSD_BOOT_DEV_SWITCH_ON ? 1 : 0);
   uint32_t tpm_version = 0;
   uint32_t tpm_status = 0;
-  int rv;
 
   /* Start timer */
   shared->timer_vb_select_firmware_enter = VbExGetTimer();
@@ -107,7 +106,7 @@ VbError_t VbSelectFirmware(VbCommonParams* cparams,
     cparams->vboot_context = (void*)&p;
 
     /* Chain to LoadFirmware() */
-    rv = LoadFirmware(&p);
+    retval = LoadFirmware(&p);
 
     /* Save NV storage, if necessary */
     if (vnc.raw_changed)
@@ -117,10 +116,8 @@ VbError_t VbSelectFirmware(VbCommonParams* cparams,
     cparams->shared_data_size = (uint32_t)p.shared_data_size;
 
     /* Exit if we failed to find an acceptable firmware */
-    if (LOAD_FIRMWARE_SUCCESS != rv) {
-      retval = VBERROR_LOAD_FIRMWARE;
+    if (VBERROR_SUCCESS != retval)
       goto VbSelectFirmware_exit;
-    }
 
     /* Translate the selected firmware path */
     if (shared->flags & VBSD_LF_USE_RO_NORMAL) {
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index ca2928df..22bddf9f 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -342,7 +342,7 @@ 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 lk_retval = LOAD_KERNEL_RECOVERY;
+  int retval = VBERROR_UNKNOWN;
   VbDiskInfo* disk_info = NULL;
   uint32_t disk_count = 0;
   uint32_t i;
@@ -369,38 +369,25 @@ uint32_t VbTryLoadKernel(VbCommonParams* cparams, LoadKernelParams* p,
     p->disk_handle = disk_info[i].handle;
     p->bytes_per_lba = disk_info[i].bytes_per_lba;
     p->ending_lba = disk_info[i].lba_count - 1;
-    lk_retval = LoadKernel(p);
-    VBDEBUG(("VbTryLoadKernel() LoadKernel() returned %d\n", lk_retval));
+    retval = LoadKernel(p);
+    VBDEBUG(("VbTryLoadKernel() LoadKernel() returned %d\n", retval));
 
     /* Stop now if we found a kernel */
     /* TODO: If recovery requested, should track the farthest we get, instead
      * of just returning the value from the last disk attempted. */
-    if (LOAD_KERNEL_SUCCESS == lk_retval)
+    if (VBERROR_SUCCESS == retval)
       break;
   }
 
   /* If we didn't succeed, don't return a disk handle */
-  if (LOAD_KERNEL_SUCCESS != lk_retval)
+  if (VBERROR_SUCCESS != retval)
     p->disk_handle = NULL;
 
   VbExDiskFreeInfo(disk_info, p->disk_handle);
 
-  /* Translate return codes */
-  switch (lk_retval) {
-    case LOAD_KERNEL_SUCCESS:
-      return VBERROR_SUCCESS;
-    case LOAD_KERNEL_NOT_FOUND:
-      VbSetRecoveryRequest(VBNV_RECOVERY_RW_NO_OS);
-      return VBERROR_NO_KERNEL_FOUND;
-    case LOAD_KERNEL_INVALID:
-      VbSetRecoveryRequest(VBNV_RECOVERY_RW_INVALID_OS);
-      return VBERROR_INVALID_KERNEL_FOUND;
-    case LOAD_KERNEL_RECOVERY:
-      return VBERROR_LOAD_KERNEL_RECOVERY;
-    default:
-      VbSetRecoveryRequest(VBNV_RECOVERY_RW_UNSPECIFIED);
-      return VBERROR_LOAD_KERNEL;
-  }
+  /* Pass through return code.  Recovery reason (if any) has already been set
+   * by LoadKernel(). */
+  return retval;
 }
 
 
diff --git a/firmware/lib/vboot_firmware.c b/firmware/lib/vboot_firmware.c
index 15aef635..ce6206c2 100644
--- a/firmware/lib/vboot_firmware.c
+++ b/firmware/lib/vboot_firmware.c
@@ -48,7 +48,7 @@ int LoadFirmware(LoadFirmwareParams* params) {
   int index;
   int i;
 
-  int retval = LOAD_FIRMWARE_RECOVERY;
+  int retval = VBERROR_UNKNOWN;
   int recovery = VBNV_RECOVERY_RO_UNSPECIFIED;
 
   /* Clear output params in case we fail */
@@ -67,19 +67,18 @@ int LoadFirmware(LoadFirmwareParams* params) {
     /* Clear test params so we don't repeat the error */
     VbNvSet(vnc, VBNV_TEST_ERROR_FUNC, 0);
     VbNvSet(vnc, VBNV_TEST_ERROR_NUM, 0);
-    /* Handle error codes */
-    switch (test_err) {
-      case LOAD_FIRMWARE_RECOVERY:
-        recovery = VBNV_RECOVERY_RO_TEST_LF;
-        goto LoadFirmwareExit;
-      default:
-        break;
+    /* All error codes currently map to simulated error */
+    if (test_err) {
+      recovery = VBNV_RECOVERY_RO_TEST_LF;
+      retval = VBERROR_SIMULATED;
+      goto LoadFirmwareExit;
     }
   }
 
   /* Must have a root key from the GBB */
   if (!gbb) {
     VBDEBUG(("No GBB\n"));
+    retval = VBERROR_INVALID_GBB;
     goto LoadFirmwareExit;
   }
   root_key = (VbPublicKey*)((uint8_t*)gbb + gbb->rootkey_offset);
@@ -98,9 +97,6 @@ int LoadFirmware(LoadFirmwareParams* params) {
 
   /* Allocate our internal data */
   lfi = (VbLoadFirmwareInternal*)VbExMalloc(sizeof(VbLoadFirmwareInternal));
-  if (!lfi)
-    return LOAD_FIRMWARE_RECOVERY;
-
   params->load_firmware_internal = (uint8_t*)lfi;
 
   /* Loop over indices */
@@ -309,7 +305,7 @@ int LoadFirmware(LoadFirmwareParams* params) {
 
     /* Success */
     VBDEBUG(("Will boot firmware index %d\n", (int)shared->firmware_index));
-    retval = LOAD_FIRMWARE_SUCCESS;
+    retval = VBERROR_SUCCESS;
   } else {
     uint8_t a = shared->check_fw_a_result;
     uint8_t b = shared->check_fw_b_result;
@@ -318,6 +314,7 @@ int LoadFirmware(LoadFirmwareParams* params) {
     /* No good firmware, so go to recovery mode. */
     VBDEBUG(("Alas, no good firmware.\n"));
     recovery = VBNV_RECOVERY_RO_INVALID_RW;
+    retval = VBERROR_LOAD_FIRMWARE;
 
     /* If the best check result fits in the range of recovery reasons, provide
      * more detail on how far we got in validation. */
@@ -329,7 +326,7 @@ int LoadFirmware(LoadFirmwareParams* params) {
 
 LoadFirmwareExit:
   /* Store recovery request, if any, then tear down non-volatile storage */
-  VbNvSet(vnc, VBNV_RECOVERY_REQUEST, LOAD_FIRMWARE_RECOVERY == retval ?
+  VbNvSet(vnc, VBNV_RECOVERY_REQUEST, VBERROR_SUCCESS != retval ?
           recovery : VBNV_RECOVERY_NOT_REQUESTED);
   VbNvTeardown(vnc);
 
diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c
index d8b25681..b3c06b35 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -121,7 +121,7 @@ int WriteAndFreeGptData(VbExDiskHandle_t disk_handle, GptData* gptdata) {
 /* disable MSVC warning on const logical expression (as in } while(0);) */
 __pragma(warning(disable: 4127))
 
-int LoadKernel(LoadKernelParams* params) {
+VbError_t LoadKernel(LoadKernelParams* params) {
   VbSharedDataHeader* shared = (VbSharedDataHeader*)params->shared_data_blob;
   VbSharedDataKernelCall* shcall = NULL;
   VbNvContext* vnc = params->nv_context;
@@ -140,7 +140,7 @@ int LoadKernel(LoadKernelParams* params) {
   BootMode boot_mode;
   uint32_t test_err = 0;
 
-  int retval = LOAD_KERNEL_RECOVERY;
+  VbError_t retval = VBERROR_UNKNOWN;
   int recovery = VBNV_RECOVERY_RO_UNSPECIFIED;
 
   /* Setup NV storage */
@@ -153,6 +153,7 @@ int LoadKernel(LoadKernelParams* params) {
       !params->kernel_buffer ||
       !params->kernel_buffer_size) {
     VBDEBUG(("LoadKernel() called with invalid params\n"));
+    retval = VBERROR_INVALID_PARAMETER;
     goto LoadKernelExit;
   }
 
@@ -191,17 +192,11 @@ int LoadKernel(LoadKernelParams* params) {
     /* Clear test params so we don't repeat the error */
     VbNvSet(vnc, VBNV_TEST_ERROR_FUNC, 0);
     VbNvSet(vnc, VBNV_TEST_ERROR_NUM, 0);
-    /* Handle error codes */
-    switch (test_err) {
-      case LOAD_KERNEL_RECOVERY:
-        recovery = VBNV_RECOVERY_RW_TEST_LK;
-        goto LoadKernelExit;
-      case LOAD_KERNEL_NOT_FOUND:
-      case LOAD_KERNEL_INVALID:
-        retval = test_err;
-        goto LoadKernelExit;
-      default:
-        break;
+    /* All error codes currently map to simulated error */
+    if (test_err) {
+      recovery = VBNV_RECOVERY_RW_TEST_LK;
+      retval = VBERROR_SIMULATED;
+      goto LoadKernelExit;
     }
   }
 
@@ -210,6 +205,7 @@ int LoadKernel(LoadKernelParams* params) {
   kbuf_sectors = KBUF_SIZE / blba;
   if (0 == kbuf_sectors) {
     VBDEBUG(("LoadKernel() called with sector size > KBUF_SIZE\n"));
+    retval = VBERROR_INVALID_PARAMETER;
     goto LoadKernelExit;
   }
 
@@ -528,22 +524,21 @@ int LoadKernel(LoadKernelParams* params) {
       shared->kernel_version_tpm = lowest_version;
 
     /* Success! */
-    retval = LOAD_KERNEL_SUCCESS;
+    retval = VBERROR_SUCCESS;
+  } else if (found_partitions > 0) {
+    shcall->check_result = VBSD_LKC_CHECK_INVALID_PARTITIONS;
+    recovery = VBNV_RECOVERY_RW_INVALID_OS;
+    retval = VBERROR_INVALID_KERNEL_FOUND;
   } else {
-    shcall->check_result = (found_partitions > 0
-                            ? VBSD_LKC_CHECK_INVALID_PARTITIONS
-                            : VBSD_LKC_CHECK_NO_PARTITIONS);
-
-    /* TODO: differentiate between finding an invalid kernel
-     * (found_partitions>0) and not finding one at all.  Right now we
-     * treat them the same, and return LOAD_KERNEL_INVALID for both. */
-    retval = LOAD_KERNEL_INVALID;
+    shcall->check_result = VBSD_LKC_CHECK_NO_PARTITIONS;
+    recovery = VBNV_RECOVERY_RW_NO_OS;
+    retval = VBERROR_NO_KERNEL_FOUND;
   }
 
 LoadKernelExit:
 
   /* Store recovery request, if any, then tear down non-volatile storage */
-  VbNvSet(vnc, VBNV_RECOVERY_REQUEST, LOAD_KERNEL_RECOVERY == retval ?
+  VbNvSet(vnc, VBNV_RECOVERY_REQUEST, VBERROR_SUCCESS != retval ?
           recovery : VBNV_RECOVERY_NOT_REQUESTED);
   VbNvTeardown(vnc);
 
diff --git a/utility/load_firmware_test.c b/utility/load_firmware_test.c
index 6584977c..4f698872 100644
--- a/utility/load_firmware_test.c
+++ b/utility/load_firmware_test.c
@@ -30,8 +30,6 @@ static char* image_path = NULL;
 /* wrapper of FmapAreaIndex; print error when not found */
 int FmapAreaIndexOrError(const FmapHeader* fh, const FmapAreaHeader* ah,
     const char* name);
-/* return NULL on error */
-const char* status_string(int status);
 
 int GetFirmwareBody(LoadFirmwareParams* params, uint64_t firmware_index) {
   CallerInternal* ci = (CallerInternal*) params->caller_internal;
@@ -140,8 +138,8 @@ int DriveLoadFirmware(const void* base_of_rom, const void* fmap,
   LoadFirmwareParams lfp;
   CallerInternal ci;
 
-  const char* status_str;
-  int index, status;
+  VbError_t status;
+  int index;
 
   void** vblock_ptr[2] = {
     &lfp.verification_block_0, &lfp.verification_block_1
@@ -197,11 +195,7 @@ int DriveLoadFirmware(const void* base_of_rom, const void* fmap,
    * happening here. */
 
   status = LoadFirmware(&lfp);
-  status_str = status_string(status);
-  if (status_str)
-    printf("LoadFirmware returns %s\n", status_str);
-  else
-    printf("LoadFirmware returns unknown status code: %d\n", status);
+  printf("LoadFirmware returned: 0x%x\n", (int)status);
 
   free(lfp.shared_data_blob);
 
@@ -217,18 +211,6 @@ int FmapAreaIndexOrError(const FmapHeader* fh, const FmapAreaHeader* ah,
   return i;
 }
 
-/* Convert status returned by LoadFirmware to string. Return NULL on error. */
-const char* status_string(int status) {
-  switch (status) {
-    case LOAD_FIRMWARE_SUCCESS:
-      return "LOAD_FIRMWARE_SUCCESS";
-    case LOAD_FIRMWARE_RECOVERY:
-      return "LOAD_FIRMWARE_RECOVERY";
-    default:
-      return NULL;
-  }
-}
-
 int main(int argc, char* argv[]) {
   int i;
   int retval = 0;
diff --git a/utility/load_kernel_test.c b/utility/load_kernel_test.c
index 00ed49dd..1f8cc2a5 100644
--- a/utility/load_kernel_test.c
+++ b/utility/load_kernel_test.c
@@ -83,7 +83,8 @@ int main(int argc, char* argv[]) {
   uint8_t* key_blob = NULL;
   VbSharedDataHeader* shared;
   GoogleBinaryBlockHeader* gbb;
-  int rv, c, argsleft;
+  VbError_t rv;
+  int c, argsleft;
   int errorcnt = 0;
   char *e = 0;
 
@@ -218,7 +219,7 @@ int main(int argc, char* argv[]) {
   rv = LoadKernel(&lkp);
   printf("LoadKernel() returned %d\n", rv);
 
-  if (LOAD_KERNEL_SUCCESS == rv) {
+  if (VBERROR_SUCCESS == rv) {
     printf("Partition number:   %" PRIu64 "\n", lkp.partition_number);
     printf("Bootloader address: %" PRIu64 "\n", lkp.bootloader_address);
     printf("Bootloader size:    %" PRIu64 "\n", lkp.bootloader_size);
@@ -245,5 +246,5 @@ int main(int argc, char* argv[]) {
 
   fclose(image_file);
   free(lkp.kernel_buffer);
-  return rv != LOAD_KERNEL_SUCCESS;
+  return rv != VBERROR_SUCCESS;
 }