[Qemu-devel] [PATCH 0/9] Wine enablement patch set v2

[Qemu-devel] [PATCH 0/9] Wine enablement patch set v2

[Alexander Graf]

Howdy,

It's been a while since I've tried to run wine in QEMU's i386
linux-user target, so I figured I'd give it a go again.

Obviously I've hit a bunch of obstacles, some of which were old,
some of which we only introduced recently.

However, with this patch set I am able to successfully run
Civilization II in wine on my ARM Chromebook:

  http://db.tt/AvHJOKSg

Beware that this patch set is based on Andreas' qom-cpu branch.
If you just want a working git tree to try this out on, check out:

  git://github.com/agraf/qemu.git linux-user-x86-tls-v1


Alex

v1 -> v2:

  - user Peter's patch for ARM is_write support
  - add h2g_unchecked() macro
  - rewrite cpu reset on cpu clone code
  - rewrite sendrcvmsg() patch


Alexander Graf (8):
  linux-user: fix segmentation fault passing with h2g(x) != x
  linux-user: Reset copied CPUs in cpu_copy() always
  linux-user: Clean up sendrecvmsg message parsing
  linux-user: Fix epoll on ARM hosts
  linux-user: Add i386 TLS setter
  linux-user: Enable NPTL for i386
  linux-user: Default to 64k guest base
  linux-user: Unlock mmap_lock when resuming guest from page_unprotect

Peter Maydell (1):
  user-exec.c: Set is_write correctly in the ARM cpu_signal_handler()

 configure                    |    1 +
 exec.c                       |    4 ++
 include/exec/cpu-all.h       |    8 ++++-
 linux-user/i386/target_cpu.h |   12 ++++++-
 linux-user/main.c            |    6 ++--
 linux-user/syscall.c         |   30 +++++++++++--------
 linux-user/syscall_defs.h    |   63 ++++++++++++++++++++++++++++-------------
 translate-all.c              |   10 ++++--
 user-exec.c                  |   12 ++++++-
 9 files changed, 101 insertions(+), 45 deletions(-)

[Qemu-devel] [PATCH 1/9] linux-user: fix segmentation fault passing with h2g(x) != x

[Alexander Graf]

When forwarding a segmentation fault into the guest process, we were passing
the host's address directly into the guest process's signal descriptor.

That obviously confused the guest process, since it didn't know what to make
of the (usually 32-bit truncated) address. Passing in h2g(address) makes the
guest process a lot happier.

To make the code more obvious, introduce a h2g_nocheck() macro that does the
same as h2g(), but allows us to convert addresses that may be outside of guest
mapped range into the guest's view of address space.

This fixes java running in arm-linux-user for me.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 include/exec/cpu-all.h |    8 ++++++--
 user-exec.c            |    4 ++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 6499cd0..320e3c4 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -209,11 +209,15 @@ extern unsigned long reserved_va;
 })
 #endif
 
-#define h2g(x) ({ \
+#define h2g_nocheck(x) ({ \
     unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
+    (abi_ulong)__ret; \
+})
+
+#define h2g(x) ({ \
     /* Check if given address fits target address space */ \
     assert(h2g_valid(x)); \
-    (abi_ulong)__ret; \
+    h2g_nocheck(x); \
 })
 
 #define saddr(x) g2h(x)
diff --git a/user-exec.c b/user-exec.c
index 26cde7c..ed15f1e 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -94,6 +94,10 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
         return 1;
     }
 
+    /* Convert forcefully to guest address space, invalid addresses
+       are still valid segv ones */
+    address = h2g_nocheck(address);
+
     env = current_cpu->env_ptr;
     /* see if it is an MMU fault */
     ret = cpu_handle_mmu_fault(env, address, is_write, MMU_USER_IDX);
-- 
1.6.0.2

[Qemu-devel] [PATCH 2/9] user-exec.c: Set is_write correctly in the ARM cpu_signal_handler()

[Alexander Graf]

From: Peter Maydell <peter.maydell@linaro.org>

In the ARM implementation of cpu_signal_handler(), set is_write
correctly using the FSR value which the kernel passes us in the
error_code field of uc_mcontext. Since the WnR bit of the FSR was
only introduced in ARMv6, this means that v5 cores will continue
to behave as before this patch, but they are not really supported
as hosts for linux-user mode anyway since they do not have the
modern behaviour for unaligned accesses.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 user-exec.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/user-exec.c b/user-exec.c
index ed15f1e..82bfa66 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -20,6 +20,7 @@
 #include "cpu.h"
 #include "disas/disas.h"
 #include "tcg.h"
+#include "qemu/bitops.h"
 
 #undef EAX
 #undef ECX
@@ -446,8 +447,11 @@ int cpu_signal_handler(int host_signum, void *pinfo,
 #else
     pc = uc->uc_mcontext.arm_pc;
 #endif
-    /* XXX: compute is_write */
-    is_write = 0;
+
+    /* error_code is the FSR value, in which bit 11 is WnR (assuming a v6 or
+     * later processor; on v5 we will always report this as a read).
+     */
+    is_write = extract32(uc->uc_mcontext.error_code, 11, 1);
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              is_write,
                              &uc->uc_sigmask, puc);
-- 
1.6.0.2

[Qemu-devel] [PATCH 3/9] linux-user: Reset copied CPUs in cpu_copy() always

[Alexander Graf]

When a new thread gets created, we need to reset non arch specific state to
get the new CPU into clean state.

However this reset should happen before the arch specific CPU contents get
copied over. Otherwise we end up having clean reset state in our newly created
thread.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 exec.c               |    4 ++++
 linux-user/syscall.c |    3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index 4e20143..7eaa3a0 100644
--- a/exec.c
+++ b/exec.c
@@ -638,6 +638,10 @@ CPUArchState *cpu_copy(CPUArchState *env)
     CPUWatchpoint *wp;
 #endif
 
+    /* Reset non arch specific state */
+    cpu_reset(ENV_GET_CPU(new_env));
+
+    /* Copy arch specific state into the new CPU */
     memcpy(new_env, env, sizeof(CPUArchState));
 
     /* Clone all break/watchpoints.
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 433d3ba..89b7698 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4234,9 +4234,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
         init_task_state(ts);
         /* we create a new CPU instance. */
         new_env = cpu_copy(env);
-#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
-        cpu_reset(ENV_GET_CPU(new_env));
-#endif
         /* Init regs that differ from the parent.  */
         cpu_clone_regs(new_env, newsp);
         new_env->opaque = ts;
-- 
1.6.0.2

[Qemu-devel] [PATCH 4/9] linux-user: Clean up sendrecvmsg message parsing

[Alexander Graf]

The cmsg handling in the linux-user code is very hard to read as it tries
to follow glibc's unreadable code closely. Let's clean it up, converting
all helpers into static inline functions, so that QEMU developers have a
chance to understand what's going on.

While at it, this also allows us to make the next target header lookup more
obvious and GUEST_BASE safe. We only compare host mapped pointers between each
other now.

During the rewrite I also saw that we never advance our target cmsg structure
to the next one. This behavior is identical to the previous code, but looks
very bogus to me.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/syscall.c      |   25 ++++++++++++-------
 linux-user/syscall_defs.h |   58 ++++++++++++++++++++++++++++++--------------
 2 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 89b7698..8eb5c31 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1120,6 +1120,7 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
     abi_long msg_controllen;
     abi_ulong target_cmsg_addr;
     struct target_cmsghdr *target_cmsg;
+    struct target_cmsghdr *first_target_cmsg;
     socklen_t space = 0;
     
     msg_controllen = tswapal(target_msgh->msg_controllen);
@@ -1129,13 +1130,14 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
     target_cmsg = lock_user(VERIFY_READ, target_cmsg_addr, msg_controllen, 1);
     if (!target_cmsg)
         return -TARGET_EFAULT;
+    first_target_cmsg = target_cmsg;
 
     while (cmsg && target_cmsg) {
         void *data = CMSG_DATA(cmsg);
-        void *target_data = TARGET_CMSG_DATA(target_cmsg);
+        void *target_data = target_cmsg_data(target_cmsg);
 
         int len = tswapal(target_cmsg->cmsg_len)
-                  - TARGET_CMSG_ALIGN(sizeof (struct target_cmsghdr));
+                  - target_cmsg_align(sizeof (struct target_cmsghdr));
 
         space += CMSG_SPACE(len);
         if (space > msgh->msg_controllen) {
@@ -1161,7 +1163,8 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
         }
 
         cmsg = CMSG_NXTHDR(msgh, cmsg);
-        target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg);
+        target_cmsg = target_cmsg_nxthdr(target_msgh, target_cmsg,
+                                         first_target_cmsg);
     }
     unlock_user(target_cmsg, target_cmsg_addr, 0);
  the_end:
@@ -1176,6 +1179,7 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
     abi_long msg_controllen;
     abi_ulong target_cmsg_addr;
     struct target_cmsghdr *target_cmsg;
+    struct target_cmsghdr *first_target_cmsg;
     socklen_t space = 0;
 
     msg_controllen = tswapal(target_msgh->msg_controllen);
@@ -1183,25 +1187,27 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
         goto the_end;
     target_cmsg_addr = tswapal(target_msgh->msg_control);
     target_cmsg = lock_user(VERIFY_WRITE, target_cmsg_addr, msg_controllen, 0);
-    if (!target_cmsg)
+    if (!target_cmsg) {
         return -TARGET_EFAULT;
+    }
+    first_target_cmsg = target_cmsg;
 
     while (cmsg && target_cmsg) {
         void *data = CMSG_DATA(cmsg);
-        void *target_data = TARGET_CMSG_DATA(target_cmsg);
+        void *target_data = target_cmsg_data(target_cmsg);
 
         int len = cmsg->cmsg_len - CMSG_ALIGN(sizeof (struct cmsghdr));
 
-        space += TARGET_CMSG_SPACE(len);
+        space += target_cmsg_space(len);
         if (space > msg_controllen) {
-            space -= TARGET_CMSG_SPACE(len);
+            space -= target_cmsg_space(len);
             gemu_log("Target cmsg overflow\n");
             break;
         }
 
         target_cmsg->cmsg_level = tswap32(cmsg->cmsg_level);
         target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type);
-        target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len));
+        target_cmsg->cmsg_len = tswapal(target_cmsg_len(len));
 
         if ((cmsg->cmsg_level == TARGET_SOL_SOCKET) &&
                                 (cmsg->cmsg_type == SCM_RIGHTS)) {
@@ -1228,7 +1234,8 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
         }
 
         cmsg = CMSG_NXTHDR(msgh, cmsg);
-        target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg);
+        target_cmsg = target_cmsg_nxthdr(target_msgh, target_cmsg,
+                                         first_target_cmsg);
     }
     unlock_user(target_cmsg, target_cmsg_addr, space);
  the_end:
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 92c01a9..84da7f7 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -199,26 +199,46 @@ struct target_cmsghdr {
     int          cmsg_type;
 };
 
-#define TARGET_CMSG_DATA(cmsg) ((unsigned char *) ((struct target_cmsghdr *) (cmsg) + 1))
-#define TARGET_CMSG_NXTHDR(mhdr, cmsg) __target_cmsg_nxthdr (mhdr, cmsg)
-#define TARGET_CMSG_ALIGN(len) (((len) + sizeof (abi_long) - 1) \
-                               & (size_t) ~(sizeof (abi_long) - 1))
-#define TARGET_CMSG_SPACE(len) (TARGET_CMSG_ALIGN (len) \
-                               + TARGET_CMSG_ALIGN (sizeof (struct target_cmsghdr)))
-#define TARGET_CMSG_LEN(len)   (TARGET_CMSG_ALIGN (sizeof (struct target_cmsghdr)) + (len))
-
-static __inline__ struct target_cmsghdr *
-__target_cmsg_nxthdr (struct target_msghdr *__mhdr, struct target_cmsghdr *__cmsg)
+static inline unsigned char *target_cmsg_data(struct target_cmsghdr *cmsg)
 {
-  struct target_cmsghdr *__ptr;
-
-  __ptr = (struct target_cmsghdr *)((unsigned char *) __cmsg
-                                    + TARGET_CMSG_ALIGN (tswapal(__cmsg->cmsg_len)));
-  if ((unsigned long)((char *)(__ptr+1) - (char *)(size_t)tswapal(__mhdr->msg_control))
-      > tswapal(__mhdr->msg_controllen))
-    /* No more entries.  */
-    return (struct target_cmsghdr *)0;
-  return __cmsg;
+    return ((unsigned char *) ((struct target_cmsghdr *) (cmsg) + 1));
+}
+
+static inline abi_ulong target_cmsg_align(abi_ulong len)
+{
+    return ((len) + sizeof(abi_long) - 1) & (size_t)~(sizeof(abi_long) - 1);
+}
+
+static inline abi_ulong target_cmsg_len(abi_ulong len)
+{
+    return target_cmsg_align(sizeof (struct target_cmsghdr)) + len;
+}
+
+static inline abi_ulong target_cmsg_space(abi_ulong len)
+{
+    return target_cmsg_len(target_cmsg_align(len));
+}
+
+static inline struct target_cmsghdr *target_cmsg_nxthdr(
+        struct target_msghdr *mhdr, struct target_cmsghdr *cmsg,
+        struct target_cmsghdr *first_cmsg)
+{
+    abi_ulong len;
+
+    /* length of all entries since the first one */
+    len = ((uintptr_t)first_cmsg - (uintptr_t)cmsg);
+    /* plus length of this header */
+    len += sizeof(struct target_cmsghdr);
+    /* plus length of this entry's data */
+    len += target_cmsg_align(tswapal(cmsg->cmsg_len));
+
+    /* no more entries */
+    if (tswapal(mhdr->msg_controllen) < len) {
+        return NULL;
+    }
+
+    /* XXX: return this header, this looks broken */
+    return cmsg;
 }
 
 
-- 
1.6.0.2

[Qemu-devel] [PATCH 5/9] linux-user: Fix epoll on ARM hosts

[Alexander Graf]

The epoll emulation uses data structures without packing them, so the
compiler might choose to add padding inside.

This patch makes the most offending one (target_epoll_event) a packed
structure to make sure we don't pad it by accident. ARM would pad it,
so declare the padding mandatory for ARM targets.

This fixes i386-on-ARM epoll emulation for me.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/syscall_defs.h |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 84da7f7..c99aed6 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2454,8 +2454,11 @@ typedef union target_epoll_data {
 
 struct target_epoll_event {
     uint32_t events;
+#ifdef TARGET_ARM
+    uint32_t __pad;
+#endif
     target_epoll_data_t data;
-};
+} QEMU_PACKED;
 #endif
 struct target_rlimit64 {
     uint64_t rlim_cur;
-- 
1.6.0.2

[Qemu-devel] [PATCH 6/9] linux-user: Add i386 TLS setter

[Alexander Graf]

We can easily set the TLS on i386. Add code to do so.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/i386/target_cpu.h |   12 ++++++++++--
 linux-user/syscall.c         |    2 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/linux-user/i386/target_cpu.h b/linux-user/i386/target_cpu.h
index abcac79..1170d84 100644
--- a/linux-user/i386/target_cpu.h
+++ b/linux-user/i386/target_cpu.h
@@ -28,6 +28,14 @@ static inline void cpu_clone_regs(CPUX86State *env, target_ulong newsp)
     env->regs[R_EAX] = 0;
 }
 
-/* TODO: need to implement cpu_set_tls() */
+#if defined(TARGET_ABI32)
+abi_long do_set_thread_area(CPUX86State *env, abi_ulong ptr);
 
-#endif
+static inline void cpu_set_tls(CPUX86State *env, target_ulong newtls)
+{
+    do_set_thread_area(env, newtls);
+    cpu_x86_load_seg(env, R_GS, env->segs[R_GS].selector);
+}
+#endif /* defined(TARGET_ABI32) */
+
+#endif /* !defined(TARGET_CPU_H) */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8eb5c31..e7d5143 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3983,7 +3983,7 @@ static abi_long do_modify_ldt(CPUX86State *env, int func, abi_ulong ptr,
 }
 
 #if defined(TARGET_I386) && defined(TARGET_ABI32)
-static abi_long do_set_thread_area(CPUX86State *env, abi_ulong ptr)
+abi_long do_set_thread_area(CPUX86State *env, abi_ulong ptr)
 {
     uint64_t *gdt_table = g2h(env->gdt.base);
     struct target_modify_ldt_ldt_s ldt_info;
-- 
1.6.0.2

[Qemu-devel] [PATCH 7/9] linux-user: Enable NPTL for i386

[Alexander Graf]

The i386 target is now able to properly handle NPTL. Enable it.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 configure |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 0e0adde..61f2770 100755
--- a/configure
+++ b/configure
@@ -4156,6 +4156,7 @@ TARGET_ABI_DIR=""
 
 case "$target_name" in
   i386)
+    target_nptl="yes"
   ;;
   x86_64)
     TARGET_BASE_ARCH=i386
-- 
1.6.0.2

[Qemu-devel] [PATCH 8/9] linux-user: Default to 64k guest base

[Alexander Graf]

Most kernels these days have protection code in place to forbid user space
to access low memory. The barrier varies between architectures though.

For this purpose we have the guest base option that allows us to offset
guest visible memory from host memory, so that the guest process thinks
it can access lower memory than it really can access.

Set the default for the guest base to 64k which should be good enough on
any host system.

This fixes running i386 wine on ARM for me.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 7f15d3d..a246cff 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -45,8 +45,8 @@ envlist_t *envlist;
 const char *cpu_model;
 unsigned long mmap_min_addr;
 #if defined(CONFIG_USE_GUEST_BASE)
-unsigned long guest_base;
-int have_guest_base;
+unsigned long guest_base = 64 * 1024;
+int have_guest_base = 1;
 #if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
 /*
  * When running 32-on-64 we should make sure we can fit all of the possible
@@ -3294,7 +3294,7 @@ static void handle_arg_cpu(const char *arg)
 static void handle_arg_guest_base(const char *arg)
 {
     guest_base = strtol(arg, NULL, 0);
-    have_guest_base = 1;
+    have_guest_base = guest_base ? 1 : 0;
 }
 
 static void handle_arg_reserved_va(const char *arg)
-- 
1.6.0.2

[Qemu-devel] [PATCH 9/9] linux-user: Unlock mmap_lock when resuming guest from page_unprotect

[Alexander Graf]

The page_unprotect() function is running everything locked. Before every
potential exit path of the function mmap_unlock() gets called to make sure
we don't leak the lock.

However, the function calls tb_invalidate_phys_page() which again can
exit a signal through longjmp, leaving our mmap_unlock() attempts in vain.

Add a hint to tb_invalidate_phys_page() that we need to unlock before we
can leave back into guest context, so that we don't leak the lock.

This fixes 16-bit i386 wine programs running in linux-user for me.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 translate-all.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index e8683d2..3b5fc7c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1148,7 +1148,8 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
 
 #if !defined(CONFIG_SOFTMMU)
 static void tb_invalidate_phys_page(tb_page_addr_t addr,
-                                    uintptr_t pc, void *puc)
+                                    uintptr_t pc, void *puc,
+                                    bool locked)
 {
     TranslationBlock *tb;
     PageDesc *p;
@@ -1206,6 +1207,9 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
            itself */
         cpu->current_tb = NULL;
         tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+        if (locked) {
+            mmap_unlock();
+        }
         cpu_resume_from_signal(env, puc);
     }
 #endif
@@ -1723,7 +1727,7 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
         if (!(p->flags & PAGE_WRITE) &&
             (flags & PAGE_WRITE) &&
             p->first_tb) {
-            tb_invalidate_phys_page(addr, 0, NULL);
+            tb_invalidate_phys_page(addr, 0, NULL, false);
         }
         p->flags = flags;
     }
@@ -1818,7 +1822,7 @@ int page_unprotect(target_ulong address, uintptr_t pc, void *puc)
 
             /* and since the content will be modified, we must invalidate
                the corresponding translated code. */
-            tb_invalidate_phys_page(addr, pc, puc);
+            tb_invalidate_phys_page(addr, pc, puc, true);
 #ifdef DEBUG_TB_CHECK
             tb_invalidate_check(addr);
 #endif
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 8/9] linux-user: Default to 64k guest base

[Riku Voipio]

On 6 July 2013 15:17, Alexander Graf <agraf@suse.de> wrote:
> Most kernels these days have protection code in place to forbid user space
> to access low memory. The barrier varies between architectures though.
>
> For this purpose we have the guest base option that allows us to offset
> guest visible memory from host memory, so that the guest process thinks
> it can access lower memory than it really can access.
>
> Set the default for the guest base to 64k which should be good enough on
> any host system.

> This fixes running i386 wine on ARM for me.

It also makes qemu-x86_64 segfault busybox running on x86_64 host:

x86_64-linux-user/qemu-x86_64 qemu-smoke/amd64/busybox ls -l
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
./smoke-test: line 28: 16061 Segmentation fault      (core dumped)
$qemudir/x86_64-linux-user/qemu-x86_64 $testdir/amd64/busybox $@

Riku

> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  linux-user/main.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 7f15d3d..a246cff 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -45,8 +45,8 @@ envlist_t *envlist;
>  const char *cpu_model;
>  unsigned long mmap_min_addr;
>  #if defined(CONFIG_USE_GUEST_BASE)
> -unsigned long guest_base;
> -int have_guest_base;
> +unsigned long guest_base = 64 * 1024;
> +int have_guest_base = 1;
>  #if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>  /*
>   * When running 32-on-64 we should make sure we can fit all of the possible
> @@ -3294,7 +3294,7 @@ static void handle_arg_cpu(const char *arg)
>  static void handle_arg_guest_base(const char *arg)
>  {
>      guest_base = strtol(arg, NULL, 0);
> -    have_guest_base = 1;
> +    have_guest_base = guest_base ? 1 : 0;
>  }
>
>  static void handle_arg_reserved_va(const char *arg)
> --
> 1.6.0.2
>

Re: [Qemu-devel] [PATCH 4/9] linux-user: Clean up sendrecvmsg message parsing

[Riku Voipio]

On 6 July 2013 15:17, Alexander Graf <agraf@suse.de> wrote:
> The cmsg handling in the linux-user code is very hard to read as it tries
> to follow glibc's unreadable code closely. Let's clean it up, converting
> all helpers into static inline functions, so that QEMU developers have a
> chance to understand what's going on.
>
> While at it, this also allows us to make the next target header lookup more
> obvious and GUEST_BASE safe. We only compare host mapped pointers between each
> other now.
>
> During the rewrite I also saw that we never advance our target cmsg structure
> to the next one. This behavior is identical to the previous code, but looks
> very bogus to me.

recvmsg01 and sendmsg01 both segfault (arm target and x86_64 host)
with both current linux-user code and after this patch. So there is
more to fix here. On the bright side this patch is not an regression
:)

> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  linux-user/syscall.c      |   25 ++++++++++++-------
>  linux-user/syscall_defs.h |   58 ++++++++++++++++++++++++++++++--------------
>  2 files changed, 55 insertions(+), 28 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 89b7698..8eb5c31 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1120,6 +1120,7 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
>      abi_long msg_controllen;
>      abi_ulong target_cmsg_addr;
>      struct target_cmsghdr *target_cmsg;
> +    struct target_cmsghdr *first_target_cmsg;
>      socklen_t space = 0;
>
>      msg_controllen = tswapal(target_msgh->msg_controllen);
> @@ -1129,13 +1130,14 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
>      target_cmsg = lock_user(VERIFY_READ, target_cmsg_addr, msg_controllen, 1);
>      if (!target_cmsg)
>          return -TARGET_EFAULT;
> +    first_target_cmsg = target_cmsg;
>
>      while (cmsg && target_cmsg) {
>          void *data = CMSG_DATA(cmsg);
> -        void *target_data = TARGET_CMSG_DATA(target_cmsg);
> +        void *target_data = target_cmsg_data(target_cmsg);
>
>          int len = tswapal(target_cmsg->cmsg_len)
> -                  - TARGET_CMSG_ALIGN(sizeof (struct target_cmsghdr));
> +                  - target_cmsg_align(sizeof (struct target_cmsghdr));
>
>          space += CMSG_SPACE(len);
>          if (space > msgh->msg_controllen) {
> @@ -1161,7 +1163,8 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
>          }
>
>          cmsg = CMSG_NXTHDR(msgh, cmsg);
> -        target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg);
> +        target_cmsg = target_cmsg_nxthdr(target_msgh, target_cmsg,
> +                                         first_target_cmsg);
>      }
>      unlock_user(target_cmsg, target_cmsg_addr, 0);
>   the_end:
> @@ -1176,6 +1179,7 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
>      abi_long msg_controllen;
>      abi_ulong target_cmsg_addr;
>      struct target_cmsghdr *target_cmsg;
> +    struct target_cmsghdr *first_target_cmsg;
>      socklen_t space = 0;
>
>      msg_controllen = tswapal(target_msgh->msg_controllen);
> @@ -1183,25 +1187,27 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
>          goto the_end;
>      target_cmsg_addr = tswapal(target_msgh->msg_control);
>      target_cmsg = lock_user(VERIFY_WRITE, target_cmsg_addr, msg_controllen, 0);
> -    if (!target_cmsg)
> +    if (!target_cmsg) {
>          return -TARGET_EFAULT;
> +    }
> +    first_target_cmsg = target_cmsg;
>
>      while (cmsg && target_cmsg) {
>          void *data = CMSG_DATA(cmsg);
> -        void *target_data = TARGET_CMSG_DATA(target_cmsg);
> +        void *target_data = target_cmsg_data(target_cmsg);
>
>          int len = cmsg->cmsg_len - CMSG_ALIGN(sizeof (struct cmsghdr));
>
> -        space += TARGET_CMSG_SPACE(len);
> +        space += target_cmsg_space(len);
>          if (space > msg_controllen) {
> -            space -= TARGET_CMSG_SPACE(len);
> +            space -= target_cmsg_space(len);
>              gemu_log("Target cmsg overflow\n");
>              break;
>          }
>
>          target_cmsg->cmsg_level = tswap32(cmsg->cmsg_level);
>          target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type);
> -        target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len));
> +        target_cmsg->cmsg_len = tswapal(target_cmsg_len(len));
>
>          if ((cmsg->cmsg_level == TARGET_SOL_SOCKET) &&
>                                  (cmsg->cmsg_type == SCM_RIGHTS)) {
> @@ -1228,7 +1234,8 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
>          }
>
>          cmsg = CMSG_NXTHDR(msgh, cmsg);
> -        target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg);
> +        target_cmsg = target_cmsg_nxthdr(target_msgh, target_cmsg,
> +                                         first_target_cmsg);
>      }
>      unlock_user(target_cmsg, target_cmsg_addr, space);
>   the_end:
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 92c01a9..84da7f7 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -199,26 +199,46 @@ struct target_cmsghdr {
>      int          cmsg_type;
>  };
>
> -#define TARGET_CMSG_DATA(cmsg) ((unsigned char *) ((struct target_cmsghdr *) (cmsg) + 1))
> -#define TARGET_CMSG_NXTHDR(mhdr, cmsg) __target_cmsg_nxthdr (mhdr, cmsg)
> -#define TARGET_CMSG_ALIGN(len) (((len) + sizeof (abi_long) - 1) \
> -                               & (size_t) ~(sizeof (abi_long) - 1))
> -#define TARGET_CMSG_SPACE(len) (TARGET_CMSG_ALIGN (len) \
> -                               + TARGET_CMSG_ALIGN (sizeof (struct target_cmsghdr)))
> -#define TARGET_CMSG_LEN(len)   (TARGET_CMSG_ALIGN (sizeof (struct target_cmsghdr)) + (len))
> -
> -static __inline__ struct target_cmsghdr *
> -__target_cmsg_nxthdr (struct target_msghdr *__mhdr, struct target_cmsghdr *__cmsg)
> +static inline unsigned char *target_cmsg_data(struct target_cmsghdr *cmsg)
>  {
> -  struct target_cmsghdr *__ptr;
> -
> -  __ptr = (struct target_cmsghdr *)((unsigned char *) __cmsg
> -                                    + TARGET_CMSG_ALIGN (tswapal(__cmsg->cmsg_len)));
> -  if ((unsigned long)((char *)(__ptr+1) - (char *)(size_t)tswapal(__mhdr->msg_control))
> -      > tswapal(__mhdr->msg_controllen))
> -    /* No more entries.  */
> -    return (struct target_cmsghdr *)0;
> -  return __cmsg;
> +    return ((unsigned char *) ((struct target_cmsghdr *) (cmsg) + 1));
> +}
> +
> +static inline abi_ulong target_cmsg_align(abi_ulong len)
> +{
> +    return ((len) + sizeof(abi_long) - 1) & (size_t)~(sizeof(abi_long) - 1);
> +}
> +
> +static inline abi_ulong target_cmsg_len(abi_ulong len)
> +{
> +    return target_cmsg_align(sizeof (struct target_cmsghdr)) + len;
> +}
> +
> +static inline abi_ulong target_cmsg_space(abi_ulong len)
> +{
> +    return target_cmsg_len(target_cmsg_align(len));
> +}
> +
> +static inline struct target_cmsghdr *target_cmsg_nxthdr(
> +        struct target_msghdr *mhdr, struct target_cmsghdr *cmsg,
> +        struct target_cmsghdr *first_cmsg)
> +{
> +    abi_ulong len;
> +
> +    /* length of all entries since the first one */
> +    len = ((uintptr_t)first_cmsg - (uintptr_t)cmsg);
> +    /* plus length of this header */
> +    len += sizeof(struct target_cmsghdr);
> +    /* plus length of this entry's data */
> +    len += target_cmsg_align(tswapal(cmsg->cmsg_len));
> +
> +    /* no more entries */
> +    if (tswapal(mhdr->msg_controllen) < len) {
> +        return NULL;
> +    }
> +
> +    /* XXX: return this header, this looks broken */
> +    return cmsg;
>  }
>
>
> --
> 1.6.0.2
>