lkml.org 
[lkml]   [2010]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 11/12] perf, x86: implement AMD IBS event configuration
    On 19.04.10 15:46:29, Stephane Eranian wrote:
    > > +       if (event->hw.sample_period)
    > > +               /*
    > > +                * The usage of the sample period attribute to
    > > +                * calculate the IBS max count value is not yet
    > > +                * supported, the max count must be in the raw config
    > > +                * value.
    > > +                */
    > > +               return -ENOSYS;
    > > +
    > What is the problem with directly using the period here, rejecting
    > any value that is off range or with bottom 4 bits set?

    Yes, I will create an updated version of this patch.

    > > +       if (event->attr.type != PERF_TYPE_RAW)
    > > +               /* only raw sample types are supported */
    > > +               return -EINVAL;
    > > +
    > > +       type = get_model_spec_type(event->attr.config);
    > > +       switch (type) {
    > > +       case MODEL_SPEC_TYPE_IBS_FETCH:
    > > +               event->hw.config = IBS_FETCH_CONFIG_MASK & event->attr.config;
    > > +               event->hw.idx = X86_PMC_IDX_SPECIAL_IBS_FETCH;
    > > +               /*
    > > +                * dirty hack, needed for __x86_pmu_enable_event(), we
    > > +                * should better change event->hw.config_base into
    > > +                * event->hw.config_msr that already includes the index
    > > +                */
    > > +               event->hw.config_base = MSR_AMD64_IBSFETCHCTL - event->hw.idx;
    > > +               break;
    > > +       case MODEL_SPEC_TYPE_IBS_OP:
    > > +               event->hw.config = IBS_OP_CONFIG_MASK & event->attr.config;
    > > +               event->hw.idx = X86_PMC_IDX_SPECIAL_IBS_OP;
    > > +               event->hw.config_base = MSR_AMD64_IBSOPCTL - event->hw.idx;
    > > +               break;
    >
    > IBSOP.cnt_ctl only available from RevC, need to check and reject if older.

    Right, for this patch I will modify IBS_OP_CONFIG_MASK to RevB only
    bits, later I will add cpuid detection and pmu revision checks in a
    separate patch.

    > > +static struct event_constraint amd_event_constraints[] =
    > > +{
    > > +       /*
    > > +        * The value for the weight of these constraints is higher
    > > +        * than in the unconstrainted case to process ibs after the
    > > +        * generic counters in x86_schedule_events().
    > > +        */
    > > +       __EVENT_CONSTRAINT(0, 1ULL << X86_PMC_IDX_SPECIAL_IBS_FETCH, 0,
    > > +                          AMD64_NUM_COUNTERS + 1),
    > > +       __EVENT_CONSTRAINT(0, 1ULL << X86_PMC_IDX_SPECIAL_IBS_OP, 0,
    > > +                          AMD64_NUM_COUNTERS + 1),
    > > +       EVENT_CONSTRAINT_END
    > > +};
    > > +
    > I think you could define EVENT_IBS_CONSTRAINT() and shorten
    > the definitions here. You could pass FETCH or OP and the macro
    > would do the bit shifting. This is how it's done for fixed counters on Intel.

    Ok, I will update this.

    -Robert

    --
    Advanced Micro Devices, Inc.
    Operating System Research Center
    email: robert.richter@amd.com

    --
    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-04-20 18:09    [W:4.134 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site