All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] target-openrisc: Add atomic instructions
@ 2015-01-25 10:25 Sebastian Macke
  2015-01-25 10:25 ` [Qemu-devel] [PATCH 1/2] target-openrisc: Separate of load/store instructions Sebastian Macke
  2015-01-25 10:25 ` [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support Sebastian Macke
  0 siblings, 2 replies; 8+ messages in thread
From: Sebastian Macke @ 2015-01-25 10:25 UTC (permalink / raw)
  To: qemu-devel, proljc, blue; +Cc: Sebastian Macke

This patch adds the new atomic operations of the
Load-link/store-conditional type which are called
l.lwa and l.swa.

For a cleaner implementation, all load and store instructions
are separated to a function.

Christian Svensson (1):
  target-openrisc: Add l.lwa/l.swa support

Sebastian Macke (1):
  target-openrisc: Separate of load/store instructions

 target-openrisc/cpu.h       |   3 +
 target-openrisc/interrupt.c |   3 +
 target-openrisc/translate.c | 188 ++++++++++++++++++++++++++++++++++----------
 3 files changed, 153 insertions(+), 41 deletions(-)

-- 
2.2.2

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

* [Qemu-devel] [PATCH 1/2] target-openrisc: Separate of load/store instructions
  2015-01-25 10:25 [Qemu-devel] [PATCH 0/2] target-openrisc: Add atomic instructions Sebastian Macke
@ 2015-01-25 10:25 ` Sebastian Macke
  2015-01-26  9:47   ` Jia Liu
  2015-01-25 10:25 ` [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support Sebastian Macke
  1 sibling, 1 reply; 8+ messages in thread
From: Sebastian Macke @ 2015-01-25 10:25 UTC (permalink / raw)
  To: qemu-devel, proljc, blue; +Cc: Sebastian Macke

This patch separates the load and store instruction to a
separate function.
The repetition of the source code can be reduced and further
optimizations can be implemented.
In this case it checks for a zero offset and optimizes it.

Signed-off-by: Sebastian Macke <sebastian@macke.de>
---
 target-openrisc/translate.c | 119 +++++++++++++++++++++++++++++---------------
 1 file changed, 78 insertions(+), 41 deletions(-)

diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index b90181d..543aa67 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -254,6 +254,63 @@ static void gen_jump(DisasContext *dc, uint32_t imm, uint32_t reg, uint32_t op0)
     gen_sync_flags(dc);
 }
 
+static void gen_loadstore(DisasContext *dc, uint32 op0,
+                          uint32_t ra, uint32_t rb, uint32_t rd,
+                          uint32_t offset)
+{
+    TCGv t0 = cpu_R[ra];
+    if (offset != 0) {
+        t0 = tcg_temp_new();
+        tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(offset, 16));
+    }
+
+    switch (op0) {
+    case 0x21:    /* l.lwz */
+        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TEUL);
+        break;
+
+    case 0x22:    /* l.lws */
+        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TESL);
+        break;
+
+    case 0x23:    /* l.lbz */
+        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_UB);
+        break;
+
+    case 0x24:    /* l.lbs */
+        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_SB);
+        break;
+
+    case 0x25:    /* l.lhz */
+        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TEUW);
+        break;
+
+    case 0x26:    /* l.lhs */
+        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TESW);
+        break;
+
+    case 0x35:    /* l.sw */
+        tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_TEUL);
+        break;
+
+    case 0x36:    /* l.sb */
+        tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_UB);
+        break;
+
+    case 0x37:    /* l.sh */
+        tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_TEUW);
+        break;
+
+    default:
+    break;
+    }
+
+    if (offset != 0) {
+        tcg_temp_free(t0);
+    }
+
+}
+
 
 static void dec_calc(DisasContext *dc, uint32_t insn)
 {
@@ -702,6 +759,9 @@ static void dec_calc(DisasContext *dc, uint32_t insn)
     }
 }
 
+
+
+
 static void dec_misc(DisasContext *dc, uint32_t insn)
 {
     uint32_t op0, op1;
@@ -710,7 +770,6 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
     uint32_t L6, K5;
 #endif
     uint32_t I16, I5, I11, N26, tmp;
-    TCGMemOp mop;
 
     op0 = extract32(insn, 26, 6);
     op1 = extract32(insn, 24, 2);
@@ -843,48 +902,37 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
 /*#ifdef TARGET_OPENRISC64
     case 0x20:     l.ld
         LOG_DIS("l.ld r%d, r%d, %d\n", rd, ra, I16);
-        check_ob64s(dc);
-        mop = MO_TEQ;
-        goto do_load;
+        gen_loadstore(dc, op0, ra, rb, rd, I16);
 #endif*/
 
     case 0x21:    /* l.lwz */
         LOG_DIS("l.lwz r%d, r%d, %d\n", rd, ra, I16);
-        mop = MO_TEUL;
-        goto do_load;
+        gen_loadstore(dc, op0, ra, rb, rd, I16);
+        break;
 
     case 0x22:    /* l.lws */
         LOG_DIS("l.lws r%d, r%d, %d\n", rd, ra, I16);
-        mop = MO_TESL;
-        goto do_load;
+        gen_loadstore(dc, op0, ra, rb, rd, I16);
+        break;
 
     case 0x23:    /* l.lbz */
         LOG_DIS("l.lbz r%d, r%d, %d\n", rd, ra, I16);
-        mop = MO_UB;
-        goto do_load;
+        gen_loadstore(dc, op0, ra, rb, rd, I16);
+        break;
 
     case 0x24:    /* l.lbs */
         LOG_DIS("l.lbs r%d, r%d, %d\n", rd, ra, I16);
-        mop = MO_SB;
-        goto do_load;
+        gen_loadstore(dc, op0, ra, rb, rd, I16);
+        break;
 
     case 0x25:    /* l.lhz */
         LOG_DIS("l.lhz r%d, r%d, %d\n", rd, ra, I16);
-        mop = MO_TEUW;
-        goto do_load;
+        gen_loadstore(dc, op0, ra, rb, rd, I16);
+        break;
 
     case 0x26:    /* l.lhs */
         LOG_DIS("l.lhs r%d, r%d, %d\n", rd, ra, I16);
-        mop = MO_TESW;
-        goto do_load;
-
-    do_load:
-        {
-            TCGv t0 = tcg_temp_new();
-            tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(I16, 16));
-            tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, mop);
-            tcg_temp_free(t0);
-        }
+        gen_loadstore(dc, op0, ra, rb, rd, I16);
         break;
 
     case 0x27:    /* l.addi */
@@ -1021,33 +1069,22 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
 /*#ifdef TARGET_OPENRISC64
     case 0x34:     l.sd
         LOG_DIS("l.sd %d, r%d, r%d, %d\n", I5, ra, rb, I11);
-        check_ob64s(dc);
-        mop = MO_TEQ;
-        goto do_store;
+        gen_loadstore(dc, op0, ra, rb, rd, tmp);
 #endif*/
 
     case 0x35:    /* l.sw */
         LOG_DIS("l.sw %d, r%d, r%d, %d\n", I5, ra, rb, I11);
-        mop = MO_TEUL;
-        goto do_store;
+        gen_loadstore(dc, op0, ra, rb, rd, tmp);
+        break;
 
     case 0x36:    /* l.sb */
         LOG_DIS("l.sb %d, r%d, r%d, %d\n", I5, ra, rb, I11);
-        mop = MO_UB;
-        goto do_store;
+        gen_loadstore(dc, op0, ra, rb, rd, tmp);
+        break;
 
     case 0x37:    /* l.sh */
         LOG_DIS("l.sh %d, r%d, r%d, %d\n", I5, ra, rb, I11);
-        mop = MO_TEUW;
-        goto do_store;
-
-    do_store:
-        {
-            TCGv t0 = tcg_temp_new();
-            tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(tmp, 16));
-            tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, mop);
-            tcg_temp_free(t0);
-        }
+        gen_loadstore(dc, op0, ra, rb, rd, tmp);
         break;
 
     default:
-- 
2.2.2

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

* [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support
  2015-01-25 10:25 [Qemu-devel] [PATCH 0/2] target-openrisc: Add atomic instructions Sebastian Macke
  2015-01-25 10:25 ` [Qemu-devel] [PATCH 1/2] target-openrisc: Separate of load/store instructions Sebastian Macke
@ 2015-01-25 10:25 ` Sebastian Macke
  2015-01-26  9:50   ` Jia Liu
  1 sibling, 1 reply; 8+ messages in thread
From: Sebastian Macke @ 2015-01-25 10:25 UTC (permalink / raw)
  To: qemu-devel, proljc, blue; +Cc: Sebastian Macke

From: Christian Svensson <blue@cmd.nu>

This patch adds support for atomic locks
and is an adaption from https://github.com/bluecmd/or1k-qemu/commits/or32-optimize

Tested via the atomic lock implementation of musl

Signed-off-by: Christian Svensson <blue@cmd.nu>
Signed-off-by: Sebastian Macke <sebastian@macke.de>
---
 target-openrisc/cpu.h       |  3 ++
 target-openrisc/interrupt.c |  3 ++
 target-openrisc/translate.c | 77 ++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
index 69b96c6..abdba75 100644
--- a/target-openrisc/cpu.h
+++ b/target-openrisc/cpu.h
@@ -302,6 +302,9 @@ typedef struct CPUOpenRISCState {
                                  in solt so far.  */
     uint32_t btaken;          /* the SR_F bit */
 
+    target_ulong lock_addr;   /* Atomicity lock address. */
+    target_ulong lock_value;  /* Atomicity lock value. */
+
     CPU_COMMON
 
     /* Fields from here on are preserved across CPU reset. */
diff --git a/target-openrisc/interrupt.c b/target-openrisc/interrupt.c
index e480cfd..68d554c 100644
--- a/target-openrisc/interrupt.c
+++ b/target-openrisc/interrupt.c
@@ -54,6 +54,9 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
     env->tlb->cpu_openrisc_map_address_data = &cpu_openrisc_get_phys_nommu;
     env->tlb->cpu_openrisc_map_address_code = &cpu_openrisc_get_phys_nommu;
 
+    /* invalidate lock */
+    env->cpu_lock_addr = -1;
+
     if (cs->exception_index > 0 && cs->exception_index < EXCP_NR) {
         env->pc = (cs->exception_index << 8);
     } else {
diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index 543aa67..6401b4b 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -55,6 +55,8 @@ typedef struct DisasContext {
 static TCGv_ptr cpu_env;
 static TCGv cpu_sr;
 static TCGv cpu_R[32];
+static TCGv cpu_lock_addr;
+static TCGv cpu_lock_value;
 static TCGv cpu_pc;
 static TCGv jmp_pc;            /* l.jr/l.jalr temp pc */
 static TCGv cpu_npc;
@@ -82,6 +84,12 @@ void openrisc_translate_init(void)
     env_flags = tcg_global_mem_new_i32(TCG_AREG0,
                                        offsetof(CPUOpenRISCState, flags),
                                        "flags");
+    cpu_lock_addr = tcg_global_mem_new(TCG_AREG0,
+                                       offsetof(CPUOpenRISCState, lock_addr),
+                                       "lock_addr");
+    cpu_lock_value = tcg_global_mem_new(TCG_AREG0,
+                                        offsetof(CPUOpenRISCState, lock_value),
+                                        "lock_value");
     cpu_pc = tcg_global_mem_new(TCG_AREG0,
                                 offsetof(CPUOpenRISCState, pc), "pc");
     cpu_npc = tcg_global_mem_new(TCG_AREG0,
@@ -254,17 +262,67 @@ static void gen_jump(DisasContext *dc, uint32_t imm, uint32_t reg, uint32_t op0)
     gen_sync_flags(dc);
 }
 
+/* According to the OpenRISC specification we should poison our atomic lock
+ * if any other store is detected to the same address. For the sake of speed
+ * and because we're single-threaded we guarantee that atomic stores
+ * fail only if an atomic load or another atomic store
+ * is executed.
+ *
+ * To prevent the potential case of an ordinary store, we save
+ * the *value* of the address at the lock time. */
+
+static void gen_atomic_load(TCGv tD, TCGv t0, DisasContext *dc)
+{
+    tcg_gen_qemu_ld_tl(tD, t0, dc->mem_idx, MO_TEUL);
+    tcg_gen_mov_i32(cpu_lock_addr, t0);
+    tcg_gen_mov_i32(cpu_lock_value, tD);
+}
+
+static void gen_atomic_store(TCGv tB, TCGv t0, DisasContext *dc)
+{
+    int store_fail;
+    int store_done;
+
+    store_fail = gen_new_label();
+    store_done = gen_new_label();
+
+    /* check address */
+    tcg_gen_brcond_i32(TCG_COND_NE, t0, cpu_lock_addr, store_fail);
+
+    /* check value */
+    TCGv val = tcg_temp_new();
+    tcg_gen_qemu_ld_tl(val, t0, dc->mem_idx, MO_TEUL);
+    tcg_gen_brcond_i32(TCG_COND_NE, val, cpu_lock_value, store_fail);
+    tcg_temp_free(val);
+
+    /* success of atomic access */
+    tcg_gen_qemu_st_tl(tB, t0, dc->mem_idx, MO_TEUL);
+    tcg_gen_ori_tl(cpu_sr, cpu_sr, SR_F);
+    tcg_gen_br(store_done);
+
+    gen_set_label(store_fail);
+    tcg_gen_andi_tl(cpu_sr, cpu_sr, ~SR_F);
+
+    gen_set_label(store_done);
+    /* the atomic store invalidates the lock address. */
+    tcg_gen_movi_i32(cpu_lock_addr, -1);
+}
+
 static void gen_loadstore(DisasContext *dc, uint32 op0,
                           uint32_t ra, uint32_t rb, uint32_t rd,
                           uint32_t offset)
 {
     TCGv t0 = cpu_R[ra];
     if (offset != 0) {
-        t0 = tcg_temp_new();
+        t0 = tcg_temp_local_new();
         tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(offset, 16));
     }
 
     switch (op0) {
+    case 0x1b:    /* l.lwa */
+        gen_atomic_load(cpu_R[rd], t0, dc);
+        break;
+
     case 0x21:    /* l.lwz */
         tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TEUL);
         break;
@@ -289,6 +347,10 @@ static void gen_loadstore(DisasContext *dc, uint32 op0,
         tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TESW);
         break;
 
+    case 0x33:    /* l.swa */
+        gen_atomic_store(cpu_R[rb], t0, dc);
+        break;
+
     case 0x35:    /* l.sw */
         tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_TEUL);
         break;
@@ -759,9 +821,6 @@ static void dec_calc(DisasContext *dc, uint32_t insn)
     }
 }
 
-
-
-
 static void dec_misc(DisasContext *dc, uint32_t insn)
 {
     uint32_t op0, op1;
@@ -905,6 +964,11 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
         gen_loadstore(dc, op0, ra, rb, rd, I16);
 #endif*/
 
+    case 0x1b:    /* l.lwa */
+        LOG_DIS("l.lwa r%d, r%d, %d\n", rd, ra, I16);
+        gen_loadstore(dc, op0, ra, rb, rd, I16);
+        break;
+
     case 0x21:    /* l.lwz */
         LOG_DIS("l.lwz r%d, r%d, %d\n", rd, ra, I16);
         gen_loadstore(dc, op0, ra, rb, rd, I16);
@@ -1072,6 +1136,11 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
         gen_loadstore(dc, op0, ra, rb, rd, tmp);
 #endif*/
 
+    case 0x33:    /* l.swa */
+        LOG_DIS("l.swa %d, r%d, r%d, %d\n", I5, ra, rb, I11);
+        gen_loadstore(dc, op0, ra, rb, rd, tmp);
+        break;
+
     case 0x35:    /* l.sw */
         LOG_DIS("l.sw %d, r%d, r%d, %d\n", I5, ra, rb, I11);
         gen_loadstore(dc, op0, ra, rb, rd, tmp);
-- 
2.2.2

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

* Re: [Qemu-devel] [PATCH 1/2] target-openrisc: Separate of load/store instructions
  2015-01-25 10:25 ` [Qemu-devel] [PATCH 1/2] target-openrisc: Separate of load/store instructions Sebastian Macke
@ 2015-01-26  9:47   ` Jia Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Jia Liu @ 2015-01-26  9:47 UTC (permalink / raw)
  To: Sebastian Macke; +Cc: qemu-devel, blue

Hi Sebastian,

On Sun, Jan 25, 2015 at 6:25 PM, Sebastian Macke <sebastian@macke.de> wrote:
> This patch separates the load and store instruction to a
> separate function.
> The repetition of the source code can be reduced and further
> optimizations can be implemented.
> In this case it checks for a zero offset and optimizes it.
>
> Signed-off-by: Sebastian Macke <sebastian@macke.de>
> ---
>  target-openrisc/translate.c | 119 +++++++++++++++++++++++++++++---------------
>  1 file changed, 78 insertions(+), 41 deletions(-)
>
> diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
> index b90181d..543aa67 100644
> --- a/target-openrisc/translate.c
> +++ b/target-openrisc/translate.c
> @@ -254,6 +254,63 @@ static void gen_jump(DisasContext *dc, uint32_t imm, uint32_t reg, uint32_t op0)
>      gen_sync_flags(dc);
>  }
>
> +static void gen_loadstore(DisasContext *dc, uint32 op0,
> +                          uint32_t ra, uint32_t rb, uint32_t rd,
> +                          uint32_t offset)
> +{
> +    TCGv t0 = cpu_R[ra];
> +    if (offset != 0) {
> +        t0 = tcg_temp_new();
> +        tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(offset, 16));
> +    }
> +
> +    switch (op0) {
> +    case 0x21:    /* l.lwz */
> +        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TEUL);
> +        break;
> +
> +    case 0x22:    /* l.lws */
> +        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TESL);
> +        break;
> +
> +    case 0x23:    /* l.lbz */
> +        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_UB);
> +        break;
> +
> +    case 0x24:    /* l.lbs */
> +        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_SB);
> +        break;
> +
> +    case 0x25:    /* l.lhz */
> +        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TEUW);
> +        break;
> +
> +    case 0x26:    /* l.lhs */
> +        tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TESW);
> +        break;
> +
> +    case 0x35:    /* l.sw */
> +        tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_TEUL);
> +        break;
> +
> +    case 0x36:    /* l.sb */
> +        tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_UB);
> +        break;
> +
> +    case 0x37:    /* l.sh */
> +        tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_TEUW);
> +        break;
> +
> +    default:
> +    break;
> +    }
> +
> +    if (offset != 0) {
> +        tcg_temp_free(t0);
> +    }
> +
> +}
> +
>
>  static void dec_calc(DisasContext *dc, uint32_t insn)
>  {
> @@ -702,6 +759,9 @@ static void dec_calc(DisasContext *dc, uint32_t insn)
>      }
>  }
>
> +
> +
> +
>  static void dec_misc(DisasContext *dc, uint32_t insn)
>  {
>      uint32_t op0, op1;
> @@ -710,7 +770,6 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
>      uint32_t L6, K5;
>  #endif
>      uint32_t I16, I5, I11, N26, tmp;
> -    TCGMemOp mop;
>
>      op0 = extract32(insn, 26, 6);
>      op1 = extract32(insn, 24, 2);
> @@ -843,48 +902,37 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
>  /*#ifdef TARGET_OPENRISC64
>      case 0x20:     l.ld
>          LOG_DIS("l.ld r%d, r%d, %d\n", rd, ra, I16);
> -        check_ob64s(dc);
> -        mop = MO_TEQ;
> -        goto do_load;
> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
>  #endif*/
>
>      case 0x21:    /* l.lwz */
>          LOG_DIS("l.lwz r%d, r%d, %d\n", rd, ra, I16);
> -        mop = MO_TEUL;
> -        goto do_load;
> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
> +        break;
>
>      case 0x22:    /* l.lws */
>          LOG_DIS("l.lws r%d, r%d, %d\n", rd, ra, I16);
> -        mop = MO_TESL;
> -        goto do_load;
> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
> +        break;
>
>      case 0x23:    /* l.lbz */
>          LOG_DIS("l.lbz r%d, r%d, %d\n", rd, ra, I16);
> -        mop = MO_UB;
> -        goto do_load;
> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
> +        break;
>
>      case 0x24:    /* l.lbs */
>          LOG_DIS("l.lbs r%d, r%d, %d\n", rd, ra, I16);
> -        mop = MO_SB;
> -        goto do_load;
> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
> +        break;
>
>      case 0x25:    /* l.lhz */
>          LOG_DIS("l.lhz r%d, r%d, %d\n", rd, ra, I16);
> -        mop = MO_TEUW;
> -        goto do_load;
> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
> +        break;
>
>      case 0x26:    /* l.lhs */
>          LOG_DIS("l.lhs r%d, r%d, %d\n", rd, ra, I16);
> -        mop = MO_TESW;
> -        goto do_load;
> -
> -    do_load:
> -        {
> -            TCGv t0 = tcg_temp_new();
> -            tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(I16, 16));
> -            tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, mop);
> -            tcg_temp_free(t0);
> -        }
> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
>          break;
>
>      case 0x27:    /* l.addi */
> @@ -1021,33 +1069,22 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
>  /*#ifdef TARGET_OPENRISC64
>      case 0x34:     l.sd
>          LOG_DIS("l.sd %d, r%d, r%d, %d\n", I5, ra, rb, I11);
> -        check_ob64s(dc);
> -        mop = MO_TEQ;
> -        goto do_store;
> +        gen_loadstore(dc, op0, ra, rb, rd, tmp);
>  #endif*/
>
>      case 0x35:    /* l.sw */
>          LOG_DIS("l.sw %d, r%d, r%d, %d\n", I5, ra, rb, I11);
> -        mop = MO_TEUL;
> -        goto do_store;
> +        gen_loadstore(dc, op0, ra, rb, rd, tmp);
> +        break;
>
>      case 0x36:    /* l.sb */
>          LOG_DIS("l.sb %d, r%d, r%d, %d\n", I5, ra, rb, I11);
> -        mop = MO_UB;
> -        goto do_store;
> +        gen_loadstore(dc, op0, ra, rb, rd, tmp);
> +        break;
>
>      case 0x37:    /* l.sh */
>          LOG_DIS("l.sh %d, r%d, r%d, %d\n", I5, ra, rb, I11);
> -        mop = MO_TEUW;
> -        goto do_store;
> -
> -    do_store:
> -        {
> -            TCGv t0 = tcg_temp_new();
> -            tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(tmp, 16));
> -            tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, mop);
> -            tcg_temp_free(t0);
> -        }
> +        gen_loadstore(dc, op0, ra, rb, rd, tmp);
>          break;
>
>      default:

Thank you, it is good to separate the related instructions.
Reviwed-by: Jia Liu <proljc@gmail.com>

> --
> 2.2.2
>

Regards,
Jia

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

* Re: [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support
  2015-01-25 10:25 ` [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support Sebastian Macke
@ 2015-01-26  9:50   ` Jia Liu
  2015-01-26 10:15     ` Christian Svensson
  2015-01-26 10:18     ` Sebastian Macke
  0 siblings, 2 replies; 8+ messages in thread
From: Jia Liu @ 2015-01-26  9:50 UTC (permalink / raw)
  To: Sebastian Macke; +Cc: qemu-devel, blue

Hi Sebastian, Christian

On Sun, Jan 25, 2015 at 6:25 PM, Sebastian Macke <sebastian@macke.de> wrote:
> From: Christian Svensson <blue@cmd.nu>
>
> This patch adds support for atomic locks
> and is an adaption from https://github.com/bluecmd/or1k-qemu/commits/or32-optimize
>
> Tested via the atomic lock implementation of musl
>
> Signed-off-by: Christian Svensson <blue@cmd.nu>
> Signed-off-by: Sebastian Macke <sebastian@macke.de>
> ---
>  target-openrisc/cpu.h       |  3 ++
>  target-openrisc/interrupt.c |  3 ++
>  target-openrisc/translate.c | 77 ++++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 79 insertions(+), 4 deletions(-)
>
> diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
> index 69b96c6..abdba75 100644
> --- a/target-openrisc/cpu.h
> +++ b/target-openrisc/cpu.h
> @@ -302,6 +302,9 @@ typedef struct CPUOpenRISCState {
>                                   in solt so far.  */
>      uint32_t btaken;          /* the SR_F bit */
>
> +    target_ulong lock_addr;   /* Atomicity lock address. */
> +    target_ulong lock_value;  /* Atomicity lock value. */
> +
>      CPU_COMMON
>
>      /* Fields from here on are preserved across CPU reset. */
> diff --git a/target-openrisc/interrupt.c b/target-openrisc/interrupt.c
> index e480cfd..68d554c 100644
> --- a/target-openrisc/interrupt.c
> +++ b/target-openrisc/interrupt.c
> @@ -54,6 +54,9 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
>      env->tlb->cpu_openrisc_map_address_data = &cpu_openrisc_get_phys_nommu;
>      env->tlb->cpu_openrisc_map_address_code = &cpu_openrisc_get_phys_nommu;
>
> +    /* invalidate lock */
> +    env->cpu_lock_addr = -1;
> +
>      if (cs->exception_index > 0 && cs->exception_index < EXCP_NR) {
>          env->pc = (cs->exception_index << 8);
>      } else {
> diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
> index 543aa67..6401b4b 100644
> --- a/target-openrisc/translate.c
> +++ b/target-openrisc/translate.c
> @@ -55,6 +55,8 @@ typedef struct DisasContext {
>  static TCGv_ptr cpu_env;
>  static TCGv cpu_sr;
>  static TCGv cpu_R[32];
> +static TCGv cpu_lock_addr;
> +static TCGv cpu_lock_value;
>  static TCGv cpu_pc;
>  static TCGv jmp_pc;            /* l.jr/l.jalr temp pc */
>  static TCGv cpu_npc;
> @@ -82,6 +84,12 @@ void openrisc_translate_init(void)
>      env_flags = tcg_global_mem_new_i32(TCG_AREG0,
>                                         offsetof(CPUOpenRISCState, flags),
>                                         "flags");
> +    cpu_lock_addr = tcg_global_mem_new(TCG_AREG0,
> +                                       offsetof(CPUOpenRISCState, lock_addr),
> +                                       "lock_addr");
> +    cpu_lock_value = tcg_global_mem_new(TCG_AREG0,
> +                                        offsetof(CPUOpenRISCState, lock_value),
> +                                        "lock_value");
>      cpu_pc = tcg_global_mem_new(TCG_AREG0,
>                                  offsetof(CPUOpenRISCState, pc), "pc");
>      cpu_npc = tcg_global_mem_new(TCG_AREG0,
> @@ -254,17 +262,67 @@ static void gen_jump(DisasContext *dc, uint32_t imm, uint32_t reg, uint32_t op0)
>      gen_sync_flags(dc);
>  }
>
> +/* According to the OpenRISC specification we should poison our atomic lock
> + * if any other store is detected to the same address. For the sake of speed
> + * and because we're single-threaded we guarantee that atomic stores
> + * fail only if an atomic load or another atomic store
> + * is executed.
> + *
> + * To prevent the potential case of an ordinary store, we save
> + * the *value* of the address at the lock time. */
> +
> +static void gen_atomic_load(TCGv tD, TCGv t0, DisasContext *dc)
> +{
> +    tcg_gen_qemu_ld_tl(tD, t0, dc->mem_idx, MO_TEUL);
> +    tcg_gen_mov_i32(cpu_lock_addr, t0);
> +    tcg_gen_mov_i32(cpu_lock_value, tD);
> +}
> +
> +static void gen_atomic_store(TCGv tB, TCGv t0, DisasContext *dc)
> +{
> +    int store_fail;
> +    int store_done;
> +
> +    store_fail = gen_new_label();
> +    store_done = gen_new_label();
> +
> +    /* check address */
> +    tcg_gen_brcond_i32(TCG_COND_NE, t0, cpu_lock_addr, store_fail);
> +
> +    /* check value */
> +    TCGv val = tcg_temp_new();
> +    tcg_gen_qemu_ld_tl(val, t0, dc->mem_idx, MO_TEUL);
> +    tcg_gen_brcond_i32(TCG_COND_NE, val, cpu_lock_value, store_fail);
> +    tcg_temp_free(val);
> +
> +    /* success of atomic access */
> +    tcg_gen_qemu_st_tl(tB, t0, dc->mem_idx, MO_TEUL);
> +    tcg_gen_ori_tl(cpu_sr, cpu_sr, SR_F);
> +    tcg_gen_br(store_done);
> +
> +    gen_set_label(store_fail);
> +    tcg_gen_andi_tl(cpu_sr, cpu_sr, ~SR_F);
> +
> +    gen_set_label(store_done);
> +    /* the atomic store invalidates the lock address. */
> +    tcg_gen_movi_i32(cpu_lock_addr, -1);
> +}
> +
>  static void gen_loadstore(DisasContext *dc, uint32 op0,
>                            uint32_t ra, uint32_t rb, uint32_t rd,
>                            uint32_t offset)
>  {
>      TCGv t0 = cpu_R[ra];
>      if (offset != 0) {
> -        t0 = tcg_temp_new();
> +        t0 = tcg_temp_local_new();
>          tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(offset, 16));
>      }
>
>      switch (op0) {
> +    case 0x1b:    /* l.lwa */
> +        gen_atomic_load(cpu_R[rd], t0, dc);
> +        break;
> +
>      case 0x21:    /* l.lwz */
>          tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TEUL);
>          break;
> @@ -289,6 +347,10 @@ static void gen_loadstore(DisasContext *dc, uint32 op0,
>          tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TESW);
>          break;
>
> +    case 0x33:    /* l.swa */
> +        gen_atomic_store(cpu_R[rb], t0, dc);
> +        break;
> +
>      case 0x35:    /* l.sw */
>          tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_TEUL);
>          break;
> @@ -759,9 +821,6 @@ static void dec_calc(DisasContext *dc, uint32_t insn)
>      }
>  }
>
> -
> -
> -

It should be blank lines in here in [patch 1/2].

>  static void dec_misc(DisasContext *dc, uint32_t insn)
>  {
>      uint32_t op0, op1;
> @@ -905,6 +964,11 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
>          gen_loadstore(dc, op0, ra, rb, rd, I16);
>  #endif*/
>
> +    case 0x1b:    /* l.lwa */
> +        LOG_DIS("l.lwa r%d, r%d, %d\n", rd, ra, I16);
> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
> +        break;
> +

Is it a new instruction new added into OpenRISC?

>      case 0x21:    /* l.lwz */
>          LOG_DIS("l.lwz r%d, r%d, %d\n", rd, ra, I16);
>          gen_loadstore(dc, op0, ra, rb, rd, I16);
> @@ -1072,6 +1136,11 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
>          gen_loadstore(dc, op0, ra, rb, rd, tmp);
>  #endif*/
>
> +    case 0x33:    /* l.swa */
> +        LOG_DIS("l.swa %d, r%d, r%d, %d\n", I5, ra, rb, I11);
> +        gen_loadstore(dc, op0, ra, rb, rd, tmp);
> +        break;
> +
>      case 0x35:    /* l.sw */
>          LOG_DIS("l.sw %d, r%d, r%d, %d\n", I5, ra, rb, I11);
>          gen_loadstore(dc, op0, ra, rb, rd, tmp);
> --
> 2.2.2
>

Regards,
Jia

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

* Re: [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support
  2015-01-26  9:50   ` Jia Liu
@ 2015-01-26 10:15     ` Christian Svensson
  2015-01-26 10:18     ` Sebastian Macke
  1 sibling, 0 replies; 8+ messages in thread
From: Christian Svensson @ 2015-01-26 10:15 UTC (permalink / raw)
  To: Jia Liu; +Cc: Sebastian Macke, qemu-devel

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

Hi Jia.

On Mon, Jan 26, 2015 at 9:50 AM, Jia Liu <proljc@gmail.com> wrote:

> Is it a new instruction new added into OpenRISC?
>

Yes. Please see the latest architecture specification for details.

It should be blank lines in here in [patch 1/2].

Care to elaborate why?

[-- Attachment #2: Type: text/html, Size: 975 bytes --]

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

* Re: [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support
  2015-01-26  9:50   ` Jia Liu
  2015-01-26 10:15     ` Christian Svensson
@ 2015-01-26 10:18     ` Sebastian Macke
  2015-01-28  2:28       ` Jia Liu
  1 sibling, 1 reply; 8+ messages in thread
From: Sebastian Macke @ 2015-01-26 10:18 UTC (permalink / raw)
  To: Jia Liu; +Cc: qemu-devel, blue

Hi Jia,

On 1/26/2015 10:50 AM, Jia Liu wrote:
> Hi Sebastian, Christian
>
> On Sun, Jan 25, 2015 at 6:25 PM, Sebastian Macke <sebastian@macke.de> wrote:
>> From: Christian Svensson <blue@cmd.nu>
>>
>> This patch adds support for atomic locks
>> and is an adaption from https://github.com/bluecmd/or1k-qemu/commits/or32-optimize
>>
>> Tested via the atomic lock implementation of musl
>>
>> Signed-off-by: Christian Svensson <blue@cmd.nu>
>> Signed-off-by: Sebastian Macke <sebastian@macke.de>
>> ---
>>   target-openrisc/cpu.h       |  3 ++
>>   target-openrisc/interrupt.c |  3 ++
>>   target-openrisc/translate.c | 77 ++++++++++++++++++++++++++++++++++++++++++---
>>   3 files changed, 79 insertions(+), 4 deletions(-)
>>
>> diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
>> index 69b96c6..abdba75 100644
>> --- a/target-openrisc/cpu.h
>> +++ b/target-openrisc/cpu.h
>> @@ -302,6 +302,9 @@ typedef struct CPUOpenRISCState {
>>                                    in solt so far.  */
>>       uint32_t btaken;          /* the SR_F bit */
>>
>> +    target_ulong lock_addr;   /* Atomicity lock address. */
>> +    target_ulong lock_value;  /* Atomicity lock value. */
>> +
>>       CPU_COMMON
>>
>>       /* Fields from here on are preserved across CPU reset. */
>> diff --git a/target-openrisc/interrupt.c b/target-openrisc/interrupt.c
>> index e480cfd..68d554c 100644
>> --- a/target-openrisc/interrupt.c
>> +++ b/target-openrisc/interrupt.c
>> @@ -54,6 +54,9 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
>>       env->tlb->cpu_openrisc_map_address_data = &cpu_openrisc_get_phys_nommu;
>>       env->tlb->cpu_openrisc_map_address_code = &cpu_openrisc_get_phys_nommu;
>>
>> +    /* invalidate lock */
>> +    env->cpu_lock_addr = -1;
>> +
>>       if (cs->exception_index > 0 && cs->exception_index < EXCP_NR) {
>>           env->pc = (cs->exception_index << 8);
>>       } else {
>> diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
>> index 543aa67..6401b4b 100644
>> --- a/target-openrisc/translate.c
>> +++ b/target-openrisc/translate.c
>> @@ -55,6 +55,8 @@ typedef struct DisasContext {
>>   static TCGv_ptr cpu_env;
>>   static TCGv cpu_sr;
>>   static TCGv cpu_R[32];
>> +static TCGv cpu_lock_addr;
>> +static TCGv cpu_lock_value;
>>   static TCGv cpu_pc;
>>   static TCGv jmp_pc;            /* l.jr/l.jalr temp pc */
>>   static TCGv cpu_npc;
>> @@ -82,6 +84,12 @@ void openrisc_translate_init(void)
>>       env_flags = tcg_global_mem_new_i32(TCG_AREG0,
>>                                          offsetof(CPUOpenRISCState, flags),
>>                                          "flags");
>> +    cpu_lock_addr = tcg_global_mem_new(TCG_AREG0,
>> +                                       offsetof(CPUOpenRISCState, lock_addr),
>> +                                       "lock_addr");
>> +    cpu_lock_value = tcg_global_mem_new(TCG_AREG0,
>> +                                        offsetof(CPUOpenRISCState, lock_value),
>> +                                        "lock_value");
>>       cpu_pc = tcg_global_mem_new(TCG_AREG0,
>>                                   offsetof(CPUOpenRISCState, pc), "pc");
>>       cpu_npc = tcg_global_mem_new(TCG_AREG0,
>> @@ -254,17 +262,67 @@ static void gen_jump(DisasContext *dc, uint32_t imm, uint32_t reg, uint32_t op0)
>>       gen_sync_flags(dc);
>>   }
>>
>> +/* According to the OpenRISC specification we should poison our atomic lock
>> + * if any other store is detected to the same address. For the sake of speed
>> + * and because we're single-threaded we guarantee that atomic stores
>> + * fail only if an atomic load or another atomic store
>> + * is executed.
>> + *
>> + * To prevent the potential case of an ordinary store, we save
>> + * the *value* of the address at the lock time. */
>> +
>> +static void gen_atomic_load(TCGv tD, TCGv t0, DisasContext *dc)
>> +{
>> +    tcg_gen_qemu_ld_tl(tD, t0, dc->mem_idx, MO_TEUL);
>> +    tcg_gen_mov_i32(cpu_lock_addr, t0);
>> +    tcg_gen_mov_i32(cpu_lock_value, tD);
>> +}
>> +
>> +static void gen_atomic_store(TCGv tB, TCGv t0, DisasContext *dc)
>> +{
>> +    int store_fail;
>> +    int store_done;
>> +
>> +    store_fail = gen_new_label();
>> +    store_done = gen_new_label();
>> +
>> +    /* check address */
>> +    tcg_gen_brcond_i32(TCG_COND_NE, t0, cpu_lock_addr, store_fail);
>> +
>> +    /* check value */
>> +    TCGv val = tcg_temp_new();
>> +    tcg_gen_qemu_ld_tl(val, t0, dc->mem_idx, MO_TEUL);
>> +    tcg_gen_brcond_i32(TCG_COND_NE, val, cpu_lock_value, store_fail);
>> +    tcg_temp_free(val);
>> +
>> +    /* success of atomic access */
>> +    tcg_gen_qemu_st_tl(tB, t0, dc->mem_idx, MO_TEUL);
>> +    tcg_gen_ori_tl(cpu_sr, cpu_sr, SR_F);
>> +    tcg_gen_br(store_done);
>> +
>> +    gen_set_label(store_fail);
>> +    tcg_gen_andi_tl(cpu_sr, cpu_sr, ~SR_F);
>> +
>> +    gen_set_label(store_done);
>> +    /* the atomic store invalidates the lock address. */
>> +    tcg_gen_movi_i32(cpu_lock_addr, -1);
>> +}
>> +
>>   static void gen_loadstore(DisasContext *dc, uint32 op0,
>>                             uint32_t ra, uint32_t rb, uint32_t rd,
>>                             uint32_t offset)
>>   {
>>       TCGv t0 = cpu_R[ra];
>>       if (offset != 0) {
>> -        t0 = tcg_temp_new();
>> +        t0 = tcg_temp_local_new();
>>           tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(offset, 16));
>>       }
>>
>>       switch (op0) {
>> +    case 0x1b:    /* l.lwa */
>> +        gen_atomic_load(cpu_R[rd], t0, dc);
>> +        break;
>> +
>>       case 0x21:    /* l.lwz */
>>           tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TEUL);
>>           break;
>> @@ -289,6 +347,10 @@ static void gen_loadstore(DisasContext *dc, uint32 op0,
>>           tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TESW);
>>           break;
>>
>> +    case 0x33:    /* l.swa */
>> +        gen_atomic_store(cpu_R[rb], t0, dc);
>> +        break;
>> +
>>       case 0x35:    /* l.sw */
>>           tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_TEUL);
>>           break;
>> @@ -759,9 +821,6 @@ static void dec_calc(DisasContext *dc, uint32_t insn)
>>       }
>>   }
>>
>> -
>> -
>> -
> It should be blank lines in here in [patch 1/2].

Yes, looks like I added three lines in patch 1/2 and then removed them 
in patch 2/2.
I guess if both patches are accepted it does not matter. Otherwise I 
will fix these in revision 2.

>
>>   static void dec_misc(DisasContext *dc, uint32_t insn)
>>   {
>>       uint32_t op0, op1;
>> @@ -905,6 +964,11 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
>>           gen_loadstore(dc, op0, ra, rb, rd, I16);
>>   #endif*/
>>
>> +    case 0x1b:    /* l.lwa */
>> +        LOG_DIS("l.lwa r%d, r%d, %d\n", rd, ra, I16);
>> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
>> +        break;
>> +
> Is it a new instruction new added into OpenRISC?

Yes, they were added last year.
http://opencores.org/websvn,filedetails?repname=openrisc&path=%2Fopenrisc%2Ftrunk%2Fdocs%2Fopenrisc-arch-1.1-rev0.pdf
Previously the kernel handled those atomic calls for single core 
implementations.
But we also managed to run multi-core OpenRISC machine. And here the 
instructions are absolutely necessary.

Fact is, that almost none of our toolchains work anymore without these 
new instructions.

>>       case 0x21:    /* l.lwz */
>>           LOG_DIS("l.lwz r%d, r%d, %d\n", rd, ra, I16);
>>           gen_loadstore(dc, op0, ra, rb, rd, I16);
>> @@ -1072,6 +1136,11 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
>>           gen_loadstore(dc, op0, ra, rb, rd, tmp);
>>   #endif*/
>>
>> +    case 0x33:    /* l.swa */
>> +        LOG_DIS("l.swa %d, r%d, r%d, %d\n", I5, ra, rb, I11);
>> +        gen_loadstore(dc, op0, ra, rb, rd, tmp);
>> +        break;
>> +
>>       case 0x35:    /* l.sw */
>>           LOG_DIS("l.sw %d, r%d, r%d, %d\n", I5, ra, rb, I11);
>>           gen_loadstore(dc, op0, ra, rb, rd, tmp);
>> --
>> 2.2.2
>>
> Regards,
> Jia

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

* Re: [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support
  2015-01-26 10:18     ` Sebastian Macke
@ 2015-01-28  2:28       ` Jia Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Jia Liu @ 2015-01-28  2:28 UTC (permalink / raw)
  To: Sebastian Macke; +Cc: qemu-devel, Christian Svensson

On Mon, Jan 26, 2015 at 6:18 PM, Sebastian Macke <sebastian@macke.de> wrote:
> Hi Jia,
>
>
> On 1/26/2015 10:50 AM, Jia Liu wrote:
>>
>> Hi Sebastian, Christian
>>
>> On Sun, Jan 25, 2015 at 6:25 PM, Sebastian Macke <sebastian@macke.de>
>> wrote:
>>>
>>> From: Christian Svensson <blue@cmd.nu>
>>>
>>> This patch adds support for atomic locks
>>> and is an adaption from
>>> https://github.com/bluecmd/or1k-qemu/commits/or32-optimize
>>>
>>> Tested via the atomic lock implementation of musl
>>>
>>> Signed-off-by: Christian Svensson <blue@cmd.nu>
>>> Signed-off-by: Sebastian Macke <sebastian@macke.de>
>>> ---
>>>   target-openrisc/cpu.h       |  3 ++
>>>   target-openrisc/interrupt.c |  3 ++
>>>   target-openrisc/translate.c | 77
>>> ++++++++++++++++++++++++++++++++++++++++++---
>>>   3 files changed, 79 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
>>> index 69b96c6..abdba75 100644
>>> --- a/target-openrisc/cpu.h
>>> +++ b/target-openrisc/cpu.h
>>> @@ -302,6 +302,9 @@ typedef struct CPUOpenRISCState {
>>>                                    in solt so far.  */
>>>       uint32_t btaken;          /* the SR_F bit */
>>>
>>> +    target_ulong lock_addr;   /* Atomicity lock address. */
>>> +    target_ulong lock_value;  /* Atomicity lock value. */
>>> +
>>>       CPU_COMMON
>>>
>>>       /* Fields from here on are preserved across CPU reset. */
>>> diff --git a/target-openrisc/interrupt.c b/target-openrisc/interrupt.c
>>> index e480cfd..68d554c 100644
>>> --- a/target-openrisc/interrupt.c
>>> +++ b/target-openrisc/interrupt.c
>>> @@ -54,6 +54,9 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
>>>       env->tlb->cpu_openrisc_map_address_data =
>>> &cpu_openrisc_get_phys_nommu;
>>>       env->tlb->cpu_openrisc_map_address_code =
>>> &cpu_openrisc_get_phys_nommu;
>>>
>>> +    /* invalidate lock */
>>> +    env->cpu_lock_addr = -1;
>>> +
>>>       if (cs->exception_index > 0 && cs->exception_index < EXCP_NR) {
>>>           env->pc = (cs->exception_index << 8);
>>>       } else {
>>> diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
>>> index 543aa67..6401b4b 100644
>>> --- a/target-openrisc/translate.c
>>> +++ b/target-openrisc/translate.c
>>> @@ -55,6 +55,8 @@ typedef struct DisasContext {
>>>   static TCGv_ptr cpu_env;
>>>   static TCGv cpu_sr;
>>>   static TCGv cpu_R[32];
>>> +static TCGv cpu_lock_addr;
>>> +static TCGv cpu_lock_value;
>>>   static TCGv cpu_pc;
>>>   static TCGv jmp_pc;            /* l.jr/l.jalr temp pc */
>>>   static TCGv cpu_npc;
>>> @@ -82,6 +84,12 @@ void openrisc_translate_init(void)
>>>       env_flags = tcg_global_mem_new_i32(TCG_AREG0,
>>>                                          offsetof(CPUOpenRISCState,
>>> flags),
>>>                                          "flags");
>>> +    cpu_lock_addr = tcg_global_mem_new(TCG_AREG0,
>>> +                                       offsetof(CPUOpenRISCState,
>>> lock_addr),
>>> +                                       "lock_addr");
>>> +    cpu_lock_value = tcg_global_mem_new(TCG_AREG0,
>>> +                                        offsetof(CPUOpenRISCState,
>>> lock_value),
>>> +                                        "lock_value");
>>>       cpu_pc = tcg_global_mem_new(TCG_AREG0,
>>>                                   offsetof(CPUOpenRISCState, pc), "pc");
>>>       cpu_npc = tcg_global_mem_new(TCG_AREG0,
>>> @@ -254,17 +262,67 @@ static void gen_jump(DisasContext *dc, uint32_t
>>> imm, uint32_t reg, uint32_t op0)
>>>       gen_sync_flags(dc);
>>>   }
>>>
>>> +/* According to the OpenRISC specification we should poison our atomic
>>> lock
>>> + * if any other store is detected to the same address. For the sake of
>>> speed
>>> + * and because we're single-threaded we guarantee that atomic stores
>>> + * fail only if an atomic load or another atomic store
>>> + * is executed.
>>> + *
>>> + * To prevent the potential case of an ordinary store, we save
>>> + * the *value* of the address at the lock time. */
>>> +
>>> +static void gen_atomic_load(TCGv tD, TCGv t0, DisasContext *dc)
>>> +{
>>> +    tcg_gen_qemu_ld_tl(tD, t0, dc->mem_idx, MO_TEUL);
>>> +    tcg_gen_mov_i32(cpu_lock_addr, t0);
>>> +    tcg_gen_mov_i32(cpu_lock_value, tD);
>>> +}
>>> +
>>> +static void gen_atomic_store(TCGv tB, TCGv t0, DisasContext *dc)
>>> +{
>>> +    int store_fail;
>>> +    int store_done;
>>> +
>>> +    store_fail = gen_new_label();
>>> +    store_done = gen_new_label();
>>> +
>>> +    /* check address */
>>> +    tcg_gen_brcond_i32(TCG_COND_NE, t0, cpu_lock_addr, store_fail);
>>> +
>>> +    /* check value */
>>> +    TCGv val = tcg_temp_new();
>>> +    tcg_gen_qemu_ld_tl(val, t0, dc->mem_idx, MO_TEUL);
>>> +    tcg_gen_brcond_i32(TCG_COND_NE, val, cpu_lock_value, store_fail);
>>> +    tcg_temp_free(val);
>>> +
>>> +    /* success of atomic access */
>>> +    tcg_gen_qemu_st_tl(tB, t0, dc->mem_idx, MO_TEUL);
>>> +    tcg_gen_ori_tl(cpu_sr, cpu_sr, SR_F);
>>> +    tcg_gen_br(store_done);
>>> +
>>> +    gen_set_label(store_fail);
>>> +    tcg_gen_andi_tl(cpu_sr, cpu_sr, ~SR_F);
>>> +
>>> +    gen_set_label(store_done);
>>> +    /* the atomic store invalidates the lock address. */
>>> +    tcg_gen_movi_i32(cpu_lock_addr, -1);
>>> +}
>>> +
>>>   static void gen_loadstore(DisasContext *dc, uint32 op0,
>>>                             uint32_t ra, uint32_t rb, uint32_t rd,
>>>                             uint32_t offset)
>>>   {
>>>       TCGv t0 = cpu_R[ra];
>>>       if (offset != 0) {
>>> -        t0 = tcg_temp_new();
>>> +        t0 = tcg_temp_local_new();
>>>           tcg_gen_addi_tl(t0, cpu_R[ra], sign_extend(offset, 16));
>>>       }
>>>
>>>       switch (op0) {
>>> +    case 0x1b:    /* l.lwa */
>>> +        gen_atomic_load(cpu_R[rd], t0, dc);
>>> +        break;
>>> +
>>>       case 0x21:    /* l.lwz */
>>>           tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TEUL);
>>>           break;
>>> @@ -289,6 +347,10 @@ static void gen_loadstore(DisasContext *dc, uint32
>>> op0,
>>>           tcg_gen_qemu_ld_tl(cpu_R[rd], t0, dc->mem_idx, MO_TESW);
>>>           break;
>>>
>>> +    case 0x33:    /* l.swa */
>>> +        gen_atomic_store(cpu_R[rb], t0, dc);
>>> +        break;
>>> +
>>>       case 0x35:    /* l.sw */
>>>           tcg_gen_qemu_st_tl(cpu_R[rb], t0, dc->mem_idx, MO_TEUL);
>>>           break;
>>> @@ -759,9 +821,6 @@ static void dec_calc(DisasContext *dc, uint32_t insn)
>>>       }
>>>   }
>>>
>>> -
>>> -
>>> -
>>
>> It should be blank lines in here in [patch 1/2].
>
>
> Yes, looks like I added three lines in patch 1/2 and then removed them in
> patch 2/2.
> I guess if both patches are accepted it does not matter. Otherwise I will
> fix these in revision 2.
>
>>
>>>   static void dec_misc(DisasContext *dc, uint32_t insn)
>>>   {
>>>       uint32_t op0, op1;
>>> @@ -905,6 +964,11 @@ static void dec_misc(DisasContext *dc, uint32_t
>>> insn)
>>>           gen_loadstore(dc, op0, ra, rb, rd, I16);
>>>   #endif*/
>>>
>>> +    case 0x1b:    /* l.lwa */
>>> +        LOG_DIS("l.lwa r%d, r%d, %d\n", rd, ra, I16);
>>> +        gen_loadstore(dc, op0, ra, rb, rd, I16);
>>> +        break;
>>> +
>>
>> Is it a new instruction new added into OpenRISC?
>
>
> Yes, they were added last year.
> http://opencores.org/websvn,filedetails?repname=openrisc&path=%2Fopenrisc%2Ftrunk%2Fdocs%2Fopenrisc-arch-1.1-rev0.pdf
> Previously the kernel handled those atomic calls for single core
> implementations.
> But we also managed to run multi-core OpenRISC machine. And here the
> instructions are absolutely necessary.

OK, I'll put it into my pull request.

>
> Fact is, that almost none of our toolchains work anymore without these new
> instructions.

Isn't Jnoas guys working on the GNU Toolchain?

>
>
>>>       case 0x21:    /* l.lwz */
>>>           LOG_DIS("l.lwz r%d, r%d, %d\n", rd, ra, I16);
>>>           gen_loadstore(dc, op0, ra, rb, rd, I16);
>>> @@ -1072,6 +1136,11 @@ static void dec_misc(DisasContext *dc, uint32_t
>>> insn)
>>>           gen_loadstore(dc, op0, ra, rb, rd, tmp);
>>>   #endif*/
>>>
>>> +    case 0x33:    /* l.swa */
>>> +        LOG_DIS("l.swa %d, r%d, r%d, %d\n", I5, ra, rb, I11);
>>> +        gen_loadstore(dc, op0, ra, rb, rd, tmp);
>>> +        break;
>>> +
>>>       case 0x35:    /* l.sw */
>>>           LOG_DIS("l.sw %d, r%d, r%d, %d\n", I5, ra, rb, I11);
>>>           gen_loadstore(dc, op0, ra, rb, rd, tmp);
>>> --
>>> 2.2.2
>>>
>> Regards,
>> Jia
>
>

Regards,
Jia

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

end of thread, other threads:[~2015-01-28  2:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-25 10:25 [Qemu-devel] [PATCH 0/2] target-openrisc: Add atomic instructions Sebastian Macke
2015-01-25 10:25 ` [Qemu-devel] [PATCH 1/2] target-openrisc: Separate of load/store instructions Sebastian Macke
2015-01-26  9:47   ` Jia Liu
2015-01-25 10:25 ` [Qemu-devel] [PATCH 2/2] target-openrisc: Add l.lwa/l.swa support Sebastian Macke
2015-01-26  9:50   ` Jia Liu
2015-01-26 10:15     ` Christian Svensson
2015-01-26 10:18     ` Sebastian Macke
2015-01-28  2:28       ` Jia Liu

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.