| Message ID | 20190722132231.10169-1-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc: Wire up clone3 syscall | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch next (f3365d1a959d5c6527efe3d38276acc9b58e3f3f) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
On Mon, Jul 22, 2019 at 11:22:31PM +1000, Michael Ellerman wrote: > Wire up the new clone3 syscall added in commit 7f192e3cd316 ("fork: > add clone3"). > > This requires a ppc_clone3 wrapper, in order to save the non-volatile > GPRs before calling into the generic syscall code. Otherwise we hit > the BUG_ON in CHECK_FULL_REGS in copy_thread(). > > Lightly tested using Christian's test code on a Power8 LE VM. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Thank you, Michael! One comment below, otherwise: Acked-by: Christian Brauner <christian@brauner.io> > --- > arch/powerpc/include/asm/unistd.h | 1 + > arch/powerpc/kernel/entry_64.S | 5 +++++ > arch/powerpc/kernel/syscalls/syscall.tbl | 1 + > 3 files changed, 7 insertions(+) > > diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h > index 68473c3c471c..b0720c7c3fcf 100644 > --- a/arch/powerpc/include/asm/unistd.h > +++ b/arch/powerpc/include/asm/unistd.h > @@ -49,6 +49,7 @@ > #define __ARCH_WANT_SYS_FORK > #define __ARCH_WANT_SYS_VFORK > #define __ARCH_WANT_SYS_CLONE > +#define __ARCH_WANT_SYS_CLONE3 > > #endif /* __ASSEMBLY__ */ > #endif /* _ASM_POWERPC_UNISTD_H_ */ > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S > index d9105fcf4021..0a0b5310f54a 100644 > --- a/arch/powerpc/kernel/entry_64.S > +++ b/arch/powerpc/kernel/entry_64.S > @@ -487,6 +487,11 @@ _GLOBAL(ppc_clone) > bl sys_clone > b .Lsyscall_exit > > +_GLOBAL(ppc_clone3) > + bl save_nvgprs > + bl sys_clone3 > + b .Lsyscall_exit > + > _GLOBAL(ppc32_swapcontext) > bl save_nvgprs > bl compat_sys_swapcontext > diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl > index f2c3bda2d39f..6886ecb590d5 100644 > --- a/arch/powerpc/kernel/syscalls/syscall.tbl > +++ b/arch/powerpc/kernel/syscalls/syscall.tbl > @@ -516,3 +516,4 @@ > 432 common fsmount sys_fsmount > 433 common fspick sys_fspick > 434 common pidfd_open sys_pidfd_open > +435 common clone3 ppc_clone3 Note that in v5.3-rc1 there's now a comment that 435 is reserved in there. So this will likely cause a merge conflict. You might want to base your change off of v5.3-rc1 instead to avoid that. :) So basically: From 10b2e4176d712e45c7cb22af4aed4ce09818785c Mon Sep 17 00:00:00 2001 From: Michael Ellerman <mpe@ellerman.id.au> Date: Mon, 22 Jul 2019 23:22:31 +1000 Subject: [PATCH] powerpc: Wire up clone3 syscall Wire up the new clone3 syscall added in commit 7f192e3cd316 ("fork: add clone3"). This requires a ppc_clone3 wrapper, in order to save the non-volatile GPRs before calling into the generic syscall code. Otherwise we hit the BUG_ON in CHECK_FULL_REGS in copy_thread(). Lightly tested using Christian's test code on a Power8 LE VM. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Acked-by: Christian Brauner <christian@brauner.io> Link: https://lore.kernel.org/r/20190722132231.10169-1-mpe@ellerman.id.au --- arch/powerpc/include/asm/unistd.h | 1 + arch/powerpc/kernel/entry_64.S | 5 +++++ arch/powerpc/kernel/syscalls/syscall.tbl | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h index 68473c3c471c..b0720c7c3fcf 100644 --- a/arch/powerpc/include/asm/unistd.h +++ b/arch/powerpc/include/asm/unistd.h @@ -49,6 +49,7 @@ #define __ARCH_WANT_SYS_FORK #define __ARCH_WANT_SYS_VFORK #define __ARCH_WANT_SYS_CLONE +#define __ARCH_WANT_SYS_CLONE3 #endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_UNISTD_H_ */ diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S index d9105fcf4021..0a0b5310f54a 100644 --- a/arch/powerpc/kernel/entry_64.S +++ b/arch/powerpc/kernel/entry_64.S @@ -487,6 +487,11 @@ _GLOBAL(ppc_clone) bl sys_clone b .Lsyscall_exit +_GLOBAL(ppc_clone3) + bl save_nvgprs + bl sys_clone3 + b .Lsyscall_exit + _GLOBAL(ppc32_swapcontext) bl save_nvgprs bl compat_sys_swapcontext diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl index 3331749aab20..6886ecb590d5 100644 --- a/arch/powerpc/kernel/syscalls/syscall.tbl +++ b/arch/powerpc/kernel/syscalls/syscall.tbl @@ -516,4 +516,4 @@ 432 common fsmount sys_fsmount 433 common fspick sys_fspick 434 common pidfd_open sys_pidfd_open -# 435 reserved for clone3 +435 common clone3 ppc_clone3
Hi, may I also ask to provide ppc_clone3 symbol also for 32-bit powerpc? Otherwise Michael's patch breaks build for me: powerpc-e500v2-linux-gnuspe-ld: arch/powerpc/kernel/systbl.o: in function `sys_call_table': (.rodata+0x6cc): undefined reference to `ppc_clone3' make: *** [Makefile:1060: vmlinux] Error 1 The patch was tested using Christian's program on a real e500 machine. --- a/arch/powerpc/kernel/entry_32.S +++ b/arch/powerpc/kernel/entry_32.S @@ -597,6 +597,14 @@ ppc_clone: stw r0,_TRAP(r1) /* register set saved */ b sys_clone + .globl ppc_clone3 +ppc_clone3: + SAVE_NVGPRS(r1) + lwz r0,_TRAP(r1) + rlwinm r0,r0,0,0,30 /* clear LSB to indicate full */ + stw r0,_TRAP(r1) /* register set saved */ + b sys_clone3 + .globl ppc_swapcontext ppc_swapcontext: SAVE_NVGPRS(r1) I don't think this trivial hunk deserves a separate patch submission. Thanks, Arseny
On Wed, Jul 24, 2019 at 12:25:14PM +0700, Arseny Solokha wrote: > Hi, > > may I also ask to provide ppc_clone3 symbol also for 32-bit powerpc? Otherwise > Michael's patch breaks build for me: Makes sense. Michael, are you planning on picking this up? :) Christian > > powerpc-e500v2-linux-gnuspe-ld: arch/powerpc/kernel/systbl.o: in function `sys_call_table': > (.rodata+0x6cc): undefined reference to `ppc_clone3' > make: *** [Makefile:1060: vmlinux] Error 1 > > The patch was tested using Christian's program on a real e500 machine. > > --- a/arch/powerpc/kernel/entry_32.S > +++ b/arch/powerpc/kernel/entry_32.S > @@ -597,6 +597,14 @@ ppc_clone: > stw r0,_TRAP(r1) /* register set saved */ > b sys_clone > > + .globl ppc_clone3 > +ppc_clone3: > + SAVE_NVGPRS(r1) > + lwz r0,_TRAP(r1) > + rlwinm r0,r0,0,0,30 /* clear LSB to indicate full */ > + stw r0,_TRAP(r1) /* register set saved */ > + b sys_clone3 > + > .globl ppc_swapcontext > ppc_swapcontext: > SAVE_NVGPRS(r1) > > I don't think this trivial hunk deserves a separate patch submission. > > Thanks, > Arseny
Christian Brauner <christian@brauner.io> writes: > On Wed, Jul 24, 2019 at 12:25:14PM +0700, Arseny Solokha wrote: >> Hi, >> >> may I also ask to provide ppc_clone3 symbol also for 32-bit powerpc? Otherwise >> Michael's patch breaks build for me: > > Makes sense. Michael, are you planning on picking this up? :) Yes I obviously (I hope) am not going to merge a patch that breaks the 32-bit build :) I'll send a v2. cheers
Christian Brauner <christian@brauner.io> writes: > On Mon, Jul 22, 2019 at 11:22:31PM +1000, Michael Ellerman wrote: >> Wire up the new clone3 syscall added in commit 7f192e3cd316 ("fork: >> add clone3"). >> >> This requires a ppc_clone3 wrapper, in order to save the non-volatile >> GPRs before calling into the generic syscall code. Otherwise we hit >> the BUG_ON in CHECK_FULL_REGS in copy_thread(). >> >> Lightly tested using Christian's test code on a Power8 LE VM. >> >> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > > Thank you, Michael! > > One comment below, otherwise: > > Acked-by: Christian Brauner <christian@brauner.io> Thanks. ... >> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl >> index f2c3bda2d39f..6886ecb590d5 100644 >> --- a/arch/powerpc/kernel/syscalls/syscall.tbl >> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl >> @@ -516,3 +516,4 @@ >> 432 common fsmount sys_fsmount >> 433 common fspick sys_fspick >> 434 common pidfd_open sys_pidfd_open >> +435 common clone3 ppc_clone3 > > Note that in v5.3-rc1 there's now a comment that 435 is reserved in > there. So this will likely cause a merge conflict. You might want to > base your change off of v5.3-rc1 instead to avoid that. :) Thanks for the heads-up. My fixes branch is already based off pre-rc1, and in general Linus can handle a trivial merge conflict like that. But given I had to send a v2 to fix the 32-bit build (doh!), I'll move my fixes up past rc1 once Linus has merged what's in there now, and then do this patch based on top of rc1, so there'll be no conflict. cheers
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h index 68473c3c471c..b0720c7c3fcf 100644 --- a/arch/powerpc/include/asm/unistd.h +++ b/arch/powerpc/include/asm/unistd.h @@ -49,6 +49,7 @@ #define __ARCH_WANT_SYS_FORK #define __ARCH_WANT_SYS_VFORK #define __ARCH_WANT_SYS_CLONE +#define __ARCH_WANT_SYS_CLONE3 #endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_UNISTD_H_ */ diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S index d9105fcf4021..0a0b5310f54a 100644 --- a/arch/powerpc/kernel/entry_64.S +++ b/arch/powerpc/kernel/entry_64.S @@ -487,6 +487,11 @@ _GLOBAL(ppc_clone) bl sys_clone b .Lsyscall_exit +_GLOBAL(ppc_clone3) + bl save_nvgprs + bl sys_clone3 + b .Lsyscall_exit + _GLOBAL(ppc32_swapcontext) bl save_nvgprs bl compat_sys_swapcontext diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl index f2c3bda2d39f..6886ecb590d5 100644 --- a/arch/powerpc/kernel/syscalls/syscall.tbl +++ b/arch/powerpc/kernel/syscalls/syscall.tbl @@ -516,3 +516,4 @@ 432 common fsmount sys_fsmount 433 common fspick sys_fspick 434 common pidfd_open sys_pidfd_open +435 common clone3 ppc_clone3
Wire up the new clone3 syscall added in commit 7f192e3cd316 ("fork: add clone3"). This requires a ppc_clone3 wrapper, in order to save the non-volatile GPRs before calling into the generic syscall code. Otherwise we hit the BUG_ON in CHECK_FULL_REGS in copy_thread(). Lightly tested using Christian's test code on a Power8 LE VM. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/include/asm/unistd.h | 1 + arch/powerpc/kernel/entry_64.S | 5 +++++ arch/powerpc/kernel/syscalls/syscall.tbl | 1 + 3 files changed, 7 insertions(+)