[PATCH v3 10/20] target/riscv: introduce ssp and enabling controls for zicfiss

Deepak Gupta posted 20 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v3 10/20] target/riscv: introduce ssp and enabling controls for zicfiss
Posted by Deepak Gupta 3 months, 2 weeks ago
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.

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.c      |  3 ++
 target/riscv/cpu.h      |  2 ++
 target/riscv/cpu_bits.h |  6 ++++
 target/riscv/csr.c      | 74 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 85 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 54fcf380ff..6b50ae0e45 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -998,6 +998,9 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
     /* on reset elp is set to NO_LP_EXPECTED */
     env->elp = NO_LP_EXPECTED;
 
+    /* on reset ssp is set to 0 */
+    env->ssp = 0;
+
     /*
      * Bits 10, 6, 2 and 12 of mideleg are read only 1 when the Hypervisor
      * extension is enabled.
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index b77481428f..53b005b34c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -224,6 +224,8 @@ struct CPUArchState {
 
     /* elp state for zicfilp extension */
     cfi_elp      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
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 2c585a63c2..226157896d 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
@@ -766,6 +769,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)
@@ -780,12 +784,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/csr.c b/target/riscv/csr.c
index a5a969a377..d72d6289fb 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -185,6 +185,47 @@ static RISCVException zcmt(CPURISCVState *env, int csrno)
     return RISCV_EXCP_NONE;
 }
 
+static RISCVException cfi_ss(CPURISCVState *env, int csrno)
+{
+    /* no cfi extension, access to csr is illegal */
+    if (!env_archcpu(env)->cfg.ext_zicfiss) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+    /*
+     * CONFIG_USER_MODE always allow access for now. Better for user mode only
+     * functionality
+     */
+#if !defined(CONFIG_USER_ONLY)
+    if (env->debugger) {
+        return RISCV_EXCP_NONE;
+    }
+    /* current priv not M */
+    if (env->priv != PRV_M) {
+        /* menvcfg says no shadow stack enable */
+        if (!get_field(env->menvcfg, MENVCFG_SSE)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+
+        /* V = 1 and henvcfg says no shadow stack enable */
+        if (env->virt_enabled &&
+            !get_field(env->henvcfg, HENVCFG_SSE)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+
+        /*
+         * SSP are not accessible to U mode if disabled via senvcfg
+         * CSR
+         */
+        if ((env->priv == PRV_U) &&
+            (!get_field(env->senvcfg, SENVCFG_SSE))) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+    }
+#endif
+
+    return RISCV_EXCP_NONE;
+}
+
 #if !defined(CONFIG_USER_ONLY)
 static RISCVException mctr(CPURISCVState *env, int csrno)
 {
@@ -596,6 +637,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 +2165,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 +2225,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 +2273,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 +4734,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,
-- 
2.44.0
Re: [PATCH v3 10/20] target/riscv: introduce ssp and enabling controls for zicfiss
Posted by Richard Henderson 3 months, 2 weeks ago
On 8/7/24 10:06, Deepak Gupta wrote:
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a5a969a377..d72d6289fb 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -185,6 +185,47 @@ static RISCVException zcmt(CPURISCVState *env, int csrno)
>       return RISCV_EXCP_NONE;
>   }
>   
> +static RISCVException cfi_ss(CPURISCVState *env, int csrno)
> +{
> +    /* no cfi extension, access to csr is illegal */
> +    if (!env_archcpu(env)->cfg.ext_zicfiss) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +    /*
> +     * CONFIG_USER_MODE always allow access for now. Better for user mode only
> +     * functionality
> +     */

In the next patch you add ubcfien, which would apply here.


r~
Re: [PATCH v3 10/20] target/riscv: introduce ssp and enabling controls for zicfiss
Posted by Richard Henderson 3 months, 2 weeks ago
On 8/7/24 12:11, Richard Henderson wrote:
> On 8/7/24 10:06, Deepak Gupta wrote:
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index a5a969a377..d72d6289fb 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -185,6 +185,47 @@ static RISCVException zcmt(CPURISCVState *env, int csrno)
>>       return RISCV_EXCP_NONE;
>>   }
>> +static RISCVException cfi_ss(CPURISCVState *env, int csrno)
>> +{
>> +    /* no cfi extension, access to csr is illegal */
>> +    if (!env_archcpu(env)->cfg.ext_zicfiss) {
>> +        return RISCV_EXCP_ILLEGAL_INST;
>> +    }
>> +    /*
>> +     * CONFIG_USER_MODE always allow access for now. Better for user mode only
>> +     * functionality
>> +     */
> 
> In the next patch you add ubcfien, which would apply here.

... anyway, surely cpu_get_bcfien() is the right check anyway?


r~

Re: [PATCH v3 10/20] target/riscv: introduce ssp and enabling controls for zicfiss
Posted by Deepak Gupta 3 months, 2 weeks ago
On Wed, Aug 07, 2024 at 12:12:52PM +1000, Richard Henderson wrote:
>On 8/7/24 12:11, Richard Henderson wrote:
>>On 8/7/24 10:06, Deepak Gupta wrote:
>>>diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>>>index a5a969a377..d72d6289fb 100644
>>>--- a/target/riscv/csr.c
>>>+++ b/target/riscv/csr.c
>>>@@ -185,6 +185,47 @@ static RISCVException zcmt(CPURISCVState *env, int csrno)
>>>      return RISCV_EXCP_NONE;
>>>  }
>>>+static RISCVException cfi_ss(CPURISCVState *env, int csrno)
>>>+{
>>>+    /* no cfi extension, access to csr is illegal */
>>>+    if (!env_archcpu(env)->cfg.ext_zicfiss) {
>>>+        return RISCV_EXCP_ILLEGAL_INST;
>>>+    }
>>>+    /*
>>>+     * CONFIG_USER_MODE always allow access for now. Better for user mode only
>>>+     * functionality
>>>+     */
>>
>>In the next patch you add ubcfien, which would apply here.
>
>... anyway, surely cpu_get_bcfien() is the right check anyway?

Yeah you're right, `cpu_get_bcfien()` works and simplify it. will fix it.
>
>
>r~