linux-kernel - Re: [PATCH] x86/events: add event map for AMD Family 17h

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] [day] [month] [year] [list]
Date:   Thu, 28 Mar 2019 21:53:05 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     "Phillips, Kim" <kim.phillips@....com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Natarajan, Janakarajan" <Janakarajan.Natarajan@....com>,
        "Suthikulpanit, Suravee" <Suravee.Suthikulpanit@....com>,
        Martin Liška <mliska@...e.cz>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, Pu Wen <puwen@...on.cn>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH] x86/events: add event map for AMD Family 17h

On Thu, Mar 21, 2019 at 09:15:22PM +0000, Phillips, Kim wrote:
> From: Kim Phillips <kim.phillips@....com>
> 
> Family 17h differs from prior families by:
>  - not supporting an L2 cache miss event
>  - having re-enumerated PMC counters for:
>    - L2 cache references
>    - front & back end stalled cycles
> 
> So we add a new amd_f17h_perfmon_event_map so that the generic
> perf event names will resolve to the correct h/w events on
> family 17h and above processors.
> 
> Reference sections 2.1.13.3.3 (stalls) and 2.1.13.3.6 (L2):
> https://www.amd.com/system/files/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf
> 
> Fixes: e40ed1542dd7 ("perf/x86: Add perf support for AMD family-17h processors")
> Signed-off-by: Kim Phillips <kim.phillips@....com>
> Cc: Janakarajan Natarajan <Janakarajan.Natarajan@....com>
> Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
> Cc: Martin Liška <mliska@...e.cz>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: Pu Wen <puwen@...on.cn>
> Cc: x86@...nel.org
> Cc: linux-kernel@...r.kernel.org
> ---
>  arch/x86/events/amd/core.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 7d2d7c801dba..764d181afa18 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -127,8 +127,25 @@ static const u64 amd_perfmon_event_map[PERF_COUNT_HW_MAX] =
>    [PERF_COUNT_HW_STALLED_CYCLES_BACKEND]	= 0x00d1, /* "Dispatch stalls" event */
>  };
>  
> +/*
> + * AMD Performance Monitor Family 17h
> + */
> +static const u64 amd_f17h_perfmon_event_map[PERF_COUNT_HW_MAX] =
> +{
> +  [PERF_COUNT_HW_CPU_CYCLES]			= 0x0076,
> +  [PERF_COUNT_HW_INSTRUCTIONS]			= 0x00c0,
> +  [PERF_COUNT_HW_CACHE_REFERENCES]		= 0xff60,
> +  [PERF_COUNT_HW_BRANCH_INSTRUCTIONS]		= 0x00c2,
> +  [PERF_COUNT_HW_BRANCH_MISSES]			= 0x00c3,
> +  [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND]	= 0x0287,
> +  [PERF_COUNT_HW_STALLED_CYCLES_BACKEND]	= 0x0187,
> +};
> +
>  static u64 amd_pmu_event_map(int hw_event)
>  {
> +	if (boot_cpu_data.x86 >= 0x17)
> +		return amd_f17h_perfmon_event_map[hw_event];
> +
>  	return amd_perfmon_event_map[hw_event];
>  }
>  
> --

Your patch has some style problems, see below. Please run it through
checkpatch before submitting.

And yeah, that amd_perfmon_event_map in that file has some wrong
formatting too - preceding whitespace instead of tabs - so you can
submit a cleanup patch ontop to fix that up too.

Thx.

---
ERROR: that open brace { should be on the previous line
#70: FILE: arch/x86/events/amd/core.c:134:
+static const u64 amd_f17h_perfmon_event_map[PERF_COUNT_HW_MAX] =
+{

WARNING: please, no spaces at the start of a line
#71: FILE: arch/x86/events/amd/core.c:135:
+  [PERF_COUNT_HW_CPU_CYCLES]^I^I^I= 0x0076,$

WARNING: please, no spaces at the start of a line
#72: FILE: arch/x86/events/amd/core.c:136:
+  [PERF_COUNT_HW_INSTRUCTIONS]^I^I^I= 0x00c0,$

WARNING: please, no spaces at the start of a line
#73: FILE: arch/x86/events/amd/core.c:137:
+  [PERF_COUNT_HW_CACHE_REFERENCES]^I^I= 0xff60,$

WARNING: please, no spaces at the start of a line
#74: FILE: arch/x86/events/amd/core.c:138:
+  [PERF_COUNT_HW_BRANCH_INSTRUCTIONS]^I^I= 0x00c2,$

WARNING: please, no spaces at the start of a line
#75: FILE: arch/x86/events/amd/core.c:139:
+  [PERF_COUNT_HW_BRANCH_MISSES]^I^I^I= 0x00c3,$

WARNING: please, no spaces at the start of a line
#76: FILE: arch/x86/events/amd/core.c:140:
+  [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND]^I= 0x0287,$

WARNING: please, no spaces at the start of a line
#77: FILE: arch/x86/events/amd/core.c:141:
+  [PERF_COUNT_HW_STALLED_CYCLES_BACKEND]^I= 0x0187,$

total: 1 errors, 8 warnings, 25 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.