Re: [PATCH v2 07/33] target/riscv: Combine minstretcfg and minstretcfgh

[Alistair Francis <alistair23@gmail.com>]

On Wed, Oct 1, 2025 at 5:31 PM Anton Johansson via
<qemu-devel@nongnu.org> wrote:
>
> According to version 20250508 of the privileged specification,
> minstretcfg is a 64-bit register and minstretcfgh refers to the top
> 32 bits of this register when XLEN == 32.  No real advantage is
> gained by keeping them separate, and combining them allows for slight
> simplification.
>
> Signed-off-by: Anton Johansson <anjo@rev.ng>

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

Alistair

> ---
>  target/riscv/cpu.h |  3 +--
>  target/riscv/csr.c | 18 ++++++++++--------
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 60f7611c00..d8f0818b08 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -420,8 +420,7 @@ struct CPUArchState {
>
>      /* PMU cycle & instret privilege mode filtering */
>      uint64_t mcyclecfg;
> -    target_ulong minstretcfg;
> -    target_ulong minstretcfgh;
> +    uint64_t minstretcfg;
>
>      /* PMU counter state */
>      PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 77d0bd7bca..83f6526723 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1115,7 +1115,8 @@ static RISCVException write_mcyclecfgh(CPURISCVState *env, int csrno,
>  static RISCVException read_minstretcfg(CPURISCVState *env, int csrno,
>                                         target_ulong *val)
>  {
> -    *val = env->minstretcfg;
> +    bool rv32 = riscv_cpu_mxl(env) == MXL_RV32;
> +    *val = extract64(env->minstretcfg, 0, rv32 ? 32 : 64);
>      return RISCV_EXCP_NONE;
>  }
>
> @@ -1142,7 +1143,7 @@ static RISCVException write_minstretcfg(CPURISCVState *env, int csrno,
>  static RISCVException read_minstretcfgh(CPURISCVState *env, int csrno,
>                                          target_ulong *val)
>  {
> -    *val = env->minstretcfgh;
> +    *val = extract64(env->minstretcfg, 32, 32);
>      return RISCV_EXCP_NONE;
>  }
>
> @@ -1159,7 +1160,8 @@ static RISCVException write_minstretcfgh(CPURISCVState *env, int csrno,
>      inh_avail_mask |= (riscv_has_ext(env, RVH) &&
>                         riscv_has_ext(env, RVS)) ? MINSTRETCFGH_BIT_VSINH : 0;
>
> -    env->minstretcfgh = val & inh_avail_mask;
> +    env->minstretcfg = deposit64(env->minstretcfg, 32, 32,
> +                                 val & inh_avail_mask);
>      return RISCV_EXCP_NONE;
>  }
>
> @@ -1249,8 +1251,7 @@ static target_ulong riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env,
>      if (counter_idx == 0) {
>          cfg_val = env->mcyclecfg;
>      } else if (counter_idx == 2) {
> -        cfg_val = rv32 ? ((uint64_t)env->minstretcfgh << 32) :
> -                  env->minstretcfg;
> +        cfg_val = env->minstretcfg;
>      } else {
>          cfg_val = env->mhpmevent_val[counter_idx];
>          cfg_val &= MHPMEVENT_FILTER_MASK;
> @@ -1572,12 +1573,13 @@ static int rmw_cd_ctr_cfgh(CPURISCVState *env, int cfg_index, target_ulong *val,
>          }
>          break;
>      case 2:          /* INSTRETCFGH */
> +        cfgh = extract64(env->minstretcfg, 32, 32);
>          if (wr_mask) {
>              wr_mask &= ~MINSTRETCFGH_BIT_MINH;
> -            env->minstretcfgh = (new_val & wr_mask) |
> -                                (env->minstretcfgh & ~wr_mask);
> +            cfgh = (new_val & wr_mask) | (cfgh & ~wr_mask);
> +            env->minstretcfg = deposit64(env->minstretcfg, 32, 32, cfgh);
>          } else {
> -            *val = env->minstretcfgh;
> +            *val = cfgh;
>          }
>          break;
>      default:
> --
> 2.51.0
>
>