| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1269968113.5258.442.camel@laptop>
Date: Tue, 30 Mar 2010 18:55:13 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Robert Richter <robert.richter@....com>
Cc: Stephane Eranian <eranian@...gle.com>, Ingo Molnar <mingo@...e.hu>,
LKML <linux-kernel@...r.kernel.org>,
Cyrill Gorcunov <gorcunov@...il.com>
Subject: Re: [PATCH 0/3] perf/core, x86: unify perfctr bitmasks
On Tue, 2010-03-30 at 17:59 +0200, Robert Richter wrote:
> > + if (attr->type == PERF_TYPE_RAW)
> > + return x86_pmu.raw_event(event);
>
> 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.
>
Right, I did have a brief look at that, you mentioned this before to
Cyrill IIRC, but the issue is that... ~brainwave~ how about the below..
>
> > 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;
>
> 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;
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 == PERF_TYPE_RAW)
+ event->hw.config |= event->attr.config & X86_RAW_EVENT_MASK;
-static u64 x86_pmu_raw_event(u64 hw_event)
-{
- return hw_event & X86_RAW_EVENT_MASK;
+ return 0;
}
/*
@@ -489,7 +486,7 @@ static int __hw_perf_event_init(struct p
hwc->last_tag = ~0ULL;
/* Processor specifics */
- err = x86_pmu.hw_config(attr, hwc);
+ err = x86_pmu.hw_config(event);
if (err)
return err;
@@ -508,16 +505,8 @@ static int __hw_perf_event_init(struct p
return -EOPNOTSUPP;
}
- /*
- * 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;
+ if (attr->type == PERF_TYPE_RAW)
return 0;
- }
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,19 @@ 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_hw_config(struct perf_event *event)
{
- return hw_event & AMD64_RAW_EVENT_MASK;
+ int ret = x86_pmu_hw_config(event);
+
+ if (ret)
+ return ret;
+
+ if (event->attr.type != PERF_TYPE_RAW)
+ return 0;
+
+ event->hw.config |= event->attr.config & AMD64_RAW_EVENT_MASK;
+
+ return 0;
}
/*
@@ -365,12 +375,11 @@ static __initconst struct x86_pmu amd_pm
.enable_all = x86_pmu_enable_all,
.enable = x86_pmu_enable_event,
.disable = x86_pmu_disable_event,
- .hw_config = x86_hw_config,
+ .hw_config = amd_pmu_hw_config,
.schedule_events = x86_schedule_events,
.eventsel = MSR_K7_EVNTSEL0,
.perfctr = MSR_K7_PERFCTR0,
.event_map = amd_pmu_event_map,
- .raw_event = amd_pmu_raw_event,
.max_events = ARRAY_SIZE(amd_perfmon_event_map),
.num_counters = 4,
.cntval_bits = 48,
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,30 @@ intel_get_event_constraints(struct cpu_h
return x86_get_event_constraints(cpuc, event);
}
+static int intel_pmu_hw_config(struct perf_event *event)
+{
+ int ret = x86_pmu_hw_config(event);
+
+ if (ret)
+ return ret;
+
+ if (event->attr.type != PERF_TYPE_RAW)
+ return 0;
+
+ if (!(event->attr.config & ARCH_PERFMON_EVENTSEL_ANY))
+ return 0;
+
+ if (x86_pmu.version < 3)
+ return -EINVAL;
+
+ if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ event->hw.config |= ARCH_PERFMON_EVENTSEL_ANY;
+
+ return 0;
+}
+
static __initconst struct x86_pmu core_pmu = {
.name = "core",
.handle_irq = x86_pmu_handle_irq,
@@ -765,12 +789,11 @@ static __initconst struct x86_pmu core_p
.enable_all = x86_pmu_enable_all,
.enable = x86_pmu_enable_event,
.disable = x86_pmu_disable_event,
- .hw_config = x86_hw_config,
+ .hw_config = x86_pmu_hw_config,
.schedule_events = x86_schedule_events,
.eventsel = MSR_ARCH_PERFMON_EVENTSEL0,
.perfctr = MSR_ARCH_PERFMON_PERFCTR0,
.event_map = intel_pmu_event_map,
- .raw_event = x86_pmu_raw_event,
.max_events = ARRAY_SIZE(intel_perfmon_event_map),
.apic = 1,
/*
@@ -804,12 +827,11 @@ static __initconst struct x86_pmu intel_
.enable_all = intel_pmu_enable_all,
.enable = intel_pmu_enable_event,
.disable = intel_pmu_disable_event,
- .hw_config = x86_hw_config,
+ .hw_config = intel_pmu_hw_config,
.schedule_events = x86_schedule_events,
.eventsel = MSR_ARCH_PERFMON_EVENTSEL0,
.perfctr = MSR_ARCH_PERFMON_PERFCTR0,
.event_map = intel_pmu_event_map,
- .raw_event = x86_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
@@ -419,20 +419,7 @@ static u64 p4_pmu_event_map(int hw_event
return config;
}
-/*
- * We don't control raw events so it's up to the caller
- * to pass sane values (and we don't count the thread number
- * on HT machine but allow HT-compatible specifics to be
- * passed on)
- */
-static u64 p4_pmu_raw_event(u64 hw_event)
-{
- return hw_event &
- (p4_config_pack_escr(P4_ESCR_MASK_HT) |
- p4_config_pack_cccr(P4_CCCR_MASK_HT));
-}
-
-static int p4_hw_config(struct perf_event_attr *attr, struct hw_perf_event *hwc)
+static int p4_hw_config(struct perf_event *event)
{
int cpu = raw_smp_processor_id();
u32 escr, cccr;
@@ -444,11 +431,29 @@ static int p4_hw_config(struct perf_even
*/
cccr = p4_default_cccr_conf(cpu);
- escr = p4_default_escr_conf(cpu, attr->exclude_kernel, attr->exclude_user);
- hwc->config = p4_config_pack_escr(escr) | p4_config_pack_cccr(cccr);
+ escr = p4_default_escr_conf(cpu, event->attr.exclude_kernel,
+ event->attr.exclude_user);
+ event->hw.config = p4_config_pack_escr(escr) |
+ p4_config_pack_cccr(cccr);
if (p4_ht_active() && p4_ht_thread(cpu))
- hwc->config = p4_set_ht_bit(hwc->config);
+ event->hw.config = p4_set_ht_bit(event->hw.config);
+
+ if (event->attr.type != PERF_TYPE_RAW)
+ return 0;
+
+ /*
+ * We don't control raw events so it's up to the caller
+ * to pass sane values (and we don't count the thread number
+ * on HT machine but allow HT-compatible specifics to be
+ * passed on)
+ *
+ * XXX: HT wide things should check perf_paranoid_cpu() &&
+ * CAP_SYS_ADMIN
+ */
+ event->hw.config |= event->attr.config &
+ (p4_config_pack_escr(P4_ESCR_MASK_HT) |
+ p4_config_pack_cccr(P4_CCCR_MASK_HT));
return 0;
}
@@ -785,7 +790,6 @@ static __initconst struct x86_pmu p4_pmu
.eventsel = MSR_P4_BPU_CCCR0,
.perfctr = MSR_P4_BPU_PERFCTR0,
.event_map = p4_pmu_event_map,
- .raw_event = p4_pmu_raw_event,
.max_events = ARRAY_SIZE(p4_general_events),
.get_event_constraints = x86_get_event_constraints,
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists