[Qemu-devel] [PATCH 0/6] linux-user improvements

[Qemu-devel] [PATCH 0/6] linux-user improvements

[Richard Henderson]

These patches are available at

  git://repo.or.cz/qemu/rth.git axp-next

and fix several problems detected by the glibc testsuite.


r~



Richard Henderson (6):
  linux-user: Perform more checks on iovec lists
  linux-user: Implement gethostname
  alpha-linux-user: Fix sigaltstack structure definition
  alpha-linux-user: Fix sigaction
  target-alpha: Fix cpu_alpha_init
  linux-user: Fix siginfo handling

 linux-user/alpha/target_signal.h |   7 +-
 linux-user/qemu.h                |   3 +
 linux-user/signal.c              |  81 ++++++++++--------
 linux-user/syscall.c             | 177 +++++++++++++++++++++++++--------------
 linux-user/syscall_defs.h        |   2 +-
 target-alpha/translate.c         |   7 +-
 6 files changed, 178 insertions(+), 99 deletions(-)

-- 
1.7.11.4

[Qemu-devel] [PATCH 1/6] linux-user: Perform more checks on iovec lists

[Richard Henderson]

Validate count between 0 and IOV_MAX.  Limit total length of
operation in the same way the kernel does.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/syscall.c | 162 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 102 insertions(+), 60 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6257a04..ceca04c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1744,55 +1744,96 @@ static abi_long do_getsockopt(int sockfd, int level, int optname,
     return ret;
 }
 
-/* FIXME
- * lock_iovec()/unlock_iovec() have a return code of 0 for success where
- * other lock functions have a return code of 0 for failure.
- */
-static abi_long lock_iovec(int type, struct iovec *vec, abi_ulong target_addr,
-                           int count, int copy)
+static struct iovec *lock_iovec(int type, abi_ulong target_addr,
+                                int count, int copy)
 {
     struct target_iovec *target_vec;
-    abi_ulong base;
+    struct iovec *vec;
+    abi_ulong total_len, max_len;
     int i;
 
-    target_vec = lock_user(VERIFY_READ, target_addr, count * sizeof(struct target_iovec), 1);
-    if (!target_vec)
-        return -TARGET_EFAULT;
-    for(i = 0;i < count; i++) {
-        base = tswapal(target_vec[i].iov_base);
-        vec[i].iov_len = tswapal(target_vec[i].iov_len);
-        if (vec[i].iov_len != 0) {
-            vec[i].iov_base = lock_user(type, base, vec[i].iov_len, copy);
-            /* Don't check lock_user return value. We must call writev even
-               if a element has invalid base address. */
+    if (count == 0) {
+        errno = 0;
+        return NULL;
+    }
+    if (count > IOV_MAX) {
+        errno = EINVAL;
+        return NULL;
+    }
+
+    vec = calloc(count, sizeof(struct iovec));
+    if (vec == NULL) {
+        errno = ENOMEM;
+        return NULL;
+    }
+
+    target_vec = lock_user(VERIFY_READ, target_addr,
+                           count * sizeof(struct target_iovec), 1);
+    if (target_vec == NULL) {
+        errno = EFAULT;
+        goto fail2;
+    }
+
+    /* ??? If host page size > target page size, this will result in a
+       value larger than what we can actually support.  */
+    max_len = 0x7fffffff & TARGET_PAGE_MASK;
+    total_len = 0;
+
+    for (i = 0; i < count; i++) {
+        abi_ulong base = tswapal(target_vec[i].iov_base);
+        abi_long len = tswapal(target_vec[i].iov_len);
+
+        if (len < 0) {
+            errno = EINVAL;
+            goto fail;
+        } else if (len == 0) {
+            /* Zero length pointer is ignored.  */
+            vec[i].iov_base = 0;
         } else {
-            /* zero length pointer is ignored */
-            vec[i].iov_base = NULL;
+            vec[i].iov_base = lock_user(type, base, len, copy);
+            if (!vec[i].iov_base) {
+                errno = EFAULT;
+                goto fail;
+            }
+            if (len > max_len - total_len) {
+                len = max_len - total_len;
+            }
         }
+        vec[i].iov_len = len;
+        total_len += len;
     }
-    unlock_user (target_vec, target_addr, 0);
-    return 0;
+
+    unlock_user(target_vec, target_addr, 0);
+    return vec;
+
+ fail:
+    free(vec);
+ fail2:
+    unlock_user(target_vec, target_addr, 0);
+    return NULL;
 }
 
-static abi_long unlock_iovec(struct iovec *vec, abi_ulong target_addr,
-                             int count, int copy)
+static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
+                         int count, int copy)
 {
     struct target_iovec *target_vec;
-    abi_ulong base;
     int i;
 
-    target_vec = lock_user(VERIFY_READ, target_addr, count * sizeof(struct target_iovec), 1);
-    if (!target_vec)
-        return -TARGET_EFAULT;
-    for(i = 0;i < count; i++) {
-        if (target_vec[i].iov_base) {
-            base = tswapal(target_vec[i].iov_base);
+    target_vec = lock_user(VERIFY_READ, target_addr,
+                           count * sizeof(struct target_iovec), 1);
+    if (target_vec) {
+        for (i = 0; i < count; i++) {
+            abi_ulong base = tswapal(target_vec[i].iov_base);
+            abi_long len = tswapal(target_vec[i].iov_base);
+            if (len < 0) {
+                break;
+            }
             unlock_user(vec[i].iov_base, base, copy ? vec[i].iov_len : 0);
         }
+        unlock_user(target_vec, target_addr, 0);
     }
-    unlock_user (target_vec, target_addr, 0);
 
-    return 0;
+    free(vec);
 }
 
 /* do_socket() Must return target values and target errnos. */
@@ -1888,8 +1929,7 @@ static abi_long do_sendrecvmsg(int fd, abi_ulong target_msg,
         ret = target_to_host_sockaddr(msg.msg_name, tswapal(msgp->msg_name),
                                 msg.msg_namelen);
         if (ret) {
-            unlock_user_struct(msgp, target_msg, send ? 0 : 1);
-            return ret;
+            goto out2;
         }
     } else {
         msg.msg_name = NULL;
@@ -1900,9 +1940,13 @@ static abi_long do_sendrecvmsg(int fd, abi_ulong target_msg,
     msg.msg_flags = tswap32(msgp->msg_flags);
 
     count = tswapal(msgp->msg_iovlen);
-    vec = alloca(count * sizeof(struct iovec));
     target_vec = tswapal(msgp->msg_iov);
-    lock_iovec(send ? VERIFY_READ : VERIFY_WRITE, vec, target_vec, count, send);
+    vec = lock_iovec(send ? VERIFY_READ : VERIFY_WRITE,
+                     target_vec, count, send);
+    if (vec == NULL) {
+        ret = -host_to_target_errno(errno);
+        goto out2;
+    }
     msg.msg_iovlen = count;
     msg.msg_iov = vec;
 
@@ -1932,6 +1976,7 @@ static abi_long do_sendrecvmsg(int fd, abi_ulong target_msg,
 
 out:
     unlock_iovec(vec, target_vec, count, !send);
+out2:
     unlock_user_struct(msgp, target_msg, send ? 0 : 1);
     return ret;
 }
@@ -7186,26 +7231,24 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
     case TARGET_NR_readv:
         {
-            int count = arg3;
-            struct iovec *vec;
-
-            vec = alloca(count * sizeof(struct iovec));
-            if (lock_iovec(VERIFY_WRITE, vec, arg2, count, 0) < 0)
-                goto efault;
-            ret = get_errno(readv(arg1, vec, count));
-            unlock_iovec(vec, arg2, count, 1);
+            struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0);
+            if (vec != NULL) {
+                ret = get_errno(readv(arg1, vec, arg3));
+                unlock_iovec(vec, arg2, arg3, 1);
+            } else {
+                ret = -host_to_target_errno(errno);
+            }
         }
         break;
     case TARGET_NR_writev:
         {
-            int count = arg3;
-            struct iovec *vec;
-
-            vec = alloca(count * sizeof(struct iovec));
-            if (lock_iovec(VERIFY_READ, vec, arg2, count, 1) < 0)
-                goto efault;
-            ret = get_errno(writev(arg1, vec, count));
-            unlock_iovec(vec, arg2, count, 0);
+            struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1);
+            if (vec != NULL) {
+                ret = get_errno(writev(arg1, vec, arg3));
+                unlock_iovec(vec, arg2, arg3, 0);
+            } else {
+                ret = -host_to_target_errno(errno);
+            }
         }
         break;
     case TARGET_NR_getsid:
@@ -8630,14 +8673,13 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_vmsplice
 	case TARGET_NR_vmsplice:
         {
-            int count = arg3;
-            struct iovec *vec;
-
-            vec = alloca(count * sizeof(struct iovec));
-            if (lock_iovec(VERIFY_READ, vec, arg2, count, 1) < 0)
-                goto efault;
-            ret = get_errno(vmsplice(arg1, vec, count, arg4));
-            unlock_iovec(vec, arg2, count, 0);
+            struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1);
+            if (vec != NULL) {
+                ret = get_errno(vmsplice(arg1, vec, arg3, arg4));
+                unlock_iovec(vec, arg2, arg3, 0);
+            } else {
+                ret = -host_to_target_errno(errno);
+            }
         }
         break;
 #endif
-- 
1.7.11.4

[Qemu-devel] [PATCH 2/6] linux-user: Implement gethostname

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/syscall.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ceca04c..925e579 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8866,6 +8866,19 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
     }
 #endif
+#ifdef TARGET_NR_gethostname
+    case TARGET_NR_gethostname:
+    {
+        char *name = lock_user(VERIFY_WRITE, arg1, arg2, 0);
+        if (name) {
+            ret = get_errno(gethostname(name, arg2));
+            unlock_user(name, arg1, arg2);
+        } else {
+            ret = -TARGET_EFAULT;
+        }
+        break;
+    }
+#endif
     default:
     unimplemented:
         gemu_log("qemu: Unsupported syscall: %d\n", num);
-- 
1.7.11.4

[Qemu-devel] [PATCH 3/6] alpha-linux-user: Fix sigaltstack structure definition

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/alpha/target_signal.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/linux-user/alpha/target_signal.h b/linux-user/alpha/target_signal.h
index 94f15f6..d3822da 100644
--- a/linux-user/alpha/target_signal.h
+++ b/linux-user/alpha/target_signal.h
@@ -6,9 +6,10 @@
 /* this struct defines a stack used during syscall handling */
 
 typedef struct target_sigaltstack {
-	abi_ulong ss_sp;
-	abi_long ss_flags;
-	abi_ulong ss_size;
+    abi_ulong ss_sp;
+    int32_t ss_flags;
+    int32_t dummy;
+    abi_ulong ss_size;
 } target_stack_t;
 
 
-- 
1.7.11.4

[Qemu-devel] [PATCH 4/6] alpha-linux-user: Fix sigaction

[Richard Henderson]

Unconditional bswap replaced by __get_user/__put_user.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/signal.c       | 22 ++++++++--------------
 linux-user/syscall_defs.h |  2 +-
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7869147..bf2dfb8 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -586,28 +586,22 @@ int do_sigaction(int sig, const struct target_sigaction *act,
             sig, act, oact);
 #endif
     if (oact) {
-        oact->_sa_handler = tswapal(k->_sa_handler);
-#if defined(TARGET_MIPS) || defined (TARGET_ALPHA)
-        oact->sa_flags = bswap32(k->sa_flags);
-#else
-        oact->sa_flags = tswapal(k->sa_flags);
-#endif
+        __put_user(k->_sa_handler, &oact->_sa_handler);
+        __put_user(k->sa_flags, &oact->sa_flags);
 #if !defined(TARGET_MIPS)
-        oact->sa_restorer = tswapal(k->sa_restorer);
+        __put_user(k->sa_restorer, &oact->sa_restorer);
 #endif
+        /* Not swapped.  */
         oact->sa_mask = k->sa_mask;
     }
     if (act) {
         /* FIXME: This is not threadsafe.  */
-        k->_sa_handler = tswapal(act->_sa_handler);
-#if defined(TARGET_MIPS) || defined (TARGET_ALPHA)
-        k->sa_flags = bswap32(act->sa_flags);
-#else
-        k->sa_flags = tswapal(act->sa_flags);
-#endif
+        __get_user(k->_sa_handler, &act->_sa_handler);
+        __get_user(k->sa_flags, &act->sa_flags);
 #if !defined(TARGET_MIPS)
-        k->sa_restorer = tswapal(act->sa_restorer);
+        __get_user(k->sa_restorer, &act->sa_restorer);
 #endif
+        /* To be swapped in target_to_host_sigset.  */
         k->sa_mask = act->sa_mask;
 
         /* we update the host linux signal state */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a98cbf7..8ca70b9 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -540,7 +540,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
 struct target_old_sigaction {
     abi_ulong _sa_handler;
     abi_ulong sa_mask;
-    abi_ulong sa_flags;
+    int32_t sa_flags;
 };
 
 struct target_rt_sigaction {
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH 4/6] alpha-linux-user: Fix sigaction

[Andreas Färber]

Am 15.09.2012 22:24, schrieb Richard Henderson:
> Unconditional bswap replaced by __get_user/__put_user.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Does that still take care of swapping with the correct size?

The issue for mips was that some oddballs are 32-bit, others long.
Please make sure that mipsn32/mipsn64 (disabled by default due to the
unresolved signal warnings) still compile, too.

Andreas

> ---
>  linux-user/signal.c       | 22 ++++++++--------------
>  linux-user/syscall_defs.h |  2 +-
>  2 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 7869147..bf2dfb8 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -586,28 +586,22 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>              sig, act, oact);
>  #endif
>      if (oact) {
> -        oact->_sa_handler = tswapal(k->_sa_handler);
> -#if defined(TARGET_MIPS) || defined (TARGET_ALPHA)
> -        oact->sa_flags = bswap32(k->sa_flags);
> -#else
> -        oact->sa_flags = tswapal(k->sa_flags);
> -#endif
> +        __put_user(k->_sa_handler, &oact->_sa_handler);
> +        __put_user(k->sa_flags, &oact->sa_flags);
>  #if !defined(TARGET_MIPS)
> -        oact->sa_restorer = tswapal(k->sa_restorer);
> +        __put_user(k->sa_restorer, &oact->sa_restorer);
>  #endif
> +        /* Not swapped.  */
>          oact->sa_mask = k->sa_mask;
>      }
>      if (act) {
>          /* FIXME: This is not threadsafe.  */
> -        k->_sa_handler = tswapal(act->_sa_handler);
> -#if defined(TARGET_MIPS) || defined (TARGET_ALPHA)
> -        k->sa_flags = bswap32(act->sa_flags);
> -#else
> -        k->sa_flags = tswapal(act->sa_flags);
> -#endif
> +        __get_user(k->_sa_handler, &act->_sa_handler);
> +        __get_user(k->sa_flags, &act->sa_flags);
>  #if !defined(TARGET_MIPS)
> -        k->sa_restorer = tswapal(act->sa_restorer);
> +        __get_user(k->sa_restorer, &act->sa_restorer);
>  #endif
> +        /* To be swapped in target_to_host_sigset.  */
>          k->sa_mask = act->sa_mask;
>  
>          /* we update the host linux signal state */
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index a98cbf7..8ca70b9 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -540,7 +540,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>  struct target_old_sigaction {
>      abi_ulong _sa_handler;
>      abi_ulong sa_mask;
> -    abi_ulong sa_flags;
> +    int32_t sa_flags;
>  };
>  
>  struct target_rt_sigaction {
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 4/6] alpha-linux-user: Fix sigaction

[Peter Maydell]

On 16 September 2012 13:58, Andreas Färber <afaerber@suse.de> wrote:
> Am 15.09.2012 22:24, schrieb Richard Henderson:
>> Unconditional bswap replaced by __get_user/__put_user.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>
> Does that still take care of swapping with the correct size?

Yes; they do target-swapping based on the type of pointer you
pass them.


-- PMM

Re: [Qemu-devel] [PATCH 4/6] alpha-linux-user: Fix sigaction

[Richard Henderson]

On 2012-09-16 05:58, Andreas Färber wrote:
> Am 15.09.2012 22:24, schrieb Richard Henderson:
>> Unconditional bswap replaced by __get_user/__put_user.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
> 
> Does that still take care of swapping with the correct size?

Yes, and automatically too.

> The issue for mips was that some oddballs are 32-bit, others long.
> Please make sure that mipsn32/mipsn64 (disabled by default due to the
> unresolved signal warnings) still compile, too.

Alpha is one of the odd balls.


r~

[Qemu-devel] [PATCH 5/6] target-alpha: Fix cpu_alpha_init

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-alpha/translate.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 12de6a3..f998f75 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -3525,6 +3525,7 @@ static const struct cpu_def_t cpu_defs[] = {
 
 CPUAlphaState * cpu_alpha_init (const char *cpu_model)
 {
+    static bool inited;
     AlphaCPU *cpu;
     CPUAlphaState *env;
     int implver, amask, i, max;
@@ -3532,7 +3533,10 @@ CPUAlphaState * cpu_alpha_init (const char *cpu_model)
     cpu = ALPHA_CPU(object_new(TYPE_ALPHA_CPU));
     env = &cpu->env;
 
-    alpha_translate_init();
+    if (!inited) {
+        inited = true;
+        alpha_translate_init();
+    }
 
     /* Default to ev67; no reason not to emulate insns by default.  */
     implver = IMPLVER_21264;
@@ -3549,6 +3553,7 @@ CPUAlphaState * cpu_alpha_init (const char *cpu_model)
     }
     env->implver = implver;
     env->amask = amask;
+    env->cpu_model_str = cpu_model;
 
     qemu_init_vcpu(env);
     return env;
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH 5/6] target-alpha: Fix cpu_alpha_init

[Andreas Färber]

Am 15.09.2012 22:24, schrieb Richard Henderson:
> Signed-off-by: Richard Henderson <rth@twiddle.net>

This is lacking a proper description. I'd be very ashamed if we lost
something so obvious during the QOM conversion. So what's the symptoms here?

Andreas

> ---
>  target-alpha/translate.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index 12de6a3..f998f75 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -3525,6 +3525,7 @@ static const struct cpu_def_t cpu_defs[] = {
>  
>  CPUAlphaState * cpu_alpha_init (const char *cpu_model)
>  {
> +    static bool inited;
>      AlphaCPU *cpu;
>      CPUAlphaState *env;
>      int implver, amask, i, max;
> @@ -3532,7 +3533,10 @@ CPUAlphaState * cpu_alpha_init (const char *cpu_model)
>      cpu = ALPHA_CPU(object_new(TYPE_ALPHA_CPU));
>      env = &cpu->env;
>  
> -    alpha_translate_init();
> +    if (!inited) {
> +        inited = true;
> +        alpha_translate_init();
> +    }
>  
>      /* Default to ev67; no reason not to emulate insns by default.  */
>      implver = IMPLVER_21264;
> @@ -3549,6 +3553,7 @@ CPUAlphaState * cpu_alpha_init (const char *cpu_model)
>      }
>      env->implver = implver;
>      env->amask = amask;
> +    env->cpu_model_str = cpu_model;
>  
>      qemu_init_vcpu(env);
>      return env;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 5/6] target-alpha: Fix cpu_alpha_init

[Richard Henderson]

On 2012-09-16 06:01, Andreas Färber wrote:
> This is lacking a proper description. I'd be very ashamed if we lost
> something so obvious during the QOM conversion. So what's the symptoms here?

The most important thing here is saving cpu_model_str.
The symptom being a SEGV on clone, when cpu_model_str is null.


r~

Re: [Qemu-devel] [PATCH 5/6] target-alpha: Fix cpu_alpha_init

[Andreas Färber]

Am 16.09.2012 18:56, schrieb Richard Henderson:
> On 2012-09-16 06:01, Andreas Färber wrote:
>> This is lacking a proper description. I'd be very ashamed if we lost
>> something so obvious during the QOM conversion. So what's the symptoms here?
> 
> The most important thing here is saving cpu_model_str.
> The symptom being a SEGV on clone, when cpu_model_str is null.

Ack on that. My understanding is it's not a regression but a bugfix.

As for the reentrancy of the translation initialization, I would prefer
that being handled in initfn, but we can first introduce it here and
move it later.

So, if you suggest a wording for the commit message I can apply this as
part of my next CPU pull (assuming that'll be earlier than linux-user).
I also have an include path fix cpu-qom.h -> cpu.h queued, and
cpu_alpha_init() is not yet returning AlphaCPU.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 5/6] target-alpha: Fix cpu_alpha_init

[Richard Henderson]

On 2012-09-16 11:00, Andreas Färber wrote:
> As for the reentrancy of the translation initialization, I would prefer
> that being handled in initfn, but we can first introduce it here and
> move it later.

Sure.  I was monkey copying the arm cpu_init for that.

> So, if you suggest a wording for the commit message I can apply this as
> part of my next CPU pull (assuming that'll be earlier than linux-user).

target-alpha: Initialize env->cpu_model_str

Save the cpu_model_str so that we have a non-null value when
creating a new cpu during clone.

Signed-off-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 6/6] linux-user: Fix siginfo handling

[Richard Henderson]

Compare signal numbers in the proper domain.
Convert all of the fields for SIGIO and SIGCHLD.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/qemu.h    |  3 +++
 linux-user/signal.c  | 59 +++++++++++++++++++++++++++++++++++-----------------
 linux-user/syscall.c |  2 +-
 3 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 69b27d7..8f871eb 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -219,6 +219,9 @@ unsigned long init_guest_space(unsigned long host_start,
 
 #include "qemu-log.h"
 
+/* syscall.c */
+int host_to_target_waitstatus(int status);
+
 /* strace.c */
 void print_syscall(int num,
                    abi_long arg1, abi_long arg2, abi_long arg3,
diff --git a/linux-user/signal.c b/linux-user/signal.c
index bf2dfb8..9842ba6 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -202,46 +202,67 @@ void target_to_host_old_sigset(sigset_t *sigset,
 static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
                                                  const siginfo_t *info)
 {
-    int sig;
-    sig = host_to_target_signal(info->si_signo);
+    int sig = host_to_target_signal(info->si_signo);
     tinfo->si_signo = sig;
     tinfo->si_errno = 0;
     tinfo->si_code = info->si_code;
-    if (sig == SIGILL || sig == SIGFPE || sig == SIGSEGV ||
-        sig == SIGBUS || sig == SIGTRAP) {
-        /* should never come here, but who knows. The information for
-           the target is irrelevant */
+
+    if (sig == TARGET_SIGILL || sig == TARGET_SIGFPE || sig == TARGET_SIGSEGV
+        || sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
+        /* Should never come here, but who knows. The information for
+           the target is irrelevant.  */
         tinfo->_sifields._sigfault._addr = 0;
-    } else if (sig == SIGIO) {
+    } else if (sig == TARGET_SIGIO) {
+        tinfo->_sifields._sigpoll._band = info->si_band;
 	tinfo->_sifields._sigpoll._fd = info->si_fd;
+    } else if (sig == TARGET_SIGCHLD) {
+        tinfo->_sifields._sigchld._pid = info->si_pid;
+        tinfo->_sifields._sigchld._uid = info->si_uid;
+        tinfo->_sifields._sigchld._status
+            = host_to_target_waitstatus(info->si_status);
+        tinfo->_sifields._sigchld._utime = info->si_utime;
+        tinfo->_sifields._sigchld._stime = info->si_stime;
     } else if (sig >= TARGET_SIGRTMIN) {
         tinfo->_sifields._rt._pid = info->si_pid;
         tinfo->_sifields._rt._uid = info->si_uid;
         /* XXX: potential problem if 64 bit */
-        tinfo->_sifields._rt._sigval.sival_ptr =
-            (abi_ulong)(unsigned long)info->si_value.sival_ptr;
+        tinfo->_sifields._rt._sigval.sival_ptr
+            = (abi_ulong)(unsigned long)info->si_value.sival_ptr;
     }
 }
 
 static void tswap_siginfo(target_siginfo_t *tinfo,
                           const target_siginfo_t *info)
 {
-    int sig;
-    sig = info->si_signo;
+    int sig = info->si_signo;
     tinfo->si_signo = tswap32(sig);
     tinfo->si_errno = tswap32(info->si_errno);
     tinfo->si_code = tswap32(info->si_code);
-    if (sig == SIGILL || sig == SIGFPE || sig == SIGSEGV ||
-        sig == SIGBUS || sig == SIGTRAP) {
-        tinfo->_sifields._sigfault._addr =
-            tswapal(info->_sifields._sigfault._addr);
-    } else if (sig == SIGIO) {
-	tinfo->_sifields._sigpoll._fd = tswap32(info->_sifields._sigpoll._fd);
+
+    if (sig == TARGET_SIGILL || sig == TARGET_SIGFPE || sig == TARGET_SIGSEGV
+        || sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
+        tinfo->_sifields._sigfault._addr
+            = tswapal(info->_sifields._sigfault._addr);
+    } else if (sig == TARGET_SIGIO) {
+        tinfo->_sifields._sigpoll._band
+            = tswap32(info->_sifields._sigpoll._band);
+        tinfo->_sifields._sigpoll._fd = tswap32(info->_sifields._sigpoll._fd);
+    } else if (sig == TARGET_SIGCHLD) {
+        tinfo->_sifields._sigchld._pid
+            = tswap32(info->_sifields._sigchld._pid);
+        tinfo->_sifields._sigchld._uid
+            = tswap32(info->_sifields._sigchld._uid);
+        tinfo->_sifields._sigchld._status
+            = tswap32(info->_sifields._sigchld._status);
+        tinfo->_sifields._sigchld._utime
+            = tswapal(info->_sifields._sigchld._utime);
+        tinfo->_sifields._sigchld._stime
+            = tswapal(info->_sifields._sigchld._stime);
     } else if (sig >= TARGET_SIGRTMIN) {
         tinfo->_sifields._rt._pid = tswap32(info->_sifields._rt._pid);
         tinfo->_sifields._rt._uid = tswap32(info->_sifields._rt._uid);
-        tinfo->_sifields._rt._sigval.sival_ptr =
-            tswapal(info->_sifields._rt._sigval.sival_ptr);
+        tinfo->_sifields._rt._sigval.sival_ptr
+            = tswapal(info->_sifields._rt._sigval.sival_ptr);
     }
 }
 
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 925e579..3676c72 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4920,7 +4920,7 @@ static int do_futex(target_ulong uaddr, int op, int val, target_ulong timeout,
 
 /* Map host to target signal numbers for the wait family of syscalls.
    Assume all other status bits are the same.  */
-static int host_to_target_waitstatus(int status)
+int host_to_target_waitstatus(int status)
 {
     if (WIFSIGNALED(status)) {
         return host_to_target_signal(WTERMSIG(status)) | (status & ~0x7f);
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH 6/6] linux-user: Fix siginfo handling

[Andreas Färber]

Am 15.09.2012 22:24, schrieb Richard Henderson:
> Compare signal numbers in the proper domain.
> Convert all of the fields for SIGIO and SIGCHLD.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/qemu.h    |  3 +++
>  linux-user/signal.c  | 59 +++++++++++++++++++++++++++++++++++-----------------
>  linux-user/syscall.c |  2 +-
>  3 files changed, 44 insertions(+), 20 deletions(-)
> 
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 69b27d7..8f871eb 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -219,6 +219,9 @@ unsigned long init_guest_space(unsigned long host_start,
>  
>  #include "qemu-log.h"
>  
> +/* syscall.c */
> +int host_to_target_waitstatus(int status);
> +
>  /* strace.c */
>  void print_syscall(int num,
>                     abi_long arg1, abi_long arg2, abi_long arg3,
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index bf2dfb8..9842ba6 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -202,46 +202,67 @@ void target_to_host_old_sigset(sigset_t *sigset,
>  static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
>                                                   const siginfo_t *info)
>  {
> -    int sig;
> -    sig = host_to_target_signal(info->si_signo);
> +    int sig = host_to_target_signal(info->si_signo);
>      tinfo->si_signo = sig;
>      tinfo->si_errno = 0;
>      tinfo->si_code = info->si_code;
> -    if (sig == SIGILL || sig == SIGFPE || sig == SIGSEGV ||
> -        sig == SIGBUS || sig == SIGTRAP) {
> -        /* should never come here, but who knows. The information for
> -           the target is irrelevant */
> +
> +    if (sig == TARGET_SIGILL || sig == TARGET_SIGFPE || sig == TARGET_SIGSEGV
> +        || sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
> +        /* Should never come here, but who knows. The information for
> +           the target is irrelevant.  */
>          tinfo->_sifields._sigfault._addr = 0;
> -    } else if (sig == SIGIO) {
> +    } else if (sig == TARGET_SIGIO) {
> +        tinfo->_sifields._sigpoll._band = info->si_band;
>  	tinfo->_sifields._sigpoll._fd = info->si_fd;
> +    } else if (sig == TARGET_SIGCHLD) {
> +        tinfo->_sifields._sigchld._pid = info->si_pid;
> +        tinfo->_sifields._sigchld._uid = info->si_uid;
> +        tinfo->_sifields._sigchld._status
> +            = host_to_target_waitstatus(info->si_status);
> +        tinfo->_sifields._sigchld._utime = info->si_utime;
> +        tinfo->_sifields._sigchld._stime = info->si_stime;
>      } else if (sig >= TARGET_SIGRTMIN) {
>          tinfo->_sifields._rt._pid = info->si_pid;
>          tinfo->_sifields._rt._uid = info->si_uid;
>          /* XXX: potential problem if 64 bit */
> -        tinfo->_sifields._rt._sigval.sival_ptr =
> -            (abi_ulong)(unsigned long)info->si_value.sival_ptr;
> +        tinfo->_sifields._rt._sigval.sival_ptr
> +            = (abi_ulong)(unsigned long)info->si_value.sival_ptr;

I don't spot any functional change here ...

>      }
>  }
>  
>  static void tswap_siginfo(target_siginfo_t *tinfo,
>                            const target_siginfo_t *info)
>  {
> -    int sig;
> -    sig = info->si_signo;
> +    int sig = info->si_signo;
>      tinfo->si_signo = tswap32(sig);
>      tinfo->si_errno = tswap32(info->si_errno);
>      tinfo->si_code = tswap32(info->si_code);
> -    if (sig == SIGILL || sig == SIGFPE || sig == SIGSEGV ||
> -        sig == SIGBUS || sig == SIGTRAP) {
> -        tinfo->_sifields._sigfault._addr =
> -            tswapal(info->_sifields._sigfault._addr);
> -    } else if (sig == SIGIO) {
> -	tinfo->_sifields._sigpoll._fd = tswap32(info->_sifields._sigpoll._fd);
> +
> +    if (sig == TARGET_SIGILL || sig == TARGET_SIGFPE || sig == TARGET_SIGSEGV
> +        || sig == TARGET_SIGBUS || sig == TARGET_SIGTRAP) {
> +        tinfo->_sifields._sigfault._addr
> +            = tswapal(info->_sifields._sigfault._addr);
> +    } else if (sig == TARGET_SIGIO) {
> +        tinfo->_sifields._sigpoll._band
> +            = tswap32(info->_sifields._sigpoll._band);
> +        tinfo->_sifields._sigpoll._fd = tswap32(info->_sifields._sigpoll._fd);
> +    } else if (sig == TARGET_SIGCHLD) {
> +        tinfo->_sifields._sigchld._pid
> +            = tswap32(info->_sifields._sigchld._pid);
> +        tinfo->_sifields._sigchld._uid
> +            = tswap32(info->_sifields._sigchld._uid);
> +        tinfo->_sifields._sigchld._status
> +            = tswap32(info->_sifields._sigchld._status);
> +        tinfo->_sifields._sigchld._utime
> +            = tswapal(info->_sifields._sigchld._utime);
> +        tinfo->_sifields._sigchld._stime
> +            = tswapal(info->_sifields._sigchld._stime);
>      } else if (sig >= TARGET_SIGRTMIN) {
>          tinfo->_sifields._rt._pid = tswap32(info->_sifields._rt._pid);
>          tinfo->_sifields._rt._uid = tswap32(info->_sifields._rt._uid);
> -        tinfo->_sifields._rt._sigval.sival_ptr =
> -            tswapal(info->_sifields._rt._sigval.sival_ptr);
> +        tinfo->_sifields._rt._sigval.sival_ptr
> +            = tswapal(info->_sifields._rt._sigval.sival_ptr);

... or here. Does Coding Style mandate this? Would be nice to separate
from functional changes then or to leave out.

Otherwise no functional issue spotted, using TARGET_SIG* after
conversion certainly makes sense. :)

Andreas

>      }
>  }
>  
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 925e579..3676c72 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4920,7 +4920,7 @@ static int do_futex(target_ulong uaddr, int op, int val, target_ulong timeout,
>  
>  /* Map host to target signal numbers for the wait family of syscalls.
>     Assume all other status bits are the same.  */
> -static int host_to_target_waitstatus(int status)
> +int host_to_target_waitstatus(int status)
>  {
>      if (WIFSIGNALED(status)) {
>          return host_to_target_signal(WTERMSIG(status)) | (status & ~0x7f);
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg