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

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
>
>

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)
>>  {

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)
> >>  {

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)
>> >>  {