Assorted integration fixes.

MSVC does not like bitfields with extra bits in them, so it made the GptEntry struct too big.

Fixed a missing return value in LoadFirmware().

Added some debug output.

Fixed calls to SetupTPM().

Tested with 'make && make runtests'.  No errors.

Review URL: http://codereview.chromium.org/2865014

5 files changed, 48 insertions, 4 deletions

diff --git a/firmware/include/sysincludes.h b/firmware/include/sysincludes.h
index be7e701d..a509114b 100644
--- a/firmware/include/sysincludes.h
+++ b/firmware/include/sysincludes.h
@@ -30,6 +30,11 @@
 #define UINT64_RSHIFT(v, shiftby) (((uint64_t)(v)) >> (shiftby))
 #define UINT64_MULT32(v, multby)  (((uint64_t)(v)) * ((uint32_t)(multby)))
 
+/* Packing macros to support compilers such as MSVC that lack
+ * __attribute__((packed)) */
+#define PACK_START
+#define PACK_STOP
+
 #else
 #include "biosincludes.h"
 #endif
diff --git a/firmware/lib/cgptlib/include/gpt.h b/firmware/lib/cgptlib/include/gpt.h
index cd5e12b9..79cc8b2b 100644
--- a/firmware/lib/cgptlib/include/gpt.h
+++ b/firmware/lib/cgptlib/include/gpt.h
@@ -12,6 +12,8 @@
 
 #include "sysincludes.h"
 
+PACK_START  /* Support packing for MSVC */
+
 #define GPT_HEADER_SIGNATURE "EFI PART"
 #define GPT_HEADER_SIGNATURE_SIZE sizeof(GPT_HEADER_SIGNATURE)
 #define GPT_HEADER_REVISION 0x00010000
@@ -58,6 +60,8 @@ typedef struct {
   } u;
 } __attribute__((packed)) Guid;
 
+#define GUID_EXPECTED_SIZE GUID_SIZE
+
 /* Some constant values */
 extern const Guid guid_unused;
 extern const Guid guid_chromeos_kernel;
@@ -86,6 +90,8 @@ typedef struct {
   /* Remainder of sector is reserved and should be 0 */
 } __attribute__((packed)) GptHeader;
 
+#define GPTHEADER_EXPECTED_SIZE 92
+
 /* GPT partition entry defines the starting and ending LBAs of a partition.
  * It also contains the unique GUID, type, and attribute bits.
  *
@@ -98,8 +104,8 @@ typedef struct {
   uint64_t ending_lba;
   union {
     struct {
-     uint64_t         : 48;
-     uint16_t gpt_att : 16;
+      uint16_t reserved[3];
+      uint16_t gpt_att;
     } __attribute__((packed)) fields;
     uint64_t whole;
   } attrs;
@@ -107,4 +113,8 @@ typedef struct {
   /* Remainder of entry is reserved and should be 0 */
 } __attribute__((packed)) GptEntry;
 
+#define GPTENTRY_EXPECTED_SIZE 128
+
+PACK_STOP  /* Support packing for MSVC */
+
 #endif  /* VBOOT_REFERENCE_CGPTLIB_GPT_H_ */
diff --git a/firmware/lib/include/vboot_struct.h b/firmware/lib/include/vboot_struct.h
index a60615c2..b33e46d5 100644
--- a/firmware/lib/include/vboot_struct.h
+++ b/firmware/lib/include/vboot_struct.h
@@ -11,6 +11,7 @@
 
 #include "sysincludes.h"
 
+PACK_START  /* Support packing for MSVC */
 
 /* Public key data */
 typedef struct VbPublicKey {
@@ -21,6 +22,8 @@ typedef struct VbPublicKey {
   uint64_t key_version;    /* Key version */
 } __attribute__((packed)) VbPublicKey;
 
+#define EXPECTED_VBPUBLICKEY_SIZE 32
+
 
 /* Signature data (a secure hash, possibly signed) */
 typedef struct VbSignature {
@@ -30,6 +33,8 @@ typedef struct VbSignature {
   uint64_t data_size;   /* Size of the data block which was signed in bytes */
 } __attribute__((packed)) VbSignature;
 
+#define EXPECTED_VBSIGNATURE_SIZE 24
+
 
 #define KEY_BLOCK_MAGIC "CHROMEOS"
 #define KEY_BLOCK_MAGIC_SIZE 8
@@ -69,6 +74,8 @@ typedef struct VbKeyBlockHeader {
  *   3) The signature data for (VBKeyBlockHeader + data_key data), pointed to
  *      by key_block_signature.sig_offset. */
 
+#define EXPECTED_VBKEYBLOCKHEADER_SIZE 112
+
 
 #define FIRMWARE_PREAMBLE_HEADER_VERSION_MAJOR 2
 #define FIRMWARE_PREAMBLE_HEADER_VERSION_MINOR 0
@@ -95,6 +102,7 @@ typedef struct VbFirmwarePreambleHeader {
  *      + body signature data), pointed to by
  *      preamble_signature.sig_offset. */
 
+#define EXPECTED_VBFIRMWAREPREAMBLEHEADER_SIZE 104
 
 #define KERNEL_PREAMBLE_HEADER_VERSION_MAJOR 2
 #define KERNEL_PREAMBLE_HEADER_VERSION_MINOR 0
@@ -121,4 +129,9 @@ typedef struct VbKernelPreambleHeader {
  *   3) The signature data for (VBFirmwarePreambleHeader + body signature
  *      data), pointed to by preamble_signature.sig_offset. */
 
+#define EXPECTED_VBKERNELPREAMBLEHEADER_SIZE 96
+
+
+PACK_STOP  /* Support packing for MSVC */
+
 #endif  /* VBOOT_REFERENCE_VBOOT_STRUCT_H_ */
diff --git a/firmware/lib/vboot_firmware.c b/firmware/lib/vboot_firmware.c
index 2fb1f6cc..36f09c91 100644
--- a/firmware/lib/vboot_firmware.c
+++ b/firmware/lib/vboot_firmware.c
@@ -55,8 +55,8 @@ int LoadFirmware(LoadFirmwareParams* params) {
     return LOAD_FIRMWARE_RECOVERY;
 
   /* Initialize the TPM and read rollback indices. */
-  /* TODO: fix SetupTPM parameter */
-  if (0 != SetupTPM(0, 0) )
+  /* TODO: fix SetupTPM parameter for developer mode */
+  if (0 != SetupTPM(RO_NORMAL_MODE, 0) )
     return LOAD_FIRMWARE_RECOVERY;
   if (0 != GetStoredVersions(FIRMWARE_VERSIONS,
                              &tpm_key_version, &tpm_fw_version))
@@ -204,6 +204,9 @@ int LoadFirmware(LoadFirmwareParams* params) {
      * is cleared only by TPM_Init at reboot.  */
     if (0 != LockFirmwareVersions())
       return LOAD_FIRMWARE_RECOVERY;
+
+    /* Success */
+    return LOAD_FIRMWARE_SUCCESS;
   }
 
   /* If we're still here, no good firmware, so go to recovery mode. */
diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c
index ea0fc295..c3b3423e 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -67,6 +67,7 @@ int WriteAndFreeGptData(GptData* gptdata) {
 
   if (gptdata->primary_header) {
     if (gptdata->modified & GPT_MODIFIED_HEADER1) {
+      debug("Updating GPT header 1\n");
       if (0 != BootDeviceWriteLBA(1, 1, gptdata->primary_header))
         return 1;
     }
@@ -75,6 +76,7 @@ int WriteAndFreeGptData(GptData* gptdata) {
 
   if (gptdata->primary_entries) {
     if (gptdata->modified & GPT_MODIFIED_ENTRIES1) {
+      debug("Updating GPT entries 1\n");
       if (0 != BootDeviceWriteLBA(2, entries_sectors,
                                   gptdata->primary_entries))
         return 1;
@@ -84,6 +86,7 @@ int WriteAndFreeGptData(GptData* gptdata) {
 
   if (gptdata->secondary_entries) {
     if (gptdata->modified & GPT_MODIFIED_ENTRIES2) {
+      debug("Updating GPT header 2\n");
       if (0 != BootDeviceWriteLBA(gptdata->drive_sectors - entries_sectors - 1,
                                   entries_sectors, gptdata->secondary_entries))
         return 1;
@@ -93,6 +96,7 @@ int WriteAndFreeGptData(GptData* gptdata) {
 
   if (gptdata->secondary_header) {
     if (gptdata->modified & GPT_MODIFIED_HEADER2) {
+      debug("Updating GPT entries 2\n");
       if (0 != BootDeviceWriteLBA(gptdata->drive_sectors - 1, 1,
                                   gptdata->secondary_header))
         return 1;
@@ -130,6 +134,15 @@ int LoadKernel(LoadKernelParams* params) {
   params->bootloader_address = 0;
   params->bootloader_size = 0;
 
+  /* Set up TPM; required in all modes */
+  if (0 != SetupTPM(
+          ((BOOT_FLAG_RECOVERY & params->boot_flags) ?
+           RO_RECOVERY_MODE : RW_NORMAL_MODE),
+          ((BOOT_FLAG_DEVELOPER & params->boot_flags) ? 1 : 0))) {
+    debug("Error setting up TPM\n");
+    return LOAD_KERNEL_RECOVERY;
+  }
+
   if (is_normal) {
     /* Read current kernel key index from TPM.  Assumes TPM is already
      * initialized. */