| Message ID | 20211221163532.2636028-6-guoren@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: compat: Add COMPAT mode support for rv64 | expand |
On Tue, Dec 21, 2021 at 5:35 PM <guoren@kernel.org> wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > Implement compat_syscall_table.c with compat_sys_call_table & fixup > system call such as truncate64,pread64,fallocate which need two > regs to indicate 64bit-arg (copied from arm64). > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > --- > arch/riscv/include/asm/syscall.h | 3 + > arch/riscv/kernel/compat_syscall_table.c | 84 ++++++++++++++++++++++++ Same here, I think most of these should go next to the actual syscalls, with the duplicates removed from other platforms, > +#define __SYSCALL_COMPAT > +#undef __LP64__ What is the #undef for? > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, > + unsigned long, prot, unsigned long, flags, > + unsigned long, fd, unsigned long, offset) > +{ > + if ((prot & PROT_WRITE) && (prot & PROT_EXEC)) > + if (unlikely(!(prot & PROT_READ))) > + return -EINVAL; > + > + return ksys_mmap_pgoff(addr, len, prot, flags, fd, offset); > +} This is one that we may have to deal with separately, introducing sys_mmap_pgoff() was a mistake in my opinion, and we should just have added a sys_mmap2() for all architectures that don't explicitly override it. Arnd
On Wed, Dec 22, 2021 at 2:15 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Dec 21, 2021 at 5:35 PM <guoren@kernel.org> wrote: > > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > Implement compat_syscall_table.c with compat_sys_call_table & fixup > > system call such as truncate64,pread64,fallocate which need two > > regs to indicate 64bit-arg (copied from arm64). > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > --- > > arch/riscv/include/asm/syscall.h | 3 + > > arch/riscv/kernel/compat_syscall_table.c | 84 ++++++++++++++++++++++++ > > Same here, I think most of these should go next to the actual syscalls, with the > duplicates removed from other platforms, Agree, I will try that next version. > > > +#define __SYSCALL_COMPAT > > +#undef __LP64__ > > What is the #undef for? See arch/riscv/include/uapi/asm/unistd.h: #ifdef __LP64__ #define __ARCH_WANT_NEW_STAT #define __ARCH_WANT_SET_GET_RLIMIT #endif /* __LP64__ */ > > > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, > > + unsigned long, prot, unsigned long, flags, > > + unsigned long, fd, unsigned long, offset) > > +{ > > + if ((prot & PROT_WRITE) && (prot & PROT_EXEC)) > > + if (unlikely(!(prot & PROT_READ))) > > + return -EINVAL; > > + > > + return ksys_mmap_pgoff(addr, len, prot, flags, fd, offset); > > +} > > This is one that we may have to deal with separately, introducing > sys_mmap_pgoff() was a mistake in my opinion, and we should just have #if __BITS_PER_LONG == 32 || defined(__SYSCALL_COMPAT) #define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _32) #else #define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _64) #endif #define __NR3264_mmap 222 __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap) > added a sys_mmap2() for all architectures that don't explicitly override it. That should be another patch, right? Let's keep it here. > > Arnd
On Wed, Dec 22, 2021 at 1:43 PM Guo Ren <guoren@kernel.org> wrote: > > On Wed, Dec 22, 2021 at 2:15 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Tue, Dec 21, 2021 at 5:35 PM <guoren@kernel.org> wrote: > > > > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > > > Implement compat_syscall_table.c with compat_sys_call_table & fixup > > > system call such as truncate64,pread64,fallocate which need two > > > regs to indicate 64bit-arg (copied from arm64). > > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > > --- > > > arch/riscv/include/asm/syscall.h | 3 + > > > arch/riscv/kernel/compat_syscall_table.c | 84 ++++++++++++++++++++++++ > > > > Same here, I think most of these should go next to the actual syscalls, with the > > duplicates removed from other platforms, > Agree, I will try that next version. > > > > > > +#define __SYSCALL_COMPAT > > > +#undef __LP64__ > > > > What is the #undef for? > > See arch/riscv/include/uapi/asm/unistd.h: > > #ifdef __LP64__ > #define __ARCH_WANT_NEW_STAT > #define __ARCH_WANT_SET_GET_RLIMIT > #endif /* __LP64__ */ Ok, in this case I would recommend changing that #ifdef to check for __SYSCALL_COMPAT instead, as removing the __LP64__ define may cause other unintended changes. > > > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, > > > + unsigned long, prot, unsigned long, flags, > > > + unsigned long, fd, unsigned long, offset) > > > +{ > > > + if ((prot & PROT_WRITE) && (prot & PROT_EXEC)) > > > + if (unlikely(!(prot & PROT_READ))) > > > + return -EINVAL; > > > + > > > + return ksys_mmap_pgoff(addr, len, prot, flags, fd, offset); > > > +} > > > > This is one that we may have to deal with separately, introducing > > sys_mmap_pgoff() was a mistake in my opinion, and we should just have > > #if __BITS_PER_LONG == 32 || defined(__SYSCALL_COMPAT) > #define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _32) > #else > #define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _64) > #endif > > #define __NR3264_mmap 222 > __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap) > > > added a sys_mmap2() for all architectures that don't explicitly override it. > That should be another patch, right? Let's keep it here. Right, I think the patch would be a nice cleanup, but it appears that riscv is among the few architectures that have defined their own nonstandard mmap2() syscall after all, despite using the standard name for the entry point. Not sure how this slipped past my original review, but it certainly can't be changed now. It also means that you need to change your implementation of compat_sys_mmap2() to match the version from arch/riscv/kernel/sys_riscv.c, rather than the version that everyone else has. Maybe leave it there and change the #ifdef to build mmap2 for both native rv32 and compat mode. FWIW, this is what a conversion from mmap_pgoff() to mmap2() would look like, I think this is an overall win, but it's entirely unrelated to your series now: https://pastebin.com/QtF55dn4 (I'm sure I got some details wrong, at least it needs some #ifdef checks). Arnd
On Wed, Dec 22, 2021 at 2:21 PM Arnd Bergmann <arnd@arndb.de> wrote: > On Wed, Dec 22, 2021 at 1:43 PM Guo Ren <guoren@kernel.org> wrote: > > Right, I think the patch would be a nice cleanup, but it appears that > riscv is among the few architectures that have defined their own > nonstandard mmap2() syscall after all, despite using the standard > name for the entry point. Not sure how this slipped past my original > review, but it certainly can't be changed now. No, I misread, the calling conventions are fine after all, it's just written in a rather odd way. > Maybe leave it there and change the #ifdef to build mmap2 for both > native rv32 and compat mode. This bit still applies though, I don't think you need to add another helper, just use the one that is already there. Arnd
On Wed, Dec 22, 2021 at 10:00 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Wed, Dec 22, 2021 at 2:21 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Wed, Dec 22, 2021 at 1:43 PM Guo Ren <guoren@kernel.org> wrote: > > > > Right, I think the patch would be a nice cleanup, but it appears that > > riscv is among the few architectures that have defined their own > > nonstandard mmap2() syscall after all, despite using the standard > > name for the entry point. Not sure how this slipped past my original > > review, but it certainly can't be changed now. > > No, I misread, the calling conventions are fine after all, it's > just written in a rather odd way. > > > Maybe leave it there and change the #ifdef to build mmap2 for both > > native rv32 and compat mode. > > This bit still applies though, I don't think you need to add another > helper, just use the one that is already there. Yes, I will: diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c index 12f8a7fce78b..9c0194f176fc 100644 --- a/arch/riscv/kernel/sys_riscv.c +++ b/arch/riscv/kernel/sys_riscv.c @@ -33,7 +33,9 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len, { return riscv_sys_mmap(addr, len, prot, flags, fd, offset, 0); } -#else +#endif + +#if defined(CONFIG_32BIT) || defined(CONFIG_COMPAT) SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, unsigned long, prot, unsigned long, flags, unsigned long, fd, off_t, offset) @@ -44,7 +46,7 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, */ return riscv_sys_mmap(addr, len, prot, flags, fd, offset, 12); } -#endif /* !CONFIG_64BIT */ +#endif > > Arnd
diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h index 7ac6a0e275f2..4ff98a22ef24 100644 --- a/arch/riscv/include/asm/syscall.h +++ b/arch/riscv/include/asm/syscall.h @@ -16,6 +16,9 @@ /* The array of function pointers for syscalls. */ extern void * const sys_call_table[]; +#ifdef CONFIG_COMPAT +extern void * const compat_sys_call_table[]; +#endif /* * Only the low 32 bits of orig_r0 are meaningful, so we return int. diff --git a/arch/riscv/kernel/compat_syscall_table.c b/arch/riscv/kernel/compat_syscall_table.c new file mode 100644 index 000000000000..9b81fb9a8683 --- /dev/null +++ b/arch/riscv/kernel/compat_syscall_table.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#define __SYSCALL_COMPAT +#undef __LP64__ + +#include <linux/compat.h> +#include <linux/syscalls.h> +#include <asm-generic/mman-common.h> +#include <asm-generic/syscalls.h> +#include <asm/syscall.h> + +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, + unsigned long, prot, unsigned long, flags, + unsigned long, fd, unsigned long, offset) +{ + if ((prot & PROT_WRITE) && (prot & PROT_EXEC)) + if (unlikely(!(prot & PROT_READ))) + return -EINVAL; + + return ksys_mmap_pgoff(addr, len, prot, flags, fd, offset); +} + +#define arg_u32p(name) u32, name##_lo, u32, name##_hi + +#define arg_u64(name) (((u64)name##_hi << 32) | \ + ((u64)name##_lo & 0xffffffff)) + +COMPAT_SYSCALL_DEFINE3(truncate64, const char __user *, pathname, + arg_u32p(length)) +{ + return ksys_truncate(pathname, arg_u64(length)); +} + +COMPAT_SYSCALL_DEFINE3(ftruncate64, unsigned int, fd, arg_u32p(length)) +{ + return ksys_ftruncate(fd, arg_u64(length)); +} + +COMPAT_SYSCALL_DEFINE6(fallocate, int, fd, int, mode, + arg_u32p(offset), arg_u32p(len)) +{ + return ksys_fallocate(fd, mode, arg_u64(offset), arg_u64(len)); +} + +COMPAT_SYSCALL_DEFINE5(pread64, unsigned int, fd, char __user *, buf, + size_t, count, arg_u32p(pos)) +{ + return ksys_pread64(fd, buf, count, arg_u64(pos)); +} + +COMPAT_SYSCALL_DEFINE5(pwrite64, unsigned int, fd, + const char __user *, buf, size_t, count, arg_u32p(pos)) +{ + return ksys_pwrite64(fd, buf, count, arg_u64(pos)); +} + +COMPAT_SYSCALL_DEFINE6(sync_file_range, int, fd, arg_u32p(offset), + arg_u32p(nbytes), unsigned int, flags) +{ + return ksys_sync_file_range(fd, arg_u64(offset), arg_u64(nbytes), + flags); +} + +COMPAT_SYSCALL_DEFINE4(readahead, int, fd, arg_u32p(offset), + size_t, count) +{ + return ksys_readahead(fd, arg_u64(offset), count); +} + +COMPAT_SYSCALL_DEFINE6(fadvise64_64, int, fd, int, advice, arg_u32p(offset), + arg_u32p(len)) +{ + return ksys_fadvise64_64(fd, arg_u64(offset), arg_u64(len), advice); +} + +#undef __SYSCALL +#define __SYSCALL(nr, call) [nr] = (call), + +asmlinkage long compat_sys_rt_sigreturn(void); + +void * const compat_sys_call_table[__NR_syscalls] = { + [0 ... __NR_syscalls - 1] = sys_ni_syscall, +#include <asm/unistd.h> +};