[PATCH v11 00/20] riscv support for control flow integrity extensions

[PATCH v11 00/20] riscv support for control flow integrity extensions

[Deepak Gupta]

v11 for riscv zicfilp and zicfiss extensions support in qemu.

Links for previous versions
[1] - v1 https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg06017.html
[2] - v2 https://lore.kernel.org/all/ed23bcbc-fdc4-4492-803c-daa95880375a@linaro.org/T/
[3] - v3 https://lists.nongnu.org/archive/html/qemu-devel/2024-08/msg01005.html
[4] - v4 https://lore.kernel.org/all/20240816010711.3055425-6-debug@rivosinc.com/T/
[5] - v5
+https://lore.kernel.org/all/20240820000129.3522346-1-debug@rivosinc.com/T/#m7b9cc847e739ec86f9569a3ca9f3d9377b01e21
[6] - v6 https://mail.gnu.org/archive/html/qemu-riscv/2024-08/msg00418.html
[7] - v7 https://lore.kernel.org/all/20240822082504.3979610-1-debug@rivosinc.com/
[8] - v8 https://lore.kernel.org/all/20240823190140.4156920-1-debug@rivosinc.com/T/
[9] - v9 https://lore.kernel.org/all/20240826152949.294506-1-debug@rivosinc.com/

---
v10:
   - Exposed *envcfg CSR and priv to qemu-user as well and removed special
     state management for *envcfg related feature enabling for qemu-user
   - Exposing zicfilp and zicfiss as different patch
v9:
   - fix switch case fallthrough for sw_check excp in patch 4
v8:
   - fixed up `gen_cmpxchg` to store extra word2 during compile to raise storeAMO always
v7:
   - Updated decode_save_opc to take extra argument of excp_uw2 and
     updated callsites
   - added a helper for promoting load faults to store faults
   - Removed stale comments and edited existed comments
v6:
   - Added support extra store word 2 for tcg compile and extraction during unwind
   - Using extra word, AMO instructions and shadow stack instructions can raise store fault
   - some alignment and cosmetic changes
   - added vmstate migration support for elp and ssp cpu state
v5:
   - Simplified elp tracking and lpad implementation as per suggestion by richard
   - Simplified shadow stack mmu checks as per suggestion by richard
   - Converged zicfiss compressed and non-comressed instructions to same translation
   - Removed trace hooks. Don't need for upstream.

v4:
   - elp state in cpu is true/false instead of enum and elp cleared
     unconditionally on trap entry. elp in *status cleared unconditionally on
     trap return.
   - Moved logic for branch tracking in instruction translation from tb_start.
   - fixed zicfiss dependency on 'A'
   - `cpu_get_fcfien/bcfien` helpers checks fixed to check for extension first.
   - removed trace hook enums. Instead added dedicated trace helpers wherever needed.
   - fixed/simplified instruction format in decoder for lpad, sspush, sspopchk
   - simplified tlb index logic for shadow stack instructions. Removed SUM TB_FLAG
   - access to ssp CSR is gated on `cpu_get_bcfien` instead of duplicated logic
   - removed vDSO related changes for now.
v3:
   - Removed prctl specific patches because they need to be upstream
     in kernel first.
   - As suggested by Richard, added TB flag if fcfi enabled
   - Re-worked translation for landing pad and shadow stack instructions
     to not require helper.
   - tcg helpers only for cfi violation cases so that trace hooks can be
     placed.
   - Style changes.
   - fixes assert condition in accel/tcg

v2:
   - added missed file (in v1) for shadow stack instructions implementation.

Deepak Gupta (20):
  target/riscv: expose *envcfg csr and priv to qemu-user as well
  target/riscv: Add zicfilp extension
  target/riscv: Introduce elp state and enabling controls for zicfilp
  target/riscv: save and restore elp state on priv transitions
  target/riscv: additional code information for sw check
  target/riscv: tracking indirect branches (fcfi) for zicfilp
  target/riscv: zicfilp `lpad` impl and branch tracking
  disas/riscv: enable `lpad` disassembly
  target/riscv: Expose zicfilp extension as a cpu property
  target/riscv: Add zicfiss extension
  target/riscv: introduce ssp and enabling controls for zicfiss
  target/riscv: tb flag for shadow stack  instructions
  target/riscv: mmu changes for zicfiss shadow stack protection
  target/riscv: AMO operations always raise store/AMO fault
  target/riscv: update `decode_save_opc` to store extra word2
  target/riscv: implement zicfiss instructions
  target/riscv: compressed encodings for sspush and sspopchk
  disas/riscv: enable disassembly for zicfiss instructions
  disas/riscv: enable disassembly for compressed sspush/sspopchk
  target/riscv: Expose zicfiss extension as a cpu property

 disas/riscv.c                                 |  77 ++++++++-
 disas/riscv.h                                 |   4 +
 target/riscv/cpu.c                            |  14 ++
 target/riscv/cpu.h                            |  31 +++-
 target/riscv/cpu_bits.h                       |  17 ++
 target/riscv/cpu_cfg.h                        |   2 +
 target/riscv/cpu_helper.c                     | 155 +++++++++++++++++-
 target/riscv/cpu_user.h                       |   1 +
 target/riscv/csr.c                            |  84 ++++++++++
 target/riscv/insn16.decode                    |   4 +
 target/riscv/insn32.decode                    |  26 ++-
 .../riscv/insn_trans/trans_privileged.c.inc   |   8 +-
 target/riscv/insn_trans/trans_rva.c.inc       |  43 ++++-
 target/riscv/insn_trans/trans_rvd.c.inc       |   4 +-
 target/riscv/insn_trans/trans_rvf.c.inc       |   4 +-
 target/riscv/insn_trans/trans_rvh.c.inc       |   8 +-
 target/riscv/insn_trans/trans_rvi.c.inc       |  61 ++++++-
 target/riscv/insn_trans/trans_rvvk.c.inc      |  10 +-
 target/riscv/insn_trans/trans_rvzacas.c.inc   |   4 +-
 target/riscv/insn_trans/trans_rvzfh.c.inc     |   4 +-
 target/riscv/insn_trans/trans_rvzicfiss.c.inc |  75 +++++++++
 target/riscv/insn_trans/trans_svinval.c.inc   |   6 +-
 target/riscv/internals.h                      |   3 +
 target/riscv/machine.c                        |  38 +++++
 target/riscv/op_helper.c                      |  18 ++
 target/riscv/pmp.c                            |   5 +
 target/riscv/pmp.h                            |   3 +-
 target/riscv/tcg/tcg-cpu.c                    |  25 +++
 target/riscv/translate.c                      |  45 ++++-
 29 files changed, 726 insertions(+), 53 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvzicfiss.c.inc

-- 
2.44.0

[PATCH v11 01/20] target/riscv: expose *envcfg csr and priv to qemu-user as well

[Deepak Gupta]

Execution environment config CSR controlling user env and current
privilege state shouldn't be limited to qemu-system only. *envcfg
CSRs control enabling of features in next lesser mode. In some cases
bits *envcfg CSR can be lit up by kernel as part of kernel policy or
software (user app) can choose to opt-in by issuing a system call
(e.g. prctl). In case of qemu-user, it should be no different because
qemu is providing underlying execution environment facility and thus
either should provide some default value in *envcfg CSRs or react to
system calls (prctls) initiated from application. priv is set to PRV_U
and menvcfg/senvcfg set to 0 for qemu-user on reest.

`henvcfg` has been left for qemu-system only because it is not expected
that someone will use qemu-user where application is expected to have
hypervisor underneath which is controlling its execution environment. If
such a need arises then `henvcfg` could be exposed as well.

Relevant discussion:
https://lore.kernel.org/all/CAKmqyKOTVWPFep2msTQVdUmJErkH+bqCcKEQ4hAnyDFPdWKe0Q@mail.gmail.com/

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 5 +++++
 target/riscv/cpu.h | 9 +++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 33ef4eb795..c4ea1d4038 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1011,7 +1011,12 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
     }
 
     pmp_unlock_entries(env);
+#else
+    env->priv = PRV_U;
+    env->senvcfg = 0;
+    env->menvcfg = 0;
 #endif
+
     env->xl = riscv_cpu_mxl(env);
     riscv_cpu_update_mask(env);
     cs->exception_index = RISCV_EXCP_NONE;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 87742047ce..270a2a031c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -226,8 +226,12 @@ struct CPUArchState {
     uint32_t elf_flags;
 #endif
 
-#ifndef CONFIG_USER_ONLY
     target_ulong priv;
+    /* CSRs for execution environment configuration */
+    uint64_t menvcfg;
+    target_ulong senvcfg;
+
+#ifndef CONFIG_USER_ONLY
     /* This contains QEMU specific information about the virt state. */
     bool virt_enabled;
     target_ulong geilen;
@@ -429,12 +433,9 @@ struct CPUArchState {
     target_ulong upmmask;
     target_ulong upmbase;
 
-    /* CSRs for execution environment configuration */
-    uint64_t menvcfg;
     uint64_t mstateen[SMSTATEEN_MAX_COUNT];
     uint64_t hstateen[SMSTATEEN_MAX_COUNT];
     uint64_t sstateen[SMSTATEEN_MAX_COUNT];
-    target_ulong senvcfg;
     uint64_t henvcfg;
 #endif
     target_ulong cur_pmmask;
-- 
2.44.0

[PATCH v11 02/20] target/riscv: Add zicfilp extension

[Deepak Gupta]

zicfilp [1] riscv cpu extension enables forward control flow integrity.
If enabled, all indirect calls must land on a landing pad instruction.

This patch sets up space for zicfilp extension in cpuconfig. zicfilp
is dependend on zicsr.

[1] - https://github.com/riscv/riscv-cfi

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c         | 1 +
 target/riscv/cpu_cfg.h     | 1 +
 target/riscv/tcg/tcg-cpu.c | 5 +++++
 3 files changed, 7 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c4ea1d4038..e3f0bd9242 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -106,6 +106,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(ziccif, PRIV_VERSION_1_11_0, has_priv_1_11),
     ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, has_priv_1_11),
     ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, has_priv_1_11),
+    ISA_EXT_DATA_ENTRY(zicfilp, PRIV_VERSION_1_12_0, ext_zicfilp),
     ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
     ISA_EXT_DATA_ENTRY(zicntr, PRIV_VERSION_1_12_0, ext_zicntr),
     ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_zicsr),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 120905a254..88d5defbb5 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -67,6 +67,7 @@ struct RISCVCPUConfig {
     bool ext_zicbom;
     bool ext_zicbop;
     bool ext_zicboz;
+    bool ext_zicfilp;
     bool ext_zicond;
     bool ext_zihintntl;
     bool ext_zihintpause;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index b8814ab753..ed19586c9d 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -623,6 +623,11 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
         cpu->pmu_avail_ctrs = 0;
     }
 
+    if (cpu->cfg.ext_zicfilp && !cpu->cfg.ext_zicsr) {
+        error_setg(errp, "zicfilp extension requires zicsr extension");
+        return;
+    }
+
     /*
      * Disable isa extensions based on priv spec after we
      * validated and set everything we need.
-- 
2.44.0

[PATCH v11 03/20] target/riscv: Introduce elp state and enabling controls for zicfilp

[Deepak Gupta]

zicfilp introduces a new state elp ("expected landing pad") in cpu.
During normal execution, elp is idle (NO_LP_EXPECTED) i.e not expecting
landing pad. On an indirect call, elp moves LP_EXPECTED. When elp is
LP_EXPECTED, only a subsquent landing pad instruction can set state back
to NO_LP_EXPECTED. On reset, elp is set to NO_LP_EXPECTED.

zicfilp is enabled via bit2 in *envcfg CSRs. Enabling control for M-mode
is in mseccfg CSR at bit position 10.

On trap, elp state is saved away in *status.
Adds elp to the migration state as well.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c      |  3 +++
 target/riscv/cpu.h      |  2 ++
 target/riscv/cpu_bits.h |  6 ++++++
 target/riscv/csr.c      | 31 +++++++++++++++++++++++++++++++
 target/riscv/machine.c  | 19 +++++++++++++++++++
 target/riscv/pmp.c      |  5 +++++
 target/riscv/pmp.h      |  3 ++-
 7 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e3f0bd9242..55754cb374 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1018,6 +1018,9 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
     env->menvcfg = 0;
 #endif
 
+    /* on reset elp is clear */
+    env->elp = false;
+
     env->xl = riscv_cpu_mxl(env);
     riscv_cpu_update_mask(env);
     cs->exception_index = RISCV_EXCP_NONE;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 270a2a031c..b2dc419ad0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -222,6 +222,8 @@ struct CPUArchState {
 
     target_ulong jvt;
 
+    /* elp state for zicfilp extension */
+    bool      elp;
 #ifdef CONFIG_USER_ONLY
     uint32_t elf_flags;
 #endif
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index c257c5ed7d..b05ebe6f29 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -545,6 +545,8 @@
 #define MSTATUS_TVM         0x00100000 /* since: priv-1.10 */
 #define MSTATUS_TW          0x00200000 /* since: priv-1.10 */
 #define MSTATUS_TSR         0x00400000 /* since: priv-1.10 */
+#define MSTATUS_SPELP       0x00800000 /* zicfilp */
+#define MSTATUS_MPELP       0x020000000000 /* zicfilp */
 #define MSTATUS_GVA         0x4000000000ULL
 #define MSTATUS_MPV         0x8000000000ULL
 
@@ -575,6 +577,7 @@ typedef enum {
 #define SSTATUS_XS          0x00018000
 #define SSTATUS_SUM         0x00040000 /* since: priv-1.10 */
 #define SSTATUS_MXR         0x00080000
+#define SSTATUS_SPELP       MSTATUS_SPELP   /* zicfilp */
 
 #define SSTATUS64_UXL       0x0000000300000000ULL
 
@@ -747,6 +750,7 @@ typedef enum RISCVException {
 
 /* Execution environment configuration bits */
 #define MENVCFG_FIOM                       BIT(0)
+#define MENVCFG_LPE                        BIT(2) /* zicfilp */
 #define MENVCFG_CBIE                       (3UL << 4)
 #define MENVCFG_CBCFE                      BIT(6)
 #define MENVCFG_CBZE                       BIT(7)
@@ -760,11 +764,13 @@ typedef enum RISCVException {
 #define MENVCFGH_STCE                      BIT(31)
 
 #define SENVCFG_FIOM                       MENVCFG_FIOM
+#define SENVCFG_LPE                        MENVCFG_LPE
 #define SENVCFG_CBIE                       MENVCFG_CBIE
 #define SENVCFG_CBCFE                      MENVCFG_CBCFE
 #define SENVCFG_CBZE                       MENVCFG_CBZE
 
 #define HENVCFG_FIOM                       MENVCFG_FIOM
+#define HENVCFG_LPE                        MENVCFG_LPE
 #define HENVCFG_CBIE                       MENVCFG_CBIE
 #define HENVCFG_CBCFE                      MENVCFG_CBCFE
 #define HENVCFG_CBZE                       MENVCFG_CBZE
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 432c59dc66..5771a14848 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1400,6 +1400,11 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
         }
     }
 
+    /* If cfi lp extension is available, then apply cfi lp mask */
+    if (env_archcpu(env)->cfg.ext_zicfilp) {
+        mask |= (MSTATUS_MPELP | MSTATUS_SPELP);
+    }
+
     mstatus = (mstatus & ~mask) | (val & mask);
 
     env->mstatus = mstatus;
@@ -2101,6 +2106,10 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
         mask |= (cfg->ext_svpbmt ? MENVCFG_PBMTE : 0) |
                 (cfg->ext_sstc ? MENVCFG_STCE : 0) |
                 (cfg->ext_svadu ? MENVCFG_ADUE : 0);
+
+        if (env_archcpu(env)->cfg.ext_zicfilp) {
+            mask |= MENVCFG_LPE;
+        }
     }
     env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
 
@@ -2153,6 +2162,10 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
         return ret;
     }
 
+    if (env_archcpu(env)->cfg.ext_zicfilp) {
+        mask |= SENVCFG_LPE;
+    }
+
     env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
     return RISCV_EXCP_NONE;
 }
@@ -2190,6 +2203,10 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
 
     if (riscv_cpu_mxl(env) == MXL_RV64) {
         mask |= env->menvcfg & (HENVCFG_PBMTE | HENVCFG_STCE | HENVCFG_ADUE);
+
+        if (env_archcpu(env)->cfg.ext_zicfilp) {
+            mask |= HENVCFG_LPE;
+        }
     }
 
     env->henvcfg = (env->henvcfg & ~mask) | (val & mask);
@@ -2654,6 +2671,10 @@ static RISCVException read_sstatus_i128(CPURISCVState *env, int csrno,
         mask |= SSTATUS64_UXL;
     }
 
+    if (env_archcpu(env)->cfg.ext_zicfilp) {
+        mask |= SSTATUS_SPELP;
+    }
+
     *val = int128_make128(sstatus, add_status_sd(MXL_RV128, sstatus));
     return RISCV_EXCP_NONE;
 }
@@ -2665,6 +2686,11 @@ static RISCVException read_sstatus(CPURISCVState *env, int csrno,
     if (env->xl != MXL_RV32 || env->debugger) {
         mask |= SSTATUS64_UXL;
     }
+
+    if (env_archcpu(env)->cfg.ext_zicfilp) {
+        mask |= SSTATUS_SPELP;
+    }
+
     /* TODO: Use SXL not MXL. */
     *val = add_status_sd(riscv_cpu_mxl(env), env->mstatus & mask);
     return RISCV_EXCP_NONE;
@@ -2680,6 +2706,11 @@ static RISCVException write_sstatus(CPURISCVState *env, int csrno,
             mask |= SSTATUS64_UXL;
         }
     }
+
+    if (env_archcpu(env)->cfg.ext_zicfilp) {
+        mask |= SSTATUS_SPELP;
+    }
+
     target_ulong newval = (env->mstatus & ~mask) | (val & mask);
     return write_mstatus(env, CSR_MSTATUS, newval);
 }
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 76f2150f78..873957c4ab 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -351,6 +351,24 @@ static const VMStateDescription vmstate_jvt = {
     }
 };
 
+static bool elp_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+
+    return cpu->cfg.ext_zicfilp;
+}
+
+static const VMStateDescription vmstate_elp = {
+    .name = "cpu/elp",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = elp_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_BOOL(env.elp, RISCVCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 10,
@@ -423,6 +441,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_debug,
         &vmstate_smstateen,
         &vmstate_jvt,
+        &vmstate_elp,
         NULL
     }
 };
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 9eea397e72..1111d08d08 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -598,6 +598,11 @@ void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
         val &= ~(MSECCFG_MMWP | MSECCFG_MML | MSECCFG_RLB);
     }
 
+    /* M-mode forward cfi to be enabled if cfi extension is implemented */
+    if (env_archcpu(env)->cfg.ext_zicfilp) {
+        val |= (val & MSECCFG_MLPE);
+    }
+
     env->mseccfg = val;
 }
 
diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
index f5c10ce85c..e0530a17a3 100644
--- a/target/riscv/pmp.h
+++ b/target/riscv/pmp.h
@@ -44,7 +44,8 @@ typedef enum {
     MSECCFG_MMWP  = 1 << 1,
     MSECCFG_RLB   = 1 << 2,
     MSECCFG_USEED = 1 << 8,
-    MSECCFG_SSEED = 1 << 9
+    MSECCFG_SSEED = 1 << 9,
+    MSECCFG_MLPE =  1 << 10,
 } mseccfg_field_t;
 
 typedef struct {
-- 
2.44.0

[PATCH v11 04/20] target/riscv: save and restore elp state on priv transitions

[Deepak Gupta]

elp state is recorded in *status on trap entry (less privilege to higher
privilege) and restored in elp from *status on trap exit (higher to less
privilege).

Additionally this patch introduces a forward cfi helper function to
determine if current privilege has forward cfi is enabled or not based on
*envcfg (for U, VU, S, VU, HS) or mseccfg csr (for M). For qemu-user, a
new field `ufcfien` is introduced which is by default set to false and
helper function returns value deposited in `ufcfien` for qemu-user.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.h        |  1 +
 target/riscv/cpu_helper.c | 54 +++++++++++++++++++++++++++++++++++++++
 target/riscv/op_helper.c  | 18 +++++++++++++
 3 files changed, 73 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index b2dc419ad0..a7c970e70c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -531,6 +531,7 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong geilen);
 bool riscv_cpu_vector_enabled(CPURISCVState *env);
 void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
 int riscv_env_mmu_index(CPURISCVState *env, bool ifetch);
+bool cpu_get_fcfien(CPURISCVState *env);
 G_NORETURN void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                                MMUAccessType access_type,
                                                int mmu_idx, uintptr_t retaddr);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 6709622dd3..5f38969aa6 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -33,6 +33,7 @@
 #include "cpu_bits.h"
 #include "debug.h"
 #include "tcg/oversized-guest.h"
+#include "pmp.h"
 
 int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
 {
@@ -63,6 +64,33 @@ int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
 #endif
 }
 
+bool cpu_get_fcfien(CPURISCVState *env)
+{
+    /* no cfi extension, return false */
+    if (!env_archcpu(env)->cfg.ext_zicfilp) {
+        return false;
+    }
+
+    switch (env->priv) {
+    case PRV_U:
+        if (riscv_has_ext(env, RVS)) {
+            return env->senvcfg & SENVCFG_LPE;
+        }
+        return env->menvcfg & MENVCFG_LPE;
+#ifndef CONFIG_USER_ONLY
+    case PRV_S:
+        if (env->virt_enabled) {
+            return env->henvcfg & HENVCFG_LPE;
+        }
+        return env->menvcfg & MENVCFG_LPE;
+    case PRV_M:
+        return env->mseccfg & MSECCFG_MLPE;
+#endif
+    default:
+        g_assert_not_reached();
+    }
+}
+
 void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
                           uint64_t *cs_base, uint32_t *pflags)
 {
@@ -546,6 +574,15 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
     }
     bool current_virt = env->virt_enabled;
 
+    /*
+     * If zicfilp extension available and henvcfg.LPE = 1,
+     * then apply SPELP mask on mstatus
+     */
+    if (env_archcpu(env)->cfg.ext_zicfilp &&
+        get_field(env->henvcfg, HENVCFG_LPE)) {
+        mstatus_mask |= SSTATUS_SPELP;
+    }
+
     g_assert(riscv_has_ext(env, RVH));
 
     if (current_virt) {
@@ -1754,6 +1791,11 @@ void riscv_cpu_do_interrupt(CPUState *cs)
     if (env->priv <= PRV_S && cause < 64 &&
         (((deleg >> cause) & 1) || s_injected || vs_injected)) {
         /* handle the trap in S-mode */
+        /* save elp status */
+        if (cpu_get_fcfien(env)) {
+            env->mstatus = set_field(env->mstatus, MSTATUS_SPELP, env->elp);
+        }
+
         if (riscv_has_ext(env, RVH)) {
             uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
 
@@ -1802,6 +1844,11 @@ void riscv_cpu_do_interrupt(CPUState *cs)
         riscv_cpu_set_mode(env, PRV_S);
     } else {
         /* handle the trap in M-mode */
+        /* save elp status */
+        if (cpu_get_fcfien(env)) {
+            env->mstatus = set_field(env->mstatus, MSTATUS_MPELP, env->elp);
+        }
+
         if (riscv_has_ext(env, RVH)) {
             if (env->virt_enabled) {
                 riscv_cpu_swap_hypervisor_regs(env);
@@ -1833,6 +1880,13 @@ void riscv_cpu_do_interrupt(CPUState *cs)
         riscv_cpu_set_mode(env, PRV_M);
     }
 
+    /*
+     * Interrupt/exception/trap delivery is asynchronous event and as per
+     * zicfilp spec CPU should clear up the ELP state. No harm in clearing
+     * unconditionally.
+     */
+    env->elp = false;
+
     /*
      * NOTE: it is not necessary to yield load reservations here. It is only
      * necessary for an SC from "another hart" to cause a load reservation
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 2baf5bc3ca..5848aaf437 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -313,6 +313,15 @@ target_ulong helper_sret(CPURISCVState *env)
 
     riscv_cpu_set_mode(env, prev_priv);
 
+    /*
+     * If forward cfi enabled for new priv, restore elp status
+     * and clear spelp in mstatus
+     */
+    if (cpu_get_fcfien(env)) {
+        env->elp = get_field(env->mstatus, MSTATUS_SPELP);
+    }
+    env->mstatus = set_field(env->mstatus, MSTATUS_SPELP, 0);
+
     return retpc;
 }
 
@@ -357,6 +366,15 @@ target_ulong helper_mret(CPURISCVState *env)
         riscv_cpu_set_virt_enabled(env, prev_virt);
     }
 
+    /*
+     * If forward cfi enabled for new priv, restore elp status
+     * and clear mpelp in mstatus
+     */
+    if (cpu_get_fcfien(env)) {
+        env->elp = get_field(env->mstatus, MSTATUS_MPELP);
+    }
+    env->mstatus = set_field(env->mstatus, MSTATUS_MPELP, 0);
+
     return retpc;
 }
 
-- 
2.44.0

[PATCH v11 05/20] target/riscv: additional code information for sw check

[Deepak Gupta]

sw check exception support was recently added. This patch further augments
sw check exception by providing support for additional code which is
provided in *tval. Adds `sw_check_code` field in cpuarchstate. Whenever
sw check exception is raised *tval gets the value deposited in
`sw_check_code`.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h        | 2 ++
 target/riscv/cpu_helper.c | 3 +++
 target/riscv/csr.c        | 1 +
 3 files changed, 6 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a7c970e70c..a0f14c759e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -224,6 +224,8 @@ struct CPUArchState {
 
     /* elp state for zicfilp extension */
     bool      elp;
+    /* sw check code for sw check exception */
+    target_ulong sw_check_code;
 #ifdef CONFIG_USER_ONLY
     uint32_t elf_flags;
 #endif
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 5f38969aa6..fffd865cb4 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1760,6 +1760,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
                 cs->watchpoint_hit = NULL;
             }
             break;
+        case RISCV_EXCP_SW_CHECK:
+            tval = env->sw_check_code;
+            break;
         default:
             break;
         }
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5771a14848..a5a969a377 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1179,6 +1179,7 @@ static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
                          (1ULL << (RISCV_EXCP_INST_PAGE_FAULT)) | \
                          (1ULL << (RISCV_EXCP_LOAD_PAGE_FAULT)) | \
                          (1ULL << (RISCV_EXCP_STORE_PAGE_FAULT)) | \
+                         (1ULL << (RISCV_EXCP_SW_CHECK)) | \
                          (1ULL << (RISCV_EXCP_INST_GUEST_PAGE_FAULT)) | \
                          (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) | \
                          (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) | \
-- 
2.44.0

[PATCH v11 06/20] target/riscv: tracking indirect branches (fcfi) for zicfilp

[Deepak Gupta]

zicfilp protects forward control flow (if enabled) by enforcing all
indirect call and jmp must land on a landing pad instruction `lpad`. If
target of an indirect call or jmp is not `lpad` then cpu/hart must raise
a sw check exception with tval = 2.

This patch implements the mechanism using TCG. Target architecture branch
instruction must define the end of a TB. Using this property, during
translation of branch instruction, TB flag = FCFI_LP_EXPECTED can be set.
Translation of target TB can check if FCFI_LP_EXPECTED flag is set and a
flag (fcfi_lp_expected) can be set in DisasContext. If `lpad` gets
translated, fcfi_lp_expected flag in DisasContext can be cleared. Else
it'll fault.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h        |  3 +++
 target/riscv/cpu_bits.h   |  3 +++
 target/riscv/cpu_helper.c | 10 ++++++++++
 target/riscv/translate.c  | 23 +++++++++++++++++++++++
 4 files changed, 39 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a0f14c759e..f372a4074b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -606,6 +606,9 @@ FIELD(TB_FLAGS, ITRIGGER, 22, 1)
 FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
 FIELD(TB_FLAGS, PRIV, 24, 2)
 FIELD(TB_FLAGS, AXL, 26, 2)
+/* zicfilp needs a TB flag to track indirect branches */
+FIELD(TB_FLAGS, FCFI_ENABLED, 28, 1)
+FIELD(TB_FLAGS, FCFI_LP_EXPECTED, 29, 1)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index b05ebe6f29..900769ce60 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -685,6 +685,9 @@ typedef enum RISCVException {
     RISCV_EXCP_SEMIHOST = 0x3f,
 } RISCVException;
 
+/* zicfilp defines lp violation results in sw check with tval = 2*/
+#define RISCV_EXCP_SW_CHECK_FCFI_TVAL      2
+
 #define RISCV_EXCP_INT_FLAG                0x80000000
 #define RISCV_EXCP_INT_MASK                0x7fffffff
 
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index fffd865cb4..c3820eff8f 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -132,6 +132,16 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
         flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
     }
 
+    if (cpu_get_fcfien(env)) {
+        /*
+         * For Forward CFI, only the expectation of a lpad at
+         * the start of the block is tracked via env->elp. env->elp
+         * is turned on during jalr translation.
+         */
+        flags = FIELD_DP32(flags, TB_FLAGS, FCFI_LP_EXPECTED, env->elp);
+        flags = FIELD_DP32(flags, TB_FLAGS, FCFI_ENABLED, 1);
+    }
+
 #ifdef CONFIG_USER_ONLY
     fs = EXT_STATUS_DIRTY;
     vs = EXT_STATUS_DIRTY;
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index acba90f170..b5c0511b4b 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -116,6 +116,9 @@ typedef struct DisasContext {
     bool frm_valid;
     bool insn_start_updated;
     const GPtrArray *decoders;
+    /* zicfilp extension. fcfi_enabled, lp expected or not */
+    bool fcfi_enabled;
+    bool fcfi_lp_expected;
 } DisasContext;
 
 static inline bool has_ext(DisasContext *ctx, uint32_t ext)
@@ -1238,6 +1241,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED);
     ctx->ztso = cpu->cfg.ext_ztso;
     ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
+    ctx->fcfi_lp_expected = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_LP_EXPECTED);
+    ctx->fcfi_enabled = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_ENABLED);
     ctx->zero = tcg_constant_tl(0);
     ctx->virt_inst_excp = false;
     ctx->decoders = cpu->decoders;
@@ -1270,6 +1275,24 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
     decode_opc(env, ctx, opcode16);
     ctx->base.pc_next += ctx->cur_insn_len;
 
+    /*
+     * If 'fcfi_lp_expected' is still true after processing the instruction,
+     * then we did not see an 'lpad' instruction, and must raise an exception.
+     * Insert code to raise the exception at the start of the insn; any other
+     * code the insn may have emitted will be deleted as dead code following
+     * the noreturn exception
+     */
+    if (ctx->fcfi_lp_expected) {
+        /* Emit after insn_start, i.e. before the op following insn_start. */
+        tcg_ctx->emit_before_op = QTAILQ_NEXT(ctx->base.insn_start, link);
+        tcg_gen_st_tl(tcg_constant_tl(RISCV_EXCP_SW_CHECK_FCFI_TVAL),
+                      tcg_env, offsetof(CPURISCVState, sw_check_code));
+        gen_helper_raise_exception(tcg_env,
+                      tcg_constant_i32(RISCV_EXCP_SW_CHECK));
+        tcg_ctx->emit_before_op = NULL;
+        ctx->base.is_jmp = DISAS_NORETURN;
+    }
+
     /* Only the first insn within a TB is allowed to cross a page boundary. */
     if (ctx->base.is_jmp == DISAS_NEXT) {
         if (ctx->itrigger || !is_same_page(&ctx->base, ctx->base.pc_next)) {
-- 
2.44.0

[PATCH v11 07/20] target/riscv: zicfilp `lpad` impl and branch tracking

[Deepak Gupta]

Implements setting lp expected when `jalr` is encountered and implements
`lpad` instruction of zicfilp. `lpad` instruction is taken out of
auipc x0, <imm_20>. This is an existing HINTNOP space. If `lpad` is
target of an indirect branch, cpu checks for 20 bit value in x7 upper
with 20 bit value embedded in `lpad`. If they don't match, cpu raises a
sw check exception with tval = 2.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu_user.h                 |  1 +
 target/riscv/insn32.decode              |  5 ++-
 target/riscv/insn_trans/trans_rvi.c.inc | 55 +++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu_user.h b/target/riscv/cpu_user.h
index 02afad608b..e6927ff847 100644
--- a/target/riscv/cpu_user.h
+++ b/target/riscv/cpu_user.h
@@ -15,5 +15,6 @@
 #define xA6 16
 #define xA7 17  /* syscall number for RVI ABI */
 #define xT0 5   /* syscall number for RVE ABI */
+#define xT2 7
 
 #endif
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index c45b8fa1d8..27108b992b 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -123,7 +123,10 @@ sfence_vm   0001000    00100 ..... 000 00000 1110011 @sfence_vm
 
 # *** RV32I Base Instruction Set ***
 lui      ....................       ..... 0110111 @u
-auipc    ....................       ..... 0010111 @u
+{
+  lpad   label:20                   00000 0010111
+  auipc  ....................       ..... 0010111 @u
+}
 jal      ....................       ..... 1101111 @j
 jalr     ............     ..... 000 ..... 1100111 @i
 beq      ....... .....    ..... 000 ..... 1100011 @b
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index 98e3806d5e..b427f3a939 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -36,6 +36,49 @@ static bool trans_lui(DisasContext *ctx, arg_lui *a)
     return true;
 }
 
+static bool trans_lpad(DisasContext *ctx, arg_lpad *a)
+{
+    /*
+     * fcfi_lp_expected can set only if fcfi was eanbled.
+     * translate further only if fcfi_lp_expected set.
+     * lpad comes from NOP space anyways, so return true if
+     * fcfi_lp_expected is false.
+     */
+    if (!ctx->fcfi_lp_expected) {
+        return true;
+    }
+
+    ctx->fcfi_lp_expected = false;
+    if ((ctx->base.pc_next) & 0x3) {
+        /*
+         * misaligned, according to spec we should raise sw check exception
+         */
+        tcg_gen_st_tl(tcg_constant_tl(RISCV_EXCP_SW_CHECK_FCFI_TVAL),
+                      tcg_env, offsetof(CPURISCVState, sw_check_code));
+        gen_helper_raise_exception(tcg_env,
+                      tcg_constant_i32(RISCV_EXCP_SW_CHECK));
+        return true;
+    }
+
+    /* per spec, label check performed only when embedded label non-zero */
+    if (a->label != 0) {
+        TCGLabel *skip = gen_new_label();
+        TCGv tmp = tcg_temp_new();
+        tcg_gen_extract_tl(tmp, get_gpr(ctx, xT2, EXT_NONE), 12, 20);
+        tcg_gen_brcondi_tl(TCG_COND_EQ, tmp, a->label, skip);
+        tcg_gen_st_tl(tcg_constant_tl(RISCV_EXCP_SW_CHECK_FCFI_TVAL),
+                      tcg_env, offsetof(CPURISCVState, sw_check_code));
+        gen_helper_raise_exception(tcg_env,
+                      tcg_constant_i32(RISCV_EXCP_SW_CHECK));
+        gen_set_label(skip);
+    }
+
+    tcg_gen_st8_tl(tcg_constant_tl(0), tcg_env,
+                  offsetof(CPURISCVState, elp));
+
+    return true;
+}
+
 static bool trans_auipc(DisasContext *ctx, arg_auipc *a)
 {
     TCGv target_pc = dest_gpr(ctx, a->rd);
@@ -75,6 +118,18 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
     gen_set_gpr(ctx, a->rd, succ_pc);
 
     tcg_gen_mov_tl(cpu_pc, target_pc);
+    if (ctx->fcfi_enabled) {
+        /*
+         * return from functions (i.e. rs1 == xRA || rs1 == xT0) are not
+         * tracked. zicfilp introduces sw guarded branch as well. sw guarded
+         * branch are not tracked. rs1 == xT2 is a sw guarded branch.
+         */
+        if (a->rs1 != xRA && a->rs1 != xT0 && a->rs1 != xT2) {
+            tcg_gen_st8_tl(tcg_constant_tl(1),
+                          tcg_env, offsetof(CPURISCVState, elp));
+        }
+    }
+
     lookup_and_goto_ptr(ctx);
 
     if (misaligned) {
-- 
2.44.0

[PATCH v11 08/20] disas/riscv: enable `lpad` disassembly

[Deepak Gupta]

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 disas/riscv.c | 18 +++++++++++++++++-
 disas/riscv.h |  2 ++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index c8364c2b07..c7c92acef7 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -974,6 +974,7 @@ typedef enum {
     rv_op_amomaxu_h = 943,
     rv_op_amocas_b  = 944,
     rv_op_amocas_h  = 945,
+    rv_op_lpad = 946,
 } rv_op;
 
 /* register names */
@@ -2232,6 +2233,7 @@ const rv_opcode_data rvi_opcode_data[] = {
     { "amomaxu.h", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
     { "amocas.b", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
     { "amocas.h", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
+    { "lpad", rv_codec_lp, rv_fmt_imm, NULL, 0, 0, 0 },
 };
 
 /* CSR names */
@@ -2925,7 +2927,13 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
             case 7: op = rv_op_andi; break;
             }
             break;
-        case 5: op = rv_op_auipc; break;
+        case 5:
+            op = rv_op_auipc;
+            if (dec->cfg->ext_zicfilp &&
+                (((inst >> 7) & 0b11111) == 0b00000)) {
+                op = rv_op_lpad;
+            }
+            break;
         case 6:
             switch ((inst >> 12) & 0b111) {
             case 0: op = rv_op_addiw; break;
@@ -4482,6 +4490,11 @@ static uint32_t operand_tbl_index(rv_inst inst)
     return ((inst << 54) >> 56);
 }
 
+static uint32_t operand_lpl(rv_inst inst)
+{
+    return inst >> 12;
+}
+
 /* decode operands */
 
 static void decode_inst_operands(rv_decode *dec, rv_isa isa)
@@ -4869,6 +4882,9 @@ static void decode_inst_operands(rv_decode *dec, rv_isa isa)
         dec->imm = sextract32(operand_rs2(inst), 0, 5);
         dec->imm1 = operand_imm2(inst);
         break;
+    case rv_codec_lp:
+        dec->imm = operand_lpl(inst);
+        break;
     };
 }
 
diff --git a/disas/riscv.h b/disas/riscv.h
index 16a08e4895..1182457aff 100644
--- a/disas/riscv.h
+++ b/disas/riscv.h
@@ -166,6 +166,7 @@ typedef enum {
     rv_codec_r2_immhl,
     rv_codec_r2_imm2_imm5,
     rv_codec_fli,
+    rv_codec_lp,
 } rv_codec;
 
 /* structures */
@@ -228,6 +229,7 @@ enum {
 #define rv_fmt_rs1_rs2                "O\t1,2"
 #define rv_fmt_rd_imm                 "O\t0,i"
 #define rv_fmt_rd_uimm                "O\t0,Ui"
+#define rv_fmt_imm                    "O\ti"
 #define rv_fmt_rd_offset              "O\t0,o"
 #define rv_fmt_rd_uoffset             "O\t0,Uo"
 #define rv_fmt_rd_rs1_rs2             "O\t0,1,2"
-- 
2.44.0

[PATCH v11 09/20] target/riscv: Expose zicfilp extension as a cpu property

[Deepak Gupta]

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 55754cb374..c9aeffee4e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1481,6 +1481,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
     /* Defaults for standard extensions */
     MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
     MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
+    MULTI_EXT_CFG_BOOL("zicfilp", ext_zicfilp, false),
     MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
     MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
     MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
-- 
2.44.0

[PATCH v11 10/20] target/riscv: Add zicfiss extension

[Deepak Gupta]

zicfiss [1] riscv cpu extension enables backward control flow integrity.

This patch sets up space for zicfiss extension in cpuconfig. And imple-
ments dependency on A, zicsr, zimop and zcmop extensions.

[1] - https://github.com/riscv/riscv-cfi

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c         |  1 +
 target/riscv/cpu_cfg.h     |  1 +
 target/riscv/tcg/tcg-cpu.c | 19 +++++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c9aeffee4e..29b4bdb40a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -107,6 +107,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, has_priv_1_11),
     ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, has_priv_1_11),
     ISA_EXT_DATA_ENTRY(zicfilp, PRIV_VERSION_1_12_0, ext_zicfilp),
+    ISA_EXT_DATA_ENTRY(zicfiss, PRIV_VERSION_1_13_0, ext_zicfiss),
     ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
     ISA_EXT_DATA_ENTRY(zicntr, PRIV_VERSION_1_12_0, ext_zicntr),
     ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_zicsr),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 88d5defbb5..2499f38407 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -68,6 +68,7 @@ struct RISCVCPUConfig {
     bool ext_zicbop;
     bool ext_zicboz;
     bool ext_zicfilp;
+    bool ext_zicfiss;
     bool ext_zicond;
     bool ext_zihintntl;
     bool ext_zihintpause;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index ed19586c9d..4da26cb926 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -618,6 +618,25 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
         cpu->cfg.ext_zihpm = false;
     }
 
+    if (cpu->cfg.ext_zicfiss) {
+        if (!cpu->cfg.ext_zicsr) {
+            error_setg(errp, "zicfiss extension requires zicsr extension");
+            return;
+        }
+        if (!riscv_has_ext(env, RVA)) {
+            error_setg(errp, "zicfiss extension requires A extension");
+            return;
+        }
+        if (!cpu->cfg.ext_zimop) {
+            error_setg(errp, "zicfiss extension requires zimop extension");
+            return;
+        }
+        if (cpu->cfg.ext_zca && !cpu->cfg.ext_zcmop) {
+            error_setg(errp, "zicfiss with zca requires zcmop extension");
+            return;
+        }
+    }
+
     if (!cpu->cfg.ext_zihpm) {
         cpu->cfg.pmu_mask = 0;
         cpu->pmu_avail_ctrs = 0;
-- 
2.44.0

[PATCH v11 11/20] target/riscv: introduce ssp and enabling controls for zicfiss

[Deepak Gupta]

zicfiss introduces a new state ssp ("shadow stack register") in cpu.
ssp is expressed as a new unprivileged csr (CSR_SSP=0x11) and holds
virtual address for shadow stack as programmed by software.

Shadow stack (for each mode) is enabled via bit3 in *envcfg CSRs.
Shadow stack can be enabled for a mode only if it's higher privileged
mode had it enabled for itself. M mode doesn't need enabling control,
it's always available if extension is available on cpu.

This patch also implements helper bcfi function which determines if bcfi
is enabled at current privilege or not. qemu-user also gets field
`ubcfien` indicating whether qemu user has shadow stack enabled or not.

Adds ssp to migration state as well.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.c        |  2 ++
 target/riscv/cpu.h        |  3 +++
 target/riscv/cpu_bits.h   |  6 +++++
 target/riscv/cpu_helper.c | 27 ++++++++++++++++++++
 target/riscv/csr.c        | 52 +++++++++++++++++++++++++++++++++++++++
 target/riscv/machine.c    | 19 ++++++++++++++
 6 files changed, 109 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 29b4bdb40a..c5ebcefeb5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1021,6 +1021,8 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
 
     /* on reset elp is clear */
     env->elp = false;
+    /* on reset ssp is set to 0 */
+    env->ssp = 0;
 
     env->xl = riscv_cpu_mxl(env);
     riscv_cpu_update_mask(env);
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f372a4074b..4ace54a2eb 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -224,6 +224,8 @@ struct CPUArchState {
 
     /* elp state for zicfilp extension */
     bool      elp;
+    /* shadow stack register for zicfiss extension */
+    target_ulong ssp;
     /* sw check code for sw check exception */
     target_ulong sw_check_code;
 #ifdef CONFIG_USER_ONLY
@@ -534,6 +536,7 @@ bool riscv_cpu_vector_enabled(CPURISCVState *env);
 void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
 int riscv_env_mmu_index(CPURISCVState *env, bool ifetch);
 bool cpu_get_fcfien(CPURISCVState *env);
+bool cpu_get_bcfien(CPURISCVState *env);
 G_NORETURN void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                                MMUAccessType access_type,
                                                int mmu_idx, uintptr_t retaddr);
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 900769ce60..48ce24dc32 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -34,6 +34,9 @@
 
 /* Control and Status Registers */
 
+/* zicfiss user ssp csr */
+#define CSR_SSP             0x011
+
 /* User Trap Setup */
 #define CSR_USTATUS         0x000
 #define CSR_UIE             0x004
@@ -754,6 +757,7 @@ typedef enum RISCVException {
 /* Execution environment configuration bits */
 #define MENVCFG_FIOM                       BIT(0)
 #define MENVCFG_LPE                        BIT(2) /* zicfilp */
+#define MENVCFG_SSE                        BIT(3) /* zicfiss */
 #define MENVCFG_CBIE                       (3UL << 4)
 #define MENVCFG_CBCFE                      BIT(6)
 #define MENVCFG_CBZE                       BIT(7)
@@ -768,12 +772,14 @@ typedef enum RISCVException {
 
 #define SENVCFG_FIOM                       MENVCFG_FIOM
 #define SENVCFG_LPE                        MENVCFG_LPE
+#define SENVCFG_SSE                        MENVCFG_SSE
 #define SENVCFG_CBIE                       MENVCFG_CBIE
 #define SENVCFG_CBCFE                      MENVCFG_CBCFE
 #define SENVCFG_CBZE                       MENVCFG_CBZE
 
 #define HENVCFG_FIOM                       MENVCFG_FIOM
 #define HENVCFG_LPE                        MENVCFG_LPE
+#define HENVCFG_SSE                        MENVCFG_SSE
 #define HENVCFG_CBIE                       MENVCFG_CBIE
 #define HENVCFG_CBCFE                      MENVCFG_CBCFE
 #define HENVCFG_CBZE                       MENVCFG_CBZE
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index c3820eff8f..f7e97eabfa 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -91,6 +91,33 @@ bool cpu_get_fcfien(CPURISCVState *env)
     }
 }
 
+bool cpu_get_bcfien(CPURISCVState *env)
+{
+    /* no cfi extension, return false */
+    if (!env_archcpu(env)->cfg.ext_zicfiss) {
+        return false;
+    }
+
+    switch (env->priv) {
+    case PRV_U:
+        if (riscv_has_ext(env, RVS)) {
+            return env->senvcfg & SENVCFG_SSE;
+        }
+        return env->menvcfg & MENVCFG_SSE;
+#ifndef CONFIG_USER_ONLY
+    case PRV_S:
+        if (env->virt_enabled) {
+            return env->henvcfg & HENVCFG_SSE;
+        }
+        return env->menvcfg & MENVCFG_SSE;
+    case PRV_M: /* M-mode shadow stack is always on if hart implements */
+        return true;
+#endif
+    default:
+        g_assert_not_reached();
+    }
+}
+
 void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
                           uint64_t *cs_base, uint32_t *pflags)
 {
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a5a969a377..ec04b2b32b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -185,6 +185,25 @@ static RISCVException zcmt(CPURISCVState *env, int csrno)
     return RISCV_EXCP_NONE;
 }
 
+static RISCVException cfi_ss(CPURISCVState *env, int csrno)
+{
+    if (!env_archcpu(env)->cfg.ext_zicfiss) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    /* if bcfi not active for current env, access to csr is illegal */
+    if (!cpu_get_bcfien(env)) {
+#if !defined(CONFIG_USER_ONLY)
+        if (env->debugger) {
+            return RISCV_EXCP_NONE;
+        }
+#endif
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return RISCV_EXCP_NONE;
+}
+
 #if !defined(CONFIG_USER_ONLY)
 static RISCVException mctr(CPURISCVState *env, int csrno)
 {
@@ -596,6 +615,19 @@ static RISCVException seed(CPURISCVState *env, int csrno)
 #endif
 }
 
+/* zicfiss CSR_SSP read and write */
+static int read_ssp(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->ssp;
+    return RISCV_EXCP_NONE;
+}
+
+static int write_ssp(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->ssp = val;
+    return RISCV_EXCP_NONE;
+}
+
 /* User Floating-Point CSRs */
 static RISCVException read_fflags(CPURISCVState *env, int csrno,
                                   target_ulong *val)
@@ -2111,6 +2143,10 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
         if (env_archcpu(env)->cfg.ext_zicfilp) {
             mask |= MENVCFG_LPE;
         }
+
+        if (env_archcpu(env)->cfg.ext_zicfiss) {
+            mask |= MENVCFG_SSE;
+        }
     }
     env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
 
@@ -2167,6 +2203,13 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
         mask |= SENVCFG_LPE;
     }
 
+    /* Higher mode SSE must be ON for next-less mode SSE to be ON */
+    if (env_archcpu(env)->cfg.ext_zicfiss &&
+        get_field(env->menvcfg, MENVCFG_SSE) &&
+        (env->virt_enabled ? get_field(env->henvcfg, HENVCFG_SSE) : true)) {
+        mask |= SENVCFG_SSE;
+    }
+
     env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
     return RISCV_EXCP_NONE;
 }
@@ -2208,6 +2251,12 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
         if (env_archcpu(env)->cfg.ext_zicfilp) {
             mask |= HENVCFG_LPE;
         }
+
+        /* H can light up SSE for VS only if HS had it from menvcfg */
+        if (env_archcpu(env)->cfg.ext_zicfiss &&
+            get_field(env->menvcfg, MENVCFG_SSE)) {
+            mask |= HENVCFG_SSE;
+        }
     }
 
     env->henvcfg = (env->henvcfg & ~mask) | (val & mask);
@@ -4663,6 +4712,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     /* Zcmt Extension */
     [CSR_JVT] = {"jvt", zcmt, read_jvt, write_jvt},
 
+    /* zicfiss Extension, shadow stack register */
+    [CSR_SSP]  = { "ssp", cfi_ss, read_ssp, write_ssp },
+
 #if !defined(CONFIG_USER_ONLY)
     /* Machine Timers and Counters */
     [CSR_MCYCLE]    = { "mcycle",    any,   read_hpmcounter,
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 873957c4ab..84d5ecf436 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -369,6 +369,24 @@ static const VMStateDescription vmstate_elp = {
     }
 };
 
+static bool ssp_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+
+    return cpu->cfg.ext_zicfiss;
+}
+
+static const VMStateDescription vmstate_ssp = {
+    .name = "cpu/ssp",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = ssp_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINTTL(env.ssp, RISCVCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 10,
@@ -442,6 +460,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_smstateen,
         &vmstate_jvt,
         &vmstate_elp,
+        &vmstate_ssp,
         NULL
     }
 };
-- 
2.44.0

Re: [PATCH v11 11/20] target/riscv: introduce ssp and enabling controls for zicfiss

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:54 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> zicfiss introduces a new state ssp ("shadow stack register") in cpu.
> ssp is expressed as a new unprivileged csr (CSR_SSP=0x11) and holds
> virtual address for shadow stack as programmed by software.
>
> Shadow stack (for each mode) is enabled via bit3 in *envcfg CSRs.
> Shadow stack can be enabled for a mode only if it's higher privileged
> mode had it enabled for itself. M mode doesn't need enabling control,
> it's always available if extension is available on cpu.
>
> This patch also implements helper bcfi function which determines if bcfi
> is enabled at current privilege or not. qemu-user also gets field
> `ubcfien` indicating whether qemu user has shadow stack enabled or not.
>
> Adds ssp to migration state as well.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Co-developed-by: Jim Shu <jim.shu@sifive.com>
> Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

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

Alistair

> ---
>  target/riscv/cpu.c        |  2 ++
>  target/riscv/cpu.h        |  3 +++
>  target/riscv/cpu_bits.h   |  6 +++++
>  target/riscv/cpu_helper.c | 27 ++++++++++++++++++++
>  target/riscv/csr.c        | 52 +++++++++++++++++++++++++++++++++++++++
>  target/riscv/machine.c    | 19 ++++++++++++++
>  6 files changed, 109 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 29b4bdb40a..c5ebcefeb5 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1021,6 +1021,8 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
>
>      /* on reset elp is clear */
>      env->elp = false;
> +    /* on reset ssp is set to 0 */
> +    env->ssp = 0;
>
>      env->xl = riscv_cpu_mxl(env);
>      riscv_cpu_update_mask(env);
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index f372a4074b..4ace54a2eb 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -224,6 +224,8 @@ struct CPUArchState {
>
>      /* elp state for zicfilp extension */
>      bool      elp;
> +    /* shadow stack register for zicfiss extension */
> +    target_ulong ssp;
>      /* sw check code for sw check exception */
>      target_ulong sw_check_code;
>  #ifdef CONFIG_USER_ONLY
> @@ -534,6 +536,7 @@ bool riscv_cpu_vector_enabled(CPURISCVState *env);
>  void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
>  int riscv_env_mmu_index(CPURISCVState *env, bool ifetch);
>  bool cpu_get_fcfien(CPURISCVState *env);
> +bool cpu_get_bcfien(CPURISCVState *env);
>  G_NORETURN void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>                                                 MMUAccessType access_type,
>                                                 int mmu_idx, uintptr_t retaddr);
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 900769ce60..48ce24dc32 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -34,6 +34,9 @@
>
>  /* Control and Status Registers */
>
> +/* zicfiss user ssp csr */
> +#define CSR_SSP             0x011
> +
>  /* User Trap Setup */
>  #define CSR_USTATUS         0x000
>  #define CSR_UIE             0x004
> @@ -754,6 +757,7 @@ typedef enum RISCVException {
>  /* Execution environment configuration bits */
>  #define MENVCFG_FIOM                       BIT(0)
>  #define MENVCFG_LPE                        BIT(2) /* zicfilp */
> +#define MENVCFG_SSE                        BIT(3) /* zicfiss */
>  #define MENVCFG_CBIE                       (3UL << 4)
>  #define MENVCFG_CBCFE                      BIT(6)
>  #define MENVCFG_CBZE                       BIT(7)
> @@ -768,12 +772,14 @@ typedef enum RISCVException {
>
>  #define SENVCFG_FIOM                       MENVCFG_FIOM
>  #define SENVCFG_LPE                        MENVCFG_LPE
> +#define SENVCFG_SSE                        MENVCFG_SSE
>  #define SENVCFG_CBIE                       MENVCFG_CBIE
>  #define SENVCFG_CBCFE                      MENVCFG_CBCFE
>  #define SENVCFG_CBZE                       MENVCFG_CBZE
>
>  #define HENVCFG_FIOM                       MENVCFG_FIOM
>  #define HENVCFG_LPE                        MENVCFG_LPE
> +#define HENVCFG_SSE                        MENVCFG_SSE
>  #define HENVCFG_CBIE                       MENVCFG_CBIE
>  #define HENVCFG_CBCFE                      MENVCFG_CBCFE
>  #define HENVCFG_CBZE                       MENVCFG_CBZE
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index c3820eff8f..f7e97eabfa 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -91,6 +91,33 @@ bool cpu_get_fcfien(CPURISCVState *env)
>      }
>  }
>
> +bool cpu_get_bcfien(CPURISCVState *env)
> +{
> +    /* no cfi extension, return false */
> +    if (!env_archcpu(env)->cfg.ext_zicfiss) {
> +        return false;
> +    }
> +
> +    switch (env->priv) {
> +    case PRV_U:
> +        if (riscv_has_ext(env, RVS)) {
> +            return env->senvcfg & SENVCFG_SSE;
> +        }
> +        return env->menvcfg & MENVCFG_SSE;
> +#ifndef CONFIG_USER_ONLY
> +    case PRV_S:
> +        if (env->virt_enabled) {
> +            return env->henvcfg & HENVCFG_SSE;
> +        }
> +        return env->menvcfg & MENVCFG_SSE;
> +    case PRV_M: /* M-mode shadow stack is always on if hart implements */
> +        return true;
> +#endif
> +    default:
> +        g_assert_not_reached();
> +    }
> +}
> +
>  void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>                            uint64_t *cs_base, uint32_t *pflags)
>  {
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a5a969a377..ec04b2b32b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -185,6 +185,25 @@ static RISCVException zcmt(CPURISCVState *env, int csrno)
>      return RISCV_EXCP_NONE;
>  }
>
> +static RISCVException cfi_ss(CPURISCVState *env, int csrno)
> +{
> +    if (!env_archcpu(env)->cfg.ext_zicfiss) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    /* if bcfi not active for current env, access to csr is illegal */
> +    if (!cpu_get_bcfien(env)) {
> +#if !defined(CONFIG_USER_ONLY)
> +        if (env->debugger) {
> +            return RISCV_EXCP_NONE;
> +        }
> +#endif
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    return RISCV_EXCP_NONE;
> +}
> +
>  #if !defined(CONFIG_USER_ONLY)
>  static RISCVException mctr(CPURISCVState *env, int csrno)
>  {
> @@ -596,6 +615,19 @@ static RISCVException seed(CPURISCVState *env, int csrno)
>  #endif
>  }
>
> +/* zicfiss CSR_SSP read and write */
> +static int read_ssp(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->ssp;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static int write_ssp(CPURISCVState *env, int csrno, target_ulong val)
> +{
> +    env->ssp = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
>  /* User Floating-Point CSRs */
>  static RISCVException read_fflags(CPURISCVState *env, int csrno,
>                                    target_ulong *val)
> @@ -2111,6 +2143,10 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>          if (env_archcpu(env)->cfg.ext_zicfilp) {
>              mask |= MENVCFG_LPE;
>          }
> +
> +        if (env_archcpu(env)->cfg.ext_zicfiss) {
> +            mask |= MENVCFG_SSE;
> +        }
>      }
>      env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
>
> @@ -2167,6 +2203,13 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>          mask |= SENVCFG_LPE;
>      }
>
> +    /* Higher mode SSE must be ON for next-less mode SSE to be ON */
> +    if (env_archcpu(env)->cfg.ext_zicfiss &&
> +        get_field(env->menvcfg, MENVCFG_SSE) &&
> +        (env->virt_enabled ? get_field(env->henvcfg, HENVCFG_SSE) : true)) {
> +        mask |= SENVCFG_SSE;
> +    }
> +
>      env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
>      return RISCV_EXCP_NONE;
>  }
> @@ -2208,6 +2251,12 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
>          if (env_archcpu(env)->cfg.ext_zicfilp) {
>              mask |= HENVCFG_LPE;
>          }
> +
> +        /* H can light up SSE for VS only if HS had it from menvcfg */
> +        if (env_archcpu(env)->cfg.ext_zicfiss &&
> +            get_field(env->menvcfg, MENVCFG_SSE)) {
> +            mask |= HENVCFG_SSE;
> +        }
>      }
>
>      env->henvcfg = (env->henvcfg & ~mask) | (val & mask);
> @@ -4663,6 +4712,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      /* Zcmt Extension */
>      [CSR_JVT] = {"jvt", zcmt, read_jvt, write_jvt},
>
> +    /* zicfiss Extension, shadow stack register */
> +    [CSR_SSP]  = { "ssp", cfi_ss, read_ssp, write_ssp },
> +
>  #if !defined(CONFIG_USER_ONLY)
>      /* Machine Timers and Counters */
>      [CSR_MCYCLE]    = { "mcycle",    any,   read_hpmcounter,
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 873957c4ab..84d5ecf436 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -369,6 +369,24 @@ static const VMStateDescription vmstate_elp = {
>      }
>  };
>
> +static bool ssp_needed(void *opaque)
> +{
> +    RISCVCPU *cpu = opaque;
> +
> +    return cpu->cfg.ext_zicfiss;
> +}
> +
> +static const VMStateDescription vmstate_ssp = {
> +    .name = "cpu/ssp",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = ssp_needed,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_UINTTL(env.ssp, RISCVCPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  const VMStateDescription vmstate_riscv_cpu = {
>      .name = "cpu",
>      .version_id = 10,
> @@ -442,6 +460,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>          &vmstate_smstateen,
>          &vmstate_jvt,
>          &vmstate_elp,
> +        &vmstate_ssp,
>          NULL
>      }
>  };
> --
> 2.44.0
>
>

[PATCH v11 12/20] target/riscv: tb flag for shadow stack instructions

[Deepak Gupta]

Shadow stack instructions can be decoded as zimop / zcmop or shadow stack
instructions depending on whether shadow stack are enabled at current
privilege. This requires a TB flag so that correct TB generation and correct
TB lookup happens. `DisasContext` gets a field indicating whether bcfi is
enabled or not.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h        | 2 ++
 target/riscv/cpu_helper.c | 4 ++++
 target/riscv/translate.c  | 4 ++++
 3 files changed, 10 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4ace54a2eb..e758f4497e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -612,6 +612,8 @@ FIELD(TB_FLAGS, AXL, 26, 2)
 /* zicfilp needs a TB flag to track indirect branches */
 FIELD(TB_FLAGS, FCFI_ENABLED, 28, 1)
 FIELD(TB_FLAGS, FCFI_LP_EXPECTED, 29, 1)
+/* zicfiss needs a TB flag so that correct TB is located based on tb flags */
+FIELD(TB_FLAGS, BCFI_ENABLED, 30, 1)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f7e97eabfa..be4ac3d54e 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -169,6 +169,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
         flags = FIELD_DP32(flags, TB_FLAGS, FCFI_ENABLED, 1);
     }
 
+    if (cpu_get_bcfien(env)) {
+        flags = FIELD_DP32(flags, TB_FLAGS, BCFI_ENABLED, 1);
+    }
+
 #ifdef CONFIG_USER_ONLY
     fs = EXT_STATUS_DIRTY;
     vs = EXT_STATUS_DIRTY;
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index b5c0511b4b..b1d251e893 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -119,6 +119,8 @@ typedef struct DisasContext {
     /* zicfilp extension. fcfi_enabled, lp expected or not */
     bool fcfi_enabled;
     bool fcfi_lp_expected;
+    /* zicfiss extension, if shadow stack was enabled during TB gen */
+    bool bcfi_enabled;
 } DisasContext;
 
 static inline bool has_ext(DisasContext *ctx, uint32_t ext)
@@ -1241,6 +1243,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED);
     ctx->ztso = cpu->cfg.ext_ztso;
     ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
+    ctx->bcfi_enabled = cpu_get_bcfien(env) &&
+                        FIELD_EX32(tb_flags, TB_FLAGS, BCFI_ENABLED);
     ctx->fcfi_lp_expected = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_LP_EXPECTED);
     ctx->fcfi_enabled = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_ENABLED);
     ctx->zero = tcg_constant_tl(0);
-- 
2.44.0

Re: [PATCH v11 12/20] target/riscv: tb flag for shadow stack instructions

[Richard Henderson]

On 8/29/24 03:47, Deepak Gupta wrote:
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -169,6 +169,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>           flags = FIELD_DP32(flags, TB_FLAGS, FCFI_ENABLED, 1);
>       }
>   
> +    if (cpu_get_bcfien(env)) {
> +        flags = FIELD_DP32(flags, TB_FLAGS, BCFI_ENABLED, 1);
> +    }
> +
>   #ifdef CONFIG_USER_ONLY
>       fs = EXT_STATUS_DIRTY;
>       vs = EXT_STATUS_DIRTY;
...
> @@ -1241,6 +1243,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>       ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED);
>       ctx->ztso = cpu->cfg.ext_ztso;
>       ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
> +    ctx->bcfi_enabled = cpu_get_bcfien(env) &&
> +                        FIELD_EX32(tb_flags, TB_FLAGS, BCFI_ENABLED);
The call to cpu_get_bcfien here is wrong -- only test TB_FLAGS.
(1) The compile-time check vs env is incorrect,
(2) You've already done the check in cpu_get_tb_cpu_state.


r~

[PATCH v11 13/20] target/riscv: mmu changes for zicfiss shadow stack protection

[Deepak Gupta]

zicfiss protects shadow stack using new page table encodings PTE.W=1,
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu_helper.c | 37 +++++++++++++++++++++++++++++++------
 target/riscv/internals.h  |  3 +++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index be4ac3d54e..39544cade6 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -893,6 +893,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
     hwaddr ppn;
     int napot_bits = 0;
     target_ulong napot_mask;
+    bool is_sstack_idx = ((mmu_idx & MMU_IDX_SS_WRITE) == MMU_IDX_SS_WRITE);
+    bool sstack_page = false;
 
     /*
      * Check if we should use the background registers for the two
@@ -1101,21 +1103,36 @@ restart:
         return TRANSLATE_FAIL;
     }
 
+    target_ulong rwx = pte & (PTE_R | PTE_W | PTE_X);
     /* Check for reserved combinations of RWX flags. */
-    switch (pte & (PTE_R | PTE_W | PTE_X)) {
-    case PTE_W:
+    switch (rwx) {
     case PTE_W | PTE_X:
         return TRANSLATE_FAIL;
+    case PTE_W:
+        /* if bcfi enabled, PTE_W is not reserved and shadow stack page */
+        if (cpu_get_bcfien(env) && first_stage) {
+            sstack_page = true;
+            /* if ss index, read and write allowed. else only read allowed */
+            rwx = is_sstack_idx ? PTE_R | PTE_W : PTE_R;
+            break;
+        }
+        return TRANSLATE_FAIL;
+    case PTE_R:
+        /* shadow stack writes to readonly memory are page faults */
+        if (is_sstack_idx && access_type == MMU_DATA_STORE) {
+            return TRANSLATE_FAIL;
+        }
+        break;
     }
 
     int prot = 0;
-    if (pte & PTE_R) {
+    if (rwx & PTE_R) {
         prot |= PAGE_READ;
     }
-    if (pte & PTE_W) {
+    if (rwx & PTE_W) {
         prot |= PAGE_WRITE;
     }
-    if (pte & PTE_X) {
+    if (rwx & PTE_X) {
         bool mxr = false;
 
         /*
@@ -1160,7 +1177,7 @@ restart:
 
     if (!((prot >> access_type) & 1)) {
         /* Access check failed */
-        return TRANSLATE_FAIL;
+        return sstack_page ? TRANSLATE_PMP_FAIL : TRANSLATE_FAIL;
     }
 
     target_ulong updated_pte = pte;
@@ -1347,9 +1364,17 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
         break;
     case MMU_DATA_LOAD:
         cs->exception_index = RISCV_EXCP_LOAD_ADDR_MIS;
+        /* shadow stack mis aligned accesses are access faults */
+        if (mmu_idx & MMU_IDX_SS_WRITE) {
+            cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
+        }
         break;
     case MMU_DATA_STORE:
         cs->exception_index = RISCV_EXCP_STORE_AMO_ADDR_MIS;
+        /* shadow stack mis aligned accesses are access faults */
+        if (mmu_idx & MMU_IDX_SS_WRITE) {
+            cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+        }
         break;
     default:
         g_assert_not_reached();
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

Re: [PATCH v11 13/20] target/riscv: mmu changes for zicfiss shadow stack protection

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:49 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> zicfiss protects shadow stack using new page table encodings PTE.W=1,
> 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>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/riscv/cpu_helper.c | 37 +++++++++++++++++++++++++++++++------
>  target/riscv/internals.h  |  3 +++
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index be4ac3d54e..39544cade6 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -893,6 +893,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>      hwaddr ppn;
>      int napot_bits = 0;
>      target_ulong napot_mask;
> +    bool is_sstack_idx = ((mmu_idx & MMU_IDX_SS_WRITE) == MMU_IDX_SS_WRITE);
> +    bool sstack_page = false;
>
>      /*
>       * Check if we should use the background registers for the two
> @@ -1101,21 +1103,36 @@ restart:
>          return TRANSLATE_FAIL;
>      }
>
> +    target_ulong rwx = pte & (PTE_R | PTE_W | PTE_X);
>      /* Check for reserved combinations of RWX flags. */
> -    switch (pte & (PTE_R | PTE_W | PTE_X)) {
> -    case PTE_W:
> +    switch (rwx) {
>      case PTE_W | PTE_X:
>          return TRANSLATE_FAIL;
> +    case PTE_W:
> +        /* if bcfi enabled, PTE_W is not reserved and shadow stack page */
> +        if (cpu_get_bcfien(env) && first_stage) {
> +            sstack_page = true;
> +            /* if ss index, read and write allowed. else only read allowed */
> +            rwx = is_sstack_idx ? PTE_R | PTE_W : PTE_R;
> +            break;
> +        }
> +        return TRANSLATE_FAIL;
> +    case PTE_R:
> +        /* shadow stack writes to readonly memory are page faults */
> +        if (is_sstack_idx && access_type == MMU_DATA_STORE) {
> +            return TRANSLATE_FAIL;
> +        }
> +        break;
>      }
>
>      int prot = 0;
> -    if (pte & PTE_R) {
> +    if (rwx & PTE_R) {
>          prot |= PAGE_READ;
>      }
> -    if (pte & PTE_W) {
> +    if (rwx & PTE_W) {
>          prot |= PAGE_WRITE;
>      }
> -    if (pte & PTE_X) {
> +    if (rwx & PTE_X) {
>          bool mxr = false;
>
>          /*
> @@ -1160,7 +1177,7 @@ restart:
>
>      if (!((prot >> access_type) & 1)) {
>          /* Access check failed */
> -        return TRANSLATE_FAIL;
> +        return sstack_page ? TRANSLATE_PMP_FAIL : TRANSLATE_FAIL;

Why is it a PMP error if it's a shadow stack page?

Alistair

Re: [PATCH v11 13/20] target/riscv: mmu changes for zicfiss shadow stack protection

[Deepak Gupta]

On Thu, Aug 29, 2024 at 09:29:49AM +1000, Alistair Francis wrote:
>On Thu, Aug 29, 2024 at 3:49 AM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> zicfiss protects shadow stack using new page table encodings PTE.W=1,
>> 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>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/riscv/cpu_helper.c | 37 +++++++++++++++++++++++++++++++------
>>  target/riscv/internals.h  |  3 +++
>>  2 files changed, 34 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index be4ac3d54e..39544cade6 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -893,6 +893,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>>      hwaddr ppn;
>>      int napot_bits = 0;
>>      target_ulong napot_mask;
>> +    bool is_sstack_idx = ((mmu_idx & MMU_IDX_SS_WRITE) == MMU_IDX_SS_WRITE);
>> +    bool sstack_page = false;
>>
>>      /*
>>       * Check if we should use the background registers for the two
>> @@ -1101,21 +1103,36 @@ restart:
>>          return TRANSLATE_FAIL;
>>      }
>>
>> +    target_ulong rwx = pte & (PTE_R | PTE_W | PTE_X);
>>      /* Check for reserved combinations of RWX flags. */
>> -    switch (pte & (PTE_R | PTE_W | PTE_X)) {
>> -    case PTE_W:
>> +    switch (rwx) {
>>      case PTE_W | PTE_X:
>>          return TRANSLATE_FAIL;
>> +    case PTE_W:
>> +        /* if bcfi enabled, PTE_W is not reserved and shadow stack page */
>> +        if (cpu_get_bcfien(env) && first_stage) {
>> +            sstack_page = true;
>> +            /* if ss index, read and write allowed. else only read allowed */
>> +            rwx = is_sstack_idx ? PTE_R | PTE_W : PTE_R;
>> +            break;
>> +        }
>> +        return TRANSLATE_FAIL;
>> +    case PTE_R:
>> +        /* shadow stack writes to readonly memory are page faults */
>> +        if (is_sstack_idx && access_type == MMU_DATA_STORE) {

While responding to your question, I noticed there is a bug here. Its a leftover from
previous patches where I was promoting shadow stack loads to stores. No need to check
`access_type == MMU_DATA_STORE` because we store unwind information as part of tcg
compile.

Will fix it.

>> +            return TRANSLATE_FAIL;
>> +        }
>> +        break;
>>      }
>>
>>      int prot = 0;
>> -    if (pte & PTE_R) {
>> +    if (rwx & PTE_R) {
>>          prot |= PAGE_READ;
>>      }
>> -    if (pte & PTE_W) {
>> +    if (rwx & PTE_W) {
>>          prot |= PAGE_WRITE;
>>      }
>> -    if (pte & PTE_X) {
>> +    if (rwx & PTE_X) {
>>          bool mxr = false;
>>
>>          /*
>> @@ -1160,7 +1177,7 @@ restart:
>>
>>      if (!((prot >> access_type) & 1)) {
>>          /* Access check failed */
>> -        return TRANSLATE_FAIL;
>> +        return sstack_page ? TRANSLATE_PMP_FAIL : TRANSLATE_FAIL;
>
>Why is it a PMP error if it's a shadow stack page?

A shadow stack page is readable by regular loads.
We are making sure of that in `case PTE_W` in above switch case.
But shadow stack page is not writeable via regular stores. And must raise
access fault. return code `TRANSLATE_PMP_FAIL` is translated to access fault
while raising fault.

>
>Alistair

Re: [PATCH v11 13/20] target/riscv: mmu changes for zicfiss shadow stack protection

[Alistair Francis]

On Thu, Aug 29, 2024 at 9:45 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> On Thu, Aug 29, 2024 at 09:29:49AM +1000, Alistair Francis wrote:
> >On Thu, Aug 29, 2024 at 3:49 AM Deepak Gupta <debug@rivosinc.com> wrote:
> >>
> >> zicfiss protects shadow stack using new page table encodings PTE.W=1,
> >> 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>
> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>  target/riscv/cpu_helper.c | 37 +++++++++++++++++++++++++++++++------
> >>  target/riscv/internals.h  |  3 +++
> >>  2 files changed, 34 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >> index be4ac3d54e..39544cade6 100644
> >> --- a/target/riscv/cpu_helper.c
> >> +++ b/target/riscv/cpu_helper.c
> >> @@ -893,6 +893,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> >>      hwaddr ppn;
> >>      int napot_bits = 0;
> >>      target_ulong napot_mask;
> >> +    bool is_sstack_idx = ((mmu_idx & MMU_IDX_SS_WRITE) == MMU_IDX_SS_WRITE);
> >> +    bool sstack_page = false;
> >>
> >>      /*
> >>       * Check if we should use the background registers for the two
> >> @@ -1101,21 +1103,36 @@ restart:
> >>          return TRANSLATE_FAIL;
> >>      }
> >>
> >> +    target_ulong rwx = pte & (PTE_R | PTE_W | PTE_X);
> >>      /* Check for reserved combinations of RWX flags. */
> >> -    switch (pte & (PTE_R | PTE_W | PTE_X)) {
> >> -    case PTE_W:
> >> +    switch (rwx) {
> >>      case PTE_W | PTE_X:
> >>          return TRANSLATE_FAIL;
> >> +    case PTE_W:
> >> +        /* if bcfi enabled, PTE_W is not reserved and shadow stack page */
> >> +        if (cpu_get_bcfien(env) && first_stage) {
> >> +            sstack_page = true;
> >> +            /* if ss index, read and write allowed. else only read allowed */
> >> +            rwx = is_sstack_idx ? PTE_R | PTE_W : PTE_R;
> >> +            break;
> >> +        }
> >> +        return TRANSLATE_FAIL;
> >> +    case PTE_R:
> >> +        /* shadow stack writes to readonly memory are page faults */
> >> +        if (is_sstack_idx && access_type == MMU_DATA_STORE) {
>
> While responding to your question, I noticed there is a bug here. Its a leftover from
> previous patches where I was promoting shadow stack loads to stores. No need to check
> `access_type == MMU_DATA_STORE` because we store unwind information as part of tcg
> compile.
>
> Will fix it.
>
> >> +            return TRANSLATE_FAIL;
> >> +        }
> >> +        break;
> >>      }
> >>
> >>      int prot = 0;
> >> -    if (pte & PTE_R) {
> >> +    if (rwx & PTE_R) {
> >>          prot |= PAGE_READ;
> >>      }
> >> -    if (pte & PTE_W) {
> >> +    if (rwx & PTE_W) {
> >>          prot |= PAGE_WRITE;
> >>      }
> >> -    if (pte & PTE_X) {
> >> +    if (rwx & PTE_X) {
> >>          bool mxr = false;
> >>
> >>          /*
> >> @@ -1160,7 +1177,7 @@ restart:
> >>
> >>      if (!((prot >> access_type) & 1)) {
> >>          /* Access check failed */
> >> -        return TRANSLATE_FAIL;
> >> +        return sstack_page ? TRANSLATE_PMP_FAIL : TRANSLATE_FAIL;
> >
> >Why is it a PMP error if it's a shadow stack page?
>
> A shadow stack page is readable by regular loads.
> We are making sure of that in `case PTE_W` in above switch case.
> But shadow stack page is not writeable via regular stores. And must raise
> access fault. return code `TRANSLATE_PMP_FAIL` is translated to access fault
> while raising fault.

Ah, ok. It's worth commenting that we are returning TRANSLATE_PMP_FAIL
as that will be translated to an access fault

Alistair

>
> >
> >Alistair

Re: [PATCH v11 13/20] target/riscv: mmu changes for zicfiss shadow stack protection

[Deepak Gupta]

On Thu, Aug 29, 2024 at 10:03:04AM +1000, Alistair Francis wrote:
>On Thu, Aug 29, 2024 at 9:45 AM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> On Thu, Aug 29, 2024 at 09:29:49AM +1000, Alistair Francis wrote:
>> >On Thu, Aug 29, 2024 at 3:49 AM Deepak Gupta <debug@rivosinc.com> wrote:
>> >>
>> >>          prot |= PAGE_WRITE;
>> >>      }
>> >> -    if (pte & PTE_X) {
>> >> +    if (rwx & PTE_X) {
>> >>          bool mxr = false;
>> >>
>> >>          /*
>> >> @@ -1160,7 +1177,7 @@ restart:
>> >>
>> >>      if (!((prot >> access_type) & 1)) {
>> >>          /* Access check failed */
>> >> -        return TRANSLATE_FAIL;
>> >> +        return sstack_page ? TRANSLATE_PMP_FAIL : TRANSLATE_FAIL;
>> >
>> >Why is it a PMP error if it's a shadow stack page?
>>
>> A shadow stack page is readable by regular loads.
>> We are making sure of that in `case PTE_W` in above switch case.
>> But shadow stack page is not writeable via regular stores. And must raise
>> access fault. return code `TRANSLATE_PMP_FAIL` is translated to access fault
>> while raising fault.
>
>Ah, ok. It's worth commenting that we are returning TRANSLATE_PMP_FAIL
>as that will be translated to an access fault

Ack.

>
>Alistair
>
>>
>> >
>> >Alistair

[PATCH v11 14/20] target/riscv: AMO operations always raise store/AMO fault

[Deepak Gupta]

This patch adds one more word for tcg compile which can be obtained during
unwind time to determine fault type for original operation (example AMO).
Depending on that, fault can be promoted to store/AMO fault.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.h         |  9 ++++++++-
 target/riscv/cpu_helper.c  | 20 ++++++++++++++++++++
 target/riscv/tcg/tcg-cpu.c |  1 +
 target/riscv/translate.c   |  2 +-
 4 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index e758f4497e..0a13604e37 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -46,8 +46,13 @@ typedef struct CPUArchState CPURISCVState;
 /*
  * RISC-V-specific extra insn start words:
  * 1: Original instruction opcode
+ * 2: more information about instruction
  */
-#define TARGET_INSN_START_EXTRA_WORDS 1
+#define TARGET_INSN_START_EXTRA_WORDS 2
+/*
+ * b0: Whether a instruction always raise a store AMO or not.
+ */
+#define RISCV_UW2_ALWAYS_STORE_AMO 1
 
 #define RV(x) ((target_ulong)1 << (x - 'A'))
 
@@ -226,6 +231,8 @@ struct CPUArchState {
     bool      elp;
     /* shadow stack register for zicfiss extension */
     target_ulong ssp;
+    /* env place holder for extra word 2 during unwind */
+    target_ulong excp_uw2;
     /* sw check code for sw check exception */
     target_ulong sw_check_code;
 #ifdef CONFIG_USER_ONLY
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 39544cade6..8294279b01 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1741,6 +1741,22 @@ static target_ulong riscv_transformed_insn(CPURISCVState *env,
     return xinsn;
 }
 
+static target_ulong promote_load_fault(target_ulong orig_cause)
+{
+    switch (orig_cause) {
+    case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
+        return RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
+
+    case RISCV_EXCP_LOAD_ACCESS_FAULT:
+        return RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+
+    case RISCV_EXCP_LOAD_PAGE_FAULT:
+        return RISCV_EXCP_STORE_PAGE_FAULT;
+    }
+
+    /* if no promotion, return original cause */
+    return orig_cause;
+}
 /*
  * Handle Traps
  *
@@ -1752,6 +1768,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
     bool write_gva = false;
+    bool always_storeamo = (env->excp_uw2 & RISCV_UW2_ALWAYS_STORE_AMO);
     uint64_t s;
 
     /*
@@ -1785,6 +1802,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
         case RISCV_EXCP_STORE_AMO_ACCESS_FAULT:
         case RISCV_EXCP_LOAD_PAGE_FAULT:
         case RISCV_EXCP_STORE_PAGE_FAULT:
+            if (always_storeamo) {
+                cause = promote_load_fault(cause);
+            }
             write_gva = env->two_stage_lookup;
             tval = env->badaddr;
             if (env->two_stage_indirect_lookup) {
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 4da26cb926..83771303a8 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -129,6 +129,7 @@ static void riscv_restore_state_to_opc(CPUState *cs,
         env->pc = pc;
     }
     env->bins = data[1];
+    env->excp_uw2 = data[2];
 }
 
 static const TCGCPUOps riscv_tcg_ops = {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index b1d251e893..16fff70dac 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1265,7 +1265,7 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
         pc_next &= ~TARGET_PAGE_MASK;
     }
 
-    tcg_gen_insn_start(pc_next, 0);
+    tcg_gen_insn_start(pc_next, 0, 0);
     ctx->insn_start_updated = false;
 }
 
-- 
2.44.0

Re: [PATCH v11 14/20] target/riscv: AMO operations always raise store/AMO fault

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:49 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> This patch adds one more word for tcg compile which can be obtained during
> unwind time to determine fault type for original operation (example AMO).
> Depending on that, fault can be promoted to store/AMO fault.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

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

Alistair

> ---
>  target/riscv/cpu.h         |  9 ++++++++-
>  target/riscv/cpu_helper.c  | 20 ++++++++++++++++++++
>  target/riscv/tcg/tcg-cpu.c |  1 +
>  target/riscv/translate.c   |  2 +-
>  4 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e758f4497e..0a13604e37 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -46,8 +46,13 @@ typedef struct CPUArchState CPURISCVState;
>  /*
>   * RISC-V-specific extra insn start words:
>   * 1: Original instruction opcode
> + * 2: more information about instruction
>   */
> -#define TARGET_INSN_START_EXTRA_WORDS 1
> +#define TARGET_INSN_START_EXTRA_WORDS 2
> +/*
> + * b0: Whether a instruction always raise a store AMO or not.
> + */
> +#define RISCV_UW2_ALWAYS_STORE_AMO 1
>
>  #define RV(x) ((target_ulong)1 << (x - 'A'))
>
> @@ -226,6 +231,8 @@ struct CPUArchState {
>      bool      elp;
>      /* shadow stack register for zicfiss extension */
>      target_ulong ssp;
> +    /* env place holder for extra word 2 during unwind */
> +    target_ulong excp_uw2;
>      /* sw check code for sw check exception */
>      target_ulong sw_check_code;
>  #ifdef CONFIG_USER_ONLY
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 39544cade6..8294279b01 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1741,6 +1741,22 @@ static target_ulong riscv_transformed_insn(CPURISCVState *env,
>      return xinsn;
>  }
>
> +static target_ulong promote_load_fault(target_ulong orig_cause)
> +{
> +    switch (orig_cause) {
> +    case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
> +        return RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
> +
> +    case RISCV_EXCP_LOAD_ACCESS_FAULT:
> +        return RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> +
> +    case RISCV_EXCP_LOAD_PAGE_FAULT:
> +        return RISCV_EXCP_STORE_PAGE_FAULT;
> +    }
> +
> +    /* if no promotion, return original cause */
> +    return orig_cause;
> +}
>  /*
>   * Handle Traps
>   *
> @@ -1752,6 +1768,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      CPURISCVState *env = &cpu->env;
>      bool write_gva = false;
> +    bool always_storeamo = (env->excp_uw2 & RISCV_UW2_ALWAYS_STORE_AMO);
>      uint64_t s;
>
>      /*
> @@ -1785,6 +1802,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>          case RISCV_EXCP_STORE_AMO_ACCESS_FAULT:
>          case RISCV_EXCP_LOAD_PAGE_FAULT:
>          case RISCV_EXCP_STORE_PAGE_FAULT:
> +            if (always_storeamo) {
> +                cause = promote_load_fault(cause);
> +            }
>              write_gva = env->two_stage_lookup;
>              tval = env->badaddr;
>              if (env->two_stage_indirect_lookup) {
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 4da26cb926..83771303a8 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -129,6 +129,7 @@ static void riscv_restore_state_to_opc(CPUState *cs,
>          env->pc = pc;
>      }
>      env->bins = data[1];
> +    env->excp_uw2 = data[2];
>  }
>
>  static const TCGCPUOps riscv_tcg_ops = {
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index b1d251e893..16fff70dac 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1265,7 +1265,7 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
>          pc_next &= ~TARGET_PAGE_MASK;
>      }
>
> -    tcg_gen_insn_start(pc_next, 0);
> +    tcg_gen_insn_start(pc_next, 0, 0);
>      ctx->insn_start_updated = false;
>  }
>
> --
> 2.44.0
>
>

[PATCH v11 15/20] target/riscv: update `decode_save_opc` to store extra word2

[Deepak Gupta]

Extra word 2 is stored during tcg compile and `decode_save_opc` needs
additional argument in order to pass the value. This will be used during
unwind to get extra information about instruction like how to massage
exceptions. Updated all callsites as well.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/insn_trans/trans_privileged.c.inc |  8 ++++----
 target/riscv/insn_trans/trans_rva.c.inc        |  4 ++--
 target/riscv/insn_trans/trans_rvd.c.inc        |  4 ++--
 target/riscv/insn_trans/trans_rvf.c.inc        |  4 ++--
 target/riscv/insn_trans/trans_rvh.c.inc        |  8 ++++----
 target/riscv/insn_trans/trans_rvi.c.inc        |  6 +++---
 target/riscv/insn_trans/trans_rvvk.c.inc       | 10 +++++-----
 target/riscv/insn_trans/trans_rvzacas.c.inc    |  4 ++--
 target/riscv/insn_trans/trans_rvzfh.c.inc      |  4 ++--
 target/riscv/insn_trans/trans_svinval.c.inc    |  6 +++---
 target/riscv/translate.c                       | 11 ++++++-----
 11 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index bc5263a4e0..ecd3b8b2c9 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -78,7 +78,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 {
 #ifndef CONFIG_USER_ONLY
     if (has_ext(ctx, RVS)) {
-        decode_save_opc(ctx);
+        decode_save_opc(ctx, 0);
         translator_io_start(&ctx->base);
         gen_helper_sret(cpu_pc, tcg_env);
         exit_tb(ctx); /* no chaining */
@@ -95,7 +95,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 static bool trans_mret(DisasContext *ctx, arg_mret *a)
 {
 #ifndef CONFIG_USER_ONLY
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     translator_io_start(&ctx->base);
     gen_helper_mret(cpu_pc, tcg_env);
     exit_tb(ctx); /* no chaining */
@@ -109,7 +109,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
 static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
 {
 #ifndef CONFIG_USER_ONLY
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_update_pc(ctx, ctx->cur_insn_len);
     gen_helper_wfi(tcg_env);
     return true;
@@ -121,7 +121,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
 static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
 {
 #ifndef CONFIG_USER_ONLY
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_helper_tlb_flush(tcg_env);
     return true;
 #endif
diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
index 39bbf60f3c..9cf3ae8019 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -34,7 +34,7 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
 {
     TCGv src1;
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     src1 = get_address(ctx, a->rs1, 0);
     if (a->rl) {
         tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
@@ -61,7 +61,7 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
     TCGLabel *l1 = gen_new_label();
     TCGLabel *l2 = gen_new_label();
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     src1 = get_address(ctx, a->rs1, 0);
     tcg_gen_brcond_tl(TCG_COND_NE, load_res, src1, l1);
 
diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
index 1f5fac65a2..d779ec75c7 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -51,7 +51,7 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
         memop |= MO_ATOM_WITHIN16;
     }
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     addr = get_address(ctx, a->rs1, a->imm);
     tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], addr, ctx->mem_idx, memop);
 
@@ -71,7 +71,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
         memop |= MO_ATOM_WITHIN16;
     }
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     addr = get_address(ctx, a->rs1, a->imm);
     tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], addr, ctx->mem_idx, memop);
     return true;
diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
index f771aa1939..084c184e65 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -52,7 +52,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
         memop |= MO_ATOM_WITHIN16;
     }
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     addr = get_address(ctx, a->rs1, a->imm);
     dest = cpu_fpr[a->rd];
     tcg_gen_qemu_ld_i64(dest, addr, ctx->mem_idx, memop);
@@ -74,7 +74,7 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a)
         memop |= MO_ATOM_WITHIN16;
     }
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     addr = get_address(ctx, a->rs1, a->imm);
     tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], addr, ctx->mem_idx, memop);
     return true;
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc b/target/riscv/insn_trans/trans_rvh.c.inc
index aa9d41c18c..03c6694430 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -44,7 +44,7 @@ static bool do_hlv(DisasContext *ctx, arg_r2 *a,
     TCGv dest = dest_gpr(ctx, a->rd);
     TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     func(dest, tcg_env, addr);
     gen_set_gpr(ctx, a->rd, dest);
     return true;
@@ -56,7 +56,7 @@ static bool do_hsv(DisasContext *ctx, arg_r2_s *a,
     TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
     TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     func(tcg_env, addr, data);
     return true;
 }
@@ -147,7 +147,7 @@ static bool trans_hfence_gvma(DisasContext *ctx, arg_sfence_vma *a)
 {
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_helper_hyp_gvma_tlb_flush(tcg_env);
     return true;
 #endif
@@ -158,7 +158,7 @@ static bool trans_hfence_vvma(DisasContext *ctx, arg_sfence_vma *a)
 {
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_helper_hyp_tlb_flush(tcg_env);
     return true;
 #endif
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index b427f3a939..a619ea7c0e 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -326,7 +326,7 @@ static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop)
     if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
         memop |= MO_ATOM_WITHIN16;
     }
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     if (get_xl(ctx) == MXL_RV128) {
         out = gen_load_i128(ctx, a, memop);
     } else {
@@ -427,7 +427,7 @@ static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
     if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
         memop |= MO_ATOM_WITHIN16;
     }
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     if (get_xl(ctx) == MXL_RV128) {
         return gen_store_i128(ctx, a, memop);
     } else {
@@ -889,7 +889,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
 static bool do_csr_post(DisasContext *ctx)
 {
     /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     /* We may have changed important cpu state -- exit to main loop. */
     gen_update_pc(ctx, ctx->cur_insn_len);
     exit_tb(ctx);
diff --git a/target/riscv/insn_trans/trans_rvvk.c.inc b/target/riscv/insn_trans/trans_rvvk.c.inc
index ae1f40174a..27bf3f0b68 100644
--- a/target/riscv/insn_trans/trans_rvvk.c.inc
+++ b/target/riscv/insn_trans/trans_rvvk.c.inc
@@ -249,7 +249,7 @@ GEN_OPIVI_WIDEN_TRANS(vwsll_vi, IMM_ZX, vwsll_vx, vwsll_vx_check)
                                                                               \
             if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {                      \
                 /* save opcode for unwinding in case we throw an exception */ \
-                decode_save_opc(s);                                           \
+                decode_save_opc(s, 0);                                        \
                 egs = tcg_constant_i32(EGS);                                  \
                 gen_helper_egs_check(egs, tcg_env);                           \
             }                                                                 \
@@ -322,7 +322,7 @@ GEN_V_UNMASKED_TRANS(vaesem_vs, vaes_check_vs, ZVKNED_EGS)
                                                                               \
             if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {                      \
                 /* save opcode for unwinding in case we throw an exception */ \
-                decode_save_opc(s);                                           \
+                decode_save_opc(s, 0);                                        \
                 egs = tcg_constant_i32(EGS);                                  \
                 gen_helper_egs_check(egs, tcg_env);                           \
             }                                                                 \
@@ -389,7 +389,7 @@ GEN_VI_UNMASKED_TRANS(vaeskf2_vi, vaeskf2_check, ZVKNED_EGS)
                                                                               \
             if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {                      \
                 /* save opcode for unwinding in case we throw an exception */ \
-                decode_save_opc(s);                                           \
+                decode_save_opc(s, 0);                                        \
                 egs = tcg_constant_i32(EGS);                                  \
                 gen_helper_egs_check(egs, tcg_env);                           \
             }                                                                 \
@@ -440,7 +440,7 @@ static bool trans_vsha2cl_vv(DisasContext *s, arg_rmrr *a)
 
         if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {
             /* save opcode for unwinding in case we throw an exception */
-            decode_save_opc(s);
+            decode_save_opc(s, 0);
             egs = tcg_constant_i32(ZVKNH_EGS);
             gen_helper_egs_check(egs, tcg_env);
         }
@@ -471,7 +471,7 @@ static bool trans_vsha2ch_vv(DisasContext *s, arg_rmrr *a)
 
         if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {
             /* save opcode for unwinding in case we throw an exception */
-            decode_save_opc(s);
+            decode_save_opc(s, 0);
             egs = tcg_constant_i32(ZVKNH_EGS);
             gen_helper_egs_check(egs, tcg_env);
         }
diff --git a/target/riscv/insn_trans/trans_rvzacas.c.inc b/target/riscv/insn_trans/trans_rvzacas.c.inc
index fcced99fc7..15e688a033 100644
--- a/target/riscv/insn_trans/trans_rvzacas.c.inc
+++ b/target/riscv/insn_trans/trans_rvzacas.c.inc
@@ -76,7 +76,7 @@ static bool gen_cmpxchg64(DisasContext *ctx, arg_atomic *a, MemOp mop)
     TCGv src1 = get_address(ctx, a->rs1, 0);
     TCGv_i64 src2 = get_gpr_pair(ctx, a->rs2);
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
     tcg_gen_atomic_cmpxchg_i64(dest, src1, dest, src2, ctx->mem_idx, mop);
 
     gen_set_gpr_pair(ctx, a->rd, dest);
@@ -121,7 +121,7 @@ static bool trans_amocas_q(DisasContext *ctx, arg_amocas_q *a)
 
     tcg_gen_concat_i64_i128(src2, src2l, src2h);
     tcg_gen_concat_i64_i128(dest, destl, desth);
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
     tcg_gen_atomic_cmpxchg_i128(dest, src1, dest, src2, ctx->mem_idx,
                                 (MO_ALIGN | MO_TEUO));
 
diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
index 1eb458b491..bece48e600 100644
--- a/target/riscv/insn_trans/trans_rvzfh.c.inc
+++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
@@ -48,7 +48,7 @@ static bool trans_flh(DisasContext *ctx, arg_flh *a)
     REQUIRE_FPU;
     REQUIRE_ZFHMIN_OR_ZFBFMIN(ctx);
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     t0 = get_gpr(ctx, a->rs1, EXT_NONE);
     if (a->imm) {
         TCGv temp = tcg_temp_new();
@@ -71,7 +71,7 @@ static bool trans_fsh(DisasContext *ctx, arg_fsh *a)
     REQUIRE_FPU;
     REQUIRE_ZFHMIN_OR_ZFBFMIN(ctx);
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     t0 = get_gpr(ctx, a->rs1, EXT_NONE);
     if (a->imm) {
         TCGv temp = tcg_temp_new();
diff --git a/target/riscv/insn_trans/trans_svinval.c.inc b/target/riscv/insn_trans/trans_svinval.c.inc
index 0f692a1088..a06c3b214f 100644
--- a/target/riscv/insn_trans/trans_svinval.c.inc
+++ b/target/riscv/insn_trans/trans_svinval.c.inc
@@ -28,7 +28,7 @@ static bool trans_sinval_vma(DisasContext *ctx, arg_sinval_vma *a)
     /* Do the same as sfence.vma currently */
     REQUIRE_EXT(ctx, RVS);
 #ifndef CONFIG_USER_ONLY
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_helper_tlb_flush(tcg_env);
     return true;
 #endif
@@ -57,7 +57,7 @@ static bool trans_hinval_vvma(DisasContext *ctx, arg_hinval_vvma *a)
     /* Do the same as hfence.vvma currently */
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_helper_hyp_tlb_flush(tcg_env);
     return true;
 #endif
@@ -70,7 +70,7 @@ static bool trans_hinval_gvma(DisasContext *ctx, arg_hinval_gvma *a)
     /* Do the same as hfence.gvma currently */
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_helper_hyp_gvma_tlb_flush(tcg_env);
     return true;
 #endif
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 16fff70dac..e677062a10 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -209,11 +209,12 @@ static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in)
     tcg_gen_movcond_i64(TCG_COND_GEU, out, in, t_max, in, t_nan);
 }
 
-static void decode_save_opc(DisasContext *ctx)
+static void decode_save_opc(DisasContext *ctx, target_ulong excp_uw2)
 {
     assert(!ctx->insn_start_updated);
     ctx->insn_start_updated = true;
     tcg_set_insn_start_param(ctx->base.insn_start, 1, ctx->opcode);
+    tcg_set_insn_start_param(ctx->base.insn_start, 2, excp_uw2);
 }
 
 static void gen_pc_plus_diff(TCGv target, DisasContext *ctx,
@@ -699,7 +700,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
     }
 
     /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_helper_set_rounding_mode(tcg_env, tcg_constant_i32(rm));
 }
 
@@ -712,7 +713,7 @@ static void gen_set_rm_chkfrm(DisasContext *ctx, int rm)
     ctx->frm_valid = true;
 
     /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, 0);
     gen_helper_set_rounding_mode_chkfrm(tcg_env, tcg_constant_i32(rm));
 }
 
@@ -1096,7 +1097,7 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
         mop |= MO_ALIGN;
     }
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
     src1 = get_address(ctx, a->rs1, 0);
     func(dest, src1, src2, ctx->mem_idx, mop);
 
@@ -1110,7 +1111,7 @@ static bool gen_cmpxchg(DisasContext *ctx, arg_atomic *a, MemOp mop)
     TCGv src1 = get_address(ctx, a->rs1, 0);
     TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
 
-    decode_save_opc(ctx);
+    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
     tcg_gen_atomic_cmpxchg_tl(dest, src1, dest, src2, ctx->mem_idx, mop);
 
     gen_set_gpr(ctx, a->rd, dest);
-- 
2.44.0

Re: [PATCH v11 15/20] target/riscv: update `decode_save_opc` to store extra word2

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:51 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> Extra word 2 is stored during tcg compile and `decode_save_opc` needs
> additional argument in order to pass the value. This will be used during
> unwind to get extra information about instruction like how to massage
> exceptions. Updated all callsites as well.

Woo!

This should fix https://gitlab.com/qemu-project/qemu/-/issues/594 as well

>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/594
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/insn_trans/trans_privileged.c.inc |  8 ++++----
>  target/riscv/insn_trans/trans_rva.c.inc        |  4 ++--
>  target/riscv/insn_trans/trans_rvd.c.inc        |  4 ++--
>  target/riscv/insn_trans/trans_rvf.c.inc        |  4 ++--
>  target/riscv/insn_trans/trans_rvh.c.inc        |  8 ++++----
>  target/riscv/insn_trans/trans_rvi.c.inc        |  6 +++---
>  target/riscv/insn_trans/trans_rvvk.c.inc       | 10 +++++-----
>  target/riscv/insn_trans/trans_rvzacas.c.inc    |  4 ++--
>  target/riscv/insn_trans/trans_rvzfh.c.inc      |  4 ++--
>  target/riscv/insn_trans/trans_svinval.c.inc    |  6 +++---
>  target/riscv/translate.c                       | 11 ++++++-----
>  11 files changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index bc5263a4e0..ecd3b8b2c9 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -78,7 +78,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
>  {
>  #ifndef CONFIG_USER_ONLY
>      if (has_ext(ctx, RVS)) {
> -        decode_save_opc(ctx);
> +        decode_save_opc(ctx, 0);
>          translator_io_start(&ctx->base);
>          gen_helper_sret(cpu_pc, tcg_env);
>          exit_tb(ctx); /* no chaining */
> @@ -95,7 +95,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
>  static bool trans_mret(DisasContext *ctx, arg_mret *a)
>  {
>  #ifndef CONFIG_USER_ONLY
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      translator_io_start(&ctx->base);
>      gen_helper_mret(cpu_pc, tcg_env);
>      exit_tb(ctx); /* no chaining */
> @@ -109,7 +109,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
>  static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
>  {
>  #ifndef CONFIG_USER_ONLY
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_update_pc(ctx, ctx->cur_insn_len);
>      gen_helper_wfi(tcg_env);
>      return true;
> @@ -121,7 +121,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
>  static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
>  {
>  #ifndef CONFIG_USER_ONLY
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_helper_tlb_flush(tcg_env);
>      return true;
>  #endif
> diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
> index 39bbf60f3c..9cf3ae8019 100644
> --- a/target/riscv/insn_trans/trans_rva.c.inc
> +++ b/target/riscv/insn_trans/trans_rva.c.inc
> @@ -34,7 +34,7 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
>  {
>      TCGv src1;
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      src1 = get_address(ctx, a->rs1, 0);
>      if (a->rl) {
>          tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
> @@ -61,7 +61,7 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
>      TCGLabel *l1 = gen_new_label();
>      TCGLabel *l2 = gen_new_label();
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      src1 = get_address(ctx, a->rs1, 0);
>      tcg_gen_brcond_tl(TCG_COND_NE, load_res, src1, l1);
>
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
> index 1f5fac65a2..d779ec75c7 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -51,7 +51,7 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a)
>          memop |= MO_ATOM_WITHIN16;
>      }
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      addr = get_address(ctx, a->rs1, a->imm);
>      tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], addr, ctx->mem_idx, memop);
>
> @@ -71,7 +71,7 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
>          memop |= MO_ATOM_WITHIN16;
>      }
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      addr = get_address(ctx, a->rs1, a->imm);
>      tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], addr, ctx->mem_idx, memop);
>      return true;
> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
> index f771aa1939..084c184e65 100644
> --- a/target/riscv/insn_trans/trans_rvf.c.inc
> +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> @@ -52,7 +52,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
>          memop |= MO_ATOM_WITHIN16;
>      }
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      addr = get_address(ctx, a->rs1, a->imm);
>      dest = cpu_fpr[a->rd];
>      tcg_gen_qemu_ld_i64(dest, addr, ctx->mem_idx, memop);
> @@ -74,7 +74,7 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a)
>          memop |= MO_ATOM_WITHIN16;
>      }
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      addr = get_address(ctx, a->rs1, a->imm);
>      tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], addr, ctx->mem_idx, memop);
>      return true;
> diff --git a/target/riscv/insn_trans/trans_rvh.c.inc b/target/riscv/insn_trans/trans_rvh.c.inc
> index aa9d41c18c..03c6694430 100644
> --- a/target/riscv/insn_trans/trans_rvh.c.inc
> +++ b/target/riscv/insn_trans/trans_rvh.c.inc
> @@ -44,7 +44,7 @@ static bool do_hlv(DisasContext *ctx, arg_r2 *a,
>      TCGv dest = dest_gpr(ctx, a->rd);
>      TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      func(dest, tcg_env, addr);
>      gen_set_gpr(ctx, a->rd, dest);
>      return true;
> @@ -56,7 +56,7 @@ static bool do_hsv(DisasContext *ctx, arg_r2_s *a,
>      TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
>      TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      func(tcg_env, addr, data);
>      return true;
>  }
> @@ -147,7 +147,7 @@ static bool trans_hfence_gvma(DisasContext *ctx, arg_sfence_vma *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
>  #ifndef CONFIG_USER_ONLY
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_helper_hyp_gvma_tlb_flush(tcg_env);
>      return true;
>  #endif
> @@ -158,7 +158,7 @@ static bool trans_hfence_vvma(DisasContext *ctx, arg_sfence_vma *a)
>  {
>      REQUIRE_EXT(ctx, RVH);
>  #ifndef CONFIG_USER_ONLY
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_helper_hyp_tlb_flush(tcg_env);
>      return true;
>  #endif
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> index b427f3a939..a619ea7c0e 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -326,7 +326,7 @@ static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop)
>      if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
>          memop |= MO_ATOM_WITHIN16;
>      }
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      if (get_xl(ctx) == MXL_RV128) {
>          out = gen_load_i128(ctx, a, memop);
>      } else {
> @@ -427,7 +427,7 @@ static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
>      if (ctx->cfg_ptr->ext_zama16b && (ctx->cur_insn_len != 2)) {
>          memop |= MO_ATOM_WITHIN16;
>      }
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      if (get_xl(ctx) == MXL_RV128) {
>          return gen_store_i128(ctx, a, memop);
>      } else {
> @@ -889,7 +889,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
>  static bool do_csr_post(DisasContext *ctx)
>  {
>      /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      /* We may have changed important cpu state -- exit to main loop. */
>      gen_update_pc(ctx, ctx->cur_insn_len);
>      exit_tb(ctx);
> diff --git a/target/riscv/insn_trans/trans_rvvk.c.inc b/target/riscv/insn_trans/trans_rvvk.c.inc
> index ae1f40174a..27bf3f0b68 100644
> --- a/target/riscv/insn_trans/trans_rvvk.c.inc
> +++ b/target/riscv/insn_trans/trans_rvvk.c.inc
> @@ -249,7 +249,7 @@ GEN_OPIVI_WIDEN_TRANS(vwsll_vi, IMM_ZX, vwsll_vx, vwsll_vx_check)
>                                                                                \
>              if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {                      \
>                  /* save opcode for unwinding in case we throw an exception */ \
> -                decode_save_opc(s);                                           \
> +                decode_save_opc(s, 0);                                        \
>                  egs = tcg_constant_i32(EGS);                                  \
>                  gen_helper_egs_check(egs, tcg_env);                           \
>              }                                                                 \
> @@ -322,7 +322,7 @@ GEN_V_UNMASKED_TRANS(vaesem_vs, vaes_check_vs, ZVKNED_EGS)
>                                                                                \
>              if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {                      \
>                  /* save opcode for unwinding in case we throw an exception */ \
> -                decode_save_opc(s);                                           \
> +                decode_save_opc(s, 0);                                        \
>                  egs = tcg_constant_i32(EGS);                                  \
>                  gen_helper_egs_check(egs, tcg_env);                           \
>              }                                                                 \
> @@ -389,7 +389,7 @@ GEN_VI_UNMASKED_TRANS(vaeskf2_vi, vaeskf2_check, ZVKNED_EGS)
>                                                                                \
>              if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {                      \
>                  /* save opcode for unwinding in case we throw an exception */ \
> -                decode_save_opc(s);                                           \
> +                decode_save_opc(s, 0);                                        \
>                  egs = tcg_constant_i32(EGS);                                  \
>                  gen_helper_egs_check(egs, tcg_env);                           \
>              }                                                                 \
> @@ -440,7 +440,7 @@ static bool trans_vsha2cl_vv(DisasContext *s, arg_rmrr *a)
>
>          if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {
>              /* save opcode for unwinding in case we throw an exception */
> -            decode_save_opc(s);
> +            decode_save_opc(s, 0);
>              egs = tcg_constant_i32(ZVKNH_EGS);
>              gen_helper_egs_check(egs, tcg_env);
>          }
> @@ -471,7 +471,7 @@ static bool trans_vsha2ch_vv(DisasContext *s, arg_rmrr *a)
>
>          if (!s->vstart_eq_zero || !s->vl_eq_vlmax) {
>              /* save opcode for unwinding in case we throw an exception */
> -            decode_save_opc(s);
> +            decode_save_opc(s, 0);
>              egs = tcg_constant_i32(ZVKNH_EGS);
>              gen_helper_egs_check(egs, tcg_env);
>          }
> diff --git a/target/riscv/insn_trans/trans_rvzacas.c.inc b/target/riscv/insn_trans/trans_rvzacas.c.inc
> index fcced99fc7..15e688a033 100644
> --- a/target/riscv/insn_trans/trans_rvzacas.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzacas.c.inc
> @@ -76,7 +76,7 @@ static bool gen_cmpxchg64(DisasContext *ctx, arg_atomic *a, MemOp mop)
>      TCGv src1 = get_address(ctx, a->rs1, 0);
>      TCGv_i64 src2 = get_gpr_pair(ctx, a->rs2);
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>      tcg_gen_atomic_cmpxchg_i64(dest, src1, dest, src2, ctx->mem_idx, mop);
>
>      gen_set_gpr_pair(ctx, a->rd, dest);
> @@ -121,7 +121,7 @@ static bool trans_amocas_q(DisasContext *ctx, arg_amocas_q *a)
>
>      tcg_gen_concat_i64_i128(src2, src2l, src2h);
>      tcg_gen_concat_i64_i128(dest, destl, desth);
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>      tcg_gen_atomic_cmpxchg_i128(dest, src1, dest, src2, ctx->mem_idx,
>                                  (MO_ALIGN | MO_TEUO));
>
> diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
> index 1eb458b491..bece48e600 100644
> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> @@ -48,7 +48,7 @@ static bool trans_flh(DisasContext *ctx, arg_flh *a)
>      REQUIRE_FPU;
>      REQUIRE_ZFHMIN_OR_ZFBFMIN(ctx);
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      t0 = get_gpr(ctx, a->rs1, EXT_NONE);
>      if (a->imm) {
>          TCGv temp = tcg_temp_new();
> @@ -71,7 +71,7 @@ static bool trans_fsh(DisasContext *ctx, arg_fsh *a)
>      REQUIRE_FPU;
>      REQUIRE_ZFHMIN_OR_ZFBFMIN(ctx);
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      t0 = get_gpr(ctx, a->rs1, EXT_NONE);
>      if (a->imm) {
>          TCGv temp = tcg_temp_new();
> diff --git a/target/riscv/insn_trans/trans_svinval.c.inc b/target/riscv/insn_trans/trans_svinval.c.inc
> index 0f692a1088..a06c3b214f 100644
> --- a/target/riscv/insn_trans/trans_svinval.c.inc
> +++ b/target/riscv/insn_trans/trans_svinval.c.inc
> @@ -28,7 +28,7 @@ static bool trans_sinval_vma(DisasContext *ctx, arg_sinval_vma *a)
>      /* Do the same as sfence.vma currently */
>      REQUIRE_EXT(ctx, RVS);
>  #ifndef CONFIG_USER_ONLY
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_helper_tlb_flush(tcg_env);
>      return true;
>  #endif
> @@ -57,7 +57,7 @@ static bool trans_hinval_vvma(DisasContext *ctx, arg_hinval_vvma *a)
>      /* Do the same as hfence.vvma currently */
>      REQUIRE_EXT(ctx, RVH);
>  #ifndef CONFIG_USER_ONLY
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_helper_hyp_tlb_flush(tcg_env);
>      return true;
>  #endif
> @@ -70,7 +70,7 @@ static bool trans_hinval_gvma(DisasContext *ctx, arg_hinval_gvma *a)
>      /* Do the same as hfence.gvma currently */
>      REQUIRE_EXT(ctx, RVH);
>  #ifndef CONFIG_USER_ONLY
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_helper_hyp_gvma_tlb_flush(tcg_env);
>      return true;
>  #endif
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 16fff70dac..e677062a10 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -209,11 +209,12 @@ static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in)
>      tcg_gen_movcond_i64(TCG_COND_GEU, out, in, t_max, in, t_nan);
>  }
>
> -static void decode_save_opc(DisasContext *ctx)
> +static void decode_save_opc(DisasContext *ctx, target_ulong excp_uw2)
>  {
>      assert(!ctx->insn_start_updated);
>      ctx->insn_start_updated = true;
>      tcg_set_insn_start_param(ctx->base.insn_start, 1, ctx->opcode);
> +    tcg_set_insn_start_param(ctx->base.insn_start, 2, excp_uw2);
>  }
>
>  static void gen_pc_plus_diff(TCGv target, DisasContext *ctx,
> @@ -699,7 +700,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
>      }
>
>      /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_helper_set_rounding_mode(tcg_env, tcg_constant_i32(rm));
>  }
>
> @@ -712,7 +713,7 @@ static void gen_set_rm_chkfrm(DisasContext *ctx, int rm)
>      ctx->frm_valid = true;
>
>      /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, 0);
>      gen_helper_set_rounding_mode_chkfrm(tcg_env, tcg_constant_i32(rm));
>  }
>
> @@ -1096,7 +1097,7 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
>          mop |= MO_ALIGN;
>      }
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>      src1 = get_address(ctx, a->rs1, 0);
>      func(dest, src1, src2, ctx->mem_idx, mop);
>
> @@ -1110,7 +1111,7 @@ static bool gen_cmpxchg(DisasContext *ctx, arg_atomic *a, MemOp mop)
>      TCGv src1 = get_address(ctx, a->rs1, 0);
>      TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>
> -    decode_save_opc(ctx);
> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>      tcg_gen_atomic_cmpxchg_tl(dest, src1, dest, src2, ctx->mem_idx, mop);
>
>      gen_set_gpr(ctx, a->rd, dest);
> --
> 2.44.0
>
>

[PATCH v11 16/20] target/riscv: implement zicfiss instructions

[Deepak Gupta]

zicfiss has following instructions
 - sspopchk: pops a value from shadow stack and compares with x1/x5.
   If they dont match, reports a sw check exception with tval = 3.
 - sspush: pushes value in x1/x5 on shadow stack
 - ssrdp: reads current shadow stack
 - ssamoswap: swaps contents of shadow stack atomically

sspopchk/sspush/ssrdp default to zimop if zimop implemented and SSE=0

If SSE=0, ssamoswap is illegal instruction exception.

This patch implements shadow stack operations for qemu-user and shadow
stack is not protected.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
---
 target/riscv/cpu_bits.h                       |  2 +
 target/riscv/insn32.decode                    | 21 +++++-
 target/riscv/insn_trans/trans_rva.c.inc       | 39 ++++++++++
 target/riscv/insn_trans/trans_rvzicfiss.c.inc | 75 +++++++++++++++++++
 target/riscv/translate.c                      |  5 ++
 5 files changed, 140 insertions(+), 2 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvzicfiss.c.inc

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 48ce24dc32..bb62fbe9ec 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -690,6 +690,8 @@ typedef enum RISCVException {
 
 /* zicfilp defines lp violation results in sw check with tval = 2*/
 #define RISCV_EXCP_SW_CHECK_FCFI_TVAL      2
+/* zicfiss defines ss violation results in sw check with tval = 3*/
+#define RISCV_EXCP_SW_CHECK_BCFI_TVAL      3
 
 #define RISCV_EXCP_INT_FLAG                0x80000000
 #define RISCV_EXCP_INT_MASK                0x7fffffff
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 27108b992b..e9139ec1b9 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -246,6 +246,7 @@ remud    0000001 .....  ..... 111 ..... 1111011 @r
 lr_w       00010 . . 00000 ..... 010 ..... 0101111 @atom_ld
 sc_w       00011 . . ..... ..... 010 ..... 0101111 @atom_st
 amoswap_w  00001 . . ..... ..... 010 ..... 0101111 @atom_st
+ssamoswap_w 01001 . . ..... ..... 010 ..... 0101111 @atom_st
 amoadd_w   00000 . . ..... ..... 010 ..... 0101111 @atom_st
 amoxor_w   00100 . . ..... ..... 010 ..... 0101111 @atom_st
 amoand_w   01100 . . ..... ..... 010 ..... 0101111 @atom_st
@@ -259,6 +260,7 @@ amomaxu_w  11100 . . ..... ..... 010 ..... 0101111 @atom_st
 lr_d       00010 . . 00000 ..... 011 ..... 0101111 @atom_ld
 sc_d       00011 . . ..... ..... 011 ..... 0101111 @atom_st
 amoswap_d  00001 . . ..... ..... 011 ..... 0101111 @atom_st
+ssamoswap_d 01001 . . ..... ..... 011 ..... 0101111 @atom_st
 amoadd_d   00000 . . ..... ..... 011 ..... 0101111 @atom_st
 amoxor_d   00100 . . ..... ..... 011 ..... 0101111 @atom_st
 amoand_d   01100 . . ..... ..... 011 ..... 0101111 @atom_st
@@ -1022,8 +1024,23 @@ amocas_d    00101 . . ..... ..... 011 ..... 0101111 @atom_st
 amocas_q    00101 . . ..... ..... 100 ..... 0101111 @atom_st
 
 # *** Zimop may-be-operation extension ***
-mop_r_n     1 . 00 .. 0111 .. ..... 100 ..... 1110011 @mop5
-mop_rr_n    1 . 00 .. 1 ..... ..... 100 ..... 1110011 @mop3
+{
+  # zicfiss instructions carved out of mop.r
+  [
+    ssrdp     1100110 11100 00000 100 rd:5  1110011
+    sspopchk  1100110 11100 00001 100 00000 1110011 &r2 rs1=1 rd=0
+    sspopchk  1100110 11100 00101 100 00000 1110011 &r2 rs1=5 rd=0
+  ]
+  mop_r_n    1 . 00 .. 0111 .. ..... 100 ..... 1110011 @mop5
+}
+{
+  # zicfiss instruction carved out of mop.rr
+  [
+    sspush    1100111 00001  00000 100 00000 1110011 &r2_s rs2=1 rs1=0
+    sspush    1100111 00101  00000 100 00000 1110011 &r2_s rs2=5 rs1=0
+  ]
+  mop_rr_n   1 . 00 .. 1 ..... ..... 100 ..... 1110011 @mop3
+}
 
 # *** Zabhb Standard Extension ***
 amoswap_b  00001 . . ..... ..... 000 ..... 0101111 @atom_st
diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
index 9cf3ae8019..a2119393a6 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -114,6 +114,25 @@ static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
     return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TESL);
 }
 
+static bool trans_ssamoswap_w(DisasContext *ctx, arg_amoswap_w *a)
+{
+    REQUIRE_A_OR_ZAAMO(ctx);
+    if (!ctx->bcfi_enabled) {
+        return false;
+    }
+
+    TCGv dest = dest_gpr(ctx, a->rd);
+    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
+
+    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
+    src1 = get_address(ctx, a->rs1, 0);
+
+    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
+                           (MO_ALIGN | MO_TESL));
+    gen_set_gpr(ctx, a->rd, dest);
+    return true;
+}
+
 static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)
 {
     REQUIRE_A_OR_ZAAMO(ctx);
@@ -183,6 +202,26 @@ static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
     return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TEUQ);
 }
 
+static bool trans_ssamoswap_d(DisasContext *ctx, arg_amoswap_w *a)
+{
+    REQUIRE_64BIT(ctx);
+    REQUIRE_A_OR_ZAAMO(ctx);
+    if (!ctx->bcfi_enabled) {
+        return false;
+    }
+
+    TCGv dest = dest_gpr(ctx, a->rd);
+    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
+
+    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
+    src1 = get_address(ctx, a->rs1, 0);
+
+    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
+                           (MO_ALIGN | MO_TESQ));
+    gen_set_gpr(ctx, a->rd, dest);
+    return true;
+}
+
 static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)
 {
     REQUIRE_64BIT(ctx);
diff --git a/target/riscv/insn_trans/trans_rvzicfiss.c.inc b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
new file mode 100644
index 0000000000..741459003d
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
@@ -0,0 +1,75 @@
+/*
+ * RISC-V translation routines for the Control-Flow Integrity Extension
+ *
+ * Copyright (c) 2024 Rivos Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+static bool trans_sspopchk(DisasContext *ctx, arg_sspopchk *a)
+{
+    if (!ctx->bcfi_enabled) {
+        return false;
+    }
+
+    TCGv addr = tcg_temp_new();
+    TCGLabel *skip = gen_new_label();
+    uint32_t tmp = (get_xl(ctx) == MXL_RV64) ? 8 : 4;
+    TCGv data = tcg_temp_new();
+    tcg_gen_ld_tl(addr, tcg_env, offsetof(CPURISCVState, ssp));
+    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
+    tcg_gen_qemu_ld_tl(data, addr, SS_MMU_INDEX(ctx),
+                       mxl_memop(ctx) | MO_ALIGN);
+    TCGv rs1 = get_gpr(ctx, a->rs1, EXT_NONE);
+    tcg_gen_brcond_tl(TCG_COND_EQ, data, rs1, skip);
+    tcg_gen_st_tl(tcg_constant_tl(RISCV_EXCP_SW_CHECK_BCFI_TVAL),
+                  tcg_env, offsetof(CPURISCVState, sw_check_code));
+    gen_helper_raise_exception(tcg_env,
+                  tcg_constant_i32(RISCV_EXCP_SW_CHECK));
+    gen_set_label(skip);
+    tcg_gen_addi_tl(addr, addr, tmp);
+    tcg_gen_st_tl(addr, tcg_env, offsetof(CPURISCVState, ssp));
+
+    return true;
+}
+
+static bool trans_sspush(DisasContext *ctx, arg_sspush *a)
+{
+    if (!ctx->bcfi_enabled) {
+        return false;
+    }
+
+    TCGv addr = tcg_temp_new();
+    int tmp = (get_xl(ctx) == MXL_RV64) ? -8 : -4;
+    TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
+    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
+    tcg_gen_ld_tl(addr, tcg_env, offsetof(CPURISCVState, ssp));
+    tcg_gen_addi_tl(addr, addr, tmp);
+    tcg_gen_qemu_st_tl(data, addr, SS_MMU_INDEX(ctx),
+                       mxl_memop(ctx) | MO_ALIGN);
+    tcg_gen_st_tl(addr, tcg_env, offsetof(CPURISCVState, ssp));
+
+    return true;
+}
+
+static bool trans_ssrdp(DisasContext *ctx, arg_ssrdp *a)
+{
+    if (!ctx->bcfi_enabled || a->rd == 0) {
+        return false;
+    }
+
+    TCGv dest = dest_gpr(ctx, a->rd);
+    tcg_gen_ld_tl(dest, tcg_env, offsetof(CPURISCVState, ssp));
+    gen_set_gpr(ctx, a->rd, dest);
+
+    return true;
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index e677062a10..2753c154ba 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -144,6 +144,8 @@ static inline bool has_ext(DisasContext *ctx, uint32_t ext)
 #define get_address_xl(ctx)    ((ctx)->address_xl)
 #endif
 
+#define mxl_memop(ctx) ((get_xl(ctx) + 1) | MO_TE)
+
 /* The word size for this machine mode. */
 static inline int __attribute__((unused)) get_xlen(DisasContext *ctx)
 {
@@ -1127,6 +1129,8 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
     return translator_ldl(env, &ctx->base, pc);
 }
 
+#define SS_MMU_INDEX(ctx) (ctx->mem_idx | MMU_IDX_SS_WRITE)
+
 /* Include insn module translation function */
 #include "insn_trans/trans_rvi.c.inc"
 #include "insn_trans/trans_rvm.c.inc"
@@ -1157,6 +1161,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
 #include "decode-insn16.c.inc"
 #include "insn_trans/trans_rvzce.c.inc"
 #include "insn_trans/trans_rvzcmop.c.inc"
+#include "insn_trans/trans_rvzicfiss.c.inc"
 
 /* Include decoders for factored-out extensions */
 #include "decode-XVentanaCondOps.c.inc"
-- 
2.44.0

Re: [PATCH v11 16/20] target/riscv: implement zicfiss instructions

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:53 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> zicfiss has following instructions
>  - sspopchk: pops a value from shadow stack and compares with x1/x5.
>    If they dont match, reports a sw check exception with tval = 3.
>  - sspush: pushes value in x1/x5 on shadow stack
>  - ssrdp: reads current shadow stack
>  - ssamoswap: swaps contents of shadow stack atomically
>
> sspopchk/sspush/ssrdp default to zimop if zimop implemented and SSE=0
>
> If SSE=0, ssamoswap is illegal instruction exception.
>
> This patch implements shadow stack operations for qemu-user and shadow
> stack is not protected.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Co-developed-by: Jim Shu <jim.shu@sifive.com>
> Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
> ---
>  target/riscv/cpu_bits.h                       |  2 +
>  target/riscv/insn32.decode                    | 21 +++++-
>  target/riscv/insn_trans/trans_rva.c.inc       | 39 ++++++++++
>  target/riscv/insn_trans/trans_rvzicfiss.c.inc | 75 +++++++++++++++++++
>  target/riscv/translate.c                      |  5 ++
>  5 files changed, 140 insertions(+), 2 deletions(-)
>  create mode 100644 target/riscv/insn_trans/trans_rvzicfiss.c.inc
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 48ce24dc32..bb62fbe9ec 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -690,6 +690,8 @@ typedef enum RISCVException {
>
>  /* zicfilp defines lp violation results in sw check with tval = 2*/
>  #define RISCV_EXCP_SW_CHECK_FCFI_TVAL      2
> +/* zicfiss defines ss violation results in sw check with tval = 3*/
> +#define RISCV_EXCP_SW_CHECK_BCFI_TVAL      3
>
>  #define RISCV_EXCP_INT_FLAG                0x80000000
>  #define RISCV_EXCP_INT_MASK                0x7fffffff
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 27108b992b..e9139ec1b9 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -246,6 +246,7 @@ remud    0000001 .....  ..... 111 ..... 1111011 @r
>  lr_w       00010 . . 00000 ..... 010 ..... 0101111 @atom_ld
>  sc_w       00011 . . ..... ..... 010 ..... 0101111 @atom_st
>  amoswap_w  00001 . . ..... ..... 010 ..... 0101111 @atom_st
> +ssamoswap_w 01001 . . ..... ..... 010 ..... 0101111 @atom_st
>  amoadd_w   00000 . . ..... ..... 010 ..... 0101111 @atom_st
>  amoxor_w   00100 . . ..... ..... 010 ..... 0101111 @atom_st
>  amoand_w   01100 . . ..... ..... 010 ..... 0101111 @atom_st
> @@ -259,6 +260,7 @@ amomaxu_w  11100 . . ..... ..... 010 ..... 0101111 @atom_st
>  lr_d       00010 . . 00000 ..... 011 ..... 0101111 @atom_ld
>  sc_d       00011 . . ..... ..... 011 ..... 0101111 @atom_st
>  amoswap_d  00001 . . ..... ..... 011 ..... 0101111 @atom_st
> +ssamoswap_d 01001 . . ..... ..... 011 ..... 0101111 @atom_st
>  amoadd_d   00000 . . ..... ..... 011 ..... 0101111 @atom_st
>  amoxor_d   00100 . . ..... ..... 011 ..... 0101111 @atom_st
>  amoand_d   01100 . . ..... ..... 011 ..... 0101111 @atom_st
> @@ -1022,8 +1024,23 @@ amocas_d    00101 . . ..... ..... 011 ..... 0101111 @atom_st
>  amocas_q    00101 . . ..... ..... 100 ..... 0101111 @atom_st
>
>  # *** Zimop may-be-operation extension ***
> -mop_r_n     1 . 00 .. 0111 .. ..... 100 ..... 1110011 @mop5
> -mop_rr_n    1 . 00 .. 1 ..... ..... 100 ..... 1110011 @mop3
> +{
> +  # zicfiss instructions carved out of mop.r
> +  [
> +    ssrdp     1100110 11100 00000 100 rd:5  1110011
> +    sspopchk  1100110 11100 00001 100 00000 1110011 &r2 rs1=1 rd=0
> +    sspopchk  1100110 11100 00101 100 00000 1110011 &r2 rs1=5 rd=0
> +  ]
> +  mop_r_n    1 . 00 .. 0111 .. ..... 100 ..... 1110011 @mop5
> +}
> +{
> +  # zicfiss instruction carved out of mop.rr
> +  [
> +    sspush    1100111 00001  00000 100 00000 1110011 &r2_s rs2=1 rs1=0
> +    sspush    1100111 00101  00000 100 00000 1110011 &r2_s rs2=5 rs1=0
> +  ]
> +  mop_rr_n   1 . 00 .. 1 ..... ..... 100 ..... 1110011 @mop3
> +}
>
>  # *** Zabhb Standard Extension ***
>  amoswap_b  00001 . . ..... ..... 000 ..... 0101111 @atom_st
> diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
> index 9cf3ae8019..a2119393a6 100644
> --- a/target/riscv/insn_trans/trans_rva.c.inc
> +++ b/target/riscv/insn_trans/trans_rva.c.inc
> @@ -114,6 +114,25 @@ static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TESL);
>  }
>
> +static bool trans_ssamoswap_w(DisasContext *ctx, arg_amoswap_w *a)
> +{
> +    REQUIRE_A_OR_ZAAMO(ctx);
> +    if (!ctx->bcfi_enabled) {
> +        return false;
> +    }
> +
> +    TCGv dest = dest_gpr(ctx, a->rd);
> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> +
> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
> +    src1 = get_address(ctx, a->rs1, 0);
> +
> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
> +                           (MO_ALIGN | MO_TESL));
> +    gen_set_gpr(ctx, a->rd, dest);
> +    return true;
> +}
> +
>  static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)
>  {
>      REQUIRE_A_OR_ZAAMO(ctx);
> @@ -183,6 +202,26 @@ static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TEUQ);
>  }
>
> +static bool trans_ssamoswap_d(DisasContext *ctx, arg_amoswap_w *a)
> +{
> +    REQUIRE_64BIT(ctx);
> +    REQUIRE_A_OR_ZAAMO(ctx);
> +    if (!ctx->bcfi_enabled) {
> +        return false;
> +    }
> +
> +    TCGv dest = dest_gpr(ctx, a->rd);
> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> +
> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
> +    src1 = get_address(ctx, a->rs1, 0);
> +
> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
> +                           (MO_ALIGN | MO_TESQ));
> +    gen_set_gpr(ctx, a->rd, dest);
> +    return true;
> +}

Why aren't these in the rvzicfiss file?

Otherwise:

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

Alistair

> +
>  static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)
>  {
>      REQUIRE_64BIT(ctx);
> diff --git a/target/riscv/insn_trans/trans_rvzicfiss.c.inc b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
> new file mode 100644
> index 0000000000..741459003d
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_rvzicfiss.c.inc
> @@ -0,0 +1,75 @@
> +/*
> + * RISC-V translation routines for the Control-Flow Integrity Extension
> + *
> + * Copyright (c) 2024 Rivos Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +static bool trans_sspopchk(DisasContext *ctx, arg_sspopchk *a)
> +{
> +    if (!ctx->bcfi_enabled) {
> +        return false;
> +    }
> +
> +    TCGv addr = tcg_temp_new();
> +    TCGLabel *skip = gen_new_label();
> +    uint32_t tmp = (get_xl(ctx) == MXL_RV64) ? 8 : 4;
> +    TCGv data = tcg_temp_new();
> +    tcg_gen_ld_tl(addr, tcg_env, offsetof(CPURISCVState, ssp));
> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
> +    tcg_gen_qemu_ld_tl(data, addr, SS_MMU_INDEX(ctx),
> +                       mxl_memop(ctx) | MO_ALIGN);
> +    TCGv rs1 = get_gpr(ctx, a->rs1, EXT_NONE);
> +    tcg_gen_brcond_tl(TCG_COND_EQ, data, rs1, skip);
> +    tcg_gen_st_tl(tcg_constant_tl(RISCV_EXCP_SW_CHECK_BCFI_TVAL),
> +                  tcg_env, offsetof(CPURISCVState, sw_check_code));
> +    gen_helper_raise_exception(tcg_env,
> +                  tcg_constant_i32(RISCV_EXCP_SW_CHECK));
> +    gen_set_label(skip);
> +    tcg_gen_addi_tl(addr, addr, tmp);
> +    tcg_gen_st_tl(addr, tcg_env, offsetof(CPURISCVState, ssp));
> +
> +    return true;
> +}
> +
> +static bool trans_sspush(DisasContext *ctx, arg_sspush *a)
> +{
> +    if (!ctx->bcfi_enabled) {
> +        return false;
> +    }
> +
> +    TCGv addr = tcg_temp_new();
> +    int tmp = (get_xl(ctx) == MXL_RV64) ? -8 : -4;
> +    TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
> +    tcg_gen_ld_tl(addr, tcg_env, offsetof(CPURISCVState, ssp));
> +    tcg_gen_addi_tl(addr, addr, tmp);
> +    tcg_gen_qemu_st_tl(data, addr, SS_MMU_INDEX(ctx),
> +                       mxl_memop(ctx) | MO_ALIGN);
> +    tcg_gen_st_tl(addr, tcg_env, offsetof(CPURISCVState, ssp));
> +
> +    return true;
> +}
> +
> +static bool trans_ssrdp(DisasContext *ctx, arg_ssrdp *a)
> +{
> +    if (!ctx->bcfi_enabled || a->rd == 0) {
> +        return false;
> +    }
> +
> +    TCGv dest = dest_gpr(ctx, a->rd);
> +    tcg_gen_ld_tl(dest, tcg_env, offsetof(CPURISCVState, ssp));
> +    gen_set_gpr(ctx, a->rd, dest);
> +
> +    return true;
> +}
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index e677062a10..2753c154ba 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -144,6 +144,8 @@ static inline bool has_ext(DisasContext *ctx, uint32_t ext)
>  #define get_address_xl(ctx)    ((ctx)->address_xl)
>  #endif
>
> +#define mxl_memop(ctx) ((get_xl(ctx) + 1) | MO_TE)
> +
>  /* The word size for this machine mode. */
>  static inline int __attribute__((unused)) get_xlen(DisasContext *ctx)
>  {
> @@ -1127,6 +1129,8 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
>      return translator_ldl(env, &ctx->base, pc);
>  }
>
> +#define SS_MMU_INDEX(ctx) (ctx->mem_idx | MMU_IDX_SS_WRITE)
> +
>  /* Include insn module translation function */
>  #include "insn_trans/trans_rvi.c.inc"
>  #include "insn_trans/trans_rvm.c.inc"
> @@ -1157,6 +1161,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
>  #include "decode-insn16.c.inc"
>  #include "insn_trans/trans_rvzce.c.inc"
>  #include "insn_trans/trans_rvzcmop.c.inc"
> +#include "insn_trans/trans_rvzicfiss.c.inc"
>
>  /* Include decoders for factored-out extensions */
>  #include "decode-XVentanaCondOps.c.inc"
> --
> 2.44.0
>
>

Re: [PATCH v11 16/20] target/riscv: implement zicfiss instructions

[Deepak Gupta]

On Thu, Aug 29, 2024 at 10:01:30AM +1000, Alistair Francis wrote:
>On Thu, Aug 29, 2024 at 3:53 AM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> zicfiss has following instructions
>>  - sspopchk: pops a value from shadow stack and compares with x1/x5.
>>    If they dont match, reports a sw check exception with tval = 3.
>>  - sspush: pushes value in x1/x5 on shadow stack
>>  - ssrdp: reads current shadow stack
>>  - ssamoswap: swaps contents of shadow stack atomically
>>
>> sspopchk/sspush/ssrdp default to zimop if zimop implemented and SSE=0
>>
>> If SSE=0, ssamoswap is illegal instruction exception.
>>
>> This patch implements shadow stack operations for qemu-user and shadow
>> stack is not protected.
>>
>> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> Co-developed-by: Jim Shu <jim.shu@sifive.com>
>> Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
>> ---
>>  target/riscv/cpu_bits.h                       |  2 +
>>  target/riscv/insn32.decode                    | 21 +++++-
>>  target/riscv/insn_trans/trans_rva.c.inc       | 39 ++++++++++
>>  target/riscv/insn_trans/trans_rvzicfiss.c.inc | 75 +++++++++++++++++++
>>  target/riscv/translate.c                      |  5 ++
>>  5 files changed, 140 insertions(+), 2 deletions(-)
>>  create mode 100644 target/riscv/insn_trans/trans_rvzicfiss.c.inc
>>
>>  # *** Zabhb Standard Extension ***
>>  amoswap_b  00001 . . ..... ..... 000 ..... 0101111 @atom_st
>> diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
>> index 9cf3ae8019..a2119393a6 100644
>> --- a/target/riscv/insn_trans/trans_rva.c.inc
>> +++ b/target/riscv/insn_trans/trans_rva.c.inc
>> @@ -114,6 +114,25 @@ static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
>>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TESL);
>>  }
>>
>> +static bool trans_ssamoswap_w(DisasContext *ctx, arg_amoswap_w *a)
>> +{
>> +    REQUIRE_A_OR_ZAAMO(ctx);
>> +    if (!ctx->bcfi_enabled) {
>> +        return false;
>> +    }
>> +
>> +    TCGv dest = dest_gpr(ctx, a->rd);
>> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>> +
>> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>> +    src1 = get_address(ctx, a->rs1, 0);
>> +
>> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
>> +                           (MO_ALIGN | MO_TESL));
>> +    gen_set_gpr(ctx, a->rd, dest);
>> +    return true;
>> +}
>> +
>>  static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)
>>  {
>>      REQUIRE_A_OR_ZAAMO(ctx);
>> @@ -183,6 +202,26 @@ static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
>>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TEUQ);
>>  }
>>
>> +static bool trans_ssamoswap_d(DisasContext *ctx, arg_amoswap_w *a)
>> +{
>> +    REQUIRE_64BIT(ctx);
>> +    REQUIRE_A_OR_ZAAMO(ctx);
>> +    if (!ctx->bcfi_enabled) {
>> +        return false;
>> +    }
>> +
>> +    TCGv dest = dest_gpr(ctx, a->rd);
>> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>> +
>> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>> +    src1 = get_address(ctx, a->rs1, 0);
>> +
>> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
>> +                           (MO_ALIGN | MO_TESQ));
>> +    gen_set_gpr(ctx, a->rd, dest);
>> +    return true;
>> +}
>
>Why aren't these in the rvzicfiss file?

`ssamoswap` encodings are coming from (reserved) AMO encodings (and not zimop)
That's why kept it in trans_rva

>
>Otherwise:
>
>Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
>Alistair
>
>> +
>>  static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)
>>  {

Re: [PATCH v11 16/20] target/riscv: implement zicfiss instructions

[Alistair Francis]

On Thu, Aug 29, 2024 at 10:06 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> On Thu, Aug 29, 2024 at 10:01:30AM +1000, Alistair Francis wrote:
> >On Thu, Aug 29, 2024 at 3:53 AM Deepak Gupta <debug@rivosinc.com> wrote:
> >>
> >> zicfiss has following instructions
> >>  - sspopchk: pops a value from shadow stack and compares with x1/x5.
> >>    If they dont match, reports a sw check exception with tval = 3.
> >>  - sspush: pushes value in x1/x5 on shadow stack
> >>  - ssrdp: reads current shadow stack
> >>  - ssamoswap: swaps contents of shadow stack atomically
> >>
> >> sspopchk/sspush/ssrdp default to zimop if zimop implemented and SSE=0
> >>
> >> If SSE=0, ssamoswap is illegal instruction exception.
> >>
> >> This patch implements shadow stack operations for qemu-user and shadow
> >> stack is not protected.
> >>
> >> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> >> Co-developed-by: Jim Shu <jim.shu@sifive.com>
> >> Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
> >> ---
> >>  target/riscv/cpu_bits.h                       |  2 +
> >>  target/riscv/insn32.decode                    | 21 +++++-
> >>  target/riscv/insn_trans/trans_rva.c.inc       | 39 ++++++++++
> >>  target/riscv/insn_trans/trans_rvzicfiss.c.inc | 75 +++++++++++++++++++
> >>  target/riscv/translate.c                      |  5 ++
> >>  5 files changed, 140 insertions(+), 2 deletions(-)
> >>  create mode 100644 target/riscv/insn_trans/trans_rvzicfiss.c.inc
> >>
> >>  # *** Zabhb Standard Extension ***
> >>  amoswap_b  00001 . . ..... ..... 000 ..... 0101111 @atom_st
> >> diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
> >> index 9cf3ae8019..a2119393a6 100644
> >> --- a/target/riscv/insn_trans/trans_rva.c.inc
> >> +++ b/target/riscv/insn_trans/trans_rva.c.inc
> >> @@ -114,6 +114,25 @@ static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
> >>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TESL);
> >>  }
> >>
> >> +static bool trans_ssamoswap_w(DisasContext *ctx, arg_amoswap_w *a)
> >> +{
> >> +    REQUIRE_A_OR_ZAAMO(ctx);
> >> +    if (!ctx->bcfi_enabled) {
> >> +        return false;
> >> +    }
> >> +
> >> +    TCGv dest = dest_gpr(ctx, a->rd);
> >> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> >> +
> >> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
> >> +    src1 = get_address(ctx, a->rs1, 0);
> >> +
> >> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
> >> +                           (MO_ALIGN | MO_TESL));
> >> +    gen_set_gpr(ctx, a->rd, dest);
> >> +    return true;
> >> +}
> >> +
> >>  static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)
> >>  {
> >>      REQUIRE_A_OR_ZAAMO(ctx);
> >> @@ -183,6 +202,26 @@ static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
> >>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TEUQ);
> >>  }
> >>
> >> +static bool trans_ssamoswap_d(DisasContext *ctx, arg_amoswap_w *a)
> >> +{
> >> +    REQUIRE_64BIT(ctx);
> >> +    REQUIRE_A_OR_ZAAMO(ctx);
> >> +    if (!ctx->bcfi_enabled) {
> >> +        return false;
> >> +    }
> >> +
> >> +    TCGv dest = dest_gpr(ctx, a->rd);
> >> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> >> +
> >> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
> >> +    src1 = get_address(ctx, a->rs1, 0);
> >> +
> >> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
> >> +                           (MO_ALIGN | MO_TESQ));
> >> +    gen_set_gpr(ctx, a->rd, dest);
> >> +    return true;
> >> +}
> >
> >Why aren't these in the rvzicfiss file?
>
> `ssamoswap` encodings are coming from (reserved) AMO encodings (and not zimop)
> That's why kept it in trans_rva

But the instructions are defined in the rvzicfiss extension, so I feel
it makes sense to include them there

Alistair

>
> >
> >Otherwise:
> >
> >Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >
> >Alistair
> >
> >> +
> >>  static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)
> >>  {

Re: [PATCH v11 16/20] target/riscv: implement zicfiss instructions

[Deepak Gupta]

On Thu, Aug 29, 2024 at 10:07:15AM +1000, Alistair Francis wrote:
>On Thu, Aug 29, 2024 at 10:06 AM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> On Thu, Aug 29, 2024 at 10:01:30AM +1000, Alistair Francis wrote:
>> >On Thu, Aug 29, 2024 at 3:53 AM Deepak Gupta <debug@rivosinc.com> wrote:
>> >>
>> >> zicfiss has following instructions
>> >>  - sspopchk: pops a value from shadow stack and compares with x1/x5.
>> >>    If they dont match, reports a sw check exception with tval = 3.
>> >>  - sspush: pushes value in x1/x5 on shadow stack
>> >>  - ssrdp: reads current shadow stack
>> >>  - ssamoswap: swaps contents of shadow stack atomically
>> >>
>> >> sspopchk/sspush/ssrdp default to zimop if zimop implemented and SSE=0
>> >>
>> >> If SSE=0, ssamoswap is illegal instruction exception.
>> >>
>> >> This patch implements shadow stack operations for qemu-user and shadow
>> >> stack is not protected.
>> >>
>> >> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> >> Co-developed-by: Jim Shu <jim.shu@sifive.com>
>> >> Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
>> >> ---
>> >>  target/riscv/cpu_bits.h                       |  2 +
>> >>  target/riscv/insn32.decode                    | 21 +++++-
>> >>  target/riscv/insn_trans/trans_rva.c.inc       | 39 ++++++++++
>> >>  target/riscv/insn_trans/trans_rvzicfiss.c.inc | 75 +++++++++++++++++++
>> >>  target/riscv/translate.c                      |  5 ++
>> >>  5 files changed, 140 insertions(+), 2 deletions(-)
>> >>  create mode 100644 target/riscv/insn_trans/trans_rvzicfiss.c.inc
>> >>
>> >>  # *** Zabhb Standard Extension ***
>> >>  amoswap_b  00001 . . ..... ..... 000 ..... 0101111 @atom_st
>> >> diff --git a/target/riscv/insn_trans/trans_rva.c.inc b/target/riscv/insn_trans/trans_rva.c.inc
>> >> index 9cf3ae8019..a2119393a6 100644
>> >> --- a/target/riscv/insn_trans/trans_rva.c.inc
>> >> +++ b/target/riscv/insn_trans/trans_rva.c.inc
>> >> @@ -114,6 +114,25 @@ static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
>> >>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TESL);
>> >>  }
>> >>
>> >> +static bool trans_ssamoswap_w(DisasContext *ctx, arg_amoswap_w *a)
>> >> +{
>> >> +    REQUIRE_A_OR_ZAAMO(ctx);
>> >> +    if (!ctx->bcfi_enabled) {
>> >> +        return false;
>> >> +    }
>> >> +
>> >> +    TCGv dest = dest_gpr(ctx, a->rd);
>> >> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>> >> +
>> >> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>> >> +    src1 = get_address(ctx, a->rs1, 0);
>> >> +
>> >> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
>> >> +                           (MO_ALIGN | MO_TESL));
>> >> +    gen_set_gpr(ctx, a->rd, dest);
>> >> +    return true;
>> >> +}
>> >> +
>> >>  static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)
>> >>  {
>> >>      REQUIRE_A_OR_ZAAMO(ctx);
>> >> @@ -183,6 +202,26 @@ static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
>> >>      return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, MO_TEUQ);
>> >>  }
>> >>
>> >> +static bool trans_ssamoswap_d(DisasContext *ctx, arg_amoswap_w *a)
>> >> +{
>> >> +    REQUIRE_64BIT(ctx);
>> >> +    REQUIRE_A_OR_ZAAMO(ctx);
>> >> +    if (!ctx->bcfi_enabled) {
>> >> +        return false;
>> >> +    }
>> >> +
>> >> +    TCGv dest = dest_gpr(ctx, a->rd);
>> >> +    TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
>> >> +
>> >> +    decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
>> >> +    src1 = get_address(ctx, a->rs1, 0);
>> >> +
>> >> +    tcg_gen_atomic_xchg_tl(dest, src1, src2, SS_MMU_INDEX(ctx),
>> >> +                           (MO_ALIGN | MO_TESQ));
>> >> +    gen_set_gpr(ctx, a->rd, dest);
>> >> +    return true;
>> >> +}
>> >
>> >Why aren't these in the rvzicfiss file?
>>
>> `ssamoswap` encodings are coming from (reserved) AMO encodings (and not zimop)
>> That's why kept it in trans_rva
>
>But the instructions are defined in the rvzicfiss extension, so I feel
>it makes sense to include them there

Ok noted.
I'll wait for a day or two. If there are not objections from anyone else, I'll move
them to rvzicfiss.

>
>Alistair
>
>>
>> >
>> >Otherwise:
>> >
>> >Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> >
>> >Alistair
>> >
>> >> +
>> >>  static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)
>> >>  {

[PATCH v11 17/20] target/riscv: compressed encodings for sspush and sspopchk

[Deepak Gupta]

sspush/sspopchk have compressed encodings carved out of zcmops.
compressed sspush is designated as c.mop.1 while compressed sspopchk
is designated as c.mop.5.

Note that c.sspush x1 exists while c.sspush x5 doesn't. Similarly
c.sspopchk x5 exists while c.sspopchk x1 doesn't.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Co-developed-by: Jim Shu <jim.shu@sifive.com>
Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/insn16.decode | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 3953bcf82d..bf893d1c2e 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -140,6 +140,10 @@ sw                110  ... ... .. ... 00 @cs_w
 addi              000 .  .....  ..... 01 @ci
 addi              010 .  .....  ..... 01 @c_li
 {
+  # c.sspush x1 carving out of zcmops
+  sspush          011 0  00001  00000 01 &r2_s rs2=1 rs1=0
+  # c.sspopchk x5 carving out of zcmops
+  sspopchk        011 0  00101  00000 01 &r2 rs1=5 rd=0
   c_mop_n         011 0 0 n:3 1 00000 01
   illegal         011 0  -----  00000 01 # c.addi16sp and c.lui, RES nzimm=0
   addi            011 .  00010  ..... 01 @c_addi16sp
-- 
2.44.0

Re: [PATCH v11 17/20] target/riscv: compressed encodings for sspush and sspopchk

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:50 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> sspush/sspopchk have compressed encodings carved out of zcmops.
> compressed sspush is designated as c.mop.1 while compressed sspopchk
> is designated as c.mop.5.
>
> Note that c.sspush x1 exists while c.sspush x5 doesn't. Similarly
> c.sspopchk x5 exists while c.sspopchk x1 doesn't.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Co-developed-by: Jim Shu <jim.shu@sifive.com>
> Co-developed-by: Andy Chiu <andy.chiu@sifive.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

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

Alistair

> ---
>  target/riscv/insn16.decode | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
> index 3953bcf82d..bf893d1c2e 100644
> --- a/target/riscv/insn16.decode
> +++ b/target/riscv/insn16.decode
> @@ -140,6 +140,10 @@ sw                110  ... ... .. ... 00 @cs_w
>  addi              000 .  .....  ..... 01 @ci
>  addi              010 .  .....  ..... 01 @c_li
>  {
> +  # c.sspush x1 carving out of zcmops
> +  sspush          011 0  00001  00000 01 &r2_s rs2=1 rs1=0
> +  # c.sspopchk x5 carving out of zcmops
> +  sspopchk        011 0  00101  00000 01 &r2 rs1=5 rd=0
>    c_mop_n         011 0 0 n:3 1 00000 01
>    illegal         011 0  -----  00000 01 # c.addi16sp and c.lui, RES nzimm=0
>    addi            011 .  00010  ..... 01 @c_addi16sp
> --
> 2.44.0
>
>

[PATCH v11 18/20] disas/riscv: enable disassembly for zicfiss instructions

[Deepak Gupta]

Enable disassembly for sspush, sspopchk, ssrdp & ssamoswap.
Disasembly is only enabled if zimop and zicfiss ext is set to true.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 disas/riscv.c | 40 +++++++++++++++++++++++++++++++++++++++-
 disas/riscv.h |  1 +
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index c7c92acef7..5eafb7f7f3 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -975,6 +975,11 @@ typedef enum {
     rv_op_amocas_b  = 944,
     rv_op_amocas_h  = 945,
     rv_op_lpad = 946,
+    rv_op_sspush = 947,
+    rv_op_sspopchk = 948,
+    rv_op_ssrdp = 949,
+    rv_op_ssamoswap_w = 950,
+    rv_op_ssamoswap_d = 951,
 } rv_op;
 
 /* register names */
@@ -2234,6 +2239,11 @@ const rv_opcode_data rvi_opcode_data[] = {
     { "amocas.b", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
     { "amocas.h", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
     { "lpad", rv_codec_lp, rv_fmt_imm, NULL, 0, 0, 0 },
+    { "sspush", rv_codec_r, rv_fmt_rs2, NULL, 0, 0, 0 },
+    { "sspopchk", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
+    { "ssrdp", rv_codec_r, rv_fmt_rd, NULL, 0, 0, 0 },
+    { "ssamoswap.w", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
+    { "ssamoswap.d", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
 };
 
 /* CSR names */
@@ -2251,6 +2261,7 @@ static const char *csr_name(int csrno)
     case 0x0009: return "vxsat";
     case 0x000a: return "vxrm";
     case 0x000f: return "vcsr";
+    case 0x0011: return "ssp";
     case 0x0015: return "seed";
     case 0x0017: return "jvt";
     case 0x0040: return "uscratch";
@@ -3077,6 +3088,8 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
             case 66: op = rv_op_amoor_w; break;
             case 67: op = rv_op_amoor_d; break;
             case 68: op = rv_op_amoor_q; break;
+            case 74: op = rv_op_ssamoswap_w; break;
+            case 75: op = rv_op_ssamoswap_d; break;
             case 96: op = rv_op_amoand_b; break;
             case 97: op = rv_op_amoand_h; break;
             case 98: op = rv_op_amoand_w; break;
@@ -4028,7 +4041,7 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
             case 3: op = rv_op_csrrc; break;
             case 4:
                 if (dec->cfg->ext_zimop) {
-                    int imm_mop5, imm_mop3;
+                    int imm_mop5, imm_mop3, reg_num;
                     if ((extract32(inst, 22, 10) & 0b1011001111)
                         == 0b1000000111) {
                         imm_mop5 = deposit32(deposit32(extract32(inst, 20, 2),
@@ -4036,11 +4049,36 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
                                                        extract32(inst, 26, 2)),
                                              4, 1, extract32(inst, 30, 1));
                         op = rv_mop_r_0 + imm_mop5;
+                        /* if zicfiss enabled and mop5 is shadow stack */
+                        if (dec->cfg->ext_zicfiss &&
+                            ((imm_mop5 & 0b11100) == 0b11100)) {
+                                /* rs1=0 means ssrdp */
+                                if ((inst & (0b011111 << 15)) == 0) {
+                                    op = rv_op_ssrdp;
+                                }
+                                /* rd=0 means sspopchk */
+                                reg_num = (inst >> 15) & 0b011111;
+                                if (((inst & (0b011111 << 7)) == 0) &&
+                                    ((reg_num == 1) || (reg_num == 5))) {
+                                    op = rv_op_sspopchk;
+                                }
+                        }
                     } else if ((extract32(inst, 25, 7) & 0b1011001)
                                == 0b1000001) {
                         imm_mop3 = deposit32(extract32(inst, 26, 2),
                                              2, 1, extract32(inst, 30, 1));
                         op = rv_mop_rr_0 + imm_mop3;
+                        /* if zicfiss enabled and mop3 is shadow stack */
+                        if (dec->cfg->ext_zicfiss &&
+                            ((imm_mop3 & 0b111) == 0b111)) {
+                                /* rs1=0 and rd=0 means sspush */
+                                reg_num = (inst >> 20) & 0b011111;
+                                if (((inst & (0b011111 << 15)) == 0) &&
+                                    ((inst & (0b011111 << 7)) == 0) &&
+                                    ((reg_num == 1) || (reg_num == 5))) {
+                                    op = rv_op_sspush;
+                                }
+                        }
                     }
                 }
                 break;
diff --git a/disas/riscv.h b/disas/riscv.h
index 1182457aff..4895c5a301 100644
--- a/disas/riscv.h
+++ b/disas/riscv.h
@@ -224,6 +224,7 @@ enum {
 
 #define rv_fmt_none                   "O\t"
 #define rv_fmt_rs1                    "O\t1"
+#define rv_fmt_rs2                    "O\t2"
 #define rv_fmt_offset                 "O\to"
 #define rv_fmt_pred_succ              "O\tp,s"
 #define rv_fmt_rs1_rs2                "O\t1,2"
-- 
2.44.0

Re: [PATCH v11 18/20] disas/riscv: enable disassembly for zicfiss instructions

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:52 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> Enable disassembly for sspush, sspopchk, ssrdp & ssamoswap.
> Disasembly is only enabled if zimop and zicfiss ext is set to true.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>

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

Alistair

> ---
>  disas/riscv.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  disas/riscv.h |  1 +
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index c7c92acef7..5eafb7f7f3 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -975,6 +975,11 @@ typedef enum {
>      rv_op_amocas_b  = 944,
>      rv_op_amocas_h  = 945,
>      rv_op_lpad = 946,
> +    rv_op_sspush = 947,
> +    rv_op_sspopchk = 948,
> +    rv_op_ssrdp = 949,
> +    rv_op_ssamoswap_w = 950,
> +    rv_op_ssamoswap_d = 951,
>  } rv_op;
>
>  /* register names */
> @@ -2234,6 +2239,11 @@ const rv_opcode_data rvi_opcode_data[] = {
>      { "amocas.b", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
>      { "amocas.h", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
>      { "lpad", rv_codec_lp, rv_fmt_imm, NULL, 0, 0, 0 },
> +    { "sspush", rv_codec_r, rv_fmt_rs2, NULL, 0, 0, 0 },
> +    { "sspopchk", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> +    { "ssrdp", rv_codec_r, rv_fmt_rd, NULL, 0, 0, 0 },
> +    { "ssamoswap.w", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
> +    { "ssamoswap.d", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
>  };
>
>  /* CSR names */
> @@ -2251,6 +2261,7 @@ static const char *csr_name(int csrno)
>      case 0x0009: return "vxsat";
>      case 0x000a: return "vxrm";
>      case 0x000f: return "vcsr";
> +    case 0x0011: return "ssp";
>      case 0x0015: return "seed";
>      case 0x0017: return "jvt";
>      case 0x0040: return "uscratch";
> @@ -3077,6 +3088,8 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>              case 66: op = rv_op_amoor_w; break;
>              case 67: op = rv_op_amoor_d; break;
>              case 68: op = rv_op_amoor_q; break;
> +            case 74: op = rv_op_ssamoswap_w; break;
> +            case 75: op = rv_op_ssamoswap_d; break;
>              case 96: op = rv_op_amoand_b; break;
>              case 97: op = rv_op_amoand_h; break;
>              case 98: op = rv_op_amoand_w; break;
> @@ -4028,7 +4041,7 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>              case 3: op = rv_op_csrrc; break;
>              case 4:
>                  if (dec->cfg->ext_zimop) {
> -                    int imm_mop5, imm_mop3;
> +                    int imm_mop5, imm_mop3, reg_num;
>                      if ((extract32(inst, 22, 10) & 0b1011001111)
>                          == 0b1000000111) {
>                          imm_mop5 = deposit32(deposit32(extract32(inst, 20, 2),
> @@ -4036,11 +4049,36 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>                                                         extract32(inst, 26, 2)),
>                                               4, 1, extract32(inst, 30, 1));
>                          op = rv_mop_r_0 + imm_mop5;
> +                        /* if zicfiss enabled and mop5 is shadow stack */
> +                        if (dec->cfg->ext_zicfiss &&
> +                            ((imm_mop5 & 0b11100) == 0b11100)) {
> +                                /* rs1=0 means ssrdp */
> +                                if ((inst & (0b011111 << 15)) == 0) {
> +                                    op = rv_op_ssrdp;
> +                                }
> +                                /* rd=0 means sspopchk */
> +                                reg_num = (inst >> 15) & 0b011111;
> +                                if (((inst & (0b011111 << 7)) == 0) &&
> +                                    ((reg_num == 1) || (reg_num == 5))) {
> +                                    op = rv_op_sspopchk;
> +                                }
> +                        }
>                      } else if ((extract32(inst, 25, 7) & 0b1011001)
>                                 == 0b1000001) {
>                          imm_mop3 = deposit32(extract32(inst, 26, 2),
>                                               2, 1, extract32(inst, 30, 1));
>                          op = rv_mop_rr_0 + imm_mop3;
> +                        /* if zicfiss enabled and mop3 is shadow stack */
> +                        if (dec->cfg->ext_zicfiss &&
> +                            ((imm_mop3 & 0b111) == 0b111)) {
> +                                /* rs1=0 and rd=0 means sspush */
> +                                reg_num = (inst >> 20) & 0b011111;
> +                                if (((inst & (0b011111 << 15)) == 0) &&
> +                                    ((inst & (0b011111 << 7)) == 0) &&
> +                                    ((reg_num == 1) || (reg_num == 5))) {
> +                                    op = rv_op_sspush;
> +                                }
> +                        }
>                      }
>                  }
>                  break;
> diff --git a/disas/riscv.h b/disas/riscv.h
> index 1182457aff..4895c5a301 100644
> --- a/disas/riscv.h
> +++ b/disas/riscv.h
> @@ -224,6 +224,7 @@ enum {
>
>  #define rv_fmt_none                   "O\t"
>  #define rv_fmt_rs1                    "O\t1"
> +#define rv_fmt_rs2                    "O\t2"
>  #define rv_fmt_offset                 "O\to"
>  #define rv_fmt_pred_succ              "O\tp,s"
>  #define rv_fmt_rs1_rs2                "O\t1,2"
> --
> 2.44.0
>
>

[PATCH v11 19/20] disas/riscv: enable disassembly for compressed sspush/sspopchk

[Deepak Gupta]

sspush and sspopchk have equivalent compressed encoding taken from zcmop.
cmop.1 is sspush x1 while cmop.5 is sspopchk x5. Due to unusual encoding
for both rs1 and rs2 from space bitfield, this required a new codec.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 disas/riscv.c | 19 ++++++++++++++++++-
 disas/riscv.h |  1 +
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 5eafb7f7f3..6e9ba42edd 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -980,6 +980,8 @@ typedef enum {
     rv_op_ssrdp = 949,
     rv_op_ssamoswap_w = 950,
     rv_op_ssamoswap_d = 951,
+    rv_op_c_sspush = 952,
+    rv_op_c_sspopchk = 953,
 } rv_op;
 
 /* register names */
@@ -2244,6 +2246,10 @@ const rv_opcode_data rvi_opcode_data[] = {
     { "ssrdp", rv_codec_r, rv_fmt_rd, NULL, 0, 0, 0 },
     { "ssamoswap.w", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
     { "ssamoswap.d", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
+    { "c.sspush", rv_codec_cmop_ss, rv_fmt_rs2, NULL, rv_op_sspush,
+      rv_op_sspush, 0 },
+    { "c.sspopchk", rv_codec_cmop_ss, rv_fmt_rs1, NULL, rv_op_sspopchk,
+      rv_op_sspopchk, 0 },
 };
 
 /* CSR names */
@@ -2604,7 +2610,13 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
             if (dec->cfg->ext_zcmop) {
                 if ((((inst >> 2) & 0b111111) == 0b100000) &&
                     (((inst >> 11) & 0b11) == 0b0)) {
-                    op = rv_c_mop_1 + ((inst >> 8) & 0b111);
+                    unsigned int cmop_code = 0;
+                    cmop_code = ((inst >> 8) & 0b111);
+                    op = rv_c_mop_1 + cmop_code;
+                    if (dec->cfg->ext_zicfiss) {
+                        op = (cmop_code == 0) ? rv_op_c_sspush : op;
+                        op = (cmop_code == 2) ? rv_op_c_sspopchk : op;
+                    }
                     break;
                 }
             }
@@ -4923,6 +4935,11 @@ static void decode_inst_operands(rv_decode *dec, rv_isa isa)
     case rv_codec_lp:
         dec->imm = operand_lpl(inst);
         break;
+    case rv_codec_cmop_ss:
+        dec->rd = rv_ireg_zero;
+        dec->rs1 = dec->rs2 = operand_crs1(inst);
+        dec->imm = 0;
+        break;
     };
 }
 
diff --git a/disas/riscv.h b/disas/riscv.h
index 4895c5a301..6a3b371cd3 100644
--- a/disas/riscv.h
+++ b/disas/riscv.h
@@ -167,6 +167,7 @@ typedef enum {
     rv_codec_r2_imm2_imm5,
     rv_codec_fli,
     rv_codec_lp,
+    rv_codec_cmop_ss,
 } rv_codec;
 
 /* structures */
-- 
2.44.0

Re: [PATCH v11 19/20] disas/riscv: enable disassembly for compressed sspush/sspopchk

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:53 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> sspush and sspopchk have equivalent compressed encoding taken from zcmop.
> cmop.1 is sspush x1 while cmop.5 is sspopchk x5. Due to unusual encoding
> for both rs1 and rs2 from space bitfield, this required a new codec.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>

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

Alistair

> ---
>  disas/riscv.c | 19 ++++++++++++++++++-
>  disas/riscv.h |  1 +
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 5eafb7f7f3..6e9ba42edd 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -980,6 +980,8 @@ typedef enum {
>      rv_op_ssrdp = 949,
>      rv_op_ssamoswap_w = 950,
>      rv_op_ssamoswap_d = 951,
> +    rv_op_c_sspush = 952,
> +    rv_op_c_sspopchk = 953,
>  } rv_op;
>
>  /* register names */
> @@ -2244,6 +2246,10 @@ const rv_opcode_data rvi_opcode_data[] = {
>      { "ssrdp", rv_codec_r, rv_fmt_rd, NULL, 0, 0, 0 },
>      { "ssamoswap.w", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
>      { "ssamoswap.d", rv_codec_r_a, rv_fmt_aqrl_rd_rs2_rs1, NULL, 0, 0, 0 },
> +    { "c.sspush", rv_codec_cmop_ss, rv_fmt_rs2, NULL, rv_op_sspush,
> +      rv_op_sspush, 0 },
> +    { "c.sspopchk", rv_codec_cmop_ss, rv_fmt_rs1, NULL, rv_op_sspopchk,
> +      rv_op_sspopchk, 0 },
>  };
>
>  /* CSR names */
> @@ -2604,7 +2610,13 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>              if (dec->cfg->ext_zcmop) {
>                  if ((((inst >> 2) & 0b111111) == 0b100000) &&
>                      (((inst >> 11) & 0b11) == 0b0)) {
> -                    op = rv_c_mop_1 + ((inst >> 8) & 0b111);
> +                    unsigned int cmop_code = 0;
> +                    cmop_code = ((inst >> 8) & 0b111);
> +                    op = rv_c_mop_1 + cmop_code;
> +                    if (dec->cfg->ext_zicfiss) {
> +                        op = (cmop_code == 0) ? rv_op_c_sspush : op;
> +                        op = (cmop_code == 2) ? rv_op_c_sspopchk : op;
> +                    }
>                      break;
>                  }
>              }
> @@ -4923,6 +4935,11 @@ static void decode_inst_operands(rv_decode *dec, rv_isa isa)
>      case rv_codec_lp:
>          dec->imm = operand_lpl(inst);
>          break;
> +    case rv_codec_cmop_ss:
> +        dec->rd = rv_ireg_zero;
> +        dec->rs1 = dec->rs2 = operand_crs1(inst);
> +        dec->imm = 0;
> +        break;
>      };
>  }
>
> diff --git a/disas/riscv.h b/disas/riscv.h
> index 4895c5a301..6a3b371cd3 100644
> --- a/disas/riscv.h
> +++ b/disas/riscv.h
> @@ -167,6 +167,7 @@ typedef enum {
>      rv_codec_r2_imm2_imm5,
>      rv_codec_fli,
>      rv_codec_lp,
> +    rv_codec_cmop_ss,
>  } rv_codec;
>
>  /* structures */
> --
> 2.44.0
>
>

[PATCH v11 20/20] target/riscv: Expose zicfiss extension as a cpu property

[Deepak Gupta]

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 target/riscv/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c5ebcefeb5..2592465e24 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1485,6 +1485,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
     MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
     MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
     MULTI_EXT_CFG_BOOL("zicfilp", ext_zicfilp, false),
+    MULTI_EXT_CFG_BOOL("zicfiss", ext_zicfiss, false),
     MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
     MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
     MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
-- 
2.44.0

Re: [PATCH v11 20/20] target/riscv: Expose zicfiss extension as a cpu property

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:52 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>

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

Alistair

> ---
>  target/riscv/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index c5ebcefeb5..2592465e24 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1485,6 +1485,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>      MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
>      MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
>      MULTI_EXT_CFG_BOOL("zicfilp", ext_zicfilp, false),
> +    MULTI_EXT_CFG_BOOL("zicfiss", ext_zicfiss, false),
>      MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
>      MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
>      MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
> --
> 2.44.0
>
>

Re: [PATCH v11 00/20] riscv support for control flow integrity extensions

[Deepak Gupta]

On Wed, Aug 28, 2024 at 10:47:18AM -0700, Deepak Gupta wrote:
>v11 for riscv zicfilp and zicfiss extensions support in qemu.

Sorry once again, some problem in my work-flow and it missed picked up below

"
zicfilp and zicfiss spec pdf
https://github.com/riscv/riscv-cfi/releases/download/v1.0/riscv-cfi.pdf

github sources to spec
https://github.com/riscv/riscv-cfi
"

>
>Links for previous versions
>[1] - v1 https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg06017.html
>[2] - v2 https://lore.kernel.org/all/ed23bcbc-fdc4-4492-803c-daa95880375a@linaro.org/T/
>[3] - v3 https://lists.nongnu.org/archive/html/qemu-devel/2024-08/msg01005.html
>[4] - v4 https://lore.kernel.org/all/20240816010711.3055425-6-debug@rivosinc.com/T/
>[5] - v5
>+https://lore.kernel.org/all/20240820000129.3522346-1-debug@rivosinc.com/T/#m7b9cc847e739ec86f9569a3ca9f3d9377b01e21
>[6] - v6 https://mail.gnu.org/archive/html/qemu-riscv/2024-08/msg00418.html
>[7] - v7 https://lore.kernel.org/all/20240822082504.3979610-1-debug@rivosinc.com/
>[8] - v8 https://lore.kernel.org/all/20240823190140.4156920-1-debug@rivosinc.com/T/
>[9] - v9 https://lore.kernel.org/all/20240826152949.294506-1-debug@rivosinc.com/
>
>---
>v10:
>   - Exposed *envcfg CSR and priv to qemu-user as well and removed special
>     state management for *envcfg related feature enabling for qemu-user
>   - Exposing zicfilp and zicfiss as different patch
>v9:
>   - fix switch case fallthrough for sw_check excp in patch 4
>v8:
>   - fixed up `gen_cmpxchg` to store extra word2 during compile to raise storeAMO always
>v7:
>   - Updated decode_save_opc to take extra argument of excp_uw2 and
>     updated callsites
>   - added a helper for promoting load faults to store faults
>   - Removed stale comments and edited existed comments
>v6:
>   - Added support extra store word 2 for tcg compile and extraction during unwind
>   - Using extra word, AMO instructions and shadow stack instructions can raise store fault
>   - some alignment and cosmetic changes
>   - added vmstate migration support for elp and ssp cpu state
>v5:
>   - Simplified elp tracking and lpad implementation as per suggestion by richard
>   - Simplified shadow stack mmu checks as per suggestion by richard
>   - Converged zicfiss compressed and non-comressed instructions to same translation
>   - Removed trace hooks. Don't need for upstream.
>
>v4:
>   - elp state in cpu is true/false instead of enum and elp cleared
>     unconditionally on trap entry. elp in *status cleared unconditionally on
>     trap return.
>   - Moved logic for branch tracking in instruction translation from tb_start.
>   - fixed zicfiss dependency on 'A'
>   - `cpu_get_fcfien/bcfien` helpers checks fixed to check for extension first.
>   - removed trace hook enums. Instead added dedicated trace helpers wherever needed.
>   - fixed/simplified instruction format in decoder for lpad, sspush, sspopchk
>   - simplified tlb index logic for shadow stack instructions. Removed SUM TB_FLAG
>   - access to ssp CSR is gated on `cpu_get_bcfien` instead of duplicated logic
>   - removed vDSO related changes for now.
>v3:
>   - Removed prctl specific patches because they need to be upstream
>     in kernel first.
>   - As suggested by Richard, added TB flag if fcfi enabled
>   - Re-worked translation for landing pad and shadow stack instructions
>     to not require helper.
>   - tcg helpers only for cfi violation cases so that trace hooks can be
>     placed.
>   - Style changes.
>   - fixes assert condition in accel/tcg
>
>v2:
>   - added missed file (in v1) for shadow stack instructions implementation.
>
>Deepak Gupta (20):
>  target/riscv: expose *envcfg csr and priv to qemu-user as well
>  target/riscv: Add zicfilp extension
>  target/riscv: Introduce elp state and enabling controls for zicfilp
>  target/riscv: save and restore elp state on priv transitions
>  target/riscv: additional code information for sw check
>  target/riscv: tracking indirect branches (fcfi) for zicfilp
>  target/riscv: zicfilp `lpad` impl and branch tracking
>  disas/riscv: enable `lpad` disassembly
>  target/riscv: Expose zicfilp extension as a cpu property
>  target/riscv: Add zicfiss extension
>  target/riscv: introduce ssp and enabling controls for zicfiss
>  target/riscv: tb flag for shadow stack  instructions
>  target/riscv: mmu changes for zicfiss shadow stack protection
>  target/riscv: AMO operations always raise store/AMO fault
>  target/riscv: update `decode_save_opc` to store extra word2
>  target/riscv: implement zicfiss instructions
>  target/riscv: compressed encodings for sspush and sspopchk
>  disas/riscv: enable disassembly for zicfiss instructions
>  disas/riscv: enable disassembly for compressed sspush/sspopchk
>  target/riscv: Expose zicfiss extension as a cpu property
>
> disas/riscv.c                                 |  77 ++++++++-
> disas/riscv.h                                 |   4 +
> target/riscv/cpu.c                            |  14 ++
> target/riscv/cpu.h                            |  31 +++-
> target/riscv/cpu_bits.h                       |  17 ++
> target/riscv/cpu_cfg.h                        |   2 +
> target/riscv/cpu_helper.c                     | 155 +++++++++++++++++-
> target/riscv/cpu_user.h                       |   1 +
> target/riscv/csr.c                            |  84 ++++++++++
> target/riscv/insn16.decode                    |   4 +
> target/riscv/insn32.decode                    |  26 ++-
> .../riscv/insn_trans/trans_privileged.c.inc   |   8 +-
> target/riscv/insn_trans/trans_rva.c.inc       |  43 ++++-
> target/riscv/insn_trans/trans_rvd.c.inc       |   4 +-
> target/riscv/insn_trans/trans_rvf.c.inc       |   4 +-
> target/riscv/insn_trans/trans_rvh.c.inc       |   8 +-
> target/riscv/insn_trans/trans_rvi.c.inc       |  61 ++++++-
> target/riscv/insn_trans/trans_rvvk.c.inc      |  10 +-
> target/riscv/insn_trans/trans_rvzacas.c.inc   |   4 +-
> target/riscv/insn_trans/trans_rvzfh.c.inc     |   4 +-
> target/riscv/insn_trans/trans_rvzicfiss.c.inc |  75 +++++++++
> target/riscv/insn_trans/trans_svinval.c.inc   |   6 +-
> target/riscv/internals.h                      |   3 +
> target/riscv/machine.c                        |  38 +++++
> target/riscv/op_helper.c                      |  18 ++
> target/riscv/pmp.c                            |   5 +
> target/riscv/pmp.h                            |   3 +-
> target/riscv/tcg/tcg-cpu.c                    |  25 +++
> target/riscv/translate.c                      |  45 ++++-
> 29 files changed, 726 insertions(+), 53 deletions(-)
> create mode 100644 target/riscv/insn_trans/trans_rvzicfiss.c.inc
>
>-- 
>2.44.0
>