diff mbox series

[v12,11/20] target/riscv: introduce ssp and enabling controls for zicfiss

Message ID 20240829233425.1005029-12-debug@rivosinc.com
State New
Headers show
Series riscv support for control flow integrity extensions | expand

Commit Message

Deepak Gupta Aug. 29, 2024, 11:34 p.m. UTC 
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>
---
 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(+)

Comments

Richard Henderson Aug. 30, 2024, 5:20 a.m. UTC | #1 
On 8/30/24 09:34, Deepak Gupta wrote:
> +bool cpu_get_bcfien(CPURISCVState *env)

It occurs to me that a better name would be "cpu_get_sspen".
The backward cfi is merely a consequence of the shadow stack.

> +{
> +    /* 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;

 From the manual:

Activating Zicfiss in M-mode is currently not supported. Additionally, when S-mode is not
implemented, activation in U-mode is also not supported.

So two of the cases above are wrong.


r~
Deepak Gupta Aug. 30, 2024, 5:56 a.m. UTC | #2 
On Fri, Aug 30, 2024 at 03:20:04PM +1000, Richard Henderson wrote:
>On 8/30/24 09:34, Deepak Gupta wrote:
>>+bool cpu_get_bcfien(CPURISCVState *env)
>
>It occurs to me that a better name would be "cpu_get_sspen".
>The backward cfi is merely a consequence of the shadow stack.

Want me to change cpu_get_fcfien as well to cpu_get_lpen ?

>
>>+{
>>+    /* 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;
>
>From the manual:
>
>Activating Zicfiss in M-mode is currently not supported. Additionally, when S-mode is not
>implemented, activation in U-mode is also not supported.

Hmm. This is a good catch.
It seems I was using an earlier spec and missed this. Or atleast
I didn't bother to check assuming that spec didn't change.
Thanks for catching it and pointing it out.

I'll fix the M-mode case by return false.

For case of,
If S-mode is not implemented then activation in U-mode is also not supported. I am thinking of
making this check during extensions validation (in case user turned zicfiss=true).
Below options:

Option 1
In `riscv_cpu_validate_set_extensions`, check for S and if S is not implemented then error out
saying that zicfiss can't be turned on.

OR

Option 2
In `riscv_cpu_validate_set_extensions`, check for S and if S is not implemented then silently
clear `ext_zicfiss` in cpu config.

I think option 1 is better.

>
>So two of the cases above are wrong.
>
>
>r~
Deepak Gupta Aug. 30, 2024, 4:30 p.m. UTC | #3 
On Thu, Aug 29, 2024 at 10:56:41PM -0700, Deepak Gupta wrote:
>On Fri, Aug 30, 2024 at 03:20:04PM +1000, Richard Henderson wrote:
>>On 8/30/24 09:34, Deepak Gupta wrote:
>>>+bool cpu_get_bcfien(CPURISCVState *env)
>>
>>It occurs to me that a better name would be "cpu_get_sspen".
>>The backward cfi is merely a consequence of the shadow stack.

On a second thought, I would like to keep to keep `cpu_get_bcfien`
We use this helper in `cpu_get_tb_cpu_state`, `get_physical_address`
and `cfi_ss`. In 2 out of 3 functions, intent is to check whether backcfi
is enabled or not and based on set TB state or perform appropriate page
table walk. Only in case of whether access to ssp is allowed or not
(i.e. `cfi_ss`), intent is sspen or not.

Let me know if you feel strongly against it.

>
>Want me to change cpu_get_fcfien as well to cpu_get_lpen ?
>
>>
>>>+{
>>>+    /* 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;
diff mbox series

Patch

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