Re: [PATCH] drivers/perf: riscv: Keep the fixed counter counting

[Atish Patra]

On 1/31/26 3:24 AM, cp0613@xxxxxxxxxxxxxxxxx wrote:
> From: Chen Pei <cp0613@xxxxxxxxxxxxxxxxx>
>
> The RISC-V SBI PMU driver disables all PMU counters during initialization
> via pmu_sbi_stop_all. For fixed counters CYCLE, TIME and INSTRET, this is
> unnecessary for the following two reasons:
>
> 1. Some kernel driver code may directly read CYCLE and INSTRET to perform
>     simple performance analysis.

Is this for some debugging purpose to read the instret/cycle count at 
boot time or real use case for driver performance analysis ?

If it is the latter, that will be problematic for various reasons such 
as context switching will lead to inaccurate numbers.

> 2. In legacy mode, user space directly reads CYCLE and INSTRET. (echo 2 >
>     /proc/sys/kernel/perf_user_access)
>
> Therefore, We keep counting CYCLE, TIME and INSTRET.
>
> Signed-off-by: Chen Pei <cp0613@xxxxxxxxxxxxxxxxx>
> ---
>   drivers/perf/riscv_pmu_sbi.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 7dd282da67ce..93aaab324443 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -899,6 +899,9 @@ static int pmu_sbi_get_ctrinfo(int nctr, unsigned long *mask)
>   
>   static inline void pmu_sbi_stop_all(struct riscv_pmu *pmu)
>   {
> +	/* We keep counting CYCLE, TIME and INSTRET. */
> +	pmu->cmask &= ~0x7;
> +

This is incorrect. The cmask should be set based on the perf_user_access 
value. We should not continue counting the CYCLE/INSTRET when legacy 
mode is not set. if (sysctl_perf_user_access == SYSCTL_LEGACY) 
csr_write(CSR_SCOUNTEREN, 0x7); else csr_write(CSR_SCOUNTEREN, 0x2);

>   	/*
>   	 * No need to check the error because we are disabling all the counters
>   	 * which may include counters that are not enabled yet.