arch: arm: clean up MPU code for ARM and NXP
* We are now *much* better at not reserving unnecessary
system MPU regions based on configuration. The #defines
for intent are now an enumerated type. As a bonus, the
implementation of _get_region_index_by_type() is much
simpler. Previously we were wasting regions for stack guard
and application memory if they were not configured.
* NXP MPU doesn't reserve the last region if HW stack
protection isn't enabled.
* Certain parts of the MPU code are now properly ifdef'd
based on configuration.
* THREAD_STACK_REGION and THREAD_STACK_USER_REGION was a
confusing construction and has now been replaced with
just THREAD_STACK_REGION, which represents the MPU region
for a user mode thread stack. Supervisor mode stacks
do not require an MPU region.
* The bounds of CONFIG_APPLICATION_MEMORY never changes
and we just do it once during initialization instead of
every context switch.
* Assertions have been added to catch out-of-bounds cases.
Fixes: #7384
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
diff --git a/arch/arm/core/cortex_m/mpu/arm_mpu.c b/arch/arm/core/cortex_m/mpu/arm_mpu.c
index e5ae942..59a8f6c 100644
--- a/arch/arm/core/cortex_m/mpu/arm_mpu.c
+++ b/arch/arm/core/cortex_m/mpu/arm_mpu.c
@@ -60,21 +60,27 @@
*/
static inline u32_t _get_region_attr_by_type(u32_t type, u32_t size)
{
+#if defined(CONFIG_USERSPACE) || defined(CONFIG_MPU_STACK_GUARD) || \
+ defined(CONFIG_APPLICATION_MEMORY)
int region_size = _size_to_mpu_rasr_size(size);
+#endif
switch (type) {
- case THREAD_STACK_USER_REGION:
- return _get_region_attr(1, P_RW_U_RW, 0, 1, 0,
- 1, 0, region_size);
+#ifdef CONFIG_USERSPACE
case THREAD_STACK_REGION:
return _get_region_attr(1, P_RW_U_RW, 0, 1, 0,
1, 0, region_size);
+#endif
+#ifdef CONFIG_MPU_STACK_GUARD
case THREAD_STACK_GUARD_REGION:
return _get_region_attr(1, P_RO_U_NA, 0, 1, 0,
1, 0, region_size);
+#endif
+#ifdef CONFIG_APPLICATION_MEMORY
case THREAD_APP_DATA_REGION:
return _get_region_attr(1, P_RW_U_RW, 0, 1, 0,
1, 0, region_size);
+#endif
default:
/* Size 0 region */
return 0;
@@ -122,35 +128,18 @@
*/
static inline u32_t _get_region_index_by_type(u32_t type)
{
- /*
- * The new MPU regions are allocated per type after the statically
- * configured regions. The type is one-indexed rather than
- * zero-indexed, therefore we need to subtract by one to get the region
- * index.
- */
- switch (type) {
- case THREAD_STACK_USER_REGION:
- return mpu_config.num_regions + THREAD_STACK_REGION - 1;
- case THREAD_STACK_REGION:
- case THREAD_STACK_GUARD_REGION:
- case THREAD_APP_DATA_REGION:
- return mpu_config.num_regions + type - 1;
- case THREAD_DOMAIN_PARTITION_REGION:
-#if defined(CONFIG_USERSPACE)
- return mpu_config.num_regions + type - 1;
-#elif defined(CONFIG_MPU_STACK_GUARD)
- return mpu_config.num_regions + type - 2;
-#else
- /*
- * Start domain partition region from stack guard region
- * since stack guard is not enabled.
- */
- return mpu_config.num_regions + type - 3;
-#endif
- default:
- __ASSERT(0, "Unsupported type");
- return 0;
- }
+ u32_t region_index;
+
+ __ASSERT(type < THREAD_MPU_REGION_LAST,
+ "unsupported region type");
+
+ region_index = mpu_config.num_regions + type;
+
+ __ASSERT(region_index < _get_num_regions(),
+ "out of MPU regions, requested %u max is %u",
+ region_index, _get_num_regions() - 1);
+
+ return region_index;
}
/**
@@ -212,32 +201,6 @@
return 0;
}
-/**
- * This internal function checks if the region is user accessible or not.
- *
- * Note:
- * The caller must provide a valid region number.
- */
-static inline int _is_user_accessible_region(u32_t r_index, int write)
-{
- u32_t r_ap;
-
- ARM_MPU_DEV->rnr = r_index;
- r_ap = ARM_MPU_DEV->rasr & MPU_RASR_AP_Msk;
-
- /* always return true if this is the thread stack region */
- if (_get_region_index_by_type(THREAD_STACK_REGION) == r_index) {
- return 1;
- }
-
- if (write) {
- return r_ap == P_RW_U_RW;
- }
-
- /* For all user accessible permissions, their AP[1] bit is l */
- return r_ap & (0x2 << MPU_RASR_AP_Pos);
-}
-
/* ARM Core MPU Driver API Implementation for ARM MPU */
/**
@@ -285,8 +248,8 @@
{
u32_t base = (u32_t)thread->stack_obj;
u32_t size = thread->stack_info.size;
- u32_t index = _get_region_index_by_type(THREAD_STACK_USER_REGION);
- u32_t region_attr = _get_region_attr_by_type(THREAD_STACK_USER_REGION,
+ u32_t index = _get_region_index_by_type(THREAD_STACK_REGION);
+ u32_t region_attr = _get_region_attr_by_type(THREAD_STACK_REGION,
size);
if (!thread->arch.priv_stack_start) {
@@ -298,19 +261,6 @@
}
/* configure stack */
_region_init(index, base, region_attr);
-
-#if defined(CONFIG_APPLICATION_MEMORY)
- /* configure app data portion */
- index = _get_region_index_by_type(THREAD_APP_DATA_REGION);
- if (index < _get_num_regions()) {
- size = (u32_t)&__app_ram_end - (u32_t)&__app_ram_start;
- region_attr =
- _get_region_attr_by_type(THREAD_APP_DATA_REGION, size);
- if (size > 0) {
- _region_init(index, (u32_t)&__app_ram_start, region_attr);
- }
- }
-#endif /* CONFIG_APPLICATION_MEMORY */
}
/**
@@ -406,7 +356,35 @@
}
/**
+ * This internal function checks if the region is user accessible or not.
+ *
+ * Note:
+ * The caller must provide a valid region number.
+ */
+static inline int _is_user_accessible_region(u32_t r_index, int write)
+{
+ u32_t r_ap;
+
+ ARM_MPU_DEV->rnr = r_index;
+ r_ap = ARM_MPU_DEV->rasr & MPU_RASR_AP_Msk;
+
+ /* always return true if this is the thread stack region */
+ if (_get_region_index_by_type(THREAD_STACK_REGION) == r_index) {
+ return 1;
+ }
+
+ if (write) {
+ return r_ap == P_RW_U_RW;
+ }
+
+ /* For all user accessible permissions, their AP[1] bit is l */
+ return r_ap & (0x2 << MPU_RASR_AP_Pos);
+}
+
+/**
* @brief validate the given buffer is user accessible or not
+ *
+ * Presumes the background mapping is NOT user accessible.
*/
int arm_core_mpu_buffer_validate(void *addr, size_t size, int write)
{
@@ -443,7 +421,7 @@
* This function provides the default configuration mechanism for the Memory
* Protection Unit (MPU).
*/
-static void _arm_mpu_config(void)
+static int arm_mpu_init(struct device *arg)
{
u32_t r_index;
@@ -459,9 +437,11 @@
mpu_config.num_regions,
_get_num_regions()
);
- return;
+ return -1;
}
+ SYS_LOG_DBG("total region count: %d", _get_num_regions());
+
/* Disable MPU */
ARM_MPU_DEV->ctrl = 0;
@@ -477,16 +457,20 @@
*/
ARM_MPU_DEV->ctrl = MPU_CTRL_ENABLE_Msk | MPU_CTRL_PRIVDEFENA_Msk;
+#if defined(CONFIG_APPLICATION_MEMORY)
+ u32_t index, size, region_attr;
+
+ /* configure app data portion */
+ index = _get_region_index_by_type(THREAD_APP_DATA_REGION);
+ size = (u32_t)&__app_ram_end - (u32_t)&__app_ram_start;
+ region_attr = _get_region_attr_by_type(THREAD_APP_DATA_REGION, size);
+ if (size > 0) {
+ _region_init(index, (u32_t)&__app_ram_start, region_attr);
+ }
+#endif
/* Make sure that all the registers are set before proceeding */
__DSB();
__ISB();
-}
-
-static int arm_mpu_init(struct device *arg)
-{
- ARG_UNUSED(arg);
-
- _arm_mpu_config();
/* Sanity check for number of regions in Cortex-M0+, M3, and M4. */
#if defined(CONFIG_CPU_CORTEX_M0PLUS) || \
diff --git a/arch/arm/core/cortex_m/mpu/nxp_mpu.c b/arch/arm/core/cortex_m/mpu/nxp_mpu.c
index 0e2dfb9..133efea 100644
--- a/arch/arm/core/cortex_m/mpu/nxp_mpu.c
+++ b/arch/arm/core/cortex_m/mpu/nxp_mpu.c
@@ -24,15 +24,19 @@
static inline u32_t _get_region_attr_by_type(u32_t type)
{
switch (type) {
- case THREAD_STACK_USER_REGION:
- return REGION_USER_MODE_ATTR;
+#ifdef CONFIG_USERSPACE
case THREAD_STACK_REGION:
- return REGION_RAM_ATTR;
+ return REGION_USER_MODE_ATTR;
+#endif
+#ifdef CONFIG_MPU_STACK_GUARD
case THREAD_STACK_GUARD_REGION:
/* The stack guard region has to be not accessible */
return REGION_RO_ATTR;
+#endif
+#ifdef CONFIG_APPLICATION_MEMORY
case THREAD_APP_DATA_REGION:
return REGION_USER_MODE_ATTR;
+#endif
default:
/* Size 0 region */
return 0;
@@ -44,6 +48,19 @@
return FSL_FEATURE_SYSMPU_DESCRIPTOR_COUNT;
}
+static inline u8_t _get_num_usable_regions(void)
+{
+ u8_t max = _get_num_regions();
+#ifdef CONFIG_MPU_STACK_GUARD
+ /* Last region reserved for stack guard.
+ * See comments in nxp_mpu_setup_sram_region()
+ */
+ return max - 1;
+#else
+ return max;
+#endif
+}
+
static void _region_init(u32_t index, u32_t region_base,
u32_t region_end, u32_t region_attr)
{
@@ -88,35 +105,19 @@
*/
static inline u32_t _get_region_index_by_type(u32_t type)
{
- /*
- * The new MPU regions are allocated per type after the statically
- * configured regions. The type is one-indexed rather than
- * zero-indexed, therefore we need to subtract by one to get the region
- * index.
- */
- switch (type) {
- case THREAD_STACK_USER_REGION:
- return mpu_config.num_regions + THREAD_STACK_REGION - 1;
- case THREAD_STACK_REGION:
- case THREAD_STACK_GUARD_REGION:
- case THREAD_APP_DATA_REGION:
- return mpu_config.num_regions + type - 1;
- case THREAD_DOMAIN_PARTITION_REGION:
-#if defined(CONFIG_USERSPACE)
- return mpu_config.num_regions + type - 1;
-#elif defined(CONFIG_MPU_STACK_GUARD)
- return mpu_config.num_regions + type - 2;
-#else
- /*
- * Start domain partition region from stack guard region
- * since stack guard is not enabled.
- */
- return mpu_config.num_regions + type - 3;
-#endif
- default:
- __ASSERT(0, "Unsupported type");
- return 0;
- }
+ u32_t region_index;
+
+ __ASSERT(type < THREAD_MPU_REGION_LAST,
+ "unsupported region type");
+
+
+ region_index = mpu_config.num_regions + type;
+
+ __ASSERT(region_index < _get_num_usable_regions(),
+ "out of MPU regions, requested %u max is %u",
+ region_index, _get_num_usable_regions() - 1);
+
+ return region_index;
}
/**
@@ -145,25 +146,7 @@
return 0;
}
-/**
- * This internal function checks if the region is user accessible or not.
- */
-static inline int _is_user_accessible_region(u32_t r_index, int write)
-{
- u32_t r_ap = SYSMPU->WORD[r_index][2];
-
- /* always return true if this is the thread stack region */
- if (_get_region_index_by_type(THREAD_STACK_REGION) == r_index) {
- return 1;
- }
-
- if (write) {
- return (r_ap & MPU_REGION_WRITE) == MPU_REGION_WRITE;
- }
-
- return (r_ap & MPU_REGION_READ) == MPU_REGION_READ;
-}
-
+#ifdef CONFIG_MPU_STACK_GUARD
static void nxp_mpu_setup_sram_region(u32_t base, u32_t size)
{
u32_t last_region = _get_num_regions() - 1;
@@ -201,6 +184,7 @@
mpu_config.mpu_regions[mpu_config.sram_region].attr);
}
+#endif /* CONFIG_MPU_STACK_GUARD */
/* ARM Core MPU Driver API Implementation for NXP MPU */
@@ -245,17 +229,14 @@
u32_t region_index = _get_region_index_by_type(type);
u32_t region_attr = _get_region_attr_by_type(type);
- /* NXP MPU supports up to 16 Regions */
- if (region_index > _get_num_regions() - 2) {
- return;
- }
-
_region_init(region_index, base,
ENDADDR_ROUND(base + size),
region_attr);
+#ifdef CONFIG_MPU_STACK_GUARD
if (type == THREAD_STACK_GUARD_REGION) {
nxp_mpu_setup_sram_region(base, size);
}
+#endif
}
#if defined(CONFIG_USERSPACE)
@@ -263,27 +244,11 @@
{
u32_t base = (u32_t)thread->stack_info.start;
u32_t size = thread->stack_info.size;
- u32_t index = _get_region_index_by_type(THREAD_STACK_USER_REGION);
- u32_t region_attr = _get_region_attr_by_type(THREAD_STACK_USER_REGION);
+ u32_t index = _get_region_index_by_type(THREAD_STACK_REGION);
+ u32_t region_attr = _get_region_attr_by_type(THREAD_STACK_REGION);
/* configure stack */
_region_init(index, base, ENDADDR_ROUND(base + size), region_attr);
-
-#if defined(CONFIG_APPLICATION_MEMORY)
- /* configure app data portion */
- index = _get_region_index_by_type(THREAD_APP_DATA_REGION);
- region_attr = _get_region_attr_by_type(THREAD_APP_DATA_REGION);
- base = (u32_t)&__app_ram_start;
- size = (u32_t)&__app_ram_end - (u32_t)&__app_ram_start;
-
- /* set up app data region if exists, otherwise disable */
- if (size > 0) {
- _region_init(index, base, ENDADDR_ROUND(base + size),
- region_attr);
- } else {
- SYSMPU->WORD[index][3] = 0;
- }
-#endif
}
/**
@@ -313,7 +278,7 @@
* Don't touch the last region, it is reserved for SRAM_1 region.
* See comments in arm_core_mpu_configure().
*/
- for (; region_index < _get_num_regions() - 1; region_index++) {
+ for (; region_index < _get_num_usable_regions(); region_index++) {
if (num_partitions && pparts->size) {
SYS_LOG_DBG("set region 0x%x 0x%x 0x%x",
region_index, pparts->start, pparts->size);
@@ -392,12 +357,29 @@
/*
* Subtract the start of domain partition regions from total regions
* should get the maximum number of free regions for memory domain
- * partitions. But we need to consume an extra 1 region to make
- * stack/stack guard protection work properly.
- * See the comments in arm_core_mpu_configure().
+ * partitions
*/
- return _get_num_regions() -
- _get_region_index_by_type(THREAD_DOMAIN_PARTITION_REGION) - 1;
+ return _get_num_usable_regions() -
+ _get_region_index_by_type(THREAD_DOMAIN_PARTITION_REGION);
+}
+
+/**
+ * This internal function checks if the region is user accessible or not
+ */
+static inline int _is_user_accessible_region(u32_t r_index, int write)
+{
+ u32_t r_ap = SYSMPU->WORD[r_index][2];
+
+ /* always return true if this is the thread stack region */
+ if (_get_region_index_by_type(THREAD_STACK_REGION) == r_index) {
+ return 1;
+ }
+
+ if (write) {
+ return (r_ap & MPU_REGION_WRITE) == MPU_REGION_WRITE;
+ }
+
+ return (r_ap & MPU_REGION_READ) == MPU_REGION_READ;
}
/**
@@ -405,10 +387,10 @@
*/
int arm_core_mpu_buffer_validate(void *addr, size_t size, int write)
{
- u32_t r_index;
+ u8_t r_index;
/* Iterate all MPU regions */
- for (r_index = 0; r_index < _get_num_regions(); r_index++) {
+ for (r_index = 0; r_index < _get_num_usable_regions(); r_index++) {
if (!_is_enabled_region(r_index) ||
!_is_in_region(r_index, (u32_t)addr, size)) {
continue;
@@ -440,12 +422,9 @@
{
u32_t r_index;
- SYS_LOG_DBG("region number: %d", _get_num_regions());
-
- /* NXP MPU supports up to 16 Regions */
- if (mpu_config.num_regions > _get_num_regions()) {
- return;
- }
+ __ASSERT(mpu_config.num_regions <= _get_num_regions(),
+ "too many static MPU regions defined");
+ SYS_LOG_DBG("total region count: %d", _get_num_regions());
/* Disable MPU */
SYSMPU->CESR &= ~SYSMPU_CESR_VLD_MASK;
@@ -467,6 +446,23 @@
nxp_mpu_enabled = 1;
+#if defined(CONFIG_APPLICATION_MEMORY)
+ u32_t index, region_attr, base, size;
+
+ /* configure app data portion */
+ index = _get_region_index_by_type(THREAD_APP_DATA_REGION);
+ region_attr = _get_region_attr_by_type(THREAD_APP_DATA_REGION);
+ base = (u32_t)&__app_ram_start;
+ size = (u32_t)&__app_ram_end - (u32_t)&__app_ram_start;
+
+ /* set up app data region if exists, otherwise disable */
+ if (size > 0) {
+ _region_init(index, base, ENDADDR_ROUND(base + size),
+ region_attr);
+ } else {
+ SYSMPU->WORD[index][3] = 0;
+ }
+#endif
/* Make sure that all the registers are set before proceeding */
__DSB();
__ISB();
diff --git a/include/arch/arm/cortex_m/mpu/arm_core_mpu_dev.h b/include/arch/arm/cortex_m/mpu/arm_core_mpu_dev.h
index c4601c8..6c2a48c 100644
--- a/include/arch/arm/cortex_m/mpu/arm_core_mpu_dev.h
+++ b/include/arch/arm/cortex_m/mpu/arm_core_mpu_dev.h
@@ -30,11 +30,21 @@
* be managed inside the MPU driver and not escalated.
*/
/* Thread Stack Region Intent Type */
-#define THREAD_STACK_USER_REGION 0x0 /* fake region for user mode stack */
-#define THREAD_STACK_REGION 0x1
-#define THREAD_APP_DATA_REGION 0x2
-#define THREAD_STACK_GUARD_REGION 0x3
-#define THREAD_DOMAIN_PARTITION_REGION 0x4
+enum {
+#ifdef CONFIG_USERSPACE
+ THREAD_STACK_REGION,
+#endif
+#ifdef CONFIG_APPLICATION_MEMORY
+ THREAD_APP_DATA_REGION,
+#endif
+#ifdef CONFIG_MPU_STACK_GUARD
+ THREAD_STACK_GUARD_REGION,
+#endif
+#ifdef CONFIG_USERSPACE
+ THREAD_DOMAIN_PARTITION_REGION,
+#endif
+ THREAD_MPU_REGION_LAST
+};
#if defined(CONFIG_ARM_CORE_MPU)
struct k_mem_domain;
