login
Search
This patch set unifies performance counter bit masks for x86. All mask are almost the same for all x86 models and thus can use the same macro definitions in arch/x86/include/asm/perf_event.h. It removes duplicate code. There is also a patch that reverts some changes of the big perf_counter -> perf_event rename. -Robert --
The function is the same as intel_pmu_raw_event(). This patch removes
the duplicate code.
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
arch/x86/kernel/cpu/perf_event.c | 2 +-
arch/x86/kernel/cpu/perf_event_intel.c | 2 ++
arch/x86/kernel/cpu/perf_event_p6.c | 20 +-------------------
3 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index f2fc2d8..81218d0 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1320,11 +1320,11 @@ undo:
}
#include "perf_event_amd.c"
-#include "perf_event_p6.c"
#include "perf_event_p4.c"
#include "perf_event_intel_lbr.c"
#include "perf_event_intel_ds.c"
#include "perf_event_intel.c"
+#include "perf_event_p6.c"
static int __cpuinit
x86_pmu_notifier(struct notifier_block *self, unsigned long action, void *hcpu)
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index 0d68531..6c5d83f 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -842,6 +842,8 @@ static __initconst struct x86_pmu intel_pmu = {
.cpu_dying = intel_pmu_cpu_dying,
};
+static __init int p6_pmu_init(void);
+
static void intel_clovertown_quirks(void)
{
/*
diff --git a/arch/x86/kernel/cpu/perf_event_p6.c b/arch/x86/kernel/cpu/perf_event_p6.c
index b26fbc7..8cdd054 100644
--- a/arch/x86/kernel/cpu/perf_event_p6.c
+++ b/arch/x86/kernel/cpu/perf_event_p6.c
@@ -27,24 +27,6 @@ static u64 p6_pmu_event_map(int hw_event)
*/
#define P6_NOP_EVENT 0x0000002EULL
-static u64 p6_pmu_raw_event(u64 hw_event)
-{
-#define P6_EVNTSEL_EVENT_MASK 0x000000FFULL
-#define P6_EVNTSEL_UNIT_MASK 0x0000FF00ULL
-#define P6_EVNTSEL_EDGE_MASK 0x00040000ULL
-#define P6_EVNTSEL_INV_MASK 0x00800000ULL
-#define P6_EVNTSEL_REG_MASK 0xFF000000ULL
-
-#define P6_EVNTSEL_MASK \
- (P6_EVNTSEL_EVENT_MASK | \
- ...ARCH_PERFMON_EVENTSEL bit masks are offen used in the kernel. This patch adds macros for the bit masks and removes local defines. Signed-off-by: Robert Richter <robert.richter@amd.com> --- arch/x86/include/asm/perf_event.h | 58 ++++++++++++++------------------ arch/x86/kernel/cpu/perf_event.c | 14 ++++++-- arch/x86/kernel/cpu/perf_event_amd.c | 15 +-------- arch/x86/kernel/cpu/perf_event_intel.c | 15 +-------- 4 files changed, 38 insertions(+), 64 deletions(-) diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h index 987bf67..f6d43db 100644 --- a/arch/x86/include/asm/perf_event.h +++ b/arch/x86/include/asm/perf_event.h @@ -18,39 +18,31 @@ #define MSR_ARCH_PERFMON_EVENTSEL0 0x186 #define MSR_ARCH_PERFMON_EVENTSEL1 0x187 -#define ARCH_PERFMON_EVENTSEL_ENABLE (1 << 22) -#define ARCH_PERFMON_EVENTSEL_ANY (1 << 21) -#define ARCH_PERFMON_EVENTSEL_INT (1 << 20) -#define ARCH_PERFMON_EVENTSEL_OS (1 << 17) -#define ARCH_PERFMON_EVENTSEL_USR (1 << 16) - -/* - * Includes eventsel and unit mask as well: - */ - - -#define INTEL_ARCH_EVTSEL_MASK 0x000000FFULL -#define INTEL_ARCH_UNIT_MASK 0x0000FF00ULL -#define INTEL_ARCH_EDGE_MASK 0x00040000ULL -#define INTEL_ARCH_INV_MASK 0x00800000ULL -#define INTEL_ARCH_CNT_MASK 0xFF000000ULL -#define INTEL_ARCH_EVENT_MASK (INTEL_ARCH_UNIT_MASK|INTEL_ARCH_EVTSEL_MASK) - -/* - * filter mask to validate fixed counter events. - * the following filters disqualify for fixed counters: - * - inv - * - edge - * - cnt-mask - * The other filters are supported by fixed counters. - * The any-thread option is supported starting with v3. - */ -#define INTEL_ARCH_FIXED_MASK \ - (INTEL_ARCH_CNT_MASK| \ - INTEL_ARCH_INV_MASK| \ - INTEL_ARCH_EDGE_MASK|\ - INTEL_ARCH_UNIT_MASK|\ - INTEL_ARCH_EVENT_MASK) +#define ARCH_PERFMON_EVENTSEL_EVENT 0x000000FFULL +#define ARCH_PERFMON_EVENTSEL_UMASK 0x0000FF00ULL +#define ...
Could you fold this with your 2/3 and create x86_pmu_raw_event() which lives in arch/x86/kernel/cpu/perf_event.c, that's more consistent wrt the X86_RAW_EVENT_MASK name and that way you don't need to re-order the #include ""s either. --
Ah, yes. Will do this and resend the patches. -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com --
The patch below replaces patches 2 and 3. -Robert --- From 2d77650a4dc5ded763dc3c120381bdbe5a0be911 Mon Sep 17 00:00:00 2001 From: Robert Richter <robert.richter@amd.com> Date: Wed, 17 Mar 2010 12:32:37 +0100 Subject: [PATCH] perf/core, x86: implement ARCH_PERFMON_EVENTSEL bit masks ARCH_PERFMON_EVENTSEL bit masks are often used in the kernel. This patch adds macros for the bit masks and removes local defines. The function intel_pmu_raw_event() becomes x86_pmu_raw_event() which is generic for x86 models and same also for p6. Duplicate code is removed. Signed-off-by: Robert Richter <robert.richter@amd.com> --- arch/x86/include/asm/perf_event.h | 58 ++++++++++++++------------------ arch/x86/kernel/cpu/perf_event.c | 19 +++++++++-- arch/x86/kernel/cpu/perf_event_amd.c | 15 +-------- arch/x86/kernel/cpu/perf_event_intel.c | 22 +----------- arch/x86/kernel/cpu/perf_event_p6.c | 20 +---------- 5 files changed, 45 insertions(+), 89 deletions(-) diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h index 987bf67..f6d43db 100644 --- a/arch/x86/include/asm/perf_event.h +++ b/arch/x86/include/asm/perf_event.h @@ -18,39 +18,31 @@ #define MSR_ARCH_PERFMON_EVENTSEL0 0x186 #define MSR_ARCH_PERFMON_EVENTSEL1 0x187 -#define ARCH_PERFMON_EVENTSEL_ENABLE (1 << 22) -#define ARCH_PERFMON_EVENTSEL_ANY (1 << 21) -#define ARCH_PERFMON_EVENTSEL_INT (1 << 20) -#define ARCH_PERFMON_EVENTSEL_OS (1 << 17) -#define ARCH_PERFMON_EVENTSEL_USR (1 << 16) - -/* - * Includes eventsel and unit mask as well: - */ - - -#define INTEL_ARCH_EVTSEL_MASK 0x000000FFULL -#define INTEL_ARCH_UNIT_MASK 0x0000FF00ULL -#define INTEL_ARCH_EDGE_MASK 0x00040000ULL -#define INTEL_ARCH_INV_MASK 0x00800000ULL -#define INTEL_ARCH_CNT_MASK 0xFF000000ULL -#define INTEL_ARCH_EVENT_MASK (INTEL_ARCH_UNIT_MASK|INTEL_ARCH_EVTSEL_MASK) - -/* - * filter mask to validate fixed counter events. - * the following ...
Commit-ID: a098f4484bc7dae23f5b62360954007b99b64600 Gitweb: http://git.kernel.org/tip/a098f4484bc7dae23f5b62360954007b99b64600 Author: Robert Richter <robert.richter@amd.com> AuthorDate: Tue, 30 Mar 2010 11:28:21 +0200 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Fri, 2 Apr 2010 19:52:03 +0200 perf, x86: implement ARCH_PERFMON_EVENTSEL bit masks ARCH_PERFMON_EVENTSEL bit masks are often used in the kernel. This patch adds macros for the bit masks and removes local defines. The function intel_pmu_raw_event() becomes x86_pmu_raw_event() which is generic for x86 models and same also for p6. Duplicate code is removed. Signed-off-by: Robert Richter <robert.richter@amd.com> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> LKML-Reference: <20100330092821.GH11907@erda.amd.com> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- arch/x86/include/asm/perf_event.h | 58 ++++++++++++++------------------ arch/x86/kernel/cpu/perf_event.c | 19 +++++++++-- arch/x86/kernel/cpu/perf_event_amd.c | 15 +-------- arch/x86/kernel/cpu/perf_event_intel.c | 22 +----------- arch/x86/kernel/cpu/perf_event_p6.c | 20 +---------- 5 files changed, 45 insertions(+), 89 deletions(-) diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h index 987bf67..f6d43db 100644 --- a/arch/x86/include/asm/perf_event.h +++ b/arch/x86/include/asm/perf_event.h @@ -18,39 +18,31 @@ #define MSR_ARCH_PERFMON_EVENTSEL0 0x186 #define MSR_ARCH_PERFMON_EVENTSEL1 0x187 -#define ARCH_PERFMON_EVENTSEL_ENABLE (1 << 22) -#define ARCH_PERFMON_EVENTSEL_ANY (1 << 21) -#define ARCH_PERFMON_EVENTSEL_INT (1 << 20) -#define ARCH_PERFMON_EVENTSEL_OS (1 << 17) -#define ARCH_PERFMON_EVENTSEL_USR (1 << 16) - -/* - * Includes eventsel and unit mask as well: - */ - - -#define INTEL_ARCH_EVTSEL_MASK 0x000000FFULL -#define INTEL_ARCH_UNIT_MASK 0x0000FF00ULL -#define INTEL_ARCH_EDGE_MASK 0x00040000ULL -#define ...
The big rename
cdd6c48 perf: Do the big rename: Performance Counters -> Performance Events
accidentally renamed some members of stucts that were named after
registers in the spec. To avoid confusion this patch reverts some
changes. The related specs are MSR descriptions in AMD's BKDGs and the
ARCHITECTURAL PERFORMANCE MONITORING section in the Intel 64 and IA-32
Architectures Software Developer's Manuals.
This patch does:
$ sed -i -e 's:num_events:num_counters:g' \
arch/x86/include/asm/perf_event.h \
arch/x86/kernel/cpu/perf_event_amd.c \
arch/x86/kernel/cpu/perf_event.c \
arch/x86/kernel/cpu/perf_event_intel.c \
arch/x86/kernel/cpu/perf_event_p6.c \
arch/x86/kernel/cpu/perf_event_p4.c \
arch/x86/oprofile/op_model_ppro.c
$ sed -i -e 's:event_bits:cntval_bits:g' -e 's:event_mask:cntval_mask:g' \
arch/x86/kernel/cpu/perf_event_amd.c \
arch/x86/kernel/cpu/perf_event.c \
arch/x86/kernel/cpu/perf_event_intel.c \
arch/x86/kernel/cpu/perf_event_p6.c \
arch/x86/kernel/cpu/perf_event_p4.c
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
arch/x86/include/asm/perf_event.h | 4 +-
arch/x86/kernel/cpu/perf_event.c | 74 ++++++++++++++++----------------
arch/x86/kernel/cpu/perf_event_amd.c | 12 +++---
arch/x86/kernel/cpu/perf_event_intel.c | 14 +++---
arch/x86/kernel/cpu/perf_event_p4.c | 14 +++---
arch/x86/kernel/cpu/perf_event_p6.c | 6 +-
arch/x86/oprofile/op_model_ppro.c | 4 +-
7 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 124dddd..987bf67 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -67,7 +67,7 @@
union cpuid10_eax {
struct {
unsigned int version_id:8;
- unsigned int num_events:8;
+ unsigned int num_counters:8;
unsigned int bit_width:8;
unsigned int mask_length:8;
} split;
@@ -76,7 +76,7 @@ union cpuid10_eax {
...Commit-ID: 948b1bb89a44561560531394c18da4a99215f772 Gitweb: http://git.kernel.org/tip/948b1bb89a44561560531394c18da4a99215f772 Author: Robert Richter <robert.richter@amd.com> AuthorDate: Mon, 29 Mar 2010 18:36:50 +0200 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Fri, 2 Apr 2010 19:52:02 +0200 perf, x86: Undo some some *_counter* -> *_event* renames The big rename: cdd6c48 perf: Do the big rename: Performance Counters -> Performance Events accidentally renamed some members of stucts that were named after registers in the spec. To avoid confusion this patch reverts some changes. The related specs are MSR descriptions in AMD's BKDGs and the ARCHITECTURAL PERFORMANCE MONITORING section in the Intel 64 and IA-32 Architectures Software Developer's Manuals. This patch does: $ sed -i -e 's:num_events:num_counters:g' \ arch/x86/include/asm/perf_event.h \ arch/x86/kernel/cpu/perf_event_amd.c \ arch/x86/kernel/cpu/perf_event.c \ arch/x86/kernel/cpu/perf_event_intel.c \ arch/x86/kernel/cpu/perf_event_p6.c \ arch/x86/kernel/cpu/perf_event_p4.c \ arch/x86/oprofile/op_model_ppro.c $ sed -i -e 's:event_bits:cntval_bits:g' -e 's:event_mask:cntval_mask:g' \ arch/x86/kernel/cpu/perf_event_amd.c \ arch/x86/kernel/cpu/perf_event.c \ arch/x86/kernel/cpu/perf_event_intel.c \ arch/x86/kernel/cpu/perf_event_p6.c \ arch/x86/kernel/cpu/perf_event_p4.c Signed-off-by: Robert Richter <robert.richter@amd.com> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> LKML-Reference: <1269880612-25800-2-git-send-email-robert.richter@amd.com> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- arch/x86/include/asm/perf_event.h | 4 +- arch/x86/kernel/cpu/perf_event.c | 74 ++++++++++++++++---------------- arch/x86/kernel/cpu/perf_event_amd.c | 12 +++--- arch/x86/kernel/cpu/perf_event_intel.c | 16 +++--- arch/x86/kernel/cpu/perf_event_p4.c | 14 +++--- arch/x86/kernel/cpu/perf_event_p6.c | 6 +- ...
But there are still fields which are unique to each vendor:
- GUEST vs. HOST on AMD
- ANY_THREAD on Intel.
For instance, I noticed that in
arch/x86/kernel/cpu/perf_event.c:__hw_perf_event_init():
if (attr->type == PERF_TYPE_RAW) {
hwc->config |= x86_pmu.raw_event(attr->config);
if ((hwc->config & ARCH_PERFMON_EVENTSEL_ANY) &&
perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
return -EACCES;
return 0;
}
Assumes ANY also exists on AMD processors. That is not the case.
This check needs to be moved into an Intel specific function.
--
Generally, ARCH_PERFMON_EVENTSEL_* refers to: Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3B: System Programming Guide, Part 2 30.2 ARCHITECTURAL PERFORMANCE MONITORING Appendix A: Performance-Monitoring Events Appendix B: Model-Specific Registers (MSRs) and AMD64_EVENTSEL_* to: AMD64 Architecture Programmer's Manual Volume 2: System Programming 13.3.1 Performance Counters X86_* is generic. If a feature is available from both vendors, the names shouln't be changed. Instead the first introduced mask should be used (at least this is my suggestion). So, there are some ARCH_PERFMON_EVENTSEL_* masks that are Intel only, which is true for ARCH_PERFMON_EVENTSEL_ANY. And indead, the code should be checked for this. ARCH_PERFMON_EVENTSEL_ANY is always cleared on AMD cpus, so this code is ok. Actually the bit is cleared for *all* cpus in x86_pmu_raw_event(), the code was and is broken for this. -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com --
Until AMD uses that bit too and you won't notice this test. This is a security --
So how about something like this on top of Robert's patches?
---
arch/x86/kernel/cpu/perf_event.c | 16 ++++++----------
arch/x86/kernel/cpu/perf_event_amd.c | 5 +++--
arch/x86/kernel/cpu/perf_event_intel.c | 15 ++++++++++++++-
arch/x86/kernel/cpu/perf_event_p4.c | 5 +++--
4 files changed, 26 insertions(+), 15 deletions(-)
Index: linux-2.6/arch/x86/kernel/cpu/perf_event.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/perf_event.c
+++ linux-2.6/arch/x86/kernel/cpu/perf_event.c
@@ -201,7 +201,7 @@ struct x86_pmu {
unsigned eventsel;
unsigned perfctr;
u64 (*event_map)(int);
- u64 (*raw_event)(u64);
+ int (*raw_event)(struct perf_event *event);
int max_events;
int num_counters;
int num_counters_fixed;
@@ -445,9 +445,10 @@ static int x86_hw_config(struct perf_eve
return 0;
}
-static u64 x86_pmu_raw_event(u64 hw_event)
+static int x86_pmu_raw_event(struct perf_event *event)
{
- return hw_event & X86_RAW_EVENT_MASK;
+ event->hw.config |= event->attr.config & X86_RAW_EVENT_MASK;
+ return 0;
}
/*
@@ -511,13 +512,8 @@ static int __hw_perf_event_init(struct p
/*
* Raw hw_event type provide the config in the hw_event structure
*/
- if (attr->type == PERF_TYPE_RAW) {
- hwc->config |= x86_pmu.raw_event(attr->config);
- if ((hwc->config & ARCH_PERFMON_EVENTSEL_ANY) &&
- perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
- return -EACCES;
- return 0;
- }
+ if (attr->type == PERF_TYPE_RAW)
+ return x86_pmu.raw_event(event);
if (attr->type == PERF_TYPE_HW_CACHE)
return set_ext_hw_attr(hwc, attr);
Index: linux-2.6/arch/x86/kernel/cpu/perf_event_amd.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/perf_event_amd.c
+++ linux-2.6/arch/x86/kernel/cpu/perf_event_amd.c
@@ -111,9 +111,10 @@ static u64 amd_pmu_event_map(int hw_even
return amd_perfmon_event_map[hw_event];
}
...[...] Looks good for me! Thanks Peter! -- Cyrill --
Looks better. Thanks. Acked-by: Stephane Eranian <eranian@google.com> --
We could go on with this and move this into *_hw_config(), but so far this change is fine to me. This would then remove .raw_event() at all. Maybe this? if (!(event->attr.config & ARCH_PERFMON_EVENTSEL_ANY)) return 0; if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN)) return -EACCES; event->hw.config |= ARCH_PERFMON_EVENTSEL_ANY; return 0; -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com --
Right, I did have a brief look at that, you mentioned this before to
You're right..
---
arch/x86/kernel/cpu/perf_event.c | 35 +++++++++-------------------
arch/x86/kernel/cpu/perf_event_amd.c | 17 ++++++++++----
arch/x86/kernel/cpu/perf_event_intel.c | 30 +++++++++++++++++++++---
arch/x86/kernel/cpu/perf_event_p4.c | 40 ++++++++++++++++++---------------
4 files changed, 73 insertions(+), 49 deletions(-)
Index: linux-2.6/arch/x86/kernel/cpu/perf_event.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/perf_event.c
+++ linux-2.6/arch/x86/kernel/cpu/perf_event.c
@@ -196,12 +196,11 @@ struct x86_pmu {
void (*enable_all)(int added);
void (*enable)(struct perf_event *);
void (*disable)(struct perf_event *);
- int (*hw_config)(struct perf_event_attr *attr, struct hw_perf_event *hwc);
+ int (*hw_config)(struct perf_event *event);
int (*schedule_events)(struct cpu_hw_events *cpuc, int n, int *assign);
unsigned eventsel;
unsigned perfctr;
u64 (*event_map)(int);
- u64 (*raw_event)(u64);
int max_events;
int num_counters;
int num_counters_fixed;
@@ -426,28 +425,26 @@ set_ext_hw_attr(struct hw_perf_event *hw
return 0;
}
-static int x86_hw_config(struct perf_event_attr *attr, struct hw_perf_event *hwc)
+static int x86_pmu_hw_config(struct perf_event *event)
{
/*
* Generate PMC IRQs:
* (keep 'enabled' bit clear for now)
*/
- hwc->config = ARCH_PERFMON_EVENTSEL_INT;
+ event->hw.config = ARCH_PERFMON_EVENTSEL_INT;
/*
* Count user and OS events unless requested not to
*/
- if (!attr->exclude_user)
- hwc->config |= ARCH_PERFMON_EVENTSEL_USR;
- if (!attr->exclude_kernel)
- hwc->config |= ARCH_PERFMON_EVENTSEL_OS;
+ if (!event->attr.exclude_user)
+ event->hw.config |= ARCH_PERFMON_EVENTSEL_USR;
+ if (!event->attr.exclude_kernel)
+ event->hw.config |= ARCH_PERFMON_EVENTSEL_OS;
- return 0;
-}
+ if (event->attr.type == ...Yeah, Robert sais me about eliminating raw_event, just remember :) [...] I'll update P4 snippet. -- Cyrill --
I like your patch, thanks Peter. -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com --
On Tue, Mar 30, 2010 at 06:55:13PM +0200, Peter Zijlstra wrote: [...] P4 events thread specific is a bit more messy in compare with architectural events. There are thread specific (TS) and thread independent (TI) events. The exact effect of mixing flags from what we call "ANY" bit is described in two matrix in SDM. So to make code simplier I chose to just bind events to a particular logical cpu, when event migrate to say a second cpu the bits just flipped in accordance on which cpu the event is going to run. Pretty simple. Even more -- if there was some RAW event which have set "ANY" bit -- they all will be just stripped and event get bound to a single cpu. I'll try to find out an easy way to satisfy this "ANY" bit request though it would require some time (perhaps today later or rather tomorrow). -- Cyrill --
Right, so don't worry about actively supporting ANY on regular events, wider than logical cpu counting is a daft thing. What would be nice to detect is if the raw event provided would be a TI (ANY) event, in which case we should apply the extra paranoia. --
Well, there is a side effect would be anyway, so I think it should be fixed via the way like: if a caller wanna get ANY event and this caller has enough rights for that -- go ahead you'll get what you want, kernel is not going to do a dirty work for you :) so I would need only fix two procedures -- event assignment (where permission will be checked as well) and event migration where I will not do any additional work for the caller. At least if I not miss anything it should not be quite difficult and invasive. Will check and send patch... later a bit. At moment we're on a safe side anyway, ie the former patch is fine for me! -- Cyrill --
ok, here is a version ot top of your patches. Compile tested
only (have no p4 under my hands)
-- Cyrill
---
x86, perf: P4 PMU -- check for permission granted on ANY event requested
In case if a caller (user) asked us to count events with
some weird mask we should check if this priviledge has been
granted since this could be a mix of mask bits we not like
which but allow if caller insist.
By ANY event term the combination of USR/OS bits in ESCR
register is assumed.
CC: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
arch/x86/include/asm/perf_event_p4.h | 19 +++++++++++++++++++
arch/x86/kernel/cpu/perf_event_p4.c | 24 +++++++++++++++++++++---
2 files changed, 40 insertions(+), 3 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
@@ -33,6 +33,9 @@
#define P4_ESCR_T1_OS 0x00000002U
#define P4_ESCR_T1_USR 0x00000001U
+#define P4_ESCR_T0_ANY (P4_ESCR_T0_OS | P4_ESCR_T0_USR)
+#define P4_ESCR_T1_ANY (P4_ESCR_T1_OS | P4_ESCR_T1_USR)
+
#define P4_ESCR_EVENT(v) ((v) << P4_ESCR_EVENT_SHIFT)
#define P4_ESCR_EMASK(v) ((v) << P4_ESCR_EVENTMASK_SHIFT)
#define P4_ESCR_TAG(v) ((v) << P4_ESCR_TAG_SHIFT)
@@ -134,6 +137,22 @@
#define P4_CONFIG_HT_SHIFT 63
#define P4_CONFIG_HT (1ULL << P4_CONFIG_HT_SHIFT)
+/*
+ * typically we set USR or/and OS bits for one of the
+ * threads only at once, any other option is treated
+ * as "odd"
+ */
+static inline bool p4_is_odd_cpl(u32 escr)
+{
+ unsigned int t0 = (escr & P4_ESCR_T0_ANY) << 0;
+ unsigned int t1 = (escr & P4_ESCR_T1_ANY) << 2;
+
+ if ((t0 ^ t1) != t0)
+ return true;
+
+ return false;
+}
+
static inline bool p4_is_event_cascaded(u64 config)
{
u32 cccr = p4_config_unpack_cccr(config);
Index: ...Updated
-- Cyrill
---
x86, perf: P4 PMU -- check for permission granted on ANY event v2
In case if a caller (user) asked us to count events with
some weird mask we should check if this priviledge has been
granted since this could be a mix of bitmasks we not like
which but allow if caller insist.
By ANY event term the combination of USR/OS bits in ESCR
register is assumed.
CC: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
arch/x86/include/asm/perf_event_p4.h | 17 +++++++++++++++++
arch/x86/kernel/cpu/perf_event_p4.c | 24 +++++++++++++++++++++---
2 files changed, 38 insertions(+), 3 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
@@ -33,6 +33,9 @@
#define P4_ESCR_T1_OS 0x00000002U
#define P4_ESCR_T1_USR 0x00000001U
+#define P4_ESCR_T0_ANY (P4_ESCR_T0_OS | P4_ESCR_T0_USR)
+#define P4_ESCR_T1_ANY (P4_ESCR_T1_OS | P4_ESCR_T1_USR)
+
#define P4_ESCR_EVENT(v) ((v) << P4_ESCR_EVENT_SHIFT)
#define P4_ESCR_EMASK(v) ((v) << P4_ESCR_EVENTMASK_SHIFT)
#define P4_ESCR_TAG(v) ((v) << P4_ESCR_TAG_SHIFT)
@@ -134,6 +137,20 @@
#define P4_CONFIG_HT_SHIFT 63
#define P4_CONFIG_HT (1ULL << P4_CONFIG_HT_SHIFT)
+/*
+ * typically we set USR or/and OS bits for one of the
+ * threads only at once, any other option is treated
+ * as "any"
+ */
+static inline bool p4_is_any_cpl(u32 escr)
+{
+ if ((escr & P4_ESCR_T0_ANY) &&
+ (escr & P4_ESCR_T1_ANY))
+ return true;
+
+ return false;
+}
+
static inline bool p4_is_event_cascaded(u64 config)
{
u32 cccr = p4_config_unpack_cccr(config);
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
+++ ...I'll test this patch. Does it need to be applied on top of Robert's patch? --
I tested this patch on top of Peter's and Robert's patch.
Works well on P4.
Only some build error, fixed by below patch.
In file included from arch/x86/kernel/cpu/perf_event.c:1326:
arch/x86/kernel/cpu/perf_event_p6.c:94: error: ‘x86_hw_config’ undeclared here (not in a function)
arch/x86/kernel/cpu/perf_event_p6.c:99: error: unknown field ‘raw_event’ specified in initializer
arch/x86/kernel/cpu/perf_event_p6.c:99: error: ‘x86_pmu_raw_event’ undeclared here (not in a function)
make[3]: *** [arch/x86/kernel/cpu/perf_event.o] Error 1
diff --git a/arch/x86/kernel/cpu/perf_event_p6.c b/arch/x86/kernel/cpu/perf_event_p6.c
index 626abc0..4ec9680 100644
--- a/arch/x86/kernel/cpu/perf_event_p6.c
+++ b/arch/x86/kernel/cpu/perf_event_p6.c
@@ -91,12 +91,11 @@ static __initconst struct x86_pmu p6_pmu = {
.enable_all = p6_pmu_enable_all,
.enable = p6_pmu_enable_event,
.disable = p6_pmu_disable_event,
- .hw_config = x86_hw_config,
+ .hw_config = x86_pmu_hw_config,
.schedule_events = x86_schedule_events,
.eventsel = MSR_P6_EVNTSEL0,
.perfctr = MSR_P6_PERFCTR0,
.event_map = p6_pmu_event_map,
- .raw_event = x86_pmu_raw_event,
.max_events = ARRAY_SIZE(p6_perfmon_event_map),
.apic = 1,
.max_period = (1ULL << 31) - 1,
--
egads, that hunk seems to have tunneled into another patch in my queue,.. /me goes fix. Thanks Lin. --
Commit-ID: b4cdc5c264b35c67007800dec3928e9547a9d70b Gitweb: http://git.kernel.org/tip/b4cdc5c264b35c67007800dec3928e9547a9d70b Author: Peter Zijlstra <a.p.zijlstra@chello.nl> AuthorDate: Tue, 30 Mar 2010 17:00:06 +0200 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Fri, 2 Apr 2010 19:52:04 +0200 perf, x86: Fix up the ANY flag stuff Stephane noticed that the ANY flag was in generic arch code, and Cyrill reported that it broke the P4 code. Solve this by merging x86_pmu::raw_event into x86_pmu::hw_config and provide intel_pmu and amd_pmu specific versions of this callback. The intel_pmu one deals with the ANY flag, the amd_pmu adds the few extra event bits AMD64 has. Reported-by: Stephane Eranian <eranian@google.com> Reported-by: Cyrill Gorcunov <gorcunov@gmail.com> Acked-by: Robert Richter <robert.richter@amd.com> Acked-by: Cyrill Gorcunov <gorcunov@gmail.com> Acked-by: Stephane Eranian <eranian@google.com> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> LKML-Reference: <1269968113.5258.442.camel@laptop> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- arch/x86/kernel/cpu/perf_event.c | 35 +++++++++------------------ arch/x86/kernel/cpu/perf_event_amd.c | 17 ++++++++++--- arch/x86/kernel/cpu/perf_event_intel.c | 30 ++++++++++++++++++++--- arch/x86/kernel/cpu/perf_event_p4.c | 40 +++++++++++++++++-------------- arch/x86/kernel/cpu/perf_event_p6.c | 3 +- 5 files changed, 74 insertions(+), 51 deletions(-) diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c index 1dd42c1..65e9c5e 100644 --- a/arch/x86/kernel/cpu/perf_event.c +++ b/arch/x86/kernel/cpu/perf_event.c @@ -196,12 +196,11 @@ struct x86_pmu { void (*enable_all)(int added); void (*enable)(struct perf_event *); void (*disable)(struct perf_event *); - int (*hw_config)(struct perf_event_attr *attr, struct hw_perf_event *hwc); + int (*hw_config)(struct perf_event *event); int (*schedule_events)(struct cpu_hw_events ...
