Re: [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h

[Jiri Olsa]

On Fri, May 15, 2020 at 02:57:30PM -0700, Stephane Eranian wrote:
> This patch series adds support for AMD Fam17h RAPL counters. As per
> AMD PPR, Fam17h support Package RAPL counters to monitor power usage.
> The RAPL counter operates as with Intel RAPL. As such, it is beneficial
> to share the code.
> 
> The series first moves the rapl.c file to common perf_events x86 and then
> adds the support.
> From the user's point of view, the interface is identical with
> /sys/devices/power. The energy-pkg event is the only one supported.
> 
> $ perf stat -a --per-socket -I 1000 -e power/energy-pkg/
> 
> Signed-off-by: Stephane Eranian 
> 
> Stephane Eranian (3):
>   perf/x86/rapl: move RAPL support to common x86 code
>   perf/x86/rapl: refactor code for Intel/AMD sharing
>   perf/x86/rapl: add AMD Fam17h RAPL support
> 
>  arch/x86/events/Kconfig|  8 ++---
>  arch/x86/events/Makefile   |  1 +
>  arch/x86/events/intel/Makefile |  2 --
>  arch/x86/events/probe.c|  4 +++
>  arch/x86/events/{intel => }/rapl.c | 55 +-
>  arch/x86/include/asm/msr-index.h   |  3 ++
>  6 files changed, 58 insertions(+), 15 deletions(-)
>  rename arch/x86/events/{intel => }/rapl.c (92%)

looks good to me, intel rapl keeps working for me ;-)

Acked-by: Jiri Olsa 

thanks,
jirka

Re: [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h

[Alexander Monakov]

On Fri, 15 May 2020, Stephane Eranian wrote:

> The series first moves the rapl.c file to common perf_events x86 and then
> adds the support.
> From the user's point of view, the interface is identical with
> /sys/devices/power. The energy-pkg event is the only one supported.

AMD also has per-core energy metering via MSR 0xc001029a, and I wonder
if you have plans to expose it to perf as well. I see it does not fit
so nicely with the existing code (as it's per-core instead of per-die).

The turbostat tool already exposes these per-core readings:

CoreCPU Avg_MHz Busy%   Bzy_MHz TSC_MHz CorWatt PkgWatt
-   -   3951100.00  3951237354.92   30.04
0   0   3945100.00  394523708.9729.98
1   1   3945100.00  394523709.11
2   2   3945100.00  394523708.96
4   3   3946100.00  394623709.32
5   4   3946100.00  394623709.11
6   5   3946100.00  394623709.39

turbostat sums the per-core energy figures to show the per-socket 54.92W
value. Though as you can see on this example, the figure is not in agreement
with the per-socket MSR you're using in your patch. Maybe the per-core
values are less reliable, but I believe I have a test that shows per-package
figure to be inaccurate as well. It would be nice if AMD clarified how the
counters work.

And, for what (little) it's worth, the series is:

Tested-by: Alexander Monakov 

Thank you.
Alexander

[PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h

[Stephane Eranian]

This patch series adds support for AMD Fam17h RAPL counters. As per
AMD PPR, Fam17h support Package RAPL counters to monitor power usage.
The RAPL counter operates as with Intel RAPL. As such, it is beneficial
to share the code.

The series first moves the rapl.c file to common perf_events x86 and then
adds the support.
>From the user's point of view, the interface is identical with
/sys/devices/power. The energy-pkg event is the only one supported.

$ perf stat -a --per-socket -I 1000 -e power/energy-pkg/

Signed-off-by: Stephane Eranian 

Stephane Eranian (3):
  perf/x86/rapl: move RAPL support to common x86 code
  perf/x86/rapl: refactor code for Intel/AMD sharing
  perf/x86/rapl: add AMD Fam17h RAPL support

 arch/x86/events/Kconfig|  8 ++---
 arch/x86/events/Makefile   |  1 +
 arch/x86/events/intel/Makefile |  2 --
 arch/x86/events/probe.c|  4 +++
 arch/x86/events/{intel => }/rapl.c | 55 +-
 arch/x86/include/asm/msr-index.h   |  3 ++
 6 files changed, 58 insertions(+), 15 deletions(-)
 rename arch/x86/events/{intel => }/rapl.c (92%)

-- 
2.26.2.761.g0e0b3e54be-goog