From 2dd91eca0e25e35ab00567461ff4f3d773db24db Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Wed, 6 Jun 2018 08:45:01 -0700 Subject: [PATCH] kernel: move thread monitor init to common code The original implementation of CONFIG_THREAD_MONITOR would try to leverage a thread's initial stack layout to provide the entry function with arguments for any given thread. This is problematic: - Some arches do not have a initial stack layout suitable for this - Some arches never enabled this at all (riscv32, nios2) - Some arches did not enable this properly - Dropping to user mode would erase or provide incorrect information. Just spend a few extra bytes to store this stuff directly in the k_thread struct and get rid of all the arch-specific code for this. Signed-off-by: Andrew Boie --- arch/arc/core/thread.c | 11 ----------- arch/arm/core/thread.c | 17 ----------------- arch/nios2/core/thread.c | 2 -- arch/posix/core/thread.c | 3 --- arch/riscv32/core/thread.c | 2 -- arch/x86/core/thread.c | 4 ---- arch/xtensa/core/thread.c | 9 --------- arch/xtensa/core/xtensa-asm2.c | 9 --------- arch/xtensa/include/kernel_arch_thread.h | 7 ------- include/kernel.h | 2 +- kernel/include/kernel_structs.h | 19 ------------------- kernel/thread.c | 18 ++++++++++++++++++ samples/subsys/debug/sysview/src/main.c | 2 +- 13 files changed, 20 insertions(+), 85 deletions(-) diff --git a/arch/arc/core/thread.c b/arch/arc/core/thread.c index c2b5d2d2ae1..90411817337 100644 --- a/arch/arc/core/thread.c +++ b/arch/arc/core/thread.c @@ -174,15 +174,6 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, thread->arch.priv_stack_size = 0; } #endif - -#ifdef CONFIG_THREAD_MONITOR - /* - * In debug mode thread->entry give direct access to the thread entry - * and the corresponding parameters. - */ - thread->entry = (struct __thread_entry *)(pInitCtx); -#endif - /* * intlock_key is constructed based on ARCv2 ISA Programmer's * Reference Manual CLRI instruction description: @@ -195,8 +186,6 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, (u32_t)pInitCtx - ___callee_saved_stack_t_SIZEOF; /* initial values in all other regs/k_thread entries are irrelevant */ - - thread_monitor_init(thread); } diff --git a/arch/arm/core/thread.c b/arch/arm/core/thread.c index 91569ee579b..4d1463e9e50 100644 --- a/arch/arm/core/thread.c +++ b/arch/arm/core/thread.c @@ -68,27 +68,10 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, char *stackEnd = pStackMem + stackSize; #endif struct __esf *pInitCtx; -#ifdef CONFIG_THREAD_MONITOR - struct __thread_entry *pMon; -#endif _new_thread_init(thread, pStackMem, stackEnd - pStackMem, priority, options); -#ifdef CONFIG_THREAD_MONITOR - pMon = (struct __thread_entry *) - STACK_ROUND_DOWN(stackEnd - sizeof(struct __thread_entry)); - pMon->pEntry = pEntry; - pMon->parameter1 = parameter1; - pMon->parameter2 = parameter2; - pMon->parameter3 = parameter3; - - thread->entry = pMon; - thread_monitor_init(thread); - - stackEnd = (char *)pMon; -#endif - /* carve the thread entry struct from the "base" of the stack */ pInitCtx = (struct __esf *)(STACK_ROUND_DOWN(stackEnd - sizeof(struct __esf))); diff --git a/arch/nios2/core/thread.c b/arch/nios2/core/thread.c index 2f6108ea138..85a9062ce47 100644 --- a/arch/nios2/core/thread.c +++ b/arch/nios2/core/thread.c @@ -57,6 +57,4 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, thread->callee_saved.ra = (u32_t)_thread_entry_wrapper; thread->callee_saved.key = NIOS2_STATUS_PIE_MSK; /* Leave the rest of thread->callee_saved junk */ - - thread_monitor_init(thread); } diff --git a/arch/posix/core/thread.c b/arch/posix/core/thread.c index 9d011fd434f..07917a2b1e0 100644 --- a/arch/posix/core/thread.c +++ b/arch/posix/core/thread.c @@ -77,10 +77,7 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, thread->callee_saved.thread_status = (u32_t)thread_status; - posix_new_thread(thread_status); - - thread_monitor_init(thread); } void posix_new_thread_pre_start(void) diff --git a/arch/riscv32/core/thread.c b/arch/riscv32/core/thread.c index 2dc56776948..2fd3357ad00 100644 --- a/arch/riscv32/core/thread.c +++ b/arch/riscv32/core/thread.c @@ -64,6 +64,4 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, stack_init->mepc = (u32_t)_thread_entry_wrapper; thread->callee_saved.sp = (u32_t)stack_init; - - thread_monitor_init(thread); } diff --git a/arch/x86/core/thread.c b/arch/x86/core/thread.c index c99c9236bf6..7abae875bb1 100644 --- a/arch/x86/core/thread.c +++ b/arch/x86/core/thread.c @@ -124,10 +124,6 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, #if defined(CONFIG_FP_SHARING) thread->arch.excNestCount = 0; #endif /* CONFIG_FP_SHARING */ -#ifdef CONFIG_THREAD_MONITOR - thread->entry = (struct __thread_entry *)&initial_frame->entry; - thread_monitor_init(thread); -#endif } #ifdef CONFIG_X86_USERSPACE diff --git a/arch/xtensa/core/thread.c b/arch/xtensa/core/thread.c index 389051e7866..86d40f1fe1b 100644 --- a/arch/xtensa/core/thread.c +++ b/arch/xtensa/core/thread.c @@ -118,17 +118,8 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, #endif thread->callee_saved.topOfStack = pInitCtx; thread->arch.flags = 0; -#ifdef CONFIG_THREAD_MONITOR - /* - * In debug mode thread->entry give direct access to the thread entry - * and the corresponding parameters. - */ - thread->entry = (struct __thread_entry *)(pInitCtx); -#endif /* initial values in all other registers/k_thread entries are * irrelevant */ - - thread_monitor_init(thread); } diff --git a/arch/xtensa/core/xtensa-asm2.c b/arch/xtensa/core/xtensa-asm2.c index b8ca9189165..b5ad493ce88 100644 --- a/arch/xtensa/core/xtensa-asm2.c +++ b/arch/xtensa/core/xtensa-asm2.c @@ -71,15 +71,6 @@ void _new_thread(struct k_thread *thread, k_thread_stack_t *stack, size_t sz, _new_thread_init(thread, base, sz, prio, opts); -#ifdef CONFIG_THREAD_MONITOR - top -= sizeof(struct __thread_entry); - thread->entry = (void *)top; - thread->entry->pEntry = entry; - - thread_monitor_init(thread); -#endif - - thread->switch_handle = xtensa_init_stack((void *)top, entry, p1, p2, p3); } diff --git a/arch/xtensa/include/kernel_arch_thread.h b/arch/xtensa/include/kernel_arch_thread.h index 2989210f252..79cebc4f78d 100644 --- a/arch/xtensa/include/kernel_arch_thread.h +++ b/arch/xtensa/include/kernel_arch_thread.h @@ -110,13 +110,6 @@ struct _thread_arch { #ifdef CONFIG_THREAD_CUSTOM_DATA void *custom_data; /* available for custom use */ #endif -#if defined(CONFIG_THREAD_MONITOR) - /* thread entry and parameters description */ - struct __thread_entry *entry; - - /* next item in list of ALL threads n*/ - struct k_thread *next_thread; -#endif #ifdef CONFIG_ERRNO int errno_var; #endif diff --git a/include/kernel.h b/include/kernel.h index d58f1a8e8a7..d9f3c2da77d 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -546,7 +546,7 @@ struct k_thread { #if defined(CONFIG_THREAD_MONITOR) /** thread entry and parameters description */ - struct __thread_entry *entry; + struct __thread_entry entry; /** next item in list of all threads */ struct k_thread *next_thread; diff --git a/kernel/include/kernel_structs.h b/kernel/include/kernel_structs.h index cf26ca9cd83..db40c459bc2 100644 --- a/kernel/include/kernel_structs.h +++ b/kernel/include/kernel_structs.h @@ -247,25 +247,6 @@ static ALWAYS_INLINE void _new_thread_init(struct k_thread *thread, #endif /* CONFIG_THREAD_STACK_INFO */ } -#if defined(CONFIG_THREAD_MONITOR) -/* - * Add a thread to the kernel's list of active threads. - */ -static ALWAYS_INLINE void thread_monitor_init(struct k_thread *thread) -{ - unsigned int key; - - key = irq_lock(); - thread->next_thread = _kernel.threads; - _kernel.threads = thread; - irq_unlock(key); -} -#else -#define thread_monitor_init(thread) \ - do {/* do nothing */ \ - } while ((0)) -#endif /* CONFIG_THREAD_MONITOR */ - #endif /* _ASMLANGUAGE */ #endif /* _kernel_structs__h_ */ diff --git a/kernel/thread.c b/kernel/thread.c index 62cd3dc8cf9..1f0f0b10102 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -298,6 +298,18 @@ void _setup_new_thread(struct k_thread *new_thread, _new_thread(new_thread, stack, stack_size, entry, p1, p2, p3, prio, options); +#ifdef CONFIG_THREAD_MONITOR + new_thread->entry.pEntry = entry; + new_thread->entry.parameter1 = p1; + new_thread->entry.parameter2 = p2; + new_thread->entry.parameter3 = p3; + + int key = irq_lock(); + + new_thread->next_thread = _kernel.threads; + _kernel.threads = new_thread; + irq_unlock(key); +#endif #ifdef CONFIG_USERSPACE _k_object_init(new_thread); _k_object_init(stack); @@ -641,6 +653,12 @@ FUNC_NORETURN void k_thread_user_mode_enter(k_thread_entry_t entry, { _current->base.user_options |= K_USER; _thread_essential_clear(); +#ifdef CONFIG_THREAD_MONITOR + _current->entry.pEntry = entry; + _current->entry.parameter1 = p1; + _current->entry.parameter2 = p2; + _current->entry.parameter3 = p3; +#endif #ifdef CONFIG_USERSPACE _arch_user_mode_enter(entry, p1, p2, p3); #else diff --git a/samples/subsys/debug/sysview/src/main.c b/samples/subsys/debug/sysview/src/main.c index 96ae5d0d275..677c8c268e0 100644 --- a/samples/subsys/debug/sysview/src/main.c +++ b/samples/subsys/debug/sysview/src/main.c @@ -99,7 +99,7 @@ static void sysview_api_send_task(const struct k_thread *thr, void *udata) char name[20]; snprintk(name, sizeof(name), "T%xE%x", - (uintptr_t)thr, (uintptr_t)thr->entry); + (uintptr_t)thr, (uintptr_t)(&thr->entry)); /* NOTE: struct k_thread is inside the stack on Zephyr 1.7. * This is not guaranteed by the API, and is likely to change