[PATCH 0/4] RISCVCPUConfig related cleanups

[PATCH 0/4] RISCVCPUConfig related cleanups

[Daniel Henrique Barboza]

Hi,

These cleanups were suggested by LIU Zhiwei during the review of the
RISCV_FEATURE_* cleanups, currently on version 7 [1].

These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.


[1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06467.html

Daniel Henrique Barboza (4):
  target/riscv/csr.c: use env_archcpu() in ctr()
  target/riscv/csr.c: simplify mctr()
  target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
  target/riscv/csr.c: avoid env_archcpu() usages when reading
    RISCVCPUConfig

 target/riscv/csr.c | 90 +++++++++++++---------------------------------
 1 file changed, 24 insertions(+), 66 deletions(-)

-- 
2.39.2

[PATCH 1/4] target/riscv/csr.c: use env_archcpu() in ctr()

[Daniel Henrique Barboza]

We don't need to use env_cpu() and CPUState().

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/csr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 75a540bfcb..3692617d13 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -108,8 +108,7 @@ static RISCVException vs(CPURISCVState *env, int csrno)
 static RISCVException ctr(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
+    RISCVCPU *cpu = env_archcpu(env);
     int ctr_index;
     target_ulong ctr_mask;
     int base_csrno = CSR_CYCLE;
-- 
2.39.2

[PATCH 2/4] target/riscv/csr.c: simplify mctr()

[Daniel Henrique Barboza]

Use riscv_cpu_cfg() to retrieve pmu_num.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/csr.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 3692617d13..0f4aa22a0f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -165,8 +165,7 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
 #if !defined(CONFIG_USER_ONLY)
 static RISCVException mctr(CPURISCVState *env, int csrno)
 {
-    CPUState *cs = env_cpu(env);
-    RISCVCPU *cpu = RISCV_CPU(cs);
+    int pmu_num = riscv_cpu_cfg(env)->pmu_num;
     int ctr_index;
     int base_csrno = CSR_MHPMCOUNTER3;
 
@@ -175,7 +174,7 @@ static RISCVException mctr(CPURISCVState *env, int csrno)
         base_csrno += 0x80;
     }
     ctr_index = csrno - base_csrno;
-    if (!cpu->cfg.pmu_num || ctr_index >= cpu->cfg.pmu_num) {
+    if (!pmu_num || ctr_index >= pmu_num) {
         /* The PMU is not enabled or counter is out of range*/
         return RISCV_EXCP_ILLEGAL_INST;
     }
-- 
2.39.2

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

[Daniel Henrique Barboza]

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

[PATCH 4/4] target/riscv/csr.c: avoid env_archcpu() usages when reading RISCVCPUConfig

[Daniel Henrique Barboza]

Retrieving the CPU pointer using env_archcpu() just to access cpu->cfg
can be avoided by using riscv_cpu_cfg().

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

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 53f1a331f9..ffa2d7b606 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -213,9 +213,7 @@ static RISCVException any32(CPURISCVState *env, int csrno)
 
 static int aia_any(CPURISCVState *env, int csrno)
 {
-    RISCVCPU *cpu = env_archcpu(env);
-
-    if (!cpu->cfg.ext_smaia) {
+    if (!riscv_cpu_cfg(env)->ext_smaia) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -224,9 +222,7 @@ static int aia_any(CPURISCVState *env, int csrno)
 
 static int aia_any32(CPURISCVState *env, int csrno)
 {
-    RISCVCPU *cpu = env_archcpu(env);
-
-    if (!cpu->cfg.ext_smaia) {
+    if (!riscv_cpu_cfg(env)->ext_smaia) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -253,9 +249,7 @@ static int smode32(CPURISCVState *env, int csrno)
 
 static int aia_smode(CPURISCVState *env, int csrno)
 {
-    RISCVCPU *cpu = env_archcpu(env);
-
-    if (!cpu->cfg.ext_ssaia) {
+    if (!riscv_cpu_cfg(env)->ext_ssaia) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -264,9 +258,7 @@ static int aia_smode(CPURISCVState *env, int csrno)
 
 static int aia_smode32(CPURISCVState *env, int csrno)
 {
-    RISCVCPU *cpu = env_archcpu(env);
-
-    if (!cpu->cfg.ext_ssaia) {
+    if (!riscv_cpu_cfg(env)->ext_ssaia) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -380,9 +372,7 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno)
 
 static int aia_hmode(CPURISCVState *env, int csrno)
 {
-    RISCVCPU *cpu = env_archcpu(env);
-
-    if (!cpu->cfg.ext_ssaia) {
+    if (!riscv_cpu_cfg(env)->ext_ssaia) {
         return RISCV_EXCP_ILLEGAL_INST;
      }
 
@@ -391,9 +381,7 @@ static int aia_hmode(CPURISCVState *env, int csrno)
 
 static int aia_hmode32(CPURISCVState *env, int csrno)
 {
-    RISCVCPU *cpu = env_archcpu(env);
-
-    if (!cpu->cfg.ext_ssaia) {
+    if (!riscv_cpu_cfg(env)->ext_ssaia) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -430,9 +418,7 @@ static RISCVException debug(CPURISCVState *env, int csrno)
 
 static RISCVException seed(CPURISCVState *env, int csrno)
 {
-    RISCVCPU *cpu = env_archcpu(env);
-
-    if (!cpu->cfg.ext_zkr) {
+    if (!riscv_cpu_cfg(env)->ext_zkr) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -555,7 +541,7 @@ static RISCVException read_vl(CPURISCVState *env, int csrno,
 
 static int read_vlenb(CPURISCVState *env, int csrno, target_ulong *val)
 {
-    *val = env_archcpu(env)->cfg.vlen >> 3;
+    *val = riscv_cpu_cfg(env)->vlen >> 3;
     return RISCV_EXCP_NONE;
 }
 
@@ -610,7 +596,7 @@ static RISCVException write_vstart(CPURISCVState *env, int csrno,
      * The vstart CSR is defined to have only enough writable bits
      * to hold the largest element index, i.e. lg2(VLEN) bits.
      */
-    env->vstart = val & ~(~0ULL << ctzl(env_archcpu(env)->cfg.vlen));
+    env->vstart = val & ~(~0ULL << ctzl(riscv_cpu_cfg(env)->vlen));
     return RISCV_EXCP_NONE;
 }
 
-- 
2.39.2

Re: [PATCH 0/4] RISCVCPUConfig related cleanups

[Richard Henderson]

On 2/24/23 07:45, Daniel Henrique Barboza wrote:
> Hi,
> 
> These cleanups were suggested by LIU Zhiwei during the review of the
> RISCV_FEATURE_* cleanups, currently on version 7 [1].
> 
> These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
> riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.

If you add

Based-on: <message-id>

to the cover letter, then patchew can stitch the two patch sets together in its git 
repository.


r~

Re: [PATCH 0/4] RISCVCPUConfig related cleanups

[Richard Henderson]

On 2/24/23 07:45, Daniel Henrique Barboza wrote:
> Hi,
> 
> These cleanups were suggested by LIU Zhiwei during the review of the
> RISCV_FEATURE_* cleanups, currently on version 7 [1].
> 
> These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
> riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.
> 
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06467.html
> 
> Daniel Henrique Barboza (4):
>    target/riscv/csr.c: use env_archcpu() in ctr()
>    target/riscv/csr.c: simplify mctr()
>    target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
>    target/riscv/csr.c: avoid env_archcpu() usages when reading
>      RISCVCPUConfig
> 
>   target/riscv/csr.c | 90 +++++++++++++---------------------------------
>   1 file changed, 24 insertions(+), 66 deletions(-)
> 

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 1/4] target/riscv/csr.c: use env_archcpu() in ctr()

[liweiwei]

On 2023/2/25 01:45, Daniel Henrique Barboza wrote:
> We don't need to use env_cpu() and CPUState().
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/csr.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 75a540bfcb..3692617d13 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -108,8 +108,7 @@ static RISCVException vs(CPURISCVState *env, int csrno)
>   static RISCVException ctr(CPURISCVState *env, int csrno)
>   {
>   #if !defined(CONFIG_USER_ONLY)
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> +    RISCVCPU *cpu = env_archcpu(env);
>       int ctr_index;
>       target_ulong ctr_mask;
>       int base_csrno = CSR_CYCLE;

This has been done by previous patchset from Bin Meng:

https://lists.nongnu.org/archive/html/qemu-riscv/2023-02/msg00276.html

Regards,

Weiwei Li

Re: [PATCH 2/4] target/riscv/csr.c: simplify mctr()

[liweiwei]

On 2023/2/25 01:45, Daniel Henrique Barboza wrote:
> Use riscv_cpu_cfg() to retrieve pmu_num.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Weiwei Li
>   target/riscv/csr.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3692617d13..0f4aa22a0f 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -165,8 +165,7 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
>   #if !defined(CONFIG_USER_ONLY)
>   static RISCVException mctr(CPURISCVState *env, int csrno)
>   {
> -    CPUState *cs = env_cpu(env);
> -    RISCVCPU *cpu = RISCV_CPU(cs);
> +    int pmu_num = riscv_cpu_cfg(env)->pmu_num;
>       int ctr_index;
>       int base_csrno = CSR_MHPMCOUNTER3;
>   
> @@ -175,7 +174,7 @@ static RISCVException mctr(CPURISCVState *env, int csrno)
>           base_csrno += 0x80;
>       }
>       ctr_index = csrno - base_csrno;
> -    if (!cpu->cfg.pmu_num || ctr_index >= cpu->cfg.pmu_num) {
> +    if (!pmu_num || ctr_index >= pmu_num) {
>           /* The PMU is not enabled or counter is out of range*/
>           return RISCV_EXCP_ILLEGAL_INST;
>       }

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

[liweiwei]

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

Re: [PATCH 4/4] target/riscv/csr.c: avoid env_archcpu() usages when reading RISCVCPUConfig

[liweiwei]

On 2023/2/25 01:45, Daniel Henrique Barboza wrote:
> Retrieving the CPU pointer using env_archcpu() just to access cpu->cfg
> can be avoided by using riscv_cpu_cfg().
>
> 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 | 32 +++++++++-----------------------
>   1 file changed, 9 insertions(+), 23 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 53f1a331f9..ffa2d7b606 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -213,9 +213,7 @@ static RISCVException any32(CPURISCVState *env, int csrno)
>   
>   static int aia_any(CPURISCVState *env, int csrno)
>   {
> -    RISCVCPU *cpu = env_archcpu(env);
> -
> -    if (!cpu->cfg.ext_smaia) {
> +    if (!riscv_cpu_cfg(env)->ext_smaia) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -224,9 +222,7 @@ static int aia_any(CPURISCVState *env, int csrno)
>   
>   static int aia_any32(CPURISCVState *env, int csrno)
>   {
> -    RISCVCPU *cpu = env_archcpu(env);
> -
> -    if (!cpu->cfg.ext_smaia) {
> +    if (!riscv_cpu_cfg(env)->ext_smaia) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -253,9 +249,7 @@ static int smode32(CPURISCVState *env, int csrno)
>   
>   static int aia_smode(CPURISCVState *env, int csrno)
>   {
> -    RISCVCPU *cpu = env_archcpu(env);
> -
> -    if (!cpu->cfg.ext_ssaia) {
> +    if (!riscv_cpu_cfg(env)->ext_ssaia) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -264,9 +258,7 @@ static int aia_smode(CPURISCVState *env, int csrno)
>   
>   static int aia_smode32(CPURISCVState *env, int csrno)
>   {
> -    RISCVCPU *cpu = env_archcpu(env);
> -
> -    if (!cpu->cfg.ext_ssaia) {
> +    if (!riscv_cpu_cfg(env)->ext_ssaia) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -380,9 +372,7 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno)
>   
>   static int aia_hmode(CPURISCVState *env, int csrno)
>   {
> -    RISCVCPU *cpu = env_archcpu(env);
> -
> -    if (!cpu->cfg.ext_ssaia) {
> +    if (!riscv_cpu_cfg(env)->ext_ssaia) {
>           return RISCV_EXCP_ILLEGAL_INST;
>        }
>   
> @@ -391,9 +381,7 @@ static int aia_hmode(CPURISCVState *env, int csrno)
>   
>   static int aia_hmode32(CPURISCVState *env, int csrno)
>   {
> -    RISCVCPU *cpu = env_archcpu(env);
> -
> -    if (!cpu->cfg.ext_ssaia) {
> +    if (!riscv_cpu_cfg(env)->ext_ssaia) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -430,9 +418,7 @@ static RISCVException debug(CPURISCVState *env, int csrno)
>   
>   static RISCVException seed(CPURISCVState *env, int csrno)
>   {
> -    RISCVCPU *cpu = env_archcpu(env);
> -
> -    if (!cpu->cfg.ext_zkr) {
> +    if (!riscv_cpu_cfg(env)->ext_zkr) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -555,7 +541,7 @@ static RISCVException read_vl(CPURISCVState *env, int csrno,
>   
>   static int read_vlenb(CPURISCVState *env, int csrno, target_ulong *val)
>   {
> -    *val = env_archcpu(env)->cfg.vlen >> 3;
> +    *val = riscv_cpu_cfg(env)->vlen >> 3;
>       return RISCV_EXCP_NONE;
>   }
>   
> @@ -610,7 +596,7 @@ static RISCVException write_vstart(CPURISCVState *env, int csrno,
>        * The vstart CSR is defined to have only enough writable bits
>        * to hold the largest element index, i.e. lg2(VLEN) bits.
>        */
> -    env->vstart = val & ~(~0ULL << ctzl(env_archcpu(env)->cfg.vlen));
> +    env->vstart = val & ~(~0ULL << ctzl(riscv_cpu_cfg(env)->vlen));
>       return RISCV_EXCP_NONE;
>   }
>   

Re: [PATCH 0/4] RISCVCPUConfig related cleanups

[liweiwei]

On 2023/2/25 01:45, Daniel Henrique Barboza wrote:
> Hi,
>
> These cleanups were suggested by LIU Zhiwei during the review of the
> RISCV_FEATURE_* cleanups, currently on version 7 [1].
>
> These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
> riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.
>
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06467.html
>
> Daniel Henrique Barboza (4):
>    target/riscv/csr.c: use env_archcpu() in ctr()
>    target/riscv/csr.c: simplify mctr()
>    target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
>    target/riscv/csr.c: avoid env_archcpu() usages when reading
>      RISCVCPUConfig
>
>   target/riscv/csr.c | 90 +++++++++++++---------------------------------
>   1 file changed, 24 insertions(+), 66 deletions(-)
>
As  I suggested in another patch, cpu_get_cfg() can also be used in 
vector_helper.c.

Regards,

Weiwei Li

Re: [PATCH 0/4] RISCVCPUConfig related cleanups

[Daniel Henrique Barboza]

On 2/25/23 03:47, liweiwei wrote:
> 
> On 2023/2/25 01:45, Daniel Henrique Barboza wrote:
>> Hi,
>>
>> These cleanups were suggested by LIU Zhiwei during the review of the
>> RISCV_FEATURE_* cleanups, currently on version 7 [1].
>>
>> These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
>> riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.
>>
>>
>> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06467.html
>>
>> Daniel Henrique Barboza (4):
>>    target/riscv/csr.c: use env_archcpu() in ctr()
>>    target/riscv/csr.c: simplify mctr()
>>    target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
>>    target/riscv/csr.c: avoid env_archcpu() usages when reading
>>      RISCVCPUConfig
>>
>>   target/riscv/csr.c | 90 +++++++++++++---------------------------------
>>   1 file changed, 24 insertions(+), 66 deletions(-)
>>
> As  I suggested in another patch, cpu_get_cfg() can also be used in vector_helper.c.


I decided to do it in a separated series together with the vector_helper.c change
I did last week:


https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg07566.html


The vector_change I did prior eliminated some of the env_archcpu() we want to
avoid so makes sense to one after the other.


Thanks,


Daniel


> 
> Regards,
> 
> Weiwei Li
> 
> 

Re: [PATCH 0/4] RISCVCPUConfig related cleanups

[Palmer Dabbelt]

On Fri, 24 Feb 2023 09:45:16 PST (-0800), dbarboza@ventanamicro.com wrote:
> Hi,
>
> These cleanups were suggested by LIU Zhiwei during the review of the
> RISCV_FEATURE_* cleanups, currently on version 7 [1].
>
> These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
> riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.
>
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06467.html
>
> Daniel Henrique Barboza (4):
>   target/riscv/csr.c: use env_archcpu() in ctr()
>   target/riscv/csr.c: simplify mctr()
>   target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
>   target/riscv/csr.c: avoid env_archcpu() usages when reading
>     RISCVCPUConfig
>
>  target/riscv/csr.c | 90 +++++++++++++---------------------------------
>  1 file changed, 24 insertions(+), 66 deletions(-)

I just based these on that patch, which landed as d4ea711704 
("target/riscv: introduce riscv_cpu_cfg()").  That resulted in a handful 
of merge conflicts, but everything looked pretty mechanical.  So it's 
queued up.

Thanks!

Re: [PATCH 0/4] RISCVCPUConfig related cleanups

[Bin Meng]

Hi Palmer,

On Thu, Mar 2, 2023 at 10:08 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Fri, 24 Feb 2023 09:45:16 PST (-0800), dbarboza@ventanamicro.com wrote:
> > Hi,
> >
> > These cleanups were suggested by LIU Zhiwei during the review of the
> > RISCV_FEATURE_* cleanups, currently on version 7 [1].
> >
> > These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
> > riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.
> >
> >
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06467.html
> >
> > Daniel Henrique Barboza (4):
> >   target/riscv/csr.c: use env_archcpu() in ctr()
> >   target/riscv/csr.c: simplify mctr()
> >   target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
> >   target/riscv/csr.c: avoid env_archcpu() usages when reading
> >     RISCVCPUConfig
> >
> >  target/riscv/csr.c | 90 +++++++++++++---------------------------------
> >  1 file changed, 24 insertions(+), 66 deletions(-)
>
> I just based these on that patch, which landed as d4ea711704
> ("target/riscv: introduce riscv_cpu_cfg()").  That resulted in a handful
> of merge conflicts, but everything looked pretty mechanical.  So it's
> queued up.
>

As Weiwei pointed out in
https://lore.kernel.org/qemu-devel/e40e75ff-37e0-94d3-e9e2-c159b0e2da68@iscas.ac.cn/,
patch#1 should be dropped.

But I see it was landed up in your tree @
https://github.com/palmer-dabbelt/qemu/commit/3c7d54f945f1b5b474ea35c0815a1618927c9384,
while my changes are already in tree @
https://github.com/palmer-dabbelt/qemu/commit/94e297071bc0a5965cc32c497a886f2cf9d32710.

Not sure why git doesn't figure that out ...

Regards,
Bin

Re: [PATCH 0/4] RISCVCPUConfig related cleanups

[Daniel Henrique Barboza]

On 3/1/23 23:24, Bin Meng wrote:
> Hi Palmer,
> 
> On Thu, Mar 2, 2023 at 10:08 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>>
>> On Fri, 24 Feb 2023 09:45:16 PST (-0800), dbarboza@ventanamicro.com wrote:
>>> Hi,
>>>
>>> These cleanups were suggested by LIU Zhiwei during the review of the
>>> RISCV_FEATURE_* cleanups, currently on version 7 [1].
>>>
>>> These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
>>> riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.
>>>
>>>
>>> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06467.html
>>>
>>> Daniel Henrique Barboza (4):
>>>    target/riscv/csr.c: use env_archcpu() in ctr()
>>>    target/riscv/csr.c: simplify mctr()
>>>    target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
>>>    target/riscv/csr.c: avoid env_archcpu() usages when reading
>>>      RISCVCPUConfig
>>>
>>>   target/riscv/csr.c | 90 +++++++++++++---------------------------------
>>>   1 file changed, 24 insertions(+), 66 deletions(-)
>>
>> I just based these on that patch, which landed as d4ea711704
>> ("target/riscv: introduce riscv_cpu_cfg()").  That resulted in a handful
>> of merge conflicts, but everything looked pretty mechanical.  So it's
>> queued up.
>>
> 
> As Weiwei pointed out in
> https://lore.kernel.org/qemu-devel/e40e75ff-37e0-94d3-e9e2-c159b0e2da68@iscas.ac.cn/,
> patch#1 should be dropped.

Yeah, that's my bad. I should've send a v2 owithout patch 1 to avoid confusion.


Daniel

> 
> But I see it was landed up in your tree @
> https://github.com/palmer-dabbelt/qemu/commit/3c7d54f945f1b5b474ea35c0815a1618927c9384,
> while my changes are already in tree @
> https://github.com/palmer-dabbelt/qemu/commit/94e297071bc0a5965cc32c497a886f2cf9d32710.
> 
> Not sure why git doesn't figure that out ...
> 
> Regards,
> Bin