On Tue, Oct 09, 2012 at 05:38:34PM +0200, Andre Przywara wrote: > In check_hw_exists() we try to detect non-emulated MSR accesses > by writing an arbitrary value into one of the PMU registers > and check if it's value after a readout is still the same. > This algorithm silently assumes that the register does not contain > the magic value already, which is wrong in at least one situation. > > Fix the algorithm to really do a read-modify-write cycle. This fixes > a warning under Xen under some circumstances on AMD family 10h CPUs. > > The reasons in more details actually sound like a story from > Believe It or Not!: > First you need an AMD family 10h/12h CPU. These do not reset the > PERF_CTR registers on a reboot. > Now you boot bare metal Linux, which goes successfully through this > check, but leaves the magic value of 0xabcd in the register. You > don't use the performance counters, but do a reboot (warm reset). > Then you choose to boot Xen. The check will be triggered with a > recent Linux kernel as Dom0 again, trying to write 0xabcd into the > MSR. Xen silently drops the write (expected), but the subsequent read > will return the value in the register, which just happens to be the > expected magic value. Thus the test misleadingly succeeds, leaving
Is that an oversight in the hypervisor? as in should it disable access to those MSRs? I thought it disabled to most of them already unless you give some extra bootup parameters? (cpufreq=dom0 or something like that). > the kernel in the belief that the PMU is available. This will trigger > the following message: > > [ 0.020294] ------------[ cut here ]------------ > [ 0.020311] WARNING: at arch/x86/xen/enlighten.c:730 > xen_apic_write+0x15/0x17() > [ 0.020318] Hardware name: empty > [ 0.020323] Modules linked in: > [ 0.020334] Pid: 1, comm: swapper/0 Not tainted 3.3.8 #7 > [ 0.020340] Call Trace: > [ 0.020354] [<ffffffff81050379>] warn_slowpath_common+0x80/0x98 > [ 0.020369] [<ffffffff810503a6>] warn_slowpath_null+0x15/0x17 > [ 0.020378] [<ffffffff810034df>] xen_apic_write+0x15/0x17 > [ 0.020392] [<ffffffff8101cb2b>] perf_events_lapic_init+0x2e/0x30 > [ 0.020410] [<ffffffff81ee4dd0>] init_hw_perf_events+0x250/0x407 > [ 0.020419] [<ffffffff81ee4b80>] ? check_bugs+0x2d/0x2d > [ 0.020430] [<ffffffff81002181>] do_one_initcall+0x7a/0x131 > [ 0.020444] [<ffffffff81edbbf9>] kernel_init+0x91/0x15d > [ 0.020456] [<ffffffff817caaa4>] kernel_thread_helper+0x4/0x10 > [ 0.020471] [<ffffffff817c347c>] ? retint_restore_args+0x5/0x6 > [ 0.020481] [<ffffffff817caaa0>] ? gs_change+0x13/0x13 > [ 0.020500] ---[ end trace a7919e7f17c0a725 ]--- > > The new code will change every of the 16 low bits read from the > register and tries to write and read-back that modified number > from the MSR. > > Signed-off-by: Andre Przywara <andre.przyw...@amd.com> > --- > arch/x86/kernel/cpu/perf_event.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/cpu/perf_event.c > b/arch/x86/kernel/cpu/perf_event.c > index 915b876..d18b2b8 100644 > --- a/arch/x86/kernel/cpu/perf_event.c > +++ b/arch/x86/kernel/cpu/perf_event.c > @@ -208,12 +208,14 @@ static bool check_hw_exists(void) > } > > /* > - * Now write a value and read it back to see if it matches, > - * this is needed to detect certain hardware emulators (qemu/kvm) > - * that don't trap on the MSR access and always return 0s. > + * Read the current value, change it and read it back to see if it > + * matches, this is needed to detect certain hardware emulators > + * (qemu/kvm) that don't trap on the MSR access and always return 0s. > */ > - val = 0xabcdUL; > reg = x86_pmu_event_addr(0); > + if (rdmsrl_safe(reg, &val)) > + goto msr_fail; > + val ^= 0xffffUL; > ret = wrmsrl_safe(reg, val); > ret |= rdmsrl_safe(reg, &val_new); > if (ret || val != val_new) > -- > 1.7.12.1 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
