From 86430d8d4655d711c0b7419092bab19e0d7062b7 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Thu, 16 Jan 2020 09:49:12 -0800 Subject: [PATCH] kernel: arch: Clarify output switch handle requirements in arch_switch The original intent was that the output handle be written through the pointer in the second argument, though not all architectures used that scheme. As it turns out, that write is becoming a synchronization signal, so it's no longer optional. Clarify the documentation in arch_switch() about this requirement, and add an instruction to the x86_64 context switch to implement it as original envisioned. Signed-off-by: Andy Ross --- arch/x86/core/intel64/locore.S | 7 ++++++ kernel/include/kernel_arch_interface.h | 31 +++++++++++++++++++++----- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/arch/x86/core/intel64/locore.S b/arch/x86/core/intel64/locore.S index 72884cbdad7..9674491ee17 100644 --- a/arch/x86/core/intel64/locore.S +++ b/arch/x86/core/intel64/locore.S @@ -220,6 +220,13 @@ z_x86_switch: movq $X86_KERNEL_CS, _thread_offset_to_cs(%rsi) movq $X86_KERNEL_DS, _thread_offset_to_ss(%rsi) #endif + /* Store the handle (i.e. our thread struct address) into the + * switch handle field, this is a synchronization signal that + * must occur after the last data from the old context is + * saved. + */ + movq %rsi, ___thread_t_switch_handle_OFFSET(%rsi) + movq %gs:__x86_tss64_t_ist1_OFFSET, %rsp /* fall through to __resume */ diff --git a/kernel/include/kernel_arch_interface.h b/kernel/include/kernel_arch_interface.h index a282aef4f37..16917a3fbc5 100644 --- a/kernel/include/kernel_arch_interface.h +++ b/kernel/include/kernel_arch_interface.h @@ -89,12 +89,31 @@ void arch_new_thread(struct k_thread *thread, k_thread_stack_t *pStack, * 3) Set the stack pointer to the value provided in switch_to * 4) Pop off all thread state from the stack we switched to and return. * - * Some arches may implement thread->switch handle as a pointer to the thread - * itself, and save context somewhere in thread->arch. In this case, on initial - * context switch from the dummy thread, thread->switch handle for the outgoing - * thread is NULL. Instead of dereferencing switched_from all the way to get - * the thread pointer, subtract ___thread_t_switch_handle_OFFSET to obtain the - * thread pointer instead. + * Some arches may implement thread->switch handle as a pointer to the + * thread itself, and save context somewhere in thread->arch. In this + * case, on initial context switch from the dummy thread, + * thread->switch handle for the outgoing thread is NULL. Instead of + * dereferencing switched_from all the way to get the thread pointer, + * subtract ___thread_t_switch_handle_OFFSET to obtain the thread + * pointer instead. That is, such a scheme would have behavior like + * (in C pseudocode): + * + * void arch_switch(void *switch_to, void **switched_from) + * { + * struct k_thread *new = switch_to; + * struct k_thread *old = CONTAINER_OF(switched_from, struct k_thread, + * switch_handle); + * + * // save old context... + * *switched_from = old; + * // restore new context... + * } + * + * Note that, regardless of the underlying handle representation, the + * incoming switched_from pointer MUST be written through with a + * non-NULL value after all relevant thread state has been saved. The + * kernel uses this as a synchronization signal to be able to wait for + * switch completion from another CPU. * * @param switch_to Incoming thread's switch handle * @param switched_from Pointer to outgoing thread's switch handle storage