[Qemu-devel] [PULL 0/8] xtensa-specific and generic fixes for linux-user

[Qemu-devel] [PULL 0/8] xtensa-specific and generic fixes for linux-user

[Max Filippov]

Hi Peter,

please pull the following batch of linux-user fixes for 2.12.

The following changes since commit dfe732fb68ef9195517f4f380a477d58a054edc1:

  Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into staging (2018-03-27 16:25:12 +0100)

are available in the git repository at:

  git://github.com/OSLL/qemu-xtensa.git tags/20180402-xtensa

for you to fetch changes up to 64a563dd8dd6ca2661d96a2e4b69f0a5465cab94:

  target/xtensa: linux-user: fix fadvise64 call (2018-04-02 04:15:35 -0700)

----------------------------------------------------------------
xtensa-specific fixes for linux-user:

- fix flushing registers for signal processing in call8 and call12 frames;
- fix PC value for restarted syscalls;
- fix sysv IPC structures;
- fix fadvise64 syscall;

generic fixes for linux-user:

- fix QEMU assertion in multithreaded application by calling cpu_copy
  under clone_lock;
- fix mq_getsetattr implementation;
- fix error propagation in clock_gettime;
- implement clock_settime.

----------------------------------------------------------------
Max Filippov (8):
      target/xtensa: fix flush_window_regs
      target/xtensa: linux-user: rewind pc for restarted syscall
      linux-user: call cpu_copy under clone_lock
      linux-user: fix mq_getsetattr implementation
      target/xtensa: linux-user: fix sysv IPC structures
      linux-user: fix error propagation in clock_gettime
      linux-user: implement clock_settime
      target/xtensa: linux-user: fix fadvise64 call

 linux-user/main.c                  |  3 +++
 linux-user/signal.c                | 55 +++++++++++++++++---------------------
 linux-user/syscall.c               | 36 +++++++++++++++++--------
 linux-user/xtensa/target_structs.h | 37 ++++++++++++++++++++-----
 4 files changed, 82 insertions(+), 49 deletions(-)

-- 
Thanks.
-- Max

[Qemu-devel] [PULL 1/8] target/xtensa: fix flush_window_regs

[Max Filippov]

flush_window_regs uses wrong stack frame to save overflow registers in
call8 and call12 frames, which results in wrong register values in
callers of a function that received a signal.
Reimplement flush_window_regs closely following window overflow
sequence.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/signal.c | 55 +++++++++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 2ea3e0321f4d..33d5ced30c98 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -7094,52 +7094,45 @@ static abi_ulong get_sigframe(struct target_sigaction *sa,
 
 static int flush_window_regs(CPUXtensaState *env)
 {
-    const uint32_t nareg_mask = env->config->nareg - 1;
     uint32_t wb = env->sregs[WINDOW_BASE];
-    uint32_t ws = (xtensa_replicate_windowstart(env) >> (wb + 1)) &
-        ((1 << env->config->nareg / 4) - 1);
-    uint32_t d = ctz32(ws) + 1;
-    uint32_t sp;
-    abi_long ret = 0;
-
-    wb += d;
-    ws >>= d;
+    uint32_t ws = xtensa_replicate_windowstart(env) >> (wb + 1);
+    unsigned d = ctz32(ws) + 1;
+    unsigned i;
+    int ret = 0;
 
-    xtensa_sync_phys_from_window(env);
-    sp = env->phys_regs[(wb * 4 + 1) & nareg_mask];
+    for (i = d; i < env->config->nareg / 4; i += d) {
+        uint32_t ssp, osp;
+        unsigned j;
 
-    while (ws && ret == 0) {
-        int d;
-        int i;
-        int idx;
+        ws >>= d;
+        xtensa_rotate_window(env, d);
 
         if (ws & 0x1) {
-            ws >>= 1;
+            ssp = env->regs[5];
             d = 1;
         } else if (ws & 0x2) {
-            ws >>= 2;
+            ssp = env->regs[9];
+            ret |= get_user_ual(osp, env->regs[1] - 12);
+            osp -= 32;
             d = 2;
-            for (i = 0; i < 4; ++i) {
-                idx = (wb * 4 + 4 + i) & nareg_mask;
-                ret |= put_user_ual(env->phys_regs[idx], sp + (i - 12) * 4);
-            }
         } else if (ws & 0x4) {
-            ws >>= 3;
+            ssp = env->regs[13];
+            ret |= get_user_ual(osp, env->regs[1] - 12);
+            osp -= 48;
             d = 3;
-            for (i = 0; i < 8; ++i) {
-                idx = (wb * 4 + 4 + i) & nareg_mask;
-                ret |= put_user_ual(env->phys_regs[idx], sp + (i - 16) * 4);
-            }
         } else {
             g_assert_not_reached();
         }
-        sp = env->phys_regs[((wb + d) * 4 + 1) & nareg_mask];
-        for (i = 0; i < 4; ++i) {
-            idx = (wb * 4 + i) & nareg_mask;
-            ret |= put_user_ual(env->phys_regs[idx], sp + (i - 4) * 4);
+
+        for (j = 0; j < 4; ++j) {
+            ret |= put_user_ual(env->regs[j], ssp - 16 + j * 4);
+        }
+        for (j = 4; j < d * 4; ++j) {
+            ret |= put_user_ual(env->regs[j], osp - 16 + j * 4);
         }
-        wb += d;
     }
+    xtensa_rotate_window(env, d);
+    g_assert(env->sregs[WINDOW_BASE] == wb);
     return ret == 0;
 }
 
-- 
2.11.0

[Qemu-devel] [PULL 2/8] target/xtensa: linux-user: rewind pc for restarted syscall

[Max Filippov]

In case of syscall restart request set pc back to the syscall
instruction.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/main.c b/linux-user/main.c
index ba09b7d0c873..8907a8411411 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4006,6 +4006,9 @@ void cpu_loop(CPUXtensaState *env)
                     break;
 
                 case -TARGET_ERESTARTSYS:
+                    env->pc -= 3;
+                    break;
+
                 case -TARGET_QEMU_ESIGRETURN:
                     break;
                 }
-- 
2.11.0

[Qemu-devel] [PULL 3/8] linux-user: call cpu_copy under clone_lock

[Max Filippov]

cpu_copy adds newly created CPU object to container/machine/unattached,
but does it w/o proper locking. As a result when multiple threads create
threads rapidly QEMU may abort with the following message:

  GLib-CRITICAL **: g_hash_table_iter_next: assertion
  'ri->version == ri->hash_table->version' failed

  ERROR:qemu/qom/object.c:1663:object_get_canonical_path_component:
  code should not be reached

E.g. this issue is observed when running glibc test nptl/tst-eintr1.
Move cpu_copy invocation under clone_lock to fix that.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/syscall.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 889abbda1e65..18ea79140f16 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6346,6 +6346,10 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
 
         ts = g_new0(TaskState, 1);
         init_task_state(ts);
+
+        /* Grab a mutex so that thread setup appears atomic.  */
+        pthread_mutex_lock(&clone_lock);
+
         /* we create a new CPU instance. */
         new_env = cpu_copy(env);
         /* Init regs that differ from the parent.  */
@@ -6364,9 +6368,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
             cpu_set_tls (new_env, newtls);
         }
 
-        /* Grab a mutex so that thread setup appears atomic.  */
-        pthread_mutex_lock(&clone_lock);
-
         memset(&info, 0, sizeof(info));
         pthread_mutex_init(&info.mutex, NULL);
         pthread_mutex_lock(&info.mutex);
-- 
2.11.0

[Qemu-devel] [PULL 4/8] linux-user: fix mq_getsetattr implementation

[Max Filippov]

mq_getsetattr implementation does not set errno correctly in case of
error. Also in the presence of both 2nd and 3rd arguments it calls both
mq_getattr and mq_setattr, whereas only the latter call would suffice.

Don't call mq_getattr in the presence of the 2nd argument. Don't copy
output back to user in case of error. Use get_errno to set errno value.

This fixes test rt/tst-mqueue2 from the glibc testsuite.

Cc: Lionel Landwerlin <lionel.landwerlin@openwide.fr>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Riku Voipio <riku.voipio@iki.fi>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/syscall.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 18ea79140f16..d51e2a00ee31 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12092,15 +12092,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             struct mq_attr posix_mq_attr_in, posix_mq_attr_out;
             ret = 0;
-            if (arg3 != 0) {
-                ret = mq_getattr(arg1, &posix_mq_attr_out);
-                copy_to_user_mq_attr(arg3, &posix_mq_attr_out);
-            }
             if (arg2 != 0) {
                 copy_from_user_mq_attr(&posix_mq_attr_in, arg2);
-                ret |= mq_setattr(arg1, &posix_mq_attr_in, &posix_mq_attr_out);
+                ret = get_errno(mq_setattr(arg1, &posix_mq_attr_in,
+                                           &posix_mq_attr_out));
+            } else if (arg3 != 0) {
+                ret = get_errno(mq_getattr(arg1, &posix_mq_attr_out));
+            }
+            if (ret == 0 && arg3 != 0) {
+                copy_to_user_mq_attr(arg3, &posix_mq_attr_out);
             }
-
         }
         break;
 #endif
-- 
2.11.0

[Qemu-devel] [PULL 5/8] target/xtensa: linux-user: fix sysv IPC structures

[Max Filippov]

- make target_ipc_perm fields match kernel definitions for xtensa;
- add target_semid64_ds with proper order of times and reserved fields
  for little/big endian specific for xtensa;
- add missing reserved fields after time fields to the target_shmid_ds;
- fix types of shm_cpid, shm_lpid and shm_nattch fields of
  target_shmid_ds to match kernel definitions for xtensa.

These changes fix guest ipcs output and fix glibc testsuite tests
sysvipc/test-sysvsem and sysvipc/test-sysvshm.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/xtensa/target_structs.h | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/linux-user/xtensa/target_structs.h b/linux-user/xtensa/target_structs.h
index 020e20e242fc..1b3d9ca314ff 100644
--- a/linux-user/xtensa/target_structs.h
+++ b/linux-user/xtensa/target_structs.h
@@ -8,21 +8,44 @@ struct target_ipc_perm {
     abi_uint cuid;                      /* Creator's user ID.  */
     abi_uint cgid;                      /* Creator's group ID.  */
     abi_uint mode;                      /* Read/write permission.  */
-    abi_ushort __seq;                   /* Sequence number.  */
+    abi_ulong __seq;                    /* Sequence number.  */
+    abi_ulong __unused1;
+    abi_ulong __unused2;
+};
+
+struct target_semid64_ds {
+  struct target_ipc_perm sem_perm;
+#ifdef TARGET_WORDS_BIGENDIAN
+  abi_ulong __unused1;
+  abi_ulong sem_otime;
+  abi_ulong __unused2;
+  abi_ulong sem_ctime;
+#else
+  abi_ulong sem_otime;
+  abi_ulong __unused1;
+  abi_ulong sem_ctime;
+  abi_ulong __unused2;
+#endif
+  abi_ulong sem_nsems;
+  abi_ulong __unused3;
+  abi_ulong __unused4;
 };
+#define TARGET_SEMID64_DS
 
 struct target_shmid_ds {
     struct target_ipc_perm shm_perm;    /* operation permission struct */
-    abi_int shm_segsz;                  /* size of segment in bytes */
+    abi_long shm_segsz;                 /* size of segment in bytes */
     abi_long shm_atime;                 /* time of last shmat() */
+    abi_ulong __unused1;
     abi_long shm_dtime;                 /* time of last shmdt() */
-    abi_long shm_ctime;                 /* time of last change by shmctl() */
-    abi_ushort shm_cpid;                /* pid of creator */
-    abi_ushort shm_lpid;                /* pid of last shmop */
-    abi_ushort shm_nattch;              /* number of current attaches */
-    abi_ushort shm_unused;              /* compatibility */
     abi_ulong __unused2;
+    abi_long shm_ctime;                 /* time of last change by shmctl() */
     abi_ulong __unused3;
+    abi_uint shm_cpid;                  /* pid of creator */
+    abi_uint shm_lpid;                  /* pid of last shmop */
+    abi_ulong shm_nattch;               /* number of current attaches */
+    abi_ulong __unused4;
+    abi_ulong __unused5;
 };
 
 #endif
-- 
2.11.0

[Qemu-devel] [PULL 6/8] linux-user: fix error propagation in clock_gettime

[Max Filippov]

host_to_target_timespec may return error if target address could not be
locked, but it is ignored.
Propagate return value of host_to_target_timespec to the caller of
clock_gettime.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d51e2a00ee31..52e2f9c16479 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11884,7 +11884,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         struct timespec ts;
         ret = get_errno(clock_gettime(arg1, &ts));
         if (!is_error(ret)) {
-            host_to_target_timespec(arg2, &ts);
+            ret = host_to_target_timespec(arg2, &ts);
         }
         break;
     }
-- 
2.11.0

[Qemu-devel] [PULL 7/8] linux-user: implement clock_settime

[Max Filippov]

This fixes glibc testsuite test rt/tst-clock2.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/syscall.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 52e2f9c16479..924fd68efcdd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11878,6 +11878,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         goto unimplemented_nowarn;
 #endif
 
+#ifdef TARGET_NR_clock_settime
+    case TARGET_NR_clock_settime:
+    {
+        struct timespec ts;
+
+        ret = target_to_host_timespec(&ts, arg2);
+        if (!is_error(ret)) {
+            ret = get_errno(clock_settime(arg1, &ts));
+        }
+        break;
+    }
+#endif
 #ifdef TARGET_NR_clock_gettime
     case TARGET_NR_clock_gettime:
     {
-- 
2.11.0

[Qemu-devel] [PULL 8/8] target/xtensa: linux-user: fix fadvise64 call

[Max Filippov]

fadvise64_64 on xtensa passes advice as the second argument and so must
be handled similar to PPC.

This fixes glibc testsuite tests posix/tst-posix_fadvise and
posix/tst-posix_fadvise64.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 924fd68efcdd..5ef517613577 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11509,7 +11509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 #ifdef TARGET_NR_fadvise64_64
     case TARGET_NR_fadvise64_64:
-#if defined(TARGET_PPC)
+#if defined(TARGET_PPC) || defined(TARGET_XTENSA)
         /* 6 args: fd, advice, offset (high, low), len (high, low) */
         ret = arg2;
         arg2 = arg3;
-- 
2.11.0

Re: [Qemu-devel] [PULL 0/8] xtensa-specific and generic fixes for linux-user

[Peter Maydell]

On 2 April 2018 at 18:13, Max Filippov <jcmvbkbc@gmail.com> wrote:
> Hi Peter,
>
> please pull the following batch of linux-user fixes for 2.12.
>
> The following changes since commit dfe732fb68ef9195517f4f380a477d58a054edc1:
>
>   Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into staging (2018-03-27 16:25:12 +0100)
>
> are available in the git repository at:
>
>   git://github.com/OSLL/qemu-xtensa.git tags/20180402-xtensa
>
> for you to fetch changes up to 64a563dd8dd6ca2661d96a2e4b69f0a5465cab94:
>
>   target/xtensa: linux-user: fix fadvise64 call (2018-04-02 04:15:35 -0700)
>
> ----------------------------------------------------------------
> xtensa-specific fixes for linux-user:
>
> - fix flushing registers for signal processing in call8 and call12 frames;
> - fix PC value for restarted syscalls;
> - fix sysv IPC structures;
> - fix fadvise64 syscall;
>
> generic fixes for linux-user:
>
> - fix QEMU assertion in multithreaded application by calling cpu_copy
>   under clone_lock;
> - fix mq_getsetattr implementation;
> - fix error propagation in clock_gettime;
> - implement clock_settime.
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM