[Xen-devel] [PATCH] perf: Check all MSRs before passing hw check

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

check_hw_exists has a number of checks which go to two exit paths:
msr_fail and bios_fail.  Checks classified as msr_fail will cause
check_hw_exists() to return false, causing the PMU not to be used;
bios_fail checks will only cause a warning to be printed, but will
return true.

The problem is that if there are both msr failures and bios failures,
and the routine hits a bios_fail check first, it will exit early and
return true, not finishing the rest of the msr checks.  If those msrs
are in fact broken, it will cause them to be used erroneously.

This changset causes check_hw_exists() to go through all of the msr
checks, failing and returning false if any of them fail.

This problem affects kernels as far back as 3.2, and should thus be
considered for backport.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
CC: Konrad Wilk <konrad.wilk@xxxxxxxxxx>
CC: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
CC: "H. Peter Anvin" <hpa@xxxxxxxxx>
CC: x86@xxxxxxxxxx
---
 arch/x86/kernel/cpu/perf_event.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 6774c17..df30c9a 100644
--- 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);
+       }
 
        return true;
 
-- 
1.7.9.5


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel