From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <george.dunlap@eu.citrix.com>
Subject: Re: [PATCH] perf: Check all MSRs before passing hw check
Date: Fri, 15 Mar 2013 14:43:10 +0000
Message-ID: <5143337E.8010301@eu.citrix.com>
References: <1363350033-16379-1-git-send-email-george.dunlap@eu.citrix.com>
	<5143273E02000078000C5E4F@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <5143273E02000078000C5E4F@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Thomas Gleixner <tglx@linutronix.de>, "x86@kernel.org" <x86@kernel.org>, "H. PeterAnvin" <hpa@zytor.com>, Konrad Wilk <konrad.wilk@oracle.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

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