lkml.org 
[lkml]   [2010]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 1/2] perf-events: Add support for supplementary event registers v2
    From
    I looked at this patch thinking how could this be reused for LBR_SELECT.

    I am wondering if storing the extra MSR value in attr.config is really the way
    to go now as opposed to adding/overloading a field.

    For OFFCORE_RESPONSE, it makes sense to use attr.config because this is
    a refinement of the event (a sub-sub event if you want).

    For LBR_SELECT, you also need to pass a value, but there is no specific event
    associated with it. You can enable LBR with any event you want. Thus,
    storing the
    LBR_SELECT value independently would also make sense. And if we have this
    field for LBR_SELECT then we may as well use it for OFFCORE_REPONSE.

    The alternative would be to consider LBR_SELECT also as a refinement of
    the event being measured. Though by itself, it wouldn't do anything, it would
    have to be combine with a PERF_SAMPLE_BRANCH_STACK to attr.sample_type.

    What do you think?

    On Fri, Nov 12, 2010 at 5:55 PM, Andi Kleen <andi@firstfloor.org> wrote:
    > From: Andi Kleen <ak@linux.intel.com>
    >
    > Intel Nehalem/Westmere have a special OFFCORE_RESPONSE event
    > that can be used to monitor any offcore accesses from a core.
    > This is a very useful event for various tunings, and it's
    > also needed to implement the generic LLC-* events correctly.
    >
    > Unfortunately this event requires programming a mask in a separate
    > register. And worse this separate register is per core, not per
    > CPU thread.
    >
    > This patch adds:
    > - Teaches perf_events that OFFCORE_RESPONSE need extra parameters.
    > The extra parameters are passed by user space in the unused upper
    > 32bits of the config word.
    > - Add support to the Intel perf_event core to schedule the per
    > core resource. I tried to add generic infrastructure for this
    > that could be also used for other core resources.
    > The basic code has is patterned after the similar AMD northbridge
    > constraints code.
    >
    > Thanks to Stephane Eranian who pointed out some problems
    > in the original version and suggested improvements.
    >
    > Full git tree:
    > git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc-2.6.git perf-offcore2
    > Cc: eranian@google.com
    > v2: Lots of updates based on review feedback. Also fixes some issues
    > Signed-off-by: Andi Kleen <ak@linux.intel.com>
    > ---
    >  arch/x86/kernel/cpu/perf_event.c       |   70 ++++++++++++++
    >  arch/x86/kernel/cpu/perf_event_intel.c |  159 ++++++++++++++++++++++++++++++++
    >  include/linux/perf_event.h             |    2 +
    >  3 files changed, 231 insertions(+), 0 deletions(-)
    >
    > diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
    > index ed63101..346006a 100644
    > --- a/arch/x86/kernel/cpu/perf_event.c
    > +++ b/arch/x86/kernel/cpu/perf_event.c
    > @@ -93,6 +93,8 @@ struct amd_nb {
    >        struct event_constraint event_constraints[X86_PMC_IDX_MAX];
    >  };
    >
    > +struct intel_percore;
    > +
    >  #define MAX_LBR_ENTRIES                16
    >
    >  struct cpu_hw_events {
    > @@ -127,6 +129,13 @@ struct cpu_hw_events {
    >        struct perf_branch_stack        lbr_stack;
    >        struct perf_branch_entry        lbr_entries[MAX_LBR_ENTRIES];
    >
    > +       /*
    > +        * Intel percore register state.
    > +        * Coordinate shared resources between HT threads.
    > +        */
    > +       int                             percore_used; /* Used by this CPU? */
    > +       struct intel_percore            *per_core;
    > +
    >        /*
    >         * AMD specific bits
    >         */
    > @@ -175,6 +184,32 @@ struct cpu_hw_events {
    >  #define for_each_event_constraint(e, c)        \
    >        for ((e) = (c); (e)->weight; (e)++)
    >
    > +/*
    > + * Extra registers for specific events.
    > + * Some events need large masks and require external MSRs.
    > + * Define a mapping to these extra registers.
    > + * The actual contents are still encoded in unused parts of the
    > + * original config u64.
    > + */
    > +struct extra_reg {
    > +       unsigned int            event;
    > +       unsigned int            msr;
    > +       unsigned int            extra_shift;
    > +       u64                     config_mask;
    > +       u64                     valid_mask;
    > +};
    > +
    > +#define EVENT_EXTRA_REG(e, ms, m, vm, es) {    \
    > +       .event = (e),           \
    > +       .msr = (ms),            \
    > +       .config_mask = (m),     \
    > +       .valid_mask = (vm),     \
    > +       .extra_shift = (es),    \
    > +       }
    > +#define INTEL_EVENT_EXTRA_REG(event, msr, vm, es)      \
    > +       EVENT_EXTRA_REG(event, msr, ARCH_PERFMON_EVENTSEL_EVENT, vm, es)
    > +#define EVENT_EXTRA_END EVENT_EXTRA_REG(0, 0, 0, 0, 0)
    > +
    >  union perf_capabilities {
    >        struct {
    >                u64     lbr_format    : 6;
    > @@ -219,6 +254,7 @@ struct x86_pmu {
    >        void            (*put_event_constraints)(struct cpu_hw_events *cpuc,
    >                                                 struct perf_event *event);
    >        struct event_constraint *event_constraints;
    > +       struct event_constraint *percore_constraints;
    >        void            (*quirks)(void);
    >        int             perfctr_second_write;
    >
    > @@ -247,6 +283,11 @@ struct x86_pmu {
    >         */
    >        unsigned long   lbr_tos, lbr_from, lbr_to; /* MSR base regs       */
    >        int             lbr_nr;                    /* hardware stack size */
    > +
    > +       /*
    > +        * Extra registers for events
    > +        */
    > +       struct extra_reg *extra_regs;
    >  };
    >
    >  static struct x86_pmu x86_pmu __read_mostly;
    > @@ -321,6 +362,33 @@ again:
    >        return new_raw_count;
    >  }
    >
    > +/*
    > + * Find and validate any extra registers to set up.
    > + */
    > +static int x86_pmu_extra_regs(u64 config, struct perf_event *event)
    > +{
    > +       struct extra_reg *er;
    > +       u64 extra;
    > +
    > +       event->hw.extra_reg = 0;
    > +       event->hw.extra_config = 0;
    > +
    > +       if (!x86_pmu.extra_regs)
    > +               return 0;
    > +
    > +       for (er = x86_pmu.extra_regs; er->msr; er++) {
    > +               if (er->event != (config & er->config_mask))
    > +                       continue;
    > +               event->hw.extra_reg = er->msr;
    > +               extra = config >> er->extra_shift;
    > +               if (extra & ~er->valid_mask)
    > +                       return -EINVAL;
    > +               event->hw.extra_config = extra;
    > +               break;
    > +       }
    > +       return 0;
    > +}
    > +
    >  static atomic_t active_events;
    >  static DEFINE_MUTEX(pmc_reserve_mutex);
    >
    > @@ -876,6 +944,8 @@ static inline void __x86_pmu_enable_event(struct hw_perf_event *hwc,
    >                                          u64 enable_mask)
    >  {
    >        wrmsrl(hwc->config_base + hwc->idx, hwc->config | enable_mask);
    > +       if (hwc->extra_reg)
    > +               wrmsrl(hwc->extra_reg, hwc->extra_config);
    >  }
    >
    >  static inline void x86_pmu_disable_event(struct perf_event *event)
    > diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
    > index c8f5c08..a84de7e 100644
    > --- a/arch/x86/kernel/cpu/perf_event_intel.c
    > +++ b/arch/x86/kernel/cpu/perf_event_intel.c
    > @@ -1,5 +1,20 @@
    >  #ifdef CONFIG_CPU_SUP_INTEL
    >
    > +/*
    > + * Per core state
    > + * This used to coordinate shared resources for HT threads.
    > + * Exists per CPU, but only the entry for the first CPU
    > + * in the core is used.
    > + */
    > +struct intel_percore {
    > +       raw_spinlock_t          lock;           /* protect structure */
    > +       int                     ref;            /* reference count */
    > +       u64                     config;         /* main counter config */
    > +       unsigned int            extra_reg;      /* extra MSR number */
    > +       u64                     extra_config;   /* extra MSR config */
    > +};
    > +static struct intel_percore __percpu *intel_percore;
    > +
    >  /*
    >  * Intel PerfMon, used on Core and later.
    >  */
    > @@ -64,6 +79,18 @@ static struct event_constraint intel_nehalem_event_constraints[] =
    >        EVENT_CONSTRAINT_END
    >  };
    >
    > +static struct extra_reg intel_nehalem_extra_regs[] =
    > +{
    > +       INTEL_EVENT_EXTRA_REG(0xb7, 0x1a6, 0xffff, 32), /* OFFCORE_RESPONSE_0 */
    > +       EVENT_EXTRA_END
    > +};
    > +
    > +static struct event_constraint intel_nehalem_percore_constraints[] =
    > +{
    > +       INTEL_EVENT_CONSTRAINT(0xb7, 0),
    > +       EVENT_CONSTRAINT_END
    > +};
    > +
    >  static struct event_constraint intel_westmere_event_constraints[] =
    >  {
    >        FIXED_EVENT_CONSTRAINT(0x00c0, 0), /* INST_RETIRED.ANY */
    > @@ -76,6 +103,20 @@ static struct event_constraint intel_westmere_event_constraints[] =
    >        EVENT_CONSTRAINT_END
    >  };
    >
    > +static struct extra_reg intel_westmere_extra_regs[] =
    > +{
    > +       INTEL_EVENT_EXTRA_REG(0xb7, 0x1a6, 0xffff, 32), /* OFFCORE_RESPONSE_0 */
    > +       INTEL_EVENT_EXTRA_REG(0xbb, 0x1a7, 0xffff, 32), /* OFFCORE_RESPONSE_1 */
    > +       EVENT_EXTRA_END
    > +};
    > +
    > +static struct event_constraint intel_westmere_percore_constraints[] =
    > +{
    > +       INTEL_EVENT_CONSTRAINT(0xb7, 0),
    > +       INTEL_EVENT_CONSTRAINT(0xbb, 0),
    > +       EVENT_CONSTRAINT_END
    > +};
    > +
    >  static struct event_constraint intel_gen_event_constraints[] =
    >  {
    >        FIXED_EVENT_CONSTRAINT(0x00c0, 0), /* INST_RETIRED.ANY */
    > @@ -794,6 +835,56 @@ intel_bts_constraints(struct perf_event *event)
    >  }
    >
    >  static struct event_constraint *
    > +intel_percore_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
    > +{
    > +       struct hw_perf_event *hwc = &event->hw;
    > +       unsigned int e = hwc->config & ARCH_PERFMON_EVENTSEL_EVENT;
    > +       struct event_constraint *c;
    > +       struct intel_percore *pc;
    > +
    > +       if (!x86_pmu.percore_constraints)
    > +               return NULL;
    > +
    > +       for (c = x86_pmu.percore_constraints; c->cmask; c++) {
    > +               if (e != c->code)
    > +                       continue;
    > +
    > +               c = NULL;
    > +
    > +               /*
    > +                * Allocate resource per core.
    > +                * Currently only one such per core resource can be allocated.
    > +                */
    > +               pc = cpuc->per_core;
    > +               if (!pc)
    > +                       break;
    > +               raw_spin_lock(&pc->lock);
    > +               if (pc->ref > 0) {
    > +                       /* Allow identical settings */
    > +                       if (hwc->config == pc->config &&
    > +                           hwc->extra_reg == pc->extra_reg &&
    > +                           hwc->extra_config == pc->extra_config) {
    > +                               pc->ref++;
    > +                               cpuc->percore_used = 1;
    > +                       } else {
    > +                               /* Deny due to conflict */
    > +                               c = &emptyconstraint;
    > +                       }
    > +               } else {
    > +                       pc->config = hwc->config;
    > +                       pc->extra_reg = hwc->extra_reg;
    > +                       pc->extra_config = hwc->extra_config;
    > +                       pc->ref = 1;
    > +                       cpuc->percore_used = 1;
    > +               }
    > +               raw_spin_unlock(&pc->lock);
    > +               return c;
    > +       }
    > +
    > +       return NULL;
    > +}
    > +
    > +static struct event_constraint *
    >  intel_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event)
    >  {
    >        struct event_constraint *c;
    > @@ -806,9 +897,38 @@ intel_get_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *event
    >        if (c)
    >                return c;
    >
    > +       c = intel_percore_constraints(cpuc, event);
    > +       if (c)
    > +               return c;
    > +
    >        return x86_get_event_constraints(cpuc, event);
    >  }
    >
    > +static void intel_put_event_constraints(struct cpu_hw_events *cpuc,
    > +                                       struct perf_event *event)
    > +{
    > +       struct event_constraint *c;
    > +       struct intel_percore *pc;
    > +       struct hw_perf_event *hwc = &event->hw;
    > +       unsigned int e = hwc->config & ARCH_PERFMON_EVENTSEL_EVENT;
    > +
    > +       if (!cpuc->percore_used)
    > +               return;
    > +
    > +       for (c = x86_pmu.percore_constraints; c->cmask; c++) {
    > +               if (e != c->code)
    > +                       continue;
    > +
    > +               pc = cpuc->per_core;
    > +               raw_spin_lock(&pc->lock);
    > +               pc->ref--;
    > +               BUG_ON(pc->ref < 0);
    > +               raw_spin_unlock(&pc->lock);
    > +               cpuc->percore_used = 0;
    > +               break;
    > +       }
    > +}
    > +
    >  static int intel_pmu_hw_config(struct perf_event *event)
    >  {
    >        int ret = x86_pmu_hw_config(event);
    > @@ -854,6 +974,7 @@ static __initconst const struct x86_pmu core_pmu = {
    >         */
    >        .max_period             = (1ULL << 31) - 1,
    >        .get_event_constraints  = intel_get_event_constraints,
    > +       .put_event_constraints  = intel_put_event_constraints,
    >        .event_constraints      = intel_core_event_constraints,
    >  };
    >
    > @@ -892,6 +1013,7 @@ static __initconst const struct x86_pmu intel_pmu = {
    >         */
    >        .max_period             = (1ULL << 31) - 1,
    >        .get_event_constraints  = intel_get_event_constraints,
    > +       .put_event_constraints  = intel_put_event_constraints,
    >
    >        .cpu_starting           = intel_pmu_cpu_starting,
    >        .cpu_dying              = intel_pmu_cpu_dying,
    > @@ -923,6 +1045,35 @@ static void intel_clovertown_quirks(void)
    >        x86_pmu.pebs_constraints = NULL;
    >  }
    >
    > +static __initdata int needs_percore = 1; /* CHANGEME */
    > +
    > +static __init int init_intel_percore(void)
    > +{
    > +       int cpu;
    > +
    > +       if (!needs_percore)
    > +               return 0;
    > +
    > +       intel_percore = alloc_percpu(struct intel_percore);
    > +       if (!intel_percore)
    > +               return -ENOMEM;
    > +
    > +       for_each_possible_cpu(cpu) {
    > +               raw_spin_lock_init(&per_cpu_ptr(intel_percore, cpu)->lock);
    > +               per_cpu(cpu_hw_events, cpu).per_core =
    > +                       per_cpu_ptr(intel_percore,
    > +                           cpumask_first(topology_thread_cpumask(cpu)));
    > +               printk("cpu %d core %d\n",
    > +                      cpu, cpumask_first(topology_thread_cpumask(cpu)));
    > +       }
    > +
    > +       return 0;
    > +}
    > +/*
    > + * Runs later because per cpu allocations don't work early on.
    > + */
    > +__initcall(init_intel_percore);
    > +
    >  static __init int intel_pmu_init(void)
    >  {
    >        union cpuid10_edx edx;
    > @@ -1010,7 +1161,11 @@ static __init int intel_pmu_init(void)
    >                intel_pmu_lbr_init_nhm();
    >
    >                x86_pmu.event_constraints = intel_nehalem_event_constraints;
    > +               x86_pmu.percore_constraints =
    > +                       intel_nehalem_percore_constraints;
    >                x86_pmu.enable_all = intel_pmu_nhm_enable_all;
    > +               x86_pmu.extra_regs = intel_nehalem_extra_regs;
    > +               needs_percore = 1;
    >                pr_cont("Nehalem events, ");
    >                break;
    >
    > @@ -1032,7 +1187,11 @@ static __init int intel_pmu_init(void)
    >                intel_pmu_lbr_init_nhm();
    >
    >                x86_pmu.event_constraints = intel_westmere_event_constraints;
    > +               x86_pmu.percore_constraints =
    > +                       intel_westmere_percore_constraints;
    >                x86_pmu.enable_all = intel_pmu_nhm_enable_all;
    > +               x86_pmu.extra_regs = intel_westmere_extra_regs;
    > +               needs_percore = 1;
    >                pr_cont("Westmere events, ");
    >                break;
    >
    > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
    > index 057bf22..6124e93 100644
    > --- a/include/linux/perf_event.h
    > +++ b/include/linux/perf_event.h
    > @@ -529,6 +529,8 @@ struct hw_perf_event {
    >                        unsigned long   event_base;
    >                        int             idx;
    >                        int             last_cpu;
    > +                       unsigned int    extra_reg;
    > +                       u64             extra_config;
    >                };
    >                struct { /* software */
    >                        struct hrtimer  hrtimer;
    > --
    > 1.7.1
    >
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-11-12 18:21    [W:3.419 / U:0.532 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site