Re: [PATCH] perf: Check all MSRs before passing hw check

[George Dunlap <george.dunlap@eu.citrix.com>]

On 15/03/13 12:50, Jan Beulich wrote:
>>>> On 15.03.13 at 13:20, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>> --- a/arch/x86/kernel/cpu/perf_event.c
>> +++ b/arch/x86/kernel/cpu/perf_event.c
>> @@ -182,6 +182,7 @@ static bool check_hw_exists(void)
>>   {
>>   	u64 val, val_new = ~0;
>>   	int i, reg, ret = 0;
>> +	int bios_fail = 0;
>>   
>>   	/*
>>   	 * Check to see if the BIOS enabled any of the counters, if so
>> @@ -193,7 +194,7 @@ static bool check_hw_exists(void)
>>   		if (ret)
>>   			goto msr_fail;
>>   		if (val & ARCH_PERFMON_EVENTSEL_ENABLE)
>> -			goto bios_fail;
>> +			bios_fail = 1;
>>   	}
>>   
>>   	if (x86_pmu.num_counters_fixed) {
>> @@ -203,7 +204,7 @@ static bool check_hw_exists(void)
>>   			goto msr_fail;
>>   		for (i = 0; i < x86_pmu.num_counters_fixed; i++) {
>>   			if (val & (0x03 << i*4))
>> -				goto bios_fail;
>> +				bios_fail = 1;
>>   		}
>>   	}
>>   
>> @@ -221,14 +222,13 @@ static bool check_hw_exists(void)
>>   	if (ret || val != val_new)
>>   		goto msr_fail;
>>   
>> -	return true;
>> -
>> -bios_fail:
>> -	/*
>> -	 * We still allow the PMU driver to operate:
>> -	 */
>> -	printk(KERN_CONT "Broken BIOS detected, complain to your hardware vendor.\n");
>> -	printk(KERN_ERR FW_BUG "the BIOS has corrupted hw-PMU resources (MSR %x is %Lx)\n", reg, val);
>> +	if (bios_fail) {
>> +		/*
>> +		 * We still allow the PMU driver to operate:
>> +		 */
>> +		printk(KERN_CONT "Broken BIOS detected, complain to your hardware vendor.\n");
>> +		printk(KERN_ERR FW_BUG "the BIOS has corrupted hw-PMU resources (MSR %x is %Lx)\n", reg, val);
> The values being printed here are not meaningful anymore with this
> patch.

Right -- then I guess the options are:
1. Don't print any values
2. Print values on the first broken MSR detected (but not subsequent ones)
3. Print values on all broken MSRs.

#2 is the current behavior, and won't risk spamming the console.

  -George