Re: [RFC PATCH] perf: honoring cpuid for number of fixed counters

[Imre Palik <imrep.amz@gmail.com>]

On 06/03/15 10:36, Peter Zijlstra wrote:
> On Wed, Jun 03, 2015 at 10:03:48AM +0200, Imre Palik wrote:
>> From: "Palik, Imre" <imrep@amazon.de>
>>
>> perf doesn't seem to honor the number of fixed counters specified by cpuid
>> leaf 0xa.  It always assume that intel CPUs have at least 3 fixed counters.
>>
>> So if some of the fixed counters are masked out by the hypervisor, it still
>> tries to check/set them.  This is good for testing the masking code in the
>> hypervisor, but not so nice otherwise.
>>
>> This patch makes perf pehave somewhat nicer when the number of fixed
>> counters is less than three.
> 
>> @@ -3042,13 +3042,6 @@ __init int intel_pmu_init(void)
>>  
>>  	x86_pmu.max_pebs_events		= min_t(unsigned, MAX_PEBS_EVENTS, x86_pmu.num_counters);
>>  
>> -	/*
>> -	 * Quirk: v2 perfmon does not report fixed-purpose events, so
>> -	 * assume at least 3 events:
>> -	 */
>> -	if (version > 1)
>> -		x86_pmu.num_counters_fixed = max((int)edx.split.num_counters_fixed, 3);
>> -
>>  	if (boot_cpu_has(X86_FEATURE_PDCM)) {
>>  		u64 capabilities;
> 
> So the problem is that there is real hardware out there that gets the
> CPUID stuff wrong, and this patch penalizes that by then not using the
> fixed counters.

I haven't thought about this.  Thanks.

> Further, the Intel Arch PerfMon v2 spec actually specifies there to be 3
> fixed function counters.
> 
> So anything that says it is v2+ and does not have the 3, is non
> compliant.
>
> I would suggest you go fix your hypervisor.

If I set up the hypervisor to advertise Arch PerfMon v1 (0 fixed counters), then  without my patch, perf still tries to use fixed counters.  So something is clearly broken here.

> Lacking that option; you could probe the MSRs to see if they're really
> there using wrmsr_safe() or something like that -- see
> check_hw_exists().

I'll send something along these lines soon.