Re: [PATCH v5 11/15] target/riscv: mmu changes for zicfiss shadow stack protection

[Deepak Gupta]

On Mon, Aug 19, 2024 at 05:01:25PM -0700, Deepak Gupta wrote:
> zicfiss protects shadow stack using new page table encodings PTE.W=0,
> PTE.R=0 and PTE.X=0. This encoding is reserved if zicfiss is not
> implemented or if shadow stack are not enabled.
> Loads on shadow stack memory are allowed while stores to shadow stack
> memory leads to access faults. Shadow stack accesses to RO memory
> leads to store page fault.
>
> To implement special nature of shadow stack memory where only selected
> stores (shadow stack stores from sspush) have to be allowed while rest
> of regular stores disallowed, new MMU TLB index is created for shadow
> stack.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> @@ -1406,6 +1431,11 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
> size,
>     qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
>                   __func__, address, access_type, mmu_idx);
>
> +    /* 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?

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;
+     }
+
     switch (access_type) {
     case MMU_INST_FETCH:
         if (pmp_violation) {
@@ -1433,7 +1438,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 
     /* If shadow stack instruction initiated this access, treat it as store */
     if (mmu_idx & MMU_IDX_SS_WRITE) {
-        access_type = MMU_DATA_STORE;
+        tcg_ctx->gen_insn_data[TARGET_INSN_START_EXTRA_WORDS-1] |= 1;
     }
 
     pmu_tlb_fill_incr_ctr(cpu, access_type);
@@ -1529,6 +1534,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
     if (ret == TRANSLATE_SUCCESS) {
         tlb_set_page(cs, address & ~(tlb_size - 1), pa & ~(tlb_size - 1),
                      prot, mmu_idx, tlb_size);
+        tcg_ctx->gen_insn_data[TARGET_INSN_START_EXTRA_WORDS-1] &= ~1;
         return true;
     } else if (probe) {


"""

>     pmu_tlb_fill_incr_ctr(cpu, access_type);
>     if (two_stage_lookup) {
>         /* Two stage lookup */
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index 0ac17bc5ad..ddbdee885b 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -30,12 +30,15 @@
>  *  - U+2STAGE          0b100
>  *  - S+2STAGE          0b101
>  *  - S+SUM+2STAGE      0b110
> + *  - Shadow stack+U   0b1000
> + *  - Shadow stack+S   0b1001
>  */
> #define MMUIdx_U            0
> #define MMUIdx_S            1
> #define MMUIdx_S_SUM        2
> #define MMUIdx_M            3
> #define MMU_2STAGE_BIT      (1 << 2)
> +#define MMU_IDX_SS_WRITE    (1 << 3)
>
> static inline int mmuidx_priv(int mmu_idx)
> {
> --
> 2.44.0