2021-10-03 00:58:37

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH] RISC-V: Include clone3() on rv32

From: Palmer Dabbelt <[email protected]>

As far as I can tell this should be enabled on rv32 as well, I'm not
sure why it's rv64-only. checksyscalls is complaining about our lack of
clone3() on rv32.

Fixes: 56ac5e213933 ("riscv: enable sys_clone3 syscall for rv64")
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/include/uapi/asm/unistd.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/uapi/asm/unistd.h b/arch/riscv/include/uapi/asm/unistd.h
index 4b989ae15d59..8062996c2dfd 100644
--- a/arch/riscv/include/uapi/asm/unistd.h
+++ b/arch/riscv/include/uapi/asm/unistd.h
@@ -18,9 +18,10 @@
#ifdef __LP64__
#define __ARCH_WANT_NEW_STAT
#define __ARCH_WANT_SET_GET_RLIMIT
-#define __ARCH_WANT_SYS_CLONE3
#endif /* __LP64__ */

+#define __ARCH_WANT_SYS_CLONE3
+
#include <asm-generic/unistd.h>

/*
--
2.33.0.800.g4c38ced690-goog


2021-10-03 15:33:31

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] RISC-V: Include clone3() on rv32

On Sun, Oct 3, 2021 at 2:58 AM Palmer Dabbelt <[email protected]> wrote:
>
> From: Palmer Dabbelt <[email protected]>
>
> As far as I can tell this should be enabled on rv32 as well, I'm not
> sure why it's rv64-only. checksyscalls is complaining about our lack of
> clone3() on rv32.
>
> Fixes: 56ac5e213933 ("riscv: enable sys_clone3 syscall for rv64")
> Signed-off-by: Palmer Dabbelt <[email protected]>

We should probably reverse the polarity of this symbol and force
architectures that don't implement it properly to say they don't
have it, but for now, it definitely makes sense to treat this the same
way on 32-bit and 64-bit risc-v.

Reviewed-by: Arnd Bergmann <[email protected]>

2021-10-04 07:21:54

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH] RISC-V: Include clone3() on rv32

On Sat, Oct 02, 2021 at 05:21:20PM -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <[email protected]>
>
> As far as I can tell this should be enabled on rv32 as well, I'm not
> sure why it's rv64-only. checksyscalls is complaining about our lack of
> clone3() on rv32.
>
> Fixes: 56ac5e213933 ("riscv: enable sys_clone3 syscall for rv64")
> Signed-off-by: Palmer Dabbelt <[email protected]>
> ---

Thanks!
Acked-by: Christian Brauner <[email protected]>

2021-10-04 11:21:28

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH] RISC-V: Include clone3() on rv32

On Sun, Oct 03, 2021 at 05:30:24PM +0200, Arnd Bergmann wrote:
> On Sun, Oct 3, 2021 at 2:58 AM Palmer Dabbelt <[email protected]> wrote:
> >
> > From: Palmer Dabbelt <[email protected]>
> >
> > As far as I can tell this should be enabled on rv32 as well, I'm not
> > sure why it's rv64-only. checksyscalls is complaining about our lack of
> > clone3() on rv32.
> >
> > Fixes: 56ac5e213933 ("riscv: enable sys_clone3 syscall for rv64")
> > Signed-off-by: Palmer Dabbelt <[email protected]>
>
> We should probably reverse the polarity of this symbol and force
> architectures that don't implement it properly to say they don't
> have it, but for now, it definitely makes sense to treat this the same
> way on 32-bit and 64-bit risc-v.

I think we had that discussion back when I added it and I think I even
proposed that or you did but then we settled on __ARCH_WANT_SYS_CLONE3.
Most likely because it fell in line with the other
__ARCH_WANT_SYS_{CLONE,FORK}.

I think at this point its alpha, ia64, nios, sparc, and sh that don't
implement it. For some it looks trivial at first glance at least (Fwiw,
nios implements sys_clone() but doesn't select __ARCH_WANT_SYS_CLONE3):

diff --git a/arch/nios2/include/uapi/asm/unistd.h b/arch/nios2/include/uapi/asm/unistd.h
index 0b4bb1d41b28..6c4f45abd3ab 100644
--- a/arch/nios2/include/uapi/asm/unistd.h
+++ b/arch/nios2/include/uapi/asm/unistd.h
@@ -18,6 +18,7 @@

#define sys_mmap2 sys_mmap_pgoff

+#define __ARCH_WANT_SYS_CLONE3
#define __ARCH_WANT_RENAMEAT
#define __ARCH_WANT_STAT64
#define __ARCH_WANT_SET_GET_RLIMIT
diff --git a/arch/nios2/kernel/entry.S b/arch/nios2/kernel/entry.S
index 0794cd7803df..c1804bda8259 100644
--- a/arch/nios2/kernel/entry.S
+++ b/arch/nios2/kernel/entry.S
@@ -396,6 +396,15 @@ ENTRY(sys_clone)
RESTORE_SWITCH_STACK
ret

+/*
+ * int clone3(struct clone_args __user *, uargs, size_t, size)
+ */
+ENTRY(sys_clone3)
+ SAVE_SWITCH_STACK
+ call sys_clone3
+ RESTORE_SWITCH_STACK
+ ret
+
ENTRY(sys_rt_sigreturn)
SAVE_SWITCH_STACK
mov r4, sp

2021-10-05 00:37:56

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] RISC-V: Include clone3() on rv32

On Mon, 04 Oct 2021 04:17:58 PDT (-0700), [email protected] wrote:
> On Sun, Oct 03, 2021 at 05:30:24PM +0200, Arnd Bergmann wrote:
>> On Sun, Oct 3, 2021 at 2:58 AM Palmer Dabbelt <[email protected]> wrote:
>> >
>> > From: Palmer Dabbelt <[email protected]>
>> >
>> > As far as I can tell this should be enabled on rv32 as well, I'm not
>> > sure why it's rv64-only. checksyscalls is complaining about our lack of
>> > clone3() on rv32.
>> >
>> > Fixes: 56ac5e213933 ("riscv: enable sys_clone3 syscall for rv64")
>> > Signed-off-by: Palmer Dabbelt <[email protected]>
>>
>> We should probably reverse the polarity of this symbol and force
>> architectures that don't implement it properly to say they don't
>> have it, but for now, it definitely makes sense to treat this the same
>> way on 32-bit and 64-bit risc-v.
>
> I think we had that discussion back when I added it and I think I even
> proposed that or you did but then we settled on __ARCH_WANT_SYS_CLONE3.
> Most likely because it fell in line with the other
> __ARCH_WANT_SYS_{CLONE,FORK}.
>
> I think at this point its alpha, ia64, nios, sparc, and sh that don't
> implement it. For some it looks trivial at first glance at least (Fwiw,
> nios implements sys_clone() but doesn't select __ARCH_WANT_SYS_CLONE3):
>
> diff --git a/arch/nios2/include/uapi/asm/unistd.h b/arch/nios2/include/uapi/asm/unistd.h
> index 0b4bb1d41b28..6c4f45abd3ab 100644
> --- a/arch/nios2/include/uapi/asm/unistd.h
> +++ b/arch/nios2/include/uapi/asm/unistd.h
> @@ -18,6 +18,7 @@
>
> #define sys_mmap2 sys_mmap_pgoff
>
> +#define __ARCH_WANT_SYS_CLONE3
> #define __ARCH_WANT_RENAMEAT
> #define __ARCH_WANT_STAT64
> #define __ARCH_WANT_SET_GET_RLIMIT
> diff --git a/arch/nios2/kernel/entry.S b/arch/nios2/kernel/entry.S
> index 0794cd7803df..c1804bda8259 100644
> --- a/arch/nios2/kernel/entry.S
> +++ b/arch/nios2/kernel/entry.S
> @@ -396,6 +396,15 @@ ENTRY(sys_clone)
> RESTORE_SWITCH_STACK
> ret
>
> +/*
> + * int clone3(struct clone_args __user *, uargs, size_t, size)
> + */
> +ENTRY(sys_clone3)
> + SAVE_SWITCH_STACK
> + call sys_clone3
> + RESTORE_SWITCH_STACK
> + ret
> +
> ENTRY(sys_rt_sigreturn)
> SAVE_SWITCH_STACK
> mov r4, sp

Thanks.

I've put this on fixes, but if you're trying to do that refactoring I've
merged it in as a single patch on top of 5.15-rc1. That's on
palmer/riscv-clone3, in case it helps someone avoid a conflict when
doing that refactoring. I'd usually offer to do the refactoring, but
I'm super buried right now with all the RISC-V stuff ;)

I want to call this fix because it's breaking my builds, these
checksyscall warnings have recently turned into errors.

2021-10-05 09:16:00

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH] RISC-V: Include clone3() on rv32

On Mon, Oct 04, 2021 at 05:35:40PM -0700, Palmer Dabbelt wrote:
> On Mon, 04 Oct 2021 04:17:58 PDT (-0700), [email protected] wrote:
> > On Sun, Oct 03, 2021 at 05:30:24PM +0200, Arnd Bergmann wrote:
> > > On Sun, Oct 3, 2021 at 2:58 AM Palmer Dabbelt <[email protected]> wrote:
> > > >
> > > > From: Palmer Dabbelt <[email protected]>
> > > >
> > > > As far as I can tell this should be enabled on rv32 as well, I'm not
> > > > sure why it's rv64-only. checksyscalls is complaining about our lack of
> > > > clone3() on rv32.
> > > >
> > > > Fixes: 56ac5e213933 ("riscv: enable sys_clone3 syscall for rv64")
> > > > Signed-off-by: Palmer Dabbelt <[email protected]>
> > >
> > > We should probably reverse the polarity of this symbol and force
> > > architectures that don't implement it properly to say they don't
> > > have it, but for now, it definitely makes sense to treat this the same
> > > way on 32-bit and 64-bit risc-v.
> >
> > I think we had that discussion back when I added it and I think I even
> > proposed that or you did but then we settled on __ARCH_WANT_SYS_CLONE3.
> > Most likely because it fell in line with the other
> > __ARCH_WANT_SYS_{CLONE,FORK}.
> >
> > I think at this point its alpha, ia64, nios, sparc, and sh that don't
> > implement it. For some it looks trivial at first glance at least (Fwiw,
> > nios implements sys_clone() but doesn't select __ARCH_WANT_SYS_CLONE3):
> >
> > diff --git a/arch/nios2/include/uapi/asm/unistd.h b/arch/nios2/include/uapi/asm/unistd.h
> > index 0b4bb1d41b28..6c4f45abd3ab 100644
> > --- a/arch/nios2/include/uapi/asm/unistd.h
> > +++ b/arch/nios2/include/uapi/asm/unistd.h
> > @@ -18,6 +18,7 @@
> >
> > #define sys_mmap2 sys_mmap_pgoff
> >
> > +#define __ARCH_WANT_SYS_CLONE3
> > #define __ARCH_WANT_RENAMEAT
> > #define __ARCH_WANT_STAT64
> > #define __ARCH_WANT_SET_GET_RLIMIT
> > diff --git a/arch/nios2/kernel/entry.S b/arch/nios2/kernel/entry.S
> > index 0794cd7803df..c1804bda8259 100644
> > --- a/arch/nios2/kernel/entry.S
> > +++ b/arch/nios2/kernel/entry.S
> > @@ -396,6 +396,15 @@ ENTRY(sys_clone)
> > RESTORE_SWITCH_STACK
> > ret
> >
> > +/*
> > + * int clone3(struct clone_args __user *, uargs, size_t, size)
> > + */
> > +ENTRY(sys_clone3)
> > + SAVE_SWITCH_STACK
> > + call sys_clone3
> > + RESTORE_SWITCH_STACK
> > + ret
> > +
> > ENTRY(sys_rt_sigreturn)
> > SAVE_SWITCH_STACK
> > mov r4, sp
>
> Thanks.
>
> I've put this on fixes, but if you're trying to do that refactoring I've
> merged it in as a single patch on top of 5.15-rc1. That's on
> palmer/riscv-clone3, in case it helps someone avoid a conflict when doing
> that refactoring. I'd usually offer to do the refactoring, but I'm super
> buried right now with all the RISC-V stuff ;)
>
> I want to call this fix because it's breaking my builds, these checksyscall
> warnings have recently turned into errors.

Oh yeah, please just merge your fix here for this issue. I think
flipping the symbol isn't that pressing right now and fixing your builds
is more urgent! :)