Re: [PATCH v2 05/10] perf/amd/ibs: Don't allow freq mode event creation through ->config interface

[Ingo Molnar <mingo@kernel.org>]

* Ravi Bangoria <ravi.bangoria@amd.com> wrote:

> Hi Ingo,
> 
> >> Most perf_event_attr->config bits directly maps to IBS_{FETCH|OP}_CTL
> >> MSR. Since the sample period is programmed in these control registers,
> >> IBS PMU driver allows opening an IBS event by setting sample period
> >> value directly in perf_event_attr->config instead of using explicit
> >> perf_event_attr->sample_period interface.
> >>
> >> However, this logic is not applicable for freq mode events since the
> >> semantics of control register fields are applicable only to fixed
> >> sample period whereas the freq mode event adjusts sample period after
> >> each and every sample. Currently, IBS driver (unintentionally) allows
> >> creating freq mode event via ->config interface, which is semantically
> >> wrong as well as detrimental because it can be misused to bypass
> >> perf_event_max_sample_rate checks.
> > 
> > Then let's fix those rate checks?
> > 
> > AFAICS this patch limits functionality because the IBS driver would 
> > have to be fixed/enhanced to support frequency based events?
> > 
> > I'd strongly favor fixing/enhancing the driver instead, as 'perf top -F 
> > 1000' is easy to use and it is a useful concept.
> 
> No. This patch does not prevent opening an IBS event in freq mode. User
> can still open freq mode IBS event with usual interface attr->freq=1 and
> attr->sample_freq=<freq>. i.e. 'perf top -F 1000' with IBS event will
> work fine with this patch.

Oh, I see - the regular tooling uses perf_event_attr->sample_period, right?

Never mind then.

Thanks,

	Ingo