Re: [RFC] x86, perf: Add an aperfmperf driver

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Apr 28, 2015 at 02:25:37PM -0700, Andy Lutomirski wrote:
> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> index 80091ae54c2b..fadc822efc90 100644
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -45,6 +45,8 @@ obj-$(CONFIG_PERF_EVENTS_INTEL_UNCORE)	+= perf_event_intel_uncore.o \
>  					   perf_event_intel_uncore_snb.o \
>  					   perf_event_intel_uncore_snbep.o \
>  					   perf_event_intel_uncore_nhmex.o
> +obj-$(CONFIG_CPU_SUP_INTEL)		+= perf_event_aperf_mperf.o
> +obj-$(CONFIG_CPU_SUP_AMD)		+= perf_event_aperf_mperf.o

Does this actually work? I would expect it to go complain about having
to build it twice if you have both set.

> diff --git a/arch/x86/kernel/cpu/perf_event_aperfmperf.c b/arch/x86/kernel/cpu/perf_event_aperfmperf.c
> new file mode 100644
> index 000000000000..6e6d113bd9ce
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/perf_event_aperfmperf.c
> @@ -0,0 +1,119 @@
> +#include <linux/perf_event.h>
> +
> +#define APERFMPERF_EVENT_APERF 0
> +#define APERFMPERF_EVENT_MPERF 1
> +

> +static int aperfmperf_event_init(struct perf_event *event)
> +{
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +
> +	if (event->attr.config != APERFMPERF_EVENT_APERF &&
> +	    event->attr.config != APERFMPERF_EVENT_MPERF)
> +		return -ENOENT;

Once we pass the type test we know its 'our' event, and we can go return
fatal errors. No other PMU will pick this up.

This could therefore turn into an -EINVAL.

> +
> +	if (event->attr.config1 != 0)
> +		return -ENOENT;

Idem.

> +	/* no sampling */
> +	if (event->hw.sample_period)
> +		return -EINVAL;

You could have set pmu::capabilities =
PERF_PMU_CAP_NO_INTERRUPT which would also have killed that dead.

> +	/* unsupported modes and filters */
> +	if (event->attr.exclude_user   ||
> +	    event->attr.exclude_kernel ||
> +	    event->attr.exclude_hv     ||
> +	    event->attr.exclude_idle   ||
> +	    event->attr.exclude_host   ||
> +	    event->attr.exclude_guest  ||
> +	    event->attr.freq           ||
> +	    event->attr.sample_period) /* no sampling */
> +		return -EINVAL;
> +
> +	event->hw.idx = -1;
> +	event->hw.event_base = (event->attr.config == APERFMPERF_EVENT_APERF ?
> +				MSR_IA32_APERF : MSR_IA32_MPERF);
> +
> +	return 0;
> +}

The rest looks about right. Very simple thing indeed ;-)