1. Patchew
  2. QEMU
  3. [PATCH v6 0/9] make write_misa a no-op and FEATURE_* cleanups
  4. View patch

[PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()

Daniel Henrique Barboza posted 9 patches 2 years, 11 months ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>
There is a newer version of this series
[PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by Daniel Henrique Barboza 2 years, 11 months ago 
We're going to do changes that requires accessing the RISCVCPUConfig
struct from the RISCVCPU, having access only to a CPURISCVState 'env'
pointer. Add a helper to make the code easier to read.

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

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 01803a020d..5e9626837b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -653,6 +653,11 @@ static inline RISCVMXL riscv_cpu_mxl(CPURISCVState *env)
 #endif
 #define riscv_cpu_mxl_bits(env) (1UL << (4 + riscv_cpu_mxl(env)))
 
+static inline const RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)
+{
+    return &env_archcpu(env)->cfg;
+}
+
 #if defined(TARGET_RISCV32)
 #define cpu_recompute_xl(env)  ((void)(env), MXL_RV32)
 #else
-- 
2.39.1
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by Richard Henderson 2 years, 11 months ago 
On 2/16/23 11:55, Daniel Henrique Barboza wrote:
> We're going to do changes that requires accessing the RISCVCPUConfig
> struct from the RISCVCPU, having access only to a CPURISCVState 'env'
> pointer. Add a helper to make the code easier to read.
> 
> Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com>
> ---
>   target/riscv/cpu.h | 5 +++++
>   1 file changed, 5 insertions(+)

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

r~
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by LIU Zhiwei 2 years, 11 months ago 
On 2023/2/17 5:55, Daniel Henrique Barboza wrote:
> We're going to do changes that requires accessing the RISCVCPUConfig
> struct from the RISCVCPU, having access only to a CPURISCVState 'env'
> pointer. Add a helper to make the code easier to read.
>
> Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com>
> ---
>   target/riscv/cpu.h | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 01803a020d..5e9626837b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -653,6 +653,11 @@ static inline RISCVMXL riscv_cpu_mxl(CPURISCVState *env)
>   #endif
>   #define riscv_cpu_mxl_bits(env) (1UL << (4 + riscv_cpu_mxl(env)))
>   
> +static inline const RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)
> +{
> +    return &env_archcpu(env)->cfg;
> +}
> +

There many places in branch should use this interface, not just in this 
patch set.

For example,

static RISCVException seed(CPURISCVState *env, int csrno)
{
     RISCVCPU *cpu = env_archcpu(env);

     if (!cpu->cfg.ext_zkr) {
         return RISCV_EXCP_ILLEGAL_INST;
     }

The cpu here will not be used, except referring to the cfg.

Do you mind to unify the use?

Zhiwei

>   #if defined(TARGET_RISCV32)
>   #define cpu_recompute_xl(env)  ((void)(env), MXL_RV32)
>   #else
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by Daniel Henrique Barboza 2 years, 11 months ago 

On 2/16/23 23:55, LIU Zhiwei wrote:
> 
> On 2023/2/17 5:55, Daniel Henrique Barboza wrote:
>> We're going to do changes that requires accessing the RISCVCPUConfig
>> struct from the RISCVCPU, having access only to a CPURISCVState 'env'
>> pointer. Add a helper to make the code easier to read.
>>
>> Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com>
>> ---
>>   target/riscv/cpu.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 01803a020d..5e9626837b 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -653,6 +653,11 @@ static inline RISCVMXL riscv_cpu_mxl(CPURISCVState *env)
>>   #endif
>>   #define riscv_cpu_mxl_bits(env) (1UL << (4 + riscv_cpu_mxl(env)))
>>   
>> +static inline const RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)
>> +{
>> +    return &env_archcpu(env)->cfg;
>> +}
>> +
> 
> There many places in branch should use this interface, not just in this patch set.
> 
> For example,
> 
> static RISCVException seed(CPURISCVState *env, int csrno)
> {
>      RISCVCPU *cpu = env_archcpu(env);
> 
>      if (!cpu->cfg.ext_zkr) {
>          return RISCV_EXCP_ILLEGAL_INST;
>      }
> 
> The cpu here will not be used, except referring to the cfg.
> 
> Do you mind to unify the use?

Sure, I can look into that to make the cfg access more uniform across the code.

I believe this would be too much to do in this series though. Let's do it as a
follow-up.


Thanks.


Daniel

> 
> Zhiwei
> 
>>   #if defined(TARGET_RISCV32)
>>   #define cpu_recompute_xl(env)  ((void)(env), MXL_RV32)
>>   #else
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by weiwei 2 years, 11 months ago 
On 2023/2/17 05:55, Daniel Henrique Barboza wrote:
> We're going to do changes that requires accessing the RISCVCPUConfig
> struct from the RISCVCPU, having access only to a CPURISCVState 'env'
> pointer. Add a helper to make the code easier to read.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Regards,

Weiwei Li
> ---
>   target/riscv/cpu.h | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 01803a020d..5e9626837b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -653,6 +653,11 @@ static inline RISCVMXL riscv_cpu_mxl(CPURISCVState *env)
>   #endif
>   #define riscv_cpu_mxl_bits(env) (1UL << (4 + riscv_cpu_mxl(env)))
>   
> +static inline const RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)
> +{
> +    return &env_archcpu(env)->cfg;
> +}
> +
>   #if defined(TARGET_RISCV32)
>   #define cpu_recompute_xl(env)  ((void)(env), MXL_RV32)
>   #else
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by LIU Zhiwei 2 years, 11 months ago 
On 2023/2/17 5:55, Daniel Henrique Barboza wrote:
> We're going to do changes that requires accessing the RISCVCPUConfig
> struct from the RISCVCPU, having access only to a CPURISCVState 'env'
> pointer. Add a helper to make the code easier to read.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/cpu.h | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 01803a020d..5e9626837b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -653,6 +653,11 @@ static inline RISCVMXL riscv_cpu_mxl(CPURISCVState *env)
>   #endif
>   #define riscv_cpu_mxl_bits(env) (1UL << (4 + riscv_cpu_mxl(env)))
>   
> +static inline const RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)

Maybe we should

static inline const* RISCVCPUConfig riscv_cpu_cfg(CPURISCVState *env) or just
static inline RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)

Zhiwei

> +{
> +    return &env_archcpu(env)->cfg;
> +}
> +
>   #if defined(TARGET_RISCV32)
>   #define cpu_recompute_xl(env)  ((void)(env), MXL_RV32)
>   #else
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by LIU Zhiwei 2 years, 11 months ago 
On 2023/2/17 9:50, LIU Zhiwei wrote:
>
> On 2023/2/17 5:55, Daniel Henrique Barboza wrote:
>> We're going to do changes that requires accessing the RISCVCPUConfig
>> struct from the RISCVCPU, having access only to a CPURISCVState 'env'
>> pointer. Add a helper to make the code easier to read.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   target/riscv/cpu.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 01803a020d..5e9626837b 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -653,6 +653,11 @@ static inline RISCVMXL 
>> riscv_cpu_mxl(CPURISCVState *env)
>>   #endif
>>   #define riscv_cpu_mxl_bits(env) (1UL << (4 + riscv_cpu_mxl(env)))
>>   +static inline const RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)
>
> Maybe we should
>
> static inline const* RISCVCPUConfig riscv_cpu_cfg(CPURISCVState *env) 
> or just
> static inline RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)

Ignore this comment. I see that you never change the fields from this 
pointer.

Zhiwei

>
> Zhiwei
>
>> +{
>> +    return &env_archcpu(env)->cfg;
>> +}
>> +
>>   #if defined(TARGET_RISCV32)
>>   #define cpu_recompute_xl(env)  ((void)(env), MXL_RV32)
>>   #else
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by LIU Zhiwei 2 years, 11 months ago 
On 2023/2/17 5:55, Daniel Henrique Barboza wrote:
> We're going to do changes that requires accessing the RISCVCPUConfig
> struct from the RISCVCPU, having access only to a CPURISCVState 'env'
> pointer. Add a helper to make the code easier to read.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/cpu.h | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 01803a020d..5e9626837b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -653,6 +653,11 @@ static inline RISCVMXL riscv_cpu_mxl(CPURISCVState *env)
>   #endif
>   #define riscv_cpu_mxl_bits(env) (1UL << (4 + riscv_cpu_mxl(env)))
>   
> +static inline const RISCVCPUConfig *riscv_cpu_cfg(CPURISCVState *env)
> +{
> +    return &env_archcpu(env)->cfg;
> +}
> +
>   #if defined(TARGET_RISCV32)
>   #define cpu_recompute_xl(env)  ((void)(env), MXL_RV32)
>   #else

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Zhiwei
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by Bin Meng 2 years, 11 months ago 
On Fri, Feb 17, 2023 at 5:57 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> We're going to do changes that requires accessing the RISCVCPUConfig
> struct from the RISCVCPU, having access only to a CPURISCVState 'env'
> pointer. Add a helper to make the code easier to read.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/cpu.h | 5 +++++
>  1 file changed, 5 insertions(+)
>

Looks like the RB tag is missing somehow?

Reviewed-by: Bin Meng <bmeng@tinylab.org>
Re: [PATCH v6 2/9] target/riscv: introduce riscv_cpu_cfg()
Posted by Bin Meng 2 years, 11 months ago 
On Fri, Feb 17, 2023 at 8:45 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Feb 17, 2023 at 5:57 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
> >
> > We're going to do changes that requires accessing the RISCVCPUConfig
> > struct from the RISCVCPU, having access only to a CPURISCVState 'env'
> > pointer. Add a helper to make the code easier to read.
> >
> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > ---
> >  target/riscv/cpu.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
>
> Looks like the RB tag is missing somehow?

Never mind. I see the difference :)

Regards,
Bin

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.