Re: [PATCH v2 08/33] target/riscv: Combine mhpmcounter and mhpmcounterh

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 10/1/25 12:32 AM, Anton Johansson wrote:
> According to version 20250508 of the privileged specification,
> mhpmconter is a 64-bit register and mhpmcounterh refers to the top
> 32 bits of this register when XLEN == 32.  No real advantage is
> gained by keeping them separate, and combining allows for slight
> simplification.
> 
> Note, the cpu/pmu VMSTATE version is bumped breaking migration from
> older versions.
> 
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   target/riscv/cpu.h     |   8 +--
>   target/riscv/csr.c     |  74 +++++++++++++--------------
>   target/riscv/machine.c |  10 ++--
>   target/riscv/pmu.c     | 111 +++++++++++------------------------------
>   4 files changed, 70 insertions(+), 133 deletions(-)
> 

...

>   RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val,
> -                                         bool upper_half, uint32_t ctr_idx)
> +                                  bool upper_half, uint32_t ctr_idx)
>   {
>       PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];
> -    target_ulong ctr_prev = upper_half ? counter->mhpmcounterh_prev :
> -                                         counter->mhpmcounter_prev;
> -    target_ulong ctr_val = upper_half ? counter->mhpmcounterh_val :
> -                                        counter->mhpmcounter_val;
> +    bool rv32 = riscv_cpu_mxl(env) == MXL_RV32;
> +    int start = upper_half ? 32 : 0;
> +    int length = rv32 ? 32 : 64;
> +    uint64_t ctr_prev = extract64(counter->mhpmcounter_prev, start, length);
> +    uint64_t ctr_val  = extract64(counter->mhpmcounter_val, start, length);
> +
> +    /* Ensure upper_half is only set for XLEN == 32 */
> +    g_assert(rv32 || !upper_half);

An assert will be triggered by extract64 first if this happens.
static inline uint64_t extract64(uint64_t value, int start, int length)
{
     assert(start >= 0 && length > 0 && length <= 64 - start);

Thus, you can move the assert before extract64 calls.