RISCVCPUConfig related cleanups

[PATCH 3/4] target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers

Posted by Daniel Henrique Barboza 2 years, 2 months ago

A common trend in this file is to retrieve a RISCVCPU pointer by first
retrieving a CPUState pointer via env_cpu(). The CPU pointer is used
only to access the RISCVCPUConfig object and nothing else.

Let's use riscv_cpu_cfg() to access what we need directly without these
2 pointers.

Suggested-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/csr.c | 50 +++++++++++-----------------------------------
 1 file changed, 12 insertions(+), 38 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0f4aa22a0f..53f1a331f9 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -46,10 +46,8 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
                                        uint64_t bit)
 {
     bool virt = riscv_cpu_virt_enabled(env);
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
 
-    if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
+    if (env->priv == PRV_M || !riscv_cpu_cfg(env)->ext_smstateen) {
         return RISCV_EXCP_NONE;
     }
 
@@ -81,7 +79,7 @@ static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
     if (!env->debugger && !riscv_cpu_fp_enabled(env) &&
-        !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+        !riscv_cpu_cfg(env)->ext_zfinx) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 #endif
@@ -90,11 +88,9 @@ static RISCVException fs(CPURISCVState *env, int csrno)
 
 static RISCVException vs(CPURISCVState *env, int csrno)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
-
     if (env->misa_ext & RVV ||
-        cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) {
+        riscv_cpu_cfg(env)->ext_zve32f ||
+        riscv_cpu_cfg(env)->ext_zve64f) {
 #if !defined(CONFIG_USER_ONLY)
         if (!env->debugger && !riscv_cpu_vector_enabled(env)) {
             return RISCV_EXCP_ILLEGAL_INST;
@@ -193,10 +189,7 @@ static RISCVException mctr32(CPURISCVState *env, int csrno)
 
 static RISCVException sscofpmf(CPURISCVState *env, int csrno)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
-
-    if (!cpu->cfg.ext_sscofpmf) {
+    if (!riscv_cpu_cfg(env)->ext_sscofpmf) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -319,10 +312,7 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
 
 static RISCVException mstateen(CPURISCVState *env, int csrno)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
-
-    if (!cpu->cfg.ext_smstateen) {
+    if (!riscv_cpu_cfg(env)->ext_smstateen) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -331,10 +321,7 @@ static RISCVException mstateen(CPURISCVState *env, int csrno)
 
 static RISCVException hstateen_pred(CPURISCVState *env, int csrno, int base)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
-
-    if (!cpu->cfg.ext_smstateen) {
+    if (!riscv_cpu_cfg(env)->ext_smstateen) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -361,10 +348,8 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
 {
     bool virt = riscv_cpu_virt_enabled(env);
     int index = csrno - CSR_SSTATEEN0;
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
 
-    if (!cpu->cfg.ext_smstateen) {
+    if (!riscv_cpu_cfg(env)->ext_smstateen) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -916,11 +901,9 @@ static RISCVException read_timeh(CPURISCVState *env, int csrno,
 
 static RISCVException sstc(CPURISCVState *env, int csrno)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
     bool hmode_check = false;
 
-    if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
+    if (!riscv_cpu_cfg(env)->ext_sstc || !env->rdtime_fn) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -1150,30 +1133,21 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
 static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
                                      target_ulong *val)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
-
-    *val = cpu->cfg.mvendorid;
+    *val = riscv_cpu_cfg(env)->mvendorid;
     return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_marchid(CPURISCVState *env, int csrno,
                                    target_ulong *val)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
-
-    *val = cpu->cfg.marchid;
+    *val = riscv_cpu_cfg(env)->marchid;
     return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_mimpid(CPURISCVState *env, int csrno,
                                   target_ulong *val)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
-
-    *val = cpu->cfg.mimpid;
+    *val = riscv_cpu_cfg(env)->mimpid;
     return RISCV_EXCP_NONE;
 }
 
-- 
2.39.2

Re: [PATCH 3/4] target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers

Posted by liweiwei 2 years, 2 months ago

On 2023/2/25 01:45, Daniel Henrique Barboza wrote:
> A common trend in this file is to retrieve a RISCVCPU pointer by first
> retrieving a CPUState pointer via env_cpu(). The CPU pointer is used
> only to access the RISCVCPUConfig object and nothing else.
>
> Let's use riscv_cpu_cfg() to access what we need directly without these
> 2 pointers.
>
> Suggested-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Weiwei Li
>   target/riscv/csr.c | 50 +++++++++++-----------------------------------
>   1 file changed, 12 insertions(+), 38 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0f4aa22a0f..53f1a331f9 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -46,10 +46,8 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
>                                          uint64_t bit)
>   {
>       bool virt = riscv_cpu_virt_enabled(env);
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
>   
> -    if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
> +    if (env->priv == PRV_M || !riscv_cpu_cfg(env)->ext_smstateen) {
>           return RISCV_EXCP_NONE;
>       }
>   
> @@ -81,7 +79,7 @@ static RISCVException fs(CPURISCVState *env, int csrno)
>   {
>   #if !defined(CONFIG_USER_ONLY)
>       if (!env->debugger && !riscv_cpu_fp_enabled(env) &&
> -        !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +        !riscv_cpu_cfg(env)->ext_zfinx) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   #endif
> @@ -90,11 +88,9 @@ static RISCVException fs(CPURISCVState *env, int csrno)
>   
>   static RISCVException vs(CPURISCVState *env, int csrno)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -
>       if (env->misa_ext & RVV ||
> -        cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) {
> +        riscv_cpu_cfg(env)->ext_zve32f ||
> +        riscv_cpu_cfg(env)->ext_zve64f) {
>   #if !defined(CONFIG_USER_ONLY)
>           if (!env->debugger && !riscv_cpu_vector_enabled(env)) {
>               return RISCV_EXCP_ILLEGAL_INST;
> @@ -193,10 +189,7 @@ static RISCVException mctr32(CPURISCVState *env, int csrno)
>   
>   static RISCVException sscofpmf(CPURISCVState *env, int csrno)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -
> -    if (!cpu->cfg.ext_sscofpmf) {
> +    if (!riscv_cpu_cfg(env)->ext_sscofpmf) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -319,10 +312,7 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
>   
>   static RISCVException mstateen(CPURISCVState *env, int csrno)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -
> -    if (!cpu->cfg.ext_smstateen) {
> +    if (!riscv_cpu_cfg(env)->ext_smstateen) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -331,10 +321,7 @@ static RISCVException mstateen(CPURISCVState *env, int csrno)
>   
>   static RISCVException hstateen_pred(CPURISCVState *env, int csrno, int base)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -
> -    if (!cpu->cfg.ext_smstateen) {
> +    if (!riscv_cpu_cfg(env)->ext_smstateen) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -361,10 +348,8 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
>   {
>       bool virt = riscv_cpu_virt_enabled(env);
>       int index = csrno - CSR_SSTATEEN0;
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
>   
> -    if (!cpu->cfg.ext_smstateen) {
> +    if (!riscv_cpu_cfg(env)->ext_smstateen) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -916,11 +901,9 @@ static RISCVException read_timeh(CPURISCVState *env, int csrno,
>   
>   static RISCVException sstc(CPURISCVState *env, int csrno)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
>       bool hmode_check = false;
>   
> -    if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
> +    if (!riscv_cpu_cfg(env)->ext_sstc || !env->rdtime_fn) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -1150,30 +1133,21 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
>   static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
>                                        target_ulong *val)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -
> -    *val = cpu->cfg.mvendorid;
> +    *val = riscv_cpu_cfg(env)->mvendorid;
>       return RISCV_EXCP_NONE;
>   }
>   
>   static RISCVException read_marchid(CPURISCVState *env, int csrno,
>                                      target_ulong *val)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -
> -    *val = cpu->cfg.marchid;
> +    *val = riscv_cpu_cfg(env)->marchid;
>       return RISCV_EXCP_NONE;
>   }
>   
>   static RISCVException read_mimpid(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> -
> -    *val = cpu->cfg.mimpid;
> +    *val = riscv_cpu_cfg(env)->mimpid;
>       return RISCV_EXCP_NONE;
>   }
>

[PATCH 1/4] target/riscv/csr.c: use env_archcpu() in ctr()
[PATCH 2/4] target/riscv/csr.c: simplify mctr()
[PATCH 3/4] target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
[PATCH 4/4] target/riscv/csr.c: avoid env_archcpu() usages when reading RISCVCPUConfig