Add structs for TPM NV simplification.  Now uses only 2 NV spaces, one for firmware and one for kernel.

Changed TlclRead / TlclWrite to take void* / const void* to reduce typecasts.

Much restructuring of rollback_index.c.

Fixed a version-packing bug in rollback_index.c (& --> |)

BUG:chrome-os-partner:304
TEST:manual testing of all code flows on CRB

Review URL: http://codereview.chromium.org/3084030
diff --git a/firmware/lib/include/rollback_index.h b/firmware/lib/include/rollback_index.h
index df132e7..f13b6ed 100644
--- a/firmware/lib/include/rollback_index.h
+++ b/firmware/lib/include/rollback_index.h
@@ -12,30 +12,47 @@
 #include "sysincludes.h"
 #include "tss_constants.h"
 
-/* Rollback version types. */
-#define FIRMWARE_VERSIONS 0
-#define KERNEL_VERSIONS   1
-
-/* Initialization mode */
-#define RO_RECOVERY_MODE 0
-#define RO_NORMAL_MODE   1
-#define RW_NORMAL_MODE   2
-
 /* TPM NVRAM location indices. */
-#define FIRST_ROLLBACK_NV_INDEX         0x1001  /* First index used here */
-#define FIRMWARE_VERSIONS_NV_INDEX      0x1001
-#define KERNEL_VERSIONS_NV_INDEX        0x1002
-#define TPM_IS_INITIALIZED_NV_INDEX     0x1003
-#define KERNEL_VERSIONS_BACKUP_NV_INDEX 0x1004
-#define KERNEL_MUST_USE_BACKUP_NV_INDEX 0x1005
-#define DEVELOPER_MODE_NV_INDEX         0x1006
-#define LAST_ROLLBACK_NV_INDEX          0x1006  /* Last index used here */
+#define FIRMWARE_NV_INDEX               0x1007
+#define KERNEL_NV_INDEX                 0x1008
 
-/* Unique ID to detect kernel space redefinition */
-#define KERNEL_SPACE_UID "GRWL"        /* unique ID with secret meaning */
-#define KERNEL_SPACE_UID_SIZE (sizeof(KERNEL_SPACE_UID) - 1)
-#define KERNEL_SPACE_INIT_DATA ((uint8_t*) "\0\0\0\0" KERNEL_SPACE_UID)
-#define KERNEL_SPACE_SIZE (sizeof(uint32_t) + KERNEL_SPACE_UID_SIZE)
+/* Structure definitions for TPM spaces */
+
+__pragma(pack(push, 1)) /* Support packing for MSVC. */
+
+/* Kernel space - KERNEL_NV_INDEX, locked with physical presence. */
+#define ROLLBACK_SPACE_KERNEL_VERSION 1
+#define ROLLBACK_SPACE_KERNEL_UID 0x4752574C  /* 'GRWL' */
+typedef struct RollbackSpaceKernel {
+  uint8_t  struct_version;      /* Struct version, for backwards
+                                 * compatibility */
+  uint32_t uid;                 /* Unique ID to detect space redefinition */
+  uint32_t kernel_versions;     /* Kernel versions */
+  uint32_t reserved;            /* Reserved for future expansion */
+} __attribute__((packed)) RollbackSpaceKernel;
+
+
+/* Flags for firmware space */
+/* Last boot was developer mode.  TPM ownership is cleared when
+ * transitioning to/from developer mode. */
+#define FLAG_LAST_BOOT_DEVELOPER 0x01
+/* There have been one or more boots which left PP unlocked, so the
+ * contents of the kernel space are untrusted and must be restored
+ * from the backup copy. */
+#define FLAG_KERNEL_SPACE_USE_BACKUP 0x02
+
+#define ROLLBACK_SPACE_FIRMWARE_VERSION 1
+/* Firmware space - FIRMWARE_NV_INDEX, locked with global lock. */
+typedef struct RollbackSpaceFirmware {
+  uint8_t  struct_version;  /* Struct version, for backwards compatibility */
+  uint8_t  flags;           /* Flags (see FLAG_* above) */
+  uint32_t fw_versions;     /* Firmware versions */
+  uint32_t reserved;            /* Reserved for future expansion */
+  RollbackSpaceKernel kernel_backup;  /* Backup of kernel space */
+} __attribute__((packed)) RollbackSpaceFirmware;
+
+__pragma(pack(pop)) /* Support packing for MSVC. */
+
 
 /* All functions return TPM_SUCCESS (zero) if successful, non-zero if error */
 
@@ -72,10 +89,12 @@
 
 /* Setup must be called.  Pass developer_mode=nonzero if in developer
  * mode. */
-uint32_t RollbackFirmwareSetup(int developer_mode);
-/* Read and Write may be called after Setup. */
-uint32_t RollbackFirmwareRead(uint16_t* key_version, uint16_t* version);
+/* TODO: use a 32-bit version instead of 2 version pieces */
+uint32_t RollbackFirmwareSetup(int developer_mode, uint16_t* key_version,
+                               uint16_t* version);
+
 /* Write may be called if the versions change */
+/* TODO: use a 32-bit version instead of 2 version pieces */
 uint32_t RollbackFirmwareWrite(uint16_t key_version, uint16_t version);
 
 /* Lock must be called */
@@ -92,6 +111,7 @@
 
 /* Read and write may be called if not in developer mode.  If called in
  * recovery mode, the effect is undefined. */
+/* TODO: use a 32-bit version instead of 2 version pieces */
 uint32_t RollbackKernelRead(uint16_t* key_version, uint16_t* version);
 uint32_t RollbackKernelWrite(uint16_t key_version, uint16_t version);
 
@@ -100,10 +120,6 @@
 
 /* The following functions are here for testing only. */
 
-/* Store 1 in *|initialized| if the TPM NVRAM spaces have been initialized, 0
- * otherwise.  Return TPM errors. */
-uint32_t GetSpacesInitialized(int* initialized);
-
 /* Issue a TPM_Clear and reenable/reactivate the TPM. */
 uint32_t TPMClearAndReenable(void);
 
diff --git a/firmware/lib/rollback_index.c b/firmware/lib/rollback_index.c
index 1540bd0..973a11b 100644
--- a/firmware/lib/rollback_index.c
+++ b/firmware/lib/rollback_index.c
@@ -25,6 +25,7 @@
     }                                                   \
   } while (0)
 
+
 uint32_t TPMClearAndReenable(void) {
   VBDEBUG(("TPM: Clear and re-enable\n"));
   RETURN_ON_FAILURE(TlclForceClear());
@@ -34,12 +35,13 @@
   return TPM_SUCCESS;
 }
 
+
 /* Like TlclWrite(), but checks for write errors due to hitting the 64-write
  * limit and clears the TPM when that happens.  This can only happen when the
  * TPM is unowned, so it is OK to clear it (and we really have no choice).
  * This is not expected to happen frequently, but it could happen.
  */
-static uint32_t SafeWrite(uint32_t index, uint8_t* data, uint32_t length) {
+static uint32_t SafeWrite(uint32_t index, const void* data, uint32_t length) {
   uint32_t result = TlclWrite(index, data, length);
   if (result == TPM_E_MAXNVWRITES) {
     RETURN_ON_FAILURE(TPMClearAndReenable());
@@ -49,6 +51,7 @@
   }
 }
 
+
 /* Similarly to SafeWrite(), this ensures we don't fail a DefineSpace because
  * we hit the TPM write limit.  This is even less likely to happen than with
  * writes because we only define spaces once at initialization, but we'd rather
@@ -64,49 +67,38 @@
   }
 }
 
-static uint32_t InitializeKernelVersionsSpaces(void) {
-  RETURN_ON_FAILURE(SafeDefineSpace(KERNEL_VERSIONS_NV_INDEX,
-                                    TPM_NV_PER_PPWRITE, KERNEL_SPACE_SIZE));
-  RETURN_ON_FAILURE(SafeWrite(KERNEL_VERSIONS_NV_INDEX, KERNEL_SPACE_INIT_DATA,
-                              KERNEL_SPACE_SIZE));
-  return TPM_SUCCESS;
+
+/* Functions to read and write firmware and kernel spaces. */
+static uint32_t ReadSpaceFirmware(RollbackSpaceFirmware* rsf) {
+  return TlclRead(FIRMWARE_NV_INDEX, rsf, sizeof(RollbackSpaceFirmware));
 }
 
-/* When the return value is TPM_SUCCESS, this function sets *|initialized| to 1
- * if the spaces have been fully initialized, to 0 if not.  Otherwise
- * *|initialized| is not changed.
- */
-uint32_t GetSpacesInitialized(int* initialized) {
-  uint32_t space_holder;
-  uint32_t result;
-  result = TlclRead(TPM_IS_INITIALIZED_NV_INDEX,
-                    (uint8_t*) &space_holder, sizeof(space_holder));
-  switch (result) {
-  case TPM_SUCCESS:
-    *initialized = 1;
-    break;
-  case TPM_E_BADINDEX:
-    *initialized = 0;
-    result = TPM_SUCCESS;
-    break;
-  }
-  return result;
+static uint32_t WriteSpaceFirmware(const RollbackSpaceFirmware* rsf) {
+  return SafeWrite(FIRMWARE_NV_INDEX, rsf, sizeof(RollbackSpaceFirmware));
 }
 
-/* Creates the NVRAM spaces, and sets their initial values as needed.
- */
-static uint32_t InitializeSpaces(void) {
-  uint32_t zero = 0;
-  uint32_t firmware_perm = TPM_NV_PER_GLOBALLOCK | TPM_NV_PER_PPWRITE;
+static uint32_t ReadSpaceKernel(RollbackSpaceKernel* rsk) {
+  return TlclRead(KERNEL_NV_INDEX, rsk, sizeof(RollbackSpaceKernel));
+}
+
+static uint32_t WriteSpaceKernel(const RollbackSpaceKernel* rsk) {
+  return SafeWrite(KERNEL_NV_INDEX, rsk, sizeof(RollbackSpaceKernel));
+}
+
+
+
+/* Creates the NVRAM spaces, and sets their initial values as needed. */
+static uint32_t InitializeSpaces(RollbackSpaceFirmware* rsf,
+                                 RollbackSpaceKernel* rsk) {
+  static const RollbackSpaceFirmware rsf_init = {
+    ROLLBACK_SPACE_FIRMWARE_VERSION, 0, 0, 0};
+  static const RollbackSpaceKernel rsk_init = {
+    ROLLBACK_SPACE_KERNEL_VERSION, ROLLBACK_SPACE_KERNEL_UID, 0, 0};
   uint8_t nvlocked = 0;
 
   VBDEBUG(("TPM: Initializing spaces\n"));
 
-  /* Force the TPM clear, in case it previously had an owner, so that we can
-   * redefine the NVRAM spaces. */
-  RETURN_ON_FAILURE(TPMClearAndReenable());
-
-  /* The TPM will not enforce the NV authorization restrictions until the 
+  /* The TPM will not enforce the NV authorization restrictions until the
    * execution of a TPM_NV_DefineSpace with the handle of TPM_NV_INDEX_LOCK.
    * Create that space if it doesn't already exist. */
   RETURN_ON_FAILURE(TlclGetFlags(NULL, NULL, &nvlocked));
@@ -116,135 +108,24 @@
     RETURN_ON_FAILURE(TlclSetNvLocked());
   }
 
-  RETURN_ON_FAILURE(SafeDefineSpace(FIRMWARE_VERSIONS_NV_INDEX,
-                                    firmware_perm, sizeof(uint32_t)));
-  RETURN_ON_FAILURE(SafeWrite(FIRMWARE_VERSIONS_NV_INDEX,
-                              (uint8_t*) &zero, sizeof(uint32_t)));
+  /* Initialize the firmware and kernel spaces */
+  Memcpy(rsf, &rsf_init, sizeof(RollbackSpaceFirmware));
+  /* Initialize the backup copy of the kernel space to the same data
+   * as the kernel space */
+  Memcpy(&rsf->kernel_backup, &rsk_init, sizeof(RollbackSpaceKernel));
+  Memcpy(rsk, &rsk_init, sizeof(RollbackSpaceKernel));
 
-  RETURN_ON_FAILURE(InitializeKernelVersionsSpaces());
-
-  /* The space KERNEL_VERSIONS_BACKUP_NV_INDEX is used to protect the kernel
-   * versions.  The content of space KERNEL_MUST_USE_BACKUP determines whether
-   * only the backup value should be trusted.
-   */
-  RETURN_ON_FAILURE(SafeDefineSpace(KERNEL_VERSIONS_BACKUP_NV_INDEX,
-                                    firmware_perm, sizeof(uint32_t)));
-  RETURN_ON_FAILURE(SafeWrite(KERNEL_VERSIONS_BACKUP_NV_INDEX,
-                              (uint8_t*) &zero, sizeof(uint32_t)));
-  RETURN_ON_FAILURE(SafeDefineSpace(KERNEL_MUST_USE_BACKUP_NV_INDEX,
-                                    firmware_perm, sizeof(uint32_t)));
-  RETURN_ON_FAILURE(SafeWrite(KERNEL_MUST_USE_BACKUP_NV_INDEX,
-                              (uint8_t*) &zero, sizeof(uint32_t)));
-  RETURN_ON_FAILURE(SafeDefineSpace(DEVELOPER_MODE_NV_INDEX,
-                                    firmware_perm, sizeof(uint32_t)));
-  RETURN_ON_FAILURE(SafeWrite(DEVELOPER_MODE_NV_INDEX,
-                              (uint8_t*) &zero, sizeof(uint32_t)));
-
-  /* The space TPM_IS_INITIALIZED_NV_INDEX is used to indicate that the TPM
-   * initialization has completed.  Without it we cannot be sure that the last
-   * space to be created was also initialized (power could have been lost right
-   * after its creation).
-   */
-  RETURN_ON_FAILURE(SafeDefineSpace(TPM_IS_INITIALIZED_NV_INDEX,
-                                    firmware_perm, sizeof(uint32_t)));
+  /* Define and set firmware and kernel spaces */
+  RETURN_ON_FAILURE(SafeDefineSpace(FIRMWARE_NV_INDEX,
+                                    TPM_NV_PER_GLOBALLOCK | TPM_NV_PER_PPWRITE,
+                                    sizeof(RollbackSpaceFirmware)));
+  RETURN_ON_FAILURE(WriteSpaceFirmware(rsf));
+  RETURN_ON_FAILURE(SafeDefineSpace(KERNEL_NV_INDEX, TPM_NV_PER_PPWRITE,
+                                    sizeof(RollbackSpaceKernel)));
+  RETURN_ON_FAILURE(WriteSpaceKernel(rsk));
   return TPM_SUCCESS;
 }
 
-static uint32_t SetDistrustKernelSpaceAtNextBoot(uint32_t distrust) {
-  uint32_t must_use_backup;
-  RETURN_ON_FAILURE(TlclRead(KERNEL_MUST_USE_BACKUP_NV_INDEX,
-                             (uint8_t*) &must_use_backup, sizeof(uint32_t)));
-  if (must_use_backup != distrust) {
-     RETURN_ON_FAILURE(SafeWrite(KERNEL_MUST_USE_BACKUP_NV_INDEX,
-                                 (uint8_t*) &distrust, sizeof(uint32_t)));
-  }
-  return TPM_SUCCESS;
-}
-
-/* Checks if the kernel version space has been mucked with.  If it has,
- * reconstructs it using the backup value.
- */
-uint32_t RecoverKernelSpace(void) {
-  uint32_t perms = 0;
-  uint8_t buffer[KERNEL_SPACE_SIZE];
-  uint32_t backup_combined_versions;
-  uint32_t must_use_backup;
-  uint32_t zero = 0;
-
-  VBDEBUG(("TPM: RecoverKernelSpace()\n"));
-
-  RETURN_ON_FAILURE(TlclRead(KERNEL_MUST_USE_BACKUP_NV_INDEX,
-                             (uint8_t*) &must_use_backup, sizeof(uint32_t)));
-  /* must_use_backup is true if the previous boot entered recovery mode. */
-
-  VBDEBUG(("TPM: must_use_backup = %d\n", must_use_backup));
-
-  /* If we can't read the kernel space, or it has the wrong permission, or it
-   * doesn't contain the right identifier, we give up.  This will need to be
-   * fixed by the recovery kernel.  We have to worry about this because at any
-   * time (even with PP turned off) the TPM owner can remove and redefine a
-   * PP-protected space (but not write to it).
-   */
-  RETURN_ON_FAILURE(TlclRead(KERNEL_VERSIONS_NV_INDEX, (uint8_t*) &buffer,
-                             KERNEL_SPACE_SIZE));
-  RETURN_ON_FAILURE(TlclGetPermissions(KERNEL_VERSIONS_NV_INDEX, &perms));
-  if (perms != TPM_NV_PER_PPWRITE ||
-      Memcmp(buffer + sizeof(uint32_t), KERNEL_SPACE_UID,
-              KERNEL_SPACE_UID_SIZE) != 0) {
-    return TPM_E_CORRUPTED_STATE;
-  }
-
-  if (must_use_backup) {
-    /* We must use the backup space because in the preceding boot cycle the
-     * primary space was left unlocked and cannot be trusted.
-     */
-    RETURN_ON_FAILURE(TlclRead(KERNEL_VERSIONS_BACKUP_NV_INDEX,
-                               (uint8_t*) &backup_combined_versions,
-                               sizeof(uint32_t)));
-    RETURN_ON_FAILURE(SafeWrite(KERNEL_VERSIONS_NV_INDEX,
-                                (uint8_t*) &backup_combined_versions,
-                                sizeof(uint32_t)));
-    RETURN_ON_FAILURE(SafeWrite(KERNEL_MUST_USE_BACKUP_NV_INDEX,
-                                (uint8_t*) &zero, 0));
-  }
-  return TPM_SUCCESS;
-}
-
-static uint32_t BackupKernelSpace(void) {
-  uint32_t kernel_versions;
-  uint32_t backup_versions;
-  VBDEBUG(("TPM: BackupKernelSpace()\n"));
-  RETURN_ON_FAILURE(TlclRead(KERNEL_VERSIONS_NV_INDEX,
-                             (uint8_t*) &kernel_versions, sizeof(uint32_t)));
-  RETURN_ON_FAILURE(TlclRead(KERNEL_VERSIONS_BACKUP_NV_INDEX,
-                             (uint8_t*) &backup_versions, sizeof(uint32_t)));
-  if (kernel_versions == backup_versions) {
-    return TPM_SUCCESS;
-  } else if (kernel_versions < backup_versions) {
-    /* This cannot happen.  We're screwed. */
-    return TPM_E_INTERNAL_INCONSISTENCY;
-  }
-  RETURN_ON_FAILURE(SafeWrite(KERNEL_VERSIONS_BACKUP_NV_INDEX,
-                              (uint8_t*) &kernel_versions, sizeof(uint32_t)));
-  return TPM_SUCCESS;
-}
-
-/* Checks for transitions between protected mode to developer mode.  When going
- * into or out of developer mode, clear the TPM.
- */
-static uint32_t CheckDeveloperModeTransition(uint32_t current_developer) {
-  uint32_t past_developer;
-  RETURN_ON_FAILURE(TlclRead(DEVELOPER_MODE_NV_INDEX,
-                             (uint8_t*) &past_developer,
-                             sizeof(past_developer)));
-  if (past_developer != current_developer) {
-    RETURN_ON_FAILURE(TPMClearAndReenable());
-    RETURN_ON_FAILURE(SafeWrite(DEVELOPER_MODE_NV_INDEX,
-                                (uint8_t*) &current_developer,
-                                sizeof(current_developer)));
-  }
-  return TPM_SUCCESS;
-}
 
 /* SetupTPM starts the TPM and establishes the root of trust for the
  * anti-rollback mechanism.  SetupTPM can fail for three reasons.  1 A bug. 2 a
@@ -265,10 +146,17 @@
  * to the TPM flashram at every reboot or wake-up, because of concerns about
  * the durability of the NVRAM.
  */
-uint32_t SetupTPM(int recovery_mode, int developer_mode) {
+uint32_t SetupTPM(int recovery_mode, int developer_mode,
+                  RollbackSpaceFirmware* rsf) {
+
+  RollbackSpaceKernel rsk;
+  int rsf_dirty = 0;
+  uint8_t new_flags = 0;
+
   uint8_t disable;
   uint8_t deactivated;
   uint32_t result;
+  uint32_t perms;
 
   VBDEBUG(("TPM: SetupTPM(r%d, d%d)\n", recovery_mode, developer_mode));
 
@@ -277,48 +165,100 @@
 
   RETURN_ON_FAILURE(TlclStartup());
 #ifdef USE_CONTINUE_SELF_TEST
-  /* TODO: ContinueSelfTest() should be faster than SelfTestFull, but may also
-   * not work properly in older TPM firmware.  For now, do the full self test. */
+  /* TODO: ContinueSelfTest() should be faster than SelfTestFull, but
+   * may also not work properly in older TPM firmware.  For now, do
+   * the full self test. */
   RETURN_ON_FAILURE(TlclContinueSelfTest());
 #else
   RETURN_ON_FAILURE(TlclSelfTestFull());
 #endif
   RETURN_ON_FAILURE(TlclAssertPhysicalPresence());
-  /* Checks that the TPM is enabled and activated. */
+
+  /* Check that the TPM is enabled and activated. */
   RETURN_ON_FAILURE(TlclGetFlags(&disable, &deactivated, NULL));
   if (disable || deactivated) {
-    VBDEBUG(("TPM: disabled (%d) or deactivated (%d).  Fixing...\n", disable, deactivated));
+    VBDEBUG(("TPM: disabled (%d) or deactivated (%d).  Fixing...\n",
+             disable, deactivated));
     RETURN_ON_FAILURE(TlclSetEnable());
     RETURN_ON_FAILURE(TlclSetDeactivated(0));
     VBDEBUG(("TPM: Must reboot to re-enable\n"));
     return TPM_E_MUST_REBOOT;
   }
-  result = RecoverKernelSpace();
-  if (result != TPM_SUCCESS) {
-     /* Check if this is the first time we run and the TPM has not been
-      * initialized yet.
-      */
-    int initialized = 0;
-    VBDEBUG(("TPM: RecoverKernelSpace() failed\n"));
-    RETURN_ON_FAILURE(GetSpacesInitialized(&initialized));
-    if (initialized) {
-      VBDEBUG(("TPM: Already initialized, so give up\n"));
-      return result;
+
+  /* Read the firmware space. */
+  result = ReadSpaceFirmware(rsf);
+  if (TPM_E_BADINDEX == result) {
+    /* This is the first time we've run, and the TPM has not been
+     * initialized.  Initialize it. */
+    VBDEBUG(("TPM: Not initialized yet.\n"));
+    RETURN_ON_FAILURE(InitializeSpaces(rsf, &rsk));
+  } else if (TPM_SUCCESS != result) {
+    VBDEBUG(("TPM: Firmware space in a bad state; giving up.\n"));
+    return TPM_E_CORRUPTED_STATE;
+  }
+  VBDEBUG(("TPM: Firmware space sv%d f%x v%x\n",
+           rsf->struct_version, rsf->flags, rsf->fw_versions));
+
+  /* Read the kernel space and verify its permissions.  If the kernel
+   * space has the wrong permission, or it doesn't contain the right
+   * identifier, we give up.  This will need to be fixed by the
+   * recovery kernel.  We have to worry about this because at any time
+   * (even with PP turned off) the TPM owner can remove and redefine a
+   * PP-protected space (but not write to it). */
+  RETURN_ON_FAILURE(ReadSpaceKernel(&rsk));
+  RETURN_ON_FAILURE(TlclGetPermissions(KERNEL_NV_INDEX, &perms));
+  if (TPM_NV_PER_PPWRITE != perms || ROLLBACK_SPACE_KERNEL_UID != rsk.uid)
+    return TPM_E_CORRUPTED_STATE;
+  VBDEBUG(("TPM: Kernel space sv%d v%x\n",
+           rsk.struct_version, rsk.kernel_versions));
+
+  /* If the kernel space and its backup are different, we need to copy
+   * one to the other.  Which one we copy depends on whether the
+   * use-backup flag is set. */
+  if (0 != Memcmp(&rsk, &rsf->kernel_backup, sizeof(RollbackSpaceKernel))) {
+    VBDEBUG(("TPM: kernel space and backup are different\n"));
+
+    if (rsf->flags & FLAG_KERNEL_SPACE_USE_BACKUP) {
+      VBDEBUG(("TPM: use backup kernel space\n"));
+      Memcpy(&rsk, &rsf->kernel_backup, sizeof(RollbackSpaceKernel));
+      RETURN_ON_FAILURE(WriteSpaceKernel(&rsk));
+    } else if (rsk.kernel_versions < rsf->kernel_backup.kernel_versions) {
+      VBDEBUG(("TPM: kernel versions %x < backup versions %x\n",
+               rsk.kernel_versions, rsf->kernel_backup.kernel_versions));
+      return TPM_E_INTERNAL_INCONSISTENCY;
     } else {
-      VBDEBUG(("TPM: Need to initialize spaces.\n"));
-      RETURN_ON_FAILURE(InitializeSpaces());
-      VBDEBUG(("TPM: Retrying RecoverKernelSpace() now that spaces are initialized.\n"));
-      RETURN_ON_FAILURE(RecoverKernelSpace());
+      VBDEBUG(("TPM: copy kernel space to backup\n"));
+      Memcpy(&rsf->kernel_backup, &rsk, sizeof(RollbackSpaceKernel));
+      rsf_dirty = 1;
     }
   }
-  RETURN_ON_FAILURE(BackupKernelSpace());
-  RETURN_ON_FAILURE(SetDistrustKernelSpaceAtNextBoot(recovery_mode));
-  RETURN_ON_FAILURE(CheckDeveloperModeTransition(developer_mode));
 
-  if (recovery_mode) {
-    /* In recovery mode global variables are usable. */
-    g_rollback_recovery_mode = 1;
+  /* Clear ownership if developer flag has toggled */
+  if ((developer_mode ? FLAG_LAST_BOOT_DEVELOPER : 0) !=
+      (rsf->flags & FLAG_LAST_BOOT_DEVELOPER)) {
+    VBDEBUG(("TPM: Developer flag changed; clearing owner.\n"));
+    RETURN_ON_FAILURE(TPMClearAndReenable());
   }
+
+  /* Update flags */
+  if (developer_mode)
+    new_flags |= FLAG_LAST_BOOT_DEVELOPER;
+  if (recovery_mode) {
+    new_flags |= FLAG_KERNEL_SPACE_USE_BACKUP;
+    g_rollback_recovery_mode = 1;  /* Global variables are usable in
+                                    * recovery mode */
+  }
+  if (rsf->flags != new_flags) {
+    rsf->flags = new_flags;
+    rsf_dirty = 1;
+  }
+
+  /* If firmware space is dirty, flush it back to the TPM */
+  if (rsf_dirty) {
+    VBDEBUG(("TPM: Updating firmware space.\n"));
+    RETURN_ON_FAILURE(WriteSpaceFirmware(rsf));
+  }
+
   VBDEBUG(("TPM: SetupTPM() succeeded\n"));
   return TPM_SUCCESS;
 }
@@ -331,7 +271,8 @@
 
 /* Dummy implementations which don't support TPM rollback protection */
 
-uint32_t RollbackFirmwareSetup(int developer_mode) {
+uint32_t RollbackFirmwareSetup(int developer_mode,
+                               uint16_t* key_version, uint16_t* version) {
 #ifndef CHROMEOS_ENVIRONMENT
   /* Initialize the TPM, but ignore return codes.  In ChromeOS
    * environment, don't even talk to the TPM. */
@@ -339,10 +280,7 @@
   TlclStartup();
   TlclSelfTestFull();
 #endif
-  return TPM_SUCCESS;
-}
 
-uint32_t RollbackFirmwareRead(uint16_t* key_version, uint16_t* version) {
   *key_version = *version = 0;
   return TPM_SUCCESS;
 }
@@ -381,26 +319,29 @@
 
 #else
 
-uint32_t RollbackFirmwareSetup(int developer_mode) {
-  return SetupTPM(0, developer_mode);
-}
+uint32_t RollbackFirmwareSetup(int developer_mode, uint16_t* key_version,
+                               uint16_t* version) {
+  RollbackSpaceFirmware rsf;
 
-uint32_t RollbackFirmwareRead(uint16_t* key_version, uint16_t* version) {
-  uint32_t firmware_versions;
-  /* Gets firmware versions. */
-  RETURN_ON_FAILURE(TlclRead(FIRMWARE_VERSIONS_NV_INDEX,
-                             (uint8_t*) &firmware_versions,
-                             sizeof(firmware_versions)));
-  *key_version = (uint16_t) (firmware_versions >> 16);
-  *version = (uint16_t) (firmware_versions & 0xffff);
+  RETURN_ON_FAILURE(SetupTPM(0, developer_mode, &rsf));
+  *key_version = (uint16_t)(rsf.fw_versions >> 16);
+  *version = (uint16_t)(rsf.fw_versions & 0xffff);
+
+  VBDEBUG(("TPM: RollbackFirmwareSetup %x %x %x\n", (int)rsf.fw_versions, (int)*key_version, (int)*version));
+
   return TPM_SUCCESS;
 }
 
 uint32_t RollbackFirmwareWrite(uint16_t key_version, uint16_t version) {
-  uint32_t combined_version = (key_version << 16) & version;
-  return SafeWrite(FIRMWARE_VERSIONS_NV_INDEX,
-                   (uint8_t*) &combined_version,
-                   sizeof(uint32_t));
+  RollbackSpaceFirmware rsf;
+  uint32_t new_versions = ((uint32_t)key_version << 16) | version;
+
+  VBDEBUG(("TPM: RollbackFirmwareWrite(%d, %d)\n", (int)key_version, (int)version));
+
+  RETURN_ON_FAILURE(ReadSpaceFirmware(&rsf));
+  VBDEBUG(("TPM: RollbackFirmwareWrite %x --> %x\n", (int)rsf.fw_versions, (int)new_versions));
+  rsf.fw_versions = new_versions;
+  return WriteSpaceFirmware(&rsf);
 }
 
 uint32_t RollbackFirmwareLock(void) {
@@ -408,52 +349,59 @@
 }
 
 uint32_t RollbackKernelRecovery(int developer_mode) {
-  uint32_t result = SetupTPM(1, developer_mode);
+  RollbackSpaceFirmware rsf;
+  uint32_t result = SetupTPM(1, developer_mode, &rsf);
   /* In recovery mode we ignore TPM malfunctions or corruptions, and leave the
    * TPM completely unlocked if and only if the dev mode switch is ON.  The
    * recovery kernel will fix the TPM (if needed) and lock it ASAP.  We leave
-   * Physical Presence on in either case.
-   */
+   * Physical Presence on in either case. */
   if (!developer_mode) {
     RETURN_ON_FAILURE(TlclSetGlobalLock());
   }
   /* We still return the result of SetupTPM even though we expect the caller to
-   * ignore it.  It's useful in unit testing.
-   */
+   * ignore it.  It's useful in unit testing. */
   return result;
 }
 
 uint32_t RollbackKernelRead(uint16_t* key_version, uint16_t* version) {
-  uint32_t kernel_versions;
   if (g_rollback_recovery_mode) {
     *key_version = 0;
     *version = 0;
   } else {
-    /* Reads kernel versions from TPM. */
-    RETURN_ON_FAILURE(TlclRead(KERNEL_VERSIONS_NV_INDEX,
-                               (uint8_t*) &kernel_versions,
-                               sizeof(kernel_versions)));
-    *key_version = (uint16_t) (kernel_versions >> 16);
-    *version = (uint16_t) (kernel_versions & 0xffff);
+    RollbackSpaceKernel rsk;
+    RETURN_ON_FAILURE(ReadSpaceKernel(&rsk));
+    *key_version = (uint16_t)(rsk.kernel_versions >> 16);
+    *version = (uint16_t)(rsk.kernel_versions & 0xffff);
+    VBDEBUG(("TPM: RollbackKernelRead %x %x %x\n", (int)rsk.kernel_versions,
+             (int)*key_version, (int)*version));
   }
   return TPM_SUCCESS;
 }
 
 uint32_t RollbackKernelWrite(uint16_t key_version, uint16_t version) {
-  if (!g_rollback_recovery_mode) {
-    uint32_t combined_version = (key_version << 16) & version;
-    return SafeWrite(KERNEL_VERSIONS_NV_INDEX,
-                     (uint8_t*) &combined_version,
-                     sizeof(uint32_t));
+
+  VBDEBUG(("TPM: RollbackKernelWrite(%d, %d)\n", (int)key_version,
+           (int)version));
+
+  if (g_rollback_recovery_mode) {
+    return TPM_SUCCESS;
+  } else {
+    RollbackSpaceKernel rsk;
+    uint32_t new_versions = ((uint32_t)key_version << 16) | version;
+
+    RETURN_ON_FAILURE(ReadSpaceKernel(&rsk));
+    VBDEBUG(("TPM: RollbackKernelWrite %x --> %x\n", (int)rsk.kernel_versions,
+             (int)new_versions));
+    rsk.kernel_versions = new_versions;
+    return WriteSpaceKernel(&rsk);
   }
-  return TPM_SUCCESS;
 }
 
 uint32_t RollbackKernelLock(void) {
-  if (!g_rollback_recovery_mode) {
-    return TlclLockPhysicalPresence();
-  } else {
+  if (g_rollback_recovery_mode) {
     return TPM_SUCCESS;
+  } else {
+    return TlclLockPhysicalPresence();
   }
 }
 
diff --git a/firmware/lib/tpm_lite/include/tlcl.h b/firmware/lib/tpm_lite/include/tlcl.h
index 2812a5b..e7bb4bf 100644
--- a/firmware/lib/tpm_lite/include/tlcl.h
+++ b/firmware/lib/tpm_lite/include/tlcl.h
@@ -68,12 +68,12 @@
 /* Writes [length] bytes of [data] to space at [index].  The TPM error code is
  * returned.
  */
-uint32_t TlclWrite(uint32_t index, uint8_t *data, uint32_t length);
+uint32_t TlclWrite(uint32_t index, const void* data, uint32_t length);
 
 /* Reads [length] bytes from space at [index] into [data].  The TPM error code
  * is returned.
  */
-uint32_t TlclRead(uint32_t index, uint8_t *data, uint32_t length);
+uint32_t TlclRead(uint32_t index, void* data, uint32_t length);
 
 /* Write-locks space at [index].  The TPM error code is returned.
  */
diff --git a/firmware/lib/tpm_lite/tlcl.c b/firmware/lib/tpm_lite/tlcl.c
index 94a2828..4bbd7a4 100644
--- a/firmware/lib/tpm_lite/tlcl.c
+++ b/firmware/lib/tpm_lite/tlcl.c
@@ -121,7 +121,7 @@
   return Send(cmd.buffer);
 }
 
-uint32_t TlclWrite(uint32_t index, uint8_t* data, uint32_t length) {
+uint32_t TlclWrite(uint32_t index, const void* data, uint32_t length) {
   struct s_tpm_nv_write_cmd cmd;
   uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE];
   const int total_length =
@@ -139,7 +139,7 @@
   return TlclSendReceive(cmd.buffer, response, sizeof(response));
 }
 
-uint32_t TlclRead(uint32_t index, uint8_t* data, uint32_t length) {
+uint32_t TlclRead(uint32_t index, void* data, uint32_t length) {
   struct s_tpm_nv_read_cmd cmd;
   uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE];
   uint32_t result_length;
diff --git a/firmware/lib/vboot_firmware.c b/firmware/lib/vboot_firmware.c
index 4fb20a1..ca2cd77 100644
--- a/firmware/lib/vboot_firmware.c
+++ b/firmware/lib/vboot_firmware.c
@@ -61,15 +61,10 @@
   }
 
   /* Initialize the TPM and read rollback indices. */
-  status = RollbackFirmwareSetup(params->boot_flags & BOOT_FLAG_DEVELOPER);
+  status = RollbackFirmwareSetup(params->boot_flags & BOOT_FLAG_DEVELOPER,
+                                 &tpm_key_version, &tpm_fw_version);
   if (0 != status) {
-    VBDEBUG(("Unable to setup TPM.\n"));
-    return (status == TPM_E_MUST_REBOOT ?
-            LOAD_FIRMWARE_REBOOT : LOAD_FIRMWARE_RECOVERY);
-  }
-  status = RollbackFirmwareRead(&tpm_key_version, &tpm_fw_version);
-  if (0 != status) {
-    VBDEBUG(("Unable to read stored versions.\n"));
+    VBDEBUG(("Unable to setup TPM and read stored versions.\n"));
     return (status == TPM_E_MUST_REBOOT ?
             LOAD_FIRMWARE_REBOOT : LOAD_FIRMWARE_RECOVERY);
   }
diff --git a/firmware/linktest/main.c b/firmware/linktest/main.c
index 5507a33..7aa34c8 100644
--- a/firmware/linktest/main.c
+++ b/firmware/linktest/main.c
@@ -29,8 +29,7 @@
   LoadKernel(0);
 
   /* rollback_index.h */
-  RollbackFirmwareSetup(0);
-  RollbackFirmwareRead(&x, &y);
+  RollbackFirmwareSetup(0, &x, &y);
   RollbackFirmwareWrite(0, 0);
   RollbackFirmwareLock();
   RollbackKernelRecovery(0);
diff --git a/firmware/version.c b/firmware/version.c
index f571be1..ea3abb9 100644
--- a/firmware/version.c
+++ b/firmware/version.c
@@ -1 +1 @@
-char* VbootVersion = "VBOOv=8eb662a6";
+char* VbootVersion = "VBOOv=38c0921e";
diff --git a/tests/Makefile b/tests/Makefile
index 095e5a4..1286210 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -22,7 +22,7 @@
 TEST_BINS = $(addprefix ${BUILD_ROOT}/,$(TEST_NAMES))
 
 TEST_LIB = ${BUILD_ROOT}/test.a
-TEST_LIB_SRCS = rollback_index_mock.c test_common.c timer_utils.c crc32_test.c
+TEST_LIB_SRCS = test_common.c timer_utils.c crc32_test.c
 TEST_LIB_OBJS = $(TEST_LIB_SRCS:%.c=${BUILD_ROOT}/%.o)
 ALL_DEPS = $(addsuffix .d,${TEST_BINS} ${TEST_LIB_OBJS})
 CFLAGS +=  -MMD -MF $@.d
diff --git a/tests/rollback_index_mock.c b/tests/rollback_index_mock.c
deleted file mode 100644
index 8e96ec1..0000000
--- a/tests/rollback_index_mock.c
+++ /dev/null
@@ -1,71 +0,0 @@
-/* Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- *
- * Mock rollback index library for testing.
- */
-
-#include "rollback_index.h"
-#include "tss_constants.h"
-
-#include <stdio.h>
-
-uint16_t g_firmware_key_version = 0;
-uint16_t g_firmware_version = 0;
-uint16_t g_kernel_key_version = 0;
-uint16_t g_kernel_version = 0;
-
-/* disable MSVC warnings on unused arguments */
-__pragma(warning (disable: 4100))
-
-uint32_t SetupTPM(int mode, int developer_flag) {
-#ifndef NDEBUG
-  VBDEBUG(("Rollback Index Library Mock: TPM initialized.\n"));
-#endif
-  return TPM_SUCCESS;
-}
-
-uint32_t GetStoredVersions(int type, uint16_t* key_version, uint16_t* version) {
-  switch (type) {
-    case FIRMWARE_VERSIONS:
-      *key_version = g_firmware_key_version;
-      *version = g_firmware_version;
-      break;
-    case KERNEL_VERSIONS:
-      *key_version = g_kernel_key_version;
-      *version = g_kernel_version;
-      break;
-  }
-  return TPM_SUCCESS;
-}
-
-uint32_t WriteStoredVersions(int type, uint16_t key_version, uint16_t version) {
-  switch (type) {
-    case FIRMWARE_VERSIONS:
-      g_firmware_key_version = key_version;
-      g_firmware_version = version;
-      break;
-    case KERNEL_VERSIONS:
-      g_kernel_key_version = key_version;
-      g_kernel_version = version;
-      break;
-  }
-#ifndef NDEBUG
-  VBDEBUG(("Rollback Index Library Mock: Stored Versions written.\n"));
-#endif
-  return TPM_SUCCESS;
-}
-
-uint32_t LockFirmwareVersions(void) {
-#ifndef NDEBUG
-  VBDEBUG(("Rollback Index Library Mock: Firmware Versions Locked.\n"));
-#endif
-  return TPM_SUCCESS;
-}
- 
-uint32_t LockKernelVersionsByLockingPP(void) {
-#ifndef NDEBUG
-  VBDEBUG(("Rollback Index Library Mock: Kernel Versions Locked.\n"));
-#endif
-  return TPM_SUCCESS;
-}