[patch 0/4] [perf -tip/master] p4 pmu updates

[patch 0/4] [perf -tip/master] p4 pmu updates

[Cyrill Gorcunov]

Hi, please pick up the following patches. Nothing serious --
a few nitfix and preparation the header for future export into
user space (in a sake of RAW events), though not exported in
real yet. Need to estimate the convenience of its usage.

Comments and complains and etc are highly appreciated!

	-- Cyrill

[patch 1/4] x86,perf: P4 PMU -- do a real check for ESCR address being in hash

[Cyrill Gorcunov]

[-- Attachment #1: x86-perf-p4-escr-check-hash --]
[-- Type: text/plain, Size: 1497 bytes --]

To prevent from clashes in future code modifications do a real
check for ESCR address being in hash. At moment the callers
are known to pass sane values but better to be on a safe side.

And comment fix.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Lin Ming <ming.m.lin@intel.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Ingo Molnar <mingo@elte.hu>
CC: Frederic Weisbecker <fweisbec@gmail.com>
---
 arch/x86/kernel/cpu/perf_event_p4.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event_p4.c
+++ linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
@@ -670,7 +670,7 @@ static void p4_pmu_swap_config_ts(struct
 
 /*
  * ESCR address hashing is tricky, ESCRs are not sequential
- * in memory but all starts from MSR_P4_BSU_ESCR0 (0x03e0) and
+ * in memory but all starts from MSR_P4_BSU_ESCR0 (0x03a0) and
  * the metric between any ESCRs is laid in range [0xa0,0xe1]
  *
  * so we make ~70% filled hashtable
@@ -735,8 +735,9 @@ static int p4_get_escr_idx(unsigned int 
 {
 	unsigned int idx = P4_ESCR_MSR_IDX(addr);
 
-	if (unlikely(idx >= P4_ESCR_MSR_TABLE_SIZE ||
-			!p4_escr_table[idx])) {
+	if (unlikely(idx >= P4_ESCR_MSR_TABLE_SIZE	||
+			!p4_escr_table[idx]		||
+			p4_escr_table[idx] != addr)) {
 		WARN_ONCE(1, "P4 PMU: Wrong address passed: %x\n", addr);
 		return -1;
 	}

[tip:perf/core] perf, x86: P4 PMU -- do a real check for ESCR address being in hash

[tip-bot for Cyrill Gorcunov]

Commit-ID:  623aab896ee1a532cb540bcf0d5ae8a88275afd5
Gitweb:     http://git.kernel.org/tip/623aab896ee1a532cb540bcf0d5ae8a88275afd5
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Wed, 19 May 2010 01:19:17 +0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 19 May 2010 09:41:05 +0200

perf, x86: P4 PMU -- do a real check for ESCR address being in hash

To prevent from clashes in future code modifications
do a real check for ESCR address being in hash. At
moment the callers are known to pass sane values but
better to be on a safe side.

And comment fix.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Lin Ming <ming.m.lin@intel.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <20100518212439.004503600@openvz.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/cpu/perf_event_p4.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_p4.c b/arch/x86/kernel/cpu/perf_event_p4.c
index 87e1803..5f8e36d 100644
--- a/arch/x86/kernel/cpu/perf_event_p4.c
+++ b/arch/x86/kernel/cpu/perf_event_p4.c
@@ -670,7 +670,7 @@ static void p4_pmu_swap_config_ts(struct hw_perf_event *hwc, int cpu)
 
 /*
  * ESCR address hashing is tricky, ESCRs are not sequential
- * in memory but all starts from MSR_P4_BSU_ESCR0 (0x03e0) and
+ * in memory but all starts from MSR_P4_BSU_ESCR0 (0x03a0) and
  * the metric between any ESCRs is laid in range [0xa0,0xe1]
  *
  * so we make ~70% filled hashtable
@@ -735,8 +735,9 @@ static int p4_get_escr_idx(unsigned int addr)
 {
 	unsigned int idx = P4_ESCR_MSR_IDX(addr);
 
-	if (unlikely(idx >= P4_ESCR_MSR_TABLE_SIZE ||
-			!p4_escr_table[idx])) {
+	if (unlikely(idx >= P4_ESCR_MSR_TABLE_SIZE	||
+			!p4_escr_table[idx]		||
+			p4_escr_table[idx] != addr)) {
 		WARN_ONCE(1, "P4 PMU: Wrong address passed: %x\n", addr);
 		return -1;
 	}

[patch 2/4] x86,perf: p4_pmu_schedule_events -- use smp_processor_id instead of raw_

[Cyrill Gorcunov]

[-- Attachment #1: x86-perf-p4-use-smp --]
[-- Type: text/plain, Size: 1290 bytes --]

This snippet somehow escaped the commit

 | commit 137351e0feeb9f25d99488ee1afc1c79f5499a9a
 | Author: Cyrill Gorcunov <gorcunov@openvz.org>
 | Date:   Sat May 8 15:25:52 2010 +0400
 |
 |    x86, perf: P4 PMU -- protect sensible procedures from preemption

so bring it eventually back. It helps to catch preemption issue
(if there will be, rule of thumb -- don't use raw_ if you can).

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Lin Ming <ming.m.lin@intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---
 arch/x86/kernel/cpu/perf_event_p4.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event_p4.c
+++ linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
@@ -762,7 +762,7 @@ static int p4_pmu_schedule_events(struct
 {
 	unsigned long used_mask[BITS_TO_LONGS(X86_PMC_IDX_MAX)];
 	unsigned long escr_mask[BITS_TO_LONGS(P4_ESCR_MSR_TABLE_SIZE)];
-	int cpu = raw_smp_processor_id();
+	int cpu = smp_processor_id();
 	struct hw_perf_event *hwc;
 	struct p4_event_bind *bind;
 	unsigned int i, thread, num;

[tip:perf/core] perf, x86: P4_pmu_schedule_events -- use smp_processor_id instead of raw_

[tip-bot for Cyrill Gorcunov]

Commit-ID:  9d36dfcf219e2ba1f1d169a7f92dcf2cbd4e05f0
Gitweb:     http://git.kernel.org/tip/9d36dfcf219e2ba1f1d169a7f92dcf2cbd4e05f0
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Wed, 19 May 2010 01:19:18 +0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 19 May 2010 09:41:05 +0200

perf, x86: P4_pmu_schedule_events -- use smp_processor_id instead of raw_

This snippet somehow escaped the commit:

 | commit 137351e0feeb9f25d99488ee1afc1c79f5499a9a
 | Author: Cyrill Gorcunov <gorcunov@openvz.org>
 | Date:   Sat May 8 15:25:52 2010 +0400
 |
 |    x86, perf: P4 PMU -- protect sensible procedures from preemption

so bring it eventually back. It helps to catch
preemption issue (if there will be, rule of thumb --
don't use raw_ if you can).

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Lin Ming <ming.m.lin@intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <20100518212439.167259349@openvz.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/cpu/perf_event_p4.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_p4.c b/arch/x86/kernel/cpu/perf_event_p4.c
index 5f8e36d..ae85d69 100644
--- a/arch/x86/kernel/cpu/perf_event_p4.c
+++ b/arch/x86/kernel/cpu/perf_event_p4.c
@@ -763,7 +763,7 @@ static int p4_pmu_schedule_events(struct cpu_hw_events *cpuc, int n, int *assign
 {
 	unsigned long used_mask[BITS_TO_LONGS(X86_PMC_IDX_MAX)];
 	unsigned long escr_mask[BITS_TO_LONGS(P4_ESCR_MSR_TABLE_SIZE)];
-	int cpu = raw_smp_processor_id();
+	int cpu = smp_processor_id();
 	struct hw_perf_event *hwc;
 	struct p4_event_bind *bind;
 	unsigned int i, thread, num;

[patch 3/4] x86,perf: P4 PMU -- add missing bit in CCCR mask

[Cyrill Gorcunov]

[-- Attachment #1: x86-perf-p4-cccr-mask-ht-t1 --]
[-- Type: text/plain, Size: 910 bytes --]

Should be there is a sake of RAW events.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Lin Ming <ming.m.lin@intel.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Ingo Molnar <mingo@elte.hu>
CC: Frederic Weisbecker <fweisbec@gmail.com>
---
 arch/x86/include/asm/perf_event_p4.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6.git/arch/x86/include/asm/perf_event_p4.h
=====================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/perf_event_p4.h
+++ linux-2.6.git/arch/x86/include/asm/perf_event_p4.h
@@ -89,7 +89,8 @@
 	P4_CCCR_ENABLE)
 
 /* HT mask */
-#define P4_CCCR_MASK_HT	(P4_CCCR_MASK | P4_CCCR_THREAD_ANY)
+#define P4_CCCR_MASK_HT				\
+	(P4_CCCR_MASK | P4_CCCR_OVF_PMI_T1 | P4_CCCR_THREAD_ANY)
 
 #define P4_GEN_ESCR_EMASK(class, name, bit)	\
 	class##__##name = ((1 << bit) << P4_ESCR_EVENTMASK_SHIFT)

[tip:perf/core] perf, x86: P4 PMU -- add missing bit in CCCR mask

[tip-bot for Cyrill Gorcunov]

Commit-ID:  ce7f15452cc1dc1eca795542367871a07f37aa79
Gitweb:     http://git.kernel.org/tip/ce7f15452cc1dc1eca795542367871a07f37aa79
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Wed, 19 May 2010 01:19:19 +0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 19 May 2010 09:41:06 +0200

perf, x86: P4 PMU -- add missing bit in CCCR mask

Should be there for the sake of RAW events.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Lin Ming <ming.m.lin@intel.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <20100518212439.354345151@openvz.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/include/asm/perf_event_p4.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/perf_event_p4.h b/arch/x86/include/asm/perf_event_p4.h
index b05400a..64a8ebf 100644
--- a/arch/x86/include/asm/perf_event_p4.h
+++ b/arch/x86/include/asm/perf_event_p4.h
@@ -89,7 +89,8 @@
 	P4_CCCR_ENABLE)
 
 /* HT mask */
-#define P4_CCCR_MASK_HT	(P4_CCCR_MASK | P4_CCCR_THREAD_ANY)
+#define P4_CCCR_MASK_HT				\
+	(P4_CCCR_MASK | P4_CCCR_OVF_PMI_T1 | P4_CCCR_THREAD_ANY)
 
 #define P4_GEN_ESCR_EMASK(class, name, bit)	\
 	class##__##name = ((1 << bit) << P4_ESCR_EVENTMASK_SHIFT)

[patch 4/4] x86,perf: P4 PMU -- prepare header for user-space inclusion

[Cyrill Gorcunov]

[-- Attachment #1: x86-perf-p4-export --]
[-- Type: text/plain, Size: 3742 bytes --]

The header need to be exported to be able to use P4 PMU
in user space applications (for RAW events mostly).

So before make a real export it should sit under linux
sources just to estimate the convenience of its structure.

Note that cache events are not exported at the moment since
they should be redesigned a bit (we need to export and support
the whole metrics set for those who need them for some wild
experiments).

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
CC: Lin Ming <ming.m.lin@intel.com>
CC: Stephane Eranian <eranian@google.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Ingo Molnar <mingo@elte.hu>
CC: Frederic Weisbecker <fweisbec@gmail.com>
---
 arch/x86/include/asm/perf_event_p4.h |   38 ++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/perf_event_p4.h
=====================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/perf_event_p4.h
+++ linux-2.6.git/arch/x86/include/asm/perf_event_p4.h
@@ -5,9 +5,6 @@
 #ifndef PERF_EVENT_P4_H
 #define PERF_EVENT_P4_H
 
-#include <linux/cpu.h>
-#include <linux/bitops.h>
-
 /*
  * NetBurst has perfomance MSRs shared between
  * threads if HT is turned on, ie for both logical
@@ -19,7 +16,6 @@
 #define ARCH_P4_RESERVED_ESCR	(2) /* IQ_ESCR(0,1) not always present */
 #define ARCH_P4_MAX_ESCR	(ARCH_P4_TOTAL_ESCR - ARCH_P4_RESERVED_ESCR)
 #define ARCH_P4_MAX_CCCR	(18)
-#define ARCH_P4_MAX_COUNTER	(ARCH_P4_MAX_CCCR / 2)
 
 #define P4_ESCR_EVENT_MASK	0x7e000000U
 #define P4_ESCR_EVENT_SHIFT	25
@@ -71,10 +67,6 @@
 #define P4_CCCR_THRESHOLD(v)		((v) << P4_CCCR_THRESHOLD_SHIFT)
 #define P4_CCCR_ESEL(v)			((v) << P4_CCCR_ESCR_SELECT_SHIFT)
 
-/* Custom bits in reerved CCCR area */
-#define P4_CCCR_CACHE_OPS_MASK		0x0000003fU
-
-
 /* Non HT mask */
 #define P4_CCCR_MASK				\
 	(P4_CCCR_OVF			|	\
@@ -96,6 +88,17 @@
 	class##__##name = ((1 << bit) << P4_ESCR_EVENTMASK_SHIFT)
 #define P4_ESCR_EMASK_BIT(class, name)		class##__##name
 
+#ifdef __KERNEL__
+
+#include <linux/cpu.h>
+#include <linux/bitops.h>
+
+/*
+ * Cache events are special and we use low 6 bits in
+ * CCCR reserved area for them
+ */
+#define P4_CCCR_CACHE_OPS_MASK		0x0000003fU
+
 /*
  * config field is 64bit width and consists of
  * HT << 63 | ESCR << 32 | CCCR
@@ -214,6 +217,14 @@ static inline u32 p4_default_escr_conf(i
 	return escr;
 }
 
+#endif /* __KERNEL__ */
+
+/*
+ * This are the events which should be used in "Event Select"
+ * field of ESCR register, they are like unique keys which allow
+ * the kernel to determinate which CCCR and COUNTER should be
+ * used to track an event
+ */
 enum P4_EVENTS {
 	P4_EVENT_TC_DELIVER_MODE,
 	P4_EVENT_BPU_FETCH_REQUEST,
@@ -263,6 +274,8 @@ enum P4_EVENTS {
 	P4_EVENT_INSTR_COMPLETED,
 };
 
+#ifdef __KERNEL__
+
 #define P4_OPCODE(event)		event##_OPCODE
 #define P4_OPCODE_ESEL(opcode)		((opcode & 0x00ff) >> 0)
 #define P4_OPCODE_EVNT(opcode)		((opcode & 0xff00) >> 8)
@@ -557,11 +570,13 @@ enum P4_EVENT_OPCODES {
 	 */
 };
 
+#endif /* __KERNEL__ */
+
 /*
  * a caller should use P4_ESCR_EMASK_NAME helper to
  * pick the EventMask needed, for example
  *
- *	P4_ESCR_EMASK_NAME(P4_EVENT_TC_DELIVER_MODE, DD)
+ *	P4_ESCR_EMASK_BIT(P4_EVENT_TC_DELIVER_MODE, DD)
  */
 enum P4_ESCR_EMASKS {
 	P4_GEN_ESCR_EMASK(P4_EVENT_TC_DELIVER_MODE, DD, 0),
@@ -753,6 +768,8 @@ enum P4_ESCR_EMASKS {
 	P4_GEN_ESCR_EMASK(P4_EVENT_INSTR_COMPLETED, BOGUS, 1),
 };
 
+#ifdef __KERNEL__
+
 /* P4 PEBS: stale for a while */
 #define P4_PEBS_METRIC_MASK	0x00001fffU
 #define P4_PEBS_UOB_TAG		0x01000000U
@@ -792,4 +809,7 @@ enum P4_CACHE_EVENTS {
 	P4_CACHE__MAX
 };
 
+#endif /* __KERNEL__ */
+
 #endif /* PERF_EVENT_P4_H */
+

Re: [patch 4/4] x86,perf: P4 PMU -- prepare header for user-space inclusion

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@openvz.org> wrote:

> The header need to be exported to be able to use P4 PMU 
> in user space applications (for RAW events mostly).

Would be much better to create a pure, separate header 
file with hardware event format details, without all these 
ugly #ifdefs mixed into a single file.

Thanks,

	Ingo

Re: [patch 4/4] x86,perf: P4 PMU -- prepare header for user-space inclusion

[Cyrill Gorcunov]

On Wednesday, May 19, 2010, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>
>> The header need to be exported to be able to use P4 PMU
>> in user space applications (for RAW events mostly).
>
> Would be much better to create a pure, separate header
> file with hardware event format details, without all these
> ugly #ifdefs mixed into a single file.
>
> Thanks,
>
>         Ingo

seems reasonable, yes! Drop this patch for a while. Thanks!