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

[Borislav Petkov]

On Thu, Mar 21, 2019 at 09:15:22PM +0000, Phillips, Kim wrote:
> From: Kim Phillips <kim.phillips@xxxxxxx>
> 
> 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@xxxxxxx>
> Cc: Janakarajan Natarajan <Janakarajan.Natarajan@xxxxxxx>
> Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx>
> Cc: Martin LiÅka <mliska@xxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: Pu Wen <puwen@xxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
>  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.