Message ID | 1404324855-15166-1-git-send-email-kan.liang@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> Signed-off-by: Andi Kleen <ak@linux.intel.com> I did not contribute to this patch, so please remove that SOB. > Signed-off-by: Kan Liang <kan.liang@intel.com> > struct extra_reg *extra_regs; > unsigned int er_flags; > + bool extra_msr_access; /* EXTRA REG MSR can be accessed */ > This doesn't look right, needs a flag for each extra register. They are completely unrelated to each other. BTW this will also cause KVM messages at each boot now. > wrmsrl(hwc->extra_reg.reg, hwc->extra_reg.config); > wrmsrl(hwc->config_base, (hwc->config | enable_mask) & ~disable_mask); > } > diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c > index adb02aa..8011d42 100644 > --- a/arch/x86/kernel/cpu/perf_event_intel.c > +++ b/arch/x86/kernel/cpu/perf_event_intel.c > @@ -2565,6 +2565,13 @@ __init int intel_pmu_init(void) > } > } > > + /* Access LBR MSR may cause #GP under certain circumstances. E.g. KVM doesn't support LBR MSR */ > + if (x86_pmu.lbr_nr) > + x86_pmu.lbr_msr_access = test_msr_access(x86_pmu.lbr_tos) & test_msr_access(x86_pmu.lbr_from); s/&/&&/ And also this doesn't cover the case when someone takes over the LBRs and they start #GPing later. So for LBR the test has to be still at each access. -Andi -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 2, 2014 at 2:14 PM, <kan.liang@intel.com> wrote: > From: Kan Liang <kan.liang@intel.com> > > x86, perf: Protect LBR and offcore rsp against KVM lying > > With -cpu host, KVM reports LBR and offcore support, if the host has support. > When the guest perf driver tries to access LBR or offcore_rsp MSR, > it #GPs all MSR accesses,since KVM doesn't handle LBR and offcore support. > So check the related MSRs access right once at initialization time to avoid the error access at runtime. > > For reproducing the issue, please build the kernel with CONFIG_KVM_INTEL = y. This is for host kernel, > And CONFIG_PARAVIRT = n and CONFIG_KVM_GUEST = n. And this is for guest kernel, right? -Jidong -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Signed-off-by: Andi Kleen <ak@linux.intel.com> > > I did not contribute to this patch, so please remove that SOB. > OK > > Signed-off-by: Kan Liang <kan.liang@intel.com> > > > struct extra_reg *extra_regs; > > unsigned int er_flags; > > + bool extra_msr_access; /* EXTRA REG MSR can be > accessed */ > > > > This doesn't look right, needs a flag for each extra register. > > They are completely unrelated to each other. The extra register is either MSR_OFFCORE_RSP_0 or MSR_OFFCORE_RSP_1. I will add two variables to handle them. > > BTW this will also cause KVM messages at each boot now. > Do you mean the "unhandled wrmsr" or " unhandled rdmsr " messages? You should not observer such error message for offcore msrs and LBR from/to msrs. But I forget to handle LBR TOS in KVM patch. You may observed unhandled rdmsr: 0x1c9 when doing LBR in guest. I will fix it in next version. > > wrmsrl(hwc->extra_reg.reg, hwc->extra_reg.config); > > wrmsrl(hwc->config_base, (hwc->config | enable_mask) & > > ~disable_mask); } diff --git a/arch/x86/kernel/cpu/perf_event_intel.c > > b/arch/x86/kernel/cpu/perf_event_intel.c > > index adb02aa..8011d42 100644 > > --- a/arch/x86/kernel/cpu/perf_event_intel.c > > +++ b/arch/x86/kernel/cpu/perf_event_intel.c > > @@ -2565,6 +2565,13 @@ __init int intel_pmu_init(void) > > } > > } > > > > + /* Access LBR MSR may cause #GP under certain circumstances. E.g. > KVM doesn't support LBR MSR */ > > + if (x86_pmu.lbr_nr) > > + x86_pmu.lbr_msr_access = > test_msr_access(x86_pmu.lbr_tos) & > > +test_msr_access(x86_pmu.lbr_from); > > s/&/&&/ > > And also this doesn't cover the case when someone takes over the LBRs and > they start #GPing later. > So for LBR the test has to be still at each access. In the second patch, the LBR test has been moved to runtime check. This case has been handled. Kan > > -Andi -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > On Wed, Jul 2, 2014 at 2:14 PM, <kan.liang@intel.com> wrote: > > From: Kan Liang <kan.liang@intel.com> > > > > x86, perf: Protect LBR and offcore rsp against KVM lying > > > > With -cpu host, KVM reports LBR and offcore support, if the host has > support. > > When the guest perf driver tries to access LBR or offcore_rsp MSR, it > > #GPs all MSR accesses,since KVM doesn't handle LBR and offcore support. > > So check the related MSRs access right once at initialization time to avoid > the error access at runtime. > > > > For reproducing the issue, please build the kernel with > CONFIG_KVM_INTEL = y. > This is for host kernel, > > And CONFIG_PARAVIRT = n and CONFIG_KVM_GUEST = n. > And this is for guest kernel, right? > Right. > -Jidong
diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h index 3b2f9bd..5d977b2 100644 --- a/arch/x86/kernel/cpu/perf_event.h +++ b/arch/x86/kernel/cpu/perf_event.h @@ -458,12 +458,13 @@ struct x86_pmu { u64 lbr_sel_mask; /* LBR_SELECT valid bits */ const int *lbr_sel_map; /* lbr_select mappings */ bool lbr_double_abort; /* duplicated lbr aborts */ - + bool lbr_msr_access; /* LBR MSR can be accessed */ /* * Extra registers for events */ struct extra_reg *extra_regs; unsigned int er_flags; + bool extra_msr_access; /* EXTRA REG MSR can be accessed */ /* * Intel host/guest support (KVM) @@ -525,6 +526,20 @@ extern u64 __read_mostly hw_cache_extra_regs [PERF_COUNT_HW_CACHE_OP_MAX] [PERF_COUNT_HW_CACHE_RESULT_MAX]; +/* + * Under certain circumstances, access certain MSR may cause #GP. + * The function tests if the input MSR can be safely accessed. + */ +static inline bool test_msr_access(unsigned long msr) +{ + u64 value; + if (rdmsrl_safe(msr, &value) < 0) + return false; + if (wrmsrl_safe(msr, value) < 0) + return false; + return true; +} + u64 x86_perf_event_update(struct perf_event *event); static inline unsigned int x86_pmu_config_addr(int index) @@ -555,7 +570,7 @@ static inline void __x86_pmu_enable_event(struct hw_perf_event *hwc, { u64 disable_mask = __this_cpu_read(cpu_hw_events.perf_ctr_virt_mask); - if (hwc->extra_reg.reg) + if (hwc->extra_reg.reg && x86_pmu.extra_msr_access) wrmsrl(hwc->extra_reg.reg, hwc->extra_reg.config); wrmsrl(hwc->config_base, (hwc->config | enable_mask) & ~disable_mask); } diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c index adb02aa..8011d42 100644 --- a/arch/x86/kernel/cpu/perf_event_intel.c +++ b/arch/x86/kernel/cpu/perf_event_intel.c @@ -2565,6 +2565,13 @@ __init int intel_pmu_init(void) } } + /* Access LBR MSR may cause #GP under certain circumstances. E.g. KVM doesn't support LBR MSR */ + if (x86_pmu.lbr_nr) + x86_pmu.lbr_msr_access = test_msr_access(x86_pmu.lbr_tos) & test_msr_access(x86_pmu.lbr_from); + /* Access extra MSR may cause #GP under certain circumstances. E.g. KVM doesn't support offcore event */ + if (x86_pmu.extra_regs) + x86_pmu.extra_msr_access = test_msr_access(x86_pmu.extra_regs->msr); + /* Support full width counters using alternative MSR range */ if (x86_pmu.intel_cap.full_width_write) { x86_pmu.max_period = x86_pmu.cntval_mask; diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c b/arch/x86/kernel/cpu/perf_event_intel_lbr.c index 9dd2459..9508d1e 100644 --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c @@ -172,7 +172,7 @@ static void intel_pmu_lbr_reset_64(void) void intel_pmu_lbr_reset(void) { - if (!x86_pmu.lbr_nr) + if (!x86_pmu.lbr_nr || !x86_pmu.lbr_msr_access) return; if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_32) @@ -334,7 +334,7 @@ void intel_pmu_lbr_read(void) { struct cpu_hw_events *cpuc = &__get_cpu_var(cpu_hw_events); - if (!cpuc->lbr_users) + if (!cpuc->lbr_users || !x86_pmu.lbr_msr_access) return; if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_32)