linux-kernel - Re: [PATCH] perf/x86: Fix open counting event error

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: <e85c1a65-fa55-457f-82d9-c25f6a4deb49@linux.intel.com>
Date: Thu, 24 Apr 2025 13:08:12 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Ingo Molnar <mingo@...nel.org>, Luo Gengkun <luogengkun@...weicloud.com>
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
 namhyung@...nel.org, mark.rutland@....com,
 alexander.shishkin@...ux.intel.com, jolsa@...nel.org, irogers@...gle.com,
 adrian.hunter@...el.com, tglx@...utronix.de, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
 ravi.bangoria@....com, linux-perf-users@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf/x86: Fix open counting event error



On 2025-04-24 12:20 p.m., Ingo Molnar wrote:
> 
> * Luo Gengkun <luogengkun@...weicloud.com> wrote:
> 
>> Perf doesn't work at perf stat for hardware events:
>>
>>  $perf stat -- sleep 1
>>  Performance counter stats for 'sleep 1':
>>              16.44 msec task-clock                       #    0.016 CPUs utilized
>>                  2      context-switches                 #  121.691 /sec
>>                  0      cpu-migrations                   #    0.000 /sec
>>                 54      page-faults                      #    3.286 K/sec
>>    <not supported>	cycles
>>    <not supported>	instructions
>>    <not supported>	branches
>>    <not supported>	branch-misses
>>
>> The reason is that the check in x86_pmu_hw_config for sampling event is
>> unexpectedly applied to the counting event.
>>
>> Fixes: 88ec7eedbbd2 ("perf/x86: Fix low freqency setting issue")
>> Signed-off-by: Luo Gengkun <luogengkun@...weicloud.com>
>> ---
>>  arch/x86/events/core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
>> index 6866cc5acb0b..3a4f031d2f44 100644
>> --- a/arch/x86/events/core.c
>> +++ b/arch/x86/events/core.c
>> @@ -629,7 +629,7 @@ int x86_pmu_hw_config(struct perf_event *event)
>>  	if (event->attr.type == event->pmu->type)
>>  		event->hw.config |= x86_pmu_get_event_config(event);
>>  
>> -	if (!event->attr.freq && x86_pmu.limit_period) {
>> +	if (is_sampling_event(event) && !event->attr.freq && x86_pmu.limit_period) {
> 
> Hm, so how come it works here, on an affected x86 system:
> 
> $ perf stat -- sleep 1
> 
>  Performance counter stats for 'sleep 1':
> 
>               0.64 msec task-clock:u                     #    0.001 CPUs utilized             
>                  0      context-switches:u               #    0.000 /sec                      
>                  0      cpu-migrations:u                 #    0.000 /sec                      
>                 73      page-faults:u                    #  114.063 K/sec                     
>            325,849      instructions:u                   #    0.56  insn per cycle            
>                                                   #    0.88  stalled cycles per insn   
>            580,323      cycles:u                         #    0.907 GHz                       
>            286,348      stalled-cycles-frontend:u        #   49.34% frontend cycles idle      
>             72,623      branches:u                       #  113.474 M/sec                     
>              4,713      branch-misses:u                  #    6.49% of all branches           
> 
> 
> ?

It doesn't affect all X86 platforms. It should only impact the platforms
with limit_period used for the non-pebs events. For Intel platforms, it
should only impact some older platforms, e.g., HSW, BDW and NHM.

For other platforms, the x86_pmu.limit_period is invoked. But the left
is not updated. So it still equals to event->attr.sample_period.
It doesn't error out.

	if (!event->attr.freq && x86_pmu.limit_period) {
		s64 left = event->attr.sample_period;
		x86_pmu.limit_period(event, &left);
		if (left > event->attr.sample_period)
			return -EINVAL;
	}

Thanks,
Kan