From ecb0f3e1591ce13fcffb6a8a4b51f34bf13dc974 Mon Sep 17 00:00:00 2001 From: Andy Gross Date: Wed, 30 Aug 2017 16:19:15 -0500 Subject: [PATCH] arm: mpu: Account for stack guard size correctly This patch fixes a couple of issues with the stack guard size and properly constructs the STACK_ALIGN and STACK_ALIGN_SIZE definitions. The ARM AAPCS requires that the stack pointers be 8 byte aligned. The STACK_ALIGN_SIZE definition is meant to contain the stack pointer alignment requirements. This is the required alignment at public API boundaries (ie stack frames). The STACK_ALIGN definition is the required alignment for the start address for stack buffer storage. STACK_ALIGN is used to validate the allocation sizes for stack buffers. The MPU_GUARD_ALIGN_AND_SIZE definition is the minimum alignment and size for the MPU. The minimum size and alignment just so happen to be 32 bytes for vanilla ARM MPU implementations. When defining stack buffers, the stack guard alignment requirements must be taken into consideration when allocating the stack memory. The __align() must be filled in with either STACK_ALIGN_SIZE or the align/size of the MPU stack guard. The align/size for the guard region will be 0 when CONFIG_MPU_STACK_GUARD is not set, and 32 bytes when it is. The _ARCH_THREAD_STACK_XXXXXX APIs need to know the minimum alignment requirements for the stack buffer memory and the stack guard size to correctly allocate and reference the stack memory. This is reflected in the macros with the use of the STACK_ALIGN definition and the MPU_GUARD_ALIGN_AND_SIZE definition. Signed-off-by: Andy Gross --- arch/arm/core/cortex_m/mpu/arm_core_mpu.c | 4 +- arch/arm/include/cortex_m/stack.h | 6 -- include/arch/arm/arch.h | 93 +++++++++++++++++++---- 3 files changed, 82 insertions(+), 21 deletions(-) diff --git a/arch/arm/core/cortex_m/mpu/arm_core_mpu.c b/arch/arm/core/cortex_m/mpu/arm_core_mpu.c index dc2491d3f97..73b224c4034 100644 --- a/arch/arm/core/cortex_m/mpu/arm_core_mpu.c +++ b/arch/arm/core/cortex_m/mpu/arm_core_mpu.c @@ -24,8 +24,8 @@ void configure_mpu_stack_guard(struct k_thread *thread) { arm_core_mpu_disable(); arm_core_mpu_configure(THREAD_STACK_GUARD_REGION, - thread->stack_info.start - STACK_ALIGN, - thread->stack_info.size); + thread->stack_info.start - MPU_GUARD_ALIGN_AND_SIZE, + thread->stack_info.size); arm_core_mpu_enable(); } #endif diff --git a/arch/arm/include/cortex_m/stack.h b/arch/arm/include/cortex_m/stack.h index d3d143ab1a2..c7e87125893 100644 --- a/arch/arm/include/cortex_m/stack.h +++ b/arch/arm/include/cortex_m/stack.h @@ -21,12 +21,6 @@ extern "C" { #endif -#ifdef CONFIG_STACK_ALIGN_DOUBLE_WORD -#define STACK_ALIGN_SIZE 8 -#else -#define STACK_ALIGN_SIZE 4 -#endif - #ifdef _ASMLANGUAGE /* nothing */ diff --git a/include/arch/arm/arch.h b/include/arch/arm/arch.h index 4553dea4e05..ac07d40a26c 100644 --- a/include/arch/arm/arch.h +++ b/include/arch/arm/arch.h @@ -42,19 +42,85 @@ extern "C" { #include #include #endif -#if defined(CONFIG_MPU_STACK_GUARD) -#define STACK_ALIGN 32 -#else /* CONFIG_MPU_STACK_GUARD */ -#define STACK_ALIGN 4 + + +/** + * @brief Declare the STACK_ALIGN_SIZE + * + * Denotes the required alignment of the stack pointer on public API + * boundaries + * + */ +#ifdef CONFIG_STACK_ALIGN_DOUBLE_WORD +#define STACK_ALIGN_SIZE 8 +#else +#define STACK_ALIGN_SIZE 4 #endif +/** + * @brief Declare a minimum MPU guard alignment and size + * + * This specifies the minimum MPU guard alignment/size for the MPU. This + * will be used to denote the guard section of the stack, if it exists. + * + * One key note is that this guard results in extra bytes being added to + * the stack. APIs which give the stack ptr and stack size will take this + * guard size into account. + * + * Stack is allocated, but initial stack pointer is at the end + * (highest address). Stack grows down to the actual allocation + * address (lowest address). Stack guard, if present, will comprise + * the lowest MPU_GUARD_ALIGN_AND_SIZE bytes of the stack. + * + * As the stack grows down, it will reach the end of the stack when it + * encounters either the stack guard region, or the stack allocation + * address. + * + * ----------------------- <---- Stack allocation address + stack size + + * | | MPU_GUARD_ALIGN_AND_SIZE + * | Some thread data | <---- Defined when thread is created + * | ... | + * |---------------------| <---- Actual initial stack ptr + * | Initial Stack Ptr | aligned to STACK_ALIGN_SIZE + * | ... | + * | ... | + * | ... | + * | ... | + * | ... | + * | ... | + * | ... | + * | ... | + * | Stack Ends | + * |---------------------- <---- Stack Buffer Ptr from API + * | MPU Guard, | + * | if present | + * ----------------------- <---- Stack Allocation address + * + */ +#if defined(CONFIG_MPU_STACK_GUARD) +#define MPU_GUARD_ALIGN_AND_SIZE 32 +#else +#define MPU_GUARD_ALIGN_AND_SIZE 0 +#endif + +/** + * @brief Define alignment of a stack buffer + * + * This is used for two different things: + * 1) Used in checks for stack size to be a multiple of the stack buffer + * alignment + * 2) Used to determine the alignment of a stack buffer + * + */ +#define STACK_ALIGN max(STACK_ALIGN_SIZE, MPU_GUARD_ALIGN_AND_SIZE) + /** * @brief Declare a toplevel thread stack memory region * * This declares a region of memory suitable for use as a thread's stack. * - * This is the generic, historical definition. Align to STACK_ALIGN and put in - * 'noinit' section so that it isn't zeroed at boot + * This is the generic, historical definition. Align to STACK_ALIGN_SIZE and + * put in * 'noinit' section so that it isn't zeroed at boot * * The declared symbol will always be a character array which can be passed to * k_thread_create, but should otherwise not be manipulated. @@ -70,7 +136,7 @@ extern "C" { */ #define _ARCH_THREAD_STACK_DEFINE(sym, size) \ struct _k_thread_stack_element __noinit __aligned(STACK_ALIGN) \ - sym[size+STACK_ALIGN] + sym[size+MPU_GUARD_ALIGN_AND_SIZE] /** * @brief Declare a toplevel array of thread stack memory regions @@ -78,8 +144,8 @@ extern "C" { * Create an array of equally sized stacks. See K_THREAD_STACK_DEFINE * definition for additional details and constraints. * - * This is the generic, historical definition. Align to STACK_ALIGN and put in - * 'noinit' section so that it isn't zeroed at boot + * This is the generic, historical definition. Align to STACK_ALIGN_SIZE and + * put in * 'noinit' section so that it isn't zeroed at boot * * @param sym Thread stack symbol name * @param nmemb Number of stacks to declare @@ -87,7 +153,7 @@ extern "C" { */ #define _ARCH_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \ struct _k_thread_stack_element __noinit __aligned(STACK_ALIGN) \ - sym[nmemb][size+STACK_ALIGN] + sym[nmemb][size+MPU_GUARD_ALIGN_AND_SIZE] /** * @brief Declare an embedded stack memory region @@ -103,7 +169,7 @@ extern "C" { */ #define _ARCH_THREAD_STACK_MEMBER(sym, size) \ struct _k_thread_stack_element __aligned(STACK_ALIGN) \ - sym[size+STACK_ALIGN] + sym[size+MPU_GUARD_ALIGN_AND_SIZE] /** * @brief Return the size in bytes of a stack memory region @@ -118,7 +184,7 @@ extern "C" { * @param sym Stack memory symbol * @return Size of the stack */ -#define _ARCH_THREAD_STACK_SIZEOF(sym) (sizeof(sym) - STACK_ALIGN) +#define _ARCH_THREAD_STACK_SIZEOF(sym) (sizeof(sym) - MPU_GUARD_ALIGN_AND_SIZE) /** * @brief Get a pointer to the physical stack buffer @@ -131,7 +197,8 @@ extern "C" { * @param sym Declared stack symbol name * @return The buffer itself, a char * */ -#define _ARCH_THREAD_STACK_BUFFER(sym) ((char *)(sym + STACK_ALIGN)) +#define _ARCH_THREAD_STACK_BUFFER(sym) \ + ((char *)(sym) + MPU_GUARD_ALIGN_AND_SIZE) #ifdef __cplusplus }