linux-kernel - Re: [PATCH 11/12] perf, x86: implement AMD IBS event configuration

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100420160557.GT11907@erda.amd.com>
Date:	Tue, 20 Apr 2010 18:05:57 +0200
From:	Robert Richter <robert.richter@....com>
To:	Stephane Eranian <eranian@...gle.com>
CC:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [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@....com

--
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/