| Message ID | b306ae7b53d67acad6d1e2f63d43505d79f81241.1684949267.git.falcon@tinylab.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/nolibc: riscv: Add full rv32 support | expand |
On 2023-05-25 01:48:11+0800, Zhangjin Wu wrote: > When compile nolibc-test.c for rv32, we got such error: > > tools/testing/selftests/nolibc/nolibc-test.c:599:57: error: ‘__NR_fstat’ undeclared (first use in this function) > 599 | CASE_TEST(syscall_args); EXPECT_SYSER(1, syscall(__NR_fstat, 0, NULL), -1, EFAULT); break; > > The generic include/uapi/asm-generic/unistd.h used by rv32 doesn't > support __NR_fstat, use __NR_statx instead: > > Running test 'syscall' > 69 syscall_noargs = 1 [OK] > 70 syscall_args = -1 EFAULT [OK] > > As tools/include/nolibc/sys.h shows, __NR_statx is either not supported > by all platforms, so, both __NR_fstat and __NR_statx are required. > > Btw, the latest riscv libc6-dev package is required, otherwise, we would > also get such error: > > In file included from /usr/riscv64-linux-gnu/include/sys/cdefs.h:452, > from /usr/riscv64-linux-gnu/include/features.h:461, > from /usr/riscv64-linux-gnu/include/bits/libc-header-start.h:33, > from /usr/riscv64-linux-gnu/include/limits.h:26, > from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/limits.h:194, > from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/syslimits.h:7, > from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/limits.h:34, > from /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/nolibc-test.c:6: > /usr/riscv64-linux-gnu/include/bits/wordsize.h:28:3: error: #error "rv32i-based targets are not supported" > 28 | # error "rv32i-based targets are not supported" > > The glibc commit 5b6113d62efa ("RISC-V: Support the 32-bit ABI > implementation") fixed up above error, so, glibc >= 2.33 (who includes > this commit) is required. It seems weird to require limits.h from the system libc at all. The only thing used from there are INT_MAX and INT_MIN. Instead we could define our own versions of INT_MAX and INT_MIN in stdint.h. #ifndef INT_MAX #define INT_MAX __INT_MAX__ #endif #ifndef INT_MIN #define INT_MIN (- __INT_MAX__ - 1) #endif Thomas
Hi, Thomas > On 2023-05-25 01:48:11+0800, Zhangjin Wu wrote: > > When compile nolibc-test.c for rv32, we got such error: > > > > tools/testing/selftests/nolibc/nolibc-test.c:599:57: error: ‘__NR_fstat’ undeclared (first use in this function) > > 599 | CASE_TEST(syscall_args); EXPECT_SYSER(1, syscall(__NR_fstat, 0, NULL), -1, EFAULT); break; > > > > The generic include/uapi/asm-generic/unistd.h used by rv32 doesn't > > support __NR_fstat, use __NR_statx instead: > > > > Running test 'syscall' > > 69 syscall_noargs = 1 [OK] > > 70 syscall_args = -1 EFAULT [OK] > > > > As tools/include/nolibc/sys.h shows, __NR_statx is either not supported > > by all platforms, so, both __NR_fstat and __NR_statx are required. > > > > Btw, the latest riscv libc6-dev package is required, otherwise, we would > > also get such error: > > > > In file included from /usr/riscv64-linux-gnu/include/sys/cdefs.h:452, > > from /usr/riscv64-linux-gnu/include/features.h:461, > > from /usr/riscv64-linux-gnu/include/bits/libc-header-start.h:33, > > from /usr/riscv64-linux-gnu/include/limits.h:26, > > from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/limits.h:194, > > from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/syslimits.h:7, > > from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/limits.h:34, > > from /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/nolibc-test.c:6: > > /usr/riscv64-linux-gnu/include/bits/wordsize.h:28:3: error: #error "rv32i-based targets are not supported" > > 28 | # error "rv32i-based targets are not supported" > > > > The glibc commit 5b6113d62efa ("RISC-V: Support the 32-bit ABI > > implementation") fixed up above error, so, glibc >= 2.33 (who includes > > this commit) is required. > > It seems weird to require limits.h from the system libc at all. > > The only thing used from there are INT_MAX and INT_MIN. > Instead we could define our own versions of INT_MAX and INT_MIN in > stdint.h. > > #ifndef INT_MAX > #define INT_MAX __INT_MAX__ > #endif > > #ifndef INT_MIN > #define INT_MIN (- __INT_MAX__ - 1) > #endif > Just verified and prepared a patch, it did work perfectly, thanks. The above commit message exactly the error info will be cleaned up in v2. Best regards, Zhangjin > Thomas
On Wed, May 24, 2023, at 19:48, Zhangjin Wu wrote: > > +static int test_syscall_args(void) > +{ > +#ifdef __NR_fstat > + return syscall(__NR_fstat, 0, NULL); > +#elif defined(__NR_statx) > + return syscall(__NR_statx, 0, NULL, 0, 0, NULL); > +#else > +#error Neither __NR_fstat nor __NR_statx defined, cannot implement > syscall_args test > +#endif > +} Does this need to work on old kernels? My impression was that this is only intended to be used with the kernel that ships the copy, so you can just rely on statx to be defined and drop the old fallbacks (same as for pselect6_time64 as I commented). Arnd
Hi Arnd, On Fri, May 26, 2023 at 11:21:02AM +0200, Arnd Bergmann wrote: > On Wed, May 24, 2023, at 19:48, Zhangjin Wu wrote: > > > > > +static int test_syscall_args(void) > > +{ > > +#ifdef __NR_fstat > > + return syscall(__NR_fstat, 0, NULL); > > +#elif defined(__NR_statx) > > + return syscall(__NR_statx, 0, NULL, 0, 0, NULL); > > +#else > > +#error Neither __NR_fstat nor __NR_statx defined, cannot implement > > syscall_args test > > +#endif > > +} > > Does this need to work on old kernels? My impression was that > this is only intended to be used with the kernel that ships the > copy, so you can just rely on statx to be defined and drop > the old fallbacks (same as for pselect6_time64 as I commented). Yes, as much as possible this is desirable because the purpose is clearly to simplify the build of applications. The reason is that some applications might want to run as well on older kernels, but may miss some syscalls on the nolibc shipped with these older ones. Since the project is quite young, it lags a lot behind what each kernel supports, so it's expected that users will take the most recent nolibc version to benefit from support of syscalls that were already present in older ones. The alternative would be to take the project out of the kernel as it was before but this would likely complicate contributions. However the selftest code is clearly specific to a kernel IMHO, since its goal is to be used to check that the shipped version of nolibc works on this kernel. As such, my view on this is that as long as it doesn't require unreasonable efforts to support older kernels for the userland code, we should try. If sometimes it's a big pain, we should not insist too much, but at least making sure that only the feature in question will not work would be desirable. Thanks, Willy
Hi, Arnd > On Wed, May 24, 2023, at 19:48, Zhangjin Wu wrote: > > > > > +static int test_syscall_args(void) > > +{ > > +#ifdef __NR_fstat > > + return syscall(__NR_fstat, 0, NULL); > > +#elif defined(__NR_statx) > > + return syscall(__NR_statx, 0, NULL, 0, 0, NULL); > > +#else > > +#error Neither __NR_fstat nor __NR_statx defined, cannot implement > > syscall_args test > > +#endif > > +} > > Does this need to work on old kernels? My impression was that > this is only intended to be used with the kernel that ships the > copy, so you can just rely on statx to be defined and drop Willy suggested this before, besides the generic unistd.h users, I only found one __NR_statx in arm64 and forgot the ones in the *.tbl files: $ grep -n statx -ur arch/ | cut -d ':' -f1,2 | tr ':' ' ' | awk '{printf("git blame -L %s,%s %s\n",$2,$2,$1);}' | bash cabcebd33b8b8 (Firoz Khan 2018-11-13 15:01:52 +0530 453) 522 common statx sys_statx a1016e94cce9f (Russell King 2017-03-09 17:14:32 +0000 414) 397 common statx sys_statx 7349ee3a97edb (Arnd Bergmann 2018-12-30 22:25:07 +0100 338) 326 common statx sys_statx fd81414666cf6 (Firoz Khan 2018-11-13 11:30:28 +0530 389) 379 common statx sys_statx 9bcbf97c62931 (Firoz Khan 2018-12-13 14:37:38 +0530 341) 330 n32 statx sys_statx 9bcbf97c62931 (Firoz Khan 2018-12-13 14:37:38 +0530 337) 326 n64 statx sys_statx 9bcbf97c62931 (Firoz Khan 2018-12-13 14:37:38 +0530 380) 366 o32 statx sys_statx 85e69701f58c9 (Firoz Khan 2018-09-09 07:22:50 +0530 395) 349 common statx sys_statx 90856087daca9 (Arnd Bergmann 2019-01-16 14:15:23 +0100 389) 379 common statx sys_statx sys_statx d25a122afd437 (Arnd Bergmann 2018-12-30 23:04:30 +0100 393) 383 common statx sys_statx 6ff645dd683af (Firoz Khan 2018-11-14 10:56:30 +0530 433) 360 common statx sys_statx fc06bac35c8c7 (Firoz Khan 2018-11-13 11:34:33 +0530 408) 398 common statx sys_statx aff8503932004 (Firoz Khan 2018-12-17 16:10:34 +0530 474) 383 nospu statx sys_statx a845a6cf1dad1 (Brian Gerst 2020-03-13 15:51:39 -0400 397) 383 i386 statx sys_statx cab56d3484d4b (Brian Gerst 2020-03-13 15:51:38 -0400 343) 332 common statx sys_statx c7914ef69dbb3 (Firoz Khan 2018-11-13 15:49:29 +0530 374) 351 common statx sys_statx 713cc9df6473f (Will Deacon 2017-03-21 18:04:26 +0000 807) #define __NR_statx 397 713cc9df6473f (Will Deacon 2017-03-21 18:04:26 +0000 808) __SYSCALL(__NR_statx, sys_statx) (2020: x86 changed the format of the *.tbl) (2019: s390 changed the format of the *.tbl) $ grep -n asm-generic/unistd.h -ur arch/ | cut -d ':' -f1,2 | tr ':' ' ' | awk '{printf("git blame -L %s,%s %s\n",$2,$2,$1);}' | bash 4adeefe161a74 arch/arc/include/asm/unistd.h (Vineet Gupta 2013-01-18 15:12:18 +0530 31) #include <asm-generic/unistd.h> 91e040a79df73 (Vineet Gupta 2016-10-20 07:39:45 -0700 35) /* Generic syscall (fs/filesystems.c - lost in asm-generic/unistd.h */ 4262a727621ce (David Howells 2012-10-11 11:05:13 +0100 25) #include <asm-generic/unistd.h> 4859bfca11c7d (Guo Ren 2018-09-05 14:25:08 +0800 9) #include <asm-generic/unistd.h> b9398a84590be arch/hexagon/include/asm/unistd.h (Richard Kuo 2011-10-31 18:35:16 -0500 40) #include <asm-generic/unistd.h> 1000197d80132 (Ley Foon Tan 2014-11-06 15:19:57 +0800 27) #include <asm-generic/unistd.h> 09abb90107202 arch/openrisc/include/asm/unistd.h (Jonas Bonn 2011-06-04 22:26:51 +0300 30) #include <asm-generic/unistd.h> 27f8899d6002e (David Abdurachmanov 2018-11-08 20:02:39 +0100 26) #include <asm-generic/unistd.h> be769645a2aef (Huacai Chen 2022-05-31 18:04:11 +0800 5) #include <asm-generic/unistd.h> (2022: the year loongarch added) $ grep -n statx, -ur fs/stat.c 677:SYSCALL_DEFINE5(statx, $ git blame -L 677,677 fs/stat.c a528d35e8bfcc (David Howells 2017-01-31 16:46:22 +0000 677) SYSCALL_DEFINE5(statx, So, statx has been added from v4.10 (2017, a528d35e8bfcc) and the last arch support is at least from v4.20 (2018, aff8503932004), perhaps it is safe enough to only reserve the statx now, will simply replace fstat with statx in the coming new revision of this patch, thanks very much. > the old fallbacks (same as for pselect6_time64 as I commented). > Did the same search for this: $ grep -n pselect6_time64 -ur arch/ | cut -d ':' -f1,2 | tr ':' ' ' | awk '{printf("git blame -L %s,+1 %s\n",$2,$1);}' | bash 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 430) 413 common pselect6_time64 sys_pselect6 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 416) 413 common pselect6_time64 sys_pselect6 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 355) 413 n32 pselect6_time64 compat_sys_pselect6_time64 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 404) 413 o32 pselect6_time64 sys_pselect6 compat_sys_pselect6_time64 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 414) 413 32 pselect6_time64 sys_pselect6 compat_sys_pselect6_time64 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 419) 413 32 pselect6_time64 - compat_sys_pselect6_time64 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 419) 413 common pselect6_time64 sys_pselect6 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 462) 413 32 pselect6_time64 sys_pselect6 compat_sys_pselect6_time64 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 422) 413 common pselect6_time64 sys_pselect6 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 503) 413 32 pselect6_time64 sys_pselect6 compat_sys_pselect6_time64 cab56d3484d4b (Brian Gerst 2020-03-13 15:51:38 -0400 421) 413 i386 pselect6_time64 sys_pselect6 compat_sys_pselect6_time64 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 387) 413 common pselect6_time64 sys_pselect6 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 838) #define __NR_pselect6_time64 413 48166e6ea47d2 (Arnd Bergmann 2019-01-10 12:45:11 +0100 839) __SYSCALL(__NR_pselect6_time64, compat_sys_pselect6_time64) (not sure if we have missed some archs) $ grep -n pselect6_time64, fs/select.c 1368:COMPAT_SYSCALL_DEFINE6(pselect6_time64, int, n, compat_ulong_t __user *, inp, $ git blame -L 1368,+1 fs/select.c e024707bccae1 (Deepa Dinamani 2018-09-19 21:41:07 -0700 1368) COMPAT_SYSCALL_DEFINE6(pselect6_time64, int, n, compat_ulong_t __user *, inp, pselect6_time64 has been added from v4.20 and the last arch support is at least from v5.0.0. As we discussed in another reply, will add pselect6_time64 at first and reserve the drop patch of the already added oldselect, newselect in the future. Thanks very much, Zhangjin > > Arnd
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 227ef2a3ebba..c86ff6018c7f 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -501,6 +501,17 @@ static int test_fork(void) } } +static int test_syscall_args(void) +{ +#ifdef __NR_fstat + return syscall(__NR_fstat, 0, NULL); +#elif defined(__NR_statx) + return syscall(__NR_statx, 0, NULL, 0, 0, NULL); +#else +#error Neither __NR_fstat nor __NR_statx defined, cannot implement syscall_args test +#endif +} + /* Run syscall tests between IDs <min> and <max>. * Return 0 on success, non-zero on failure. */ @@ -597,7 +608,7 @@ int run_syscall(int min, int max) CASE_TEST(write_badf); EXPECT_SYSER(1, write(-1, &tmp, 1), -1, EBADF); break; CASE_TEST(write_zero); EXPECT_SYSZR(1, write(1, &tmp, 0)); break; CASE_TEST(syscall_noargs); EXPECT_SYSEQ(1, syscall(__NR_getpid), getpid()); break; - CASE_TEST(syscall_args); EXPECT_SYSER(1, syscall(__NR_fstat, 0, NULL), -1, EFAULT); break; + CASE_TEST(syscall_args); EXPECT_SYSER(1, test_syscall_args(), -1, EFAULT); break; case __LINE__: return ret; /* must be last */ /* note: do not set any defaults so as to permit holes above */
When compile nolibc-test.c for rv32, we got such error: tools/testing/selftests/nolibc/nolibc-test.c:599:57: error: ‘__NR_fstat’ undeclared (first use in this function) 599 | CASE_TEST(syscall_args); EXPECT_SYSER(1, syscall(__NR_fstat, 0, NULL), -1, EFAULT); break; The generic include/uapi/asm-generic/unistd.h used by rv32 doesn't support __NR_fstat, use __NR_statx instead: Running test 'syscall' 69 syscall_noargs = 1 [OK] 70 syscall_args = -1 EFAULT [OK] As tools/include/nolibc/sys.h shows, __NR_statx is either not supported by all platforms, so, both __NR_fstat and __NR_statx are required. Btw, the latest riscv libc6-dev package is required, otherwise, we would also get such error: In file included from /usr/riscv64-linux-gnu/include/sys/cdefs.h:452, from /usr/riscv64-linux-gnu/include/features.h:461, from /usr/riscv64-linux-gnu/include/bits/libc-header-start.h:33, from /usr/riscv64-linux-gnu/include/limits.h:26, from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/limits.h:194, from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/syslimits.h:7, from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/limits.h:34, from /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/nolibc-test.c:6: /usr/riscv64-linux-gnu/include/bits/wordsize.h:28:3: error: #error "rv32i-based targets are not supported" 28 | # error "rv32i-based targets are not supported" The glibc commit 5b6113d62efa ("RISC-V: Support the 32-bit ABI implementation") fixed up above error, so, glibc >= 2.33 (who includes this commit) is required. Signed-off-by: Zhangjin Wu <falcon@tinylab.org> --- tools/testing/selftests/nolibc/nolibc-test.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)