On Tue, Jun 25, 2019 at 07:21:35AM -0700, [email protected] wrote: > From: Kan Liang <[email protected]> > > If a user first sample a PEBS event on a fixed counter, then sample a > non-PEBS event on the same fixed counter on Icelake, it will trigger > spurious NMI. For example, > > perf record -e 'cycles:p' -a > perf record -e 'cycles' -a > > The error message for spurious NMI. > > [June 21 15:38] Uhhuh. NMI received for unknown reason 30 on CPU 2. > [ +0.000000] Do you have a strange power saving mode enabled? > [ +0.000000] Dazed and confused, but trying to continue > > The issue was introduced by the following commit: > > commit 6f55967ad9d9 ("perf/x86/intel: Fix race in > intel_pmu_disable_event()") > > The commit moves the intel_pmu_pebs_disable() after > intel_pmu_disable_fixed(), which returns immediately. > The related bit of PEBS_ENABLE MSR will never be cleared for the fixed > counter. Then a non-PEBS event runs on the fixed counter, but the bit > on PEBS_ENABLE is still set, which trigger spurious NMI. > > Check and disable PEBS for fixed counter after intel_pmu_disable_fixed(). > > Reported-by: Yi, Ammy <[email protected]> > Signed-off-by: Kan Liang <[email protected]> > Fixes: 6f55967ad9d9 ("perf/x86/intel: Fix race in intel_pmu_disable_event()") > --- > arch/x86/events/intel/core.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > index 4377bf6a6f82..464316218b77 100644 > --- a/arch/x86/events/intel/core.c > +++ b/arch/x86/events/intel/core.c > @@ -2160,12 +2160,10 @@ static void intel_pmu_disable_event(struct perf_event > *event) > cpuc->intel_ctrl_host_mask &= ~(1ull << hwc->idx); > cpuc->intel_cp_status &= ~(1ull << hwc->idx); > > - if (unlikely(hwc->config_base == MSR_ARCH_PERFMON_FIXED_CTR_CTRL)) { > + if (unlikely(hwc->config_base == MSR_ARCH_PERFMON_FIXED_CTR_CTRL)) > intel_pmu_disable_fixed(hwc); > - return; > - } > - > - x86_pmu_disable_event(event); > + else > + x86_pmu_disable_event(event); >
oops, I overlooed this, looks good Acked-by: Jiri Olsa <[email protected]> thanks, jirka > /* > * Needs to be called after x86_pmu_disable_event, > -- > 2.14.5 >
