Skip to contentSkip to conversation

x86/perf/intel: Introduce PMU flag for Extended PEBS

PATCH In review [email protected] 3 participants 16 messages Mar 9, 2018 – Jul 25, 2018

Subsystems (2)
x86 perf events
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/4 x86/perf/intel: Introduce PMU flag for Extended PEBS

From: Kan Liang <[email protected]>

The Extended PEBS feature, introduced in Goldmont Plus microarchitecture, supports all events as "Extended PEBS".

Introduce flag PMU_FL_PEBS_ALL to indicate the platforms which support extended PEBS.
To support all events, it needs to support all constraints for PEBS. To avoid duplicating all the constraints in the PEBS table, making the PEBS code search the normal constraints too.

Based-on-code-from: Andi Kleen <[email protected]>

Signed-off-by Kan Liang <[email protected]>
---
 arch/x86/events/intel/ds.c   | 7 +++++++
 arch/x86/events/perf_event.h | 1 +
 2 files changed, 8 insertions(+)
arch/x86/events/intel/ds.c +7 -0
index 0366cd3159ca..4fa43f65835e 100644
@@ -869,6 +869,13 @@ struct event_constraint *intel_pebs_constraints(struct perf_event *event)
 		}
 	}
 
+	/*
+	 * Extended PEBS support
+	 * Makes the PEBS code search the normal constraints.
+	 */
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		return NULL;
+
 	return &emptyconstraint;
 }
arch/x86/events/perf_event.h +1 -0
index bb90184410ab..598892e001b5 100644
@@ -668,6 +668,7 @@ do {									\
 #define PMU_FL_HAS_RSP_1	0x2 /* has 2 equivalent offcore_rsp regs   */
 #define PMU_FL_EXCL_CNTRS	0x4 /* has exclusive counter requirements  */
 #define PMU_FL_EXCL_ENABLED	0x8 /* exclusive counter active */
+#define PMU_FL_PEBS_ALL		0x10 /* all events are valid PEBS events */
 
 #define EVENT_VAR(_id)  event_attr_##_id
 #define EVENT_PTR(_id) &event_attr_##_id.attr.attr
Signature · 1 line
2.14.3
author
#1 lore raw

PATCH 2/4 x86/perf/intel: Support PEBS on fixed counters

From: Kan Liang <[email protected]>

The Extended PEBS feature supports PEBS on fixed-function performance counters as well as all four general purpose counters.

It has to change the order of PEBS and fixed counter enabling to make sure PEBS is enabled for the fixed counters.
The change of the order doesn't impact the behavior of current code on other platforms which don't support extended PEBS.
Because there is no dependency among those enable/disable functions.

Don't enable IRQ generation (0x8) for MSR_ARCH_PERFMON_FIXED_CTR_CTRL. The PEBS ucode will handle the interrupt generation.

Based-on-code-from: Andi Kleen <[email protected]>

Signed-off-by Kan Liang <[email protected]>
---
 arch/x86/events/intel/core.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
arch/x86/events/intel/core.c +14 -10
index a6a70ba5da85..ef47a418d819 100644
@@ -2041,15 +2041,15 @@ 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(event->attr.precise_ip))
+		intel_pmu_pebs_disable(event);
+
 	if (unlikely(hwc->config_base == MSR_ARCH_PERFMON_FIXED_CTR_CTRL)) {
 		intel_pmu_disable_fixed(hwc);
 		return;
 	}
 
 	x86_pmu_disable_event(event);
-
-	if (unlikely(event->attr.precise_ip))
-		intel_pmu_pebs_disable(event);
 }
 
 static void intel_pmu_del_event(struct perf_event *event)

@@ -2068,17 +2068,21 @@ static void intel_pmu_read_event(struct perf_event *event)
 		x86_perf_event_update(event);
 }
 
-static void intel_pmu_enable_fixed(struct hw_perf_event *hwc)
+static void intel_pmu_enable_fixed(struct perf_event *event)
 {
+	struct hw_perf_event *hwc = &event->hw;
 	int idx = hwc->idx - INTEL_PMC_IDX_FIXED;
 	u64 ctrl_val, bits, mask;
 
 	/*
-	 * Enable IRQ generation (0x8),
+	 * Enable IRQ generation (0x8), if not PEBS,
 	 * and enable ring-3 counting (0x2) and ring-0 counting (0x1)
 	 * if requested:
 	 */
-	bits = 0x8ULL;
+	if (event->attr.precise_ip)
+		bits = 0;
+	else
+		bits = 0x8ULL;
 	if (hwc->config & ARCH_PERFMON_EVENTSEL_USR)
 		bits |= 0x2;
 	if (hwc->config & ARCH_PERFMON_EVENTSEL_OS)

@@ -2120,14 +2124,14 @@ static void intel_pmu_enable_event(struct perf_event *event)
 	if (unlikely(event_is_checkpointed(event)))
 		cpuc->intel_cp_status |= (1ull << hwc->idx);
 
+	if (unlikely(event->attr.precise_ip))
+		intel_pmu_pebs_enable(event);
+
 	if (unlikely(hwc->config_base == MSR_ARCH_PERFMON_FIXED_CTR_CTRL)) {
-		intel_pmu_enable_fixed(hwc);
+		intel_pmu_enable_fixed(event);
 		return;
 	}
 
-	if (unlikely(event->attr.precise_ip))
-		intel_pmu_pebs_enable(event);
-
 	__x86_pmu_enable_event(hwc, ARCH_PERFMON_EVENTSEL_ENABLE);
 }
Signature · 1 line
2.14.3
merge bot
#2 lore raw

[tip:perf/core] perf/x86/intel: Support PEBS on fixed counters

perf/x86/intel: Support PEBS on fixed counters
Commit-ID:  4f08b6255adb1e379b4fcc8d304ec1263d465677
Gitweb:     https://git.kernel.org/tip/4f08b6255adb1e379b4fcc8d304ec1263d465677
Author:     Kan Liang <[email protected]>
AuthorDate: Thu, 8 Mar 2018 18:15:40 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 25 Jul 2018 11:50:50 +0200

perf/x86/intel: Support PEBS on fixed counters

The Extended PEBS feature supports PEBS on fixed-function performance counters as well as all four general purpose counters.

It has to change the order of PEBS and fixed counter enabling to make sure PEBS is enabled for the fixed counters.

The change of the order doesn't impact the behavior of current code on other platforms which don't support extended PEBS.
Because there is no dependency among those enable/disable functions.

Don't enable IRQ generation (0x8) for MSR_ARCH_PERFMON_FIXED_CTR_CTRL. The PEBS ucode will handle the interrupt generation.

Based-on-code-from: Andi Kleen <[email protected]>

Signed-off-by Kan Liang <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Vince Weaver <[email protected]>
Cc [email protected]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 arch/x86/events/intel/core.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
arch/x86/events/intel/core.c +13 -11
index 86f0c15dcc2d..d5a3124605f5 100644
@@ -2041,15 +2041,15 @@ 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(event->attr.precise_ip))
+		intel_pmu_pebs_disable(event);
+
 	if (unlikely(hwc->config_base == MSR_ARCH_PERFMON_FIXED_CTR_CTRL)) {
 		intel_pmu_disable_fixed(hwc);
 		return;
 	}
 
 	x86_pmu_disable_event(event);
-
-	if (unlikely(event->attr.precise_ip))
-		intel_pmu_pebs_disable(event);
 }
 
 static void intel_pmu_del_event(struct perf_event *event)

@@ -2068,17 +2068,19 @@ static void intel_pmu_read_event(struct perf_event *event)
 		x86_perf_event_update(event);
 }
 
-static void intel_pmu_enable_fixed(struct hw_perf_event *hwc)
+static void intel_pmu_enable_fixed(struct perf_event *event)
 {
+	struct hw_perf_event *hwc = &event->hw;
 	int idx = hwc->idx - INTEL_PMC_IDX_FIXED;
-	u64 ctrl_val, bits, mask;
+	u64 ctrl_val, mask, bits = 0;
 
 	/*
-	 * Enable IRQ generation (0x8),
+	 * Enable IRQ generation (0x8), if not PEBS,
 	 * and enable ring-3 counting (0x2) and ring-0 counting (0x1)
 	 * if requested:
 	 */
-	bits = 0x8ULL;
+	if (!event->attr.precise_ip)
+		bits |= 0x8;
 	if (hwc->config & ARCH_PERFMON_EVENTSEL_USR)
 		bits |= 0x2;
 	if (hwc->config & ARCH_PERFMON_EVENTSEL_OS)

@@ -2120,14 +2122,14 @@ static void intel_pmu_enable_event(struct perf_event *event)
 	if (unlikely(event_is_checkpointed(event)))
 		cpuc->intel_cp_status |= (1ull << hwc->idx);
 
+	if (unlikely(event->attr.precise_ip))
+		intel_pmu_pebs_enable(event);
+
 	if (unlikely(hwc->config_base == MSR_ARCH_PERFMON_FIXED_CTR_CTRL)) {
-		intel_pmu_enable_fixed(hwc);
+		intel_pmu_enable_fixed(event);
 		return;
 	}
 
-	if (unlikely(event->attr.precise_ip))
-		intel_pmu_pebs_enable(event);
-
 	__x86_pmu_enable_event(hwc, ARCH_PERFMON_EVENTSEL_ENABLE);
 }
author
#3 lore raw

PATCH 3/4 perf/x86/intel/ds: Handle PEBS overflow for fixed counters

In reply to [email protected]

From: Kan Liang <[email protected]>

The pebs_drain() need to support fixed counters. The DS Save Area now include "counter reset value" fields for each fixed counters.

Extend the related variables (e.g. mask, counters, error) to support fixed counters. There is no extended PEBS in PEBS v2 and earlier PEBS format. Only need to change the code for PEBS v3 and later PEBS format.

Extend the pebs_event_reset to support new "counter reset value" fields.

Increase the reserve space for fixed counters.

Based-on-code-from: Andi Kleen <[email protected]>

Signed-off-by Kan Liang <[email protected]>
---
 arch/x86/events/intel/core.c    |  5 ++++-
 arch/x86/events/intel/ds.c      | 36 +++++++++++++++++++++++++++---------
 arch/x86/events/perf_event.h    |  3 +++
 arch/x86/include/asm/intel_ds.h |  3 ++-
 4 files changed, 36 insertions(+), 11 deletions(-)
arch/x86/events/intel/core.c +4 -1
index ef47a418d819..86149b87cce8 100644
@@ -2280,7 +2280,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 	 * counters from the GLOBAL_STATUS mask and we always process PEBS
 	 * events via drain_pebs().
 	 */
-	status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		status &= ~(cpuc->pebs_enabled & EXTENDED_PEBS_COUNTER_MASK);
+	else
+		status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
 
 	/*
 	 * PEBS overflow sets bit 62 in the global status register
arch/x86/events/intel/ds.c +27 -9
index 4fa43f65835e..48b31319e401 100644
@@ -901,10 +901,16 @@ static inline void pebs_update_threshold(struct cpu_hw_events *cpuc)
 {
 	struct debug_store *ds = cpuc->ds;
 	u64 threshold;
+	int reserved;
+
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		reserved = x86_pmu.max_pebs_events + x86_pmu.num_counters_fixed;
+	else
+		reserved = x86_pmu.max_pebs_events;
 
 	if (cpuc->n_pebs == cpuc->n_large_pebs) {
 		threshold = ds->pebs_absolute_maximum -
-			x86_pmu.max_pebs_events * x86_pmu.pebs_record_size;
+			reserved * x86_pmu.pebs_record_size;
 	} else {
 		threshold = ds->pebs_buffer_base + x86_pmu.pebs_record_size;
 	}

@@ -968,7 +974,11 @@ void intel_pmu_pebs_enable(struct perf_event *event)
 	 * This must be done in pmu::start(), because PERF_EVENT_IOC_PERIOD.
 	 */
 	if (hwc->flags & PERF_X86_EVENT_AUTO_RELOAD) {
-		ds->pebs_event_reset[hwc->idx] =
+		unsigned int idx = hwc->idx;
+
+		if (idx >= INTEL_PMC_IDX_FIXED)
+			idx = MAX_PEBS_EVENTS + (idx - INTEL_PMC_IDX_FIXED);
+		ds->pebs_event_reset[idx] =
 			(u64)(-hwc->sample_period) & x86_pmu.cntval_mask;
 	} else {
 		ds->pebs_event_reset[hwc->idx] = 0;

@@ -1466,9 +1476,10 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 	struct debug_store *ds = cpuc->ds;
 	struct perf_event *event;
 	void *base, *at, *top;
-	short counts[MAX_PEBS_EVENTS] = {};
-	short error[MAX_PEBS_EVENTS] = {};
-	int bit, i;
+	short counts[INTEL_PMC_IDX_FIXED + MAX_FIXED_PEBS_EVENTS] = {};
+	short error[INTEL_PMC_IDX_FIXED + MAX_FIXED_PEBS_EVENTS] = {};
+	int bit, i, size;
+	u64 mask;
 
 	if (!x86_pmu.pebs_active)
 		return;

@@ -1478,6 +1489,13 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 
 	ds->pebs_index = ds->pebs_buffer_base;
 
+	mask = (1ULL << x86_pmu.max_pebs_events) - 1;
+	size = x86_pmu.max_pebs_events;
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL) {
+		mask |= ((1ULL << x86_pmu.num_counters_fixed) - 1) << INTEL_PMC_IDX_FIXED;
+		size = INTEL_PMC_IDX_FIXED + x86_pmu.num_counters_fixed;
+	}
+
 	if (unlikely(base >= top)) {
 		/*
 		 * The drain_pebs() could be called twice in a short period

@@ -1487,7 +1505,7 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		 * update the event->count for this case.
 		 */
 		for_each_set_bit(bit, (unsigned long *)&cpuc->pebs_enabled,
-				 x86_pmu.max_pebs_events) {
+				 size) {
 			event = cpuc->events[bit];
 			if (event->hw.flags & PERF_X86_EVENT_AUTO_RELOAD)
 				intel_pmu_save_and_restart_reload(event, 0);

@@ -1500,12 +1518,12 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		u64 pebs_status;
 
 		pebs_status = p->status & cpuc->pebs_enabled;
-		pebs_status &= (1ULL << x86_pmu.max_pebs_events) - 1;
+		pebs_status &= mask;
 
 		/* PEBS v3 has more accurate status bits */
 		if (x86_pmu.intel_cap.pebs_format >= 3) {
 			for_each_set_bit(bit, (unsigned long *)&pebs_status,
-					 x86_pmu.max_pebs_events)
+					 size)
 				counts[bit]++;
 
 			continue;

@@ -1553,7 +1571,7 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		counts[bit]++;
 	}
 
-	for (bit = 0; bit < x86_pmu.max_pebs_events; bit++) {
+	for (bit = 0; bit < size; bit++) {
 		if ((counts[bit] == 0) && (error[bit] == 0))
 			continue;
arch/x86/events/perf_event.h +3 -0
index 598892e001b5..b4564c01f394 100644
@@ -80,6 +80,9 @@ struct amd_nb {
 };
 
 #define PEBS_COUNTER_MASK	((1ULL << MAX_PEBS_EVENTS) - 1)
+#define EXTENDED_PEBS_COUNTER_MASK	\
+	(PEBS_COUNTER_MASK |		\
+	(((1ULL << MAX_FIXED_PEBS_EVENTS) - 1) << INTEL_PMC_IDX_FIXED))
 
 /*
  * Flags PEBS can handle without an PMI.
arch/x86/include/asm/intel_ds.h +2 -1
index 62a9f4966b42..ae26df1c2789 100644
@@ -8,6 +8,7 @@
 
 /* The maximal number of PEBS events: */
 #define MAX_PEBS_EVENTS		8
+#define MAX_FIXED_PEBS_EVENTS	3
 
 /*
  * A debug store configuration.

@@ -23,7 +24,7 @@ struct debug_store {
 	u64	pebs_index;
 	u64	pebs_absolute_maximum;
 	u64	pebs_interrupt_threshold;
-	u64	pebs_event_reset[MAX_PEBS_EVENTS];
+	u64	pebs_event_reset[MAX_PEBS_EVENTS + MAX_FIXED_PEBS_EVENTS];
 } __aligned(PAGE_SIZE);
 
 DECLARE_PER_CPU_PAGE_ALIGNED(struct debug_store, cpu_debug_store);
Signature · 1 line
2.14.3
maintainer
#4 lore raw

Re: [PATCH 3/4] perf/x86/intel/ds: Handle PEBS overflow for fixed counters

Quoted 6 lines · On Thu, Mar 08, 2018 at 06:15:41PM -0800, [email protected] wrote:
arch/x86/events/intel/core.c +0 -0
index ef47a418d819..86149b87cce8 100644
@@ -2280,7 +2280,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 	 * counters from the GLOBAL_STATUS mask and we always process PEBS

arch/x86/events/intel/core.c static int intel_pmu_handle_irq(struct pt_regs *regs) · line 2289

Quoted 10 lines
arch/x86/events/intel/core.c +4 -1
 	 * events via drain_pebs().
 	 */
-	status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		status &= ~(cpuc->pebs_enabled & EXTENDED_PEBS_COUNTER_MASK);
+	else
+		status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
 
 	/*
 	 * PEBS overflow sets bit 62 in the global status register

Doesn't this re-introduce the problem fixed in commit fd583ad1563be, where pebs_enabled:32-34 are PEBS Load Latency, instead of fixed counters?

maintainer
#5 lore raw

Re: [PATCH 3/4] perf/x86/intel/ds: Handle PEBS overflow for fixed counters

Quoted 15 lines · Peter Zijlstra
Quoted 14 lines · On Thu, Mar 08, 2018 at 06:15:41PM -0800, [email protected] wrote:
arch/x86/events/intel/core.c +4 -1
index ef47a418d819..86149b87cce8 100644
@@ -2280,7 +2280,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 	 * counters from the GLOBAL_STATUS mask and we always process PEBS
 	 * events via drain_pebs().
 	 */
-	status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		status &= ~(cpuc->pebs_enabled & EXTENDED_PEBS_COUNTER_MASK);
+	else
+		status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
Quoted 6 lines

arch/x86/events/intel/core.c static int intel_pmu_handle_irq(struct pt_regs *regs) · line 2289

Quoted 2 lines
arch/x86/events/intel/core.c +0 -0
 	/*
 	 * PEBS overflow sets bit 62 in the global status register

Doesn't this re-introduce the problem fixed in commit fd583ad1563be, where pebs_enabled:32-34 are PEBS Load Latency, instead of fixed counters?

*GROAN* the MSR definitions now differ between Core and Atom :-(

maintainer
#6 lore raw

Re: [PATCH 3/4] perf/x86/intel/ds: Handle PEBS overflow for fixed counters

In reply to Peter Zijlstra
Quoted 15 lines · Peter Zijlstra
Quoted 14 lines · On Thu, Mar 08, 2018 at 06:15:41PM -0800, [email protected] wrote:
arch/x86/events/intel/core.c +4 -1
index ef47a418d819..86149b87cce8 100644
@@ -2280,7 +2280,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 	 * counters from the GLOBAL_STATUS mask and we always process PEBS
 	 * events via drain_pebs().
 	 */
-	status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		status &= ~(cpuc->pebs_enabled & EXTENDED_PEBS_COUNTER_MASK);
+	else
+		status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
Quoted 6 lines

arch/x86/events/intel/core.c static int intel_pmu_handle_irq(struct pt_regs *regs) · line 2289

Quoted 2 lines
arch/x86/events/intel/core.c +0 -0
 	/*
 	 * PEBS overflow sets bit 62 in the global status register

Doesn't this re-introduce the problem fixed in commit fd583ad1563be, where pebs_enabled:32-34 are PEBS Load Latency, instead of fixed counters?

Also, since they 'fixed' that conflict, the PEBS_ALL version could be:

	state &= cpuc->pebs_enabled;

Right?

author
#7 lore raw

Re: [PATCH 3/4] perf/x86/intel/ds: Handle PEBS overflow for fixed counters

Quoted 23 lines · Peter Zijlstra
Quoted 15 lines · Peter Zijlstra
Quoted 14 lines · On Thu, Mar 08, 2018 at 06:15:41PM -0800, [email protected] wrote:
arch/x86/events/intel/core.c +4 -1
index ef47a418d819..86149b87cce8 100644
@@ -2280,7 +2280,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
  	 * counters from the GLOBAL_STATUS mask and we always process PEBS
  	 * events via drain_pebs().
  	 */
-	status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		status &= ~(cpuc->pebs_enabled & EXTENDED_PEBS_COUNTER_MASK);
+	else
+		status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
Quoted 6 lines

arch/x86/events/intel/core.c static int intel_pmu_handle_irq(struct pt_regs *regs) · line 2289

Quoted 2 lines
arch/x86/events/intel/core.c +0 -0
  	/*
  	 * PEBS overflow sets bit 62 in the global status register

Doesn't this re-introduce the problem fixed in commit fd583ad1563be, where pebs_enabled:32-34 are PEBS Load Latency, instead of fixed counters?

Quoted 5 lines

Also, since they 'fixed' that conflict, the PEBS_ALL version could be:

	state &= cpuc->pebs_enabled;

Right?

Right.

Thanks,
Kan

author
#8 lore raw

Re: [PATCH 3/4] perf/x86/intel/ds: Handle PEBS overflow for fixed counters

Quoted 26 lines · Kan
Quoted 23 lines · Peter Zijlstra
Quoted 21 lines · Peter Zijlstra
Quoted 6 lines · On Thu, Mar 08, 2018 at 06:15:41PM -0800, [email protected] wrote:
arch/x86/events/intel/core.c +0 -0
index ef47a418d819..86149b87cce8 100644
@@ -2280,7 +2280,10 @@ static int intel_pmu_handle_irq(struct 
 pt_regs *regs)

arch/x86/events/intel/core.c static int intel_pmu_handle_irq(struct · line 2289

Quoted 10 lines
arch/x86/events/intel/core.c +4 -1
        * counters from the GLOBAL_STATUS mask and we always process PEBS
        * events via drain_pebs().
        */
-    status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+    if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+        status &= ~(cpuc->pebs_enabled & EXTENDED_PEBS_COUNTER_MASK);
+    else
+        status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
       /*
        * PEBS overflow sets bit 62 in the global status register

Doesn't this re-introduce the problem fixed in commit fd583ad1563be, where pebs_enabled:32-34 are PEBS Load Latency, instead of fixed counters?

Quoted 6 lines
Quoted 5 lines

Also, since they 'fixed' that conflict, the PEBS_ALL version could be:

    state &= cpuc->pebs_enabled;

Right?

Here, we need to clear the bit for PEBS counters.
For PEBS_ALL version, it should be
status &= ~cpuc->pebs_enabled;

Thanks,
Kan

merge bot
#9 lore raw

[tip:perf/core] perf/x86/intel/ds: Handle PEBS overflow for fixed counters

In reply to [email protected]
perf/x86/intel/ds: Handle PEBS overflow for fixed counters
Commit-ID:  ec71a398c1bf6d8188cb24ebab6f5202523d95e1
Gitweb:     https://git.kernel.org/tip/ec71a398c1bf6d8188cb24ebab6f5202523d95e1
Author:     Kan Liang <[email protected]>
AuthorDate: Thu, 8 Mar 2018 18:15:41 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 25 Jul 2018 11:50:50 +0200

perf/x86/intel/ds: Handle PEBS overflow for fixed counters

The pebs_drain() need to support fixed counters. The DS Save Area now include "counter reset value" fields for each fixed counters.

Extend the related variables (e.g. mask, counters, error) to support fixed counters. There is no extended PEBS in PEBS v2 and earlier PEBS format. Only need to change the code for PEBS v3 and later PEBS format.

Extend the pebs_event_reset[] logic to support new "counter reset value" fields.

Increase the reserve space for fixed counters.

Based-on-code-from: Andi Kleen <[email protected]>

Signed-off-by Kan Liang <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Vince Weaver <[email protected]>
Cc [email protected]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 arch/x86/events/intel/core.c    |  5 ++++-
 arch/x86/events/intel/ds.c      | 36 +++++++++++++++++++++++++++---------
 arch/x86/include/asm/intel_ds.h |  3 ++-
 3 files changed, 33 insertions(+), 11 deletions(-)
arch/x86/events/intel/core.c +4 -1
index d5a3124605f5..b1a49a108a59 100644
@@ -2282,7 +2282,10 @@ again:
 	 * counters from the GLOBAL_STATUS mask and we always process PEBS
 	 * events via drain_pebs().
 	 */
-	status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		status &= ~cpuc->pebs_enabled;
+	else
+		status &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
 
 	/*
 	 * PEBS overflow sets bit 62 in the global status register
arch/x86/events/intel/ds.c +27 -9
index 9fd9cb1d2cc8..595b96ae8a00 100644
@@ -903,10 +903,16 @@ static inline void pebs_update_threshold(struct cpu_hw_events *cpuc)
 {
 	struct debug_store *ds = cpuc->ds;
 	u64 threshold;
+	int reserved;
+
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		reserved = x86_pmu.max_pebs_events + x86_pmu.num_counters_fixed;
+	else
+		reserved = x86_pmu.max_pebs_events;
 
 	if (cpuc->n_pebs == cpuc->n_large_pebs) {
 		threshold = ds->pebs_absolute_maximum -
-			x86_pmu.max_pebs_events * x86_pmu.pebs_record_size;
+			reserved * x86_pmu.pebs_record_size;
 	} else {
 		threshold = ds->pebs_buffer_base + x86_pmu.pebs_record_size;
 	}

@@ -970,7 +976,11 @@ void intel_pmu_pebs_enable(struct perf_event *event)
 	 * This must be done in pmu::start(), because PERF_EVENT_IOC_PERIOD.
 	 */
 	if (hwc->flags & PERF_X86_EVENT_AUTO_RELOAD) {
-		ds->pebs_event_reset[hwc->idx] =
+		unsigned int idx = hwc->idx;
+
+		if (idx >= INTEL_PMC_IDX_FIXED)
+			idx = MAX_PEBS_EVENTS + (idx - INTEL_PMC_IDX_FIXED);
+		ds->pebs_event_reset[idx] =
 			(u64)(-hwc->sample_period) & x86_pmu.cntval_mask;
 	} else {
 		ds->pebs_event_reset[hwc->idx] = 0;

@@ -1488,9 +1498,10 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 	struct debug_store *ds = cpuc->ds;
 	struct perf_event *event;
 	void *base, *at, *top;
-	short counts[MAX_PEBS_EVENTS] = {};
-	short error[MAX_PEBS_EVENTS] = {};
-	int bit, i;
+	short counts[INTEL_PMC_IDX_FIXED + MAX_FIXED_PEBS_EVENTS] = {};
+	short error[INTEL_PMC_IDX_FIXED + MAX_FIXED_PEBS_EVENTS] = {};
+	int bit, i, size;
+	u64 mask;
 
 	if (!x86_pmu.pebs_active)
 		return;

@@ -1500,6 +1511,13 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 
 	ds->pebs_index = ds->pebs_buffer_base;
 
+	mask = (1ULL << x86_pmu.max_pebs_events) - 1;
+	size = x86_pmu.max_pebs_events;
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL) {
+		mask |= ((1ULL << x86_pmu.num_counters_fixed) - 1) << INTEL_PMC_IDX_FIXED;
+		size = INTEL_PMC_IDX_FIXED + x86_pmu.num_counters_fixed;
+	}
+
 	if (unlikely(base >= top)) {
 		/*
 		 * The drain_pebs() could be called twice in a short period

@@ -1509,7 +1527,7 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		 * update the event->count for this case.
 		 */
 		for_each_set_bit(bit, (unsigned long *)&cpuc->pebs_enabled,
-				 x86_pmu.max_pebs_events) {
+				 size) {
 			event = cpuc->events[bit];
 			if (event->hw.flags & PERF_X86_EVENT_AUTO_RELOAD)
 				intel_pmu_save_and_restart_reload(event, 0);

@@ -1522,12 +1540,12 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		u64 pebs_status;
 
 		pebs_status = p->status & cpuc->pebs_enabled;
-		pebs_status &= (1ULL << x86_pmu.max_pebs_events) - 1;
+		pebs_status &= mask;
 
 		/* PEBS v3 has more accurate status bits */
 		if (x86_pmu.intel_cap.pebs_format >= 3) {
 			for_each_set_bit(bit, (unsigned long *)&pebs_status,
-					 x86_pmu.max_pebs_events)
+					 size)
 				counts[bit]++;
 
 			continue;

@@ -1575,7 +1593,7 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		counts[bit]++;
 	}
 
-	for (bit = 0; bit < x86_pmu.max_pebs_events; bit++) {
+	for (bit = 0; bit < size; bit++) {
 		if ((counts[bit] == 0) && (error[bit] == 0))
 			continue;
arch/x86/include/asm/intel_ds.h +2 -1
index 62a9f4966b42..ae26df1c2789 100644
@@ -8,6 +8,7 @@
 
 /* The maximal number of PEBS events: */
 #define MAX_PEBS_EVENTS		8
+#define MAX_FIXED_PEBS_EVENTS	3
 
 /*
  * A debug store configuration.

@@ -23,7 +24,7 @@ struct debug_store {
 	u64	pebs_index;
 	u64	pebs_absolute_maximum;
 	u64	pebs_interrupt_threshold;
-	u64	pebs_event_reset[MAX_PEBS_EVENTS];
+	u64	pebs_event_reset[MAX_PEBS_EVENTS + MAX_FIXED_PEBS_EVENTS];
 } __aligned(PAGE_SIZE);
 
 DECLARE_PER_CPU_PAGE_ALIGNED(struct debug_store, cpu_debug_store);
author
#10 lore raw

PATCH 4/4 perf/x86/intel: Support Extended PEBS for Goldmont Plus

In reply to [email protected]

From: Kan Liang <[email protected]>

Enable the extended PEBS for Goldmont Plus.

There is no specific PEBS constrains for Goldmont Plus. Removing the pebs_constraints for Goldmont Plus.

Signed-off-by Kan Liang <[email protected]>
---
 arch/x86/events/intel/core.c | 2 +-
 arch/x86/events/intel/ds.c   | 6 ------
 2 files changed, 1 insertion(+), 7 deletions(-)
arch/x86/events/intel/core.c +1 -1
index 86149b87cce8..7d7c02ed714b 100644
@@ -4069,7 +4069,6 @@ __init int intel_pmu_init(void)
 		intel_pmu_lbr_init_skl();
 
 		x86_pmu.event_constraints = intel_slm_event_constraints;
-		x86_pmu.pebs_constraints = intel_glp_pebs_event_constraints;
 		x86_pmu.extra_regs = intel_glm_extra_regs;
 		/*
 		 * It's recommended to use CPU_CLK_UNHALTED.CORE_P + NPEBS

@@ -4079,6 +4078,7 @@ __init int intel_pmu_init(void)
 		x86_pmu.pebs_prec_dist = true;
 		x86_pmu.lbr_pt_coexist = true;
 		x86_pmu.flags |= PMU_FL_HAS_RSP_1;
+		x86_pmu.flags |= PMU_FL_PEBS_ALL;
 		x86_pmu.get_event_constraints = glp_get_event_constraints;
 		x86_pmu.cpu_events = glm_events_attrs;
 		/* Goldmont Plus has 4-wide pipeline */
arch/x86/events/intel/ds.c +0 -6
index 48b31319e401..c1749bb77417 100644
@@ -711,12 +711,6 @@ struct event_constraint intel_glm_pebs_event_constraints[] = {
 	EVENT_CONSTRAINT_END
 };
 
-struct event_constraint intel_glp_pebs_event_constraints[] = {
-	/* Allow all events as PEBS with no flags */
-	INTEL_ALL_EVENT_CONSTRAINT(0, 0xf),
-	EVENT_CONSTRAINT_END
-};
-
 struct event_constraint intel_nehalem_pebs_event_constraints[] = {
 	INTEL_PLD_CONSTRAINT(0x100b, 0xf),      /* MEM_INST_RETIRED.* */
 	INTEL_FLAGS_EVENT_CONSTRAINT(0x0f, 0xf),    /* MEM_UNCORE_RETIRED.* */
Signature · 1 line
2.14.3
merge bot
#11 lore raw

[tip:perf/core] perf/x86/intel: Support Extended PEBS for Goldmont Plus

perf/x86/intel: Support Extended PEBS for Goldmont Plus
Commit-ID:  a38b0ba1b7d2e7a6d19877540240e8a4352fc93c
Gitweb:     https://git.kernel.org/tip/a38b0ba1b7d2e7a6d19877540240e8a4352fc93c
Author:     Kan Liang <[email protected]>
AuthorDate: Thu, 8 Mar 2018 18:15:42 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 25 Jul 2018 11:50:50 +0200

perf/x86/intel: Support Extended PEBS for Goldmont Plus

Enable the extended PEBS for Goldmont Plus.

There is no specific PEBS constrains for Goldmont Plus. Removing the pebs_constraints for Goldmont Plus.

Signed-off-by Kan Liang <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Vince Weaver <[email protected]>
Cc [email protected]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 arch/x86/events/intel/core.c | 2 +-
 arch/x86/events/intel/ds.c   | 6 ------
 2 files changed, 1 insertion(+), 7 deletions(-)
arch/x86/events/intel/core.c +1 -1
index b1a49a108a59..035c37481f57 100644
@@ -4077,7 +4077,6 @@ __init int intel_pmu_init(void)
 		intel_pmu_lbr_init_skl();
 
 		x86_pmu.event_constraints = intel_slm_event_constraints;
-		x86_pmu.pebs_constraints = intel_glp_pebs_event_constraints;
 		x86_pmu.extra_regs = intel_glm_extra_regs;
 		/*
 		 * It's recommended to use CPU_CLK_UNHALTED.CORE_P + NPEBS

@@ -4087,6 +4086,7 @@ __init int intel_pmu_init(void)
 		x86_pmu.pebs_prec_dist = true;
 		x86_pmu.lbr_pt_coexist = true;
 		x86_pmu.flags |= PMU_FL_HAS_RSP_1;
+		x86_pmu.flags |= PMU_FL_PEBS_ALL;
 		x86_pmu.get_event_constraints = glp_get_event_constraints;
 		x86_pmu.cpu_events = glm_events_attrs;
 		/* Goldmont Plus has 4-wide pipeline */
arch/x86/events/intel/ds.c +0 -6
index 595b96ae8a00..b7b01d762d32 100644
@@ -713,12 +713,6 @@ struct event_constraint intel_glm_pebs_event_constraints[] = {
 	EVENT_CONSTRAINT_END
 };
 
-struct event_constraint intel_glp_pebs_event_constraints[] = {
-	/* Allow all events as PEBS with no flags */
-	INTEL_ALL_EVENT_CONSTRAINT(0, 0xf),
-	EVENT_CONSTRAINT_END
-};
-
 struct event_constraint intel_nehalem_pebs_event_constraints[] = {
 	INTEL_PLD_CONSTRAINT(0x100b, 0xf),      /* MEM_INST_RETIRED.* */
 	INTEL_FLAGS_EVENT_CONSTRAINT(0x0f, 0xf),    /* MEM_UNCORE_RETIRED.* */
maintainer
#12 lore raw
In reply to [email protected]
Quoted 11 lines · On Thu, Mar 08, 2018 at 06:15:39PM -0800, [email protected] wrote:

From: Kan Liang <[email protected]>

The Extended PEBS feature, introduced in Goldmont Plus microarchitecture, supports all events as "Extended PEBS".

Introduce flag PMU_FL_PEBS_ALL to indicate the platforms which support extended PEBS.
To support all events, it needs to support all constraints for PEBS. To avoid duplicating all the constraints in the PEBS table, making the PEBS code search the normal constraints too.

So I like PEBS_ALL.. what I don't like is that it seems to be mutually exclusive with PEBS Load Latency.

Took the patches.

author
#13 lore raw
Quoted 13 lines · Peter Zijlstra
Quoted 11 lines · On Thu, Mar 08, 2018 at 06:15:39PM -0800, [email protected] wrote:

From: Kan Liang <[email protected]>

The Extended PEBS feature, introduced in Goldmont Plus microarchitecture, supports all events as "Extended PEBS".

Introduce flag PMU_FL_PEBS_ALL to indicate the platforms which support extended PEBS.
To support all events, it needs to support all constraints for PEBS. To avoid duplicating all the constraints in the PEBS table, making the PEBS code search the normal constraints too.

Quoted 2 lines

So I like PEBS_ALL.. what I don't like is that it seems to be mutually exclusive with PEBS Load Latency.

Right, MSR_PEBS_ENABLE:32-35 is model specific.
For Atom,
Goldmont and earlier platform, they are reserved. Goldmont Plus, 32-34 are for fixed counter, 35 is reserved.
For Core,
from Nehalem to latest 8th, 32-35 are for Load Latency.

Quoted 1 line

Took the patches.

Thanks.

Kan

maintainer
#14 lore raw
Quoted 4 lines · Kan
Quoted 2 lines

So I like PEBS_ALL.. what I don't like is that it seems to be mutually exclusive with PEBS Load Latency.

Right, MSR_PEBS_ENABLE:32-35 is model specific.

Doesn't mean they couldn't have avoided conflicting bits.

Quoted 5 lines

For Atom,
Goldmont and earlier platform, they are reserved. Goldmont Plus, 32-34 are for fixed counter, 35 is reserved.
For Core,
from Nehalem to latest 8th, 32-35 are for Load Latency.

Seems rather unfortunate to me. Because PEBS_ALL is good, but since they took conflicting bits, we'll have yet another variant when/if (I hope they do) they bring it to Core :/

merge bot
#15 lore raw

[tip:perf/core] perf/x86/intel: Introduce PMU flag for Extended PEBS

In reply to [email protected]
perf/x86/intel: Introduce PMU flag for Extended PEBS
Commit-ID:  3196234039155a33c80e52d7aa41a29dce9a5c51
Gitweb:     https://git.kernel.org/tip/3196234039155a33c80e52d7aa41a29dce9a5c51
Author:     Kan Liang <[email protected]>
AuthorDate: Thu, 8 Mar 2018 18:15:39 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 25 Jul 2018 11:50:49 +0200

perf/x86/intel: Introduce PMU flag for Extended PEBS

The Extended PEBS feature, introduced in the Goldmont Plus microarchitecture, supports all events as "Extended PEBS".

Introduce flag PMU_FL_PEBS_ALL to indicate the platforms which support extended PEBS.

To support all events, it needs to support all constraints for PEBS. To avoid duplicating all the constraints in the PEBS table, making the PEBS code search the normal constraints too.

Based-on-code-from: Andi Kleen <[email protected]>

Signed-off-by Kan Liang <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Vince Weaver <[email protected]>
Cc [email protected]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 arch/x86/events/intel/ds.c   | 7 +++++++
 arch/x86/events/perf_event.h | 1 +
 2 files changed, 8 insertions(+)
arch/x86/events/intel/ds.c +7 -0
index 8dbba77e0518..9fd9cb1d2cc8 100644
@@ -871,6 +871,13 @@ struct event_constraint *intel_pebs_constraints(struct perf_event *event)
 		}
 	}
 
+	/*
+	 * Extended PEBS support
+	 * Makes the PEBS code search the normal constraints.
+	 */
+	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+		return NULL;
+
 	return &emptyconstraint;
 }
arch/x86/events/perf_event.h +1 -0
index 2430398befd8..156286335351 100644
@@ -673,6 +673,7 @@ do {									\
 #define PMU_FL_HAS_RSP_1	0x2 /* has 2 equivalent offcore_rsp regs   */
 #define PMU_FL_EXCL_CNTRS	0x4 /* has exclusive counter requirements  */
 #define PMU_FL_EXCL_ENABLED	0x8 /* exclusive counter active */
+#define PMU_FL_PEBS_ALL		0x10 /* all events are valid PEBS events */
 
 #define EVENT_VAR(_id)  event_attr_##_id
 #define EVENT_PTR(_id) &event_attr_##_id.attr.attr