diff mbox series

[09/13] tools/nolibc: sys_poll: riscv: use __NR_ppoll_time64 for rv32

Message ID ec5af2ae25264eddce4b50380bfd24f9490eca75.1684949268.git.falcon@tinylab.org
State New
Headers show
Series [01/13] Revert "tools/nolibc: riscv: Support __NR_llseek for rv32" | expand

Commit Message

Zhangjin Wu May 24, 2023, 5:57 p.m. UTC
rv32 uses the generic include/uapi/asm-generic/unistd.h and it has no
__NR_ppoll after kernel commit d4c08b9776b3 ("riscv: Use latest system
call ABI"), use __NR_ppoll_time64 instead.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/include/nolibc/std.h   | 1 +
 tools/include/nolibc/sys.h   | 7 ++++++-
 tools/include/nolibc/types.h | 6 ++++++
 3 files changed, 13 insertions(+), 1 deletion(-)

Comments

Arnd Bergmann May 26, 2023, 9:34 a.m. UTC | #1
On Fri, May 26, 2023, at 09:15, Thomas Weißschuh wrote:
> On 2023-05-25 01:57:24+0800, Zhangjin Wu wrote:
>> rv32 uses the generic include/uapi/asm-generic/unistd.h and it has no

>>  
>> +/* needed by time64 syscalls */
>> +struct timespec64 {
>> +	time64_t	tv_sec;		/* seconds */
>> +	long		tv_nsec;	/* nanoseconds */
>> +};
>
> A question to you and Willy, as it's also done the same for other types:
>
> What is the advantage of custom definitions over using the one from the
> kernel (maybe via a typedef).
>
> From linux/time_types.h:
>
> struct __kernel_timespec {
> 	__kernel_time64_t tv_set;
> 	long long tv_nsec;
> };

I agree the __kernel_* types are what we should be using when
interacting with system calls directly, that is definitely what
they are intended for.

I would go further here and completely drop support for 32-bit
time_t/off_t and derived types in nolibc. Unfortunately, the
kernel's include/uapi/linux/time.h header still defines the
old types, this is one of the last remnants the time32 syscalls
definitions in the kernel headers, and this already conflicts
with the glibc and musl definitions, so anything that includes
this header is broken on real systems. I think it makes most
sense for nolibc to just use the linux/time_types.h header
instead and use something like

#define timespec   __kernel_timespec
#define itimerspec __kernel_itimerspec
typedef __kernel_time64_t time_t;
/* timeval is only provided for users, not compatible with syscalls */
struct timeval { __kernel_time64_t tv_sec; __s64 tv_nsec; };

so we can drop all the fallbacks for old 32-bit targets. This
also allows running with CONFIG_COMPAT_32BIT_TIME disabled.

     Arnd
Willy Tarreau May 28, 2023, 10:29 a.m. UTC | #2
Hi Zhangjin,

On Sun, May 28, 2023 at 04:25:09PM +0800, Zhangjin Wu wrote:
> Just a status update ...
> 
> I'm working on the pure time64 and 64bit off_t syscalls support, it almost
> worked (tested on rv32/64, arm32/64), thanks very much for your suggestions.
> 
> It includes:
> 
> * Based on linux/types.h and
>     * Use 64bit off_t
>     * Use 64bit time_t
>     * the new std.h looks like this
> 
>     typedef uint32_t __kernel_dev_t;
>     
>     typedef __kernel_dev_t          dev_t;
>     typedef __kernel_ulong_t        ino_t;
>     typedef __kernel_mode_t         mode_t;
>     typedef __kernel_pid_t          pid_t;
>     typedef __kernel_uid32_t        uid_t;
>     typedef __kernel_gid32_t        gid_t;
>     typedef __kernel_loff_t         off_t;
>     typedef __kernel_time64_t       time_t;
>     typedef uint32_t                nlink_t;
>     typedef uint64_t                blksize_t;
>     typedef uint64_t                blkcnt_t;
> 
> * Use __kernel_timespec as timespec
> * Use 64bit time_t based struct timeval
>     * Disable gettimeofday syscall completely for 32bit platforms
>         * And disable the gettimeofday_bad1/2 test case too

When you say "disable", you mean "remap", right ? Or do you mean
"break in 2023 code that was expected to break only in 2038 after
the hardware supporting it no longer exists" ?

Thanks,
Willy
Arnd Bergmann May 28, 2023, 10:55 a.m. UTC | #3
On Sun, May 28, 2023, at 12:29, Willy Tarreau wrote:
> On Sun, May 28, 2023 at 04:25:09PM +0800, Zhangjin Wu wrote:
>> 
>> * Use __kernel_timespec as timespec
>> * Use 64bit time_t based struct timeval
>>     * Disable gettimeofday syscall completely for 32bit platforms
>>         * And disable the gettimeofday_bad1/2 test case too
>
> When you say "disable", you mean "remap", right ? Or do you mean
> "break in 2023 code that was expected to break only in 2038 after

clock_gettime() has been supported for a very long time, so both
time() and gettimeofday() can be trivial wrappers around that.

Nothing really should be using the timezone argument, so I'd
just ignore that in nolibc. (it's a little trickier for /sbin/init
setting the initial timezone, but I hope we can ignore that here).

clock_gettime() as a function call that takes a timespec argument
in turn should be a wrapper around either sys_clock_gettime64 (on
32-bit architectures) or sys_clock_gettime_old() (on 64-bit
architectures, or as a fallback on old 32-bit kernels after
clock_gettime64 fails).

On normal libc implementations, the low-level
sys_clock_gettime64() and sys_clock_gettime_old(), whatever
they are named, would call vdso first and then fall back
to the syscall, but I don't think that's necessary for nolibc.

I'd define them the same as the kernel, with
sys_clock_gettime64() taking a __kernel_timespec, and
sys_clock_gettime_old() takeing a __kernel_old_timespec.

    Arnd
diff mbox series

Patch

diff --git a/tools/include/nolibc/std.h b/tools/include/nolibc/std.h
index 83c0b0cb9564..221385c0e823 100644
--- a/tools/include/nolibc/std.h
+++ b/tools/include/nolibc/std.h
@@ -32,6 +32,7 @@  typedef   signed long         off_t;
 typedef   signed long     blksize_t;
 typedef   signed long      blkcnt_t;
 typedef   signed long        time_t;
+typedef     long long       time64_t;
 typedef     long long        loff_t;
 
 #endif /* _NOLIBC_STD_H */
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 0ff77c0a06d7..08d38175bd7b 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -923,8 +923,13 @@  int pivot_root(const char *new, const char *old)
 static __attribute__((unused))
 int sys_poll(struct pollfd *fds, int nfds, int timeout)
 {
-#if defined(__NR_ppoll)
+#if defined(__NR_ppoll) || defined(__NR_ppoll_time64)
+#ifdef __NR_ppoll
 	struct timespec t;
+#else
+	struct timespec64 t;
+#define __NR_ppoll __NR_ppoll_time64
+#endif
 
 	if (timeout >= 0) {
 		t.tv_sec  = timeout / 1000;
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 15b0baffd336..ee914391439c 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -203,6 +203,12 @@  struct stat {
 	time_t    st_ctime;   /* time of last status change */
 };
 
+/* needed by time64 syscalls */
+struct timespec64 {
+	time64_t	tv_sec;		/* seconds */
+	long		tv_nsec;	/* nanoseconds */
+};
+
 /* WARNING, it only deals with the 4096 first majors and 256 first minors */
 #define makedev(major, minor) ((dev_t)((((major) & 0xfff) << 8) | ((minor) & 0xff)))
 #define major(dev) ((unsigned int)(((dev) >> 8) & 0xfff))