All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] target-mips: rework conditional stores for mttcg
@ 2016-09-21  8:07 Leon Alrae
  2016-09-21  8:07 ` [Qemu-devel] [PATCH 1/2] target-mips: compare virtual addresses in LL/SC sequence Leon Alrae
  2016-09-21  8:07 ` [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg Leon Alrae
  0 siblings, 2 replies; 8+ messages in thread
From: Leon Alrae @ 2016-09-21  8:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: aurelien, rth

This small series changes MIPS conditional stores implementation for mttcg.
Specifically we compare virtual address of LL and SC (rather than physical)
which allows us to have just a single inlined implementation for user and
system emulation and to use new atomic helpers.

This is done in 2 steps:
Patch 1: modifies existing SC implementation to use virtual addresses
Patch 2: SC emulation rework and making use of cmpxchg

These patches apply on top of Richard's atomic series. I've done only
partial testing since many of my Linux images hit the abort() due to
EXCP_ATOMIC -- but IIUC this is a missing piece in atomic helpers rather
than a problem in the code gen.

v2:
* improved and simplified SC implementation according to Richard's comments

Leon Alrae (2):
  target-mips: compare virtual addresses in LL/SC sequence
  target-mips: reimplement SC instruction and use cmpxchg

 linux-user/main.c       |  58 -------------------------
 target-mips/cpu.h       |   7 +--
 target-mips/helper.c    |   6 +--
 target-mips/helper.h    |   2 -
 target-mips/machine.c   |   7 +--
 target-mips/op_helper.c |  52 +++++++----------------
 target-mips/translate.c | 111 +++++++++++++++++-------------------------------
 7 files changed, 63 insertions(+), 180 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/2] target-mips: compare virtual addresses in LL/SC sequence
  2016-09-21  8:07 [Qemu-devel] [PATCH 0/2] target-mips: rework conditional stores for mttcg Leon Alrae
@ 2016-09-21  8:07 ` Leon Alrae
  2016-09-21  8:07 ` [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg Leon Alrae
  1 sibling, 0 replies; 8+ messages in thread
From: Leon Alrae @ 2016-09-21  8:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: aurelien, rth

Until now we have been comparing physical addresses in LL/SC sequence.
Unfortunately that means that on each SC we have to do the address
translation which is a quite complex operation. If we could get rid of
it then it would allow us to throw away SC helpers and benefit from having
common implementation of SC in user and system mode (currently we have
2 separate implementations selected by #ifdef CONFIG_USER_ONLY).

Given our LL/SC emulation is already very simplified (as we only compare
the address and value), using virtual addresses instead of physical does
not seem to be a gross violation. Correct guest software should not rely
on LL/SC if they accesses the same physical address via different virtual
addresses or if page mapping gets changed between LL/SC due to manipulating
tlb entries. MIPS Instruction Set Manual clearly says that an RMW sequence
must use the same address in the LL and SC (virtual address, physical
address, cacheability and coherency attributes must be identical). Otherwise
the result of the SC is not predictable. This patch takes advantage of this
fact and removes the virtual -> physical address translation from SC helper.

lladdr served as Coprocessor 0 LLAddr register which captures physical
address of the most recent LL instruction, and also lladdr was used for
comparison with following SC physical address.
This patch changes the meaning of lladdr - now it will only keep the virtual
address of the most recent LL. Additionally we introduce CP0_LLAddr which is
the actual Coperocessor 0 LLAddr register that guest can access.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 target-mips/cpu.h       |  3 ++-
 target-mips/machine.c   |  7 ++++---
 target-mips/op_helper.c | 29 +++++++++++++++++------------
 target-mips/translate.c |  4 ++--
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 5182dc7..78555b9 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -494,10 +494,11 @@ struct CPUMIPSState {
 #define CP0C5_NFExists   0
     int32_t CP0_Config6;
     int32_t CP0_Config7;
+    uint64_t CP0_LLAddr;
     uint64_t CP0_MAAR[MIPS_MAAR_MAX];
     int32_t CP0_MAARI;
     /* XXX: Maybe make LLAddr per-TC? */
-    uint64_t lladdr;
+    target_ulong lladdr; /* LL virtual address compared against SC */
     target_ulong llval;
     target_ulong llnewval;
     target_ulong llreg;
diff --git a/target-mips/machine.c b/target-mips/machine.c
index a27f2f1..deebdf2 100644
--- a/target-mips/machine.c
+++ b/target-mips/machine.c
@@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = {
 
 const VMStateDescription vmstate_mips_cpu = {
     .name = "cpu",
-    .version_id = 8,
-    .minimum_version_id = 8,
+    .version_id = 9,
+    .minimum_version_id = 9,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
         /* Active TC */
@@ -274,9 +274,10 @@ const VMStateDescription vmstate_mips_cpu = {
         VMSTATE_INT32(env.CP0_Config3, MIPSCPU),
         VMSTATE_INT32(env.CP0_Config6, MIPSCPU),
         VMSTATE_INT32(env.CP0_Config7, MIPSCPU),
+        VMSTATE_UINT64(env.CP0_LLAddr, MIPSCPU),
         VMSTATE_UINT64_ARRAY(env.CP0_MAAR, MIPSCPU, MIPS_MAAR_MAX),
         VMSTATE_INT32(env.CP0_MAARI, MIPSCPU),
-        VMSTATE_UINT64(env.lladdr, MIPSCPU),
+        VMSTATE_UINTTL(env.lladdr, MIPSCPU),
         VMSTATE_UINTTL_ARRAY(env.CP0_WatchLo, MIPSCPU, 8),
         VMSTATE_INT32_ARRAY(env.CP0_WatchHi, MIPSCPU, 8),
         VMSTATE_UINTTL(env.CP0_XContext, MIPSCPU),
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index ea2f2ab..e0c9842 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -271,15 +271,15 @@ static inline hwaddr do_translate_address(CPUMIPSState *env,
                                                       target_ulong address,
                                                       int rw, uintptr_t retaddr)
 {
-    hwaddr lladdr;
+    hwaddr paddr;
     CPUState *cs = CPU(mips_env_get_cpu(env));
 
-    lladdr = cpu_mips_translate_address(env, address, rw);
+    paddr = cpu_mips_translate_address(env, address, rw);
 
-    if (lladdr == -1LL) {
+    if (paddr == -1LL) {
         cpu_loop_exit_restore(cs, retaddr);
     } else {
-        return lladdr;
+        return paddr;
     }
 }
 
@@ -290,7 +290,8 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int mem_idx)  \
         env->CP0_BadVAddr = arg;                                              \
         do_raise_exception(env, EXCP_AdEL, GETPC());                          \
     }                                                                         \
-    env->lladdr = do_translate_address(env, arg, 0, GETPC());                 \
+    env->CP0_LLAddr = do_translate_address(env, arg, 0, GETPC());             \
+    env->lladdr = arg;                                                        \
     env->llval = do_##insn(env, arg, mem_idx, GETPC());                       \
     return env->llval;                                                        \
 }
@@ -310,7 +311,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1,              \
         env->CP0_BadVAddr = arg2;                                             \
         do_raise_exception(env, EXCP_AdES, GETPC());                          \
     }                                                                         \
-    if (do_translate_address(env, arg2, 1, GETPC()) == env->lladdr) {         \
+    if (arg2 == env->lladdr) {                                                \
         tmp = do_##ld_insn(env, arg2, mem_idx, GETPC());                      \
         if (tmp == env->llval) {                                              \
             do_##st_insn(env, arg2, arg1, mem_idx, GETPC());                  \
@@ -885,7 +886,7 @@ target_ulong helper_mftc0_status(CPUMIPSState *env)
 
 target_ulong helper_mfc0_lladdr(CPUMIPSState *env)
 {
-    return (int32_t)(env->lladdr >> env->CP0_LLAddr_shift);
+    return (int32_t)(env->CP0_LLAddr >> env->CP0_LLAddr_shift);
 }
 
 target_ulong helper_mfc0_maar(CPUMIPSState *env)
@@ -961,7 +962,7 @@ target_ulong helper_dmfc0_tcschefback(CPUMIPSState *env)
 
 target_ulong helper_dmfc0_lladdr(CPUMIPSState *env)
 {
-    return env->lladdr >> env->CP0_LLAddr_shift;
+    return env->CP0_LLAddr >> env->CP0_LLAddr_shift;
 }
 
 target_ulong helper_dmfc0_maar(CPUMIPSState *env)
@@ -1189,7 +1190,8 @@ void helper_mtc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
 {
     env->active_tc.PC = arg1;
     env->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
-    env->lladdr = 0ULL;
+    env->CP0_LLAddr = 0;
+    env->lladdr = 0;
     /* MIPS16 not implemented. */
 }
 
@@ -1201,12 +1203,14 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
     if (other_tc == other->current_tc) {
         other->active_tc.PC = arg1;
         other->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
-        other->lladdr = 0ULL;
+        other->CP0_LLAddr = 0;
+        other->lladdr = 0;
         /* MIPS16 not implemented. */
     } else {
         other->tcs[other_tc].PC = arg1;
         other->tcs[other_tc].CP0_TCStatus &= ~(1 << CP0TCSt_TDS);
-        other->lladdr = 0ULL;
+        other->CP0_LLAddr = 0;
+        other->lladdr = 0;
         /* MIPS16 not implemented. */
     }
 }
@@ -1591,7 +1595,7 @@ void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1)
 {
     target_long mask = env->CP0_LLAddr_rw_bitmask;
     arg1 = arg1 << env->CP0_LLAddr_shift;
-    env->lladdr = (env->lladdr & ~mask) | (arg1 & mask);
+    env->CP0_LLAddr = (env->CP0_LLAddr & ~mask) | (arg1 & mask);
 }
 
 #define MTC0_MAAR_MASK(env) \
@@ -2277,6 +2281,7 @@ static inline void exception_return(CPUMIPSState *env)
 void helper_eret(CPUMIPSState *env)
 {
     exception_return(env);
+    env->CP0_LLAddr = 1;
     env->lladdr = 1;
 }
 
diff --git a/target-mips/translate.c b/target-mips/translate.c
index d829738..5d0732f 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -4826,7 +4826,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
     case 17:
         switch (sel) {
         case 0:
-            gen_mfhc0_load64(arg, offsetof(CPUMIPSState, lladdr),
+            gen_mfhc0_load64(arg, offsetof(CPUMIPSState, CP0_LLAddr),
                              ctx->CP0_LLAddr_shift);
             rn = "LLAddr";
             break;
@@ -20114,7 +20114,7 @@ void mips_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
                 env->CP0_Status, env->CP0_Cause, env->CP0_EPC);
     cpu_fprintf(f, "    Config0 0x%08x Config1 0x%08x LLAddr 0x%016"
                 PRIx64 "\n",
-                env->CP0_Config0, env->CP0_Config1, env->lladdr);
+                env->CP0_Config0, env->CP0_Config1, env->CP0_LLAddr);
     cpu_fprintf(f, "    Config2 0x%08x Config3 0x%08x\n",
                 env->CP0_Config2, env->CP0_Config3);
     cpu_fprintf(f, "    Config4 0x%08x Config5 0x%08x\n",
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg
  2016-09-21  8:07 [Qemu-devel] [PATCH 0/2] target-mips: rework conditional stores for mttcg Leon Alrae
  2016-09-21  8:07 ` [Qemu-devel] [PATCH 1/2] target-mips: compare virtual addresses in LL/SC sequence Leon Alrae
@ 2016-09-21  8:07 ` Leon Alrae
  2016-09-21 20:16   ` Richard Henderson
  1 sibling, 1 reply; 8+ messages in thread
From: Leon Alrae @ 2016-09-21  8:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: aurelien, rth

This patch completely rewrites conditional stores. Now we use cmpxchg and
no longer need separate implementations for user and system emulation.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 linux-user/main.c       |  58 --------------------------
 target-mips/cpu.h       |   4 --
 target-mips/helper.c    |   6 +--
 target-mips/helper.h    |   2 -
 target-mips/op_helper.c |  25 -----------
 target-mips/translate.c | 107 +++++++++++++++++-------------------------------
 6 files changed, 39 insertions(+), 163 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 0d0bf9d..bc1b307 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2230,55 +2230,6 @@ static const uint8_t mips_syscall_args[] = {
 #  undef MIPS_SYS
 # endif /* O32 */
 
-static int do_store_exclusive(CPUMIPSState *env)
-{
-    target_ulong addr;
-    target_ulong page_addr;
-    target_ulong val;
-    int flags;
-    int segv = 0;
-    int reg;
-    int d;
-
-    addr = env->lladdr;
-    page_addr = addr & TARGET_PAGE_MASK;
-    start_exclusive();
-    mmap_lock();
-    flags = page_get_flags(page_addr);
-    if ((flags & PAGE_READ) == 0) {
-        segv = 1;
-    } else {
-        reg = env->llreg & 0x1f;
-        d = (env->llreg & 0x20) != 0;
-        if (d) {
-            segv = get_user_s64(val, addr);
-        } else {
-            segv = get_user_s32(val, addr);
-        }
-        if (!segv) {
-            if (val != env->llval) {
-                env->active_tc.gpr[reg] = 0;
-            } else {
-                if (d) {
-                    segv = put_user_u64(env->llnewval, addr);
-                } else {
-                    segv = put_user_u32(env->llnewval, addr);
-                }
-                if (!segv) {
-                    env->active_tc.gpr[reg] = 1;
-                }
-            }
-        }
-    }
-    env->lladdr = -1;
-    if (!segv) {
-        env->active_tc.PC += 4;
-    }
-    mmap_unlock();
-    end_exclusive();
-    return segv;
-}
-
 /* Break codes */
 enum {
     BRK_OVERFLOW = 6,
@@ -2426,15 +2377,6 @@ done_syscall:
                   }
             }
             break;
-        case EXCP_SC:
-            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->active_tc.PC;
-                queue_signal(env, info.si_signo, &info);
-            }
-            break;
         case EXCP_DSPDIS:
             info.si_signo = TARGET_SIGILL;
             info.si_errno = 0;
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 78555b9..6c268f0 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -500,8 +500,6 @@ struct CPUMIPSState {
     /* XXX: Maybe make LLAddr per-TC? */
     target_ulong lladdr; /* LL virtual address compared against SC */
     target_ulong llval;
-    target_ulong llnewval;
-    target_ulong llreg;
     uint64_t CP0_LLAddr_rw_bitmask;
     int CP0_LLAddr_shift;
     target_ulong CP0_WatchLo[8];
@@ -796,8 +794,6 @@ enum {
 
     EXCP_LAST = EXCP_TLBRI,
 };
-/* Dummy exception for conditional stores.  */
-#define EXCP_SC 0x100
 
 /*
  * This is an interrnally generated WAKE request line.
diff --git a/target-mips/helper.c b/target-mips/helper.c
index c864b15..67b19e6 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -958,10 +958,8 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
 {
     CPUState *cs = CPU(mips_env_get_cpu(env));
 
-    if (exception < EXCP_SC) {
-        qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n",
-                      __func__, exception, error_code);
-    }
+    qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n",
+                  __func__, exception, error_code);
     cs->exception_index = exception;
     env->error_code = error_code;
 
diff --git a/target-mips/helper.h b/target-mips/helper.h
index 666936c..dd68751 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -13,10 +13,8 @@ DEF_HELPER_4(swr, void, env, tl, tl, int)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(ll, tl, env, tl, int)
-DEF_HELPER_4(sc, tl, env, tl, tl, int)
 #ifdef TARGET_MIPS64
 DEF_HELPER_3(lld, tl, env, tl, int)
-DEF_HELPER_4(scd, tl, env, tl, tl, int)
 #endif
 #endif
 
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index e0c9842..9f094ad 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -300,31 +300,6 @@ HELPER_LD_ATOMIC(ll, lw, 0x3)
 HELPER_LD_ATOMIC(lld, ld, 0x7)
 #endif
 #undef HELPER_LD_ATOMIC
-
-#define HELPER_ST_ATOMIC(name, ld_insn, st_insn, almask)                      \
-target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1,              \
-                           target_ulong arg2, int mem_idx)                    \
-{                                                                             \
-    target_long tmp;                                                          \
-                                                                              \
-    if (arg2 & almask) {                                                      \
-        env->CP0_BadVAddr = arg2;                                             \
-        do_raise_exception(env, EXCP_AdES, GETPC());                          \
-    }                                                                         \
-    if (arg2 == env->lladdr) {                                                \
-        tmp = do_##ld_insn(env, arg2, mem_idx, GETPC());                      \
-        if (tmp == env->llval) {                                              \
-            do_##st_insn(env, arg2, arg1, mem_idx, GETPC());                  \
-            return 1;                                                         \
-        }                                                                     \
-    }                                                                         \
-    return 0;                                                                 \
-}
-HELPER_ST_ATOMIC(sc, lw, sw, 0x3)
-#ifdef TARGET_MIPS64
-HELPER_ST_ATOMIC(scd, ld, sd, 0x7)
-#endif
-#undef HELPER_ST_ATOMIC
 #endif
 
 #ifdef TARGET_WORDS_BIGENDIAN
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 5d0732f..097005d 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1364,6 +1364,7 @@ static TCGv_i32 hflags;
 static TCGv_i32 fpu_fcr0, fpu_fcr31;
 static TCGv_i64 fpu_f64[32];
 static TCGv_i64 msa_wr_d[64];
+static TCGv cpu_lladdr, cpu_llval;
 
 #include "exec/gen-icount.h"
 
@@ -2050,46 +2051,6 @@ OP_LD_ATOMIC(lld,ld64);
 #endif
 #undef OP_LD_ATOMIC
 
-#ifdef CONFIG_USER_ONLY
-#define OP_ST_ATOMIC(insn,fname,ldname,almask)                               \
-static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx) \
-{                                                                            \
-    TCGv t0 = tcg_temp_new();                                                \
-    TCGLabel *l1 = gen_new_label();                                          \
-    TCGLabel *l2 = gen_new_label();                                          \
-                                                                             \
-    tcg_gen_andi_tl(t0, arg2, almask);                                       \
-    tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, l1);                              \
-    tcg_gen_st_tl(arg2, cpu_env, offsetof(CPUMIPSState, CP0_BadVAddr));          \
-    generate_exception(ctx, EXCP_AdES);                                      \
-    gen_set_label(l1);                                                       \
-    tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUMIPSState, lladdr));                  \
-    tcg_gen_brcond_tl(TCG_COND_NE, arg2, t0, l2);                            \
-    tcg_gen_movi_tl(t0, rt | ((almask << 3) & 0x20));                        \
-    tcg_gen_st_tl(t0, cpu_env, offsetof(CPUMIPSState, llreg));                   \
-    tcg_gen_st_tl(arg1, cpu_env, offsetof(CPUMIPSState, llnewval));              \
-    generate_exception_end(ctx, EXCP_SC);                                    \
-    gen_set_label(l2);                                                       \
-    tcg_gen_movi_tl(t0, 0);                                                  \
-    gen_store_gpr(t0, rt);                                                   \
-    tcg_temp_free(t0);                                                       \
-}
-#else
-#define OP_ST_ATOMIC(insn,fname,ldname,almask)                               \
-static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx) \
-{                                                                            \
-    TCGv t0 = tcg_temp_new();                                                \
-    gen_helper_1e2i(insn, t0, arg1, arg2, ctx->mem_idx);                     \
-    gen_store_gpr(t0, rt);                                                   \
-    tcg_temp_free(t0);                                                       \
-}
-#endif
-OP_ST_ATOMIC(sc,st32,ld32s,0x3);
-#if defined(TARGET_MIPS64)
-OP_ST_ATOMIC(scd,st64,ld64,0x7);
-#endif
-#undef OP_ST_ATOMIC
-
 static void gen_base_offset_addr (DisasContext *ctx, TCGv addr,
                                   int base, int16_t offset)
 {
@@ -2335,33 +2296,34 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
 
 
 /* Store conditional */
-static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
-                         int base, int16_t offset)
+static void gen_st_cond(DisasContext *ctx, int rt, int base, int offset,
+                        TCGMemOp tcg_mo)
 {
-    TCGv t0, t1;
+    TCGv addr, t0, val;
+    TCGLabel *l1 = gen_new_label();
+    TCGLabel *done = gen_new_label();
 
-#ifdef CONFIG_USER_ONLY
     t0 = tcg_temp_local_new();
-    t1 = tcg_temp_local_new();
-#else
-    t0 = tcg_temp_new();
-    t1 = tcg_temp_new();
-#endif
-    gen_base_offset_addr(ctx, t0, base, offset);
-    gen_load_gpr(t1, rt);
-    switch (opc) {
-#if defined(TARGET_MIPS64)
-    case OPC_SCD:
-    case R6_OPC_SCD:
-        op_st_scd(t1, t0, rt, ctx);
-        break;
-#endif
-    case OPC_SC:
-    case R6_OPC_SC:
-        op_st_sc(t1, t0, rt, ctx);
-        break;
-    }
-    tcg_temp_free(t1);
+    addr = tcg_temp_new();
+    /* compare the address against that of the preceeding LL */
+    gen_base_offset_addr(ctx, addr, base, offset);
+    tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l1);
+    tcg_temp_free(addr);
+    tcg_gen_movi_tl(t0, 0);
+    tcg_gen_br(done);
+
+    gen_set_label(l1);
+    /* generate cmpxchg */
+    val = tcg_temp_new();
+    gen_load_gpr(val, rt);
+    tcg_gen_atomic_cmpxchg_tl(t0, cpu_lladdr, cpu_llval, val,
+                              ctx->mem_idx, tcg_mo);
+    tcg_gen_setcond_tl(TCG_COND_EQ, t0, t0, cpu_llval);
+    tcg_temp_free(val);
+
+    gen_set_label(done);
+    /* store the result into the register */
+    gen_store_gpr(t0, rt);
     tcg_temp_free(t0);
 }
 
@@ -14700,13 +14662,13 @@ static void decode_micromips32_opc(CPUMIPSState *env, DisasContext *ctx)
             gen_st(ctx, mips32_op, rt, rs, SIMM(ctx->opcode, 0, 12));
             break;
         case SC:
-            gen_st_cond(ctx, OPC_SC, rt, rs, offset);
+            gen_st_cond(ctx, rt, rs, offset, MO_TESL);
             break;
 #if defined(TARGET_MIPS64)
         case SCD:
             check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
-            gen_st_cond(ctx, OPC_SCD, rt, rs, offset);
+            gen_st_cond(ctx, rt, rs, offset, MO_TEQ);
             break;
 #endif
         case PREF:
@@ -17421,7 +17383,7 @@ static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx)
         }
         break;
     case R6_OPC_SC:
-        gen_st_cond(ctx, op1, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, MO_TESL);
         break;
     case R6_OPC_LL:
         gen_ld(ctx, op1, rt, rs, imm);
@@ -17445,7 +17407,7 @@ static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx)
         break;
 #if defined(TARGET_MIPS64)
     case R6_OPC_SCD:
-        gen_st_cond(ctx, op1, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, MO_TEQ);
         break;
     case R6_OPC_LLD:
         gen_ld(ctx, op1, rt, rs, imm);
@@ -19521,7 +19483,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
     case OPC_SC:
         check_insn(ctx, ISA_MIPS2);
          check_insn_opc_removed(ctx, ISA_MIPS32R6);
-         gen_st_cond(ctx, op, rt, rs, imm);
+         gen_st_cond(ctx, rt, rs, imm, MO_TESL);
          break;
     case OPC_CACHE:
         check_insn_opc_removed(ctx, ISA_MIPS32R6);
@@ -19807,7 +19769,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         check_insn_opc_removed(ctx, ISA_MIPS32R6);
         check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
-        gen_st_cond(ctx, op, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, MO_TEQ);
         break;
     case OPC_BNVC: /* OPC_BNEZALC, OPC_BNEC, OPC_DADDI */
         if (ctx->insn_flags & ISA_MIPS32R6) {
@@ -20180,6 +20142,11 @@ void mips_tcg_init(void)
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
 
+    cpu_lladdr = tcg_global_mem_new(cpu_env, offsetof(CPUMIPSState, lladdr),
+                                    "lladdr");
+    cpu_llval = tcg_global_mem_new(cpu_env, offsetof(CPUMIPSState, llval),
+                                   "llval");
+
     inited = 1;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg
  2016-09-21  8:07 ` [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg Leon Alrae
@ 2016-09-21 20:16   ` Richard Henderson
  2016-09-27  7:02     ` Leon Alrae
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2016-09-21 20:16 UTC (permalink / raw)
  To: Leon Alrae, qemu-devel; +Cc: aurelien

On 09/21/2016 01:07 AM, Leon Alrae wrote:
> +    tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l1);
> +    tcg_temp_free(addr);
> +    tcg_gen_movi_tl(t0, 0);
> +    tcg_gen_br(done);
> +
> +    gen_set_label(l1);
> +    /* generate cmpxchg */
> +    val = tcg_temp_new();
> +    gen_load_gpr(val, rt);
> +    tcg_gen_atomic_cmpxchg_tl(t0, cpu_lladdr, cpu_llval, val,
> +                              ctx->mem_idx, tcg_mo);
> +    tcg_gen_setcond_tl(TCG_COND_EQ, t0, t0, cpu_llval);
> +    tcg_temp_free(val);
> +
> +    gen_set_label(done);
> +    /* store the result into the register */
> +    gen_store_gpr(t0, rt);
>      tcg_temp_free(t0);

The only thing I would change is to duplicate the gen_store_gpr into both 
branches, so that we don't have to store t0 into the stack across the blocks.

Otherwise,

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


r~

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg
  2016-09-21 20:16   ` Richard Henderson
@ 2016-09-27  7:02     ` Leon Alrae
  0 siblings, 0 replies; 8+ messages in thread
From: Leon Alrae @ 2016-09-27  7:02 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, aurelien

On Wed, Sep 21, 2016 at 01:16:28PM -0700, Richard Henderson wrote:
> On 09/21/2016 01:07 AM, Leon Alrae wrote:
> >+    tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l1);
> >+    tcg_temp_free(addr);
> >+    tcg_gen_movi_tl(t0, 0);
> >+    tcg_gen_br(done);
> >+
> >+    gen_set_label(l1);
> >+    /* generate cmpxchg */
> >+    val = tcg_temp_new();
> >+    gen_load_gpr(val, rt);
> >+    tcg_gen_atomic_cmpxchg_tl(t0, cpu_lladdr, cpu_llval, val,
> >+                              ctx->mem_idx, tcg_mo);
> >+    tcg_gen_setcond_tl(TCG_COND_EQ, t0, t0, cpu_llval);
> >+    tcg_temp_free(val);
> >+
> >+    gen_set_label(done);
> >+    /* store the result into the register */
> >+    gen_store_gpr(t0, rt);
> >     tcg_temp_free(t0);
> 
> The only thing I would change is to duplicate the gen_store_gpr into
> both branches, so that we don't have to store t0 into the stack
> across the blocks.

Done in v3.

> 
> Otherwise,
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks for reviewing.

Leon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg
  2016-09-16 16:48   ` Richard Henderson
@ 2016-09-19 11:35     ` Leon Alrae
  0 siblings, 0 replies; 8+ messages in thread
From: Leon Alrae @ 2016-09-19 11:35 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, aurelien

On Fri, Sep 16, 2016 at 09:48:51AM -0700, Richard Henderson wrote:
> On 09/15/2016 01:44 AM, Leon Alrae wrote:
> > /* Store conditional */
> >+static void gen_st_cond(DisasContext *ctx, int rt, int base, int offset,
> >+                        int size)
> > {
> >+    TCGv addr, t0, val;
> >+    TCGLabel *l1 = gen_new_label();
> >+    TCGLabel *l2 = gen_new_label();
> >+    TCGLabel *done = gen_new_label();
> >
> >-#ifdef CONFIG_USER_ONLY
> >     t0 = tcg_temp_local_new();
> >+    addr = tcg_temp_local_new();
> >+    /* check the alignment of the address */
> >+    gen_base_offset_addr(ctx, addr, base, offset);
> >+    tcg_gen_andi_tl(t0, addr, size - 1);
> 
> You shouldn't have to test the alignment here, as the alignment
> should have been tested during the load-locked, and the (aligned)
> address will be compared.

This is to satisfy the requirement that unaligned SC generates Address
Error exception. But I agree that in practice this doesn't seem
particularly useful since LL will do that.

> 
> 
> >+    /* compare the address against that of the preceeding LL */
> >+    tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l2);
> >+    tcg_gen_movi_tl(t0, 0);
> >+    tcg_gen_br(done);
> ...
> >+#ifdef TARGET_MIPS64
> >+    case 8: /* SCD */
> >+        tcg_gen_atomic_cmpxchg_i64(t0, addr, cpu_llval, val,
> >+                                   ctx->mem_idx, MO_TEQ);
> >         break;
> > #endif
> >-    case OPC_SC:
> >-    case R6_OPC_SC:
> >-        op_st_sc(t1, t0, rt, ctx);
> >+    case 4: /* SC */
> >+        {
> >+            TCGv_i32 val32 = tcg_temp_new_i32();
> >+            TCGv_i32 llval32 = tcg_temp_new_i32();
> >+            TCGv_i32 old32 = tcg_temp_new_i32();
> >+            tcg_gen_trunc_tl_i32(val32, val);
> >+            tcg_gen_trunc_tl_i32(llval32, cpu_llval);
> >+
> >+            tcg_gen_atomic_cmpxchg_i32(old32, addr, llval32, val32,
> >+                                       ctx->mem_idx, MO_TESL);
> >+            tcg_gen_ext_i32_tl(t0, old32);
> 
> You can use tcg_gen_atomic_cmpxchg_tl so that you do not need to do
> all of this truncation yourself.  Which means that if you replace
> the size parameter with a TCGMemOp parameter (MO_TEQ vs MO_TESL) you
> can make all this code common.

Ah, yes.

> 
> Further, local temporaries are less than ideal and should be avoided
> if possible.  Using them results in an extra store into the local
> stack frame.
> 
> We can avoid this for addr by noting that once you have compared
> addr to cpu_lladdr, you can free addr and use cpu_lladdr in the
> actual cmpxchg.

Ok. I'll correct in v2.

Thanks,
Leon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg
  2016-09-15  8:44 ` [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg Leon Alrae
@ 2016-09-16 16:48   ` Richard Henderson
  2016-09-19 11:35     ` Leon Alrae
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2016-09-16 16:48 UTC (permalink / raw)
  To: Leon Alrae, qemu-devel; +Cc: aurelien

On 09/15/2016 01:44 AM, Leon Alrae wrote:
>  /* Store conditional */
> +static void gen_st_cond(DisasContext *ctx, int rt, int base, int offset,
> +                        int size)
>  {
> +    TCGv addr, t0, val;
> +    TCGLabel *l1 = gen_new_label();
> +    TCGLabel *l2 = gen_new_label();
> +    TCGLabel *done = gen_new_label();
>
> -#ifdef CONFIG_USER_ONLY
>      t0 = tcg_temp_local_new();
> +    addr = tcg_temp_local_new();
> +    /* check the alignment of the address */
> +    gen_base_offset_addr(ctx, addr, base, offset);
> +    tcg_gen_andi_tl(t0, addr, size - 1);

You shouldn't have to test the alignment here, as the alignment should have 
been tested during the load-locked, and the (aligned) address will be compared.


> +    /* compare the address against that of the preceeding LL */
> +    tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l2);
> +    tcg_gen_movi_tl(t0, 0);
> +    tcg_gen_br(done);
...
> +#ifdef TARGET_MIPS64
> +    case 8: /* SCD */
> +        tcg_gen_atomic_cmpxchg_i64(t0, addr, cpu_llval, val,
> +                                   ctx->mem_idx, MO_TEQ);
>          break;
>  #endif
> -    case OPC_SC:
> -    case R6_OPC_SC:
> -        op_st_sc(t1, t0, rt, ctx);
> +    case 4: /* SC */
> +        {
> +            TCGv_i32 val32 = tcg_temp_new_i32();
> +            TCGv_i32 llval32 = tcg_temp_new_i32();
> +            TCGv_i32 old32 = tcg_temp_new_i32();
> +            tcg_gen_trunc_tl_i32(val32, val);
> +            tcg_gen_trunc_tl_i32(llval32, cpu_llval);
> +
> +            tcg_gen_atomic_cmpxchg_i32(old32, addr, llval32, val32,
> +                                       ctx->mem_idx, MO_TESL);
> +            tcg_gen_ext_i32_tl(t0, old32);

You can use tcg_gen_atomic_cmpxchg_tl so that you do not need to do all of this 
truncation yourself.  Which means that if you replace the size parameter with a 
TCGMemOp parameter (MO_TEQ vs MO_TESL) you can make all this code common.

Further, local temporaries are less than ideal and should be avoided if 
possible.  Using them results in an extra store into the local stack frame.

We can avoid this for addr by noting that once you have compared addr to 
cpu_lladdr, you can free addr and use cpu_lladdr in the actual cmpxchg.


r~

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg
  2016-09-15  8:44 [Qemu-devel] [PATCH 0/2] target-mips: rework conditional stores for mttcg Leon Alrae
@ 2016-09-15  8:44 ` Leon Alrae
  2016-09-16 16:48   ` Richard Henderson
  0 siblings, 1 reply; 8+ messages in thread
From: Leon Alrae @ 2016-09-15  8:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: aurelien, rth

This patch completely rewrites conditional stores. Now we use cmpxchg and
no longer need separate implementations for user and system emulation.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 linux-user/main.c       |  58 ---------------------
 target-mips/cpu.h       |   4 --
 target-mips/helper.c    |   6 +--
 target-mips/helper.h    |   2 -
 target-mips/op_helper.c |  25 ---------
 target-mips/translate.c | 131 ++++++++++++++++++++++++------------------------
 6 files changed, 67 insertions(+), 159 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 0d0bf9d..bc1b307 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2230,55 +2230,6 @@ static const uint8_t mips_syscall_args[] = {
 #  undef MIPS_SYS
 # endif /* O32 */
 
-static int do_store_exclusive(CPUMIPSState *env)
-{
-    target_ulong addr;
-    target_ulong page_addr;
-    target_ulong val;
-    int flags;
-    int segv = 0;
-    int reg;
-    int d;
-
-    addr = env->lladdr;
-    page_addr = addr & TARGET_PAGE_MASK;
-    start_exclusive();
-    mmap_lock();
-    flags = page_get_flags(page_addr);
-    if ((flags & PAGE_READ) == 0) {
-        segv = 1;
-    } else {
-        reg = env->llreg & 0x1f;
-        d = (env->llreg & 0x20) != 0;
-        if (d) {
-            segv = get_user_s64(val, addr);
-        } else {
-            segv = get_user_s32(val, addr);
-        }
-        if (!segv) {
-            if (val != env->llval) {
-                env->active_tc.gpr[reg] = 0;
-            } else {
-                if (d) {
-                    segv = put_user_u64(env->llnewval, addr);
-                } else {
-                    segv = put_user_u32(env->llnewval, addr);
-                }
-                if (!segv) {
-                    env->active_tc.gpr[reg] = 1;
-                }
-            }
-        }
-    }
-    env->lladdr = -1;
-    if (!segv) {
-        env->active_tc.PC += 4;
-    }
-    mmap_unlock();
-    end_exclusive();
-    return segv;
-}
-
 /* Break codes */
 enum {
     BRK_OVERFLOW = 6,
@@ -2426,15 +2377,6 @@ done_syscall:
                   }
             }
             break;
-        case EXCP_SC:
-            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->active_tc.PC;
-                queue_signal(env, info.si_signo, &info);
-            }
-            break;
         case EXCP_DSPDIS:
             info.si_signo = TARGET_SIGILL;
             info.si_errno = 0;
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 78555b9..6c268f0 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -500,8 +500,6 @@ struct CPUMIPSState {
     /* XXX: Maybe make LLAddr per-TC? */
     target_ulong lladdr; /* LL virtual address compared against SC */
     target_ulong llval;
-    target_ulong llnewval;
-    target_ulong llreg;
     uint64_t CP0_LLAddr_rw_bitmask;
     int CP0_LLAddr_shift;
     target_ulong CP0_WatchLo[8];
@@ -796,8 +794,6 @@ enum {
 
     EXCP_LAST = EXCP_TLBRI,
 };
-/* Dummy exception for conditional stores.  */
-#define EXCP_SC 0x100
 
 /*
  * This is an interrnally generated WAKE request line.
diff --git a/target-mips/helper.c b/target-mips/helper.c
index c864b15..67b19e6 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -958,10 +958,8 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
 {
     CPUState *cs = CPU(mips_env_get_cpu(env));
 
-    if (exception < EXCP_SC) {
-        qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n",
-                      __func__, exception, error_code);
-    }
+    qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n",
+                  __func__, exception, error_code);
     cs->exception_index = exception;
     env->error_code = error_code;
 
diff --git a/target-mips/helper.h b/target-mips/helper.h
index 666936c..dd68751 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -13,10 +13,8 @@ DEF_HELPER_4(swr, void, env, tl, tl, int)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(ll, tl, env, tl, int)
-DEF_HELPER_4(sc, tl, env, tl, tl, int)
 #ifdef TARGET_MIPS64
 DEF_HELPER_3(lld, tl, env, tl, int)
-DEF_HELPER_4(scd, tl, env, tl, tl, int)
 #endif
 #endif
 
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index e0c9842..9f094ad 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -300,31 +300,6 @@ HELPER_LD_ATOMIC(ll, lw, 0x3)
 HELPER_LD_ATOMIC(lld, ld, 0x7)
 #endif
 #undef HELPER_LD_ATOMIC
-
-#define HELPER_ST_ATOMIC(name, ld_insn, st_insn, almask)                      \
-target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1,              \
-                           target_ulong arg2, int mem_idx)                    \
-{                                                                             \
-    target_long tmp;                                                          \
-                                                                              \
-    if (arg2 & almask) {                                                      \
-        env->CP0_BadVAddr = arg2;                                             \
-        do_raise_exception(env, EXCP_AdES, GETPC());                          \
-    }                                                                         \
-    if (arg2 == env->lladdr) {                                                \
-        tmp = do_##ld_insn(env, arg2, mem_idx, GETPC());                      \
-        if (tmp == env->llval) {                                              \
-            do_##st_insn(env, arg2, arg1, mem_idx, GETPC());                  \
-            return 1;                                                         \
-        }                                                                     \
-    }                                                                         \
-    return 0;                                                                 \
-}
-HELPER_ST_ATOMIC(sc, lw, sw, 0x3)
-#ifdef TARGET_MIPS64
-HELPER_ST_ATOMIC(scd, ld, sd, 0x7)
-#endif
-#undef HELPER_ST_ATOMIC
 #endif
 
 #ifdef TARGET_WORDS_BIGENDIAN
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 5d0732f..1936739 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1364,6 +1364,7 @@ static TCGv_i32 hflags;
 static TCGv_i32 fpu_fcr0, fpu_fcr31;
 static TCGv_i64 fpu_f64[32];
 static TCGv_i64 msa_wr_d[64];
+static TCGv cpu_lladdr, cpu_llval;
 
 #include "exec/gen-icount.h"
 
@@ -2050,46 +2051,6 @@ OP_LD_ATOMIC(lld,ld64);
 #endif
 #undef OP_LD_ATOMIC
 
-#ifdef CONFIG_USER_ONLY
-#define OP_ST_ATOMIC(insn,fname,ldname,almask)                               \
-static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx) \
-{                                                                            \
-    TCGv t0 = tcg_temp_new();                                                \
-    TCGLabel *l1 = gen_new_label();                                          \
-    TCGLabel *l2 = gen_new_label();                                          \
-                                                                             \
-    tcg_gen_andi_tl(t0, arg2, almask);                                       \
-    tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, l1);                              \
-    tcg_gen_st_tl(arg2, cpu_env, offsetof(CPUMIPSState, CP0_BadVAddr));          \
-    generate_exception(ctx, EXCP_AdES);                                      \
-    gen_set_label(l1);                                                       \
-    tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUMIPSState, lladdr));                  \
-    tcg_gen_brcond_tl(TCG_COND_NE, arg2, t0, l2);                            \
-    tcg_gen_movi_tl(t0, rt | ((almask << 3) & 0x20));                        \
-    tcg_gen_st_tl(t0, cpu_env, offsetof(CPUMIPSState, llreg));                   \
-    tcg_gen_st_tl(arg1, cpu_env, offsetof(CPUMIPSState, llnewval));              \
-    generate_exception_end(ctx, EXCP_SC);                                    \
-    gen_set_label(l2);                                                       \
-    tcg_gen_movi_tl(t0, 0);                                                  \
-    gen_store_gpr(t0, rt);                                                   \
-    tcg_temp_free(t0);                                                       \
-}
-#else
-#define OP_ST_ATOMIC(insn,fname,ldname,almask)                               \
-static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, DisasContext *ctx) \
-{                                                                            \
-    TCGv t0 = tcg_temp_new();                                                \
-    gen_helper_1e2i(insn, t0, arg1, arg2, ctx->mem_idx);                     \
-    gen_store_gpr(t0, rt);                                                   \
-    tcg_temp_free(t0);                                                       \
-}
-#endif
-OP_ST_ATOMIC(sc,st32,ld32s,0x3);
-#if defined(TARGET_MIPS64)
-OP_ST_ATOMIC(scd,st64,ld64,0x7);
-#endif
-#undef OP_ST_ATOMIC
-
 static void gen_base_offset_addr (DisasContext *ctx, TCGv addr,
                                   int base, int16_t offset)
 {
@@ -2335,33 +2296,66 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
 
 
 /* Store conditional */
-static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt,
-                         int base, int16_t offset)
+static void gen_st_cond(DisasContext *ctx, int rt, int base, int offset,
+                        int size)
 {
-    TCGv t0, t1;
+    TCGv addr, t0, val;
+    TCGLabel *l1 = gen_new_label();
+    TCGLabel *l2 = gen_new_label();
+    TCGLabel *done = gen_new_label();
 
-#ifdef CONFIG_USER_ONLY
     t0 = tcg_temp_local_new();
-    t1 = tcg_temp_local_new();
-#else
-    t0 = tcg_temp_new();
-    t1 = tcg_temp_new();
-#endif
-    gen_base_offset_addr(ctx, t0, base, offset);
-    gen_load_gpr(t1, rt);
-    switch (opc) {
-#if defined(TARGET_MIPS64)
-    case OPC_SCD:
-    case R6_OPC_SCD:
-        op_st_scd(t1, t0, rt, ctx);
+    addr = tcg_temp_local_new();
+    /* check the alignment of the address */
+    gen_base_offset_addr(ctx, addr, base, offset);
+    tcg_gen_andi_tl(t0, addr, size - 1);
+    tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, l1);
+    tcg_gen_st_tl(addr, cpu_env, offsetof(CPUMIPSState, CP0_BadVAddr));
+    generate_exception(ctx, EXCP_AdES);
+    tcg_gen_br(done);
+
+    gen_set_label(l1);
+    /* compare the address against that of the preceeding LL */
+    tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l2);
+    tcg_gen_movi_tl(t0, 0);
+    tcg_gen_br(done);
+
+    gen_set_label(l2);
+    /* generate cmpxchg */
+    val = tcg_temp_new();
+    gen_load_gpr(val, rt);
+    switch (size) {
+#ifdef TARGET_MIPS64
+    case 8: /* SCD */
+        tcg_gen_atomic_cmpxchg_i64(t0, addr, cpu_llval, val,
+                                   ctx->mem_idx, MO_TEQ);
         break;
 #endif
-    case OPC_SC:
-    case R6_OPC_SC:
-        op_st_sc(t1, t0, rt, ctx);
+    case 4: /* SC */
+        {
+            TCGv_i32 val32 = tcg_temp_new_i32();
+            TCGv_i32 llval32 = tcg_temp_new_i32();
+            TCGv_i32 old32 = tcg_temp_new_i32();
+            tcg_gen_trunc_tl_i32(val32, val);
+            tcg_gen_trunc_tl_i32(llval32, cpu_llval);
+
+            tcg_gen_atomic_cmpxchg_i32(old32, addr, llval32, val32,
+                                       ctx->mem_idx, MO_TESL);
+            tcg_gen_ext_i32_tl(t0, old32);
+
+            tcg_temp_free_i32(old32);
+            tcg_temp_free_i32(llval32);
+            tcg_temp_free_i32(val32);
+        }
         break;
     }
-    tcg_temp_free(t1);
+    tcg_gen_setcond_tl(TCG_COND_EQ, t0, t0, cpu_llval);
+    tcg_temp_free(val);
+
+    gen_set_label(done);
+    /* store the result into the register */
+    gen_store_gpr(t0, rt);
+    tcg_temp_free(addr);
     tcg_temp_free(t0);
 }
 
@@ -14700,13 +14694,13 @@ static void decode_micromips32_opc(CPUMIPSState *env, DisasContext *ctx)
             gen_st(ctx, mips32_op, rt, rs, SIMM(ctx->opcode, 0, 12));
             break;
         case SC:
-            gen_st_cond(ctx, OPC_SC, rt, rs, offset);
+            gen_st_cond(ctx, rt, rs, offset, 4);
             break;
 #if defined(TARGET_MIPS64)
         case SCD:
             check_insn(ctx, ISA_MIPS3);
             check_mips_64(ctx);
-            gen_st_cond(ctx, OPC_SCD, rt, rs, offset);
+            gen_st_cond(ctx, rt, rs, offset, 8);
             break;
 #endif
         case PREF:
@@ -17421,7 +17415,7 @@ static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx)
         }
         break;
     case R6_OPC_SC:
-        gen_st_cond(ctx, op1, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, 4);
         break;
     case R6_OPC_LL:
         gen_ld(ctx, op1, rt, rs, imm);
@@ -17445,7 +17439,7 @@ static void decode_opc_special3_r6(CPUMIPSState *env, DisasContext *ctx)
         break;
 #if defined(TARGET_MIPS64)
     case R6_OPC_SCD:
-        gen_st_cond(ctx, op1, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, 8);
         break;
     case R6_OPC_LLD:
         gen_ld(ctx, op1, rt, rs, imm);
@@ -19521,7 +19515,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
     case OPC_SC:
         check_insn(ctx, ISA_MIPS2);
          check_insn_opc_removed(ctx, ISA_MIPS32R6);
-         gen_st_cond(ctx, op, rt, rs, imm);
+         gen_st_cond(ctx, rt, rs, imm, 4);
          break;
     case OPC_CACHE:
         check_insn_opc_removed(ctx, ISA_MIPS32R6);
@@ -19807,7 +19801,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
         check_insn_opc_removed(ctx, ISA_MIPS32R6);
         check_insn(ctx, ISA_MIPS3);
         check_mips_64(ctx);
-        gen_st_cond(ctx, op, rt, rs, imm);
+        gen_st_cond(ctx, rt, rs, imm, 8);
         break;
     case OPC_BNVC: /* OPC_BNEZALC, OPC_BNEC, OPC_DADDI */
         if (ctx->insn_flags & ISA_MIPS32R6) {
@@ -20180,6 +20174,11 @@ void mips_tcg_init(void)
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
 
+    cpu_lladdr = tcg_global_mem_new(cpu_env, offsetof(CPUMIPSState, lladdr),
+                                    "lladdr");
+    cpu_llval = tcg_global_mem_new(cpu_env, offsetof(CPUMIPSState, llval),
+                                   "llval");
+
     inited = 1;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-09-27  7:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-21  8:07 [Qemu-devel] [PATCH 0/2] target-mips: rework conditional stores for mttcg Leon Alrae
2016-09-21  8:07 ` [Qemu-devel] [PATCH 1/2] target-mips: compare virtual addresses in LL/SC sequence Leon Alrae
2016-09-21  8:07 ` [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg Leon Alrae
2016-09-21 20:16   ` Richard Henderson
2016-09-27  7:02     ` Leon Alrae
  -- strict thread matches above, loose matches on Subject: below --
2016-09-15  8:44 [Qemu-devel] [PATCH 0/2] target-mips: rework conditional stores for mttcg Leon Alrae
2016-09-15  8:44 ` [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg Leon Alrae
2016-09-16 16:48   ` Richard Henderson
2016-09-19 11:35     ` Leon Alrae

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.