Re: target/riscv: rdcycleh problem

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 13/5/26 07:44, Antony Pavlov wrote:
> On Wed, 13 May 2026 at 04:50, Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Tue, May 12, 2026 at 1:43 PM Philippe Mathieu-Daudé
>> <philmd@linaro.org> wrote:
>>>
>>> Cc'ing qemu-riscv@ list
>>>
>>> On 12/5/26 02:50, Antony Pavlov wrote:
>>>> Hi!
>>>>
>>>> I use qemu-system-riscv32 v11.0.0 for running this code:
>>>>
>>>>       15258:    c80025f3    rdcycleh a1
>>>>       1525c:    c0002573    rdcycle  a0
>>>>       15260:    c80027f3    rdcycleh a5
>>>>       15264:    fef59ae3    bne      a1,a5,15258
>>>>
>>>> The code is recommended by riscv specification for reading 64-bit
>>>> cycle counter on 32-bit system (see
>>>> https://docs.riscv.org/reference/isa/unpriv/counters.html#7-1-1-zicntr-extension-for-base-counters-and-timers
>>>> for details).
>>>>
>>>> When running on qemu-system-riscv32 both the rdcycle and the rdcycleh
>>>> instructions read low 32-bit of the cycle counter so the branch
>>>> instruction always go to 15258.
>>>
>>> Interestingly I'm having similar 32/64 bit conversion issue with
>>> the set of CSR functions in my single binary prototype due to this
>>> target_ulong use.
>>>
>>>>
>>>> This quick and dirty patch fixes my problem:
>>>>
>>>> --- a/target/riscv/csr.c
>>>> +++ b/target/riscv/csr.c
>>>> @@ -1389,8 +1389,11 @@ RISCVException riscv_pmu_read_ctr(CPURISCVState
>>>> *env, target_ulong *val,
>>>>         */
>>>>        if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
>>>>            riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) {
>>>> -        *val = riscv_pmu_ctr_get_fixed_counters_val(env, ctr_idx) -
>>>> +        uint64_t t;
>>>> +
>>>> +        t = riscv_pmu_ctr_get_fixed_counters_val(env, ctr_idx) -
>>>>                                                        ctr_prev + ctr_val;
>>>> +        *val = extract64(t, start, length);
>>>>        } else {
>>>>            *val = ctr_val;
>>>>        }
>>>>
>>>> Have you any suggestion?
>>
>> Yeah, it looks like a bug using the `target_ulong` (which is only
>> 32-bits on RV32). This is something that should be fixed, if you can
>> send a patch that would be great
> 
> I'll send the patch within a few days.

As you said this change is "quick and dirty". I suppose what Alistair
suggested is to replace 'target_ulong *' by 'uint64_t *' where relevant,
doing the appropriate truncations when riscv_cpu_mxl(env) == MXL_RV32.