On 8/20/24 17:35, Deepak Gupta wrote:
+ /* If shadow stack instruction initiated this access, treat it as store */
+ if (mmu_idx & MMU_IDX_SS_WRITE) {
+ access_type = MMU_DATA_STORE;
+ }
+
I think I forgot to address this. Do you still want me to fix this up like you
had suggested?
Yes, this still needs fixing.
IIRC, you mentioned to use TARGET_INSN_START_EXTRA_WORDS=2. Honestly I don't
know
what it means and how its used. Based on git grep and some readup, are you expecting
something
along the below lines?
"""
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index fee31b8037..dfd2efa941 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -47,7 +47,7 @@ typedef struct CPUArchState CPURISCVState;
* RISC-V-specific extra insn start words:
* 1: Original instruction opcode
*/
-#define TARGET_INSN_START_EXTRA_WORDS 1
+#define TARGET_INSN_START_EXTRA_WORDS 2
#define RV(x) ((target_ulong)1 << (x - 'A'))
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f74a1216b1..b266177e46 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1271,6 +1271,11 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong
address,
{
CPUState *cs = env_cpu(env);
+ if (!pmp_violation &&
+ tcg_ctx->gen_insn_data[TARGET_INSN_START_EXTRA_WORDS-1] & 1) {
+ tcg_ctx->gen_insn_data[TARGET_INSN_START_EXTRA_WORDS-1] &= ~1;
+ access_type = MMU_DATA_STORE;
+ }
The first thing to understand is that the unwind data is stored by the compiler and
recovered by the unwinder.
The unwind data is exposed to the target via one of two methods:
(1) TCGCPUOps.restore_state_to_opc, i.e. riscv_restore_state_to_opc.
The data[] argument contains the extra words.
With this method, the extra words are restored to env and are
available in a later call to riscv_cpu_do_interrupt.
Compare env->bins from the first extra word, which is used exactly so.
This is probably the easiest and best option.
You'd promote LOAD* to STORE_AMO* while dispatching the interrupt.
(2) cpu_unwind_state_data()
With this method, you have immediate access to the extra words,
and don't need to store them anywhere else.
This is supposed to be used when we are *not* going to raise
an exception, merely look something up and continue execution.
Otherwise, we'd be performing the unwind operation twice,
and it's not cheap.
So, tcg_ctx->gen_insn_data[] is not something you'd ever touch,
and this is the wrong spot to do anything.
r~
