[PATCH v3 0/2] linux-user: support of semtimedop syscall

[PATCH v3 0/2] linux-user: support of semtimedop syscall

[Matus Kysel]

Updated version according Laurent's comments.

v3: split patch and add some arch specific modification	
 

[PATCH v3 1/2] linux-user: refactor ipc syscall

[Matus Kysel]

Refactoring ipc syscall for s390x and SPARC, so it matches glibc implementation

Signed-off-by: Matus Kysel <mkysel@tachyum.com>
---
 linux-user/syscall.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 97de9fb5c9..990412733b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -814,9 +814,14 @@ safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
               const struct timespec *, req, struct timespec *, rem)
 #endif
 #ifdef __NR_ipc
+#ifdef __s390x__
+safe_syscall5(int, ipc, int, call, long, first, long, second, long, third,
+              void *, ptr)
+#else
 safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
               void *, ptr, long, fifth)
 #endif
+#endif
 #ifdef __NR_msgsnd
 safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
               int, flags)
@@ -4053,8 +4058,13 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
 #endif
 #ifdef __NR_ipc
     if (ret == -TARGET_ENOSYS) {
+#ifdef __s390x__
+        ret = get_errno(safe_ipc(IPCOP_msgsnd, msqid, msgsz, msgflg,
+                                 host_mb));
+#else
         ret = get_errno(safe_ipc(IPCOP_msgsnd, msqid, msgsz, msgflg,
                                  host_mb, 0));
+#endif
     }
 #endif
     g_free(host_mb);
@@ -4063,6 +4073,20 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
     return ret;
 }
 
+#ifdef __NR_ipc
+#if defined(__sparc__)
+/* SPARC for msgrcv it does not use the kludge on final 2 arguments.  */
+#define MSGRCV_ARGS(__msgp, __msgtyp) __msgp, __msgtyp
+#elif defined(__s390x__)
+/* The s390 sys_ipc variant has only five parameters.  */
+#define MSGRCV_ARGS(__msgp, __msgtyp) \
+    ((long int[]){(long int)__msgp, __msgtyp})
+#else
+#define MSGRCV_ARGS(__msgp, __msgtyp) \
+    ((long int[]){(long int)__msgp, __msgtyp}), 0
+#endif
+#endif
+
 static inline abi_long do_msgrcv(int msqid, abi_long msgp,
                                  ssize_t msgsz, abi_long msgtyp,
                                  int msgflg)
@@ -4091,7 +4115,7 @@ static inline abi_long do_msgrcv(int msqid, abi_long msgp,
 #ifdef __NR_ipc
     if (ret == -TARGET_ENOSYS) {
         ret = get_errno(safe_ipc(IPCOP_CALL(1, IPCOP_msgrcv), msqid, msgsz,
-                        msgflg, host_mb, msgtyp));
+                        msgflg, MSGRCV_ARGS(host_mb, msgtyp)));
     }
 #endif
 
-- 
2.17.1

[PATCH v3 2/2] linux-user: support of semtimedop syscall

[Matus Kysel]

We should add support of semtimedop syscall as new version of glibc
2.31 uses semop based on semtimedop
(commit: https://gitlab.com/freedesktop-sdk/mirrors/sourceware/glibc/-/commit/765cdd0bffd77960ae852104fc4ea5edcdb8aed3 ).

Signed-off-by: Matus Kysel <mkysel@tachyum.com>
---
 linux-user/syscall.c | 58 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 990412733b..538a7e673c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1232,7 +1232,8 @@ static inline abi_long copy_to_user_timeval64(abi_ulong target_tv_addr,
     defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6) || \
     defined(TARGET_NR_nanosleep) || defined(TARGET_NR_clock_settime) || \
     defined(TARGET_NR_utimensat) || defined(TARGET_NR_mq_timedsend) || \
-    defined(TARGET_NR_mq_timedreceive)
+    defined(TARGET_NR_mq_timedreceive) || defined(TARGET_NR_ipc) || \
+    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop)
 static inline abi_long target_to_host_timespec(struct timespec *host_ts,
                                                abi_ulong target_addr)
 {
@@ -3880,25 +3881,53 @@ static inline abi_long target_to_host_sembuf(struct sembuf *host_sembuf,
     return 0;
 }
 
-static inline abi_long do_semop(int semid, abi_long ptr, unsigned nsops)
+#if defined(TARGET_NR_ipc) || defined(TARGET_NR_semop) || \
+    defined(TARGET_NR_semtimedop)
+
+/*
+ * This macro is required to handle the s390 variants, which passes the
+ * arguments in a different order than default.
+ */
+#ifdef __s390x__
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+  (__nsops), (__timeout), (__sops)
+#else
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+  (__nsops), 0, (__sops), (__timeout)
+#endif
+
+static inline abi_long do_semtimedop(int semid,
+                                     abi_long ptr,
+                                     unsigned nsops,
+                                     abi_long timeout)
 {
     struct sembuf sops[nsops];
+    struct timespec ts, *pts = NULL;
     abi_long ret;
 
+    if (timeout) {
+        pts = &ts;
+        if (target_to_host_timespec(pts, timeout)) {
+            return -TARGET_EFAULT;
+        }
+    }
+
     if (target_to_host_sembuf(sops, ptr, nsops))
         return -TARGET_EFAULT;
 
     ret = -TARGET_ENOSYS;
 #ifdef __NR_semtimedop
-    ret = get_errno(safe_semtimedop(semid, sops, nsops, NULL));
+    ret = get_errno(safe_semtimedop(semid, sops, nsops, pts));
 #endif
 #ifdef __NR_ipc
     if (ret == -TARGET_ENOSYS) {
-        ret = get_errno(safe_ipc(IPCOP_semtimedop, semid, nsops, 0, sops, 0));
+        ret = get_errno(safe_ipc(IPCOP_semtimedop, semid,
+                                 SEMTIMEDOP_IPC_ARGS(nsops, sops, (long)pts)));
     }
 #endif
     return ret;
 }
+#endif
 
 struct target_msqid_ds
 {
@@ -4393,7 +4422,20 @@ static abi_long do_ipc(CPUArchState *cpu_env,
 
     switch (call) {
     case IPCOP_semop:
-        ret = do_semop(first, ptr, second);
+        ret = do_semtimedop(first, ptr, second, 0);
+        break;
+    case IPCOP_semtimedop:
+    /*
+     * The s390 sys_ipc variant has only five parameters instead of six
+     * (as for default variant) and the only difference is the handling of
+     * SEMTIMEDOP where on s390 the third parameter is used as a pointer
+     * to a struct timespec where the generic variant uses fifth parameter.
+     */
+#if defined(TARGET_S390X)
+        ret = do_semtimedop(first, ptr, second, third);
+#else
+        ret = do_semtimedop(first, ptr, second, fifth);
+#endif
         break;
 
     case IPCOP_semget:
@@ -9644,7 +9686,11 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_semop
     case TARGET_NR_semop:
-        return do_semop(arg1, arg2, arg3);
+        return do_semtimedop(arg1, arg2, arg3, 0);
+#endif
+#ifdef TARGET_NR_semtimedop
+    case TARGET_NR_semtimedop:
+        return do_semtimedop(arg1, arg2, arg3, arg4);
 #endif
 #ifdef TARGET_NR_semctl
     case TARGET_NR_semctl:
-- 
2.17.1

Re: [PATCH v3 0/2] linux-user: support of semtimedop syscall

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200626124612.58593-1-mkysel@tachyum.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v3 0/2] linux-user: support of semtimedop syscall 
Type: series
Message-id: 20200626124612.58593-1-mkysel@tachyum.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   611ac63..10f7ffa  master     -> master
 * [new tag]         patchew/1593177220-28143-1-git-send-email-bmeng.cn@gmail.com -> patchew/1593177220-28143-1-git-send-email-bmeng.cn@gmail.com
 - [tag update]      patchew/20200625184838.28172-1-philmd@redhat.com -> patchew/20200625184838.28172-1-philmd@redhat.com
 - [tag update]      patchew/20200626033144.790098-1-richard.henderson@linaro.org -> patchew/20200626033144.790098-1-richard.henderson@linaro.org
 - [tag update]      patchew/20200626092317.3875-1-mark.cave-ayland@ilande.co.uk -> patchew/20200626092317.3875-1-mark.cave-ayland@ilande.co.uk
 * [new tag]         patchew/20200626124612.58593-1-mkysel@tachyum.com -> patchew/20200626124612.58593-1-mkysel@tachyum.com
 * [new tag]         patchew/20200626130658.76498-1-vsementsov@virtuozzo.com -> patchew/20200626130658.76498-1-vsementsov@virtuozzo.com
 - [tag update]      patchew/cover.1592315226.git.balaton@eik.bme.hu -> patchew/cover.1592315226.git.balaton@eik.bme.hu
Switched to a new branch 'test'
9b6fb16 linux-user: support of semtimedop syscall
230a3ca linux-user: refactor ipc syscall

=== OUTPUT BEGIN ===
1/2 Checking commit 230a3cadaac4 (linux-user: refactor ipc syscall)
WARNING: architecture specific defines should be avoided
#20: FILE: linux-user/syscall.c:817:
+#ifdef __s390x__

WARNING: architecture specific defines should be avoided
#35: FILE: linux-user/syscall.c:4061:
+#ifdef __s390x__

WARNING: architecture specific defines should be avoided
#49: FILE: linux-user/syscall.c:4076:
+#ifdef __NR_ipc

WARNING: architecture specific defines should be avoided
#50: FILE: linux-user/syscall.c:4077:
+#if defined(__sparc__)

ERROR: Macros with complex values should be enclosed in parenthesis
#52: FILE: linux-user/syscall.c:4079:
+#define MSGRCV_ARGS(__msgp, __msgtyp) __msgp, __msgtyp

ERROR: Macros with complex values should be enclosed in parenthesis
#58: FILE: linux-user/syscall.c:4085:
+#define MSGRCV_ARGS(__msgp, __msgtyp) \
+    ((long int[]){(long int)__msgp, __msgtyp}), 0

total: 2 errors, 4 warnings, 55 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/2 Checking commit 9b6fb16d5bb7 (linux-user: support of semtimedop syscall)
WARNING: architecture specific defines should be avoided
#40: FILE: linux-user/syscall.c:3891:
+#ifdef __s390x__

ERROR: Macros with complex values should be enclosed in parenthesis
#41: FILE: linux-user/syscall.c:3892:
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+  (__nsops), (__timeout), (__sops)

ERROR: Macros with complex values should be enclosed in parenthesis
#44: FILE: linux-user/syscall.c:3895:
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+  (__nsops), 0, (__sops), (__timeout)

total: 2 errors, 1 warnings, 98 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200626124612.58593-1-mkysel@tachyum.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 1/2] linux-user: refactor ipc syscall

[Laurent Vivier]

Le 26/06/2020 à 14:46, Matus Kysel a écrit :
> Refactoring ipc syscall for s390x and SPARC, so it matches glibc implementation
> 
> Signed-off-by: Matus Kysel <mkysel@tachyum.com>
> ---
>  linux-user/syscall.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 97de9fb5c9..990412733b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -814,9 +814,14 @@ safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
>                const struct timespec *, req, struct timespec *, rem)
>  #endif
>  #ifdef __NR_ipc
> +#ifdef __s390x__
> +safe_syscall5(int, ipc, int, call, long, first, long, second, long, third,
> +              void *, ptr)
> +#else
>  safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
>                void *, ptr, long, fifth)
>  #endif
> +#endif
>  #ifdef __NR_msgsnd
>  safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
>                int, flags)
> @@ -4053,8 +4058,13 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
>  #endif
>  #ifdef __NR_ipc
>      if (ret == -TARGET_ENOSYS) {
> +#ifdef __s390x__
> +        ret = get_errno(safe_ipc(IPCOP_msgsnd, msqid, msgsz, msgflg,
> +                                 host_mb));
> +#else
>          ret = get_errno(safe_ipc(IPCOP_msgsnd, msqid, msgsz, msgflg,
>                                   host_mb, 0));
> +#endif
>      }
>  #endif
>      g_free(host_mb);
> @@ -4063,6 +4073,20 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
>      return ret;
>  }
>  
> +#ifdef __NR_ipc
> +#if defined(__sparc__)
> +/* SPARC for msgrcv it does not use the kludge on final 2 arguments.  */
> +#define MSGRCV_ARGS(__msgp, __msgtyp) __msgp, __msgtyp
> +#elif defined(__s390x__)
> +/* The s390 sys_ipc variant has only five parameters.  */
> +#define MSGRCV_ARGS(__msgp, __msgtyp) \
> +    ((long int[]){(long int)__msgp, __msgtyp})
> +#else
> +#define MSGRCV_ARGS(__msgp, __msgtyp) \
> +    ((long int[]){(long int)__msgp, __msgtyp}), 0
> +#endif
> +#endif
> +
>  static inline abi_long do_msgrcv(int msqid, abi_long msgp,
>                                   ssize_t msgsz, abi_long msgtyp,
>                                   int msgflg)
> @@ -4091,7 +4115,7 @@ static inline abi_long do_msgrcv(int msqid, abi_long msgp,
>  #ifdef __NR_ipc
>      if (ret == -TARGET_ENOSYS) {
>          ret = get_errno(safe_ipc(IPCOP_CALL(1, IPCOP_msgrcv), msqid, msgsz,
> -                        msgflg, host_mb, msgtyp));
> +                        msgflg, MSGRCV_ARGS(host_mb, msgtyp)));
>      }
>  #endif
>  
> 

This patch breaks build because there is safe_ipc() that is not updated
to use only 5 arguments with s390x. This is updated in the next patch so
the build in the end works, but it breaks bisect so you should fix that.

Otherwise:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks,
Laurent

Re: [PATCH v3 2/2] linux-user: support of semtimedop syscall

[Laurent Vivier]

Le 26/06/2020 à 14:46, Matus Kysel a écrit :
> We should add support of semtimedop syscall as new version of glibc
> 2.31 uses semop based on semtimedop
> (commit: https://gitlab.com/freedesktop-sdk/mirrors/sourceware/glibc/-/commit/765cdd0bffd77960ae852104fc4ea5edcdb8aed3 ).
> 
> Signed-off-by: Matus Kysel <mkysel@tachyum.com>
> ---
>  linux-user/syscall.c | 58 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 990412733b..538a7e673c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1232,7 +1232,8 @@ static inline abi_long copy_to_user_timeval64(abi_ulong target_tv_addr,
>      defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6) || \
>      defined(TARGET_NR_nanosleep) || defined(TARGET_NR_clock_settime) || \
>      defined(TARGET_NR_utimensat) || defined(TARGET_NR_mq_timedsend) || \
> -    defined(TARGET_NR_mq_timedreceive)
> +    defined(TARGET_NR_mq_timedreceive) || defined(TARGET_NR_ipc) || \
> +    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop)
>  static inline abi_long target_to_host_timespec(struct timespec *host_ts,
>                                                 abi_ulong target_addr)
>  {
> @@ -3880,25 +3881,53 @@ static inline abi_long target_to_host_sembuf(struct sembuf *host_sembuf,
>      return 0;
>  }
>  
> -static inline abi_long do_semop(int semid, abi_long ptr, unsigned nsops)
> +#if defined(TARGET_NR_ipc) || defined(TARGET_NR_semop) || \
> +    defined(TARGET_NR_semtimedop)
> +
> +/*
> + * This macro is required to handle the s390 variants, which passes the
> + * arguments in a different order than default.
> + */
> +#ifdef __s390x__
> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
> +  (__nsops), (__timeout), (__sops)
> +#else
> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
> +  (__nsops), 0, (__sops), (__timeout)
> +#endif
> +
> +static inline abi_long do_semtimedop(int semid,
> +                                     abi_long ptr,
> +                                     unsigned nsops,
> +                                     abi_long timeout)
>  {
>      struct sembuf sops[nsops];
> +    struct timespec ts, *pts = NULL;
>      abi_long ret;
>  
> +    if (timeout) {
> +        pts = &ts;
> +        if (target_to_host_timespec(pts, timeout)) {
> +            return -TARGET_EFAULT;
> +        }
> +    }
> +
>      if (target_to_host_sembuf(sops, ptr, nsops))
>          return -TARGET_EFAULT;
>  
>      ret = -TARGET_ENOSYS;
>  #ifdef __NR_semtimedop
> -    ret = get_errno(safe_semtimedop(semid, sops, nsops, NULL));
> +    ret = get_errno(safe_semtimedop(semid, sops, nsops, pts));
>  #endif
>  #ifdef __NR_ipc
>      if (ret == -TARGET_ENOSYS) {
> -        ret = get_errno(safe_ipc(IPCOP_semtimedop, semid, nsops, 0, sops, 0));
> +        ret = get_errno(safe_ipc(IPCOP_semtimedop, semid,
> +                                 SEMTIMEDOP_IPC_ARGS(nsops, sops, (long)pts)));
>      }
>  #endif
>      return ret;
>  }
> +#endif
>  
>  struct target_msqid_ds
>  {
> @@ -4393,7 +4422,20 @@ static abi_long do_ipc(CPUArchState *cpu_env,
>  
>      switch (call) {
>      case IPCOP_semop:
> -        ret = do_semop(first, ptr, second);
> +        ret = do_semtimedop(first, ptr, second, 0);
> +        break;
> +    case IPCOP_semtimedop:
> +    /*
> +     * The s390 sys_ipc variant has only five parameters instead of six
> +     * (as for default variant) and the only difference is the handling of
> +     * SEMTIMEDOP where on s390 the third parameter is used as a pointer
> +     * to a struct timespec where the generic variant uses fifth parameter.
> +     */
> +#if defined(TARGET_S390X)
> +        ret = do_semtimedop(first, ptr, second, third);
> +#else
> +        ret = do_semtimedop(first, ptr, second, fifth);
> +#endif
>          break;
>  
>      case IPCOP_semget:
> @@ -9644,7 +9686,11 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>  #endif
>  #ifdef TARGET_NR_semop
>      case TARGET_NR_semop:
> -        return do_semop(arg1, arg2, arg3);
> +        return do_semtimedop(arg1, arg2, arg3, 0);
> +#endif
> +#ifdef TARGET_NR_semtimedop
> +    case TARGET_NR_semtimedop:
> +        return do_semtimedop(arg1, arg2, arg3, arg4);
>  #endif
>  #ifdef TARGET_NR_semctl
>      case TARGET_NR_semctl:
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v3 1/2] linux-user: refactor ipc syscall

[Laurent Vivier]

Le 10/07/2020 à 14:46, Laurent Vivier a écrit :
> Le 26/06/2020 à 14:46, Matus Kysel a écrit :
>> Refactoring ipc syscall for s390x and SPARC, so it matches glibc implementation
>>
>> Signed-off-by: Matus Kysel <mkysel@tachyum.com>
>> ---
>>  linux-user/syscall.c | 26 +++++++++++++++++++++++++-
>>  1 file changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 97de9fb5c9..990412733b 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -814,9 +814,14 @@ safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
>>                const struct timespec *, req, struct timespec *, rem)
>>  #endif
>>  #ifdef __NR_ipc
>> +#ifdef __s390x__
>> +safe_syscall5(int, ipc, int, call, long, first, long, second, long, third,
>> +              void *, ptr)
>> +#else
>>  safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
>>                void *, ptr, long, fifth)
>>  #endif
>> +#endif
>>  #ifdef __NR_msgsnd
>>  safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
>>                int, flags)
>> @@ -4053,8 +4058,13 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
>>  #endif
>>  #ifdef __NR_ipc
>>      if (ret == -TARGET_ENOSYS) {
>> +#ifdef __s390x__
>> +        ret = get_errno(safe_ipc(IPCOP_msgsnd, msqid, msgsz, msgflg,
>> +                                 host_mb));
>> +#else
>>          ret = get_errno(safe_ipc(IPCOP_msgsnd, msqid, msgsz, msgflg,
>>                                   host_mb, 0));
>> +#endif
>>      }
>>  #endif
>>      g_free(host_mb);
>> @@ -4063,6 +4073,20 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
>>      return ret;
>>  }
>>  
>> +#ifdef __NR_ipc
>> +#if defined(__sparc__)
>> +/* SPARC for msgrcv it does not use the kludge on final 2 arguments.  */
>> +#define MSGRCV_ARGS(__msgp, __msgtyp) __msgp, __msgtyp
>> +#elif defined(__s390x__)
>> +/* The s390 sys_ipc variant has only five parameters.  */
>> +#define MSGRCV_ARGS(__msgp, __msgtyp) \
>> +    ((long int[]){(long int)__msgp, __msgtyp})
>> +#else
>> +#define MSGRCV_ARGS(__msgp, __msgtyp) \
>> +    ((long int[]){(long int)__msgp, __msgtyp}), 0
>> +#endif
>> +#endif
>> +
>>  static inline abi_long do_msgrcv(int msqid, abi_long msgp,
>>                                   ssize_t msgsz, abi_long msgtyp,
>>                                   int msgflg)
>> @@ -4091,7 +4115,7 @@ static inline abi_long do_msgrcv(int msqid, abi_long msgp,
>>  #ifdef __NR_ipc
>>      if (ret == -TARGET_ENOSYS) {
>>          ret = get_errno(safe_ipc(IPCOP_CALL(1, IPCOP_msgrcv), msqid, msgsz,
>> -                        msgflg, host_mb, msgtyp));
>> +                        msgflg, MSGRCV_ARGS(host_mb, msgtyp)));
>>      }
>>  #endif
>>  
>>
> 
> This patch breaks build because there is safe_ipc() that is not updated
> to use only 5 arguments with s390x. This is updated in the next patch so
> the build in the end works, but it breaks bisect so you should fix that.
> 
> Otherwise:
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> Thanks,
> Laurent
> 

I have merged PATH 1 and 2 and applied to my linux-user-for-5.1 branch.

Thanks,
Laurent

Re: [PATCH v3 2/2] linux-user: support of semtimedop syscall

[Laurent Vivier]

Le 26/06/2020 à 14:46, Matus Kysel a écrit :
> We should add support of semtimedop syscall as new version of glibc
> 2.31 uses semop based on semtimedop
> (commit: https://gitlab.com/freedesktop-sdk/mirrors/sourceware/glibc/-/commit/765cdd0bffd77960ae852104fc4ea5edcdb8aed3 ).
> 
> Signed-off-by: Matus Kysel <mkysel@tachyum.com>
> ---
>  linux-user/syscall.c | 58 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 990412733b..538a7e673c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1232,7 +1232,8 @@ static inline abi_long copy_to_user_timeval64(abi_ulong target_tv_addr,
>      defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6) || \
>      defined(TARGET_NR_nanosleep) || defined(TARGET_NR_clock_settime) || \
>      defined(TARGET_NR_utimensat) || defined(TARGET_NR_mq_timedsend) || \
> -    defined(TARGET_NR_mq_timedreceive)
> +    defined(TARGET_NR_mq_timedreceive) || defined(TARGET_NR_ipc) || \
> +    defined(TARGET_NR_semop) || defined(TARGET_NR_semtimedop)
>  static inline abi_long target_to_host_timespec(struct timespec *host_ts,
>                                                 abi_ulong target_addr)
>  {
> @@ -3880,25 +3881,53 @@ static inline abi_long target_to_host_sembuf(struct sembuf *host_sembuf,
>      return 0;
>  }
>  
> -static inline abi_long do_semop(int semid, abi_long ptr, unsigned nsops)
> +#if defined(TARGET_NR_ipc) || defined(TARGET_NR_semop) || \
> +    defined(TARGET_NR_semtimedop)
> +
> +/*
> + * This macro is required to handle the s390 variants, which passes the
> + * arguments in a different order than default.
> + */
> +#ifdef __s390x__
> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
> +  (__nsops), (__timeout), (__sops)
> +#else
> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
> +  (__nsops), 0, (__sops), (__timeout)
> +#endif
> +
> +static inline abi_long do_semtimedop(int semid,
> +                                     abi_long ptr,
> +                                     unsigned nsops,
> +                                     abi_long timeout)
>  {
>      struct sembuf sops[nsops];
> +    struct timespec ts, *pts = NULL;
>      abi_long ret;
>  
> +    if (timeout) {
> +        pts = &ts;
> +        if (target_to_host_timespec(pts, timeout)) {
> +            return -TARGET_EFAULT;
> +        }
> +    }
> +
>      if (target_to_host_sembuf(sops, ptr, nsops))
>          return -TARGET_EFAULT;
>  
>      ret = -TARGET_ENOSYS;
>  #ifdef __NR_semtimedop
> -    ret = get_errno(safe_semtimedop(semid, sops, nsops, NULL));
> +    ret = get_errno(safe_semtimedop(semid, sops, nsops, pts));
>  #endif
>  #ifdef __NR_ipc
>      if (ret == -TARGET_ENOSYS) {
> -        ret = get_errno(safe_ipc(IPCOP_semtimedop, semid, nsops, 0, sops, 0));
> +        ret = get_errno(safe_ipc(IPCOP_semtimedop, semid,
> +                                 SEMTIMEDOP_IPC_ARGS(nsops, sops, (long)pts)));
>      }
>  #endif
>      return ret;
>  }
> +#endif
>  
>  struct target_msqid_ds
>  {
> @@ -4393,7 +4422,20 @@ static abi_long do_ipc(CPUArchState *cpu_env,
>  
>      switch (call) {
>      case IPCOP_semop:
> -        ret = do_semop(first, ptr, second);
> +        ret = do_semtimedop(first, ptr, second, 0);
> +        break;
> +    case IPCOP_semtimedop:
> +    /*
> +     * The s390 sys_ipc variant has only five parameters instead of six
> +     * (as for default variant) and the only difference is the handling of
> +     * SEMTIMEDOP where on s390 the third parameter is used as a pointer
> +     * to a struct timespec where the generic variant uses fifth parameter.
> +     */
> +#if defined(TARGET_S390X)
> +        ret = do_semtimedop(first, ptr, second, third);
> +#else
> +        ret = do_semtimedop(first, ptr, second, fifth);
> +#endif
>          break;
>  
>      case IPCOP_semget:
> @@ -9644,7 +9686,11 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>  #endif
>  #ifdef TARGET_NR_semop
>      case TARGET_NR_semop:
> -        return do_semop(arg1, arg2, arg3);
> +        return do_semtimedop(arg1, arg2, arg3, 0);
> +#endif
> +#ifdef TARGET_NR_semtimedop
> +    case TARGET_NR_semtimedop:
> +        return do_semtimedop(arg1, arg2, arg3, arg4);
>  #endif
>  #ifdef TARGET_NR_semctl
>      case TARGET_NR_semctl:
> 

I have merged PATH 1 and 2 and applied to my linux-user-for-5.1 branch.

Thanks,
Laurent