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>
diff --git a/firmware/include/load_firmware_fw.h b/firmware/include/load_firmware_fw.h
index c8034c1..1189654 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 @@
} 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 @@
int GetFirmwareBody(LoadFirmwareParams* params, uint64_t firmware_index);
-/* Functions provided by verified boot library to PEI */
+/* Functions provided by vboot_firmware to wrapper */
-/* Early setup for LoadFirmware(). This should be called as soon as the TPM
- * is available in the boot process.
+/* Load the rewritable firmware.
*
- * Returns LOAD_FIRMWARE_SUCCESS if successful, error code on failure. */
-int LoadFirmwareSetup(void);
-
-/* Attempts to 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 @@
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 96a78d9..dfbd4e8 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 @@
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 e61340e..79a461b 100644
--- a/firmware/include/vboot_api.h
+++ b/firmware/include/vboot_api.h
@@ -90,7 +90,11 @@
/* 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 6ffca2d..6a3f89e 100644
--- a/firmware/lib/vboot_api_firmware.c
+++ b/firmware/lib/vboot_api_firmware.c
@@ -36,7 +36,6 @@
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 @@
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 @@
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 ca2928d..22bddf9 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -342,7 +342,7 @@
* 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 @@
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 15aef63..ce6206c 100644
--- a/firmware/lib/vboot_firmware.c
+++ b/firmware/lib/vboot_firmware.c
@@ -48,7 +48,7 @@
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 @@
/* 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 @@
/* 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 @@
/* 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 @@
/* 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 @@
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 d8b2568..b3c06b3 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -121,7 +121,7 @@
/* 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 @@
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 @@
!params->kernel_buffer ||
!params->kernel_buffer_size) {
VBDEBUG(("LoadKernel() called with invalid params\n"));
+ retval = VBERROR_INVALID_PARAMETER;
goto LoadKernelExit;
}
@@ -191,17 +192,11 @@
/* 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 @@
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 @@
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 6584977..4f69887 100644
--- a/utility/load_firmware_test.c
+++ b/utility/load_firmware_test.c
@@ -30,8 +30,6 @@
/* 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 @@
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 @@
* 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 @@
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 00ed49d..1f8cc2a 100644
--- a/utility/load_kernel_test.c
+++ b/utility/load_kernel_test.c
@@ -83,7 +83,8 @@
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 @@
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 @@
fclose(image_file);
free(lkp.kernel_buffer);
- return rv != LOAD_KERNEL_SUCCESS;
+ return rv != VBERROR_SUCCESS;
}