Skip to contentSkip to conversation

powerpc: Call restore_sprs() on initial context switch

PATCH New Anton Blanchard 2 participants 2 messages Dec 9, 2015 PowerPC

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 powerpc: Call restore_sprs() on initial context switch

commit 152d523e6307 ("powerpc: Create context switch helpers save_sprs() and restore_sprs()") has an issue with newly created processes and kernel threads. These do not call back through __switch_to(), instead returning straight to ret_from_fork() or ret_from_kernel_thread().

This means restore_sprs() is not getting called. Add a call to it in ret_from_fork() and ret_from_kernel_thread().

Fixes 152d523e6307 ("powerpc: Create context switch helpers save_sprs() and restore_sprs()")
---
 arch/powerpc/kernel/entry_32.S | 20 ++++++++++++++++++++
 arch/powerpc/kernel/entry_64.S | 20 ++++++++++++++++++++
 arch/powerpc/kernel/process.c  | 19 ++++++++++++-------
 3 files changed, 52 insertions(+), 7 deletions(-)
arch/powerpc/kernel/entry_32.S +20 -0
index 2405631..64802a1 100644
@@ -423,6 +423,16 @@ END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
 	.globl	ret_from_fork
 ret_from_fork:
 	REST_NVGPRS(r1)
+	/*
+	 * The first context switch for a process doesn't return through
+	 * __switch_to(), so we need to call restore_sprs() here.
+	 */
+	mr	r4,r2
+	bl	restore_sprs
+	/*
+	 * restore_sprs() returns a pointer to the previous task struct,
+	 * which is what schedule_tail() expects to be passed.
+	 */
 	bl	schedule_tail
 	li	r3,0
 	b	ret_from_syscall
@@ -430,6 +440,16 @@ ret_from_fork:
 	.globl	ret_from_kernel_thread
 ret_from_kernel_thread:
 	REST_NVGPRS(r1)
+	/*
+	 * The first context switch for a kernel thread doesn't return through
+	 * __switch_to(), so we need to call restore_sprs() here.
+	 */
+	mr	r4,r2
+	bl	restore_sprs
+	/*
+	 * restore_sprs() returns a pointer to the previous task struct,
+	 * which is what schedule_tail() expects to be passed.
+	 */
 	bl	schedule_tail
 	mtlr	r14
 	mr	r3,r15
arch/powerpc/kernel/entry_64.S +20 -0
index c8b4225..a21a861 100644
@@ -409,12 +409,32 @@ _GLOBAL(ppc_switch_endian)
 	b	.Lsyscall_exit
 
 _GLOBAL(ret_from_fork)
+	/*
+	 * The first context switch for a process doesn't return through
+	 * __switch_to(), so we need to call restore_sprs() here.
+	 */
+	ld	r4,PACACURRENT(r13)
+	bl	restore_sprs
+	/*
+	 * restore_sprs() returns a pointer to the previous task struct,
+	 * which is what schedule_tail() expects to be passed.
+	 */
 	bl	schedule_tail
 	REST_NVGPRS(r1)
 	li	r3,0
 	b	.Lsyscall_exit
 
 _GLOBAL(ret_from_kernel_thread)
+	/*
+	 * The first context switch for a kernel thread doesn't return through
+	 * __switch_to(), so we need to call restore_sprs() here.
+	 */
+	ld	r4,PACACURRENT(r13)
+	bl	restore_sprs
+	/*
+	 * restore_sprs() returns a pointer to the previous task struct,
+	 * which is what schedule_tail() expects to be passed.
+	 */
 	bl	schedule_tail
 	REST_NVGPRS(r1)
 	mtlr	r14
arch/powerpc/kernel/process.c +12 -7
index 73d5ac0..ec10250 100644
@@ -854,9 +854,12 @@ static inline void save_sprs(struct thread_struct *t)
 #endif
 }
 
-static inline void restore_sprs(struct thread_struct *old_thread,
-				struct thread_struct *new_thread)
+struct task_struct *restore_sprs(struct task_struct *old,
+				 struct task_struct *new)
 {
+	struct thread_struct *old_thread = &old->thread;
+	struct thread_struct *new_thread = &new->thread;
+
 #ifdef CONFIG_ALTIVEC
 	if (cpu_has_feature(CPU_FTR_ALTIVEC) &&
 	    old_thread->vrsave != new_thread->vrsave)
@@ -891,6 +894,12 @@ static inline void restore_sprs(struct thread_struct *old_thread,
 			mtspr(SPRN_TAR, new_thread->tar);
 	}
 #endif
+
+	/*
+	 * ret_from_fork and ret_from_kernel_thread expect us to
+	 * return a pointer to the previous task struct.
+	 */
+	return old;
 }
 
 struct task_struct *__switch_to(struct task_struct *prev,
@@ -966,11 +975,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
 
 	last = _switch(old_thread, new_thread);
 
-	/* Need to recalculate these after calling _switch() */
-	old_thread = &last->thread;
-	new_thread = &current->thread;
-
-	restore_sprs(old_thread, new_thread);
+	restore_sprs(last, current);
 
 #ifdef CONFIG_PPC_BOOK3S_64
 	if (current_thread_info()->local_flags & _TLF_LAZY_MMU) {
Signature · 1 line
2.5.0
#1 lore raw
reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…

Hi Anton,

[auto build test ERROR on powerpc/next]
[also build test ERROR on next-20151208]
[cannot apply to v4.4-rc4]

url:    https://github.com/0day-ci/linux/commits/Anton-Blanchard/powerpc-Call-restore_sprs-on-initial-context-switch/20151209-123550
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)

reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc

All errors (new ones prefixed by >>):

arch/powerpc/kernel/process.c: In function 'restore_sprs':

>> arch/powerpc/kernel/process.c:861:24: error: unused variable 'new_thread' [-Werror=unused-variable]
     struct thread_struct *new_thread = &new->thread;
                           ^
>> arch/powerpc/kernel/process.c:860:24: error: unused variable 'old_thread' [-Werror=unused-variable]
     struct thread_struct *old_thread = &old->thread;
                           ^

cc1: all warnings being treated as errors

vim +/new_thread +861 arch/powerpc/kernel/process.c

854 #endif
855 }
856
857 struct task_struct *restore_sprs(struct task_struct *old,
858 struct task_struct *new)
859 {

 > 860		struct thread_struct *old_thread = &old->thread;
 > 861		struct thread_struct *new_thread = &new->thread;

862
863 #ifdef CONFIG_ALTIVEC
864 if (cpu_has_feature(CPU_FTR_ALTIVEC) &&

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation