[rom_ext, ownership] Fix the flash region configuration

I did not write correct test cases that would check a flash
configuration similar to the owner config already deployed in some skus.
This oversight could cause ownership initialization to fail on devies
with such a configuration.

1. Permit a maximum of 3 regions per side in the flash configuration.
   The regions must be fully within the bounds of SlotA or SlotB and
   may not overlap the ROM_EXT region.
2. Apply the region configuration in order.  Previously, there was a
   correspondence between the index of the region in the owner config
   and the MP_REGION register that it would land in, but this makes
   ownership transfers prone to configuration clashes.
3. Flash configuration is done in two passes to configure each side
   independently (the reason for this is to allow next_owner's flash
   config to apply to the non-primary side during ownership transfer).
   The flash_apply function now takex an index parameter to manage
   the desination MP_REGION register between passes.
4. Create a unittest case with a flash configuration similar to the
   already-deployed configuration. Include the ROM_EXT, application,
   filesystem and reserved segments.
5. Update the existing test cases to accomodate the new configuration
   scheme (e.g. applying in order rather than by index).

Signed-off-by: Chris Frantz <[email protected]>

sw/device/silicon_creator/lib/error.h

@@ -223,6 +223,7 @@ enum module_ {
   X(kErrorOwnershipFlashConfigRomExt, ERROR_(15, kModuleOwnership, kInvalidArgument)), \
   X(kErrorOwnershipFlashConfigBounds, ERROR_(16, kModuleOwnership, kInvalidArgument)), \
   X(kErrorOwnershipInvalidAlgorithm,  ERROR_(17, kModuleOwnership, kInvalidArgument)), \
+  X(kErrorOwnershipFlashConfigSlots,  ERROR_(18, kModuleOwnership, kInvalidArgument)), \
   /* Group all of the tag version error codes together */ \
   X(kErrorOwnershipOWNRVersion,       ERROR_(0x70, kModuleOwnership, kInvalidArgument)), \
   X(kErrorOwnershipAPPKVersion,       ERROR_(0x71, kModuleOwnership, kInvalidArgument)), \

sw/device/silicon_creator/lib/ownership/datatypes.h

@@ -295,6 +295,11 @@ enum {
 #define FLASH_CONFIG_HIGH_ENDURANCE       ((bitfield_field32_t) { .mask = 0xF, .index = 8 })
 // clang-format on
 
+/**
+ * The maximum number of owner_flash_region_t allower per slot.
+ */
+#define FLASH_CONFIG_REGIONS_PER_SLOT 3
+
 /**
  * The owner flash region describes a region of flash and its configuration
  * properties (ie: ECC, Scrambling, High Endurance, etc).

sw/device/silicon_creator/lib/ownership/owner_block.c

@@ -27,6 +27,11 @@ enum {
   kFlashPageSize = FLASH_CTRL_PARAM_BYTES_PER_PAGE,
   kFlashTotalSize = 2 * kFlashBankSize,
 
+  kFlashSlotAStart = 0,
+  kFlashSlotAEnd = kFlashSlotAStart + kFlashBankSize,
+  kFlashSlotBStart = kFlashBankSize,
+  kFlashSlotBEnd = kFlashSlotBStart + kFlashBankSize,
+
   kRomExtSizeInPages = CHIP_ROM_EXT_SIZE_MAX / kFlashPageSize,
   kRomExtAStart = 0 / kFlashPageSize,
   kRomExtAEnd = kRomExtAStart + kRomExtSizeInPages,
@@ -198,34 +203,59 @@ hardened_bool_t rom_ext_flash_exclusive(uint32_t start, uint32_t end) {
              : kHardenedBoolFalse;
 }
 
+static hardened_bool_t in_flash_slot(uint32_t bank_start, uint32_t start,
+                                     uint32_t end) {
+  uint32_t bank_end = bank_start + kFlashBankSize;
+  return (bank_start <= start && start < bank_end && bank_start < end &&
+          end <= bank_end)
+             ? kHardenedBoolTrue
+             : kHardenedBoolFalse;
+}
+
 rom_error_t owner_block_flash_check(const owner_flash_config_t *flash) {
   size_t len = (flash->header.length - sizeof(owner_flash_config_t)) /
                sizeof(owner_flash_region_t);
   if (len > kProtectSlots - kRomExtRegions) {
     return kErrorOwnershipFlashConfigLength;
   }
 
+  uint32_t num_slot_a = 0;
+  uint32_t num_slot_b = 0;
   const owner_flash_region_t *config = flash->config;
   uint32_t crypt = 0;
   for (size_t i = 0; i < len; ++i, ++config, crypt += 0x11111111) {
     uint32_t start = config->start;
     uint32_t end = start + config->size;
-    if (end > kFlashTotalSize) {
-      return kErrorOwnershipFlashConfigBounds;
-    }
     // When checking the flash configuration, a region is a ROM_EXT region if
     // it overlaps the ROM_EXT bounds.  It is an error to accept a new config
     // with a flash region that overlaps the ROM_EXT.
     if (rom_ext_flash_overlap(start, end) == kHardenedBoolTrue) {
       return kErrorOwnershipFlashConfigRomExt;
+    } else if (in_flash_slot(kFlashSlotAStart, start, end) ==
+               kHardenedBoolTrue) {
+      num_slot_a += 1;
+      if (num_slot_a > FLASH_CONFIG_REGIONS_PER_SLOT) {
+        return kErrorOwnershipFlashConfigSlots;
+      }
+    } else if (in_flash_slot(kFlashSlotBStart, start, end) ==
+               kHardenedBoolTrue) {
+      num_slot_b += 1;
+      if (num_slot_b > FLASH_CONFIG_REGIONS_PER_SLOT) {
+        return kErrorOwnershipFlashConfigSlots;
+      }
+    } else {
+      // Flash regions are not allowed to span between slots or extend beyond
+      // the end of flash.
+      return kErrorOwnershipFlashConfigBounds;
     }
   }
   return kErrorOk;
 }
 
 rom_error_t owner_block_flash_apply(const owner_flash_config_t *flash,
                                     uint32_t config_side,
-                                    uint32_t owner_lockdown) {
+                                    uint32_t owner_lockdown,
+                                    uint32_t *mp_index) {
   if ((hardened_bool_t)flash == kHardenedBoolFalse)
     return kErrorOk;
 
@@ -239,13 +269,31 @@ rom_error_t owner_block_flash_apply(const owner_flash_config_t *flash,
                                              : 0;
   size_t len = (flash->header.length - sizeof(owner_flash_config_t)) /
                sizeof(owner_flash_region_t);
-  if (len > kProtectSlots - kRomExtRegions) {
+  if (len > kProtectSlots) {
+    // We really want `kProtectSlots - kRomExtRegions`, but we have to
+    // accomodate configurations that may have specified ROM_EXT protection
+    // regions.  We deal with that fact in the loop below.
     return kErrorOwnershipFlashConfigLength;
   }
 
   const owner_flash_region_t *config = flash->config;
   uint32_t crypt = 0;
   for (size_t i = 0; i < len; ++i, ++config, crypt += 0x11111111) {
+    // When applying the flash configuration, a region is a ROM_EXT region if
+    // it is exclusively within the ROM_EXT bounds.  This sets the creator
+    // the lockdown policy for a region that is exclusively the creator
+    // region.
+    uint32_t region_start = config->start;
+    uint32_t region_end = region_start + config->size;
+    if (rom_ext_flash_exclusive(region_start, region_end) ==
+        kHardenedBoolTrue) {
+      // Flash region is a ROM_EXT region.  Do nothing.
+      // Some early configurations explicitly set parameters for the ROM_EXT
+      // region.  We ignore these regions in favor of the ROM_EXT setting
+      // its own protection parameters.
+      continue;
+    }
+
     if (config->start >= start && config->start + config->size <= end) {
       uint32_t val = config->properties ^ crypt;
       flash_ctrl_cfg_t cfg = {
@@ -266,40 +314,33 @@ rom_error_t owner_block_flash_apply(const owner_flash_config_t *flash,
           bitfield_field32_read(val, FLASH_CONFIG_LOCK) != kMultiBitBool4False
               ? kHardenedBoolTrue
               : kHardenedBoolFalse;
-      uint32_t region_start = config->start;
-      uint32_t region_end = region_start + config->size;
-
-      // When applying the flash configuration, a region is a ROM_EXT region if
-      // it is exclusively within the ROM_EXT bounds.  This sets the creator
-      // the lockdown policy for a region that is exclusively the creator
-      // region.
-      if (rom_ext_flash_exclusive(region_start, region_end) ==
-          kHardenedBoolTrue) {
-        // Flash region is a ROM_EXT region.  Do nothing.
-        // Some early configurations explicitly set parameters for the ROM_EXT
-        // region.  We ignore these regions in favor of the ROM_EXT setting
-        // its own protection parameters.
-        continue;
-      } else {
-        // Flash region is an owner region.
-        // If the config_side is the same as the owner lockdown side, and
-        // protect_when_primary is requested, deny write/erase to the region.
-        if (config_side == owner_lockdown && pwp != kMultiBitBool4False) {
-          perm.write = kMultiBitBool4False;
-          perm.erase = kMultiBitBool4False;
-        }
-        // If we aren't in a lockdown state, then do not lock the region
-        // configuration via the flash_ctrl regwen bits.
-        if (owner_lockdown == kHardenedBoolFalse) {
-          lock = kHardenedBoolFalse;
-        }
+
+      // Flash region is an owner region.
+      // If the config_side is the same as the owner lockdown side, and
+      // protect_when_primary is requested, deny write/erase to the region.
+      if (config_side == owner_lockdown && pwp != kMultiBitBool4False) {
+        perm.write = kMultiBitBool4False;
+        perm.erase = kMultiBitBool4False;
+      }
+
+      // If we aren't in a lockdown state, then do not lock the region
+      // configuration via the flash_ctrl regwen bits.
+      if (owner_lockdown == kHardenedBoolFalse) {
+        lock = kHardenedBoolFalse;
+      }
+
+      if (*mp_index < 2 * FLASH_CONFIG_REGIONS_PER_SLOT) {
+        // We can only apply the region protection of mp_index is
+        // within its acceptable bounds.
+        flash_ctrl_data_region_protect(kRomExtRegions + *mp_index,
+                                       config->start, config->size, perm, cfg,
+                                       lock);
+        SEC_MMIO_WRITE_INCREMENT(kFlashCtrlSecMmioDataRegionProtect + lock ==
+                                         kHardenedBoolTrue
+                                     ? kFlashCtrlSecMmioDataRegionProtectLock
+                                     : 0);
       }
-      flash_ctrl_data_region_protect(kRomExtRegions + i, config->start,
-                                     config->size, perm, cfg, lock);
-      SEC_MMIO_WRITE_INCREMENT(kFlashCtrlSecMmioDataRegionProtect + lock ==
-                                       kHardenedBoolTrue
-                                   ? kFlashCtrlSecMmioDataRegionProtectLock
-                                   : 0);
+      *mp_index += 1;
     }
   }
   return kErrorOk;

sw/device/silicon_creator/lib/ownership/owner_block.h

@@ -118,11 +118,14 @@ rom_error_t owner_block_flash_check(const owner_flash_config_t *flash);
  * @param owner_lockdown Apply any special lockdown configuration to
  *                       silicon_owner regions on the specified side of the
  *                       flash.  May use kHardenedBoolFalse to skip lockdown.
+ * @param mp_index The destination configuration index.  The value should be
+ *                 initialized to zero before the first call to this function.
  * @return error code.
  */
 rom_error_t owner_block_flash_apply(const owner_flash_config_t *flash,
                                     uint32_t config_side,
-                                    uint32_t owner_lockdown);
+                                    uint32_t owner_lockdown,
+                                    uint32_t *mp_index);
 
 /**
  * Apply the flash info configuration parameters from the owner block.