Messages in this thread Patch in this message | | | Subject | Re: [PATCH 0/3] perf/core, x86: unify perfctr bitmasks | From | Peter Zijlstra <> | Date | Tue, 30 Mar 2010 17:00:55 +0200 |
| |
On Tue, 2010-03-30 at 15:53 +0200, Stephane Eranian wrote: > > 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 > > Until AMD uses that bit too and you won't notice this test. This is a security > check specific to Intel and it should be in an Intel-specific function. > > > for *all* cpus in x86_pmu_raw_event(), the code was and is broken for > > this. > > > Yes, needs to be authorized for any perfmon v3 and later revisions.
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]; } -static u64 amd_pmu_raw_event(u64 hw_event) +static int amd_pmu_raw_event(struct perf_event *event) { - return hw_event & AMD64_RAW_EVENT_MASK; + event->hw.config |= event->attr.config & AMD64_RAW_EVENT_MASK; + return 0; } /* Index: linux-2.6/arch/x86/kernel/cpu/perf_event_intel.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/cpu/perf_event_intel.c +++ linux-2.6/arch/x86/kernel/cpu/perf_event_intel.c @@ -758,6 +758,19 @@ intel_get_event_constraints(struct cpu_h return x86_get_event_constraints(cpuc, event); } +static int intel_pmu_raw_event(struct perf_event *event) +{ + int ret = x86_pmu_raw_event(event); + if (ret) + return ret; + + if ((event->hw.config & ARCH_PERFMON_EVENTSEL_ANY) && + perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN)) + return -EACCES; + + return 0; +} + static __initconst struct x86_pmu core_pmu = { .name = "core", .handle_irq = x86_pmu_handle_irq, @@ -809,7 +822,7 @@ static __initconst struct x86_pmu intel_ .eventsel = MSR_ARCH_PERFMON_EVENTSEL0, .perfctr = MSR_ARCH_PERFMON_PERFCTR0, .event_map = intel_pmu_event_map, - .raw_event = x86_pmu_raw_event, + .raw_event = intel_pmu_raw_event, .max_events = ARRAY_SIZE(intel_perfmon_event_map), .apic = 1, /* Index: linux-2.6/arch/x86/kernel/cpu/perf_event_p4.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/cpu/perf_event_p4.c +++ linux-2.6/arch/x86/kernel/cpu/perf_event_p4.c @@ -425,11 +425,12 @@ static u64 p4_pmu_event_map(int hw_event * on HT machine but allow HT-compatible specifics to be * passed on) */ -static u64 p4_pmu_raw_event(u64 hw_event) +static int p4_pmu_raw_event(struct perf_event *event) { - return hw_event & + event->hw.config |= event->attr.config & (p4_config_pack_escr(P4_ESCR_MASK_HT) | p4_config_pack_cccr(P4_CCCR_MASK_HT)); + return 0; } static int p4_hw_config(struct perf_event_attr *attr, struct hw_perf_event *hwc)
| |