Messages in this thread | | | Date | Tue, 20 Apr 2010 18:05:57 +0200 | From | Robert Richter <> | 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@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/
| |