All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] powerpc: improve copy_thread
@ 2023-01-31 16:55 Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 1/8] powerpc: copy_thread remove unused pkey code Nicholas Piggin
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

The way copy_thread works, particularly with kernel threads and kernel
created user threads is a bit muddled and confusing. This series tries
to straighten things out a bit.

Thanks,
Nick

Nicholas Piggin (8):
  powerpc: copy_thread remove unused pkey code
  powerpc: copy_thread make ret_from_fork register setup consistent
  powerpc: use switch frame for ret_from_kernel_thread parameters
  powerpc/64: ret_from_fork avoid restoring regs twice
  powerpc: copy_thread differentiate kthreads and user mode threads
  powerpc: differentiate kthread from user kernel thread start
  powerpc: copy_thread don't set _TIF_RESTOREALL
  powerpc: copy_thread don't set ppr in user interrupt frame regs

 arch/powerpc/kernel/entry_32.S     |  23 +++++-
 arch/powerpc/kernel/interrupt_64.S |  28 ++++++-
 arch/powerpc/kernel/process.c      | 121 ++++++++++++++++-------------
 3 files changed, 110 insertions(+), 62 deletions(-)

-- 
2.37.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/8] powerpc: copy_thread remove unused pkey code
  2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
@ 2023-01-31 16:55 ` Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 2/8] powerpc: copy_thread make ret_from_fork register setup consistent Nicholas Piggin
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

The pkey registers (AMR, IAMR) do not get loaded from the switch frame
so it is pointless to save anything there. Remove the dead code.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/process.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index c22cc234672f..ba10505f62c1 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1814,6 +1814,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 	sp -= STACK_SWITCH_FRAME_SIZE;
 	((unsigned long *)sp)[0] = sp + STACK_SWITCH_FRAME_SIZE;
 	kregs = (struct pt_regs *)(sp + STACK_SWITCH_FRAME_REGS);
+	kregs->nip = ppc_function_entry(f);
 	p->thread.ksp = sp;
 
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
@@ -1846,17 +1847,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 
 	p->thread.tidr = 0;
 #endif
-	/*
-	 * Run with the current AMR value of the kernel
-	 */
-#ifdef CONFIG_PPC_PKEY
-	if (mmu_has_feature(MMU_FTR_BOOK3S_KUAP))
-		kregs->amr = AMR_KUAP_BLOCKED;
-
-	if (mmu_has_feature(MMU_FTR_BOOK3S_KUEP))
-		kregs->iamr = AMR_KUEP_BLOCKED;
-#endif
-	kregs->nip = ppc_function_entry(f);
 	return 0;
 }
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/8] powerpc: copy_thread make ret_from_fork register setup consistent
  2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 1/8] powerpc: copy_thread remove unused pkey code Nicholas Piggin
@ 2023-01-31 16:55 ` Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 3/8] powerpc: use switch frame for ret_from_kernel_thread parameters Nicholas Piggin
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

The ret_from_fork code for 64e and 32-bit set r3 for
syscall_exit_prepare the same way that 64s does, so there should be no
need to special-case them in copy_thread.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/entry_32.S | 2 +-
 arch/powerpc/kernel/process.c  | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 5604c9a1ac22..755408c63be8 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -183,7 +183,7 @@ syscall_exit_finish:
 ret_from_fork:
 	REST_NVGPRS(r1)
 	bl	schedule_tail
-	li	r3,0
+	li	r3,0	/* fork() return value */
 	b	ret_from_syscall
 
 	.globl	ret_from_kernel_thread
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ba10505f62c1..dc66ca668b44 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1785,9 +1785,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 			childregs->gpr[1] = usp;
 		((unsigned long *)sp)[0] = childregs->gpr[1];
 		p->thread.regs = childregs;
-		/* 64s sets this in ret_from_fork */
-		if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64))
-			childregs->gpr[3] = 0;  /* Result from fork() */
 		if (clone_flags & CLONE_SETTLS) {
 			if (!is_32bit_task())
 				childregs->gpr[13] = tls;
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/8] powerpc: use switch frame for ret_from_kernel_thread parameters
  2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 1/8] powerpc: copy_thread remove unused pkey code Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 2/8] powerpc: copy_thread make ret_from_fork register setup consistent Nicholas Piggin
@ 2023-01-31 16:55 ` Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 4/8] powerpc/64: ret_from_fork avoid restoring regs twice Nicholas Piggin
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

The kernel thread path in copy_thread creates a user interrupt frame on
stack and stores the function and arg parameters there, and
ret_from_kernel_thread loads them. This is a slightly confusing way to
overload that frame. Non-volatile registers are loaded from the switch
frame, so the parameters can be stored there.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/entry_32.S     | 1 -
 arch/powerpc/kernel/interrupt_64.S | 1 -
 arch/powerpc/kernel/process.c      | 9 +++++++++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 755408c63be8..c3fdb3081d3d 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -188,7 +188,6 @@ ret_from_fork:
 
 	.globl	ret_from_kernel_thread
 ret_from_kernel_thread:
-	REST_NVGPRS(r1)
 	bl	schedule_tail
 	mtctr	r14
 	mr	r3,r15
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index fccc34489add..d60e7e7564df 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -741,7 +741,6 @@ _GLOBAL(ret_from_fork)
 
 _GLOBAL(ret_from_kernel_thread)
 	bl	schedule_tail
-	REST_NVGPRS(r1)
 	mtctr	r14
 	mr	r3,r15
 #ifdef CONFIG_PPC64_ELF_ABI_V2
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index dc66ca668b44..6cea224b7e60 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1812,6 +1812,15 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 	((unsigned long *)sp)[0] = sp + STACK_SWITCH_FRAME_SIZE;
 	kregs = (struct pt_regs *)(sp + STACK_SWITCH_FRAME_REGS);
 	kregs->nip = ppc_function_entry(f);
+	if (unlikely(args->fn)) {
+		/*
+		 * Put kthread create details in non-volatile GPRs in the
+		 * switch frame so they are loaded by _switch before it
+		 * returns to ret_from_kernel_thread.
+		 */
+		kregs->gpr[14] = ppc_function_entry((void *)args->fn);
+		kregs->gpr[15] = (unsigned long)args->fn_arg;
+	}
 	p->thread.ksp = sp;
 
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/8] powerpc/64: ret_from_fork avoid restoring regs twice
  2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
                   ` (2 preceding siblings ...)
  2023-01-31 16:55 ` [PATCH 3/8] powerpc: use switch frame for ret_from_kernel_thread parameters Nicholas Piggin
@ 2023-01-31 16:55 ` Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 5/8] powerpc: copy_thread differentiate kthreads and user mode threads Nicholas Piggin
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

If the system call return path always restores NVGPRs then there is no
need for ret_from_fork to do it. The HANDLER_RESTORE_NVGPRS does the
right thing for this.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/interrupt_64.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index d60e7e7564df..bac1f89501ac 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -728,14 +728,14 @@ DEFINE_FIXED_SYMBOL(__end_soft_masked, text)
 #ifdef CONFIG_PPC_BOOK3S
 _GLOBAL(ret_from_fork_scv)
 	bl	schedule_tail
-	REST_NVGPRS(r1)
+	HANDLER_RESTORE_NVGPRS()
 	li	r3,0	/* fork() return value */
 	b	.Lsyscall_vectored_common_exit
 #endif
 
 _GLOBAL(ret_from_fork)
 	bl	schedule_tail
-	REST_NVGPRS(r1)
+	HANDLER_RESTORE_NVGPRS()
 	li	r3,0	/* fork() return value */
 	b	.Lsyscall_exit
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/8] powerpc: copy_thread differentiate kthreads and user mode threads
  2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
                   ` (3 preceding siblings ...)
  2023-01-31 16:55 ` [PATCH 4/8] powerpc/64: ret_from_fork avoid restoring regs twice Nicholas Piggin
@ 2023-01-31 16:55 ` Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 6/8] powerpc: differentiate kthread from user kernel thread start Nicholas Piggin
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

When copy_thread is given a kernel function to run in arg->fn, this does
not necessarily mean it is a kernel thread. User threads can be created
this way (e.g., kernel_init, see also x86's copy_thread). These user
threads run a kernel function which may kernel_execve and return, which
then returns like a userspace exec syscall. Kernel threads can be
differentiated by PF_KTHREAD, will always have arg->fn set, and should
never return from that function, but call kthread_exit() to exit.

Create separate paths for the kthread and user kernel thread creation
logic. The kthread path will never exit and does not require a user
interrupt frame, so it gets a minimal stack frame.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/process.c | 99 +++++++++++++++++++++--------------
 1 file changed, 61 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 6cea224b7e60..82aad157c5f6 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1739,10 +1739,7 @@ static void setup_ksp_vsid(struct task_struct *p, unsigned long sp)
  */
 int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 {
-	unsigned long clone_flags = args->flags;
-	unsigned long usp = args->stack;
-	unsigned long tls = args->tls;
-	struct pt_regs *childregs, *kregs;
+	struct pt_regs *kregs; /* Switch frame regs */
 	extern void ret_from_fork(void);
 	extern void ret_from_fork_scv(void);
 	extern void ret_from_kernel_thread(void);
@@ -1755,49 +1752,77 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 
 	klp_init_thread_info(p);
 
-	/* Create initial stack frame. */
-	sp -= STACK_USER_INT_FRAME_SIZE;
-	*(unsigned long *)(sp + STACK_INT_FRAME_MARKER) = STACK_FRAME_REGS_MARKER;
-
-	/* Copy registers */
-	childregs = (struct pt_regs *)(sp + STACK_INT_FRAME_REGS);
-	if (unlikely(args->fn)) {
+	if (unlikely(p->flags & PF_KTHREAD)) {
 		/* kernel thread */
+
+		/* Create initial minimum stack frame. */
+		sp -= STACK_FRAME_MIN_SIZE;
 		((unsigned long *)sp)[0] = 0;
-		memset(childregs, 0, sizeof(struct pt_regs));
-		childregs->gpr[1] = sp + STACK_USER_INT_FRAME_SIZE;
-		/* function */
-		if (args->fn)
-			childregs->gpr[14] = ppc_function_entry((void *)args->fn);
+
+		f = ret_from_kernel_thread;
+		p->thread.regs = NULL;	/* no user register state */
 #ifdef CONFIG_PPC64
 		clear_tsk_thread_flag(p, TIF_32BIT);
-		childregs->softe = IRQS_ENABLED;
 #endif
-		childregs->gpr[15] = (unsigned long)args->fn_arg;
-		p->thread.regs = NULL;	/* no user register state */
-		ti->flags |= _TIF_RESTOREALL;
-		f = ret_from_kernel_thread;
 	} else {
 		/* user thread */
-		struct pt_regs *regs = current_pt_regs();
-		*childregs = *regs;
-		if (usp)
-			childregs->gpr[1] = usp;
-		((unsigned long *)sp)[0] = childregs->gpr[1];
-		p->thread.regs = childregs;
-		if (clone_flags & CLONE_SETTLS) {
-			if (!is_32bit_task())
-				childregs->gpr[13] = tls;
+		struct pt_regs *childregs;
+
+		/* Create initial user return stack frame. */
+		sp -= STACK_USER_INT_FRAME_SIZE;
+		*(unsigned long *)(sp + STACK_INT_FRAME_MARKER) = STACK_FRAME_REGS_MARKER;
+
+		childregs = (struct pt_regs *)(sp + STACK_INT_FRAME_REGS);
+
+		if (unlikely(args->fn)) {
+			/*
+			 * A user space thread, but it first runs a kernel
+			 * thread, and then returns as though it had called
+			 * execve rather than fork.
+			 */
+			((unsigned long *)sp)[0] = 0;
+			memset(childregs, 0, sizeof(struct pt_regs));
+#ifdef CONFIG_PPC64
+			childregs->softe = IRQS_ENABLED;
+#endif
+			ti->flags |= _TIF_RESTOREALL;
+			f = ret_from_kernel_thread;
+		} else {
+			struct pt_regs *regs = current_pt_regs();
+			unsigned long clone_flags = args->flags;
+			unsigned long usp = args->stack;
+
+			/* Copy registers */
+			*childregs = *regs;
+			if (usp)
+				childregs->gpr[1] = usp;
+			((unsigned long *)sp)[0] = childregs->gpr[1];
+#ifdef CONFIG_PPC_IRQ_SOFT_MASK_DEBUG
+			WARN_ON_ONCE(childregs->softe != IRQS_ENABLED);
+#endif
+			if (clone_flags & CLONE_SETTLS) {
+				unsigned long tls = args->tls;
+
+				if (!is_32bit_task())
+					childregs->gpr[13] = tls;
+				else
+					childregs->gpr[2] = tls;
+			}
+
+			if (trap_is_scv(regs))
+				f = ret_from_fork_scv;
 			else
-				childregs->gpr[2] = tls;
+				f = ret_from_fork;
 		}
 
-		if (trap_is_scv(regs))
-			f = ret_from_fork_scv;
-		else
-			f = ret_from_fork;
+#ifdef CONFIG_PPC64
+		if (cpu_has_feature(CPU_FTR_HAS_PPR))
+			childregs->ppr = DEFAULT_PPR;
+#endif
+
+		childregs->msr &= ~(MSR_FP|MSR_VEC|MSR_VSX);
+		p->thread.regs = childregs;
 	}
-	childregs->msr &= ~(MSR_FP|MSR_VEC|MSR_VSX);
 
 	/*
 	 * The way this works is that at some point in the future
@@ -1848,8 +1873,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 		p->thread.dscr_inherit = current->thread.dscr_inherit;
 		p->thread.dscr = mfspr(SPRN_DSCR);
 	}
-	if (cpu_has_feature(CPU_FTR_HAS_PPR))
-		childregs->ppr = DEFAULT_PPR;
 
 	p->thread.tidr = 0;
 #endif
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/8] powerpc: differentiate kthread from user kernel thread start
  2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
                   ` (4 preceding siblings ...)
  2023-01-31 16:55 ` [PATCH 5/8] powerpc: copy_thread differentiate kthreads and user mode threads Nicholas Piggin
@ 2023-01-31 16:55 ` Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 7/8] powerpc: copy_thread don't set _TIF_RESTOREALL Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 8/8] powerpc: copy_thread don't set ppr in user interrupt frame regs Nicholas Piggin
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

Kernel created user threads start similarly to kernel threads in that
they call a kernel function after first returning from _switch, so they
share ret_from_kernel_thread for this. Kernel threads never return from
that function though, whereas user threads often do (although some
don't, e.g., IO threads).

Split these startup functions in two, and catch kernel threads that
improperly return from their function. This is intended to make the
complicated code a little bit easier to understand.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/entry_32.S     | 20 ++++++++++++++++++--
 arch/powerpc/kernel/interrupt_64.S | 18 +++++++++++++++++-
 arch/powerpc/kernel/process.c      |  7 ++++---
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index c3fdb3081d3d..c33ac0b454dc 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -186,8 +186,8 @@ ret_from_fork:
 	li	r3,0	/* fork() return value */
 	b	ret_from_syscall
 
-	.globl	ret_from_kernel_thread
-ret_from_kernel_thread:
+	.globl	ret_from_kernel_user_thread
+ret_from_kernel_user_thread:
 	bl	schedule_tail
 	mtctr	r14
 	mr	r3,r15
@@ -196,6 +196,22 @@ ret_from_kernel_thread:
 	li	r3,0
 	b	ret_from_syscall
 
+	.globl	start_kernel_thread
+start_kernel_thread:
+	bl	schedule_tail
+	mtctr	r14
+	mr	r3,r15
+	PPC440EP_ERR42
+	bctrl
+	/*
+	 * This must not return. We actually want to BUG here, not WARN,
+	 * because BUG will exit the process which is what the kernel thread
+	 * should have done, which may give some hope of continuing.
+	 */
+100:	trap
+	EMIT_BUG_ENTRY 100b,__FILE__,__LINE__,0
+
+
 /*
  * This routine switches between two different tasks.  The process
  * state of one is saved on its kernel stack.  Then the state
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index bac1f89501ac..90370b89905b 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -739,7 +739,7 @@ _GLOBAL(ret_from_fork)
 	li	r3,0	/* fork() return value */
 	b	.Lsyscall_exit
 
-_GLOBAL(ret_from_kernel_thread)
+_GLOBAL(ret_from_kernel_user_thread)
 	bl	schedule_tail
 	mtctr	r14
 	mr	r3,r15
@@ -749,3 +749,19 @@ _GLOBAL(ret_from_kernel_thread)
 	bctrl
 	li	r3,0
 	b	.Lsyscall_exit
+
+_GLOBAL(start_kernel_thread)
+	bl	schedule_tail
+	mtctr	r14
+	mr	r3,r15
+#ifdef CONFIG_PPC64_ELF_ABI_V2
+	mr	r12,r14
+#endif
+	bctrl
+	/*
+	 * This must not return. We actually want to BUG here, not WARN,
+	 * because BUG will exit the process which is what the kernel thread
+	 * should have done, which may give some hope of continuing.
+	 */
+100:	trap
+	EMIT_BUG_ENTRY 100b,__FILE__,__LINE__,0
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 82aad157c5f6..53215cdb19dd 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1742,7 +1742,8 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 	struct pt_regs *kregs; /* Switch frame regs */
 	extern void ret_from_fork(void);
 	extern void ret_from_fork_scv(void);
-	extern void ret_from_kernel_thread(void);
+	extern void ret_from_kernel_user_thread(void);
+	extern void start_kernel_thread(void);
 	void (*f)(void);
 	unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
 	struct thread_info *ti = task_thread_info(p);
@@ -1759,7 +1760,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 		sp -= STACK_FRAME_MIN_SIZE;
 		((unsigned long *)sp)[0] = 0;
 
-		f = ret_from_kernel_thread;
+		f = start_kernel_thread;
 		p->thread.regs = NULL;	/* no user register state */
 #ifdef CONFIG_PPC64
 		clear_tsk_thread_flag(p, TIF_32BIT);
@@ -1786,7 +1787,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 			childregs->softe = IRQS_ENABLED;
 #endif
 			ti->flags |= _TIF_RESTOREALL;
-			f = ret_from_kernel_thread;
+			f = ret_from_kernel_user_thread;
 		} else {
 			struct pt_regs *regs = current_pt_regs();
 			unsigned long clone_flags = args->flags;
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 7/8] powerpc: copy_thread don't set _TIF_RESTOREALL
  2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
                   ` (5 preceding siblings ...)
  2023-01-31 16:55 ` [PATCH 6/8] powerpc: differentiate kthread from user kernel thread start Nicholas Piggin
@ 2023-01-31 16:55 ` Nicholas Piggin
  2023-01-31 16:55 ` [PATCH 8/8] powerpc: copy_thread don't set ppr in user interrupt frame regs Nicholas Piggin
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

In the kernel user thread path, don't set _TIF_RESTOREALL because the
thread is required to call kernel_execve() before it returns, which will
set _TIF_RESTOREALL if necessary via start_thread().

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/interrupt_64.S | 5 +++++
 arch/powerpc/kernel/process.c      | 2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 90370b89905b..f53012254fca 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -748,6 +748,11 @@ _GLOBAL(ret_from_kernel_user_thread)
 #endif
 	bctrl
 	li	r3,0
+	/*
+	 * It does not matter whether this returns via the scv or sc path
+	 * because it returns as execve() and therefore has no calling ABI
+	 * (i.e., it sets registers according to the exec()ed entry point).
+	 */
 	b	.Lsyscall_exit
 
 _GLOBAL(start_kernel_thread)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 53215cdb19dd..e67597fd998f 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1746,7 +1746,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 	extern void start_kernel_thread(void);
 	void (*f)(void);
 	unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
-	struct thread_info *ti = task_thread_info(p);
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
 	int i;
 #endif
@@ -1786,7 +1785,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 #ifdef CONFIG_PPC64
 			childregs->softe = IRQS_ENABLED;
 #endif
-			ti->flags |= _TIF_RESTOREALL;
 			f = ret_from_kernel_user_thread;
 		} else {
 			struct pt_regs *regs = current_pt_regs();
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 8/8] powerpc: copy_thread don't set ppr in user interrupt frame regs
  2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
                   ` (6 preceding siblings ...)
  2023-01-31 16:55 ` [PATCH 7/8] powerpc: copy_thread don't set _TIF_RESTOREALL Nicholas Piggin
@ 2023-01-31 16:55 ` Nicholas Piggin
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2023-01-31 16:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

syscalls do not set the PPR field in their interrupt frame and return
from syscall always sets the default PPR for userspace, so setting the
value in the ret_from_fork frame is not necessary and mildly
inconsistent. Remove it.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/process.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index e67597fd998f..3685a74a9041 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1814,11 +1814,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 				f = ret_from_fork;
 		}
 
-#ifdef CONFIG_PPC64
-		if (cpu_has_feature(CPU_FTR_HAS_PPR))
-			childregs->ppr = DEFAULT_PPR;
-#endif
-
 		childregs->msr &= ~(MSR_FP|MSR_VEC|MSR_VSX);
 		p->thread.regs = childregs;
 	}
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-01-31 17:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-31 16:55 [PATCH 0/8] powerpc: improve copy_thread Nicholas Piggin
2023-01-31 16:55 ` [PATCH 1/8] powerpc: copy_thread remove unused pkey code Nicholas Piggin
2023-01-31 16:55 ` [PATCH 2/8] powerpc: copy_thread make ret_from_fork register setup consistent Nicholas Piggin
2023-01-31 16:55 ` [PATCH 3/8] powerpc: use switch frame for ret_from_kernel_thread parameters Nicholas Piggin
2023-01-31 16:55 ` [PATCH 4/8] powerpc/64: ret_from_fork avoid restoring regs twice Nicholas Piggin
2023-01-31 16:55 ` [PATCH 5/8] powerpc: copy_thread differentiate kthreads and user mode threads Nicholas Piggin
2023-01-31 16:55 ` [PATCH 6/8] powerpc: differentiate kthread from user kernel thread start Nicholas Piggin
2023-01-31 16:55 ` [PATCH 7/8] powerpc: copy_thread don't set _TIF_RESTOREALL Nicholas Piggin
2023-01-31 16:55 ` [PATCH 8/8] powerpc: copy_thread don't set ppr in user interrupt frame regs Nicholas Piggin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.