Add support for merged normal+dev firmware

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

When this is merged into an actual firmware build, can test it:
* dev switch off -> no dev screen, won't boot self-signed kernel
* dev switch on --> dev warning screen, will boot self-signed kernel
(e.g., it acts like the Cr-48)

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

4 files changed, 31 insertions, 37 deletions

diff --git a/firmware/include/load_kernel_fw.h b/firmware/include/load_kernel_fw.h
index 6dd6c93d..737b7c95 100644
--- a/firmware/include/load_kernel_fw.h
+++ b/firmware/include/load_kernel_fw.h
@@ -30,7 +30,9 @@
 #define BOOT_FLAG_RECOVERY  UINT64_C(0x02)
 /* Skip check of kernel buffer address */
 #define BOOT_FLAG_SKIP_ADDR_CHECK UINT64_C(0x04)
-/* Active main firmware is developer-type, not normal-type or recovery-type. */
+/* TODO: remove flag, once the firmware builds which call
+ * vboot_reference are updated.  This flag now does nothing.  See
+ * crosbug.com/17457. */
 #define BOOT_FLAG_DEV_FIRMWARE UINT64_C(0x08)
 
 typedef struct LoadKernelParams {
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index c5e97dd7..11ff828e 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -406,7 +406,6 @@ VbError_t VbBootNormal(VbCommonParams* cparams, LoadKernelParams* p) {
 }
 
 
-#ifdef BUILD_FVDEVELOPER
 /* Developer mode delays.  All must be multiples of DEV_DELAY_INCREMENT */
 #define DEV_DELAY_INCREMENT 250  /* Delay each loop, in msec */
 #define DEV_DELAY_BEEP1 20000    /* Beep for first time at this time */
@@ -479,8 +478,6 @@ VbError_t VbBootDeveloper(VbCommonParams* cparams, LoadKernelParams* p) {
   return VbTryLoadKernel(cparams, p, VB_DISK_FLAG_FIXED);
 }
 
-#endif  /* BUILD_FVDEVELOPER */
-
 
 /* Delay between disk checks in recovery mode */
 #define REC_DELAY_INCREMENT 250
@@ -557,7 +554,7 @@ VbError_t VbBootRecovery(VbCommonParams* cparams, LoadKernelParams* p) {
 VbError_t VbSelectAndLoadKernel(VbCommonParams* cparams,
                                 VbSelectAndLoadKernelParams* kparams) {
   VbSharedDataHeader* shared = (VbSharedDataHeader*)cparams->shared_data_blob;
-  VbError_t retval;
+  VbError_t retval = VBERROR_SUCCESS;
   LoadKernelParams p;
 
   VBDEBUG(("VbSelectAndLoadKernel() start\n"));
@@ -587,23 +584,39 @@ VbError_t VbSelectAndLoadKernel(VbCommonParams* cparams,
   if (shared->flags & VBSD_BOOT_DEV_SWITCH_ON)
     p.boot_flags |= BOOT_FLAG_DEVELOPER;
 
+  /* Handle separate normal and developer firmware builds. */
+#if defined(VBOOT_FIRMWARE_TYPE_NORMAL)
+  /* Normal-type firmware always acts like the dev switch is off. */
+  p.boot_flags &= ~BOOT_FLAG_DEVELOPER;
+#elif defined(VBOOT_FIRMWARE_TYPE_DEVELOPER)
+  /* Developer-type firmware fails if the dev switch is off. */
+  if (!(p.boot_flags & BOOT_FLAG_DEVELOPER)) {
+    /* Dev firmware should be signed with a key that only verifies
+     * when the dev switch is on, so we should never get here. */
+    VBDEBUG(("Developer firmware called with dev switch off!\n"));
+    VbSetRecoveryRequest(VBNV_RECOVERY_RW_DEV_MISMATCH);
+    retval = 1;
+  }
+#else
+  /* Recovery firmware, or merged normal+developer firmware.  No
+   * need to override flags. */
+#endif
+
   /* Select boot path */
-  if (shared->recovery_reason) {
+  if (VBERROR_SUCCESS != retval) {
+    /* Failure during setup; don't attempt booting a kernel */
+  } else if (shared->recovery_reason) {
     /* Recovery boot */
     p.boot_flags |= BOOT_FLAG_RECOVERY;
     retval = VbBootRecovery(cparams, &p);
     VbDisplayScreen(cparams, VB_SCREEN_BLANK, 0);
-  } else {
-    /* TODO: vboot compiler define for developer mode; this is the H2C one */
-#ifdef BUILD_FVDEVELOPER
+  } else if (p.boot_flags & BOOT_FLAG_DEVELOPER) {
     /* Developer boot */
-    p.boot_flags |= BOOT_FLAG_DEV_FIRMWARE;
     retval = VbBootDeveloper(cparams, &p);
     VbDisplayScreen(cparams, VB_SCREEN_BLANK, 0);
-#else
+  } else {
     /* Normal boot */
     retval = VbBootNormal(cparams, &p);
-#endif
   }
 
   if (VBERROR_SUCCESS == retval) {
diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c
index 97d32d34..07fd0c7d 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -22,8 +22,8 @@
 
 typedef enum BootMode {
   kBootRecovery = 0,  /* Recovery firmware, regardless of dev switch position */
-  kBootNormal = 1,    /* Normal firmware */
-  kBootDev = 2        /* Dev firmware AND dev switch is on */
+  kBootNormal = 1,    /* Normal boot - kernel must be verified */
+  kBootDev = 2        /* Developer boot - self-signed kernel ok */
 } BootMode;
 
 
@@ -169,13 +169,10 @@ int LoadKernel(LoadKernelParams* params) {
   dev_switch = (BOOT_FLAG_DEVELOPER & params->boot_flags ? 1 : 0);
   if (rec_switch)
     boot_mode = kBootRecovery;
-  else if (BOOT_FLAG_DEV_FIRMWARE & params->boot_flags)
+  else if (dev_switch)
     boot_mode = kBootDev;
-  else {
-    /* Normal firmware */
+  else
     boot_mode = kBootNormal;
-    dev_switch = 0;  /* Always do a fully verified boot */
-  }
 
   /* Set up tracking for this call.  This wraps around if called many times,
    * so we need to initialize the call entry each time. */
@@ -220,15 +217,6 @@ int LoadKernel(LoadKernelParams* params) {
     goto LoadKernelExit;
   }
 
-  if (kBootDev == boot_mode && !dev_switch) {
-      /* Dev firmware should be signed such that it never boots with the dev
-       * switch is off; so something is terribly wrong. */
-    VBDEBUG(("LoadKernel() called with dev firmware but dev switch off\n"));
-    shcall->check_result = VBSD_LKC_CHECK_DEV_SWITCH_MISMATCH;
-    recovery = VBNV_RECOVERY_RW_DEV_MISMATCH;
-    goto LoadKernelExit;
-  }
-
   if (kBootRecovery == boot_mode) {
     /* Use the recovery key to verify the kernel */
     kernel_subkey = (VbPublicKey*)((uint8_t*)gbb + gbb->recovery_key_offset);
diff --git a/utility/load_kernel_test.c b/utility/load_kernel_test.c
index 6918e8c0..00ed49dd 100644
--- a/utility/load_kernel_test.c
+++ b/utility/load_kernel_test.c
@@ -192,15 +192,6 @@ int main(int argc, char* argv[]) {
    * heap instead of its actual target address in the firmware. */
   lkp.boot_flags |= BOOT_FLAG_SKIP_ADDR_CHECK;
 
-  /* If the boot flags are for developer mode, non-recovery, add the dev-type
-   * firmware bit.  LoadKernel() masks off the developer bit if the dev
-   * firmware bit is absent, to keep normal firmware from verifying dev
-   * kernels. */
-  if ((lkp.boot_flags & BOOT_FLAG_DEVELOPER)
-      && !(lkp.boot_flags & BOOT_FLAG_RECOVERY)) {
-    lkp.boot_flags |= BOOT_FLAG_DEV_FIRMWARE;
-  }
-
   printf("bootflags = %" PRIu64 "\n", lkp.boot_flags);
 
   /* Get image size */