[Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Nathan Froyd]

This patch series adds NPTL support in Linux user-mode emulation to
32-bit PowerPC targets.

The main complication comes from implementing atomic instructions
properly.  We chose to implement a simplistic model:

- reserved loads record the value loaded;

- conditional stores check that the memory at the effective address
  contains the value loaded by the previous reserved load, in addition
  to all other checks.  if so, the store succeeds; otherwise, it fails.

It is possible to implement something more sophisticated using mprotect:

- reserved loads write-protect the page from which the value is loaded;

- regular stores to the page (through SIGSEGV handling) remove the write
  protection (which is roughly how the architecture really works);

- conditional stores fail if the page was not write-protected, in
  addition to all other checks.  If the store succeeds, then the page is
  unprotected.

but the simple scheme works well enough and should be somewhat faster.
The simple scheme is what's already done for system mode, too; it's even
slightly dumber in system mode because we don't check for equality of
values.

The patch series has been tested against the glibc testsuite, where it
passes a good chunk (90%+) of the testsuite.  The other 10% are
basically things that are not going to work in QEMU anytime soon
(e.g. sharing futexes between multiple processes, using clone(2)
directly, etc.).  I should note that proper testing requires a patch to
use a correct exit status for uncaught signals; such a patch has been
posted to this list before by Riku Voipio.  (I have a different local
version that I used instead.)  Testing with recent glibc also requires
adding support for private futexes and a few other futex operations;
again, a patch for this has been posted by Riku and I used a slightly
different local version.

-Nathan

Nathan Froyd (7):
  linux-user: initialize mmap_mutex properly
  target-ppc: fix cpu_clone_regs
  target-ppc: add cpu_set_tls
  target-ppc: retain l{w,d}arx loaded value
  target-ppc: add exceptions for conditional stores
  linux-user: handle POWERPC_EXCP_STCX
  enable NPTL for ppc-linux-user targets in configure

 configure              |    2 +
 linux-user/main.c      |   68 ++++++++++++++++++++++++++++++++++++++
 linux-user/mmap.c      |    2 +-
 target-ppc/cpu.h       |   24 +++++++++++--
 target-ppc/helper.c    |    2 +-
 target-ppc/machine.c   |    4 +-
 target-ppc/op_helper.c |    4 +-
 target-ppc/translate.c |   84 ++++++++++++++++++++++++++++++++++--------------
 8 files changed, 156 insertions(+), 34 deletions(-)

[Qemu-devel] [PATCH 1/7] linux-user: initialize mmap_mutex properly

[Nathan Froyd]

We initialize mmap_mutex in any child threads/processes, but we need to
correctly statically initialize it for the original process.

Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
---
 linux-user/mmap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 6f300a0..a21b558 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -36,7 +36,7 @@
 //#define DEBUG_MMAP
 
 #if defined(USE_NPTL)
-pthread_mutex_t mmap_mutex;
+pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER;
 static int __thread mmap_lock_count;
 
 void mmap_lock(void)
-- 
1.6.3.2

[Qemu-devel] [PATCH 2/7] target-ppc: fix cpu_clone_regs

[Nathan Froyd]

We only need to make sure that the clone syscall looks like it
succeeded, not clobber 60% of the register set.

Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
---
 target-ppc/cpu.h |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 0e8b49f..f977168 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -818,11 +818,9 @@ static inline int cpu_mmu_index (CPUState *env)
 #if defined(CONFIG_USER_ONLY)
 static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 {
-    int i;
     if (newsp)
         env->gpr[1] = newsp;
-    for (i = 7; i < 32; i++)
-        env->gpr[i] = 0;
+    env->gpr[3] = 0;
 }
 #endif
 
-- 
1.6.3.2

[Qemu-devel] [PATCH 3/7] target-ppc: add cpu_set_tls

[Nathan Froyd]

Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
---
 target-ppc/cpu.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index f977168..3fa1654 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1452,4 +1452,15 @@ static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
     *flags = env->hflags;
 }
 
+static inline void cpu_set_tls(CPUState *env, target_ulong newtls)
+{
+#if defined(TARGET_PPC64)
+    /* The kernel checks TIF_32BIT here; we don't support loading 32-bit
+       binaries on PPC64 yet. */
+    env->gpr[13] = newtls;
+#else
+    env->gpr[2] = newtls;
+#endif
+}
+
 #endif /* !defined (__CPU_PPC_H__) */
-- 
1.6.3.2

[Qemu-devel] [PATCH 4/7] target-ppc: retain l{w,d}arx loaded value

[Nathan Froyd]

We do this so we can check on the corresponding stc{w,d}x. whether the
value has changed.  It's a poor man's form of implementing atomic
operations and is valid only for NPTL usermode Linux emulation.

Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
---
 target-ppc/cpu.h       |    4 +++-
 target-ppc/helper.c    |    2 +-
 target-ppc/machine.c   |    4 ++--
 target-ppc/op_helper.c |    4 ++--
 target-ppc/translate.c |   13 +++++++++----
 5 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 3fa1654..8d62218 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -563,7 +563,9 @@ struct CPUPPCState {
     /* XER */
     target_ulong xer;
     /* Reservation address */
-    target_ulong reserve;
+    target_ulong reserve_addr;
+    /* Reservation value */
+    target_ulong reserve_val;
 
     /* Those ones are used in supervisor mode only */
     /* machine state register */
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index db765e3..4a6a1ce 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -2807,7 +2807,7 @@ void cpu_ppc_reset (void *opaque)
         env->msr |= (1ULL << MSR_SF);
 #endif
     hreg_compute_hflags(env);
-    env->reserve = (target_ulong)-1ULL;
+    env->reserve_addr = (target_ulong)-1ULL;
     /* Be sure no exception or interrupt is pending */
     env->pending_interrupts = 0;
     env->exception_index = POWERPC_EXCP_NONE;
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 8b82005..3541f07 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -28,7 +28,7 @@ void cpu_save(QEMUFile *f, void *opaque)
     for (i = 0; i < 8; i++)
         qemu_put_be32s(f, &env->crf[i]);
     qemu_put_betls(f, &env->xer);
-    qemu_put_betls(f, &env->reserve);
+    qemu_put_betls(f, &env->reserve_addr);
     qemu_put_betls(f, &env->msr);
     for (i = 0; i < 4; i++)
         qemu_put_betls(f, &env->tgpr[i]);
@@ -115,7 +115,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     for (i = 0; i < 8; i++)
         qemu_get_be32s(f, &env->crf[i]);
     qemu_get_betls(f, &env->xer);
-    qemu_get_betls(f, &env->reserve);
+    qemu_get_betls(f, &env->reserve_addr);
     qemu_get_betls(f, &env->msr);
     for (i = 0; i < 4; i++)
         qemu_get_betls(f, &env->tgpr[i]);
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 2c6a27f..bdebf0d 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -330,8 +330,8 @@ static void do_dcbz(target_ulong addr, int dcache_line_size)
     for (i = 0 ; i < dcache_line_size ; i += 4) {
         stl(addr + i , 0);
     }
-    if (env->reserve == addr)
-        env->reserve = (target_ulong)-1ULL;
+    if (env->reserve_addr == addr)
+        env->reserve_addr = (target_ulong)-1ULL;
 }
 
 void helper_dcbz(target_ulong addr)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 24c78d1..5401434 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -159,7 +159,8 @@ void ppc_translate_init(void)
                                  offsetof(CPUState, xer), "xer");
 
     cpu_reserve = tcg_global_mem_new(TCG_AREG0,
-                                     offsetof(CPUState, reserve), "reserve");
+                                     offsetof(CPUState, reserve_addr),
+                                     "reserve_addr");
 
     cpu_fpscr = tcg_global_mem_new_i32(TCG_AREG0,
                                        offsetof(CPUState, fpscr), "fpscr");
@@ -3136,12 +3137,14 @@ GEN_HANDLER(isync, 0x13, 0x16, 0x04, 0x03FFF801, PPC_MEM)
 GEN_HANDLER(lwarx, 0x1F, 0x14, 0x00, 0x00000001, PPC_RES)
 {
     TCGv t0;
+    TCGv gpr = cpu_gpr[rD(ctx->opcode)];
     gen_set_access_type(ctx, ACCESS_RES);
     t0 = tcg_temp_local_new();
     gen_addr_reg_index(ctx, t0);
     gen_check_align(ctx, t0, 0x03);
-    gen_qemu_ld32u(ctx, cpu_gpr[rD(ctx->opcode)], t0);
+    gen_qemu_ld32u(ctx, gpr, t0);
     tcg_gen_mov_tl(cpu_reserve, t0);
+    tcg_gen_st_tl(gpr, cpu_env, offsetof(CPUState, reserve_val));
     tcg_temp_free(t0);
 }
 
@@ -3171,12 +3174,14 @@ GEN_HANDLER2(stwcx_, "stwcx.", 0x1F, 0x16, 0x04, 0x00000000, PPC_RES)
 GEN_HANDLER(ldarx, 0x1F, 0x14, 0x02, 0x00000001, PPC_64B)
 {
     TCGv t0;
+    TCGv gpr = cpu_gpr[rD(ctx->opcode)];
     gen_set_access_type(ctx, ACCESS_RES);
     t0 = tcg_temp_local_new();
     gen_addr_reg_index(ctx, t0);
     gen_check_align(ctx, t0, 0x07);
-    gen_qemu_ld64(ctx, cpu_gpr[rD(ctx->opcode)], t0);
+    gen_qemu_ld64(ctx, gpr, t0);
     tcg_gen_mov_tl(cpu_reserve, t0);
+    tcg_gen_st_tl(gpr, cpu_env, offsetof(CPUState, reserve_val);
     tcg_temp_free(t0);
 }
 
@@ -8086,7 +8091,7 @@ void cpu_dump_state (CPUState *env, FILE *f,
             a = 'E';
         cpu_fprintf(f, " %c%c", a, env->crf[i] & 0x01 ? 'O' : ' ');
     }
-    cpu_fprintf(f, " ]             RES " ADDRX "\n", env->reserve);
+    cpu_fprintf(f, " ]             RES " ADDRX "\n", env->reserve_addr);
     for (i = 0; i < 32; i++) {
         if ((i & (RFPL - 1)) == 0)
             cpu_fprintf(f, "FPR%02d", i);
-- 
1.6.3.2

Re: [Qemu-devel] [PATCH 4/7] target-ppc: retain l{w,d}arx loaded value

[Riku Voipio]

Not that I can really review this patch, but;

On Thu, Jun 04, 2009 at 11:51:59AM -0700, Nathan Froyd wrote:
>  GEN_HANDLER(ldarx, 0x1F, 0x14, 0x02, 0x00000001, PPC_64B)
>  {
>      TCGv t0;
> +    TCGv gpr = cpu_gpr[rD(ctx->opcode)];
>      gen_set_access_type(ctx, ACCESS_RES);
>      t0 = tcg_temp_local_new();
>      gen_addr_reg_index(ctx, t0);
>      gen_check_align(ctx, t0, 0x07);
> -    gen_qemu_ld64(ctx, cpu_gpr[rD(ctx->opcode)], t0);
> +    gen_qemu_ld64(ctx, gpr, t0);
>      tcg_gen_mov_tl(cpu_reserve, t0);
> +    tcg_gen_st_tl(gpr, cpu_env, offsetof(CPUState, reserve_val);

missing a closing parens?

>      tcg_temp_free(t0);
>  }

[Qemu-devel] [PATCH 5/7] target-ppc: add exceptions for conditional stores

[Nathan Froyd]

Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
---
 target-ppc/cpu.h       |    5 +++
 target-ppc/translate.c |   71 ++++++++++++++++++++++++++++++++++-------------
 2 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 8d62218..1733a45 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -220,6 +220,7 @@ enum {
     /* Qemu exceptions: special cases we want to stop translation            */
     POWERPC_EXCP_SYNC         = 0x202, /* context synchronizing instruction  */
     POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only      */
+    POWERPC_EXCP_STCX         = 0x204 /* Conditional stores in user mode     */
 };
 
 /* Exceptions error codes                                                    */
@@ -566,6 +567,10 @@ struct CPUPPCState {
     target_ulong reserve_addr;
     /* Reservation value */
     target_ulong reserve_val;
+    /* Reservation store address */
+    target_ulong reserve_ea;
+    /* Reserved store source register and size */
+    target_ulong reserve_info;
 
     /* Those ones are used in supervisor mode only */
     /* machine state register */
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 5401434..78094c7 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3148,24 +3148,49 @@ GEN_HANDLER(lwarx, 0x1F, 0x14, 0x00, 0x00000001, PPC_RES)
     tcg_temp_free(t0);
 }
 
+#if defined(CONFIG_USER_ONLY)
+static void gen_conditional_store (DisasContext *ctx, TCGv EA,
+                                   int reg, int size)
+{
+    TCGv t0 = tcg_temp_new();
+    uint32_t save_exception = ctx->exception;
+
+    tcg_gen_st_tl(EA, cpu_env, offsetof(CPUState, reserve_ea));
+    tcg_gen_movi_tl(t0, (size << 5) | reg);
+    tcg_gen_st_tl(t0, cpu_env, offsetof(CPUState, reserve_info));
+    tcg_temp_free(t0);
+    gen_update_nip(ctx, ctx->nip-4);
+    ctx->exception = POWERPC_EXCP_BRANCH;
+    gen_exception(ctx, POWERPC_EXCP_STCX);
+    ctx->exception = save_exception;
+}
+#endif
+
 /* stwcx. */
 GEN_HANDLER2(stwcx_, "stwcx.", 0x1F, 0x16, 0x04, 0x00000000, PPC_RES)
 {
-    int l1;
     TCGv t0;
     gen_set_access_type(ctx, ACCESS_RES);
     t0 = tcg_temp_local_new();
     gen_addr_reg_index(ctx, t0);
     gen_check_align(ctx, t0, 0x03);
-    tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_xer);
-    tcg_gen_shri_i32(cpu_crf[0], cpu_crf[0], XER_SO);
-    tcg_gen_andi_i32(cpu_crf[0], cpu_crf[0], 1);
-    l1 = gen_new_label();
-    tcg_gen_brcond_tl(TCG_COND_NE, t0, cpu_reserve, l1);
-    tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 1 << CRF_EQ);
-    gen_qemu_st32(ctx, cpu_gpr[rS(ctx->opcode)], t0);
-    gen_set_label(l1);
-    tcg_gen_movi_tl(cpu_reserve, -1);
+#if defined(CONFIG_USER_ONLY)
+    gen_conditional_store(ctx, t0, rS(ctx->opcode), 4);
+#else
+    {
+        int l1;
+
+        tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_xer);
+        tcg_gen_shri_i32(cpu_crf[0], cpu_crf[0], XER_SO);
+        tcg_gen_andi_i32(cpu_crf[0], cpu_crf[0], 1);
+        l1 = gen_new_label();
+        tcg_gen_brcond_tl(TCG_COND_NE, t0, cpu_reserve, l1);
+        tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 1 << CRF_EQ);
+        gen_qemu_st32(ctx, cpu_gpr[rS(ctx->opcode)], t0);
+        gen_set_label(l1);
+        tcg_gen_movi_tl(cpu_reserve, -1);
+    }
+#endif
     tcg_temp_free(t0);
 }
 
@@ -3188,21 +3213,27 @@ GEN_HANDLER(ldarx, 0x1F, 0x14, 0x02, 0x00000001, PPC_64B)
 /* stdcx. */
 GEN_HANDLER2(stdcx_, "stdcx.", 0x1F, 0x16, 0x06, 0x00000000, PPC_64B)
 {
-    int l1;
     TCGv t0;
     gen_set_access_type(ctx, ACCESS_RES);
     t0 = tcg_temp_local_new();
     gen_addr_reg_index(ctx, t0);
     gen_check_align(ctx, t0, 0x07);
-    tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_xer);
-    tcg_gen_shri_i32(cpu_crf[0], cpu_crf[0], XER_SO);
-    tcg_gen_andi_i32(cpu_crf[0], cpu_crf[0], 1);
-    l1 = gen_new_label();
-    tcg_gen_brcond_tl(TCG_COND_NE, t0, cpu_reserve, l1);
-    tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 1 << CRF_EQ);
-    gen_qemu_st64(ctx, cpu_gpr[rS(ctx->opcode)], t0);
-    gen_set_label(l1);
-    tcg_gen_movi_tl(cpu_reserve, -1);
+#if defined(CONFIG_USER_ONLY)
+    gen_conditional_store(ctx, t0, rS(ctx->opcode), 8);
+#else
+    {
+        int l1;
+        tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_xer);
+        tcg_gen_shri_i32(cpu_crf[0], cpu_crf[0], XER_SO);
+        tcg_gen_andi_i32(cpu_crf[0], cpu_crf[0], 1);
+        l1 = gen_new_label();
+        tcg_gen_brcond_tl(TCG_COND_NE, t0, cpu_reserve, l1);
+        tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 1 << CRF_EQ);
+        gen_qemu_st64(ctx, cpu_gpr[rS(ctx->opcode)], t0);
+        gen_set_label(l1);
+        tcg_gen_movi_tl(cpu_reserve, -1);
+    }
+#endif
     tcg_temp_free(t0);
 }
 #endif /* defined(TARGET_PPC64) */
-- 
1.6.3.2

[Qemu-devel] [PATCH 6/7] linux-user: handle POWERPC_EXCP_STCX

[Nathan Froyd]

We handle conditional stores as an exception so we can ensure that no
other thread is changing memory out from underneath us.

Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
---
 linux-user/main.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index dc39b05..7797e7f 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1088,6 +1088,63 @@ do {                                                                          \
     log_cpu_state(env, 0);                                                      \
 } while (0)
 
+static int do_store_exclusive(CPUPPCState *env)
+{
+    target_ulong addr;
+    target_ulong page_addr;
+    target_ulong val;
+    int flags;
+    int segv = 0;
+
+    addr = env->reserve_ea;
+    page_addr = addr & TARGET_PAGE_MASK;
+    start_exclusive();
+    mmap_lock();
+    flags = page_get_flags(page_addr);
+    if ((flags & PAGE_READ) == 0) {
+        segv = 1;
+    } else {
+        int reg = env->reserve_info & 0x1f;
+        int size = (env->reserve_info >> 5) & 0xf;
+        int stored = 0;
+
+        if (addr == env->reserve_addr) {
+            switch (size) {
+            case 1: segv = get_user_u8(val, addr); break;
+            case 2: segv = get_user_u16(val, addr); break;
+            case 4: segv = get_user_u32(val, addr); break;
+#if defined(TARGET_PPC64)
+            case 8: segv = get_user_u64(val, addr); break;
+#endif
+            default: abort();
+            }
+            if (!segv && val == env->reserve_val) {
+                val = env->gpr[reg];
+                switch (size) {
+                case 1: segv = put_user_u8(val, addr); break;
+                case 2: segv = put_user_u16(val, addr); break;
+                case 4: segv = put_user_u32(val, addr); break;
+#if defined(TARGET_PPC64)
+                case 8: segv = put_user_u64(val, addr); break;
+#endif
+                default: abort();
+                }
+                if (!segv) {
+                    stored = 1;
+                }
+            }
+        }
+        env->crf[0] = (stored << 1) | xer_so;
+        env->reserve_addr = (target_ulong)-1;
+    }
+    if (!segv) {
+        env->nip += 4;
+    }
+    mmap_unlock();
+    end_exclusive();
+    return segv;
+}
+
 void cpu_loop(CPUPPCState *env)
 {
     target_siginfo_t info;
@@ -1095,7 +1152,9 @@ void cpu_loop(CPUPPCState *env)
     uint32_t ret;
 
     for(;;) {
+        cpu_exec_start(env);
         trapnr = cpu_ppc_exec(env);
+        cpu_exec_end(env);
         switch(trapnr) {
         case POWERPC_EXCP_NONE:
             /* Just go on */
@@ -1470,6 +1529,15 @@ void cpu_loop(CPUPPCState *env)
             printf("syscall returned 0x%08x (%d)\n", ret, ret);
 #endif
             break;
+        case POWERPC_EXCP_STCX:
+            if (do_store_exclusive(env)) {
+                info.si_signo = TARGET_SIGSEGV;
+                info.si_errno = 0;
+                info.si_code = TARGET_SEGV_MAPERR;
+                info._sifields._sigfault._addr = env->nip;
+                queue_signal(env, info.si_signo, &info);
+            }
+            break;
         case EXCP_DEBUG:
             {
                 int sig;
-- 
1.6.3.2

[Qemu-devel] [PATCH 7/7] enable NPTL for ppc-linux-user targets in configure

[Nathan Froyd]

Enabling support for ppc64-linux-user should be easy enough to do later.

Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
---
 configure |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index b2a2540..6ba47c7 100755
--- a/configure
+++ b/configure
@@ -1892,6 +1892,7 @@ case "$target_cpu" in
     echo "#define TARGET_ARCH \"ppc\"" >> $config_h
     echo "#define TARGET_PPC 1" >> $config_h
     gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
+    target_nptl="yes"
   ;;
   ppcemb)
     echo "TARGET_ARCH=ppcemb" >> $config_mak
@@ -1905,6 +1906,7 @@ case "$target_cpu" in
       echo "#define CONFIG_KVM 1" >> $config_h
     fi
     gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
+    target_nptl="yes"
   ;;
   ppc64)
     echo "TARGET_ARCH=ppc64" >> $config_mak
-- 
1.6.3.2

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[malc]

On Thu, 4 Jun 2009, Nathan Froyd wrote:

> This patch series adds NPTL support in Linux user-mode emulation to
> 32-bit PowerPC targets.
> 
> The main complication comes from implementing atomic instructions
> properly.  We chose to implement a simplistic model:
> 
> - reserved loads record the value loaded;
> 
> - conditional stores check that the memory at the effective address
>   contains the value loaded by the previous reserved load, in addition
>   to all other checks.  if so, the store succeeds; otherwise, it fails.

I think this will break code that relies on the fact that ll/sc is not
affected by the ABA problem.

> 
> It is possible to implement something more sophisticated using mprotect:
> 
> - reserved loads write-protect the page from which the value is loaded;
> 
> - regular stores to the page (through SIGSEGV handling) remove the write
>   protection (which is roughly how the architecture really works);
> 
> - conditional stores fail if the page was not write-protected, in
>   addition to all other checks.  If the store succeeds, then the page is
>   unprotected.
> 
> but the simple scheme works well enough and should be somewhat faster.
> The simple scheme is what's already done for system mode, too; it's even
> slightly dumber in system mode because we don't check for equality of
> values.

[..snip..]

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Paul Brook]

On Saturday 06 June 2009, malc wrote:
> On Thu, 4 Jun 2009, Nathan Froyd wrote:
> > This patch series adds NPTL support in Linux user-mode emulation to
> > 32-bit PowerPC targets.
> >
> > The main complication comes from implementing atomic instructions
> > properly.  We chose to implement a simplistic model:
> >
> > - reserved loads record the value loaded;

An important point here is that the address/value pair is per thread/cpu.
A nice side-effect is that these loads reduce to a simple atomic load and some 
thread local bookkeeping. Conditional stores require somewhat more exotic 
atomic operations, but still don't need to go poking at system global state or 
other CPUs.
This sounds strange at first reading, but ll/sc semantics are deliberately 
designed to minimize contention between unrelated CPUs/resources in large 
systems.

> > - conditional stores check that the memory at the effective address
> >   contains the value loaded by the previous reserved load, in addition
> >   to all other checks.  if so, the store succeeds; otherwise, it fails.
>
> I think this will break code that relies on the fact that ll/sc is not
> affected by the ABA problem.

I'm not absolutely certain about PPC, but on other architectures (ARM, MIPS, 
Alpha) this implementation is sufficient.

The only questionable case is when a second thread overwrites and then 
restores the original value between a locked load and a conditional store. 
However limited coherency and memory ordering between CPUs make it impossible 
to know whether this modify+restore occurred before or after the initial load.

The worst that can happen here is that another thread gains and releases the 
lock[1] while the current thread is in the process of acquiring the lock.  
Even when this happens it is impossible for two threads to acquire the lock 
simultaneously. The only difference is the window between ll and sc. During 
this period we don't know whether we have the lock or not, so it's extremely 
unlikely that we will do anything that relies on no other thread having the 
lock. In practice ll/sc are always used as matching pairs with no intervening 
memory accesses, so this is never a problem.

I could probably come up with synthetic testcases where qemu behavior is 
observably different to real hardware. However I'm pretty certain this never 
occurs in real code, and it is questionable whether such behavior is 
architecturally defined.

If you still believe this is a problem you need come up with an actual 
testcase that demonstrates how this can introduce a race condition.

Paul

[1] Lock acquisition is the most obvious example, but the same applies to any 
atomic operation implemented on top of ll/sc. 

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Nathan Froyd]

On Sat, Jun 06, 2009 at 03:04:38AM +0400, malc wrote:
> On Thu, 4 Jun 2009, Nathan Froyd wrote:
> > The main complication comes from implementing atomic instructions
> > properly.  We chose to implement a simplistic model:
> > 
> > - reserved loads record the value loaded;
> > 
> > - conditional stores check that the memory at the effective address
> >   contains the value loaded by the previous reserved load, in addition
> >   to all other checks.  if so, the store succeeds; otherwise, it fails.
> 
> I think this will break code that relies on the fact that ll/sc is not
> affected by the ABA problem.
>
> > It is possible to implement something more sophisticated using mprotect:

The simpler scheme is slightly faster than mprotect, but not
astonishingly so.  In practice, it works out OK, even if it doesn't
adhere to a particular interpretation of the chip docs.

I'm ambivalent about which scheme goes in: I just would like to see
something go in so atomic instructions/user-mode emulation threading can
be properly supported on PPC (and other architectures to follow, I'm
sure...).

-Nathan

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]

On Thu, Jun 04, 2009 at 11:51:55AM -0700, Nathan Froyd <froydnj@codesourcery.com> wrote:
> The patch series has been tested against the glibc testsuite, where it
> passes a good chunk (90%+) of the testsuite.  The other 10% are
> basically things that are not going to work in QEMU anytime soon
> (e.g. sharing futexes between multiple processes, using clone(2)
> directly, etc.).  I should note that proper testing requires a patch to
> use a correct exit status for uncaught signals; such a patch has been
> posted to this list before by Riku Voipio.  (I have a different local
> version that I used instead.)  Testing with recent glibc also requires
> adding support for private futexes and a few other futex operations;
> again, a patch for this has been posted by Riku and I used a slightly
> different local version.

Hi,

I tried this series (on top of current git 3a41759) + Riku's patch. I
built a static qemu-ppc binary, then tried:

host# chroot . /qemu-ppc -L . /bin/bash

Where the current directory was a PPC chroot. It launched bash just
fine, but when I tried to launch a command from the ppc bash, I got:

chroot# /bin/uname
bash: /bin/uname: No such file or directory

While the binary is there:

host# file bin/uname
bin/uname: ELF 32-bit MSB executable, PowerPC or cisco 4500, version 1
(SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.24,
with unknown capability 0x41000000 = 0x11676e75, with unknown capability
0x10000 = 0x90401, stripped

Let me know if you need more info to reproduce the issue.

Thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Daniel Jacobowitz]

On Wed, Jun 10, 2009 at 01:37:45AM +0200, Miklos Vajna wrote:
> On Thu, Jun 04, 2009 at 11:51:55AM -0700, Nathan Froyd <froydnj@codesourcery.com> wrote:
> > The patch series has been tested against the glibc testsuite, where it
> > passes a good chunk (90%+) of the testsuite.  The other 10% are
> > basically things that are not going to work in QEMU anytime soon
> > (e.g. sharing futexes between multiple processes, using clone(2)
> > directly, etc.).  I should note that proper testing requires a patch to
> > use a correct exit status for uncaught signals; such a patch has been
> > posted to this list before by Riku Voipio.  (I have a different local
> > version that I used instead.)  Testing with recent glibc also requires
> > adding support for private futexes and a few other futex operations;
> > again, a patch for this has been posted by Riku and I used a slightly
> > different local version.
> 
> Hi,
> 
> I tried this series (on top of current git 3a41759) + Riku's patch. I
> built a static qemu-ppc binary, then tried:
> 
> host# chroot . /qemu-ppc -L . /bin/bash
> 
> Where the current directory was a PPC chroot. It launched bash just
> fine, but when I tried to launch a command from the ppc bash, I got:
> 
> chroot# /bin/uname
> bash: /bin/uname: No such file or directory

This isn't expected to work, is it?  QEMU does not intercept exec
system calls for target binaries.

[Maybe it should...]

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

On Tue, Jun 09, 2009 at 08:24:47PM -0400, Daniel Jacobowitz <drow@false.org> wrote:
> > host# chroot . /qemu-ppc -L . /bin/bash
> > 
> > Where the current directory was a PPC chroot. It launched bash just
> > fine, but when I tried to launch a command from the ppc bash, I got:
> > 
> > chroot# /bin/uname
> > bash: /bin/uname: No such file or directory
> 
> This isn't expected to work, is it?  QEMU does not intercept exec
> system calls for target binaries.
> 
> [Maybe it should...]

Hmm. According to this mail it used to work in the non-NPTL case:

http://osdir.com/ml/emulators.qemu/2004-02/msg00162.html

Or have I missed something?

Thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Daniel Jacobowitz]

On Wed, Jun 10, 2009 at 02:30:48AM +0200, Miklos Vajna wrote:
> On Tue, Jun 09, 2009 at 08:24:47PM -0400, Daniel Jacobowitz <drow@false.org> wrote:
> > > host# chroot . /qemu-ppc -L . /bin/bash
> > > 
> > > Where the current directory was a PPC chroot. It launched bash just
> > > fine, but when I tried to launch a command from the ppc bash, I got:
> > > 
> > > chroot# /bin/uname
> > > bash: /bin/uname: No such file or directory
> > 
> > This isn't expected to work, is it?  QEMU does not intercept exec
> > system calls for target binaries.
> > 
> > [Maybe it should...]
> 
> Hmm. According to this mail it used to work in the non-NPTL case:
> 
> http://osdir.com/ml/emulators.qemu/2004-02/msg00162.html
> 
> Or have I missed something?

Oh, are you using binfmt_misc?  That's different...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Tue, Jun 09, 2009 at 10:49:55PM -0400, Daniel Jacobowitz <drow@false.org> wrote:
> > Hmm. According to this mail it used to work in the non-NPTL case:
> > 
> > http://osdir.com/ml/emulators.qemu/2004-02/msg00162.html
> > 
> > Or have I missed something?
> 
> Oh, are you using binfmt_misc?  That's different...

Sorry, I forgot to mention it. I did a

echo ":qemu-ppc:M::\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x14:\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfb\xff\xff\xff:/usr/bin/qemu-ppc:" > /proc/sys/fs/binfmt_misc/register

And the a test binary using fork() works fine as well:

$ file hello
hello: ELF 32-bit MSB executable, PowerPC or cisco 4500, version 1
(SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.4.3, not
stripped

$ ./hello
pid is 0
pid is 14461

Thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Laurent Vivier]

Le mercredi 10 juin 2009 à 01:37 +0200, Miklos Vajna a écrit :
> On Thu, Jun 04, 2009 at 11:51:55AM -0700, Nathan Froyd <froydnj@codesourcery.com> wrote:
> > The patch series has been tested against the glibc testsuite, where it
> > passes a good chunk (90%+) of the testsuite.  The other 10% are
> > basically things that are not going to work in QEMU anytime soon
> > (e.g. sharing futexes between multiple processes, using clone(2)
> > directly, etc.).  I should note that proper testing requires a patch to
> > use a correct exit status for uncaught signals; such a patch has been
> > posted to this list before by Riku Voipio.  (I have a different local
> > version that I used instead.)  Testing with recent glibc also requires
> > adding support for private futexes and a few other futex operations;
> > again, a patch for this has been posted by Riku and I used a slightly
> > different local version.
> 
> Hi,
> 
> I tried this series (on top of current git 3a41759) + Riku's patch. I
> built a static qemu-ppc binary, then tried:
> 
> host# chroot . /qemu-ppc -L . /bin/bash
> 
> Where the current directory was a PPC chroot. It launched bash just
> fine, but when I tried to launch a command from the ppc bash, I got:
> 
> chroot# /bin/uname
> bash: /bin/uname: No such file or directory

Did you configure binfmt_misc kernel module to load PPC binaries with
qemu-ppc ?

Regards,
Laurent

> While the binary is there:
> 
> host# file bin/uname
> bin/uname: ELF 32-bit MSB executable, PowerPC or cisco 4500, version 1
> (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.24,
> with unknown capability 0x41000000 = 0x11676e75, with unknown capability
> 0x10000 = 0x90401, stripped
> 
> Let me know if you need more info to reproduce the issue.
> 
> Thanks.
-- 
--------------------- laurent@vivier.eu ----------------------
"Tout ce qui est impossible reste à accomplir"    Jules Verne
"Things are only impossible until they're not" Jean-Luc Picard

Re: [Qemu-devel] [PATCH 0/7] target-ppc/linux-user: NPTL support

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

On Wed, Jun 10, 2009 at 02:57:08AM +0200, Laurent Vivier <Laurent@vivier.eu> wrote:
> > chroot# /bin/uname
> > bash: /bin/uname: No such file or directory
> 
> Did you configure binfmt_misc kernel module to load PPC binaries with
> qemu-ppc ?

Yes:

$ cat /proc/sys/fs/binfmt_misc/qemu-ppc
enabled
interpreter /usr/bin/qemu-ppc
flags:
offset 0
magic 7f454c4601020100000000000000000000020014
mask fffffffffffffffffffffffffffffffffbffffff

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]