Oleg and Sebastian found that touching MSR_IA32_DEBUGCTLMSR from NMI
context is problematic since the only way to change the various
unrelated bits in there is:
debugctl = get_debugctlmsr()
/* frob flags in debugctl */
update_debugctlmsr(debugctl);
Which is entirely unsafe if we prod at the MSR from NMI context.
In particular the path that is responsible is:
x86_pmu_handle_irq() (NMI handler)
x86_pmu_stop()
x86_pmu.disable -> intel_pmu_disable_event()
intel_pmu_lbr_disable()
__intel_pmu_lbr_disable()
wrmsrl(MSR_IA32_DEBUGCTLMSR,... );
So remove intel_pmu_lbr_{en,dis}able() from
intel_pmu_{en,dis}able_event() and stick them in
intel_{get,put}_event_constraints() which are only ever called from
regular contexts.
We combine intel_pmu_needs_lbr_smpl(), which tells us if the events
wants LBR data, with event->hw.branch_reg.alloc, which given
intel_shared_regs_constraints() is set if our event got the shared
resource, to give us a correctly balanced condition in
{get,put}_event_constraints() for intel_pmu_lbr_{en,dis}able().
Also change core_pmu to only use x86_get_event_constraints since it
doesn't support any of the fancy DS (BTS,PEBS), LBR and OFFCORE features
that make up intel_{get,put}_event_constraints.
Signed-off-by: Peter Zijlstra <[email protected]>
---
arch/x86/kernel/cpu/perf_event_intel.c | 48 ++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c
b/arch/x86/kernel/cpu/perf_event_intel.c
index 0d3d63a..ef4cd36 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -821,6 +821,11 @@ static inline bool intel_pmu_needs_lbr_smpl(struct
perf_event *event)
return false;
}
+static inline bool intel_pmu_has_lbr(struct perf_event *event)
+{
+ return intel_pmu_needs_lbr_smpl(event) && event->hw.branch_reg.alloc;
+}
+
static void intel_pmu_disable_all(void)
{
struct cpu_hw_events *cpuc = &__get_cpu_var(cpu_hw_events);
@@ -975,13 +980,6 @@ static void intel_pmu_disable_event(struct perf_event
*event)
cpuc->intel_ctrl_guest_mask &= ~(1ull << hwc->idx);
cpuc->intel_ctrl_host_mask &= ~(1ull << hwc->idx);
- /*
- * must disable before any actual event
- * because any event may be combined with LBR
- */
- if (intel_pmu_needs_lbr_smpl(event))
- intel_pmu_lbr_disable(event);
-
if (unlikely(hwc->config_base == MSR_ARCH_PERFMON_FIXED_CTR_CTRL)) {
intel_pmu_disable_fixed(hwc);
return;
@@ -1036,12 +1034,6 @@ static void intel_pmu_enable_event(struct perf_event
*event)
intel_pmu_enable_bts(hwc->config);
return;
}
- /*
- * must enabled before any actual event
- * because any event may be combined with LBR
- */
- if (intel_pmu_needs_lbr_smpl(event))
- intel_pmu_lbr_enable(event);
if (event->attr.exclude_host)
cpuc->intel_ctrl_guest_mask |= (1ull << hwc->idx);
@@ -1382,17 +1374,28 @@ intel_get_event_constraints(struct cpu_hw_events *cpuc,
struct perf_event *event
c = intel_bts_constraints(event);
if (c)
- return c;
+ goto got_constraint;
c = intel_pebs_constraints(event);
if (c)
- return c;
+ goto got_constraint;
c = intel_shared_regs_constraints(cpuc, event);
if (c)
- return c;
+ goto got_constraint;
+
+ c = x86_get_event_constraints(cpuc, event);
+
+got_constraint:
- return x86_get_event_constraints(cpuc, event);
+ /*
+ * Must enabled before any actual event because any event may be
+ * combined with LBR.
+ */
+ if (intel_pmu_has_lbr(event))
+ intel_pmu_lbr_enable(event);
+
+ return c;
}
static void
@@ -1413,6 +1416,14 @@ intel_put_shared_regs_event_constraints(struct
cpu_hw_events *cpuc,
static void intel_put_event_constraints(struct cpu_hw_events *cpuc,
struct perf_event *event)
{
+ /*
+ * Must disabled after any actual event because any event may be
+ * combined with LBR, but must be done before releasing the shared
+ * regs resource since that protects the LBR state.
+ */
+ if (intel_pmu_has_lbr(event))
+ intel_pmu_lbr_disable(event);
+
intel_put_shared_regs_event_constraints(cpuc, event);
}
@@ -1623,8 +1634,7 @@ static __initconst const struct x86_pmu core_pmu = {
* the generic event period:
*/
.max_period = (1ULL << 31) - 1,
- .get_event_constraints = intel_get_event_constraints,
- .put_event_constraints = intel_put_event_constraints,
+ .get_event_constraints = x86_get_event_constraints,
.event_constraints = intel_core_event_constraints,
.guest_get_msrs = core_guest_get_msrs,
.format_attrs = intel_arch_formats_attr,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
