Re: [PATCH v2] tools/power turbostat: Fix RAPL summary collection on AMD processors
From: Artem S. Tashkinov
Date: Tue Apr 20 2021 - 10:06:27 EST
On 4/20/21 1:15 PM, Chen Yu wrote:
On Tue, Apr 20, 2021 at 10:07:01AM +0200, Borislav Petkov wrote:
On Tue, Apr 20, 2021 at 10:03:36AM +0800, Chen Yu wrote:
On Mon, Apr 19, 2021 at 02:58:12PM -0500, Terry Bowman wrote:
Turbostat fails to correctly collect and display RAPL summary information
on Family 17h and 19h AMD processors. Running turbostat on these processors
returns immediately. If turbostat is working correctly then RAPL summary
data is displayed until the user provided command completes. If a command
is not provided by the user then turbostat is designed to continuously
display RAPL information until interrupted.
The issue is due to offset_to_idx() and idx_to_offset() missing support for
AMD MSR addresses/offsets. offset_to_idx()'s switch statement is missing
cases for AMD MSRs and idx_to_offset() does not include a path to return
AMD MSR(s) for any idx.
The solution is add AMD MSR support to offset_to_idx() and idx_to_offset().
These functions are split-out and renamed along architecture vendor lines
for supporting both AMD and Intel MSRs.
Fixes: 9972d5d84d76 ("tools/power turbostat: Enable accumulate RAPL display")
Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
Thanks for fixing, Terry, and previously there was a patch for this from Bas Nieuwenhuizen:
https://lkml.org/lkml/2021/3/12/682
and it is expected to have been merged in Len's branch already.
Expected?
So is it or is it not?
This patch was sent to Len and it is not in public repo yet. He is preparing for a new
release of turbostat as merge window is approaching.
And can you folks agree on a patch already and give it to Artem for
testing (CCed) because he's triggering it too:
https://bugzilla.kernel.org/show_bug.cgi?id=212357
Okay. I would vote for the the patch from Bas as it was a combined work from two
authors and tested by several AMD users. But let me paste it here too for Artem to
see if this also works for him:
From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
From: Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx>
Date: Fri, 12 Mar 2021 21:27:40 +0800
Subject: [PATCH] tools/power/turbostat: Fix turbostat for AMD Zen CPUs
It was reported that on Zen+ system turbostat started exiting,
which was tracked down to the MSR_PKG_ENERGY_STAT read failing because
offset_to_idx wasn't returning a non-negative index.
This patch combined the modification from Bingsong Si and
Bas Nieuwenhuizen and addd the MSR to the index system as alternative for
MSR_PKG_ENERGY_STATUS.
Fixes: 9972d5d84d76 ("tools/power turbostat: Enable accumulate RAPL display")
Reported-by: youling257 <youling257@xxxxxxxxx>
Tested-by: youling257 <youling257@xxxxxxxxx>
Tested-by: sibingsong <owen.si@xxxxxxxxx>
Tested-by: Kurt Garloff <kurt@xxxxxxxxxx>
Co-developed-by: Bingsong Si <owen.si@xxxxxxxxx>
Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
---
tools/power/x86/turbostat/turbostat.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index a7c4f0772e53..a7c965734fdf 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -297,7 +297,10 @@ int idx_to_offset(int idx)
switch (idx) {
case IDX_PKG_ENERGY:
- offset = MSR_PKG_ENERGY_STATUS;
+ if (do_rapl & RAPL_AMD_F17H)
+ offset = MSR_PKG_ENERGY_STAT;
+ else
+ offset = MSR_PKG_ENERGY_STATUS;
break;
case IDX_DRAM_ENERGY:
offset = MSR_DRAM_ENERGY_STATUS;
@@ -326,6 +329,7 @@ int offset_to_idx(int offset)
switch (offset) {
case MSR_PKG_ENERGY_STATUS:
+ case MSR_PKG_ENERGY_STAT:
idx = IDX_PKG_ENERGY;
break;
case MSR_DRAM_ENERGY_STATUS:
@@ -353,7 +357,7 @@ int idx_valid(int idx)
{
switch (idx) {
case IDX_PKG_ENERGY:
- return do_rapl & RAPL_PKG;
+ return do_rapl & (RAPL_PKG | RAPL_AMD_F17H);
case IDX_DRAM_ENERGY:
return do_rapl & RAPL_DRAM;
case IDX_PP0_ENERGY:
The patch works for me.
Tested-by: Artem S. Tashkinov <aros@xxxxxxx>