From dc5d935d122ed78bf4c6a856515481dcd356c2e0 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 2 Jun 2017 12:56:47 -0700 Subject: [PATCH] kernel: introduce stack definition macros The existing __stack decorator is not flexible enough for upcoming thread stack memory protection scenarios. Wrap the entire thing in a declaration macro abstraction instead, which can be implemented on a per-arch or per-SOC basis. Issue: ZEP-2185 Signed-off-by: Andrew Boie --- include/kernel.h | 113 +++++++++++++++++++++++++++++++++++++++-- include/misc/stack.h | 4 ++ kernel/init.c | 26 ++++------ kernel/system_work_q.c | 4 +- 4 files changed, 126 insertions(+), 21 deletions(-) diff --git a/include/kernel.h b/include/kernel.h index ee04edd73e6..667bbd6a026 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -273,7 +273,7 @@ enum execution_context_types { /** * @brief Analyze the main, idle, interrupt and system workqueue call stacks * - * This routine calls @ref stack_analyze on the 4 call stacks declared and + * This routine calls @ref STACK_ANALYZE on the 4 call stacks declared and * maintained by the kernel. The sizes of those 4 call stacks are defined by: * * CONFIG_MAIN_STACK_SIZE @@ -367,6 +367,11 @@ typedef void (*k_thread_entry_t)(void *p1, void *p2, void *p3); * K_FP_REGS, and K_SSE_REGS. Multiple options may be specified by separating * them using "|" (the logical OR operator). * + * The stack itself should be declared with K_THREAD_STACK_DEFINE or variant + * macros. The stack size parameter should either be a defined constant + * also passed to K_THREAD_STACK_DEFINE, or the value of K_THREAD_STACK_SIZEOF. + * Do not use regular C sizeof(). + * * @param stack Pointer to the stack space. * @param stack_size Stack size in bytes. * @param entry Thread entry function. @@ -581,7 +586,7 @@ struct _static_thread_data { #define K_THREAD_DEFINE(name, stack_size, \ entry, p1, p2, p3, \ prio, options, delay) \ - char __noinit __stack _k_thread_stack_##name[stack_size]; \ + K_THREAD_STACK_DEFINE(_k_thread_stack_##name, stack_size); \ struct k_thread _k_thread_obj_##name; \ struct _static_thread_data _k_thread_data_##name __aligned(4) \ __in_section(_static_thread_data, static, name) = \ @@ -1953,8 +1958,11 @@ static inline int k_work_pending(struct k_work *work) * processing thread, which runs forever. * * @param work_q Address of workqueue. - * @param stack Pointer to work queue thread's stack space. - * @param stack_size Size of the work queue thread's stack (in bytes). + * @param stack Pointer to work queue thread's stack space, as defined by + * K_THREAD_STACK_DEFINE() + * @param stack_size Size of the work queue thread's stack (in bytes), which + * should either be the same constant passed to + * K_THREAD_STACK_DEFINE() or the value of K_THREAD_STACK_SIZEOF(). * @param prio Priority of the work queue's thread. * * @return N/A @@ -3729,6 +3737,103 @@ extern void _init_static_threads(void); extern int _is_thread_essential(void); extern void _timer_expiration_handler(struct _timeout *t); +/* arch/cpu.h may declare an architecture or platform-specific macro + * for properly declaring stacks, compatible with MMU/MPU constraints if + * enabled + */ +#ifdef _ARCH_THREAD_STACK_DEFINE +#define K_THREAD_STACK_DEFINE(sym, size) _ARCH_THREAD_STACK_DEFINE(sym, size) +#define K_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \ + _ARCH_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) +#define K_THREAD_STACK_MEMBER(sym, size) _ARCH_THREAD_STACK_MEMBER(sym, size) +#define K_THREAD_STACK_SIZEOF(sym) _ARCH_THREAD_STACK_SIZEOF(sym) +#define K_THREAD_STACK_BUFFER(sym) _ARCH_THREAD_STACK_BUFFER(sym) +#else +/** + * @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 + * + * The declared symbol will always be a character array which can be passed to + * k_thread_create, but should otherwise not be manipulated. + * + * It is legal to precede this definition with the 'static' keyword. + * + * It is NOT legal to take the sizeof(sym) and pass that to the stackSize + * parameter of k_thread_create(), it may not be the same as the + * 'size' parameter. Use K_THREAD_STACK_SIZEOF() instead. + * + * @param sym Thread stack symbol name + * @param size Size of the stack memory region + */ +#define K_THREAD_STACK_DEFINE(sym, size) \ + char __noinit __aligned(STACK_ALIGN) sym[size] + +/** + * @brief Declare a toplevel array of thread stack memory regions + * + * 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 + * + * @param sym Thread stack symbol name + * @param nmemb Number of stacks to declare + * @param size Size of the stack memory region + */ + +#define K_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \ + char __noinit __aligned(STACK_ALIGN) sym[nmemb][size] + +/** + * @brief Declare an embedded stack memory region + * + * Used for stacks embedded within other data structures. Use is highly + * discouraged but in some cases necessary. For memory protection scenarios, + * it is very important that any RAM preceding this member not be writable + * by threads else a stack overflow will lead to silent corruption. In other + * words, the containing data structure should live in RAM owned by the kernel. + * + * @param sym Thread stack symbol name + * @param size Size of the stack memory region + */ +#define K_THREAD_STACK_MEMBER(sym, size) \ + char __aligned(STACK_ALIGN) sym[size] + +/** + * @brief Return the size in bytes of a stack memory region + * + * Convenience macro for passing the desired stack size to k_thread_create() + * since the underlying implementation may actually create something larger + * (for instance a guard area). + * + * The value returned here is guaranteed to match the 'size' parameter + * passed to K_THREAD_STACK_DEFINE and related macros. + * + * @param sym Stack memory symbol + * @return Size of the stack + */ +#define K_THREAD_STACK_SIZEOF(sym) sizeof(sym) + +/** + * @brief Get a pointer to the physical stack buffer + * + * Convenience macro to get at the real underlying stack buffer used by + * the CPU. Guaranteed to be a character pointer of size K_THREAD_STACK_SIZEOF. + * This is really only intended for diagnostic tools which want to examine + * stack memory contents. + * + * @param sym Declared stack symbol name + * @return The buffer itself, a char * + */ +#define K_THREAD_STACK_BUFFER(sym) sym + +#endif /* _ARCH_DECLARE_STACK */ + #ifdef __cplusplus } #endif diff --git a/include/misc/stack.h b/include/misc/stack.h index 679e65e527a..9617a3bcba0 100644 --- a/include/misc/stack.h +++ b/include/misc/stack.h @@ -73,4 +73,8 @@ static inline void stack_analyze(const char *name, const char *stack, } #endif +#define STACK_ANALYZE(name, sym) \ + stack_analyze(name, K_THREAD_STACK_BUFFER(sym), \ + K_THREAD_STACK_SIZEOF(sym)) + #endif /* _MISC_STACK_H_ */ diff --git a/kernel/init.c b/kernel/init.c index cfed8b54ec2..2a52bae98ea 100644 --- a/kernel/init.c +++ b/kernel/init.c @@ -78,8 +78,8 @@ u64_t __noinit __end_tick_tsc; #define MAIN_STACK_SIZE CONFIG_MAIN_STACK_SIZE -char __noinit __stack _main_stack[MAIN_STACK_SIZE]; -char __noinit __stack _idle_stack[IDLE_STACK_SIZE]; +K_THREAD_STACK_DEFINE(_main_stack, MAIN_STACK_SIZE); +K_THREAD_STACK_DEFINE(_idle_stack, IDLE_STACK_SIZE); static struct k_thread _main_thread_s; static struct k_thread _idle_thread_s; @@ -98,7 +98,7 @@ k_tid_t const _idle_thread = (k_tid_t)&_idle_thread_s; #if CONFIG_ISR_STACK_SIZE & (STACK_ALIGN - 1) #error "ISR_STACK_SIZE must be a multiple of the stack alignment" #endif -char __noinit __stack _interrupt_stack[CONFIG_ISR_STACK_SIZE]; +K_THREAD_STACK_DEFINE(_interrupt_stack, CONFIG_ISR_STACK_SIZE); #ifdef CONFIG_SYS_CLOCK_EXISTS #include @@ -120,15 +120,13 @@ void k_call_stacks_analyze(void) #endif /* CONFIG_ARC */ printk("Kernel stacks:\n"); - stack_analyze("main ", _main_stack, sizeof(_main_stack)); - stack_analyze("idle ", _idle_stack, sizeof(_idle_stack)); + STACK_ANALYZE("main ", _main_stack); + STACK_ANALYZE("idle ", _idle_stack); #if defined(CONFIG_ARC) && CONFIG_RGF_NUM_BANKS != 1 - stack_analyze("firq ", _firq_stack, sizeof(_firq_stack)); + STACK_ANALYZE("firq ", _firq_stack); #endif /* CONFIG_ARC */ - stack_analyze("interrupt", _interrupt_stack, - sizeof(_interrupt_stack)); - stack_analyze("workqueue", sys_work_q_stack, - sizeof(sys_work_q_stack)); + STACK_ANALYZE("interrupt", _interrupt_stack); + STACK_ANALYZE("workqueue", sys_work_q_stack); #endif /* CONFIG_INIT_STACKS && CONFIG_PRINTK */ } @@ -329,12 +327,10 @@ extern void *__stack_chk_guard; FUNC_NORETURN void _Cstart(void) { #ifdef CONFIG_ARCH_HAS_CUSTOM_SWAP_TO_MAIN - void *dummy_thread = NULL; + struct k_thread *dummy_thread = NULL; #else - /* floating point is NOT used during kernel init */ - - char __stack dummy_stack[_K_THREAD_NO_FLOAT_SIZEOF]; - void *dummy_thread = dummy_stack; + struct k_thread dummy_thread_memory; + struct k_thread *dummy_thread = &dummy_thread_memory; #endif /* diff --git a/kernel/system_work_q.c b/kernel/system_work_q.c index f7810673a5b..2027b596fed 100644 --- a/kernel/system_work_q.c +++ b/kernel/system_work_q.c @@ -14,7 +14,7 @@ #include #include -char __noinit __stack sys_work_q_stack[CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE]; +K_THREAD_STACK_DEFINE(sys_work_q_stack, CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE); struct k_work_q k_sys_work_q; @@ -24,7 +24,7 @@ static int k_sys_work_q_init(struct device *dev) k_work_q_start(&k_sys_work_q, sys_work_q_stack, - sizeof(sys_work_q_stack), + K_THREAD_STACK_SIZEOF(sys_work_q_stack), CONFIG_SYSTEM_WORKQUEUE_PRIORITY); return 0;