Re: [PATCH 1/2] perf riscv: Implement rdtsc()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf riscv: uninitialized upper 32 bits in rdtsc() on RV32
--

commit fa857133aeb89c2c952b09a7575fc1ec0de78663
Author: Chen Pei <cp0613@linux.alibaba.com>

perf riscv: Implement rdtsc()

There is no riscv implementation of rdtsc(), so perf links against the
weak stub in util/tsc.c and every caller reads a constant 0.
This commit implements rdtsc() over the time CSR with rdtime, which is a
TSC equivalent on riscv.

> diff --git a/tools/perf/arch/riscv/util/tsc.c b/tools/perf/arch/riscv/util/tsc.c
> new file mode 100644
> index 0000000000000..7cc4710ac38c3
> --- /dev/null
> +++ b/tools/perf/arch/riscv/util/tsc.c
> @@ -0,0 +1,19 @@
[ ... ]
> +u64 rdtsc(void)
> +{
> +	u64 val;
> +
> +	/*
> +	 * The time CSR ticks at a constant frequency and is the same counter
> +	 * the kernel feeds into the user page conversion fields through
> +	 * sched_clock(), so it is a TSC equivalent here.
> +	 */
> +	asm volatile("rdtime %0" : "=r" (val));

[Severity: Medium]
On 32-bit RISC-V (RV32), does this leave the upper 32 bits of val
uninitialized?

The rdtime instruction on RV32 is a pseudo-instruction that reads only the
lower 32 bits of the time CSR. Because the target variable val is a 64-bit
integer (u64), the compiler maps it to a register pair on RV32.

However, the inline assembly constraint "=r" only writes to the first 32-bit
register, leaving the upper 32 bits completely uninitialized and containing
garbage. This could corrupt perf timestamp calculations when running on an
RV32 system.

Proper 64-bit CSR reading on RV32 requires a loop reading both the time and
timeh CSRs.

> +
> +	return val;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928094739.2605-1-cp0613@linux.alibaba.com?part=1