From 23fc8c22f15c4ffb5a49fa662c4cbb209811413d Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Mon, 28 Sep 2020 14:29:31 +1000 Subject: cortex-m mpu: support configuring regions with difficult alignment The existing configuration code assumes that provided addresses are at least as aligned as the requested size, which is not true on NPCX797WC (and likely others) where RAM regions are only 64k-aligned but have larger sizes (like 256k). Use a new greedy approach to configuring the MPU which handles these situations corrently: for any given request take the largest possible chunk from the bottom of the memory region (subject to size and address alignment). Maximize the space by aggressively using MPU subregions- this means that in many well-aligned situations this algorithm selects a larger region than the requested size and enables one subregion, but in more difficult situations it is capable of enabling subregions with more exotic positions. BUG=b:169276765 BRANCH=zork TEST=With a test harness to print out computed configurations, manually verified the correctness of a variety taken from real chip configurations (request first, MPU region(s) indented): 0x20000000 size 0x1000 # stm32f03x 0x20000000 size 0x8000 srd fe 0x20000000 size 0x2000 # stm32f03x 0x20000000 size 0x10000 srd fe 0x20000000 size 0x2800 # stm32l100 0x20000000 size 0x4000 srd e0 0x20000000 size 0x4000 # stm32f412 0x20000000 size 0x20000 srd fe 0x80000 size 0xc000 # it8320 0x80000 size 0x20000 srd f8 0xff200000 size 0xa0000 # ish5p4 0xff200000 size 0x100000 srd e0 0x200b0000 size 0x20000 # npcx797wb 0x20080000 size 0x80000 srd e7 0x10070000 size 0x40000 # npcx797wb 0x10000000 size 0x80000 srd 7f 0x10080000 size 0x80000 srd f8 0x200c0000 size 0x10000 # npcx796f 0x20080000 size 0x80000 srd ef 0x10090000 size 0x30000 # npcx796f 0x10080000 size 0x80000 srd f1 0x10090000 size 0x20 0x10090000 size 0x100 srd fe Further verified MPU configuration with the new algorithm succeeds on Dalboz, and test/mpu.c passes on Dragonclaw. Signed-off-by: Peter Marheine Change-Id: I71d8e2b37c7e20fc7a39166b90eea0b7f7ebcf43 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2434601 Reviewed-by: Edward Hill --- core/cortex-m/mpu.c | 154 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 95 insertions(+), 59 deletions(-) (limited to 'core') diff --git a/core/cortex-m/mpu.c b/core/cortex-m/mpu.c index d1db27d78e..5a78a9a013 100644 --- a/core/cortex-m/mpu.c +++ b/core/cortex-m/mpu.c @@ -94,6 +94,76 @@ int mpu_update_region(uint8_t region, uint32_t addr, uint8_t size_bit, return EC_SUCCESS; } +/* + * Greedily configure the largest possible part of the given region from the + * base address. + * + * Returns EC_SUCCESS on success and sets *consumed to the number of bytes + * mapped from the base address. In case of error, the value of *consumed is + * unpredictable. + * + * For instance, if addr is 0x10070000 and size is 0x30000 then memory in the + * range 0x10070000-0x10080000 will be configured and *consumed will be set to + * 0x10000. + */ +static int mpu_config_region_greedy(uint8_t region, uint32_t addr, + uint32_t size, uint16_t attr, + uint8_t enable, uint32_t *consumed) +{ + /* + * Compute candidate alignment to be used for the MPU region. + * + * This is the minimum of the base address and size alignment, since + * regions must be naturally aligned to their size. + */ + uint8_t natural_alignment = MIN(addr == 0 ? 32 : alignment_log2(addr), + alignment_log2(size)); + uint8_t subregion_disable = 0; + + if (natural_alignment >= 5) { + uint32_t subregion_base, subregion_size; + /* + * For MPU regions larger than 256 bytes we can use subregions, + * (which are a minimum of 32 bytes in size) making the actual + * MPU region 8x larger. Depending on the address alignment this + * can allow us to cover a larger area (and never a smaller + * one). + */ + natural_alignment += 3; + /* Region size cannot exceed 4GB. */ + if (natural_alignment > 32) + natural_alignment = 32; + + /* + * Generate the subregion mask by walking through each, + * disabling if it is not completely contained in the requested + * range. + */ + subregion_base = addr & ~((1 << natural_alignment) - 1); + subregion_size = 1 << (natural_alignment - 3); + *consumed = 0; + for (int sr_idx = 0; sr_idx < 8; sr_idx++) { + if (subregion_base < addr || + (subregion_base + subregion_size) > (addr + size)) + /* lsb of subregion mask is lowest address */ + subregion_disable |= 1 << sr_idx; + else + /* not disabled means consumed */ + *consumed += subregion_size; + + subregion_base += subregion_size; + } + } else { + /* Not using subregions; all enabled */ + *consumed = 1 << natural_alignment; + } + + return mpu_update_region(region, + addr & ~((1 << natural_alignment) - 1), + natural_alignment, + attr, enable, subregion_disable); +} + /** * Configure a region * @@ -103,76 +173,42 @@ int mpu_update_region(uint8_t region, uint32_t addr, uint8_t size_bit, * attr: Attribute bits. Current value will be overwritten if enable is set. * enable: Enables the region if non zero. Otherwise, disables the region. * - * Returns EC_SUCCESS on success or -EC_ERROR_INVAL if a parameter is invalid. + * Returns EC_SUCCESS on success, -EC_ERROR_OVERFLOW if it is not possible to + * fully configure the given region, or -EC_ERROR_INVAL if a parameter is + * invalid (such as the address or size having unsupported alignment). */ int mpu_config_region(uint8_t region, uint32_t addr, uint32_t size, uint16_t attr, uint8_t enable) { int rv; - int size_bit = 0; - uint8_t blocks, srd1, srd2; + uint32_t consumed; - if (!size) + /* Zero size doesn't require configuration */ + if (size == 0) return EC_SUCCESS; - /* Bit position of first '1' in size */ - size_bit = 31 - __builtin_clz(size); - /* Min. region size is 32 bytes */ - if (size_bit < 5) - return -EC_ERROR_INVAL; - - /* If size is a power of 2 then represent it with a single MPU region */ - if (POWER_OF_TWO(size)) - return mpu_update_region(region, addr, size_bit, attr, enable, - 0); - - /* Sub-regions are not supported for region <= 128 bytes */ - if (size_bit < 7) - return -EC_ERROR_INVAL; - /* Verify we can represent range with <= 2 regions */ - if (size & ~(0x3f << (size_bit - 5))) - return -EC_ERROR_INVAL; - - /* - * Round up size of first region to power of 2. - * Calculate the number of fully occupied blocks (block size = - * region size / 8) in the first region. - */ - blocks = size >> (size_bit - 2); - - /* Represent occupied blocks of two regions with srd mask. */ - srd1 = BIT(blocks) - 1; - srd2 = (1 << ((size >> (size_bit - 5)) & 0x7)) - 1; - - /* - * Second region not supported for DATA_RAM_TEXT, also verify size of - * second region is sufficient to support sub-regions. - */ - if (srd2 && (region == REGION_DATA_RAM_TEXT || size_bit < 10)) - return -EC_ERROR_INVAL; - - /* Write first region. */ - rv = mpu_update_region(region, addr, size_bit + 1, attr, enable, ~srd1); + rv = mpu_config_region_greedy(region, addr, size, + attr, enable, &consumed); if (rv != EC_SUCCESS) return rv; + ASSERT(consumed <= size); + addr += consumed; + size -= consumed; + + /* Regions other than DATA_RAM_TEXT may use two MPU regions */ + if (size > 0 && region != REGION_DATA_RAM_TEXT) { + rv = mpu_config_region_greedy(region + 1, addr, size, + attr, enable, &consumed); + if (rv != EC_SUCCESS) + return rv; + ASSERT(consumed <= size); + addr += consumed; + size -= consumed; + } - /* - * Second protection region (if necessary) begins at the first block - * we marked unoccupied in the first region. - * Size of the second region is the block size of first region. - */ - addr += (1 << (size_bit - 2)) * blocks; - - /* - * Now represent occupied blocks in the second region. It's possible - * that the first region completely represented the occupied area, if - * so then no second protection region is required. - */ - if (srd2) - rv = mpu_update_region(region + 1, addr, size_bit - 2, attr, - enable, ~srd2); - - return rv; + if (size > 0) + return EC_ERROR_OVERFLOW; + return EC_SUCCESS; } /** -- cgit v1.2.1