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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aab19ab9-0946-4477-8ca2-bc93621a54a0@linux.dev>
Date: Wed, 4 Feb 2026 01:17:25 -0800
From: Atish Patra <atish.patra@...ux.dev>
To: cp0613@...ux.alibaba.com, anup@...infault.org, alex@...ti.fr,
 pjw@...nel.org, guoren@...nel.org
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers/perf: riscv: Keep the fixed counter counting


On 1/31/26 3:24 AM, cp0613@...ux.alibaba.com wrote:
> From: Chen Pei <cp0613@...ux.alibaba.com>
>
> 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@...ux.alibaba.com>
> ---
>   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.