linux-kernel - Re: [RFC] x86,perf: Implement minimal P4 PMU driver v14

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: <20100310192928.GD7219@erda.amd.com>
Date:	Wed, 10 Mar 2010 20:29:28 +0100
From:	Robert Richter <robert.richter@....com>
To:	Cyrill Gorcunov <gorcunov@...nvz.org>
CC:	Ingo Molnar <mingo@...e.hu>, Lin Ming <ming.m.lin@...el.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Stephane Eranian <eranian@...gle.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] x86,perf: Implement minimal P4 PMU driver v14

On 10.03.10 21:31:02, Cyrill Gorcunov wrote:
>  arch/x86/include/asm/perf_event.h      |    2 
>  arch/x86/include/asm/perf_p4.h         |  707 +++++++++++++++++++++++++++++++++

If so, it should be perf_event_p4.h.

>  arch/x86/kernel/cpu/perf_event.c       |   46 +-
>  arch/x86/kernel/cpu/perf_event_amd.c   |    2 
>  arch/x86/kernel/cpu/perf_event_intel.c |   15 
>  arch/x86/kernel/cpu/perf_event_p4.c    |  612 ++++++++++++++++++++++++++++
>  arch/x86/kernel/cpu/perf_event_p6.c    |    2 
>  7 files changed, 1363 insertions(+), 23 deletions(-)

> Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event.c
> =====================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event.c
> +++ linux-2.6.git/arch/x86/kernel/cpu/perf_event.c
> @@ -190,6 +190,8 @@ struct x86_pmu {
>  	void		(*enable_all)(void);
>  	void		(*enable)(struct perf_event *);
>  	void		(*disable)(struct perf_event *);
> +	int		(*hw_config)(struct perf_event_attr *attr, struct hw_perf_event *hwc);
> +	int		(*schedule_events)(struct cpu_hw_events *cpuc, int n, int *assign);

I don't like this extension since it widened the interface without
additional use.

(*hw_config) could be instead implemented in (*event_map).
(*schedule_events) could be implemented by a special p4 handler for
(*enable) in struct pmu. Maybe there are other solutions for both
cases, but it should be possible by adoption of existing functions.

The current implementation of model specific functions is
sufficient. We have already the following:

* event initialization: x86_pmu.raw_event(), x86_pmu.event_map()
* event enable:   event->pmu->enable(), x86_pmu.enable()
* event disable:  event->pmu->disable(), x86_pmu.disable()

Maybe I miss something in the list above. The introduction of more
function pointers should be reduced to a minimum.

If the pmu differs heavily you even could return a different pmu for
such an event.

-Robert

>  	unsigned	eventsel;
>  	unsigned	perfctr;
>  	u64		(*event_map)(int);
> @@ -415,6 +417,25 @@ set_ext_hw_attr(struct hw_perf_event *hw
>  	return 0;
>  }

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