[PATCH v3 21/34] target/riscv: Fix size of priv

Anton Johansson via posted 34 patches 3 months, 4 weeks ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Laurent Vivier <laurent@vivier.eu>, Christoph Muellner <christoph.muellner@vrull.eu>
There is a newer version of this series
[PATCH v3 21/34] target/riscv: Fix size of priv
Posted by Anton Johansson via 3 months, 4 weeks ago
The priv field of CPUArchState only stores values in the range [0,3],
fix to 8 bits in size and update relevant function arguments.  Introduce
a new privilege_mode_t typedef for passing around the privilege mode.

Signed-off-by: Anton Johansson <anjo@rev.ng>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/riscv/cpu.h        | 15 +++++++++++----
 target/riscv/cpu_helper.c | 12 +++++++-----
 target/riscv/machine.c    |  2 +-
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 37035a9541..1d5d74f11b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -109,6 +109,12 @@ typedef struct riscv_cpu_profile {
 
 extern RISCVCPUProfile *riscv_profiles[];
 
+/*
+ * Type large enough to hold all PRV_* fields, update CPUArchState::priv
+ * migration field if changing this type.
+ */
+typedef uint8_t privilege_mode_t;
+
 /* Privileged specification version */
 #define PRIV_VER_1_10_0_STR "v1.10.0"
 #define PRIV_VER_1_11_0_STR "v1.11.0"
@@ -264,7 +270,7 @@ struct CPUArchState {
     uint32_t elf_flags;
 #endif
 
-    target_ulong priv;
+    privilege_mode_t priv;
     /* CSRs for execution environment configuration */
     uint64_t menvcfg;
     uint64_t senvcfg;
@@ -650,10 +656,11 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
 RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit);
 #endif /* !CONFIG_USER_ONLY */
 
-void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en);
+void riscv_cpu_set_mode(CPURISCVState *env, privilege_mode_t newpriv,
+                        bool virt_en);
 
 void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
-    enum CTRType type, target_ulong prev_priv, bool prev_virt);
+    enum CTRType type, privilege_mode_t prev_priv, bool prev_virt);
 void riscv_ctr_clear(CPURISCVState *env);
 
 void riscv_translate_init(void);
@@ -724,7 +731,7 @@ static inline int cpu_address_mode(CPURISCVState *env)
     return mode;
 }
 
-static inline RISCVMXL cpu_get_xl(CPURISCVState *env, target_ulong mode)
+static inline RISCVMXL cpu_get_xl(CPURISCVState *env, privilege_mode_t mode)
 {
     RISCVMXL xl = env->misa_mxl;
     /*
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 7790244d93..26c3c846a5 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -799,7 +799,7 @@ void riscv_ctr_clear(CPURISCVState *env)
     memset(env->ctr_data, 0x0, sizeof(env->ctr_data));
 }
 
-static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
+static uint64_t riscv_ctr_priv_to_mask(privilege_mode_t priv, bool virt)
 {
     switch (priv) {
     case PRV_M:
@@ -819,7 +819,8 @@ static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
     g_assert_not_reached();
 }
 
-static uint64_t riscv_ctr_get_control(CPURISCVState *env, target_long priv,
+static uint64_t riscv_ctr_get_control(CPURISCVState *env,
+                                      privilege_mode_t priv,
                                       bool virt)
 {
     switch (priv) {
@@ -841,7 +842,8 @@ static uint64_t riscv_ctr_get_control(CPURISCVState *env, target_long priv,
  * and src privilege is less than target privilege. This includes the virtual
  * state as well.
  */
-static bool riscv_ctr_check_xte(CPURISCVState *env, target_long src_prv,
+static bool riscv_ctr_check_xte(CPURISCVState *env,
+                                privilege_mode_t src_prv,
                                 bool src_virt)
 {
     target_long tgt_prv = env->priv;
@@ -930,7 +932,7 @@ static bool riscv_ctr_check_xte(CPURISCVState *env, target_long src_prv,
  *    idx = (sctrstatus.WRPTR - entry - 1) & (depth - 1);
  */
 void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
-    enum CTRType type, target_ulong src_priv, bool src_virt)
+    enum CTRType type, privilege_mode_t src_priv, bool src_virt)
 {
     bool tgt_virt = env->virt_enabled;
     uint64_t src_mask = riscv_ctr_priv_to_mask(src_priv, src_virt);
@@ -1028,7 +1030,7 @@ void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
     env->sctrstatus = set_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK, head);
 }
 
-void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
+void riscv_cpu_set_mode(CPURISCVState *env, privilege_mode_t newpriv, bool virt_en)
 {
     g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
 
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index c55794c554..ce5e44325d 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -445,7 +445,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         VMSTATE_UINT32(env.misa_ext, RISCVCPU),
         VMSTATE_UNUSED(4),
         VMSTATE_UINT32(env.misa_ext_mask, RISCVCPU),
-        VMSTATE_UINTTL(env.priv, RISCVCPU),
+        VMSTATE_UINT8(env.priv, RISCVCPU),
         VMSTATE_BOOL(env.virt_enabled, RISCVCPU),
         VMSTATE_UINT64(env.resetvec, RISCVCPU),
         VMSTATE_UINT64(env.mhartid, RISCVCPU),
-- 
2.51.0
Re: [PATCH v3 21/34] target/riscv: Fix size of priv
Posted by Alistair Francis 3 months, 3 weeks ago
On Wed, Oct 15, 2025 at 6:37 AM Anton Johansson via
<qemu-devel@nongnu.org> wrote:
>
> The priv field of CPUArchState only stores values in the range [0,3],
> fix to 8 bits in size and update relevant function arguments.  Introduce
> a new privilege_mode_t typedef for passing around the privilege mode.
>
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

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

Alistair

> ---
>  target/riscv/cpu.h        | 15 +++++++++++----
>  target/riscv/cpu_helper.c | 12 +++++++-----
>  target/riscv/machine.c    |  2 +-
>  3 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 37035a9541..1d5d74f11b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -109,6 +109,12 @@ typedef struct riscv_cpu_profile {
>
>  extern RISCVCPUProfile *riscv_profiles[];
>
> +/*
> + * Type large enough to hold all PRV_* fields, update CPUArchState::priv
> + * migration field if changing this type.
> + */
> +typedef uint8_t privilege_mode_t;
> +
>  /* Privileged specification version */
>  #define PRIV_VER_1_10_0_STR "v1.10.0"
>  #define PRIV_VER_1_11_0_STR "v1.11.0"
> @@ -264,7 +270,7 @@ struct CPUArchState {
>      uint32_t elf_flags;
>  #endif
>
> -    target_ulong priv;
> +    privilege_mode_t priv;
>      /* CSRs for execution environment configuration */
>      uint64_t menvcfg;
>      uint64_t senvcfg;
> @@ -650,10 +656,11 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
>  RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit);
>  #endif /* !CONFIG_USER_ONLY */
>
> -void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en);
> +void riscv_cpu_set_mode(CPURISCVState *env, privilege_mode_t newpriv,
> +                        bool virt_en);
>
>  void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
> -    enum CTRType type, target_ulong prev_priv, bool prev_virt);
> +    enum CTRType type, privilege_mode_t prev_priv, bool prev_virt);
>  void riscv_ctr_clear(CPURISCVState *env);
>
>  void riscv_translate_init(void);
> @@ -724,7 +731,7 @@ static inline int cpu_address_mode(CPURISCVState *env)
>      return mode;
>  }
>
> -static inline RISCVMXL cpu_get_xl(CPURISCVState *env, target_ulong mode)
> +static inline RISCVMXL cpu_get_xl(CPURISCVState *env, privilege_mode_t mode)
>  {
>      RISCVMXL xl = env->misa_mxl;
>      /*
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 7790244d93..26c3c846a5 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -799,7 +799,7 @@ void riscv_ctr_clear(CPURISCVState *env)
>      memset(env->ctr_data, 0x0, sizeof(env->ctr_data));
>  }
>
> -static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
> +static uint64_t riscv_ctr_priv_to_mask(privilege_mode_t priv, bool virt)
>  {
>      switch (priv) {
>      case PRV_M:
> @@ -819,7 +819,8 @@ static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
>      g_assert_not_reached();
>  }
>
> -static uint64_t riscv_ctr_get_control(CPURISCVState *env, target_long priv,
> +static uint64_t riscv_ctr_get_control(CPURISCVState *env,
> +                                      privilege_mode_t priv,
>                                        bool virt)
>  {
>      switch (priv) {
> @@ -841,7 +842,8 @@ static uint64_t riscv_ctr_get_control(CPURISCVState *env, target_long priv,
>   * and src privilege is less than target privilege. This includes the virtual
>   * state as well.
>   */
> -static bool riscv_ctr_check_xte(CPURISCVState *env, target_long src_prv,
> +static bool riscv_ctr_check_xte(CPURISCVState *env,
> +                                privilege_mode_t src_prv,
>                                  bool src_virt)
>  {
>      target_long tgt_prv = env->priv;
> @@ -930,7 +932,7 @@ static bool riscv_ctr_check_xte(CPURISCVState *env, target_long src_prv,
>   *    idx = (sctrstatus.WRPTR - entry - 1) & (depth - 1);
>   */
>  void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
> -    enum CTRType type, target_ulong src_priv, bool src_virt)
> +    enum CTRType type, privilege_mode_t src_priv, bool src_virt)
>  {
>      bool tgt_virt = env->virt_enabled;
>      uint64_t src_mask = riscv_ctr_priv_to_mask(src_priv, src_virt);
> @@ -1028,7 +1030,7 @@ void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
>      env->sctrstatus = set_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK, head);
>  }
>
> -void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
> +void riscv_cpu_set_mode(CPURISCVState *env, privilege_mode_t newpriv, bool virt_en)
>  {
>      g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
>
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index c55794c554..ce5e44325d 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -445,7 +445,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>          VMSTATE_UINT32(env.misa_ext, RISCVCPU),
>          VMSTATE_UNUSED(4),
>          VMSTATE_UINT32(env.misa_ext_mask, RISCVCPU),
> -        VMSTATE_UINTTL(env.priv, RISCVCPU),
> +        VMSTATE_UINT8(env.priv, RISCVCPU),
>          VMSTATE_BOOL(env.virt_enabled, RISCVCPU),
>          VMSTATE_UINT64(env.resetvec, RISCVCPU),
>          VMSTATE_UINT64(env.mhartid, RISCVCPU),
> --
> 2.51.0
>
>