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

Deepak Gupta posted 20 patches 1 month ago
[PATCH v16 04/20] target/riscv: save and restore elp state on priv transitions
Posted by Deepak Gupta 1 month ago
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).

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  | 17 ++++++++++++
 3 files changed, 72 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index e9f26b5121..6c5e199e72 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -547,6 +547,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 a935377b4a..d7b776c556 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) {
@@ -1760,6 +1797,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;
 
@@ -1808,6 +1850,11 @@ void riscv_cpu_do_interrupt(CPUState *cs)
         riscv_cpu_set_mode(env, PRV_S, virt);
     } 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);
@@ -1839,6 +1886,13 @@ void riscv_cpu_do_interrupt(CPUState *cs)
         riscv_cpu_set_mode(env, PRV_M, virt);
     }
 
+    /*
+     * 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 25a5263573..eddedacf4b 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -309,6 +309,15 @@ target_ulong helper_sret(CPURISCVState *env)
 
     riscv_cpu_set_mode(env, prev_priv, prev_virt);
 
+    /*
+     * 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;
 }
 
@@ -349,6 +358,14 @@ target_ulong helper_mret(CPURISCVState *env)
     }
 
     riscv_cpu_set_mode(env, prev_priv, 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.45.0
Re: [PATCH v16 04/20] target/riscv: save and restore elp state on priv transitions
Posted by Alistair Francis 2 weeks, 2 days ago
On Wed, Oct 9, 2024 at 8:52 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> 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).
>
> 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.h        |  1 +
>  target/riscv/cpu_helper.c | 54 +++++++++++++++++++++++++++++++++++++++
>  target/riscv/op_helper.c  | 17 ++++++++++++
>  3 files changed, 72 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e9f26b5121..6c5e199e72 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -547,6 +547,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 a935377b4a..d7b776c556 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) {
> @@ -1760,6 +1797,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;
>
> @@ -1808,6 +1850,11 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>          riscv_cpu_set_mode(env, PRV_S, virt);
>      } 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);
> @@ -1839,6 +1886,13 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>          riscv_cpu_set_mode(env, PRV_M, virt);
>      }
>
> +    /*
> +     * 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 25a5263573..eddedacf4b 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -309,6 +309,15 @@ target_ulong helper_sret(CPURISCVState *env)
>
>      riscv_cpu_set_mode(env, prev_priv, prev_virt);
>
> +    /*
> +     * 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;
>  }
>
> @@ -349,6 +358,14 @@ target_ulong helper_mret(CPURISCVState *env)
>      }
>
>      riscv_cpu_set_mode(env, prev_priv, 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.45.0
>
>