On 30.10.20 09:50, chensong wrote: > > > On 2020年10月28日 00:33, Jan Kiszka wrote: >> On 29.09.20 09:19, chensong wrote: >>> Upstream has used timespec64 to replace timespec inside kernel and >>> used __kernel_timespec to replace timespect in syscalls, therefore >>> we must keep aligned with upstream. >>> >>> clock_settime is a point to get started at, it involves 2 parts: >>> 1, syscall in ./include/trace/events/cobalt-posix.h for 64bits >>> 2, compat syscall in kernel/cobalt/posix/syscall32.c for 32bits >>> 3, define __kernel_timespec in wrapper.h for compatibility >>> >>> some new functions are implemented to keep the compatibility with >>> other syscalls which haven't been handled yet and will be removed >>> in the end of the work. >>> >>> Signed-off-by: chensong <[email protected]> >>> >>> CC: Jan Kiszka <[email protected]> >>> CC: Henning Schild <[email protected]> >>> --- >>> include/cobalt/kernel/clock.h | 6 ++--- >>> include/cobalt/kernel/compat.h | 3 +++ >>> .../cobalt/include/asm-generic/xenomai/wrappers.h | 9 +++++++ >>> kernel/cobalt/posix/clock.c | 28 >>> +++++++++++++++++----- >>> kernel/cobalt/posix/clock.h | 15 ++++++++++-- >>> kernel/cobalt/posix/compat.c | 11 +++++++++ >>> kernel/cobalt/posix/syscall32.c | 6 ++--- >>> kernel/cobalt/trace/cobalt-posix.h | 24 >>> +++++++++++++++++-- >>> 8 files changed, 86 insertions(+), 16 deletions(-) >>> >>> diff --git a/include/cobalt/kernel/clock.h >>> b/include/cobalt/kernel/clock.h >>> index c26776a..cf9d5ec 100644 >>> --- a/include/cobalt/kernel/clock.h >>> +++ b/include/cobalt/kernel/clock.h >>> @@ -52,7 +52,7 @@ struct xnclock { >>> xnticks_t (*read_raw)(struct xnclock *clock); >>> xnticks_t (*read_monotonic)(struct xnclock *clock); >>> int (*set_time)(struct xnclock *clock, >>> - const struct timespec *ts); >>> + const struct timespec64 *ts); >>> xnsticks_t (*ns_to_ticks)(struct xnclock *clock, >>> xnsticks_t ns); >>> xnsticks_t (*ticks_to_ns)(struct xnclock *clock, >>> @@ -213,7 +213,7 @@ static inline xnticks_t >>> xnclock_read_monotonic(struct xnclock *clock) >>> } >>> >>> static inline int xnclock_set_time(struct xnclock *clock, >>> - const struct timespec *ts) >>> + const struct timespec64 *ts) >>> { >>> if (likely(clock == &nkclock)) >>> return -EINVAL; >>> @@ -266,7 +266,7 @@ static inline xnticks_t >>> xnclock_read_monotonic(struct xnclock *clock) >>> } >>> >>> static inline int xnclock_set_time(struct xnclock *clock, >>> - const struct timespec *ts) >>> + const struct timespec64 *ts) >>> { >>> /* >>> * There is no way to change the core clock's idea of time. >>> diff --git a/include/cobalt/kernel/compat.h >>> b/include/cobalt/kernel/compat.h >>> index 05754cb..70e2c03 100644 >>> --- a/include/cobalt/kernel/compat.h >>> +++ b/include/cobalt/kernel/compat.h >>> @@ -89,6 +89,9 @@ struct compat_rtdm_mmap_request { >>> int sys32_get_timespec(struct timespec *ts, >>> const struct compat_timespec __user *cts); >>> >>> +int sys64_get_timespec(struct timespec64 *ts, >>> + const struct compat_timespec __user *cts); >>> + >>> int sys32_put_timespec(struct compat_timespec __user *cts, >>> const struct timespec *ts); >>> >>> diff --git a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h >>> b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h >>> index 23993dc..e5375de 100644 >>> --- a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h >>> +++ b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h >>> @@ -156,4 +156,13 @@ devm_hwmon_device_register_with_groups(struct >>> device *dev, const char *name, >>> #error "Xenomai/cobalt requires Linux kernel 3.10 or above" >>> #endif /* < 3.10 */ >>> >>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0) >>> +typedef long long __kernel_time64_t; >>> +struct __kernel_timespec { >>> + __kernel_time64_t tv_sec; /* seconds */ >>> + long long tv_nsec; /* nanoseconds */ >>> +}; >>> +#endif /* < 4.18 */ >>> + >>> + >>> #endif /* _COBALT_ASM_GENERIC_WRAPPERS_H */ >>> diff --git a/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c >>> index 561358e..c960d7b 100644 >>> --- a/kernel/cobalt/posix/clock.c >>> +++ b/kernel/cobalt/posix/clock.c >>> @@ -194,7 +194,7 @@ COBALT_SYSCALL(clock_gettime, current, >>> return 0; >>> } >>> >>> -int __cobalt_clock_settime(clockid_t clock_id, const struct timespec >>> *ts) >>> +int __cobalt_clock_settime(clockid_t clock_id, const struct >>> timespec64 *ts) >>> { >>> int _ret, ret = 0; >>> xnticks_t now; >>> @@ -207,7 +207,7 @@ int __cobalt_clock_settime(clockid_t clock_id, >>> const struct timespec *ts) >>> case CLOCK_REALTIME: >>> xnlock_get_irqsave(&nklock, s); >>> now = xnclock_read_realtime(&nkclock); >>> - xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns(ts) - now)); >>> + xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns64(ts) - now)); >>> xnlock_put_irqrestore(&nklock, s); >>> break; >>> default: >>> @@ -242,15 +242,31 @@ int __cobalt_clock_adjtime(clockid_t clock_id, >>> struct timex *tx) >>> return 0; >>> } >>> >>> +static int __cobalt_get_timespec64(struct timespec64 *ts, >>> + const struct __kernel_timespec __user *uts) >>> +{ >>> + struct __kernel_timespec kts; >>> + int ret; >>> + >>> + ret = cobalt_copy_from_user(&kts, uts, sizeof(kts)); >>> + if (ret) >>> + return -EFAULT; >>> + >>> + ts->tv_sec = kts.tv_sec; >>> + ts->tv_nsec = kts.tv_nsec; >>> + >>> + return 0; >>> + >>> +} >>> COBALT_SYSCALL(clock_settime, current, >>> - (clockid_t clock_id, const struct timespec __user *u_ts)) >>> + (clockid_t clock_id, const struct __kernel_timespec __user *u_ts)) >> >> This is still a point where I see ABI breakage: On a 32-bit system, >> struct timespec was using a 32-bit tv_sec. Now with struct >> _kernel_timespec, this will become a 64-bit value. That is what we want >> for new applications being built after this patch. However, existing >> application - and that includes existing Xenomai libs built before this >> change (stable kernel ABI...) will continue to deliver 32-bit tv_sec >> here and break over this syscall change. >> >> To model such a change in an ABI-preserving way, we need some >> "clock_settime64" syscall that new userspace will pick by default, and >> an unmodified (those 2038-wise broken) clock_settime. Same for all other >> syscalls and drivers taking time_t directly or indirectly. >> >> Jan > > how about this way: > > we copy the idea from musl's clock_settime(./src/time/clock_settime.c) > which is: > > #define IS32BIT(x) !((x)+0x80000000ULL>>32) > > int clock_settime(clockid_t clk, const struct timespec *ts) > { > #ifdef SYS_clock_settime64 > time_t s = ts->tv_sec; > long ns = ts->tv_nsec; > int r = -ENOSYS; > if (SYS_clock_settime == SYS_clock_settime64 || !IS32BIT(s)) > r = __syscall(SYS_clock_settime64, clk, > ((long long[]){s, ns})); > if (SYS_clock_settime == SYS_clock_settime64 || r!=-ENOSYS) > return __syscall_ret(r); > if (!IS32BIT(s)) > return __syscall_ret(-ENOTSUP); > return syscall(SYS_clock_settime, clk, ((long[]){s, ns})); > #else > return syscall(SYS_clock_settime, clk, ts); > #endif > } > > to ./lib/cobalt/clock.c, which is > > COBALT_IMPL(int, clock_settime, (clockid_t clock_id, const struct > timespec *tp)) > { > int ret; > > ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp); > if (ret) { > errno = ret; > return -1; > } > > return 0; > } > > in userspace, we use IS32BIT to check if tv_sec is 32bits, and switch it > to long long and pass to kernel space.
But that does not address the problem that old userspace libs will pass down 32-bit tv_sec, ie. will hand over smaller data structures. Also musl expects that there is a SYS_clock_settime64. At the same time, we won't have to address the case of running a newer lib over an older kernel. Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
