| Message ID | ce73e32d4b399581b25d2323fad1d817d66dd11f.1584051142.git.alistair.francis@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | linux-user: generate syscall_nr.sh for RISC-V | expand |
Le 12/03/2020 à 23:13, Alistair Francis a écrit : > Add support for host and target futex_time64. If futex_time64 exists on > the host we try that first before falling back to the standard futux > syscall. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > linux-user/syscall.c | 144 +++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 131 insertions(+), 13 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 60fd775d9c..9ae7a05e38 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c ... > @@ -6890,6 +6905,55 @@ static inline abi_long host_to_target_statx(struct target_statx *host_stx, > } > #endif > > +static int do_sys_futex(int *uaddr, int op, int val, > + const struct timespec *timeout, int *uaddr2, > + int val3) > +{ > +#if HOST_LONG_BITS == 64 > +#if defined(__NR_futex) > + /* always a 64-bit time_t, it doesn't define _time64 version */ > + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); > + > +#endif > +#else /* HOST_LONG_BITS == 64 */ > +#if defined(__NR_futex_time64) > + if (sizeof(timeout->tv_sec) == 8) { > + /* _time64 function on 32bit arch */ > + return sys_futex_time64(uaddr, op, val, timeout, uaddr2, val3); > + } > +#endif > +#if defined(__NR_futex) > + /* old function on 32bit arch */ > + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); > +#endif > +#endif /* HOST_LONG_BITS == 64 */ > + return -TARGET_ENOSYS; You cannot return -TARGET_ENOSYS because return value is supposed to be a host value as you have direct value from sys_futex(). ... > @@ -7505,7 +7619,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, > ts = cpu->opaque; > if (ts->child_tidptr) { > put_user_u32(0, ts->child_tidptr); > - sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, > + do_sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, > NULL, NULL, 0); And return value is ignored. Anyway at this point we can't do anything if it doesn't work, but it should not happen. So I think the best to do is to add a g_assert_not_reached() in place of "return -TARGET_ENOSYS" in do_sys_futex() (or "#error" if no futex function is available). Thanks, Laurent
On Fri, Mar 13, 2020 at 1:14 AM Laurent Vivier <laurent@vivier.eu> wrote: > > Le 12/03/2020 à 23:13, Alistair Francis a écrit : > > Add support for host and target futex_time64. If futex_time64 exists on > > the host we try that first before falling back to the standard futux > > syscall. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > linux-user/syscall.c | 144 +++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 131 insertions(+), 13 deletions(-) > > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 60fd775d9c..9ae7a05e38 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > ... > > @@ -6890,6 +6905,55 @@ static inline abi_long host_to_target_statx(struct target_statx *host_stx, > > } > > #endif > > > > +static int do_sys_futex(int *uaddr, int op, int val, > > + const struct timespec *timeout, int *uaddr2, > > + int val3) > > +{ > > +#if HOST_LONG_BITS == 64 > > +#if defined(__NR_futex) > > + /* always a 64-bit time_t, it doesn't define _time64 version */ > > + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); > > + > > +#endif > > +#else /* HOST_LONG_BITS == 64 */ > > +#if defined(__NR_futex_time64) > > + if (sizeof(timeout->tv_sec) == 8) { > > + /* _time64 function on 32bit arch */ > > + return sys_futex_time64(uaddr, op, val, timeout, uaddr2, val3); > > + } > > +#endif > > +#if defined(__NR_futex) > > + /* old function on 32bit arch */ > > + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); > > +#endif > > +#endif /* HOST_LONG_BITS == 64 */ > > + return -TARGET_ENOSYS; > > You cannot return -TARGET_ENOSYS because return value is supposed to be > a host value as you have direct value from sys_futex(). > > ... > > > @@ -7505,7 +7619,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, > > ts = cpu->opaque; > > if (ts->child_tidptr) { > > put_user_u32(0, ts->child_tidptr); > > - sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, > > + do_sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, > > NULL, NULL, 0); > > And return value is ignored. Anyway at this point we can't do anything > if it doesn't work, but it should not happen. So I think the best to do > is to add a g_assert_not_reached() in place of "return -TARGET_ENOSYS" > in do_sys_futex() (or "#error" if no futex function is available). It sounds like you applied this series, so I'm not going to fix this up. Let me know if you would like me to. Alistair > > Thanks, > Laurent > >
On Fri, Mar 13, 2020 at 3:12 PM Alistair Francis <alistair23@gmail.com> wrote: > > On Fri, Mar 13, 2020 at 1:14 AM Laurent Vivier <laurent@vivier.eu> wrote: > > > > Le 12/03/2020 à 23:13, Alistair Francis a écrit : > > > Add support for host and target futex_time64. If futex_time64 exists on > > > the host we try that first before falling back to the standard futux > > > syscall. > > > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > > --- > > > linux-user/syscall.c | 144 +++++++++++++++++++++++++++++++++++++++---- > > > 1 file changed, 131 insertions(+), 13 deletions(-) > > > > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > > index 60fd775d9c..9ae7a05e38 100644 > > > --- a/linux-user/syscall.c > > > +++ b/linux-user/syscall.c > > ... > > > @@ -6890,6 +6905,55 @@ static inline abi_long host_to_target_statx(struct target_statx *host_stx, > > > } > > > #endif > > > > > > +static int do_sys_futex(int *uaddr, int op, int val, > > > + const struct timespec *timeout, int *uaddr2, > > > + int val3) > > > +{ > > > +#if HOST_LONG_BITS == 64 > > > +#if defined(__NR_futex) > > > + /* always a 64-bit time_t, it doesn't define _time64 version */ > > > + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); > > > + > > > +#endif > > > +#else /* HOST_LONG_BITS == 64 */ > > > +#if defined(__NR_futex_time64) > > > + if (sizeof(timeout->tv_sec) == 8) { > > > + /* _time64 function on 32bit arch */ > > > + return sys_futex_time64(uaddr, op, val, timeout, uaddr2, val3); > > > + } > > > +#endif > > > +#if defined(__NR_futex) > > > + /* old function on 32bit arch */ > > > + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); > > > +#endif > > > +#endif /* HOST_LONG_BITS == 64 */ > > > + return -TARGET_ENOSYS; > > > > You cannot return -TARGET_ENOSYS because return value is supposed to be > > a host value as you have direct value from sys_futex(). > > > > ... > > > > > @@ -7505,7 +7619,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, > > > ts = cpu->opaque; > > > if (ts->child_tidptr) { > > > put_user_u32(0, ts->child_tidptr); > > > - sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, > > > + do_sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, > > > NULL, NULL, 0); > > > > And return value is ignored. Anyway at this point we can't do anything > > if it doesn't work, but it should not happen. So I think the best to do > > is to add a g_assert_not_reached() in place of "return -TARGET_ENOSYS" > > in do_sys_futex() (or "#error" if no futex function is available). > > It sounds like you applied this series, so I'm not going to fix this > up. Let me know if you would like me to. Ha, I read the wrong cover letter. I'll send a new version of this. Alistair > > Alistair > > > > > Thanks, > > Laurent > > > >
Le 13/03/2020 à 23:13, Alistair Francis a écrit : > On Fri, Mar 13, 2020 at 3:12 PM Alistair Francis <alistair23@gmail.com> wrote: >> >> On Fri, Mar 13, 2020 at 1:14 AM Laurent Vivier <laurent@vivier.eu> wrote: >>> >>> Le 12/03/2020 à 23:13, Alistair Francis a écrit : >>>> Add support for host and target futex_time64. If futex_time64 exists on >>>> the host we try that first before falling back to the standard futux >>>> syscall. >>>> >>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> >>>> --- >>>> linux-user/syscall.c | 144 +++++++++++++++++++++++++++++++++++++++---- >>>> 1 file changed, 131 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >>>> index 60fd775d9c..9ae7a05e38 100644 >>>> --- a/linux-user/syscall.c >>>> +++ b/linux-user/syscall.c >>> ... >>>> @@ -6890,6 +6905,55 @@ static inline abi_long host_to_target_statx(struct target_statx *host_stx, >>>> } >>>> #endif >>>> >>>> +static int do_sys_futex(int *uaddr, int op, int val, >>>> + const struct timespec *timeout, int *uaddr2, >>>> + int val3) >>>> +{ >>>> +#if HOST_LONG_BITS == 64 >>>> +#if defined(__NR_futex) >>>> + /* always a 64-bit time_t, it doesn't define _time64 version */ >>>> + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); >>>> + >>>> +#endif >>>> +#else /* HOST_LONG_BITS == 64 */ >>>> +#if defined(__NR_futex_time64) >>>> + if (sizeof(timeout->tv_sec) == 8) { >>>> + /* _time64 function on 32bit arch */ >>>> + return sys_futex_time64(uaddr, op, val, timeout, uaddr2, val3); >>>> + } >>>> +#endif >>>> +#if defined(__NR_futex) >>>> + /* old function on 32bit arch */ >>>> + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); >>>> +#endif >>>> +#endif /* HOST_LONG_BITS == 64 */ >>>> + return -TARGET_ENOSYS; >>> >>> You cannot return -TARGET_ENOSYS because return value is supposed to be >>> a host value as you have direct value from sys_futex(). >>> >>> ... >>> >>>> @@ -7505,7 +7619,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, >>>> ts = cpu->opaque; >>>> if (ts->child_tidptr) { >>>> put_user_u32(0, ts->child_tidptr); >>>> - sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, >>>> + do_sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, >>>> NULL, NULL, 0); >>> >>> And return value is ignored. Anyway at this point we can't do anything >>> if it doesn't work, but it should not happen. So I think the best to do >>> is to add a g_assert_not_reached() in place of "return -TARGET_ENOSYS" >>> in do_sys_futex() (or "#error" if no futex function is available). >> >> It sounds like you applied this series, so I'm not going to fix this >> up. Let me know if you would like me to. > > Ha, I read the wrong cover letter. > > I'll send a new version of this. I have applied all the series except this patch. Thanks, Laurent
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 60fd775d9c..9ae7a05e38 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -245,7 +245,12 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 arg4,type5 arg5, \ #define __NR_sys_rt_sigqueueinfo __NR_rt_sigqueueinfo #define __NR_sys_rt_tgsigqueueinfo __NR_rt_tgsigqueueinfo #define __NR_sys_syslog __NR_syslog -#define __NR_sys_futex __NR_futex +#if defined(__NR_futex) +# define __NR_sys_futex __NR_futex +#endif +#if defined(__NR_futex_time64) +# define __NR_sys_futex_time64 __NR_futex_time64 +#endif #define __NR_sys_inotify_init __NR_inotify_init #define __NR_sys_inotify_add_watch __NR_inotify_add_watch #define __NR_sys_inotify_rm_watch __NR_inotify_rm_watch @@ -295,10 +300,16 @@ _syscall1(int,exit_group,int,error_code) #if defined(TARGET_NR_set_tid_address) && defined(__NR_set_tid_address) _syscall1(int,set_tid_address,int *,tidptr) #endif -#if defined(TARGET_NR_futex) && defined(__NR_futex) +#if (defined(TARGET_NR_futex) && defined(__NR_futex)) || \ + (defined(TARGET_NR_futex_time64) && \ + (HOST_LONG_BITS == 64 && defined(__NR_futex))) _syscall6(int,sys_futex,int *,uaddr,int,op,int,val, const struct timespec *,timeout,int *,uaddr2,int,val3) #endif +#if (defined(TARGET_NR_futex_time64) && defined(__NR_futex_teim64)) +_syscall6(int,sys_futex_time64,int *,uaddr,int,op,int,val, + const struct timespec *,timeout,int *,uaddr2,int,val3) +#endif #define __NR_sys_sched_getaffinity __NR_sched_getaffinity _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned int, len, unsigned long *, user_mask_ptr); @@ -762,10 +773,14 @@ safe_syscall5(int, ppoll, struct pollfd *, ufds, unsigned int, nfds, safe_syscall6(int, epoll_pwait, int, epfd, struct epoll_event *, events, int, maxevents, int, timeout, const sigset_t *, sigmask, size_t, sigsetsize) -#ifdef TARGET_NR_futex +#if defined(__NR_futex) safe_syscall6(int,futex,int *,uaddr,int,op,int,val, \ const struct timespec *,timeout,int *,uaddr2,int,val3) #endif +#if defined(__NR_futex_time64) +safe_syscall6(int,futex_time64,int *,uaddr,int,op,int,val, \ + const struct timespec *,timeout,int *,uaddr2,int,val3) +#endif safe_syscall2(int, rt_sigsuspend, sigset_t *, newset, size_t, sigsetsize) safe_syscall2(int, kill, pid_t, pid, int, sig) safe_syscall2(int, tkill, int, tid, int, sig) @@ -1229,7 +1244,7 @@ static inline abi_long target_to_host_timespec(struct timespec *host_ts, } #endif -#if defined(TARGET_NR_clock_settime64) +#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64) static inline abi_long target_to_host_timespec64(struct timespec *host_ts, abi_ulong target_addr) { @@ -6890,6 +6905,55 @@ static inline abi_long host_to_target_statx(struct target_statx *host_stx, } #endif +static int do_sys_futex(int *uaddr, int op, int val, + const struct timespec *timeout, int *uaddr2, + int val3) +{ +#if HOST_LONG_BITS == 64 +#if defined(__NR_futex) + /* always a 64-bit time_t, it doesn't define _time64 version */ + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); + +#endif +#else /* HOST_LONG_BITS == 64 */ +#if defined(__NR_futex_time64) + if (sizeof(timeout->tv_sec) == 8) { + /* _time64 function on 32bit arch */ + return sys_futex_time64(uaddr, op, val, timeout, uaddr2, val3); + } +#endif +#if defined(__NR_futex) + /* old function on 32bit arch */ + return sys_futex(uaddr, op, val, timeout, uaddr2, val3); +#endif +#endif /* HOST_LONG_BITS == 64 */ + return -TARGET_ENOSYS; +} + +static int do_safe_futex(int *uaddr, int op, int val, + const struct timespec *timeout, int *uaddr2, + int val3) +{ +#if HOST_LONG_BITS == 64 +#if defined(__NR_futex) + /* always a 64-bit time_t, it doesn't define _time64 version */ + return get_errno(safe_futex(uaddr, op, val, timeout, uaddr2, val3)); +#endif +#else /* HOST_LONG_BITS == 64 */ +#if defined(__NR_futex_time64) + if (sizeof(timeout->tv_sec) == 8) { + /* _time64 function on 32bit arch */ + return get_errno(safe_futex_time64(uaddr, op, val, timeout, uaddr2, + val3)); + } +#endif +#if defined(__NR_futex) + /* old function on 32bit arch */ + return get_errno(safe_futex(uaddr, op, val, timeout, uaddr2, val3)); +#endif +#endif /* HOST_LONG_BITS == 64 */ + return -TARGET_ENOSYS; +} /* ??? Using host futex calls even when target atomic operations are not really atomic probably breaks things. However implementing @@ -6919,12 +6983,61 @@ static int do_futex(target_ulong uaddr, int op, int val, target_ulong timeout, } else { pts = NULL; } - return get_errno(safe_futex(g2h(uaddr), op, tswap32(val), + return get_errno(do_safe_futex(g2h(uaddr), op, tswap32(val), + pts, NULL, val3)); + case FUTEX_WAKE: + return get_errno(do_safe_futex(g2h(uaddr), op, val, NULL, NULL, 0)); + case FUTEX_FD: + return get_errno(do_safe_futex(g2h(uaddr), op, val, NULL, NULL, 0)); + case FUTEX_REQUEUE: + case FUTEX_CMP_REQUEUE: + case FUTEX_WAKE_OP: + /* For FUTEX_REQUEUE, FUTEX_CMP_REQUEUE, and FUTEX_WAKE_OP, the + TIMEOUT parameter is interpreted as a uint32_t by the kernel. + But the prototype takes a `struct timespec *'; insert casts + to satisfy the compiler. We do not need to tswap TIMEOUT + since it's not compared to guest memory. */ + pts = (struct timespec *)(uintptr_t) timeout; + return get_errno(do_safe_futex(g2h(uaddr), op, val, pts, + g2h(uaddr2), + (base_op == FUTEX_CMP_REQUEUE + ? tswap32(val3) + : val3))); + default: + return -TARGET_ENOSYS; + } +} +#endif + +#if defined(TARGET_NR_futex_time64) +static int do_futex_time64(target_ulong uaddr, int op, int val, target_ulong timeout, + target_ulong uaddr2, int val3) +{ + struct timespec ts, *pts; + int base_op; + + /* ??? We assume FUTEX_* constants are the same on both host + and target. */ +#ifdef FUTEX_CMD_MASK + base_op = op & FUTEX_CMD_MASK; +#else + base_op = op; +#endif + switch (base_op) { + case FUTEX_WAIT: + case FUTEX_WAIT_BITSET: + if (timeout) { + pts = &ts; + target_to_host_timespec64(pts, timeout); + } else { + pts = NULL; + } + return get_errno(do_safe_futex(g2h(uaddr), op, tswap32(val), pts, NULL, val3)); case FUTEX_WAKE: - return get_errno(safe_futex(g2h(uaddr), op, val, NULL, NULL, 0)); + return get_errno(do_safe_futex(g2h(uaddr), op, val, NULL, NULL, 0)); case FUTEX_FD: - return get_errno(safe_futex(g2h(uaddr), op, val, NULL, NULL, 0)); + return get_errno(do_safe_futex(g2h(uaddr), op, val, NULL, NULL, 0)); case FUTEX_REQUEUE: case FUTEX_CMP_REQUEUE: case FUTEX_WAKE_OP: @@ -6934,16 +7047,17 @@ static int do_futex(target_ulong uaddr, int op, int val, target_ulong timeout, to satisfy the compiler. We do not need to tswap TIMEOUT since it's not compared to guest memory. */ pts = (struct timespec *)(uintptr_t) timeout; - return get_errno(safe_futex(g2h(uaddr), op, val, pts, - g2h(uaddr2), - (base_op == FUTEX_CMP_REQUEUE - ? tswap32(val3) - : val3))); + return get_errno(do_safe_futex(g2h(uaddr), op, val, pts, + g2h(uaddr2), + (base_op == FUTEX_CMP_REQUEUE + ? tswap32(val3) + : val3))); default: return -TARGET_ENOSYS; } } #endif + #if defined(TARGET_NR_name_to_handle_at) && defined(CONFIG_OPEN_BY_HANDLE) static abi_long do_name_to_handle_at(abi_long dirfd, abi_long pathname, abi_long handle, abi_long mount_id, @@ -7505,7 +7619,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, ts = cpu->opaque; if (ts->child_tidptr) { put_user_u32(0, ts->child_tidptr); - sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, + do_sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, NULL, NULL, 0); } thread_cpu = NULL; @@ -11597,6 +11711,10 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, case TARGET_NR_futex: return do_futex(arg1, arg2, arg3, arg4, arg5, arg6); #endif +#ifdef TARGET_NR_futex_time64 + case TARGET_NR_futex_time64: + return do_futex_time64(arg1, arg2, arg3, arg4, arg5, arg6); +#endif #if defined(TARGET_NR_inotify_init) && defined(__NR_inotify_init) case TARGET_NR_inotify_init: ret = get_errno(sys_inotify_init());
Add support for host and target futex_time64. If futex_time64 exists on the host we try that first before falling back to the standard futux syscall. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- linux-user/syscall.c | 144 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 131 insertions(+), 13 deletions(-)