| Message ID | 5559527F.9090300@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 18, 2015 at 03:36:10PM +0800, yalin wang wrote: > this patch fix the function in kernel_thread(), > when kernel is build as THUMB2 or BE8 endian, we should > also set the correct bit in CPSR, so that kernel can return to > the correct state to execute. Why do you think any of this is needed? When a kernel thread is created via kernel_thread(), copy_thread() is called with the function pointer in stack_start, and the functions argument in stk_size. When the scheduler switches to the thread, it reads the register state from thread->cpu_context, thereby loading r4 and r5 with the function argument and function pointer, and directing the PC to ret_from_fork. (For normal user clones and forks, r4 and r5 in kernel space will be zero.) The scheduler switch preserves the CPSR from the previous task, so if we're running a T2 BE8 kernel, the new thread will have its ret_from_fork called in T2 BE8 mode. ret_from_fork checks for a non-zero r5, and if so, calls that function, which will also see the CPSR set appropriately for the kernel mode. Functions called from kernel_thread() are not permitted to return, so we will never read the "childregs" off the top of the kernel stack. Childregs are initialised because we expect them to be at the top of every kernel stack. Ergo, this patch is not needed at all.
2015-05-18 17:40 GMT+08:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: > On Mon, May 18, 2015 at 03:36:10PM +0800, yalin wang wrote: >> this patch fix the function in kernel_thread(), >> when kernel is build as THUMB2 or BE8 endian, we should >> also set the correct bit in CPSR, so that kernel can return to >> the correct state to execute. > > Why do you think any of this is needed? > > When a kernel thread is created via kernel_thread(), copy_thread() is > called with the function pointer in stack_start, and the functions > argument in stk_size. > > When the scheduler switches to the thread, it reads the register state > from thread->cpu_context, thereby loading r4 and r5 with the function > argument and function pointer, and directing the PC to ret_from_fork. > (For normal user clones and forks, r4 and r5 in kernel space will be > zero.) > > The scheduler switch preserves the CPSR from the previous task, so if > we're running a T2 BE8 kernel, the new thread will have its ret_from_fork > called in T2 BE8 mode. > > ret_from_fork checks for a non-zero r5, and if so, calls that function, > which will also see the CPSR set appropriately for the kernel mode. > > Functions called from kernel_thread() are not permitted to return, so > we will never read the "childregs" off the top of the kernel stack. > Childregs are initialised because we expect them to be at the top of > every kernel stack. > > Ergo, this patch is not needed at all. Genius explanation, i see your idea, i really make a mistake about user thread / kernel thread return method . it is really a trick for kernel thread return implemented like this. :) Thank you !
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index f192a2a..9a7ab32 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -220,6 +220,12 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start, thread->cpu_context.r4 = stk_sz; thread->cpu_context.r5 = stack_start; childregs->ARM_cpsr = SVC_MODE; +#ifdef CONFIG_THUMB2_KERNEL + childregs->ARM_cpsr |= PSR_T_BIT; +#endif +#ifdef CONFIG_CPU_ENDIAN_BE8 + childregs->ARM_cpsr |= PSR_E_BIT; +#endif } thread->cpu_context.pc = (unsigned long)ret_from_fork;