lkml.org 
[lkml]   [2010]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH 0/3] perf/core, x86: unify perfctr bitmasks
    From
    Date
    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)



    \
     
     \ /
      Last update: 2010-03-30 17:03    [W:0.032 / U:0.248 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site