[PATCH RESEND V5 0/6] perf, amd: Enable AMD family 15h northbridge counters

[PATCH RESEND V5 0/6] perf, amd: Enable AMD family 15h northbridge counters

[Jacob Shin]

The following patchset enables 4 additional performance counters in
AMD family 15h processors that count northbridge events -- such as
number of DRAM accesses.

This patchset is based on previous work done by Robert Richter
<rric@kernel.org> :

https://lkml.org/lkml/2012/6/19/324

The main differences are:

* The northbridge counters are indexed contiguously right above the
  core performance counters.

* MSR address offset calculations are moved to architecture specific
  files.

* Interrups are set up to be delivered only to a single core.

V5:
Rebased against latest tip

V4:
* Moved interrupt core select set up back to event constraints
  function, sicne during ->hw_config time we do not yet know on which
  CPU the the event will run on.
* Tested on and made minor revisions to make sure that the patchset is
  compatible with upcoming AMD Family 16h processors, and will support
  core and NB counters without any further patches.

V3:
Addressed the following feedback/comments from Robert's review
* https://lkml.org/lkml/2012/11/16/484
* https://lkml.org/lkml/2012/11/26/162

V2:
Separate out Robert's patches, and add properly ordered certificate of
origins.

Jacob Shin (4):
  perf, amd: Use proper naming scheme for AMD bit field definitions
  perf, x86: Move MSR address offset calculation to architecture
    specific files
  perf, x86: Allow for architecture specific RDPMC indexes
  perf, amd: Enable northbridge performance counters on AMD family 15h

Robert Richter (2):
  perf, amd: Rework northbridge event constraints handler
  perf, amd: Generalize northbridge constraints code for family 15h

 arch/x86/include/asm/cpufeature.h     |    2 +
 arch/x86/include/asm/perf_event.h     |   13 +-
 arch/x86/include/uapi/asm/msr-index.h |    2 +
 arch/x86/kernel/cpu/perf_event.c      |    2 +-
 arch/x86/kernel/cpu/perf_event.h      |   25 ++-
 arch/x86/kernel/cpu/perf_event_amd.c  |  318 +++++++++++++++++++++++++--------
 6 files changed, 268 insertions(+), 94 deletions(-)

-- 
1.7.9.5

[PATCH RESEND V5 1/6] perf, amd: Rework northbridge event constraints handler

[Jacob Shin]

From: Robert Richter <rric@kernel.org>

Code simplification. No functional changes.

Signed-off-by: Robert Richter <rric@kernel.org>
Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 arch/x86/kernel/cpu/perf_event_amd.c |   68 +++++++++++++---------------------
 1 file changed, 26 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
index c93bc4e..e7963c7 100644
--- a/arch/x86/kernel/cpu/perf_event_amd.c
+++ b/arch/x86/kernel/cpu/perf_event_amd.c
@@ -256,9 +256,8 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
 	struct amd_nb *nb = cpuc->amd_nb;
-	struct perf_event *old = NULL;
-	int max = x86_pmu.num_counters;
-	int i, j, k = -1;
+	struct perf_event *old;
+	int idx, new = -1;
 
 	/*
 	 * if not NB event or no NB, then no constraints
@@ -276,48 +275,33 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
 	 * because of successive calls to x86_schedule_events() from
 	 * hw_perf_group_sched_in() without hw_perf_enable()
 	 */
-	for (i = 0; i < max; i++) {
-		/*
-		 * keep track of first free slot
-		 */
-		if (k == -1 && !nb->owners[i])
-			k = i;
+	for (idx = 0; idx < x86_pmu.num_counters; idx++) {
+		if (new == -1 || hwc->idx == idx)
+			/* assign free slot, prefer hwc->idx */
+			old = cmpxchg(nb->owners + idx, NULL, event);
+		else if (nb->owners[idx] == event)
+			/* event already present */
+			old = event;
+		else
+			continue;
+
+		if (old && old != event)
+			continue;
+
+		/* reassign to this slot */
+		if (new != -1)
+			cmpxchg(nb->owners + new, event, NULL);
+		new = idx;
 
 		/* already present, reuse */
-		if (nb->owners[i] == event)
-			goto done;
-	}
-	/*
-	 * not present, so grab a new slot
-	 * starting either at:
-	 */
-	if (hwc->idx != -1) {
-		/* previous assignment */
-		i = hwc->idx;
-	} else if (k != -1) {
-		/* start from free slot found */
-		i = k;
-	} else {
-		/*
-		 * event not found, no slot found in
-		 * first pass, try again from the
-		 * beginning
-		 */
-		i = 0;
-	}
-	j = i;
-	do {
-		old = cmpxchg(nb->owners+i, NULL, event);
-		if (!old)
+		if (old == event)
 			break;
-		if (++i == max)
-			i = 0;
-	} while (i != j);
-done:
-	if (!old)
-		return &nb->event_constraints[i];
-
-	return &emptyconstraint;
+	}
+
+	if (new == -1)
+		return &emptyconstraint;
+
+	return &nb->event_constraints[new];
 }
 
 static struct amd_nb *amd_alloc_nb(int cpu)
-- 
1.7.9.5

[PATCH RESEND V5 2/6] perf, amd: Generalize northbridge constraints code for family 15h

[Jacob Shin]

From: Robert Richter <rric@kernel.org>

Generalize northbridge constraints code for family 10h so that later
we can reuse the same code path with other AMD processor families that
have the same northbridge event constraints.

Signed-off-by: Robert Richter <rric@kernel.org>
Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 arch/x86/kernel/cpu/perf_event_amd.c |   43 ++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
index e7963c7..9541fe5 100644
--- a/arch/x86/kernel/cpu/perf_event_amd.c
+++ b/arch/x86/kernel/cpu/perf_event_amd.c
@@ -188,20 +188,13 @@ static inline int amd_has_nb(struct cpu_hw_events *cpuc)
 	return nb && nb->nb_id != -1;
 }
 
-static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
-				      struct perf_event *event)
+static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
+					   struct perf_event *event)
 {
-	struct hw_perf_event *hwc = &event->hw;
 	struct amd_nb *nb = cpuc->amd_nb;
 	int i;
 
 	/*
-	 * only care about NB events
-	 */
-	if (!(amd_has_nb(cpuc) && amd_is_nb_event(hwc)))
-		return;
-
-	/*
 	 * need to scan whole list because event may not have
 	 * been assigned during scheduling
 	 *
@@ -247,12 +240,13 @@ static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
   *
   * Given that resources are allocated (cmpxchg), they must be
   * eventually freed for others to use. This is accomplished by
-  * calling amd_put_event_constraints().
+  * calling __amd_put_nb_event_constraints()
   *
   * Non NB events are not impacted by this restriction.
   */
 static struct event_constraint *
-amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
+__amd_get_nb_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event,
+			       struct event_constraint *c)
 {
 	struct hw_perf_event *hwc = &event->hw;
 	struct amd_nb *nb = cpuc->amd_nb;
@@ -260,12 +254,6 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
 	int idx, new = -1;
 
 	/*
-	 * if not NB event or no NB, then no constraints
-	 */
-	if (!(amd_has_nb(cpuc) && amd_is_nb_event(hwc)))
-		return &unconstrained;
-
-	/*
 	 * detect if already present, if so reuse
 	 *
 	 * cannot merge with actual allocation
@@ -275,7 +263,7 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
 	 * because of successive calls to x86_schedule_events() from
 	 * hw_perf_group_sched_in() without hw_perf_enable()
 	 */
-	for (idx = 0; idx < x86_pmu.num_counters; idx++) {
+	for_each_set_bit(idx, c->idxmsk, X86_PMC_IDX_MAX) {
 		if (new == -1 || hwc->idx == idx)
 			/* assign free slot, prefer hwc->idx */
 			old = cmpxchg(nb->owners + idx, NULL, event);
@@ -391,6 +379,25 @@ static void amd_pmu_cpu_dead(int cpu)
 	}
 }
 
+static struct event_constraint *
+amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
+{
+	/*
+	 * if not NB event or no NB, then no constraints
+	 */
+	if (!(amd_has_nb(cpuc) && amd_is_nb_event(&event->hw)))
+		return &unconstrained;
+
+	return __amd_get_nb_event_constraints(cpuc, event, &unconstrained);
+}
+
+static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
+				      struct perf_event *event)
+{
+	if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
+		__amd_put_nb_event_constraints(cpuc, event);
+}
+
 PMU_FORMAT_ATTR(event,	"config:0-7,32-35");
 PMU_FORMAT_ATTR(umask,	"config:8-15"	);
 PMU_FORMAT_ATTR(edge,	"config:18"	);
-- 
1.7.9.5

[PATCH RESEND V5 3/6] perf, amd: Use proper naming scheme for AMD bit field definitions

[Jacob Shin]

Update these AMD bit field names to be consistent with naming
convention followed by the rest of the file.

Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 arch/x86/include/asm/perf_event.h    |    4 ++--
 arch/x86/kernel/cpu/perf_event_amd.c |    8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 4fabcdf..2234eaaec 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -29,8 +29,8 @@
 #define ARCH_PERFMON_EVENTSEL_INV			(1ULL << 23)
 #define ARCH_PERFMON_EVENTSEL_CMASK			0xFF000000ULL
 
-#define AMD_PERFMON_EVENTSEL_GUESTONLY			(1ULL << 40)
-#define AMD_PERFMON_EVENTSEL_HOSTONLY			(1ULL << 41)
+#define AMD64_EVENTSEL_GUESTONLY			(1ULL << 40)
+#define AMD64_EVENTSEL_HOSTONLY				(1ULL << 41)
 
 #define AMD64_EVENTSEL_EVENT	\
 	(ARCH_PERFMON_EVENTSEL_EVENT | (0x0FULL << 32))
diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
index 9541fe5..0c2cc51 100644
--- a/arch/x86/kernel/cpu/perf_event_amd.c
+++ b/arch/x86/kernel/cpu/perf_event_amd.c
@@ -156,9 +156,9 @@ static int amd_pmu_hw_config(struct perf_event *event)
 		event->hw.config &= ~(ARCH_PERFMON_EVENTSEL_USR |
 				      ARCH_PERFMON_EVENTSEL_OS);
 	else if (event->attr.exclude_host)
-		event->hw.config |= AMD_PERFMON_EVENTSEL_GUESTONLY;
+		event->hw.config |= AMD64_EVENTSEL_GUESTONLY;
 	else if (event->attr.exclude_guest)
-		event->hw.config |= AMD_PERFMON_EVENTSEL_HOSTONLY;
+		event->hw.config |= AMD64_EVENTSEL_HOSTONLY;
 
 	if (event->attr.type != PERF_TYPE_RAW)
 		return 0;
@@ -336,7 +336,7 @@ static void amd_pmu_cpu_starting(int cpu)
 	struct amd_nb *nb;
 	int i, nb_id;
 
-	cpuc->perf_ctr_virt_mask = AMD_PERFMON_EVENTSEL_HOSTONLY;
+	cpuc->perf_ctr_virt_mask = AMD64_EVENTSEL_HOSTONLY;
 
 	if (boot_cpu_data.x86_max_cores < 2)
 		return;
@@ -669,7 +669,7 @@ void amd_pmu_disable_virt(void)
 	 * SVM is disabled the Guest-only bits still gets set and the counter
 	 * will not count anything.
 	 */
-	cpuc->perf_ctr_virt_mask = AMD_PERFMON_EVENTSEL_HOSTONLY;
+	cpuc->perf_ctr_virt_mask = AMD64_EVENTSEL_HOSTONLY;
 
 	/* Reload all events */
 	x86_pmu_disable_all();
-- 
1.7.9.5

[PATCH RESEND V5 4/6] perf, x86: Move MSR address offset calculation to architecture specific files

[Jacob Shin]

Move counter index to MSR address offset calculation to architecture
specific files. This prepares the way for perf_event_amd to enable
counter addresses that are not contiguous -- for example AMD Family
15h processors have 6 core performance counters starting at 0xc0010200
and 4 northbridge performance counters starting at 0xc0010240.

Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 arch/x86/kernel/cpu/perf_event.h     |   21 ++++-------------
 arch/x86/kernel/cpu/perf_event_amd.c |   42 ++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index 115c1ea..4440218 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -325,6 +325,7 @@ struct x86_pmu {
 	int		(*schedule_events)(struct cpu_hw_events *cpuc, int n, int *assign);
 	unsigned	eventsel;
 	unsigned	perfctr;
+	int		(*addr_offset)(int index, int eventsel);
 	u64		(*event_map)(int);
 	int		max_events;
 	int		num_counters;
@@ -446,28 +447,16 @@ extern u64 __read_mostly hw_cache_extra_regs
 
 u64 x86_perf_event_update(struct perf_event *event);
 
-static inline int x86_pmu_addr_offset(int index)
-{
-	int offset;
-
-	/* offset = X86_FEATURE_PERFCTR_CORE ? index << 1 : index */
-	alternative_io(ASM_NOP2,
-		       "shll $1, %%eax",
-		       X86_FEATURE_PERFCTR_CORE,
-		       "=a" (offset),
-		       "a"  (index));
-
-	return offset;
-}
-
 static inline unsigned int x86_pmu_config_addr(int index)
 {
-	return x86_pmu.eventsel + x86_pmu_addr_offset(index);
+	return x86_pmu.eventsel +
+		(x86_pmu.addr_offset ? x86_pmu.addr_offset(index, 1) : index);
 }
 
 static inline unsigned int x86_pmu_event_addr(int index)
 {
-	return x86_pmu.perfctr + x86_pmu_addr_offset(index);
+	return x86_pmu.perfctr +
+		(x86_pmu.addr_offset ? x86_pmu.addr_offset(index, 0) : index);
 }
 
 int x86_setup_perfctr(struct perf_event *event);
diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
index 0c2cc51..ef1df38 100644
--- a/arch/x86/kernel/cpu/perf_event_amd.c
+++ b/arch/x86/kernel/cpu/perf_event_amd.c
@@ -132,6 +132,47 @@ static u64 amd_pmu_event_map(int hw_event)
 	return amd_perfmon_event_map[hw_event];
 }
 
+/*
+ * Previously calculated offsets
+ */
+static unsigned int event_offsets[X86_PMC_IDX_MAX] __read_mostly;
+static unsigned int count_offsets[X86_PMC_IDX_MAX] __read_mostly;
+
+/*
+ * Legacy CPUs:
+ *   4 counters starting at 0xc0010000 each offset by 1
+ *
+ * CPUs with core performance counter extensions:
+ *   6 counters starting at 0xc0010200 each offset by 2
+ */
+static inline int amd_pmu_addr_offset(int index, int eventsel)
+{
+	int offset;
+
+	if (!index)
+		return index;
+
+	if (eventsel)
+		offset = event_offsets[index];
+	else
+		offset = count_offsets[index];
+
+	if (offset)
+		return offset;
+
+	if (!cpu_has_perfctr_core)
+		offset = index;
+	else
+		offset = index << 1;
+
+	if (eventsel)
+		event_offsets[index] = offset;
+	else
+		count_offsets[index] = offset;
+
+	return offset;
+}
+
 static int amd_pmu_hw_config(struct perf_event *event)
 {
 	int ret;
@@ -578,6 +619,7 @@ static __initconst const struct x86_pmu amd_pmu = {
 	.schedule_events	= x86_schedule_events,
 	.eventsel		= MSR_K7_EVNTSEL0,
 	.perfctr		= MSR_K7_PERFCTR0,
+	.addr_offset            = amd_pmu_addr_offset,
 	.event_map		= amd_pmu_event_map,
 	.max_events		= ARRAY_SIZE(amd_perfmon_event_map),
 	.num_counters		= AMD64_NUM_COUNTERS,
-- 
1.7.9.5

[PATCH RESEND V5 5/6] perf, x86: Allow for architecture specific RDPMC indexes

[Jacob Shin]

Similar to config_base and event_base, allow architecture specific
RDPMC ECX values.

Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 arch/x86/kernel/cpu/perf_event.c     |    2 +-
 arch/x86/kernel/cpu/perf_event.h     |    6 ++++++
 arch/x86/kernel/cpu/perf_event_amd.c |    6 ++++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 4428fd1..b63982b 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -835,7 +835,7 @@ static inline void x86_assign_hw_event(struct perf_event *event,
 	} else {
 		hwc->config_base = x86_pmu_config_addr(hwc->idx);
 		hwc->event_base  = x86_pmu_event_addr(hwc->idx);
-		hwc->event_base_rdpmc = hwc->idx;
+		hwc->event_base_rdpmc = x86_pmu_rdpmc_index(hwc->idx);
 	}
 }
 
diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index 4440218..c910657 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -326,6 +326,7 @@ struct x86_pmu {
 	unsigned	eventsel;
 	unsigned	perfctr;
 	int		(*addr_offset)(int index, int eventsel);
+	int		(*rdpmc_index)(int index);
 	u64		(*event_map)(int);
 	int		max_events;
 	int		num_counters;
@@ -459,6 +460,11 @@ static inline unsigned int x86_pmu_event_addr(int index)
 		(x86_pmu.addr_offset ? x86_pmu.addr_offset(index, 0) : index);
 }
 
+static inline int x86_pmu_rdpmc_index(int index)
+{
+	return x86_pmu.rdpmc_index ? x86_pmu.rdpmc_index(index) : index;
+}
+
 int x86_setup_perfctr(struct perf_event *event);
 
 int x86_pmu_hw_config(struct perf_event *event);
diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
index ef1df38..faf9072 100644
--- a/arch/x86/kernel/cpu/perf_event_amd.c
+++ b/arch/x86/kernel/cpu/perf_event_amd.c
@@ -173,6 +173,11 @@ static inline int amd_pmu_addr_offset(int index, int eventsel)
 	return offset;
 }
 
+static inline int amd_pmu_rdpmc_index(int index)
+{
+	return index;
+}
+
 static int amd_pmu_hw_config(struct perf_event *event)
 {
 	int ret;
@@ -620,6 +625,7 @@ static __initconst const struct x86_pmu amd_pmu = {
 	.eventsel		= MSR_K7_EVNTSEL0,
 	.perfctr		= MSR_K7_PERFCTR0,
 	.addr_offset            = amd_pmu_addr_offset,
+	.rdpmc_index		= amd_pmu_rdpmc_index,
 	.event_map		= amd_pmu_event_map,
 	.max_events		= ARRAY_SIZE(amd_perfmon_event_map),
 	.num_counters		= AMD64_NUM_COUNTERS,
-- 
1.7.9.5

[PATCH RESEND V5 6/6] perf, amd: Enable northbridge performance counters on AMD family 15h

[Jacob Shin]

On AMD family 15h processors, there are 4 new performance counters
(in addition to 6 core performance counters) that can be used for
counting northbridge events (i.e. DRAM accesses). Their bit fields are
almost identical to the core performance counters. However, unlike the
core performance counters, these MSRs are shared between multiple
cores (that share the same northbridge). We will reuse the same code
path as existing family 10h northbridge event constraints handler
logic to enforce this sharing.

Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 arch/x86/include/asm/cpufeature.h     |    2 +
 arch/x86/include/asm/perf_event.h     |    9 ++
 arch/x86/include/uapi/asm/msr-index.h |    2 +
 arch/x86/kernel/cpu/perf_event_amd.c  |  167 +++++++++++++++++++++++++++++----
 4 files changed, 160 insertions(+), 20 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 2d9075e..93fe929 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -167,6 +167,7 @@
 #define X86_FEATURE_TBM		(6*32+21) /* trailing bit manipulations */
 #define X86_FEATURE_TOPOEXT	(6*32+22) /* topology extensions CPUID leafs */
 #define X86_FEATURE_PERFCTR_CORE (6*32+23) /* core performance counter extensions */
+#define X86_FEATURE_PERFCTR_NB  (6*32+24) /* NB performance counter extensions */
 
 /*
  * Auxiliary flags: Linux defined - For features scattered in various
@@ -309,6 +310,7 @@ extern const char * const x86_power_flags[32];
 #define cpu_has_hypervisor	boot_cpu_has(X86_FEATURE_HYPERVISOR)
 #define cpu_has_pclmulqdq	boot_cpu_has(X86_FEATURE_PCLMULQDQ)
 #define cpu_has_perfctr_core	boot_cpu_has(X86_FEATURE_PERFCTR_CORE)
+#define cpu_has_perfctr_nb	boot_cpu_has(X86_FEATURE_PERFCTR_NB)
 #define cpu_has_cx8		boot_cpu_has(X86_FEATURE_CX8)
 #define cpu_has_cx16		boot_cpu_has(X86_FEATURE_CX16)
 #define cpu_has_eager_fpu	boot_cpu_has(X86_FEATURE_EAGER_FPU)
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 2234eaaec..57cb634 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -29,9 +29,14 @@
 #define ARCH_PERFMON_EVENTSEL_INV			(1ULL << 23)
 #define ARCH_PERFMON_EVENTSEL_CMASK			0xFF000000ULL
 
+#define AMD64_EVENTSEL_INT_CORE_ENABLE			(1ULL << 36)
 #define AMD64_EVENTSEL_GUESTONLY			(1ULL << 40)
 #define AMD64_EVENTSEL_HOSTONLY				(1ULL << 41)
 
+#define AMD64_EVENTSEL_INT_CORE_SEL_SHIFT		37
+#define AMD64_EVENTSEL_INT_CORE_SEL_MASK		\
+	(0xFULL << AMD64_EVENTSEL_INT_CORE_SEL_SHIFT)
+
 #define AMD64_EVENTSEL_EVENT	\
 	(ARCH_PERFMON_EVENTSEL_EVENT | (0x0FULL << 32))
 #define INTEL_ARCH_EVENT_MASK	\
@@ -46,8 +51,12 @@
 #define AMD64_RAW_EVENT_MASK		\
 	(X86_RAW_EVENT_MASK          |  \
 	 AMD64_EVENTSEL_EVENT)
+#define AMD64_RAW_EVENT_MASK_NB		\
+	(AMD64_EVENTSEL_EVENT        |  \
+	 ARCH_PERFMON_EVENTSEL_UMASK)
 #define AMD64_NUM_COUNTERS				4
 #define AMD64_NUM_COUNTERS_CORE				6
+#define AMD64_NUM_COUNTERS_NB				4
 
 #define ARCH_PERFMON_UNHALTED_CORE_CYCLES_SEL		0x3c
 #define ARCH_PERFMON_UNHALTED_CORE_CYCLES_UMASK		(0x00 << 8)
diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
index 433a59f..075a402 100644
--- a/arch/x86/include/uapi/asm/msr-index.h
+++ b/arch/x86/include/uapi/asm/msr-index.h
@@ -194,6 +194,8 @@
 /* Fam 15h MSRs */
 #define MSR_F15H_PERF_CTL		0xc0010200
 #define MSR_F15H_PERF_CTR		0xc0010201
+#define MSR_F15H_NB_PERF_CTL		0xc0010240
+#define MSR_F15H_NB_PERF_CTR		0xc0010241
 
 /* Fam 10h MSRs */
 #define MSR_FAM10H_MMIO_CONF_BASE	0xc0010058
diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
index faf9072..1a80e05 100644
--- a/arch/x86/kernel/cpu/perf_event_amd.c
+++ b/arch/x86/kernel/cpu/perf_event_amd.c
@@ -132,11 +132,14 @@ static u64 amd_pmu_event_map(int hw_event)
 	return amd_perfmon_event_map[hw_event];
 }
 
+static struct event_constraint *amd_nb_event_constraint;
+
 /*
  * Previously calculated offsets
  */
 static unsigned int event_offsets[X86_PMC_IDX_MAX] __read_mostly;
 static unsigned int count_offsets[X86_PMC_IDX_MAX] __read_mostly;
+static unsigned int rdpmc_indexes[X86_PMC_IDX_MAX] __read_mostly;
 
 /*
  * Legacy CPUs:
@@ -144,10 +147,14 @@ static unsigned int count_offsets[X86_PMC_IDX_MAX] __read_mostly;
  *
  * CPUs with core performance counter extensions:
  *   6 counters starting at 0xc0010200 each offset by 2
+ *
+ * CPUs with north bridge performance counter extensions:
+ *   4 additional counters starting at 0xc0010240 each offset by 2
+ *   (indexed right above either one of the above core counters)
  */
 static inline int amd_pmu_addr_offset(int index, int eventsel)
 {
-	int offset;
+	int offset, first, base;
 
 	if (!index)
 		return index;
@@ -160,7 +167,23 @@ static inline int amd_pmu_addr_offset(int index, int eventsel)
 	if (offset)
 		return offset;
 
-	if (!cpu_has_perfctr_core)
+	if (amd_nb_event_constraint &&
+	    test_bit(index, amd_nb_event_constraint->idxmsk)) {
+		/*
+		 * calculate the offset of NB counters with respect to
+		 * base eventsel or perfctr
+		 */
+
+		first = find_first_bit(amd_nb_event_constraint->idxmsk,
+				       X86_PMC_IDX_MAX);
+
+		if (eventsel)
+			base = MSR_F15H_NB_PERF_CTL - x86_pmu.eventsel;
+		else
+			base = MSR_F15H_NB_PERF_CTR - x86_pmu.perfctr;
+
+		offset = base + ((index - first) << 1);
+	} else if (!cpu_has_perfctr_core)
 		offset = index;
 	else
 		offset = index << 1;
@@ -175,24 +198,36 @@ static inline int amd_pmu_addr_offset(int index, int eventsel)
 
 static inline int amd_pmu_rdpmc_index(int index)
 {
-	return index;
-}
+	int ret, first;
 
-static int amd_pmu_hw_config(struct perf_event *event)
-{
-	int ret;
+	if (!index)
+		return index;
 
-	/* pass precise event sampling to ibs: */
-	if (event->attr.precise_ip && get_ibs_caps())
-		return -ENOENT;
+	ret = rdpmc_indexes[index];
 
-	ret = x86_pmu_hw_config(event);
 	if (ret)
 		return ret;
 
-	if (has_branch_stack(event))
-		return -EOPNOTSUPP;
+	if (amd_nb_event_constraint &&
+	    test_bit(index, amd_nb_event_constraint->idxmsk)) {
+		/*
+		 * according to the mnual, ECX value of the NB counters is
+		 * the index of the NB counter (0, 1, 2 or 3) plus 6
+		 */
+
+		first = find_first_bit(amd_nb_event_constraint->idxmsk,
+				       X86_PMC_IDX_MAX);
+		ret = index - first + 6;
+	} else
+		ret = index;
+
+	rdpmc_indexes[index] = ret;
 
+	return ret;
+}
+
+static int amd_core_hw_config(struct perf_event *event)
+{
 	if (event->attr.exclude_host && event->attr.exclude_guest)
 		/*
 		 * When HO == GO == 1 the hardware treats that as GO == HO == 0
@@ -206,10 +241,29 @@ static int amd_pmu_hw_config(struct perf_event *event)
 	else if (event->attr.exclude_guest)
 		event->hw.config |= AMD64_EVENTSEL_HOSTONLY;
 
-	if (event->attr.type != PERF_TYPE_RAW)
-		return 0;
+	return 0;
+}
 
-	event->hw.config |= event->attr.config & AMD64_RAW_EVENT_MASK;
+/*
+ * NB counters do not support the following event select bits:
+ *   Host/Guest only
+ *   Counter mask
+ *   Invert counter mask
+ *   Edge detect
+ *   OS/User mode
+ */
+static int amd_nb_hw_config(struct perf_event *event)
+{
+	if (event->attr.exclude_user || event->attr.exclude_kernel ||
+	    event->attr.exclude_host || event->attr.exclude_guest)
+		return -EINVAL;
+
+	event->hw.config &= ~(ARCH_PERFMON_EVENTSEL_USR |
+			      ARCH_PERFMON_EVENTSEL_OS);
+
+	if (event->hw.config & ~(AMD64_RAW_EVENT_MASK_NB |
+				 ARCH_PERFMON_EVENTSEL_INT))
+		return -EINVAL;
 
 	return 0;
 }
@@ -227,6 +281,11 @@ static inline int amd_is_nb_event(struct hw_perf_event *hwc)
 	return (hwc->config & 0xe0) == 0xe0;
 }
 
+static inline int amd_is_perfctr_nb_event(struct hw_perf_event *hwc)
+{
+	return amd_nb_event_constraint && amd_is_nb_event(hwc);
+}
+
 static inline int amd_has_nb(struct cpu_hw_events *cpuc)
 {
 	struct amd_nb *nb = cpuc->amd_nb;
@@ -234,6 +293,30 @@ static inline int amd_has_nb(struct cpu_hw_events *cpuc)
 	return nb && nb->nb_id != -1;
 }
 
+static int amd_pmu_hw_config(struct perf_event *event)
+{
+	int ret;
+
+	/* pass precise event sampling to ibs: */
+	if (event->attr.precise_ip && get_ibs_caps())
+		return -ENOENT;
+
+	if (has_branch_stack(event))
+		return -EOPNOTSUPP;
+
+	ret = x86_pmu_hw_config(event);
+	if (ret)
+		return ret;
+
+	if (event->attr.type == PERF_TYPE_RAW)
+		event->hw.config |= event->attr.config & AMD64_RAW_EVENT_MASK;
+
+	if (amd_is_perfctr_nb_event(&event->hw))
+		return amd_nb_hw_config(event);
+
+	return amd_core_hw_config(event);
+}
+
 static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
 					   struct perf_event *event)
 {
@@ -254,6 +337,19 @@ static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
 	}
 }
 
+static void amd_nb_interrupt_hw_config(struct hw_perf_event *hwc)
+{
+	int core_id = cpu_data(smp_processor_id()).cpu_core_id;
+
+	/* deliver interrupts only to this core */
+	if (hwc->config & ARCH_PERFMON_EVENTSEL_INT) {
+		hwc->config |= AMD64_EVENTSEL_INT_CORE_ENABLE;
+		hwc->config &= ~AMD64_EVENTSEL_INT_CORE_SEL_MASK;
+		hwc->config |= (u64)(core_id) <<
+			AMD64_EVENTSEL_INT_CORE_SEL_SHIFT;
+	}
+}
+
  /*
   * AMD64 NorthBridge events need special treatment because
   * counter access needs to be synchronized across all cores
@@ -299,6 +395,12 @@ __amd_get_nb_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *ev
 	struct perf_event *old;
 	int idx, new = -1;
 
+	if (!c)
+		c = &unconstrained;
+
+	if (cpuc->is_fake)
+		return c;
+
 	/*
 	 * detect if already present, if so reuse
 	 *
@@ -335,6 +437,9 @@ __amd_get_nb_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *ev
 	if (new == -1)
 		return &emptyconstraint;
 
+	if (amd_is_perfctr_nb_event(hwc))
+		amd_nb_interrupt_hw_config(hwc);
+
 	return &nb->event_constraints[new];
 }
 
@@ -434,7 +539,8 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
 	if (!(amd_has_nb(cpuc) && amd_is_nb_event(&event->hw)))
 		return &unconstrained;
 
-	return __amd_get_nb_event_constraints(cpuc, event, &unconstrained);
+	return __amd_get_nb_event_constraints(cpuc, event,
+					      amd_nb_event_constraint);
 }
 
 static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
@@ -533,6 +639,9 @@ static struct event_constraint amd_f15_PMC30 = EVENT_CONSTRAINT_OVERLAP(0, 0x09,
 static struct event_constraint amd_f15_PMC50 = EVENT_CONSTRAINT(0, 0x3F, 0);
 static struct event_constraint amd_f15_PMC53 = EVENT_CONSTRAINT(0, 0x38, 0);
 
+static struct event_constraint amd_NBPMC96 = EVENT_CONSTRAINT(0, 0x3C0, 0);
+static struct event_constraint amd_NBPMC74 = EVENT_CONSTRAINT(0, 0xF0, 0);
+
 static struct event_constraint *
 amd_get_event_constraints_f15h(struct cpu_hw_events *cpuc, struct perf_event *event)
 {
@@ -598,8 +707,8 @@ amd_get_event_constraints_f15h(struct cpu_hw_events *cpuc, struct perf_event *ev
 			return &amd_f15_PMC20;
 		}
 	case AMD_EVENT_NB:
-		/* not yet implemented */
-		return &emptyconstraint;
+		return __amd_get_nb_event_constraints(cpuc, event,
+						      amd_nb_event_constraint);
 	default:
 		return &emptyconstraint;
 	}
@@ -647,7 +756,7 @@ static __initconst const struct x86_pmu amd_pmu = {
 
 static int setup_event_constraints(void)
 {
-	if (boot_cpu_data.x86 >= 0x15)
+	if (boot_cpu_data.x86 == 0x15)
 		x86_pmu.get_event_constraints = amd_get_event_constraints_f15h;
 	return 0;
 }
@@ -677,6 +786,23 @@ static int setup_perfctr_core(void)
 	return 0;
 }
 
+static int setup_perfctr_nb(void)
+{
+	if (!cpu_has_perfctr_nb)
+		return -ENODEV;
+
+	x86_pmu.num_counters += AMD64_NUM_COUNTERS_NB;
+
+	if (cpu_has_perfctr_core)
+		amd_nb_event_constraint = &amd_NBPMC96;
+	else
+		amd_nb_event_constraint = &amd_NBPMC74;
+
+	printk(KERN_INFO "perf: AMD northbridge performance counters detected\n");
+
+	return 0;
+}
+
 __init int amd_pmu_init(void)
 {
 	/* Performance-monitoring supported from K7 and later: */
@@ -687,6 +813,7 @@ __init int amd_pmu_init(void)
 
 	setup_event_constraints();
 	setup_perfctr_core();
+	setup_perfctr_nb();
 
 	/* Events are common for all AMDs */
 	memcpy(hw_cache_event_ids, amd_hw_cache_event_ids,
-- 
1.7.9.5

Re: [PATCH RESEND V5 0/6] perf, amd: Enable AMD family 15h northbridge counters

[Stephane Eranian]

On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> The following patchset enables 4 additional performance counters in
> AMD family 15h processors that count northbridge events -- such as
> number of DRAM accesses.
>
In order for me to test this patch set more thoroughly it would help if you
could also provide me a patch to add the Fam15h uncore events to libpfm4.
In the past, Robert Richter took care of this. I hope you can fill his role for
this. So please, if you could send me the patch quickly, that would help
the review of your patch.

Thanks.



> This patchset is based on previous work done by Robert Richter
> <rric@kernel.org> :
>
> https://lkml.org/lkml/2012/6/19/324
>
> The main differences are:
>
> * The northbridge counters are indexed contiguously right above the
>   core performance counters.
>
> * MSR address offset calculations are moved to architecture specific
>   files.
>
> * Interrups are set up to be delivered only to a single core.
>
> V5:
> Rebased against latest tip
>
> V4:
> * Moved interrupt core select set up back to event constraints
>   function, sicne during ->hw_config time we do not yet know on which
>   CPU the the event will run on.
> * Tested on and made minor revisions to make sure that the patchset is
>   compatible with upcoming AMD Family 16h processors, and will support
>   core and NB counters without any further patches.
>
> V3:
> Addressed the following feedback/comments from Robert's review
> * https://lkml.org/lkml/2012/11/16/484
> * https://lkml.org/lkml/2012/11/26/162
>
> V2:
> Separate out Robert's patches, and add properly ordered certificate of
> origins.
>
> Jacob Shin (4):
>   perf, amd: Use proper naming scheme for AMD bit field definitions
>   perf, x86: Move MSR address offset calculation to architecture
>     specific files
>   perf, x86: Allow for architecture specific RDPMC indexes
>   perf, amd: Enable northbridge performance counters on AMD family 15h
>
> Robert Richter (2):
>   perf, amd: Rework northbridge event constraints handler
>   perf, amd: Generalize northbridge constraints code for family 15h
>
>  arch/x86/include/asm/cpufeature.h     |    2 +
>  arch/x86/include/asm/perf_event.h     |   13 +-
>  arch/x86/include/uapi/asm/msr-index.h |    2 +
>  arch/x86/kernel/cpu/perf_event.c      |    2 +-
>  arch/x86/kernel/cpu/perf_event.h      |   25 ++-
>  arch/x86/kernel/cpu/perf_event_amd.c  |  318 +++++++++++++++++++++++++--------
>  6 files changed, 268 insertions(+), 94 deletions(-)
>
> --
> 1.7.9.5
>
>

Re: [PATCH RESEND V5 0/6] perf, amd: Enable AMD family 15h northbridge counters

[Jacob Shin]

On Thu, Jan 24, 2013 at 02:31:59PM +0100, Stephane Eranian wrote:
> On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> > The following patchset enables 4 additional performance counters in
> > AMD family 15h processors that count northbridge events -- such as
> > number of DRAM accesses.
> >
> In order for me to test this patch set more thoroughly it would help if you
> could also provide me a patch to add the Fam15h uncore events to libpfm4.
> In the past, Robert Richter took care of this. I hope you can fill his role for
> this. So please, if you could send me the patch quickly, that would help
> the review of your patch.

Hi Stephane,

Here is the corresponding libpfm4 patch. Thank you for taking the time
to review the patchset. I hope this helps .. If we can get AMD related
perf kernel side patchsets to upstream, I will be more than happy to
support AMD related libpfm4 efforts going forward.

Thanks!

>From 47d3267dfa24b9071c76f4a22bd059b0e4032002 Mon Sep 17 00:00:00 2001
From: Jacob Shin <jacob.shin@amd.com>
Date: Thu, 24 Jan 2013 15:37:37 -0600
Subject: [PATCH 1/1] Add AMD Family 15h northbridge performance events

libpfm4 side support for the following Linux kernel patchset:
  http://lkml.org/lkml/2013/1/10/450

Reference -- BIOS and Kernel Developer Guide (BKDG) for AMD Family 15h
 Models 00h-0Fh Processors:
  http://support.amd.com/us/Processor_TechDocs/42301_15h_Mod_00h-0Fh_BKDG.pdf

Signed-off-by: Jacob Shin <jacob.shin@amd.com>
---
 lib/events/amd64_events_fam15h.h |  155 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 155 insertions(+)

diff --git a/lib/events/amd64_events_fam15h.h b/lib/events/amd64_events_fam15h.h
index 7f654e8..0276782 100644
--- a/lib/events/amd64_events_fam15h.h
+++ b/lib/events/amd64_events_fam15h.h
@@ -752,6 +752,126 @@ static const amd64_umask_t amd64_fam15h_l2_prefetcher_trigger_events[]={
    },
 };
 
+static const amd64_umask_t amd64_fam15h_dram_accesses[]={
+   { .uname = "DCT0_PAGE_HIT",
+     .udesc = "DCT0 Page hit",
+     .ucode = 0x1,
+   },
+   { .uname = "DCT0_PAGE_MISS",
+     .udesc = "DCT0 Page Miss",
+     .ucode = 0x2,
+   },
+   { .uname = "DCT0_PAGE_CONFLICT",
+     .udesc = "DCT0 Page Conflict",
+     .ucode = 0x4,
+   },
+   { .uname = "DCT1_PAGE_HIT",
+     .udesc = "DCT1 Page hit",
+     .ucode = 0x8,
+   },
+   { .uname = "DCT1_PAGE_MISS",
+     .udesc = "DCT1 Page Miss",
+     .ucode = 0x10,
+   },
+   { .uname = "DCT1_PAGE_CONFLICT",
+     .udesc = "DCT1 Page Conflict",
+     .ucode = 0x20,
+   },
+   { .uname  = "ALL",
+     .udesc  = "All sub-events selected",
+     .ucode  = 0x3f,
+     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
+   },
+};
+
+static const amd64_umask_t amd64_fam15h_dram_controller_page_table_overflows[]={
+   { .uname = "DCT0_PAGE_TABLE_OVERFLOW",
+     .udesc = "DCT0 Page Table Overflow",
+     .ucode = 0x1,
+   },
+   { .uname = "DCT1_PAGE_TABLE_OVERFLOW",
+     .udesc = "DCT1 Page Table Overflow",
+     .ucode = 0x2,
+   },
+   { .uname  = "ALL",
+     .udesc  = "All sub-events selected",
+     .ucode  = 0x3,
+     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
+   },
+};
+
+static const amd64_umask_t amd64_fam15h_memory_controller_dram_command_slots_missed[]={
+   { .uname = "DCT0_COMMAND_SLOTS_MISSED",
+     .udesc = "DCT0 Command Slots Missed (in MemClks)",
+     .ucode = 0x1,
+   },
+   { .uname = "DCT1_COMMAND_SLOTS_MISSED",
+     .udesc = "DCT1 Command Slots Missed (in MemClks)",
+     .ucode = 0x2,
+   },
+   { .uname  = "ALL",
+     .udesc  = "All sub-events selected",
+     .ucode  = 0x3,
+     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
+   },
+};
+
+static const amd64_umask_t amd64_fam15h_memory_controller_turnarounds[]={
+   { .uname = "DCT0_DIMM_TURNAROUND",
+     .udesc = "DCT0 DIMM (chip select) turnaround",
+     .ucode = 0x1,
+   },
+   { .uname = "DCT0_READ_TO_WRITE_TURNAROUND",
+     .udesc = "DCT0 Read to write turnaround",
+     .ucode = 0x2,
+   },
+   { .uname = "DCT0_WRITE_TO_READ_TURNAROUND",
+     .udesc = "DCT0 Write to read turnaround",
+     .ucode = 0x4,
+   },
+   { .uname = "DCT1_DIMM_TURNAROUND",
+     .udesc = "DCT1 DIMM (chip select) turnaround",
+     .ucode = 0x8,
+   },
+   { .uname = "DCT1_READ_TO_WRITE_TURNAROUND",
+     .udesc = "DCT1 Read to write turnaround",
+     .ucode = 0x10,
+   },
+   { .uname = "DCT1_WRITE_TO_READ_TURNAROUND",
+     .udesc = "DCT1 Write to read turnaround",
+     .ucode = 0x20,
+   },
+   { .uname  = "ALL",
+     .udesc  = "All sub-events selected",
+     .ucode  = 0x3f,
+     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
+   },
+};
+
+static const amd64_umask_t amd64_fam15h_memory_controller_bypass_counter_saturation[]={
+   { .uname = "MEMORY_CONTROLLER_HIGH_PRIORITY_BYPASS",
+     .udesc = "Memory controller high priority bypass",
+     .ucode = 0x1,
+   },
+   { .uname = "MEMORY_CONTROLLER_MEDIUM_PRIORITY_BYPASS",
+     .udesc = "Memory controller medium priority bypass",
+     .ucode = 0x2,
+   },
+   { .uname = "DCT0_DCQ_BYPASS",
+     .udesc = "DCT0 DCQ bypass",
+     .ucode = 0x4,
+   },
+   { .uname = "DCT1_DCQ_BYPASS",
+     .udesc = "DCT1 DCQ bypass",
+     .ucode = 0x8,
+   },
+   { .uname  = "ALL",
+     .udesc  = "All sub-events selected",
+     .ucode  = 0xf,
+     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
+   },
+};
+
 static const amd64_entry_t amd64_fam15h_pe[]={
 { .name    = "DISPATCHED_FPU_OPS",
   .desc    = "FPU Pipe Assignment",
@@ -1256,4 +1376,39 @@ static const amd64_entry_t amd64_fam15h_pe[]={
   .modmsk  = AMD64_FAM15H_ATTRS,
   .code    = 0x1d8,
 },
+{ .name    = "DRAM_ACCESSES",
+  .desc    = "DRAM Accesses",
+  .code    = 0xe0,
+  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_dram_accesses),
+  .ngrp    = 1,
+  .umasks  = amd64_fam15h_dram_accesses,
+},
+{ .name    = "DRAM_CONTROLLER_PAGE_TABLE_OVERFLOWS",
+  .desc    = "DRAM Controller Page Table Overflows",
+  .code    = 0xe1,
+  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_dram_controller_page_table_overflows),
+  .ngrp    = 1,
+  .umasks  = amd64_fam15h_dram_controller_page_table_overflows,
+},
+{ .name    = "MEMORY_CONTROLLER_DRAM_COMMAND_SLOTS_MISSED",
+  .desc    = "Memory Controller DRAM Command Slots Missed",
+  .code    = 0xe2,
+  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_dram_command_slots_missed),
+  .ngrp    = 1,
+  .umasks  = amd64_fam15h_memory_controller_dram_command_slots_missed,
+},
+{ .name    = "MEMORY_CONTROLLER_TURNAROUNDS",
+  .desc    = "Memory Controller Turnarounds",
+  .code    = 0xe3,
+  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_turnarounds),
+  .ngrp    = 1,
+  .umasks  = amd64_fam15h_memory_controller_turnarounds,
+},
+{ .name    = "MEMORY_CONTROLLER_BYPASS_COUNTER_SATURATION",
+  .desc    = "Memory Controller Bypass Counter Saturation",
+  .code    = 0xe4,
+  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_bypass_counter_saturation),
+  .ngrp    = 1,
+  .umasks  = amd64_fam15h_memory_controller_bypass_counter_saturation,
+},
 };
-- 
1.7.9.5

Re: [perfmon2] [PATCH RESEND V5 0/6] perf, amd: Enable AMD family 15h northbridge counters

[Stephane Eranian]

Hi Jacob,

I will apply this patch to libpfm4.
But I have a question. Why aren't the other uncore
events included here as well? I am talking about
the events listed in BKDG sections 3.16.2 to 3.16.6?
Are  those NOT supported by your kernel patchset?


On Thu, Jan 24, 2013 at 11:06 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> On Thu, Jan 24, 2013 at 02:31:59PM +0100, Stephane Eranian wrote:
>> On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
>> > The following patchset enables 4 additional performance counters in
>> > AMD family 15h processors that count northbridge events -- such as
>> > number of DRAM accesses.
>> >
>> In order for me to test this patch set more thoroughly it would help if you
>> could also provide me a patch to add the Fam15h uncore events to libpfm4.
>> In the past, Robert Richter took care of this. I hope you can fill his role for
>> this. So please, if you could send me the patch quickly, that would help
>> the review of your patch.
>
> Hi Stephane,
>
> Here is the corresponding libpfm4 patch. Thank you for taking the time
> to review the patchset. I hope this helps .. If we can get AMD related
> perf kernel side patchsets to upstream, I will be more than happy to
> support AMD related libpfm4 efforts going forward.
>
> Thanks!
>
> >From 47d3267dfa24b9071c76f4a22bd059b0e4032002 Mon Sep 17 00:00:00 2001
> From: Jacob Shin <jacob.shin@amd.com>
> Date: Thu, 24 Jan 2013 15:37:37 -0600
> Subject: [PATCH 1/1] Add AMD Family 15h northbridge performance events
>
> libpfm4 side support for the following Linux kernel patchset:
>   http://lkml.org/lkml/2013/1/10/450
>
> Reference -- BIOS and Kernel Developer Guide (BKDG) for AMD Family 15h
>  Models 00h-0Fh Processors:
>   http://support.amd.com/us/Processor_TechDocs/42301_15h_Mod_00h-0Fh_BKDG.pdf
>
> Signed-off-by: Jacob Shin <jacob.shin@amd.com>
> ---
>  lib/events/amd64_events_fam15h.h |  155 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 155 insertions(+)
>
> diff --git a/lib/events/amd64_events_fam15h.h b/lib/events/amd64_events_fam15h.h
> index 7f654e8..0276782 100644
> --- a/lib/events/amd64_events_fam15h.h
> +++ b/lib/events/amd64_events_fam15h.h
> @@ -752,6 +752,126 @@ static const amd64_umask_t amd64_fam15h_l2_prefetcher_trigger_events[]={
>     },
>  };
>
> +static const amd64_umask_t amd64_fam15h_dram_accesses[]={
> +   { .uname = "DCT0_PAGE_HIT",
> +     .udesc = "DCT0 Page hit",
> +     .ucode = 0x1,
> +   },
> +   { .uname = "DCT0_PAGE_MISS",
> +     .udesc = "DCT0 Page Miss",
> +     .ucode = 0x2,
> +   },
> +   { .uname = "DCT0_PAGE_CONFLICT",
> +     .udesc = "DCT0 Page Conflict",
> +     .ucode = 0x4,
> +   },
> +   { .uname = "DCT1_PAGE_HIT",
> +     .udesc = "DCT1 Page hit",
> +     .ucode = 0x8,
> +   },
> +   { .uname = "DCT1_PAGE_MISS",
> +     .udesc = "DCT1 Page Miss",
> +     .ucode = 0x10,
> +   },
> +   { .uname = "DCT1_PAGE_CONFLICT",
> +     .udesc = "DCT1 Page Conflict",
> +     .ucode = 0x20,
> +   },
> +   { .uname  = "ALL",
> +     .udesc  = "All sub-events selected",
> +     .ucode  = 0x3f,
> +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> +   },
> +};
> +
> +static const amd64_umask_t amd64_fam15h_dram_controller_page_table_overflows[]={
> +   { .uname = "DCT0_PAGE_TABLE_OVERFLOW",
> +     .udesc = "DCT0 Page Table Overflow",
> +     .ucode = 0x1,
> +   },
> +   { .uname = "DCT1_PAGE_TABLE_OVERFLOW",
> +     .udesc = "DCT1 Page Table Overflow",
> +     .ucode = 0x2,
> +   },
> +   { .uname  = "ALL",
> +     .udesc  = "All sub-events selected",
> +     .ucode  = 0x3,
> +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> +   },
> +};
> +
> +static const amd64_umask_t amd64_fam15h_memory_controller_dram_command_slots_missed[]={
> +   { .uname = "DCT0_COMMAND_SLOTS_MISSED",
> +     .udesc = "DCT0 Command Slots Missed (in MemClks)",
> +     .ucode = 0x1,
> +   },
> +   { .uname = "DCT1_COMMAND_SLOTS_MISSED",
> +     .udesc = "DCT1 Command Slots Missed (in MemClks)",
> +     .ucode = 0x2,
> +   },
> +   { .uname  = "ALL",
> +     .udesc  = "All sub-events selected",
> +     .ucode  = 0x3,
> +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> +   },
> +};
> +
> +static const amd64_umask_t amd64_fam15h_memory_controller_turnarounds[]={
> +   { .uname = "DCT0_DIMM_TURNAROUND",
> +     .udesc = "DCT0 DIMM (chip select) turnaround",
> +     .ucode = 0x1,
> +   },
> +   { .uname = "DCT0_READ_TO_WRITE_TURNAROUND",
> +     .udesc = "DCT0 Read to write turnaround",
> +     .ucode = 0x2,
> +   },
> +   { .uname = "DCT0_WRITE_TO_READ_TURNAROUND",
> +     .udesc = "DCT0 Write to read turnaround",
> +     .ucode = 0x4,
> +   },
> +   { .uname = "DCT1_DIMM_TURNAROUND",
> +     .udesc = "DCT1 DIMM (chip select) turnaround",
> +     .ucode = 0x8,
> +   },
> +   { .uname = "DCT1_READ_TO_WRITE_TURNAROUND",
> +     .udesc = "DCT1 Read to write turnaround",
> +     .ucode = 0x10,
> +   },
> +   { .uname = "DCT1_WRITE_TO_READ_TURNAROUND",
> +     .udesc = "DCT1 Write to read turnaround",
> +     .ucode = 0x20,
> +   },
> +   { .uname  = "ALL",
> +     .udesc  = "All sub-events selected",
> +     .ucode  = 0x3f,
> +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> +   },
> +};
> +
> +static const amd64_umask_t amd64_fam15h_memory_controller_bypass_counter_saturation[]={
> +   { .uname = "MEMORY_CONTROLLER_HIGH_PRIORITY_BYPASS",
> +     .udesc = "Memory controller high priority bypass",
> +     .ucode = 0x1,
> +   },
> +   { .uname = "MEMORY_CONTROLLER_MEDIUM_PRIORITY_BYPASS",
> +     .udesc = "Memory controller medium priority bypass",
> +     .ucode = 0x2,
> +   },
> +   { .uname = "DCT0_DCQ_BYPASS",
> +     .udesc = "DCT0 DCQ bypass",
> +     .ucode = 0x4,
> +   },
> +   { .uname = "DCT1_DCQ_BYPASS",
> +     .udesc = "DCT1 DCQ bypass",
> +     .ucode = 0x8,
> +   },
> +   { .uname  = "ALL",
> +     .udesc  = "All sub-events selected",
> +     .ucode  = 0xf,
> +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> +   },
> +};
> +
>  static const amd64_entry_t amd64_fam15h_pe[]={
>  { .name    = "DISPATCHED_FPU_OPS",
>    .desc    = "FPU Pipe Assignment",
> @@ -1256,4 +1376,39 @@ static const amd64_entry_t amd64_fam15h_pe[]={
>    .modmsk  = AMD64_FAM15H_ATTRS,
>    .code    = 0x1d8,
>  },
> +{ .name    = "DRAM_ACCESSES",
> +  .desc    = "DRAM Accesses",
> +  .code    = 0xe0,
> +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_dram_accesses),
> +  .ngrp    = 1,
> +  .umasks  = amd64_fam15h_dram_accesses,
> +},
> +{ .name    = "DRAM_CONTROLLER_PAGE_TABLE_OVERFLOWS",
> +  .desc    = "DRAM Controller Page Table Overflows",
> +  .code    = 0xe1,
> +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_dram_controller_page_table_overflows),
> +  .ngrp    = 1,
> +  .umasks  = amd64_fam15h_dram_controller_page_table_overflows,
> +},
> +{ .name    = "MEMORY_CONTROLLER_DRAM_COMMAND_SLOTS_MISSED",
> +  .desc    = "Memory Controller DRAM Command Slots Missed",
> +  .code    = 0xe2,
> +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_dram_command_slots_missed),
> +  .ngrp    = 1,
> +  .umasks  = amd64_fam15h_memory_controller_dram_command_slots_missed,
> +},
> +{ .name    = "MEMORY_CONTROLLER_TURNAROUNDS",
> +  .desc    = "Memory Controller Turnarounds",
> +  .code    = 0xe3,
> +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_turnarounds),
> +  .ngrp    = 1,
> +  .umasks  = amd64_fam15h_memory_controller_turnarounds,
> +},
> +{ .name    = "MEMORY_CONTROLLER_BYPASS_COUNTER_SATURATION",
> +  .desc    = "Memory Controller Bypass Counter Saturation",
> +  .code    = 0xe4,
> +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_bypass_counter_saturation),
> +  .ngrp    = 1,
> +  .umasks  = amd64_fam15h_memory_controller_bypass_counter_saturation,
> +},
>  };
> --
> 1.7.9.5
>
>
>
> ------------------------------------------------------------------------------
> Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
> MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
> with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
> MVPs and experts. ON SALE this month only -- learn more at:
> http://p.sf.net/sfu/learnnow-d2d
> _______________________________________________
> perfmon2-devel mailing list
> perfmon2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/perfmon2-devel

Re: [PATCH RESEND V5 1/6] perf, amd: Rework northbridge event constraints handler

[Stephane Eranian]

On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> From: Robert Richter <rric@kernel.org>
>
> Code simplification. No functional changes.
>
> Signed-off-by: Robert Richter <rric@kernel.org>
> Signed-off-by: Jacob Shin <jacob.shin@amd.com>

Acked-by: Stephane Eranian <eranian@google.com>

> ---
>  arch/x86/kernel/cpu/perf_event_amd.c |   68 +++++++++++++---------------------
>  1 file changed, 26 insertions(+), 42 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
> index c93bc4e..e7963c7 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd.c
> @@ -256,9 +256,8 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
>  {
>         struct hw_perf_event *hwc = &event->hw;
>         struct amd_nb *nb = cpuc->amd_nb;
> -       struct perf_event *old = NULL;
> -       int max = x86_pmu.num_counters;
> -       int i, j, k = -1;
> +       struct perf_event *old;
> +       int idx, new = -1;
>
>         /*
>          * if not NB event or no NB, then no constraints
> @@ -276,48 +275,33 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
>          * because of successive calls to x86_schedule_events() from
>          * hw_perf_group_sched_in() without hw_perf_enable()
>          */
> -       for (i = 0; i < max; i++) {
> -               /*
> -                * keep track of first free slot
> -                */
> -               if (k == -1 && !nb->owners[i])
> -                       k = i;
> +       for (idx = 0; idx < x86_pmu.num_counters; idx++) {
> +               if (new == -1 || hwc->idx == idx)
> +                       /* assign free slot, prefer hwc->idx */
> +                       old = cmpxchg(nb->owners + idx, NULL, event);
> +               else if (nb->owners[idx] == event)
> +                       /* event already present */
> +                       old = event;
> +               else
> +                       continue;
> +
> +               if (old && old != event)
> +                       continue;
> +
> +               /* reassign to this slot */
> +               if (new != -1)
> +                       cmpxchg(nb->owners + new, event, NULL);
> +               new = idx;
>
>                 /* already present, reuse */
> -               if (nb->owners[i] == event)
> -                       goto done;
> -       }
> -       /*
> -        * not present, so grab a new slot
> -        * starting either at:
> -        */
> -       if (hwc->idx != -1) {
> -               /* previous assignment */
> -               i = hwc->idx;
> -       } else if (k != -1) {
> -               /* start from free slot found */
> -               i = k;
> -       } else {
> -               /*
> -                * event not found, no slot found in
> -                * first pass, try again from the
> -                * beginning
> -                */
> -               i = 0;
> -       }
> -       j = i;
> -       do {
> -               old = cmpxchg(nb->owners+i, NULL, event);
> -               if (!old)
> +               if (old == event)
>                         break;
> -               if (++i == max)
> -                       i = 0;
> -       } while (i != j);
> -done:
> -       if (!old)
> -               return &nb->event_constraints[i];
> -
> -       return &emptyconstraint;
> +       }
> +
> +       if (new == -1)
> +               return &emptyconstraint;
> +
> +       return &nb->event_constraints[new];
>  }
>
>  static struct amd_nb *amd_alloc_nb(int cpu)
> --
> 1.7.9.5
>
>

Re: [PATCH RESEND V5 2/6] perf, amd: Generalize northbridge constraints code for family 15h

[Stephane Eranian]

On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> From: Robert Richter <rric@kernel.org>
>
> Generalize northbridge constraints code for family 10h so that later
> we can reuse the same code path with other AMD processor families that
> have the same northbridge event constraints.
>
> Signed-off-by: Robert Richter <rric@kernel.org>
> Signed-off-by: Jacob Shin <jacob.shin@amd.com>
> ---
>  arch/x86/kernel/cpu/perf_event_amd.c |   43 ++++++++++++++++++++--------------
>  1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
> index e7963c7..9541fe5 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd.c
> @@ -188,20 +188,13 @@ static inline int amd_has_nb(struct cpu_hw_events *cpuc)
>         return nb && nb->nb_id != -1;
>  }
>
> -static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
> -                                     struct perf_event *event)
> +static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
> +                                          struct perf_event *event)
>  {
> -       struct hw_perf_event *hwc = &event->hw;
>         struct amd_nb *nb = cpuc->amd_nb;
>         int i;
>
>         /*
> -        * only care about NB events
> -        */
> -       if (!(amd_has_nb(cpuc) && amd_is_nb_event(hwc)))
> -               return;
> -
> -       /*
>          * need to scan whole list because event may not have
>          * been assigned during scheduling
>          *
> @@ -247,12 +240,13 @@ static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
>    *
>    * Given that resources are allocated (cmpxchg), they must be
>    * eventually freed for others to use. This is accomplished by
> -  * calling amd_put_event_constraints().
> +  * calling __amd_put_nb_event_constraints()
>    *
>    * Non NB events are not impacted by this restriction.
>    */
>  static struct event_constraint *
> -amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
> +__amd_get_nb_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event,
> +                              struct event_constraint *c)
>  {
>         struct hw_perf_event *hwc = &event->hw;
>         struct amd_nb *nb = cpuc->amd_nb;
> @@ -260,12 +254,6 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
>         int idx, new = -1;
>
>         /*
> -        * if not NB event or no NB, then no constraints
> -        */
> -       if (!(amd_has_nb(cpuc) && amd_is_nb_event(hwc)))
> -               return &unconstrained;
> -
> -       /*
>          * detect if already present, if so reuse
>          *
>          * cannot merge with actual allocation
> @@ -275,7 +263,7 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
>          * because of successive calls to x86_schedule_events() from
>          * hw_perf_group_sched_in() without hw_perf_enable()
>          */
> -       for (idx = 0; idx < x86_pmu.num_counters; idx++) {
> +       for_each_set_bit(idx, c->idxmsk, X86_PMC_IDX_MAX) {

So here you're using   X86_PMC_IDX_MAX but in
__amd_put_nb_event_constraints() you're using
x86_pmu.num_counters.

There is implicit assumption in the AMD code the counters index
namespace is contiguous. That
means the uncore counters show up right after the core counters. On
Fam15h, that would be NB
counters start at index 6, on Fam10h at index 4. In that case, the
constraint mask cannot have bits set
beyond num_counters, so why not use that limit in
amd_get_event_constraints()? It would significantly
cut down on the number of iterations in the loop from 64 down to 10 on Fam15h.


>                 if (new == -1 || hwc->idx == idx)
>                         /* assign free slot, prefer hwc->idx */
>                         old = cmpxchg(nb->owners + idx, NULL, event);
> @@ -391,6 +379,25 @@ static void amd_pmu_cpu_dead(int cpu)
>         }
>  }
>
> +static struct event_constraint *
> +amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
> +{
> +       /*
> +        * if not NB event or no NB, then no constraints
> +        */
> +       if (!(amd_has_nb(cpuc) && amd_is_nb_event(&event->hw)))
> +               return &unconstrained;
> +
> +       return __amd_get_nb_event_constraints(cpuc, event, &unconstrained);
> +}
> +
> +static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
> +                                     struct perf_event *event)
> +{
> +       if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
> +               __amd_put_nb_event_constraints(cpuc, event);
> +}
> +
>  PMU_FORMAT_ATTR(event, "config:0-7,32-35");
>  PMU_FORMAT_ATTR(umask, "config:8-15"   );
>  PMU_FORMAT_ATTR(edge,  "config:18"     );
> --
> 1.7.9.5
>
>

Re: [PATCH RESEND V5 3/6] perf, amd: Use proper naming scheme for AMD bit field definitions

[Stephane Eranian]

On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> Update these AMD bit field names to be consistent with naming
> convention followed by the rest of the file.
>
> Signed-off-by: Jacob Shin <jacob.shin@amd.com>

Acked-by: Stephane Eranian <eranian@google.com>

> ---
>  arch/x86/include/asm/perf_event.h    |    4 ++--
>  arch/x86/kernel/cpu/perf_event_amd.c |    8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
> index 4fabcdf..2234eaaec 100644
> --- a/arch/x86/include/asm/perf_event.h
> +++ b/arch/x86/include/asm/perf_event.h
> @@ -29,8 +29,8 @@
>  #define ARCH_PERFMON_EVENTSEL_INV                      (1ULL << 23)
>  #define ARCH_PERFMON_EVENTSEL_CMASK                    0xFF000000ULL
>
> -#define AMD_PERFMON_EVENTSEL_GUESTONLY                 (1ULL << 40)
> -#define AMD_PERFMON_EVENTSEL_HOSTONLY                  (1ULL << 41)
> +#define AMD64_EVENTSEL_GUESTONLY                       (1ULL << 40)
> +#define AMD64_EVENTSEL_HOSTONLY                                (1ULL << 41)
>
>  #define AMD64_EVENTSEL_EVENT   \
>         (ARCH_PERFMON_EVENTSEL_EVENT | (0x0FULL << 32))
> diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
> index 9541fe5..0c2cc51 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd.c
> @@ -156,9 +156,9 @@ static int amd_pmu_hw_config(struct perf_event *event)
>                 event->hw.config &= ~(ARCH_PERFMON_EVENTSEL_USR |
>                                       ARCH_PERFMON_EVENTSEL_OS);
>         else if (event->attr.exclude_host)
> -               event->hw.config |= AMD_PERFMON_EVENTSEL_GUESTONLY;
> +               event->hw.config |= AMD64_EVENTSEL_GUESTONLY;
>         else if (event->attr.exclude_guest)
> -               event->hw.config |= AMD_PERFMON_EVENTSEL_HOSTONLY;
> +               event->hw.config |= AMD64_EVENTSEL_HOSTONLY;
>
>         if (event->attr.type != PERF_TYPE_RAW)
>                 return 0;
> @@ -336,7 +336,7 @@ static void amd_pmu_cpu_starting(int cpu)
>         struct amd_nb *nb;
>         int i, nb_id;
>
> -       cpuc->perf_ctr_virt_mask = AMD_PERFMON_EVENTSEL_HOSTONLY;
> +       cpuc->perf_ctr_virt_mask = AMD64_EVENTSEL_HOSTONLY;
>
>         if (boot_cpu_data.x86_max_cores < 2)
>                 return;
> @@ -669,7 +669,7 @@ void amd_pmu_disable_virt(void)
>          * SVM is disabled the Guest-only bits still gets set and the counter
>          * will not count anything.
>          */
> -       cpuc->perf_ctr_virt_mask = AMD_PERFMON_EVENTSEL_HOSTONLY;
> +       cpuc->perf_ctr_virt_mask = AMD64_EVENTSEL_HOSTONLY;
>
>         /* Reload all events */
>         x86_pmu_disable_all();
> --
> 1.7.9.5
>
>

Re: [PATCH RESEND V5 4/6] perf, x86: Move MSR address offset calculation to architecture specific files

[Stephane Eranian]

On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> Move counter index to MSR address offset calculation to architecture
> specific files. This prepares the way for perf_event_amd to enable
> counter addresses that are not contiguous -- for example AMD Family
> 15h processors have 6 core performance counters starting at 0xc0010200
> and 4 northbridge performance counters starting at 0xc0010240.
>
> Signed-off-by: Jacob Shin <jacob.shin@amd.com>
> ---
>  arch/x86/kernel/cpu/perf_event.h     |   21 ++++-------------
>  arch/x86/kernel/cpu/perf_event_amd.c |   42 ++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
> index 115c1ea..4440218 100644
> --- a/arch/x86/kernel/cpu/perf_event.h
> +++ b/arch/x86/kernel/cpu/perf_event.h
> @@ -325,6 +325,7 @@ struct x86_pmu {
>         int             (*schedule_events)(struct cpu_hw_events *cpuc, int n, int *assign);
>         unsigned        eventsel;
>         unsigned        perfctr;
> +       int             (*addr_offset)(int index, int eventsel);
>         u64             (*event_map)(int);
>         int             max_events;
>         int             num_counters;
> @@ -446,28 +447,16 @@ extern u64 __read_mostly hw_cache_extra_regs
>
>  u64 x86_perf_event_update(struct perf_event *event);
>
> -static inline int x86_pmu_addr_offset(int index)
> -{
> -       int offset;
> -
> -       /* offset = X86_FEATURE_PERFCTR_CORE ? index << 1 : index */
> -       alternative_io(ASM_NOP2,
> -                      "shll $1, %%eax",
> -                      X86_FEATURE_PERFCTR_CORE,
> -                      "=a" (offset),
> -                      "a"  (index));
> -
> -       return offset;
> -}
> -
>  static inline unsigned int x86_pmu_config_addr(int index)
>  {
> -       return x86_pmu.eventsel + x86_pmu_addr_offset(index);
> +       return x86_pmu.eventsel +
> +               (x86_pmu.addr_offset ? x86_pmu.addr_offset(index, 1) : index);
>  }
>
>  static inline unsigned int x86_pmu_event_addr(int index)
>  {
> -       return x86_pmu.perfctr + x86_pmu_addr_offset(index);
> +       return x86_pmu.perfctr +
> +               (x86_pmu.addr_offset ? x86_pmu.addr_offset(index, 0) : index);
>  }
Would be better to use a constant name instead of 1 and 0 to name a event_sel
vs. a counter. It would help the reader understand what this is about
as that may
be useful for other processors as well.

>  int x86_setup_perfctr(struct perf_event *event);
> diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
> index 0c2cc51..ef1df38 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd.c
> @@ -132,6 +132,47 @@ static u64 amd_pmu_event_map(int hw_event)
>         return amd_perfmon_event_map[hw_event];
>  }
>
> +/*
> + * Previously calculated offsets
> + */
> +static unsigned int event_offsets[X86_PMC_IDX_MAX] __read_mostly;
> +static unsigned int count_offsets[X86_PMC_IDX_MAX] __read_mostly;
> +
> +/*
> + * Legacy CPUs:
> + *   4 counters starting at 0xc0010000 each offset by 1
> + *
> + * CPUs with core performance counter extensions:
> + *   6 counters starting at 0xc0010200 each offset by 2
> + */
> +static inline int amd_pmu_addr_offset(int index, int eventsel)
> +{
> +       int offset;
> +
> +       if (!index)
> +               return index;
> +
> +       if (eventsel)
> +               offset = event_offsets[index];
> +       else
> +               offset = count_offsets[index];
> +
> +       if (offset)
> +               return offset;
> +
> +       if (!cpu_has_perfctr_core)
> +               offset = index;
> +       else
> +               offset = index << 1;
> +
> +       if (eventsel)
> +               event_offsets[index] = offset;
> +       else
> +               count_offsets[index] = offset;
> +
> +       return offset;
> +}
> +
>  static int amd_pmu_hw_config(struct perf_event *event)
>  {
>         int ret;
> @@ -578,6 +619,7 @@ static __initconst const struct x86_pmu amd_pmu = {
>         .schedule_events        = x86_schedule_events,
>         .eventsel               = MSR_K7_EVNTSEL0,
>         .perfctr                = MSR_K7_PERFCTR0,
> +       .addr_offset            = amd_pmu_addr_offset,
>         .event_map              = amd_pmu_event_map,
>         .max_events             = ARRAY_SIZE(amd_perfmon_event_map),
>         .num_counters           = AMD64_NUM_COUNTERS,
> --
> 1.7.9.5
>
>

Re: [PATCH RESEND V5 5/6] perf, x86: Allow for architecture specific RDPMC indexes

[Stephane Eranian]

On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> Similar to config_base and event_base, allow architecture specific
> RDPMC ECX values.
>
> Signed-off-by: Jacob Shin <jacob.shin@amd.com>
Acked-by: Stephane Eranian <eranian@google.com>

> ---
>  arch/x86/kernel/cpu/perf_event.c     |    2 +-
>  arch/x86/kernel/cpu/perf_event.h     |    6 ++++++
>  arch/x86/kernel/cpu/perf_event_amd.c |    6 ++++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index 4428fd1..b63982b 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -835,7 +835,7 @@ static inline void x86_assign_hw_event(struct perf_event *event,
>         } else {
>                 hwc->config_base = x86_pmu_config_addr(hwc->idx);
>                 hwc->event_base  = x86_pmu_event_addr(hwc->idx);
> -               hwc->event_base_rdpmc = hwc->idx;
> +               hwc->event_base_rdpmc = x86_pmu_rdpmc_index(hwc->idx);
>         }
>  }
>
> diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
> index 4440218..c910657 100644
> --- a/arch/x86/kernel/cpu/perf_event.h
> +++ b/arch/x86/kernel/cpu/perf_event.h
> @@ -326,6 +326,7 @@ struct x86_pmu {
>         unsigned        eventsel;
>         unsigned        perfctr;
>         int             (*addr_offset)(int index, int eventsel);
> +       int             (*rdpmc_index)(int index);
>         u64             (*event_map)(int);
>         int             max_events;
>         int             num_counters;
> @@ -459,6 +460,11 @@ static inline unsigned int x86_pmu_event_addr(int index)
>                 (x86_pmu.addr_offset ? x86_pmu.addr_offset(index, 0) : index);
>  }
>
> +static inline int x86_pmu_rdpmc_index(int index)
> +{
> +       return x86_pmu.rdpmc_index ? x86_pmu.rdpmc_index(index) : index;
> +}
> +
>  int x86_setup_perfctr(struct perf_event *event);
>
>  int x86_pmu_hw_config(struct perf_event *event);
> diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
> index ef1df38..faf9072 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd.c
> @@ -173,6 +173,11 @@ static inline int amd_pmu_addr_offset(int index, int eventsel)
>         return offset;
>  }
>
> +static inline int amd_pmu_rdpmc_index(int index)
> +{
> +       return index;
> +}
> +
>  static int amd_pmu_hw_config(struct perf_event *event)
>  {
>         int ret;
> @@ -620,6 +625,7 @@ static __initconst const struct x86_pmu amd_pmu = {
>         .eventsel               = MSR_K7_EVNTSEL0,
>         .perfctr                = MSR_K7_PERFCTR0,
>         .addr_offset            = amd_pmu_addr_offset,
> +       .rdpmc_index            = amd_pmu_rdpmc_index,
>         .event_map              = amd_pmu_event_map,
>         .max_events             = ARRAY_SIZE(amd_perfmon_event_map),
>         .num_counters           = AMD64_NUM_COUNTERS,
> --
> 1.7.9.5
>
>

Re: [PATCH RESEND V5 6/6] perf, amd: Enable northbridge performance counters on AMD family 15h

[Stephane Eranian]

On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> On AMD family 15h processors, there are 4 new performance counters
> (in addition to 6 core performance counters) that can be used for
> counting northbridge events (i.e. DRAM accesses). Their bit fields are
> almost identical to the core performance counters. However, unlike the
> core performance counters, these MSRs are shared between multiple
> cores (that share the same northbridge). We will reuse the same code
> path as existing family 10h northbridge event constraints handler
> logic to enforce this sharing.
>
> Signed-off-by: Jacob Shin <jacob.shin@amd.com>
> ---
>  arch/x86/include/asm/cpufeature.h     |    2 +
>  arch/x86/include/asm/perf_event.h     |    9 ++
>  arch/x86/include/uapi/asm/msr-index.h |    2 +
>  arch/x86/kernel/cpu/perf_event_amd.c  |  167 +++++++++++++++++++++++++++++----
>  4 files changed, 160 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 2d9075e..93fe929 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -167,6 +167,7 @@
>  #define X86_FEATURE_TBM                (6*32+21) /* trailing bit manipulations */
>  #define X86_FEATURE_TOPOEXT    (6*32+22) /* topology extensions CPUID leafs */
>  #define X86_FEATURE_PERFCTR_CORE (6*32+23) /* core performance counter extensions */
> +#define X86_FEATURE_PERFCTR_NB  (6*32+24) /* NB performance counter extensions */
>
>  /*
>   * Auxiliary flags: Linux defined - For features scattered in various
> @@ -309,6 +310,7 @@ extern const char * const x86_power_flags[32];
>  #define cpu_has_hypervisor     boot_cpu_has(X86_FEATURE_HYPERVISOR)
>  #define cpu_has_pclmulqdq      boot_cpu_has(X86_FEATURE_PCLMULQDQ)
>  #define cpu_has_perfctr_core   boot_cpu_has(X86_FEATURE_PERFCTR_CORE)
> +#define cpu_has_perfctr_nb     boot_cpu_has(X86_FEATURE_PERFCTR_NB)
>  #define cpu_has_cx8            boot_cpu_has(X86_FEATURE_CX8)
>  #define cpu_has_cx16           boot_cpu_has(X86_FEATURE_CX16)
>  #define cpu_has_eager_fpu      boot_cpu_has(X86_FEATURE_EAGER_FPU)
> diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
> index 2234eaaec..57cb634 100644
> --- a/arch/x86/include/asm/perf_event.h
> +++ b/arch/x86/include/asm/perf_event.h
> @@ -29,9 +29,14 @@
>  #define ARCH_PERFMON_EVENTSEL_INV                      (1ULL << 23)
>  #define ARCH_PERFMON_EVENTSEL_CMASK                    0xFF000000ULL
>
> +#define AMD64_EVENTSEL_INT_CORE_ENABLE                 (1ULL << 36)
>  #define AMD64_EVENTSEL_GUESTONLY                       (1ULL << 40)
>  #define AMD64_EVENTSEL_HOSTONLY                                (1ULL << 41)
>
> +#define AMD64_EVENTSEL_INT_CORE_SEL_SHIFT              37
> +#define AMD64_EVENTSEL_INT_CORE_SEL_MASK               \
> +       (0xFULL << AMD64_EVENTSEL_INT_CORE_SEL_SHIFT)
> +
Interestingly enough, this bitfield is not yet documented in the
public BKDG from March 2012.
I assume it will be in the next rev.

>  #define AMD64_EVENTSEL_EVENT   \
>         (ARCH_PERFMON_EVENTSEL_EVENT | (0x0FULL << 32))
>  #define INTEL_ARCH_EVENT_MASK  \
> @@ -46,8 +51,12 @@
>  #define AMD64_RAW_EVENT_MASK           \
>         (X86_RAW_EVENT_MASK          |  \
>          AMD64_EVENTSEL_EVENT)
> +#define AMD64_RAW_EVENT_MASK_NB                \
> +       (AMD64_EVENTSEL_EVENT        |  \
> +        ARCH_PERFMON_EVENTSEL_UMASK)
>  #define AMD64_NUM_COUNTERS                             4
>  #define AMD64_NUM_COUNTERS_CORE                                6
> +#define AMD64_NUM_COUNTERS_NB                          4
>
>  #define ARCH_PERFMON_UNHALTED_CORE_CYCLES_SEL          0x3c
>  #define ARCH_PERFMON_UNHALTED_CORE_CYCLES_UMASK                (0x00 << 8)
> diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
> index 433a59f..075a402 100644
> --- a/arch/x86/include/uapi/asm/msr-index.h
> +++ b/arch/x86/include/uapi/asm/msr-index.h
> @@ -194,6 +194,8 @@
>  /* Fam 15h MSRs */
>  #define MSR_F15H_PERF_CTL              0xc0010200
>  #define MSR_F15H_PERF_CTR              0xc0010201
> +#define MSR_F15H_NB_PERF_CTL           0xc0010240
> +#define MSR_F15H_NB_PERF_CTR           0xc0010241
>
>  /* Fam 10h MSRs */
>  #define MSR_FAM10H_MMIO_CONF_BASE      0xc0010058
> diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
> index faf9072..1a80e05 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd.c
> @@ -132,11 +132,14 @@ static u64 amd_pmu_event_map(int hw_event)
>         return amd_perfmon_event_map[hw_event];
>  }
>
> +static struct event_constraint *amd_nb_event_constraint;
> +
>  /*
>   * Previously calculated offsets
>   */
>  static unsigned int event_offsets[X86_PMC_IDX_MAX] __read_mostly;
>  static unsigned int count_offsets[X86_PMC_IDX_MAX] __read_mostly;
> +static unsigned int rdpmc_indexes[X86_PMC_IDX_MAX] __read_mostly;
>
>  /*
>   * Legacy CPUs:
> @@ -144,10 +147,14 @@ static unsigned int count_offsets[X86_PMC_IDX_MAX] __read_mostly;
>   *
>   * CPUs with core performance counter extensions:
>   *   6 counters starting at 0xc0010200 each offset by 2
> + *
> + * CPUs with north bridge performance counter extensions:
> + *   4 additional counters starting at 0xc0010240 each offset by 2
> + *   (indexed right above either one of the above core counters)
>   */
>  static inline int amd_pmu_addr_offset(int index, int eventsel)
>  {
> -       int offset;
> +       int offset, first, base;
>
>         if (!index)
>                 return index;
> @@ -160,7 +167,23 @@ static inline int amd_pmu_addr_offset(int index, int eventsel)
>         if (offset)
>                 return offset;
>
> -       if (!cpu_has_perfctr_core)
> +       if (amd_nb_event_constraint &&
> +           test_bit(index, amd_nb_event_constraint->idxmsk)) {
> +               /*
> +                * calculate the offset of NB counters with respect to
> +                * base eventsel or perfctr
> +                */
> +
> +               first = find_first_bit(amd_nb_event_constraint->idxmsk,
> +                                      X86_PMC_IDX_MAX);
> +
> +               if (eventsel)
> +                       base = MSR_F15H_NB_PERF_CTL - x86_pmu.eventsel;
> +               else
> +                       base = MSR_F15H_NB_PERF_CTR - x86_pmu.perfctr;
> +
> +               offset = base + ((index - first) << 1);
> +       } else if (!cpu_has_perfctr_core)
>                 offset = index;
>         else
>                 offset = index << 1;
> @@ -175,24 +198,36 @@ static inline int amd_pmu_addr_offset(int index, int eventsel)
>
>  static inline int amd_pmu_rdpmc_index(int index)
>  {
> -       return index;
> -}
> +       int ret, first;
>
> -static int amd_pmu_hw_config(struct perf_event *event)
> -{
> -       int ret;
> +       if (!index)
> +               return index;
>
> -       /* pass precise event sampling to ibs: */
> -       if (event->attr.precise_ip && get_ibs_caps())
> -               return -ENOENT;
> +       ret = rdpmc_indexes[index];
>
> -       ret = x86_pmu_hw_config(event);
>         if (ret)
>                 return ret;
>
> -       if (has_branch_stack(event))
> -               return -EOPNOTSUPP;
> +       if (amd_nb_event_constraint &&
> +           test_bit(index, amd_nb_event_constraint->idxmsk)) {
> +               /*
> +                * according to the mnual, ECX value of the NB counters is
> +                * the index of the NB counter (0, 1, 2 or 3) plus 6
> +                */
> +
> +               first = find_first_bit(amd_nb_event_constraint->idxmsk,
> +                                      X86_PMC_IDX_MAX);
> +               ret = index - first + 6;
> +       } else
> +               ret = index;
> +
> +       rdpmc_indexes[index] = ret;
>
> +       return ret;
> +}
> +
> +static int amd_core_hw_config(struct perf_event *event)
> +{
>         if (event->attr.exclude_host && event->attr.exclude_guest)
>                 /*
>                  * When HO == GO == 1 the hardware treats that as GO == HO == 0
> @@ -206,10 +241,29 @@ static int amd_pmu_hw_config(struct perf_event *event)
>         else if (event->attr.exclude_guest)
>                 event->hw.config |= AMD64_EVENTSEL_HOSTONLY;
>
> -       if (event->attr.type != PERF_TYPE_RAW)
> -               return 0;
> +       return 0;
> +}
>
> -       event->hw.config |= event->attr.config & AMD64_RAW_EVENT_MASK;
> +/*
> + * NB counters do not support the following event select bits:
> + *   Host/Guest only
> + *   Counter mask
> + *   Invert counter mask
> + *   Edge detect
> + *   OS/User mode
> + */
> +static int amd_nb_hw_config(struct perf_event *event)
> +{
> +       if (event->attr.exclude_user || event->attr.exclude_kernel ||
> +           event->attr.exclude_host || event->attr.exclude_guest)
> +               return -EINVAL;
> +
> +       event->hw.config &= ~(ARCH_PERFMON_EVENTSEL_USR |
> +                             ARCH_PERFMON_EVENTSEL_OS);
> +
> +       if (event->hw.config & ~(AMD64_RAW_EVENT_MASK_NB |
> +                                ARCH_PERFMON_EVENTSEL_INT))
> +               return -EINVAL;
>
>         return 0;
>  }
> @@ -227,6 +281,11 @@ static inline int amd_is_nb_event(struct hw_perf_event *hwc)
>         return (hwc->config & 0xe0) == 0xe0;
>  }
>
> +static inline int amd_is_perfctr_nb_event(struct hw_perf_event *hwc)
> +{
> +       return amd_nb_event_constraint && amd_is_nb_event(hwc);
> +}
> +
>  static inline int amd_has_nb(struct cpu_hw_events *cpuc)
>  {
>         struct amd_nb *nb = cpuc->amd_nb;
> @@ -234,6 +293,30 @@ static inline int amd_has_nb(struct cpu_hw_events *cpuc)
>         return nb && nb->nb_id != -1;
>  }
>
> +static int amd_pmu_hw_config(struct perf_event *event)
> +{
> +       int ret;
> +
> +       /* pass precise event sampling to ibs: */
> +       if (event->attr.precise_ip && get_ibs_caps())
> +               return -ENOENT;
> +
> +       if (has_branch_stack(event))
> +               return -EOPNOTSUPP;
> +
> +       ret = x86_pmu_hw_config(event);
> +       if (ret)
> +               return ret;
> +
> +       if (event->attr.type == PERF_TYPE_RAW)
> +               event->hw.config |= event->attr.config & AMD64_RAW_EVENT_MASK;
> +
> +       if (amd_is_perfctr_nb_event(&event->hw))
> +               return amd_nb_hw_config(event);
> +
> +       return amd_core_hw_config(event);
> +}
> +
>  static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
>                                            struct perf_event *event)
>  {
> @@ -254,6 +337,19 @@ static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
>         }
>  }
>
> +static void amd_nb_interrupt_hw_config(struct hw_perf_event *hwc)
> +{
> +       int core_id = cpu_data(smp_processor_id()).cpu_core_id;
> +
> +       /* deliver interrupts only to this core */
> +       if (hwc->config & ARCH_PERFMON_EVENTSEL_INT) {
> +               hwc->config |= AMD64_EVENTSEL_INT_CORE_ENABLE;
> +               hwc->config &= ~AMD64_EVENTSEL_INT_CORE_SEL_MASK;
> +               hwc->config |= (u64)(core_id) <<
> +                       AMD64_EVENTSEL_INT_CORE_SEL_SHIFT;
> +       }
> +}
> +
Well, given the model that you are using, i.e., fused with the core
PMU, then you can
supposedly measure NB events in per-thread mode. If the thread migrates from one
CPU to another. Then the uncore interrupt has to follow. Otherwise, you
may get an interrupt on CPU0 where there is no active events if the
thread is now
running on CPU1, for instance. So this does not work well. I think for
NB events,
you may want to disable per-thread support. This is how it's done for
Intel uncore.

The  NB interrupt is useful for:
- 64-bit virtualization of the HW counter
- sampling.

Now, I admit given the list NB events, it does not really make sense
to sample on
those and hope to correlate to a meaningful address in a program. For
all I know,
the actual cause of an event may come from a program that is not even running on
the measured core (assuming you were doing: perf record -a -C 0 -e
r01e0 sleep 10).

Next, I tried running a simple example:

  $ perf record -a -C 0 -e r01e0 sleep 10

But, there is nothing running on CPU0, so it's idle and even goes low-power.
Yet, if on CPU1 there is load causing memory traffic and therefore
firing the event.

First, in your setup, you're using the current CPU and NOT the target CPU of the
event. Even though, you want to measure CPU0, you can be invoking
perf_event_open()
from CPU3. Unless that setup phase is guaranteed to ALWAYS run on the target
CPU  for the event, here CPU0, that setup won't work.

I am using a simple memory benchmark called triad for which I can pin where
the memory init (-i) and execution (-r) occur. So here I run everything on CPU0
and I sample only one NB event via CPU0. First, I count the actual number of
events in per-process mode. This gives me a ballpark figure for a sampling
period.

$ perf stat --no-big-num --pfm-event dram_accesses:dct0_page_miss
triad -i 0 -r 0
          62302388 dram_accesses:dct0_page_miss
       6.099754492 seconds time elapsed

Want about 1000 samples, so I use -c 62302388/1000

$ perf record -a -C 0  -c 62302  --pfm-event
dram_accesses:dct0_page_miss  triad -i 0 -r 0
$ perf report -D | tail -6
dram_accesses:dct0_page_miss stats:
           TOTAL events:       3451
            MMAP events:       2197
            COMM events:        254
            EXIT events:          2
          SAMPLE events:        998

Got about a 1000 samples. All is good.

Now, I modify the setup by forcing triad to init and run on CPU3 but I
am still measuring
from CPU0:

$ perf record -a -C 0  -c 62302  --pfm-event
dram_accesses:dct0_page_miss  triad -i 3 -r 3 -l 3
$ perf report -D | tail -6
           TOTAL events:       2792
            MMAP events:       2201
            COMM events:        255
        THROTTLE events:          3
      UNTHROTTLE events:          3
          SAMPLE events:        330

I have lost 2/3rd of the samples. Why?

NB counters are shared across all 4 cores (of my Fam15h CPU), so when
the counter
overflows it will interrupt CPU0. But how come samples disappear?

At first, I thought it could be due to the same problem that exists on
Intel uncore:
uncore interrupt does not wake up a core in low-power state. But that
is not quite
the problem here. Instead, you have to look at throttling. Why is this
run throttled?
Capturing about 1000 samples over 6s. We are very far from the default
100000 intrs/event/cpu/s. So something is wrong here. Looks like CPU0 may get
a burst of NB interrupts and decides the throttle for a timer tick or
so. It does not
happen a lot but enough to cause 2/3rd of the sample to disappear. If I raise
the threshold to 1000000 (million), then the problem goes away:

$ perf report -D | tail -6
dram_accesses:dct0_page_miss stats:
           TOTAL events:       3296
            MMAP events:       2037
            COMM events:        249
          SAMPLE events:       1010

So something is broken with NB interrupts and I don't quite know what it is.

But in general, I remain convinced that unless you have broadcast
interrupt and perf_event is modified to handle this case, sampling on
the NB is pointless. So we might as well disable this and keep counting.
But that does still require working NB interrupts. Worst case, we can do
like on Intel and use hrtimer-based polling.


>   /*
>    * AMD64 NorthBridge events need special treatment because
>    * counter access needs to be synchronized across all cores
> @@ -299,6 +395,12 @@ __amd_get_nb_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *ev
>         struct perf_event *old;
>         int idx, new = -1;
>
> +       if (!c)
> +               c = &unconstrained;
> +
> +       if (cpuc->is_fake)
> +               return c;
> +
>         /*
>          * detect if already present, if so reuse
>          *
> @@ -335,6 +437,9 @@ __amd_get_nb_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *ev
>         if (new == -1)
>                 return &emptyconstraint;
>
> +       if (amd_is_perfctr_nb_event(hwc))
> +               amd_nb_interrupt_hw_config(hwc);
> +
>         return &nb->event_constraints[new];
>  }
>
> @@ -434,7 +539,8 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
>         if (!(amd_has_nb(cpuc) && amd_is_nb_event(&event->hw)))
>                 return &unconstrained;
>
> -       return __amd_get_nb_event_constraints(cpuc, event, &unconstrained);
> +       return __amd_get_nb_event_constraints(cpuc, event,
> +                                             amd_nb_event_constraint);
>  }
>
>  static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
> @@ -533,6 +639,9 @@ static struct event_constraint amd_f15_PMC30 = EVENT_CONSTRAINT_OVERLAP(0, 0x09,
>  static struct event_constraint amd_f15_PMC50 = EVENT_CONSTRAINT(0, 0x3F, 0);
>  static struct event_constraint amd_f15_PMC53 = EVENT_CONSTRAINT(0, 0x38, 0);
>
> +static struct event_constraint amd_NBPMC96 = EVENT_CONSTRAINT(0, 0x3C0, 0);
> +static struct event_constraint amd_NBPMC74 = EVENT_CONSTRAINT(0, 0xF0, 0);
> +
>  static struct event_constraint *
>  amd_get_event_constraints_f15h(struct cpu_hw_events *cpuc, struct perf_event *event)
>  {
> @@ -598,8 +707,8 @@ amd_get_event_constraints_f15h(struct cpu_hw_events *cpuc, struct perf_event *ev
>                         return &amd_f15_PMC20;
>                 }
>         case AMD_EVENT_NB:
> -               /* not yet implemented */
> -               return &emptyconstraint;
> +               return __amd_get_nb_event_constraints(cpuc, event,
> +                                                     amd_nb_event_constraint);
>         default:
>                 return &emptyconstraint;
>         }
> @@ -647,7 +756,7 @@ static __initconst const struct x86_pmu amd_pmu = {
>
>  static int setup_event_constraints(void)
>  {
> -       if (boot_cpu_data.x86 >= 0x15)
> +       if (boot_cpu_data.x86 == 0x15)
>                 x86_pmu.get_event_constraints = amd_get_event_constraints_f15h;
>         return 0;
>  }
> @@ -677,6 +786,23 @@ static int setup_perfctr_core(void)
>         return 0;
>  }
>
> +static int setup_perfctr_nb(void)
> +{
> +       if (!cpu_has_perfctr_nb)
> +               return -ENODEV;
> +
> +       x86_pmu.num_counters += AMD64_NUM_COUNTERS_NB;
> +
> +       if (cpu_has_perfctr_core)
> +               amd_nb_event_constraint = &amd_NBPMC96;
> +       else
> +               amd_nb_event_constraint = &amd_NBPMC74;
> +
> +       printk(KERN_INFO "perf: AMD northbridge performance counters detected\n");
> +
> +       return 0;
> +}
> +
>  __init int amd_pmu_init(void)
>  {
>         /* Performance-monitoring supported from K7 and later: */
> @@ -687,6 +813,7 @@ __init int amd_pmu_init(void)
>
>         setup_event_constraints();
>         setup_perfctr_core();
> +       setup_perfctr_nb();
>
>         /* Events are common for all AMDs */
>         memcpy(hw_cache_event_ids, amd_hw_cache_event_ids,
> --
> 1.7.9.5
>
>

Re: [perfmon2] [PATCH RESEND V5 0/6] perf, amd: Enable AMD family 15h northbridge counters

[Jacob Shin]

On Fri, Jan 25, 2013 at 10:42:57AM +0100, Stephane Eranian wrote:
> Hi Jacob,
> 
> I will apply this patch to libpfm4.
> But I have a question. Why aren't the other uncore
> events included here as well? I am talking about
> the events listed in BKDG sections 3.16.2 to 3.16.6?
> Are  those NOT supported by your kernel patchset?

Oh, you are right, they are supported. I'm not sure why I overlooked
them. I will send out a V2 that includes all of those events as well.

Sorry about that.

-Jacob

> 
> 
> On Thu, Jan 24, 2013 at 11:06 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> > On Thu, Jan 24, 2013 at 02:31:59PM +0100, Stephane Eranian wrote:
> >> On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> >> > The following patchset enables 4 additional performance counters in
> >> > AMD family 15h processors that count northbridge events -- such as
> >> > number of DRAM accesses.
> >> >
> >> In order for me to test this patch set more thoroughly it would help if you
> >> could also provide me a patch to add the Fam15h uncore events to libpfm4.
> >> In the past, Robert Richter took care of this. I hope you can fill his role for
> >> this. So please, if you could send me the patch quickly, that would help
> >> the review of your patch.
> >
> > Hi Stephane,
> >
> > Here is the corresponding libpfm4 patch. Thank you for taking the time
> > to review the patchset. I hope this helps .. If we can get AMD related
> > perf kernel side patchsets to upstream, I will be more than happy to
> > support AMD related libpfm4 efforts going forward.
> >
> > Thanks!
> >
> > >From 47d3267dfa24b9071c76f4a22bd059b0e4032002 Mon Sep 17 00:00:00 2001
> > From: Jacob Shin <jacob.shin@amd.com>
> > Date: Thu, 24 Jan 2013 15:37:37 -0600
> > Subject: [PATCH 1/1] Add AMD Family 15h northbridge performance events
> >
> > libpfm4 side support for the following Linux kernel patchset:
> >   http://lkml.org/lkml/2013/1/10/450
> >
> > Reference -- BIOS and Kernel Developer Guide (BKDG) for AMD Family 15h
> >  Models 00h-0Fh Processors:
> >   http://support.amd.com/us/Processor_TechDocs/42301_15h_Mod_00h-0Fh_BKDG.pdf
> >
> > Signed-off-by: Jacob Shin <jacob.shin@amd.com>
> > ---
> >  lib/events/amd64_events_fam15h.h |  155 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 155 insertions(+)
> >
> > diff --git a/lib/events/amd64_events_fam15h.h b/lib/events/amd64_events_fam15h.h
> > index 7f654e8..0276782 100644
> > --- a/lib/events/amd64_events_fam15h.h
> > +++ b/lib/events/amd64_events_fam15h.h
> > @@ -752,6 +752,126 @@ static const amd64_umask_t amd64_fam15h_l2_prefetcher_trigger_events[]={
> >     },
> >  };
> >
> > +static const amd64_umask_t amd64_fam15h_dram_accesses[]={
> > +   { .uname = "DCT0_PAGE_HIT",
> > +     .udesc = "DCT0 Page hit",
> > +     .ucode = 0x1,
> > +   },
> > +   { .uname = "DCT0_PAGE_MISS",
> > +     .udesc = "DCT0 Page Miss",
> > +     .ucode = 0x2,
> > +   },
> > +   { .uname = "DCT0_PAGE_CONFLICT",
> > +     .udesc = "DCT0 Page Conflict",
> > +     .ucode = 0x4,
> > +   },
> > +   { .uname = "DCT1_PAGE_HIT",
> > +     .udesc = "DCT1 Page hit",
> > +     .ucode = 0x8,
> > +   },
> > +   { .uname = "DCT1_PAGE_MISS",
> > +     .udesc = "DCT1 Page Miss",
> > +     .ucode = 0x10,
> > +   },
> > +   { .uname = "DCT1_PAGE_CONFLICT",
> > +     .udesc = "DCT1 Page Conflict",
> > +     .ucode = 0x20,
> > +   },
> > +   { .uname  = "ALL",
> > +     .udesc  = "All sub-events selected",
> > +     .ucode  = 0x3f,
> > +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> > +   },
> > +};
> > +
> > +static const amd64_umask_t amd64_fam15h_dram_controller_page_table_overflows[]={
> > +   { .uname = "DCT0_PAGE_TABLE_OVERFLOW",
> > +     .udesc = "DCT0 Page Table Overflow",
> > +     .ucode = 0x1,
> > +   },
> > +   { .uname = "DCT1_PAGE_TABLE_OVERFLOW",
> > +     .udesc = "DCT1 Page Table Overflow",
> > +     .ucode = 0x2,
> > +   },
> > +   { .uname  = "ALL",
> > +     .udesc  = "All sub-events selected",
> > +     .ucode  = 0x3,
> > +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> > +   },
> > +};
> > +
> > +static const amd64_umask_t amd64_fam15h_memory_controller_dram_command_slots_missed[]={
> > +   { .uname = "DCT0_COMMAND_SLOTS_MISSED",
> > +     .udesc = "DCT0 Command Slots Missed (in MemClks)",
> > +     .ucode = 0x1,
> > +   },
> > +   { .uname = "DCT1_COMMAND_SLOTS_MISSED",
> > +     .udesc = "DCT1 Command Slots Missed (in MemClks)",
> > +     .ucode = 0x2,
> > +   },
> > +   { .uname  = "ALL",
> > +     .udesc  = "All sub-events selected",
> > +     .ucode  = 0x3,
> > +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> > +   },
> > +};
> > +
> > +static const amd64_umask_t amd64_fam15h_memory_controller_turnarounds[]={
> > +   { .uname = "DCT0_DIMM_TURNAROUND",
> > +     .udesc = "DCT0 DIMM (chip select) turnaround",
> > +     .ucode = 0x1,
> > +   },
> > +   { .uname = "DCT0_READ_TO_WRITE_TURNAROUND",
> > +     .udesc = "DCT0 Read to write turnaround",
> > +     .ucode = 0x2,
> > +   },
> > +   { .uname = "DCT0_WRITE_TO_READ_TURNAROUND",
> > +     .udesc = "DCT0 Write to read turnaround",
> > +     .ucode = 0x4,
> > +   },
> > +   { .uname = "DCT1_DIMM_TURNAROUND",
> > +     .udesc = "DCT1 DIMM (chip select) turnaround",
> > +     .ucode = 0x8,
> > +   },
> > +   { .uname = "DCT1_READ_TO_WRITE_TURNAROUND",
> > +     .udesc = "DCT1 Read to write turnaround",
> > +     .ucode = 0x10,
> > +   },
> > +   { .uname = "DCT1_WRITE_TO_READ_TURNAROUND",
> > +     .udesc = "DCT1 Write to read turnaround",
> > +     .ucode = 0x20,
> > +   },
> > +   { .uname  = "ALL",
> > +     .udesc  = "All sub-events selected",
> > +     .ucode  = 0x3f,
> > +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> > +   },
> > +};
> > +
> > +static const amd64_umask_t amd64_fam15h_memory_controller_bypass_counter_saturation[]={
> > +   { .uname = "MEMORY_CONTROLLER_HIGH_PRIORITY_BYPASS",
> > +     .udesc = "Memory controller high priority bypass",
> > +     .ucode = 0x1,
> > +   },
> > +   { .uname = "MEMORY_CONTROLLER_MEDIUM_PRIORITY_BYPASS",
> > +     .udesc = "Memory controller medium priority bypass",
> > +     .ucode = 0x2,
> > +   },
> > +   { .uname = "DCT0_DCQ_BYPASS",
> > +     .udesc = "DCT0 DCQ bypass",
> > +     .ucode = 0x4,
> > +   },
> > +   { .uname = "DCT1_DCQ_BYPASS",
> > +     .udesc = "DCT1 DCQ bypass",
> > +     .ucode = 0x8,
> > +   },
> > +   { .uname  = "ALL",
> > +     .udesc  = "All sub-events selected",
> > +     .ucode  = 0xf,
> > +     .uflags = AMD64_FL_NCOMBO | AMD64_FL_DFL,
> > +   },
> > +};
> > +
> >  static const amd64_entry_t amd64_fam15h_pe[]={
> >  { .name    = "DISPATCHED_FPU_OPS",
> >    .desc    = "FPU Pipe Assignment",
> > @@ -1256,4 +1376,39 @@ static const amd64_entry_t amd64_fam15h_pe[]={
> >    .modmsk  = AMD64_FAM15H_ATTRS,
> >    .code    = 0x1d8,
> >  },
> > +{ .name    = "DRAM_ACCESSES",
> > +  .desc    = "DRAM Accesses",
> > +  .code    = 0xe0,
> > +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_dram_accesses),
> > +  .ngrp    = 1,
> > +  .umasks  = amd64_fam15h_dram_accesses,
> > +},
> > +{ .name    = "DRAM_CONTROLLER_PAGE_TABLE_OVERFLOWS",
> > +  .desc    = "DRAM Controller Page Table Overflows",
> > +  .code    = 0xe1,
> > +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_dram_controller_page_table_overflows),
> > +  .ngrp    = 1,
> > +  .umasks  = amd64_fam15h_dram_controller_page_table_overflows,
> > +},
> > +{ .name    = "MEMORY_CONTROLLER_DRAM_COMMAND_SLOTS_MISSED",
> > +  .desc    = "Memory Controller DRAM Command Slots Missed",
> > +  .code    = 0xe2,
> > +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_dram_command_slots_missed),
> > +  .ngrp    = 1,
> > +  .umasks  = amd64_fam15h_memory_controller_dram_command_slots_missed,
> > +},
> > +{ .name    = "MEMORY_CONTROLLER_TURNAROUNDS",
> > +  .desc    = "Memory Controller Turnarounds",
> > +  .code    = 0xe3,
> > +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_turnarounds),
> > +  .ngrp    = 1,
> > +  .umasks  = amd64_fam15h_memory_controller_turnarounds,
> > +},
> > +{ .name    = "MEMORY_CONTROLLER_BYPASS_COUNTER_SATURATION",
> > +  .desc    = "Memory Controller Bypass Counter Saturation",
> > +  .code    = 0xe4,
> > +  .numasks = LIBPFM_ARRAY_SIZE(amd64_fam15h_memory_controller_bypass_counter_saturation),
> > +  .ngrp    = 1,
> > +  .umasks  = amd64_fam15h_memory_controller_bypass_counter_saturation,
> > +},
> >  };
> > --
> > 1.7.9.5
> >
> >
> >
> > ------------------------------------------------------------------------------
> > Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
> > MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
> > with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
> > MVPs and experts. ON SALE this month only -- learn more at:
> > http://p.sf.net/sfu/learnnow-d2d
> > _______________________________________________
> > perfmon2-devel mailing list
> > perfmon2-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
> 

Re: [PATCH RESEND V5 2/6] perf, amd: Generalize northbridge constraints code for family 15h

[Jacob Shin]

On Fri, Jan 25, 2013 at 12:07:40PM +0100, Stephane Eranian wrote:
> On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> > From: Robert Richter <rric@kernel.org>
> >
> > Generalize northbridge constraints code for family 10h so that later
> > we can reuse the same code path with other AMD processor families that
> > have the same northbridge event constraints.
> >
> > Signed-off-by: Robert Richter <rric@kernel.org>
> > Signed-off-by: Jacob Shin <jacob.shin@amd.com>
> > ---
> >  arch/x86/kernel/cpu/perf_event_amd.c |   43 ++++++++++++++++++++--------------
> >  1 file changed, 25 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
> > index e7963c7..9541fe5 100644
> > --- a/arch/x86/kernel/cpu/perf_event_amd.c
> > +++ b/arch/x86/kernel/cpu/perf_event_amd.c
> > @@ -188,20 +188,13 @@ static inline int amd_has_nb(struct cpu_hw_events *cpuc)
> >         return nb && nb->nb_id != -1;
> >  }
> >
> > -static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
> > -                                     struct perf_event *event)
> > +static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
> > +                                          struct perf_event *event)
> >  {
> > -       struct hw_perf_event *hwc = &event->hw;
> >         struct amd_nb *nb = cpuc->amd_nb;
> >         int i;
> >
> >         /*
> > -        * only care about NB events
> > -        */
> > -       if (!(amd_has_nb(cpuc) && amd_is_nb_event(hwc)))
> > -               return;
> > -
> > -       /*
> >          * need to scan whole list because event may not have
> >          * been assigned during scheduling
> >          *
> > @@ -247,12 +240,13 @@ static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
> >    *
> >    * Given that resources are allocated (cmpxchg), they must be
> >    * eventually freed for others to use. This is accomplished by
> > -  * calling amd_put_event_constraints().
> > +  * calling __amd_put_nb_event_constraints()
> >    *
> >    * Non NB events are not impacted by this restriction.
> >    */
> >  static struct event_constraint *
> > -amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
> > +__amd_get_nb_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event,
> > +                              struct event_constraint *c)
> >  {
> >         struct hw_perf_event *hwc = &event->hw;
> >         struct amd_nb *nb = cpuc->amd_nb;
> > @@ -260,12 +254,6 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
> >         int idx, new = -1;
> >
> >         /*
> > -        * if not NB event or no NB, then no constraints
> > -        */
> > -       if (!(amd_has_nb(cpuc) && amd_is_nb_event(hwc)))
> > -               return &unconstrained;
> > -
> > -       /*
> >          * detect if already present, if so reuse
> >          *
> >          * cannot merge with actual allocation
> > @@ -275,7 +263,7 @@ amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
> >          * because of successive calls to x86_schedule_events() from
> >          * hw_perf_group_sched_in() without hw_perf_enable()
> >          */
> > -       for (idx = 0; idx < x86_pmu.num_counters; idx++) {
> > +       for_each_set_bit(idx, c->idxmsk, X86_PMC_IDX_MAX) {
> 
> So here you're using   X86_PMC_IDX_MAX but in
> __amd_put_nb_event_constraints() you're using
> x86_pmu.num_counters.
> 
> There is implicit assumption in the AMD code the counters index
> namespace is contiguous. That
> means the uncore counters show up right after the core counters. On
> Fam15h, that would be NB
> counters start at index 6, on Fam10h at index 4. In that case, the
> constraint mask cannot have bits set
> beyond num_counters, so why not use that limit in
> amd_get_event_constraints()? It would significantly
> cut down on the number of iterations in the loop from 64 down to 10 on Fam15h.

Yes, you are right, I will change that in V6.

Thanks,

> 
> 
> >                 if (new == -1 || hwc->idx == idx)
> >                         /* assign free slot, prefer hwc->idx */
> >                         old = cmpxchg(nb->owners + idx, NULL, event);
> > @@ -391,6 +379,25 @@ static void amd_pmu_cpu_dead(int cpu)
> >         }
> >  }
> >
> > +static struct event_constraint *
> > +amd_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
> > +{
> > +       /*
> > +        * if not NB event or no NB, then no constraints
> > +        */
> > +       if (!(amd_has_nb(cpuc) && amd_is_nb_event(&event->hw)))
> > +               return &unconstrained;
> > +
> > +       return __amd_get_nb_event_constraints(cpuc, event, &unconstrained);
> > +}
> > +
> > +static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
> > +                                     struct perf_event *event)
> > +{
> > +       if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
> > +               __amd_put_nb_event_constraints(cpuc, event);
> > +}
> > +
> >  PMU_FORMAT_ATTR(event, "config:0-7,32-35");
> >  PMU_FORMAT_ATTR(umask, "config:8-15"   );
> >  PMU_FORMAT_ATTR(edge,  "config:18"     );
> > --
> > 1.7.9.5
> >
> >
> 

Re: [PATCH RESEND V5 4/6] perf, x86: Move MSR address offset calculation to architecture specific files

[Jacob Shin]

On Fri, Jan 25, 2013 at 12:15:37PM +0100, Stephane Eranian wrote:
> On Thu, Jan 10, 2013 at 8:50 PM, Jacob Shin <jacob.shin@amd.com> wrote:
> > Move counter index to MSR address offset calculation to architecture
> > specific files. This prepares the way for perf_event_amd to enable
> > counter addresses that are not contiguous -- for example AMD Family
> > 15h processors have 6 core performance counters starting at 0xc0010200
> > and 4 northbridge performance counters starting at 0xc0010240.
> >
> > Signed-off-by: Jacob Shin <jacob.shin@amd.com>
> > ---
> >  arch/x86/kernel/cpu/perf_event.h     |   21 ++++-------------
> >  arch/x86/kernel/cpu/perf_event_amd.c |   42 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+), 16 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
> > index 115c1ea..4440218 100644
> > --- a/arch/x86/kernel/cpu/perf_event.h
> > +++ b/arch/x86/kernel/cpu/perf_event.h
> > @@ -325,6 +325,7 @@ struct x86_pmu {
> >         int             (*schedule_events)(struct cpu_hw_events *cpuc, int n, int *assign);
> >         unsigned        eventsel;
> >         unsigned        perfctr;
> > +       int             (*addr_offset)(int index, int eventsel);
> >         u64             (*event_map)(int);
> >         int             max_events;
> >         int             num_counters;
> > @@ -446,28 +447,16 @@ extern u64 __read_mostly hw_cache_extra_regs
> >
> >  u64 x86_perf_event_update(struct perf_event *event);
> >
> > -static inline int x86_pmu_addr_offset(int index)
> > -{
> > -       int offset;
> > -
> > -       /* offset = X86_FEATURE_PERFCTR_CORE ? index << 1 : index */
> > -       alternative_io(ASM_NOP2,
> > -                      "shll $1, %%eax",
> > -                      X86_FEATURE_PERFCTR_CORE,
> > -                      "=a" (offset),
> > -                      "a"  (index));
> > -
> > -       return offset;
> > -}
> > -
> >  static inline unsigned int x86_pmu_config_addr(int index)
> >  {
> > -       return x86_pmu.eventsel + x86_pmu_addr_offset(index);
> > +       return x86_pmu.eventsel +
> > +               (x86_pmu.addr_offset ? x86_pmu.addr_offset(index, 1) : index);
> >  }
> >
> >  static inline unsigned int x86_pmu_event_addr(int index)
> >  {
> > -       return x86_pmu.perfctr + x86_pmu_addr_offset(index);
> > +       return x86_pmu.perfctr +
> > +               (x86_pmu.addr_offset ? x86_pmu.addr_offset(index, 0) : index);
> >  }
> Would be better to use a constant name instead of 1 and 0 to name a event_sel
> vs. a counter. It would help the reader understand what this is about
> as that may
> be useful for other processors as well.

Yes will do .. or should I use bool instead? Which would be preferred?

> 
> >  int x86_setup_perfctr(struct perf_event *event);
> > diff --git a/arch/x86/kernel/cpu/perf_event_amd.c b/arch/x86/kernel/cpu/perf_event_amd.c
> > index 0c2cc51..ef1df38 100644
> > --- a/arch/x86/kernel/cpu/perf_event_amd.c
> > +++ b/arch/x86/kernel/cpu/perf_event_amd.c
> > @@ -132,6 +132,47 @@ static u64 amd_pmu_event_map(int hw_event)
> >         return amd_perfmon_event_map[hw_event];
> >  }
> >
> > +/*
> > + * Previously calculated offsets
> > + */
> > +static unsigned int event_offsets[X86_PMC_IDX_MAX] __read_mostly;
> > +static unsigned int count_offsets[X86_PMC_IDX_MAX] __read_mostly;
> > +
> > +/*
> > + * Legacy CPUs:
> > + *   4 counters starting at 0xc0010000 each offset by 1
> > + *
> > + * CPUs with core performance counter extensions:
> > + *   6 counters starting at 0xc0010200 each offset by 2
> > + */
> > +static inline int amd_pmu_addr_offset(int index, int eventsel)
> > +{
> > +       int offset;
> > +
> > +       if (!index)
> > +               return index;
> > +
> > +       if (eventsel)
> > +               offset = event_offsets[index];
> > +       else
> > +               offset = count_offsets[index];
> > +
> > +       if (offset)
> > +               return offset;
> > +
> > +       if (!cpu_has_perfctr_core)
> > +               offset = index;
> > +       else
> > +               offset = index << 1;
> > +
> > +       if (eventsel)
> > +               event_offsets[index] = offset;
> > +       else
> > +               count_offsets[index] = offset;
> > +
> > +       return offset;
> > +}
> > +
> >  static int amd_pmu_hw_config(struct perf_event *event)
> >  {
> >         int ret;
> > @@ -578,6 +619,7 @@ static __initconst const struct x86_pmu amd_pmu = {
> >         .schedule_events        = x86_schedule_events,
> >         .eventsel               = MSR_K7_EVNTSEL0,
> >         .perfctr                = MSR_K7_PERFCTR0,
> > +       .addr_offset            = amd_pmu_addr_offset,
> >         .event_map              = amd_pmu_event_map,
> >         .max_events             = ARRAY_SIZE(amd_perfmon_event_map),
> >         .num_counters           = AMD64_NUM_COUNTERS,
> > --
> > 1.7.9.5
> >
> >
>