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

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

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 reserved[3];
-     uint16_t gpt_att : 16;
+      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_ */

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_ */

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 */
+  /* TODO: fix SetupTPM parameter for developer mode */
-  if (0 != SetupTPM(0, 0) )
+  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. */

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. */

tests/cgptlib_test.c

@@ -171,6 +171,18 @@ static void BuildTestGptData(GptData* gpt) {
 }
 
 
+/* Tests if the structures are the expected size; if this fails,
+ * struct packing is not working properly. */
+static int StructSizeTest() {
+
+  EXPECT(GUID_EXPECTED_SIZE == sizeof(Guid));
+  EXPECT(GPTHEADER_EXPECTED_SIZE == sizeof(GptHeader));
+  EXPECT(GPTENTRY_EXPECTED_SIZE == sizeof(GptEntry));
+
+  return TEST_OK;
+}
+
+
 /* Tests if the default structure returned by BuildTestGptData() is good. */
 static int TestBuildTestGptData() {
   GptData* gpt;
@@ -1088,6 +1100,7 @@ int main(int argc, char *argv[]) {
     test_func fp;
     int retval;
   } test_cases[] = {
+    { TEST_CASE(StructSizeTest), },
     { TEST_CASE(TestBuildTestGptData), },
     { TEST_CASE(ParameterTests), },
     { TEST_CASE(HeaderCrcTest), },

tests/vboot_common_tests.c

@@ -11,9 +11,23 @@
 #include "test_common.h"
 #include "vboot_common.h"
 
+/* Test struct packing */
+static void StructPackingTest(void) {
+  TEST_EQ(EXPECTED_VBPUBLICKEY_SIZE, sizeof(VbPublicKey),
+          "sizeof(VbPublicKey)");
+  TEST_EQ(EXPECTED_VBSIGNATURE_SIZE, sizeof(VbSignature),
+          "sizeof(VbSignature)");
+  TEST_EQ(EXPECTED_VBKEYBLOCKHEADER_SIZE, sizeof(VbKeyBlockHeader),
+          "sizeof(VbKeyBlockHeader)");
+  TEST_EQ(EXPECTED_VBFIRMWAREPREAMBLEHEADER_SIZE,
+          sizeof(VbFirmwarePreambleHeader), "sizeof(VbFirmwarePreambleHeader)");
+  TEST_EQ(EXPECTED_VBKERNELPREAMBLEHEADER_SIZE,
+          sizeof(VbKernelPreambleHeader), "sizeof(VbKernelPreambleHeader)");
+}
+
 
 /* Helper functions not dependent on specific key sizes */
-void VerifyHelperFunctions(void) {
+static void VerifyHelperFunctions(void) {
 
   {
     uint8_t p[1];
@@ -91,7 +105,7 @@ void VerifyHelperFunctions(void) {
 int main(int argc, char* argv[]) {
   int error_code = 0;
 
-  /* Test helper functions */
+  StructPackingTest();
   VerifyHelperFunctions();
 
   if (!gTestSuccess)