All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] target/riscv: Annotate atomic operations
@ 2022-04-01 12:59 ` Richard Henderson
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-04-01 12:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, bin.meng, alistair.francis, palmer

If an atomic operation fails on RISC-V, we want to generate
a store/amo fault and not a load fault.

Annotate amo insns, so that we can recognize them after unwinding.
Transform the implementation access type to store/amo for reporting.


r~


Richard Henderson (2):
  target/riscv: Use cpu_loop_exit_restore directly from mmu faults
  target/riscv: Mark amo insns during translation

 target/riscv/cpu.h                      | 15 ++++++
 target/riscv/cpu.c                      |  3 ++
 target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
 target/riscv/translate.c                |  9 ++++
 target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
 5 files changed, 79 insertions(+), 21 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH 0/2] target/riscv: Annotate atomic operations
@ 2022-04-01 12:59 ` Richard Henderson
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-04-01 12:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis, qemu-riscv, palmer, bin.meng

If an atomic operation fails on RISC-V, we want to generate
a store/amo fault and not a load fault.

Annotate amo insns, so that we can recognize them after unwinding.
Transform the implementation access type to store/amo for reporting.


r~


Richard Henderson (2):
  target/riscv: Use cpu_loop_exit_restore directly from mmu faults
  target/riscv: Mark amo insns during translation

 target/riscv/cpu.h                      | 15 ++++++
 target/riscv/cpu.c                      |  3 ++
 target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
 target/riscv/translate.c                |  9 ++++
 target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
 5 files changed, 79 insertions(+), 21 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH 1/2] target/riscv: Use cpu_loop_exit_restore directly from mmu faults
  2022-04-01 12:59 ` Richard Henderson
@ 2022-04-01 12:59   ` Richard Henderson
  -1 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-04-01 12:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, bin.meng, alistair.francis, palmer

The riscv_raise_exception function stores its argument into
exception_index and then exits to the main loop.  When we
have already set exception_index, we can just exit directly.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 1c60fb2e80..126251d5da 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1150,7 +1150,7 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
     env->badaddr = addr;
     env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
                             riscv_cpu_two_stage_lookup(mmu_idx);
-    riscv_raise_exception(&cpu->env, cs->exception_index, retaddr);
+    cpu_loop_exit_restore(cs, retaddr);
 }
 
 void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
@@ -1175,7 +1175,7 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     env->badaddr = addr;
     env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
                             riscv_cpu_two_stage_lookup(mmu_idx);
-    riscv_raise_exception(env, cs->exception_index, retaddr);
+    cpu_loop_exit_restore(cs, retaddr);
 }
 
 bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
@@ -1311,7 +1311,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                             first_stage_error,
                             riscv_cpu_virt_enabled(env) ||
                                 riscv_cpu_two_stage_lookup(mmu_idx));
-        riscv_raise_exception(env, cs->exception_index, retaddr);
+        cpu_loop_exit_restore(cs, retaddr);
     }
 
     return true;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 1/2] target/riscv: Use cpu_loop_exit_restore directly from mmu faults
@ 2022-04-01 12:59   ` Richard Henderson
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-04-01 12:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis, qemu-riscv, palmer, bin.meng

The riscv_raise_exception function stores its argument into
exception_index and then exits to the main loop.  When we
have already set exception_index, we can just exit directly.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 1c60fb2e80..126251d5da 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1150,7 +1150,7 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
     env->badaddr = addr;
     env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
                             riscv_cpu_two_stage_lookup(mmu_idx);
-    riscv_raise_exception(&cpu->env, cs->exception_index, retaddr);
+    cpu_loop_exit_restore(cs, retaddr);
 }
 
 void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
@@ -1175,7 +1175,7 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     env->badaddr = addr;
     env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
                             riscv_cpu_two_stage_lookup(mmu_idx);
-    riscv_raise_exception(env, cs->exception_index, retaddr);
+    cpu_loop_exit_restore(cs, retaddr);
 }
 
 bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
@@ -1311,7 +1311,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                             first_stage_error,
                             riscv_cpu_virt_enabled(env) ||
                                 riscv_cpu_two_stage_lookup(mmu_idx));
-        riscv_raise_exception(env, cs->exception_index, retaddr);
+        cpu_loop_exit_restore(cs, retaddr);
     }
 
     return true;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/2] target/riscv: Mark amo insns during translation
  2022-04-01 12:59 ` Richard Henderson
@ 2022-04-01 12:59   ` Richard Henderson
  -1 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-04-01 12:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, bin.meng, alistair.francis, palmer

Atomic memory operations perform both reads and writes as part
of their implementation, but always raise write faults.

Use TARGET_INSN_START_EXTRA_WORDS to mark amo insns in the
opcode stream, and force the access type to write at the
point of raising the exception.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.h                      | 15 ++++++
 target/riscv/cpu.c                      |  3 ++
 target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
 target/riscv/translate.c                |  9 ++++
 target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
 5 files changed, 79 insertions(+), 21 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c069fe85fa..3de4da3fa1 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -290,6 +290,13 @@ struct CPUArchState {
     /* True if in debugger mode.  */
     bool debugger;
 
+    /*
+     * True if unwinding through an amo insn.  Used to transform a
+     * read fault into a store_amo fault; only valid immediately
+     * after cpu_restore_state().
+     */
+    bool unwind_amo;
+
     /*
      * CSRs for PointerMasking extension
      */
@@ -517,6 +524,14 @@ FIELD(TB_FLAGS, XL, 20, 2)
 FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
 FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
 
+#ifndef CONFIG_USER_ONLY
+/*
+ * RISC-V-specific extra insn start words:
+ * 1: True if the instruction is AMO, false otherwise.
+ */
+#define TARGET_INSN_START_EXTRA_WORDS 1
+#endif
+
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
 #else
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddda4906ff..3818d5ba80 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -396,6 +396,9 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
     } else {
         env->pc = data[0];
     }
+#ifndef CONFIG_USER_ONLY
+    env->unwind_amo = data[1];
+#endif
 }
 
 static void riscv_cpu_reset(DeviceState *dev)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 126251d5da..b5bbe6fc39 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1139,26 +1139,11 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
 
-    if (access_type == MMU_DATA_STORE) {
-        cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
-    } else if (access_type == MMU_DATA_LOAD) {
-        cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
-    } else {
-        cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
+    cpu_restore_state(cs, retaddr, true);
+    if (env->unwind_amo) {
+        access_type = MMU_DATA_STORE;
     }
 
-    env->badaddr = addr;
-    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
-                            riscv_cpu_two_stage_lookup(mmu_idx);
-    cpu_loop_exit_restore(cs, retaddr);
-}
-
-void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
-                                   MMUAccessType access_type, int mmu_idx,
-                                   uintptr_t retaddr)
-{
-    RISCVCPU *cpu = RISCV_CPU(cs);
-    CPURISCVState *env = &cpu->env;
     switch (access_type) {
     case MMU_INST_FETCH:
         cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
@@ -1172,10 +1157,43 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     default:
         g_assert_not_reached();
     }
+
     env->badaddr = addr;
     env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
                             riscv_cpu_two_stage_lookup(mmu_idx);
-    cpu_loop_exit_restore(cs, retaddr);
+    cpu_loop_exit(cs);
+}
+
+void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+                                   MMUAccessType access_type, int mmu_idx,
+                                   uintptr_t retaddr)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+
+    cpu_restore_state(cs, retaddr, true);
+    if (env->unwind_amo) {
+        access_type = MMU_DATA_STORE;
+    }
+
+    switch (access_type) {
+    case MMU_INST_FETCH:
+        cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
+        break;
+    case MMU_DATA_LOAD:
+        cs->exception_index = RISCV_EXCP_LOAD_ADDR_MIS;
+        break;
+    case MMU_DATA_STORE:
+        cs->exception_index = RISCV_EXCP_STORE_AMO_ADDR_MIS;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    env->badaddr = addr;
+    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
+                            riscv_cpu_two_stage_lookup(mmu_idx);
+    cpu_loop_exit(cs);
 }
 
 bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
@@ -1307,11 +1325,15 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     } else if (probe) {
         return false;
     } else {
+        cpu_restore_state(cs, retaddr, true);
+        if (env->unwind_amo) {
+            access_type = MMU_DATA_STORE;
+        }
         raise_mmu_exception(env, address, access_type, pmp_violation,
                             first_stage_error,
                             riscv_cpu_virt_enabled(env) ||
                                 riscv_cpu_two_stage_lookup(mmu_idx));
-        cpu_loop_exit_restore(cs, retaddr);
+        cpu_loop_exit(cs);
     }
 
     return true;
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index fac998a6b5..ae4b0d1524 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -107,6 +107,10 @@ typedef struct DisasContext {
     /* PointerMasking extension */
     bool pm_mask_enabled;
     bool pm_base_enabled;
+#ifndef CONFIG_USER_ONLY
+    /* TCG op of the current insn_start.  */
+    TCGOp *insn_start;
+#endif
 } DisasContext;
 
 static inline bool has_ext(DisasContext *ctx, uint32_t ext)
@@ -1105,7 +1109,12 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
 
+#ifdef CONFIG_USER_ONLY
     tcg_gen_insn_start(ctx->base.pc_next);
+#else
+    tcg_gen_insn_start(ctx->base.pc_next, 0);
+    ctx->insn_start = tcg_last_op();
+#endif
 }
 
 static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
index 45db82c9be..66faa8f1da 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -37,6 +37,13 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
     return true;
 }
 
+static void record_insn_start_amo(DisasContext *ctx)
+{
+#ifndef CONFIG_USER_ONLY
+    tcg_set_insn_start_param(ctx->insn_start, 1, 1);
+#endif
+}
+
 static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
 {
     TCGv dest, src1, src2;
@@ -73,6 +80,7 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
      */
     tcg_gen_movi_tl(load_res, -1);
 
+    record_insn_start_amo(ctx);
     return true;
 }
 
@@ -85,8 +93,9 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
     TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
 
     func(dest, src1, src2, ctx->mem_idx, mop);
-
     gen_set_gpr(ctx, a->rd, dest);
+
+    record_insn_start_amo(ctx);
     return true;
 }
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/2] target/riscv: Mark amo insns during translation
@ 2022-04-01 12:59   ` Richard Henderson
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2022-04-01 12:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis, qemu-riscv, palmer, bin.meng

Atomic memory operations perform both reads and writes as part
of their implementation, but always raise write faults.

Use TARGET_INSN_START_EXTRA_WORDS to mark amo insns in the
opcode stream, and force the access type to write at the
point of raising the exception.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.h                      | 15 ++++++
 target/riscv/cpu.c                      |  3 ++
 target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
 target/riscv/translate.c                |  9 ++++
 target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
 5 files changed, 79 insertions(+), 21 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c069fe85fa..3de4da3fa1 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -290,6 +290,13 @@ struct CPUArchState {
     /* True if in debugger mode.  */
     bool debugger;
 
+    /*
+     * True if unwinding through an amo insn.  Used to transform a
+     * read fault into a store_amo fault; only valid immediately
+     * after cpu_restore_state().
+     */
+    bool unwind_amo;
+
     /*
      * CSRs for PointerMasking extension
      */
@@ -517,6 +524,14 @@ FIELD(TB_FLAGS, XL, 20, 2)
 FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
 FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
 
+#ifndef CONFIG_USER_ONLY
+/*
+ * RISC-V-specific extra insn start words:
+ * 1: True if the instruction is AMO, false otherwise.
+ */
+#define TARGET_INSN_START_EXTRA_WORDS 1
+#endif
+
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
 #else
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddda4906ff..3818d5ba80 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -396,6 +396,9 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
     } else {
         env->pc = data[0];
     }
+#ifndef CONFIG_USER_ONLY
+    env->unwind_amo = data[1];
+#endif
 }
 
 static void riscv_cpu_reset(DeviceState *dev)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 126251d5da..b5bbe6fc39 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1139,26 +1139,11 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
 
-    if (access_type == MMU_DATA_STORE) {
-        cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
-    } else if (access_type == MMU_DATA_LOAD) {
-        cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
-    } else {
-        cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
+    cpu_restore_state(cs, retaddr, true);
+    if (env->unwind_amo) {
+        access_type = MMU_DATA_STORE;
     }
 
-    env->badaddr = addr;
-    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
-                            riscv_cpu_two_stage_lookup(mmu_idx);
-    cpu_loop_exit_restore(cs, retaddr);
-}
-
-void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
-                                   MMUAccessType access_type, int mmu_idx,
-                                   uintptr_t retaddr)
-{
-    RISCVCPU *cpu = RISCV_CPU(cs);
-    CPURISCVState *env = &cpu->env;
     switch (access_type) {
     case MMU_INST_FETCH:
         cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
@@ -1172,10 +1157,43 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     default:
         g_assert_not_reached();
     }
+
     env->badaddr = addr;
     env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
                             riscv_cpu_two_stage_lookup(mmu_idx);
-    cpu_loop_exit_restore(cs, retaddr);
+    cpu_loop_exit(cs);
+}
+
+void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+                                   MMUAccessType access_type, int mmu_idx,
+                                   uintptr_t retaddr)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+
+    cpu_restore_state(cs, retaddr, true);
+    if (env->unwind_amo) {
+        access_type = MMU_DATA_STORE;
+    }
+
+    switch (access_type) {
+    case MMU_INST_FETCH:
+        cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
+        break;
+    case MMU_DATA_LOAD:
+        cs->exception_index = RISCV_EXCP_LOAD_ADDR_MIS;
+        break;
+    case MMU_DATA_STORE:
+        cs->exception_index = RISCV_EXCP_STORE_AMO_ADDR_MIS;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    env->badaddr = addr;
+    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
+                            riscv_cpu_two_stage_lookup(mmu_idx);
+    cpu_loop_exit(cs);
 }
 
 bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
@@ -1307,11 +1325,15 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     } else if (probe) {
         return false;
     } else {
+        cpu_restore_state(cs, retaddr, true);
+        if (env->unwind_amo) {
+            access_type = MMU_DATA_STORE;
+        }
         raise_mmu_exception(env, address, access_type, pmp_violation,
                             first_stage_error,
                             riscv_cpu_virt_enabled(env) ||
                                 riscv_cpu_two_stage_lookup(mmu_idx));
-        cpu_loop_exit_restore(cs, retaddr);
+        cpu_loop_exit(cs);
     }
 
     return true;
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index fac998a6b5..ae4b0d1524 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -107,6 +107,10 @@ typedef struct DisasContext {
     /* PointerMasking extension */
     bool pm_mask_enabled;
     bool pm_base_enabled;
+#ifndef CONFIG_USER_ONLY
+    /* TCG op of the current insn_start.  */
+    TCGOp *insn_start;
+#endif
 } DisasContext;
 
 static inline bool has_ext(DisasContext *ctx, uint32_t ext)
@@ -1105,7 +1109,12 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
 
+#ifdef CONFIG_USER_ONLY
     tcg_gen_insn_start(ctx->base.pc_next);
+#else
+    tcg_gen_insn_start(ctx->base.pc_next, 0);
+    ctx->insn_start = tcg_last_op();
+#endif
 }
 
 static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
index 45db82c9be..66faa8f1da 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -37,6 +37,13 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
     return true;
 }
 
+static void record_insn_start_amo(DisasContext *ctx)
+{
+#ifndef CONFIG_USER_ONLY
+    tcg_set_insn_start_param(ctx->insn_start, 1, 1);
+#endif
+}
+
 static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
 {
     TCGv dest, src1, src2;
@@ -73,6 +80,7 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
      */
     tcg_gen_movi_tl(load_res, -1);
 
+    record_insn_start_amo(ctx);
     return true;
 }
 
@@ -85,8 +93,9 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
     TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
 
     func(dest, src1, src2, ctx->mem_idx, mop);
-
     gen_set_gpr(ctx, a->rd, dest);
+
+    record_insn_start_amo(ctx);
     return true;
 }
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/riscv: Use cpu_loop_exit_restore directly from mmu faults
  2022-04-01 12:59   ` Richard Henderson
@ 2022-04-08  4:35     ` Alistair Francis
  -1 siblings, 0 replies; 14+ messages in thread
From: Alistair Francis @ 2022-04-08  4:35 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Alistair Francis, Palmer Dabbelt, Bin Meng, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Fri, Apr 1, 2022 at 11:01 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The riscv_raise_exception function stores its argument into
> exception_index and then exits to the main loop.  When we
> have already set exception_index, we can just exit directly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu_helper.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 1c60fb2e80..126251d5da 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1150,7 +1150,7 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
>      env->badaddr = addr;
>      env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
>                              riscv_cpu_two_stage_lookup(mmu_idx);
> -    riscv_raise_exception(&cpu->env, cs->exception_index, retaddr);
> +    cpu_loop_exit_restore(cs, retaddr);
>  }
>
>  void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> @@ -1175,7 +1175,7 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>      env->badaddr = addr;
>      env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
>                              riscv_cpu_two_stage_lookup(mmu_idx);
> -    riscv_raise_exception(env, cs->exception_index, retaddr);
> +    cpu_loop_exit_restore(cs, retaddr);
>  }
>
>  bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> @@ -1311,7 +1311,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>                              first_stage_error,
>                              riscv_cpu_virt_enabled(env) ||
>                                  riscv_cpu_two_stage_lookup(mmu_idx));
> -        riscv_raise_exception(env, cs->exception_index, retaddr);
> +        cpu_loop_exit_restore(cs, retaddr);
>      }
>
>      return true;
> --
> 2.25.1
>
>


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/riscv: Use cpu_loop_exit_restore directly from mmu faults
@ 2022-04-08  4:35     ` Alistair Francis
  0 siblings, 0 replies; 14+ messages in thread
From: Alistair Francis @ 2022-04-08  4:35 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
	Alistair Francis, Palmer Dabbelt

On Fri, Apr 1, 2022 at 11:01 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The riscv_raise_exception function stores its argument into
> exception_index and then exits to the main loop.  When we
> have already set exception_index, we can just exit directly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu_helper.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 1c60fb2e80..126251d5da 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1150,7 +1150,7 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
>      env->badaddr = addr;
>      env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
>                              riscv_cpu_two_stage_lookup(mmu_idx);
> -    riscv_raise_exception(&cpu->env, cs->exception_index, retaddr);
> +    cpu_loop_exit_restore(cs, retaddr);
>  }
>
>  void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> @@ -1175,7 +1175,7 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>      env->badaddr = addr;
>      env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
>                              riscv_cpu_two_stage_lookup(mmu_idx);
> -    riscv_raise_exception(env, cs->exception_index, retaddr);
> +    cpu_loop_exit_restore(cs, retaddr);
>  }
>
>  bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> @@ -1311,7 +1311,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>                              first_stage_error,
>                              riscv_cpu_virt_enabled(env) ||
>                                  riscv_cpu_two_stage_lookup(mmu_idx));
> -        riscv_raise_exception(env, cs->exception_index, retaddr);
> +        cpu_loop_exit_restore(cs, retaddr);
>      }
>
>      return true;
> --
> 2.25.1
>
>


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target/riscv: Mark amo insns during translation
  2022-04-01 12:59   ` Richard Henderson
@ 2022-04-08  6:16     ` Alistair Francis
  -1 siblings, 0 replies; 14+ messages in thread
From: Alistair Francis @ 2022-04-08  6:16 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Alistair Francis, Palmer Dabbelt, Bin Meng, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Fri, Apr 1, 2022 at 11:04 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Atomic memory operations perform both reads and writes as part
> of their implementation, but always raise write faults.
>
> Use TARGET_INSN_START_EXTRA_WORDS to mark amo insns in the
> opcode stream, and force the access type to write at the
> point of raising the exception.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.h                      | 15 ++++++
>  target/riscv/cpu.c                      |  3 ++
>  target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
>  target/riscv/translate.c                |  9 ++++
>  target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
>  5 files changed, 79 insertions(+), 21 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c069fe85fa..3de4da3fa1 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -290,6 +290,13 @@ struct CPUArchState {
>      /* True if in debugger mode.  */
>      bool debugger;
>
> +    /*
> +     * True if unwinding through an amo insn.  Used to transform a
> +     * read fault into a store_amo fault; only valid immediately
> +     * after cpu_restore_state().
> +     */
> +    bool unwind_amo;
> +
>      /*
>       * CSRs for PointerMasking extension
>       */
> @@ -517,6 +524,14 @@ FIELD(TB_FLAGS, XL, 20, 2)
>  FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
>  FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
>
> +#ifndef CONFIG_USER_ONLY
> +/*
> + * RISC-V-specific extra insn start words:
> + * 1: True if the instruction is AMO, false otherwise.
> + */
> +#define TARGET_INSN_START_EXTRA_WORDS 1
> +#endif
> +
>  #ifdef TARGET_RISCV32
>  #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
>  #else
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..3818d5ba80 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -396,6 +396,9 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
>      } else {
>          env->pc = data[0];
>      }
> +#ifndef CONFIG_USER_ONLY
> +    env->unwind_amo = data[1];
> +#endif
>  }
>
>  static void riscv_cpu_reset(DeviceState *dev)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 126251d5da..b5bbe6fc39 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1139,26 +1139,11 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      CPURISCVState *env = &cpu->env;
>
> -    if (access_type == MMU_DATA_STORE) {
> -        cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> -    } else if (access_type == MMU_DATA_LOAD) {
> -        cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
> -    } else {
> -        cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
> +    cpu_restore_state(cs, retaddr, true);
> +    if (env->unwind_amo) {
> +        access_type = MMU_DATA_STORE;
>      }
>
> -    env->badaddr = addr;
> -    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> -                            riscv_cpu_two_stage_lookup(mmu_idx);
> -    cpu_loop_exit_restore(cs, retaddr);
> -}
> -
> -void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> -                                   MMUAccessType access_type, int mmu_idx,
> -                                   uintptr_t retaddr)
> -{
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -    CPURISCVState *env = &cpu->env;
>      switch (access_type) {
>      case MMU_INST_FETCH:
>          cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> @@ -1172,10 +1157,43 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>      default:
>          g_assert_not_reached();
>      }
> +
>      env->badaddr = addr;
>      env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
>                              riscv_cpu_two_stage_lookup(mmu_idx);
> -    cpu_loop_exit_restore(cs, retaddr);
> +    cpu_loop_exit(cs);
> +}
> +
> +void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> +                                   MMUAccessType access_type, int mmu_idx,
> +                                   uintptr_t retaddr)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +
> +    cpu_restore_state(cs, retaddr, true);
> +    if (env->unwind_amo) {
> +        access_type = MMU_DATA_STORE;
> +    }
> +
> +    switch (access_type) {
> +    case MMU_INST_FETCH:
> +        cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> +        break;
> +    case MMU_DATA_LOAD:
> +        cs->exception_index = RISCV_EXCP_LOAD_ADDR_MIS;
> +        break;
> +    case MMU_DATA_STORE:
> +        cs->exception_index = RISCV_EXCP_STORE_AMO_ADDR_MIS;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    env->badaddr = addr;
> +    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> +                            riscv_cpu_two_stage_lookup(mmu_idx);
> +    cpu_loop_exit(cs);
>  }
>
>  bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> @@ -1307,11 +1325,15 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>      } else if (probe) {
>          return false;
>      } else {
> +        cpu_restore_state(cs, retaddr, true);
> +        if (env->unwind_amo) {
> +            access_type = MMU_DATA_STORE;
> +        }
>          raise_mmu_exception(env, address, access_type, pmp_violation,
>                              first_stage_error,
>                              riscv_cpu_virt_enabled(env) ||
>                                  riscv_cpu_two_stage_lookup(mmu_idx));
> -        cpu_loop_exit_restore(cs, retaddr);
> +        cpu_loop_exit(cs);
>      }
>
>      return true;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index fac998a6b5..ae4b0d1524 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -107,6 +107,10 @@ typedef struct DisasContext {
>      /* PointerMasking extension */
>      bool pm_mask_enabled;
>      bool pm_base_enabled;
> +#ifndef CONFIG_USER_ONLY
> +    /* TCG op of the current insn_start.  */
> +    TCGOp *insn_start;
> +#endif
>  } DisasContext;
>
>  static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -1105,7 +1109,12 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
>  {
>      DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
> +#ifdef CONFIG_USER_ONLY
>      tcg_gen_insn_start(ctx->base.pc_next);
> +#else
> +    tcg_gen_insn_start(ctx->base.pc_next, 0);
> +    ctx->insn_start = tcg_last_op();
> +#endif
>  }
>
>  static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
> diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
> index 45db82c9be..66faa8f1da 100644
> --- a/target/riscv/insn_trans/trans_rva.c.inc
> +++ b/target/riscv/insn_trans/trans_rva.c.inc
> @@ -37,6 +37,13 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
>      return true;
>  }
>
> +static void record_insn_start_amo(DisasContext *ctx)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    tcg_set_insn_start_param(ctx->insn_start, 1, 1);
> +#endif
> +}
> +
>  static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
>  {
>      TCGv dest, src1, src2;
> @@ -73,6 +80,7 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
>       */
>      tcg_gen_movi_tl(load_res, -1);
>
> +    record_insn_start_amo(ctx);
>      return true;
>  }
>
> @@ -85,8 +93,9 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
>      TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>
>      func(dest, src1, src2, ctx->mem_idx, mop);
> -
>      gen_set_gpr(ctx, a->rd, dest);
> +
> +    record_insn_start_amo(ctx);
>      return true;
>  }
>
> --
> 2.25.1
>
>


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target/riscv: Mark amo insns during translation
@ 2022-04-08  6:16     ` Alistair Francis
  0 siblings, 0 replies; 14+ messages in thread
From: Alistair Francis @ 2022-04-08  6:16 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
	Alistair Francis, Palmer Dabbelt

On Fri, Apr 1, 2022 at 11:04 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Atomic memory operations perform both reads and writes as part
> of their implementation, but always raise write faults.
>
> Use TARGET_INSN_START_EXTRA_WORDS to mark amo insns in the
> opcode stream, and force the access type to write at the
> point of raising the exception.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.h                      | 15 ++++++
>  target/riscv/cpu.c                      |  3 ++
>  target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
>  target/riscv/translate.c                |  9 ++++
>  target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
>  5 files changed, 79 insertions(+), 21 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c069fe85fa..3de4da3fa1 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -290,6 +290,13 @@ struct CPUArchState {
>      /* True if in debugger mode.  */
>      bool debugger;
>
> +    /*
> +     * True if unwinding through an amo insn.  Used to transform a
> +     * read fault into a store_amo fault; only valid immediately
> +     * after cpu_restore_state().
> +     */
> +    bool unwind_amo;
> +
>      /*
>       * CSRs for PointerMasking extension
>       */
> @@ -517,6 +524,14 @@ FIELD(TB_FLAGS, XL, 20, 2)
>  FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
>  FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
>
> +#ifndef CONFIG_USER_ONLY
> +/*
> + * RISC-V-specific extra insn start words:
> + * 1: True if the instruction is AMO, false otherwise.
> + */
> +#define TARGET_INSN_START_EXTRA_WORDS 1
> +#endif
> +
>  #ifdef TARGET_RISCV32
>  #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
>  #else
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..3818d5ba80 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -396,6 +396,9 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
>      } else {
>          env->pc = data[0];
>      }
> +#ifndef CONFIG_USER_ONLY
> +    env->unwind_amo = data[1];
> +#endif
>  }
>
>  static void riscv_cpu_reset(DeviceState *dev)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 126251d5da..b5bbe6fc39 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1139,26 +1139,11 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      CPURISCVState *env = &cpu->env;
>
> -    if (access_type == MMU_DATA_STORE) {
> -        cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> -    } else if (access_type == MMU_DATA_LOAD) {
> -        cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
> -    } else {
> -        cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
> +    cpu_restore_state(cs, retaddr, true);
> +    if (env->unwind_amo) {
> +        access_type = MMU_DATA_STORE;
>      }
>
> -    env->badaddr = addr;
> -    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> -                            riscv_cpu_two_stage_lookup(mmu_idx);
> -    cpu_loop_exit_restore(cs, retaddr);
> -}
> -
> -void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> -                                   MMUAccessType access_type, int mmu_idx,
> -                                   uintptr_t retaddr)
> -{
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -    CPURISCVState *env = &cpu->env;
>      switch (access_type) {
>      case MMU_INST_FETCH:
>          cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> @@ -1172,10 +1157,43 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>      default:
>          g_assert_not_reached();
>      }
> +
>      env->badaddr = addr;
>      env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
>                              riscv_cpu_two_stage_lookup(mmu_idx);
> -    cpu_loop_exit_restore(cs, retaddr);
> +    cpu_loop_exit(cs);
> +}
> +
> +void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> +                                   MMUAccessType access_type, int mmu_idx,
> +                                   uintptr_t retaddr)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    CPURISCVState *env = &cpu->env;
> +
> +    cpu_restore_state(cs, retaddr, true);
> +    if (env->unwind_amo) {
> +        access_type = MMU_DATA_STORE;
> +    }
> +
> +    switch (access_type) {
> +    case MMU_INST_FETCH:
> +        cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> +        break;
> +    case MMU_DATA_LOAD:
> +        cs->exception_index = RISCV_EXCP_LOAD_ADDR_MIS;
> +        break;
> +    case MMU_DATA_STORE:
> +        cs->exception_index = RISCV_EXCP_STORE_AMO_ADDR_MIS;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    env->badaddr = addr;
> +    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> +                            riscv_cpu_two_stage_lookup(mmu_idx);
> +    cpu_loop_exit(cs);
>  }
>
>  bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> @@ -1307,11 +1325,15 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>      } else if (probe) {
>          return false;
>      } else {
> +        cpu_restore_state(cs, retaddr, true);
> +        if (env->unwind_amo) {
> +            access_type = MMU_DATA_STORE;
> +        }
>          raise_mmu_exception(env, address, access_type, pmp_violation,
>                              first_stage_error,
>                              riscv_cpu_virt_enabled(env) ||
>                                  riscv_cpu_two_stage_lookup(mmu_idx));
> -        cpu_loop_exit_restore(cs, retaddr);
> +        cpu_loop_exit(cs);
>      }
>
>      return true;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index fac998a6b5..ae4b0d1524 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -107,6 +107,10 @@ typedef struct DisasContext {
>      /* PointerMasking extension */
>      bool pm_mask_enabled;
>      bool pm_base_enabled;
> +#ifndef CONFIG_USER_ONLY
> +    /* TCG op of the current insn_start.  */
> +    TCGOp *insn_start;
> +#endif
>  } DisasContext;
>
>  static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -1105,7 +1109,12 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
>  {
>      DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
> +#ifdef CONFIG_USER_ONLY
>      tcg_gen_insn_start(ctx->base.pc_next);
> +#else
> +    tcg_gen_insn_start(ctx->base.pc_next, 0);
> +    ctx->insn_start = tcg_last_op();
> +#endif
>  }
>
>  static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
> diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
> index 45db82c9be..66faa8f1da 100644
> --- a/target/riscv/insn_trans/trans_rva.c.inc
> +++ b/target/riscv/insn_trans/trans_rva.c.inc
> @@ -37,6 +37,13 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
>      return true;
>  }
>
> +static void record_insn_start_amo(DisasContext *ctx)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    tcg_set_insn_start_param(ctx->insn_start, 1, 1);
> +#endif
> +}
> +
>  static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
>  {
>      TCGv dest, src1, src2;
> @@ -73,6 +80,7 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
>       */
>      tcg_gen_movi_tl(load_res, -1);
>
> +    record_insn_start_amo(ctx);
>      return true;
>  }
>
> @@ -85,8 +93,9 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
>      TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>
>      func(dest, src1, src2, ctx->mem_idx, mop);
> -
>      gen_set_gpr(ctx, a->rd, dest);
> +
> +    record_insn_start_amo(ctx);
>      return true;
>  }
>
> --
> 2.25.1
>
>


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] target/riscv: Annotate atomic operations
  2022-04-01 12:59 ` Richard Henderson
@ 2022-04-12  0:10   ` Alistair Francis
  -1 siblings, 0 replies; 14+ messages in thread
From: Alistair Francis @ 2022-04-12  0:10 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Alistair Francis, Palmer Dabbelt, Bin Meng, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Fri, Apr 1, 2022 at 11:00 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> If an atomic operation fails on RISC-V, we want to generate
> a store/amo fault and not a load fault.
>
> Annotate amo insns, so that we can recognize them after unwinding.
> Transform the implementation access type to store/amo for reporting.
>
>
> r~
>
>
> Richard Henderson (2):
>   target/riscv: Use cpu_loop_exit_restore directly from mmu faults
>   target/riscv: Mark amo insns during translation
>
>  target/riscv/cpu.h                      | 15 ++++++
>  target/riscv/cpu.c                      |  3 ++
>  target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
>  target/riscv/translate.c                |  9 ++++
>  target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
>  5 files changed, 79 insertions(+), 21 deletions(-)

Thanks!

Applied to riscv-to-apply.next

Alistair

>
> --
> 2.25.1
>
>


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] target/riscv: Annotate atomic operations
@ 2022-04-12  0:10   ` Alistair Francis
  0 siblings, 0 replies; 14+ messages in thread
From: Alistair Francis @ 2022-04-12  0:10 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
	Alistair Francis, Palmer Dabbelt

On Fri, Apr 1, 2022 at 11:00 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> If an atomic operation fails on RISC-V, we want to generate
> a store/amo fault and not a load fault.
>
> Annotate amo insns, so that we can recognize them after unwinding.
> Transform the implementation access type to store/amo for reporting.
>
>
> r~
>
>
> Richard Henderson (2):
>   target/riscv: Use cpu_loop_exit_restore directly from mmu faults
>   target/riscv: Mark amo insns during translation
>
>  target/riscv/cpu.h                      | 15 ++++++
>  target/riscv/cpu.c                      |  3 ++
>  target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
>  target/riscv/translate.c                |  9 ++++
>  target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
>  5 files changed, 79 insertions(+), 21 deletions(-)

Thanks!

Applied to riscv-to-apply.next

Alistair

>
> --
> 2.25.1
>
>


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target/riscv: Mark amo insns during translation
  2022-04-08  6:16     ` Alistair Francis
@ 2022-04-19  7:43       ` Atish Patra
  -1 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2022-04-19  7:43 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Bin Meng, Richard Henderson,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

On Thu, Apr 7, 2022 at 11:17 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Apr 1, 2022 at 11:04 PM Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > Atomic memory operations perform both reads and writes as part
> > of their implementation, but always raise write faults.
> >
> > Use TARGET_INSN_START_EXTRA_WORDS to mark amo insns in the
> > opcode stream, and force the access type to write at the
> > point of raising the exception.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>

This results in the following segfault in 5.17
Qemu tree: riscv-to-apply.next

[    1.134496] Run /init as init process
[    1.329796] mount[61]: unhandled signal 11 code 0x2 at
0x00000000000c29c6 in busybox[10000+159000]
[    1.331185] CPU: 2 PID: 61 Comm: mount Not tainted 5.17.0 #59
[    1.331632] Hardware name: riscv-virtio,qemu (DT)
[    1.332053] epc : 00000000000c29c6 ra : 00000000000a03f2 sp :
00007fffd6707ae0
[    1.332350]  gp : 000000000016c408 tp : 00000000001707a0 t0 :
0000000000001000
[    1.332575]  t1 : 0000000000000000 t2 : 0000000000080000 s0 :
0000000000163398
[    1.332797]  s1 : 0000000000163398 a0 : 0000000000000000 a1 :
0000000000000000
[    1.333018]  a2 : 0000000000171590 a3 : 0000000000000000 a4 :
0000000000000001
[    1.333371]  a5 : 0000000000000001 a6 : fffffffffbada498 a7 :
000000000000003f
[    1.333607]  s2 : 0000000000000004 s3 : 0000000000000001 s4 :
0000000000000000
[    1.333829]  s5 : 0000000000000003 s6 : 0000000000000000 s7 :
000000000016c280
[    1.334052]  s8 : 0000000000000001 s9 : 0000000000170828 s10:
0000000000000000
[    1.334275]  s11: 0000000000000001 t3 : 0000000000000000 t4 :
00000000001410f0
[    1.334500]  t5 : 0000000000000005 t6 : ffffffffffffffff
[    1.334669] status: 0000000200004020 badaddr: 00000000000c29c6
cause: 000000000000000f
Segmentation fault



> Alistair
>
> > ---
> >  target/riscv/cpu.h                      | 15 ++++++
> >  target/riscv/cpu.c                      |  3 ++
> >  target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
> >  target/riscv/translate.c                |  9 ++++
> >  target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
> >  5 files changed, 79 insertions(+), 21 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index c069fe85fa..3de4da3fa1 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -290,6 +290,13 @@ struct CPUArchState {
> >      /* True if in debugger mode.  */
> >      bool debugger;
> >
> > +    /*
> > +     * True if unwinding through an amo insn.  Used to transform a
> > +     * read fault into a store_amo fault; only valid immediately
> > +     * after cpu_restore_state().
> > +     */
> > +    bool unwind_amo;
> > +
> >      /*
> >       * CSRs for PointerMasking extension
> >       */
> > @@ -517,6 +524,14 @@ FIELD(TB_FLAGS, XL, 20, 2)
> >  FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
> >  FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
> >
> > +#ifndef CONFIG_USER_ONLY
> > +/*
> > + * RISC-V-specific extra insn start words:
> > + * 1: True if the instruction is AMO, false otherwise.
> > + */
> > +#define TARGET_INSN_START_EXTRA_WORDS 1
> > +#endif
> > +
> >  #ifdef TARGET_RISCV32
> >  #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
> >  #else
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index ddda4906ff..3818d5ba80 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -396,6 +396,9 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
> >      } else {
> >          env->pc = data[0];
> >      }
> > +#ifndef CONFIG_USER_ONLY
> > +    env->unwind_amo = data[1];
> > +#endif
> >  }
> >
> >  static void riscv_cpu_reset(DeviceState *dev)
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 126251d5da..b5bbe6fc39 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -1139,26 +1139,11 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> >      RISCVCPU *cpu = RISCV_CPU(cs);
> >      CPURISCVState *env = &cpu->env;
> >
> > -    if (access_type == MMU_DATA_STORE) {
> > -        cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> > -    } else if (access_type == MMU_DATA_LOAD) {
> > -        cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
> > -    } else {
> > -        cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
> > +    cpu_restore_state(cs, retaddr, true);
> > +    if (env->unwind_amo) {
> > +        access_type = MMU_DATA_STORE;
> >      }
> >
> > -    env->badaddr = addr;
> > -    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> > -                            riscv_cpu_two_stage_lookup(mmu_idx);
> > -    cpu_loop_exit_restore(cs, retaddr);
> > -}
> > -
> > -void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> > -                                   MMUAccessType access_type, int mmu_idx,
> > -                                   uintptr_t retaddr)
> > -{
> > -    RISCVCPU *cpu = RISCV_CPU(cs);
> > -    CPURISCVState *env = &cpu->env;
> >      switch (access_type) {
> >      case MMU_INST_FETCH:
> >          cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> > @@ -1172,10 +1157,43 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> >      default:
> >          g_assert_not_reached();
> >      }
> > +
> >      env->badaddr = addr;
> >      env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> >                              riscv_cpu_two_stage_lookup(mmu_idx);
> > -    cpu_loop_exit_restore(cs, retaddr);
> > +    cpu_loop_exit(cs);
> > +}
> > +
> > +void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> > +                                   MMUAccessType access_type, int mmu_idx,
> > +                                   uintptr_t retaddr)
> > +{
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +    CPURISCVState *env = &cpu->env;
> > +
> > +    cpu_restore_state(cs, retaddr, true);
> > +    if (env->unwind_amo) {
> > +        access_type = MMU_DATA_STORE;
> > +    }
> > +
> > +    switch (access_type) {
> > +    case MMU_INST_FETCH:
> > +        cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> > +        break;
> > +    case MMU_DATA_LOAD:
> > +        cs->exception_index = RISCV_EXCP_LOAD_ADDR_MIS;
> > +        break;
> > +    case MMU_DATA_STORE:
> > +        cs->exception_index = RISCV_EXCP_STORE_AMO_ADDR_MIS;
> > +        break;
> > +    default:
> > +        g_assert_not_reached();
> > +    }
> > +
> > +    env->badaddr = addr;
> > +    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> > +                            riscv_cpu_two_stage_lookup(mmu_idx);
> > +    cpu_loop_exit(cs);
> >  }
> >
> >  bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > @@ -1307,11 +1325,15 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> >      } else if (probe) {
> >          return false;
> >      } else {
> > +        cpu_restore_state(cs, retaddr, true);
> > +        if (env->unwind_amo) {
> > +            access_type = MMU_DATA_STORE;
> > +        }
> >          raise_mmu_exception(env, address, access_type, pmp_violation,
> >                              first_stage_error,
> >                              riscv_cpu_virt_enabled(env) ||
> >                                  riscv_cpu_two_stage_lookup(mmu_idx));
> > -        cpu_loop_exit_restore(cs, retaddr);
> > +        cpu_loop_exit(cs);
> >      }
> >
> >      return true;
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index fac998a6b5..ae4b0d1524 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -107,6 +107,10 @@ typedef struct DisasContext {
> >      /* PointerMasking extension */
> >      bool pm_mask_enabled;
> >      bool pm_base_enabled;
> > +#ifndef CONFIG_USER_ONLY
> > +    /* TCG op of the current insn_start.  */
> > +    TCGOp *insn_start;
> > +#endif
> >  } DisasContext;
> >
> >  static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> > @@ -1105,7 +1109,12 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
> >  {
> >      DisasContext *ctx = container_of(dcbase, DisasContext, base);
> >
> > +#ifdef CONFIG_USER_ONLY
> >      tcg_gen_insn_start(ctx->base.pc_next);
> > +#else
> > +    tcg_gen_insn_start(ctx->base.pc_next, 0);
> > +    ctx->insn_start = tcg_last_op();
> > +#endif
> >  }
> >
> >  static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
> > diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
> > index 45db82c9be..66faa8f1da 100644
> > --- a/target/riscv/insn_trans/trans_rva.c.inc
> > +++ b/target/riscv/insn_trans/trans_rva.c.inc
> > @@ -37,6 +37,13 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
> >      return true;
> >  }
> >
> > +static void record_insn_start_amo(DisasContext *ctx)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> > +    tcg_set_insn_start_param(ctx->insn_start, 1, 1);
> > +#endif
> > +}
> > +
> >  static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
> >  {
> >      TCGv dest, src1, src2;
> > @@ -73,6 +80,7 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
> >       */
> >      tcg_gen_movi_tl(load_res, -1);
> >
> > +    record_insn_start_amo(ctx);
> >      return true;
> >  }
> >
> > @@ -85,8 +93,9 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
> >      TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> >
> >      func(dest, src1, src2, ctx->mem_idx, mop);
> > -
> >      gen_set_gpr(ctx, a->rd, dest);
> > +
> > +    record_insn_start_amo(ctx);
> >      return true;
> >  }
> >
> > --
> > 2.25.1
> >
> >
>


-- 
Regards,
Atish


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target/riscv: Mark amo insns during translation
@ 2022-04-19  7:43       ` Atish Patra
  0 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2022-04-19  7:43 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Richard Henderson, Alistair Francis, Palmer Dabbelt, Bin Meng,
	open list:RISC-V, qemu-devel@nongnu.org Developers

On Thu, Apr 7, 2022 at 11:17 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Apr 1, 2022 at 11:04 PM Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > Atomic memory operations perform both reads and writes as part
> > of their implementation, but always raise write faults.
> >
> > Use TARGET_INSN_START_EXTRA_WORDS to mark amo insns in the
> > opcode stream, and force the access type to write at the
> > point of raising the exception.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>

This results in the following segfault in 5.17
Qemu tree: riscv-to-apply.next

[    1.134496] Run /init as init process
[    1.329796] mount[61]: unhandled signal 11 code 0x2 at
0x00000000000c29c6 in busybox[10000+159000]
[    1.331185] CPU: 2 PID: 61 Comm: mount Not tainted 5.17.0 #59
[    1.331632] Hardware name: riscv-virtio,qemu (DT)
[    1.332053] epc : 00000000000c29c6 ra : 00000000000a03f2 sp :
00007fffd6707ae0
[    1.332350]  gp : 000000000016c408 tp : 00000000001707a0 t0 :
0000000000001000
[    1.332575]  t1 : 0000000000000000 t2 : 0000000000080000 s0 :
0000000000163398
[    1.332797]  s1 : 0000000000163398 a0 : 0000000000000000 a1 :
0000000000000000
[    1.333018]  a2 : 0000000000171590 a3 : 0000000000000000 a4 :
0000000000000001
[    1.333371]  a5 : 0000000000000001 a6 : fffffffffbada498 a7 :
000000000000003f
[    1.333607]  s2 : 0000000000000004 s3 : 0000000000000001 s4 :
0000000000000000
[    1.333829]  s5 : 0000000000000003 s6 : 0000000000000000 s7 :
000000000016c280
[    1.334052]  s8 : 0000000000000001 s9 : 0000000000170828 s10:
0000000000000000
[    1.334275]  s11: 0000000000000001 t3 : 0000000000000000 t4 :
00000000001410f0
[    1.334500]  t5 : 0000000000000005 t6 : ffffffffffffffff
[    1.334669] status: 0000000200004020 badaddr: 00000000000c29c6
cause: 000000000000000f
Segmentation fault



> Alistair
>
> > ---
> >  target/riscv/cpu.h                      | 15 ++++++
> >  target/riscv/cpu.c                      |  3 ++
> >  target/riscv/cpu_helper.c               | 62 +++++++++++++++++--------
> >  target/riscv/translate.c                |  9 ++++
> >  target/riscv/insn_trans/trans_rva.c.inc | 11 ++++-
> >  5 files changed, 79 insertions(+), 21 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index c069fe85fa..3de4da3fa1 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -290,6 +290,13 @@ struct CPUArchState {
> >      /* True if in debugger mode.  */
> >      bool debugger;
> >
> > +    /*
> > +     * True if unwinding through an amo insn.  Used to transform a
> > +     * read fault into a store_amo fault; only valid immediately
> > +     * after cpu_restore_state().
> > +     */
> > +    bool unwind_amo;
> > +
> >      /*
> >       * CSRs for PointerMasking extension
> >       */
> > @@ -517,6 +524,14 @@ FIELD(TB_FLAGS, XL, 20, 2)
> >  FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
> >  FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
> >
> > +#ifndef CONFIG_USER_ONLY
> > +/*
> > + * RISC-V-specific extra insn start words:
> > + * 1: True if the instruction is AMO, false otherwise.
> > + */
> > +#define TARGET_INSN_START_EXTRA_WORDS 1
> > +#endif
> > +
> >  #ifdef TARGET_RISCV32
> >  #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
> >  #else
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index ddda4906ff..3818d5ba80 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -396,6 +396,9 @@ void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb,
> >      } else {
> >          env->pc = data[0];
> >      }
> > +#ifndef CONFIG_USER_ONLY
> > +    env->unwind_amo = data[1];
> > +#endif
> >  }
> >
> >  static void riscv_cpu_reset(DeviceState *dev)
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 126251d5da..b5bbe6fc39 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -1139,26 +1139,11 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> >      RISCVCPU *cpu = RISCV_CPU(cs);
> >      CPURISCVState *env = &cpu->env;
> >
> > -    if (access_type == MMU_DATA_STORE) {
> > -        cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> > -    } else if (access_type == MMU_DATA_LOAD) {
> > -        cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
> > -    } else {
> > -        cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
> > +    cpu_restore_state(cs, retaddr, true);
> > +    if (env->unwind_amo) {
> > +        access_type = MMU_DATA_STORE;
> >      }
> >
> > -    env->badaddr = addr;
> > -    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> > -                            riscv_cpu_two_stage_lookup(mmu_idx);
> > -    cpu_loop_exit_restore(cs, retaddr);
> > -}
> > -
> > -void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> > -                                   MMUAccessType access_type, int mmu_idx,
> > -                                   uintptr_t retaddr)
> > -{
> > -    RISCVCPU *cpu = RISCV_CPU(cs);
> > -    CPURISCVState *env = &cpu->env;
> >      switch (access_type) {
> >      case MMU_INST_FETCH:
> >          cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> > @@ -1172,10 +1157,43 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> >      default:
> >          g_assert_not_reached();
> >      }
> > +
> >      env->badaddr = addr;
> >      env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> >                              riscv_cpu_two_stage_lookup(mmu_idx);
> > -    cpu_loop_exit_restore(cs, retaddr);
> > +    cpu_loop_exit(cs);
> > +}
> > +
> > +void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> > +                                   MMUAccessType access_type, int mmu_idx,
> > +                                   uintptr_t retaddr)
> > +{
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +    CPURISCVState *env = &cpu->env;
> > +
> > +    cpu_restore_state(cs, retaddr, true);
> > +    if (env->unwind_amo) {
> > +        access_type = MMU_DATA_STORE;
> > +    }
> > +
> > +    switch (access_type) {
> > +    case MMU_INST_FETCH:
> > +        cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> > +        break;
> > +    case MMU_DATA_LOAD:
> > +        cs->exception_index = RISCV_EXCP_LOAD_ADDR_MIS;
> > +        break;
> > +    case MMU_DATA_STORE:
> > +        cs->exception_index = RISCV_EXCP_STORE_AMO_ADDR_MIS;
> > +        break;
> > +    default:
> > +        g_assert_not_reached();
> > +    }
> > +
> > +    env->badaddr = addr;
> > +    env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
> > +                            riscv_cpu_two_stage_lookup(mmu_idx);
> > +    cpu_loop_exit(cs);
> >  }
> >
> >  bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > @@ -1307,11 +1325,15 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> >      } else if (probe) {
> >          return false;
> >      } else {
> > +        cpu_restore_state(cs, retaddr, true);
> > +        if (env->unwind_amo) {
> > +            access_type = MMU_DATA_STORE;
> > +        }
> >          raise_mmu_exception(env, address, access_type, pmp_violation,
> >                              first_stage_error,
> >                              riscv_cpu_virt_enabled(env) ||
> >                                  riscv_cpu_two_stage_lookup(mmu_idx));
> > -        cpu_loop_exit_restore(cs, retaddr);
> > +        cpu_loop_exit(cs);
> >      }
> >
> >      return true;
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index fac998a6b5..ae4b0d1524 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -107,6 +107,10 @@ typedef struct DisasContext {
> >      /* PointerMasking extension */
> >      bool pm_mask_enabled;
> >      bool pm_base_enabled;
> > +#ifndef CONFIG_USER_ONLY
> > +    /* TCG op of the current insn_start.  */
> > +    TCGOp *insn_start;
> > +#endif
> >  } DisasContext;
> >
> >  static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> > @@ -1105,7 +1109,12 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
> >  {
> >      DisasContext *ctx = container_of(dcbase, DisasContext, base);
> >
> > +#ifdef CONFIG_USER_ONLY
> >      tcg_gen_insn_start(ctx->base.pc_next);
> > +#else
> > +    tcg_gen_insn_start(ctx->base.pc_next, 0);
> > +    ctx->insn_start = tcg_last_op();
> > +#endif
> >  }
> >
> >  static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
> > diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
> > index 45db82c9be..66faa8f1da 100644
> > --- a/target/riscv/insn_trans/trans_rva.c.inc
> > +++ b/target/riscv/insn_trans/trans_rva.c.inc
> > @@ -37,6 +37,13 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
> >      return true;
> >  }
> >
> > +static void record_insn_start_amo(DisasContext *ctx)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> > +    tcg_set_insn_start_param(ctx->insn_start, 1, 1);
> > +#endif
> > +}
> > +
> >  static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
> >  {
> >      TCGv dest, src1, src2;
> > @@ -73,6 +80,7 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
> >       */
> >      tcg_gen_movi_tl(load_res, -1);
> >
> > +    record_insn_start_amo(ctx);
> >      return true;
> >  }
> >
> > @@ -85,8 +93,9 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
> >      TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> >
> >      func(dest, src1, src2, ctx->mem_idx, mop);
> > -
> >      gen_set_gpr(ctx, a->rd, dest);
> > +
> > +    record_insn_start_amo(ctx);
> >      return true;
> >  }
> >
> > --
> > 2.25.1
> >
> >
>


-- 
Regards,
Atish


^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-04-19  8:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-01 12:59 [PATCH 0/2] target/riscv: Annotate atomic operations Richard Henderson
2022-04-01 12:59 ` Richard Henderson
2022-04-01 12:59 ` [PATCH 1/2] target/riscv: Use cpu_loop_exit_restore directly from mmu faults Richard Henderson
2022-04-01 12:59   ` Richard Henderson
2022-04-08  4:35   ` Alistair Francis
2022-04-08  4:35     ` Alistair Francis
2022-04-01 12:59 ` [PATCH 2/2] target/riscv: Mark amo insns during translation Richard Henderson
2022-04-01 12:59   ` Richard Henderson
2022-04-08  6:16   ` Alistair Francis
2022-04-08  6:16     ` Alistair Francis
2022-04-19  7:43     ` Atish Patra
2022-04-19  7:43       ` Atish Patra
2022-04-12  0:10 ` [PATCH 0/2] target/riscv: Annotate atomic operations Alistair Francis
2022-04-12  0:10   ` Alistair Francis

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.