The RAPL Joule Counter is 32 bit, turbostat would
only print a *star* instead of printing the actual energy
consumed due to the overflow of RAPL register.
Introduce the accumulated RAPL mechanism to avoid the possible
overflow.
Chen Yu (3):
tools/power turbostat: Make the energy variable to be 64 bit
tools/power turbostat: Introduce functions to accumulate RAPL
consumption
tools/power turbostat: Enable accumulate RAPL display
tools/power/x86/turbostat/Makefile | 2 +-
tools/power/x86/turbostat/turbostat.c | 292 ++++++++++++++++++++++----
2 files changed, 248 insertions(+), 46 deletions(-)
--
2.17.1
Change the energy variable from 32bit to 64bit, so that it can record long
time duration. After this conversion, adjust the DELTA_WRAP32() accordingly.
Signed-off-by: Chen Yu <[email protected]>
---
tools/power/x86/turbostat/turbostat.c | 30 ++++++++++++---------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 33b370865d16..95f3047e94ae 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -211,12 +211,12 @@ struct pkg_data {
long long gfx_rc6_ms;
unsigned int gfx_mhz;
unsigned int package_id;
- unsigned int energy_pkg; /* MSR_PKG_ENERGY_STATUS */
- unsigned int energy_dram; /* MSR_DRAM_ENERGY_STATUS */
- unsigned int energy_cores; /* MSR_PP0_ENERGY_STATUS */
- unsigned int energy_gfx; /* MSR_PP1_ENERGY_STATUS */
- unsigned int rapl_pkg_perf_status; /* MSR_PKG_PERF_STATUS */
- unsigned int rapl_dram_perf_status; /* MSR_DRAM_PERF_STATUS */
+ unsigned long long energy_pkg; /* MSR_PKG_ENERGY_STATUS */
+ unsigned long long energy_dram; /* MSR_DRAM_ENERGY_STATUS */
+ unsigned long long energy_cores; /* MSR_PP0_ENERGY_STATUS */
+ unsigned long long energy_gfx; /* MSR_PP1_ENERGY_STATUS */
+ unsigned long long rapl_pkg_perf_status; /* MSR_PKG_PERF_STATUS */
+ unsigned long long rapl_dram_perf_status; /* MSR_DRAM_PERF_STATUS */
unsigned int pkg_temp_c;
unsigned long long counter[MAX_ADDED_COUNTERS];
} *package_even, *package_odd;
@@ -858,13 +858,13 @@ int dump_counters(struct thread_data *t, struct core_data *c,
outp += sprintf(outp, "pc10: %016llX\n", p->pc10);
outp += sprintf(outp, "cpu_lpi: %016llX\n", p->cpu_lpi);
outp += sprintf(outp, "sys_lpi: %016llX\n", p->sys_lpi);
- outp += sprintf(outp, "Joules PKG: %0X\n", p->energy_pkg);
- outp += sprintf(outp, "Joules COR: %0X\n", p->energy_cores);
- outp += sprintf(outp, "Joules GFX: %0X\n", p->energy_gfx);
- outp += sprintf(outp, "Joules RAM: %0X\n", p->energy_dram);
- outp += sprintf(outp, "Throttle PKG: %0X\n",
+ outp += sprintf(outp, "Joules PKG: %0llX\n", p->energy_pkg);
+ outp += sprintf(outp, "Joules COR: %0llX\n", p->energy_cores);
+ outp += sprintf(outp, "Joules GFX: %0llX\n", p->energy_gfx);
+ outp += sprintf(outp, "Joules RAM: %0llX\n", p->energy_dram);
+ outp += sprintf(outp, "Throttle PKG: %0llX\n",
p->rapl_pkg_perf_status);
- outp += sprintf(outp, "Throttle RAM: %0X\n",
+ outp += sprintf(outp, "Throttle RAM: %0llX\n",
p->rapl_dram_perf_status);
outp += sprintf(outp, "PTM: %dC\n", p->pkg_temp_c);
@@ -1210,11 +1210,7 @@ void format_all_counters(struct thread_data *t, struct core_data *c, struct pkg_
}
#define DELTA_WRAP32(new, old) \
- if (new > old) { \
- old = new - old; \
- } else { \
- old = 0x100000000 + new - old; \
- }
+ old = ((((unsigned long long)new << 32) - ((unsigned long long)old << 32)) >> 32);
int
delta_package(struct pkg_data *new, struct pkg_data *old)
--
2.17.1
Since the RAPL Joule Counter is 32 bit, turbostat would
only print a *star* instead of printing the actual energy
consumed to indicate the overflow due to long duration.
This does not meet the requirement from servers as the
sampling time of turbostat is usually very long on servers.
So maintain a set of MSR buffer, and update them
periodically before the 32bit MSR register is wrapped round,
so as to avoid the overflow.
The idea is similar to the implementation of ktime_get():
Periodical MSR timer:
total_rapl_sum += (current_rapl_msr - last_rapl_msr);
Using get_msr_sum() to get the accumulated RAPL:
return (current_rapl_msr - last_rapl_msr) + total_rapl_sum;
The accumulated RAPL mechanism will be turned on in next patch.
Originally-by: Aaron Lu <[email protected]>
Reviewed-by: Doug Smythies <[email protected]>
Tested-by: Doug Smythies <[email protected]>
Signed-off-by: Chen Yu <[email protected]>
---
v2: According to Len's suggestion:
1. Enable the accumulated RAPL mechanism by default.
2. Re-use the rapl_joule_counter_range to represent the
the timeout of periodical timer.
3. Remove the condition check in v1 patch when reading RAPL registers.
v3: Doug found two issues, fix them accordingly:
1. The max timeout should be calculated according to 2 * TDP.
2. There's no need to wrap the accumulated 64 bits RAPL.
---
tools/power/x86/turbostat/Makefile | 2 +-
tools/power/x86/turbostat/turbostat.c | 224 +++++++++++++++++++++++++-
2 files changed, 219 insertions(+), 7 deletions(-)
diff --git a/tools/power/x86/turbostat/Makefile b/tools/power/x86/turbostat/Makefile
index 2b6551269e43..d08765531bcb 100644
--- a/tools/power/x86/turbostat/Makefile
+++ b/tools/power/x86/turbostat/Makefile
@@ -16,7 +16,7 @@ override CFLAGS += -D_FORTIFY_SOURCE=2
%: %.c
@mkdir -p $(BUILD_OUTPUT)
- $(CC) $(CFLAGS) $< -o $(BUILD_OUTPUT)/$@ $(LDFLAGS) -lcap
+ $(CC) $(CFLAGS) $< -o $(BUILD_OUTPUT)/$@ $(LDFLAGS) -lcap -lrt
.PHONY : clean
clean :
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 95f3047e94ae..44fd3822a5fa 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -259,6 +259,113 @@ struct msr_counter {
#define SYSFS_PERCPU (1 << 1)
};
+/*
+ * The accumulated sum of MSR is defined as a monotonic
+ * increasing MSR, it will be accumulated periodically,
+ * despite its register's bit width.
+ */
+enum {
+ IDX_PKG_ENERGY,
+ IDX_DRAM_ENERGY,
+ IDX_PP0_ENERGY,
+ IDX_PP1_ENERGY,
+ IDX_PKG_PERF,
+ IDX_DRAM_PERF,
+ IDX_COUNT,
+};
+
+int get_msr_sum(int cpu, off_t offset, unsigned long long *msr);
+
+struct msr_sum_array {
+ /* get_msr_sum() = sum + (get_msr() - last) */
+ struct {
+ /*The accumulated MSR value is updated by the timer*/
+ unsigned long long sum;
+ /*The MSR footprint recorded in last timer*/
+ unsigned long long last;
+ } entries[IDX_COUNT];
+};
+
+/* The percpu MSR sum array.*/
+struct msr_sum_array *per_cpu_msr_sum;
+
+int idx_to_offset(int idx)
+{
+ int offset;
+
+ switch (idx) {
+ case IDX_PKG_ENERGY:
+ offset = MSR_PKG_ENERGY_STATUS;
+ break;
+ case IDX_DRAM_ENERGY:
+ offset = MSR_DRAM_ENERGY_STATUS;
+ break;
+ case IDX_PP0_ENERGY:
+ offset = MSR_PP0_ENERGY_STATUS;
+ break;
+ case IDX_PP1_ENERGY:
+ offset = MSR_PP1_ENERGY_STATUS;
+ break;
+ case IDX_PKG_PERF:
+ offset = MSR_PKG_PERF_STATUS;
+ break;
+ case IDX_DRAM_PERF:
+ offset = MSR_DRAM_PERF_STATUS;
+ break;
+ default:
+ offset = -1;
+ }
+ return offset;
+}
+
+int offset_to_idx(int offset)
+{
+ int idx;
+
+ switch (offset) {
+ case MSR_PKG_ENERGY_STATUS:
+ idx = IDX_PKG_ENERGY;
+ break;
+ case MSR_DRAM_ENERGY_STATUS:
+ idx = IDX_DRAM_ENERGY;
+ break;
+ case MSR_PP0_ENERGY_STATUS:
+ idx = IDX_PP0_ENERGY;
+ break;
+ case MSR_PP1_ENERGY_STATUS:
+ idx = IDX_PP1_ENERGY;
+ break;
+ case MSR_PKG_PERF_STATUS:
+ idx = IDX_PKG_PERF;
+ break;
+ case MSR_DRAM_PERF_STATUS:
+ idx = IDX_DRAM_PERF;
+ break;
+ default:
+ idx = -1;
+ }
+ return idx;
+}
+
+int idx_valid(int idx)
+{
+ switch (idx) {
+ case IDX_PKG_ENERGY:
+ return do_rapl & RAPL_PKG;
+ case IDX_DRAM_ENERGY:
+ return do_rapl & RAPL_DRAM;
+ case IDX_PP0_ENERGY:
+ return do_rapl & RAPL_CORES_ENERGY_STATUS;
+ case IDX_PP1_ENERGY:
+ return do_rapl & RAPL_GFX;
+ case IDX_PKG_PERF:
+ return do_rapl & RAPL_PKG_PERF_STATUS;
+ case IDX_DRAM_PERF:
+ return do_rapl & RAPL_DRAM_PERF_STATUS;
+ default:
+ return 0;
+ }
+}
struct sys_counters {
unsigned int added_thread_counters;
unsigned int added_core_counters;
@@ -1250,12 +1357,12 @@ delta_package(struct pkg_data *new, struct pkg_data *old)
old->gfx_mhz = new->gfx_mhz;
- DELTA_WRAP32(new->energy_pkg, old->energy_pkg);
- DELTA_WRAP32(new->energy_cores, old->energy_cores);
- DELTA_WRAP32(new->energy_gfx, old->energy_gfx);
- DELTA_WRAP32(new->energy_dram, old->energy_dram);
- DELTA_WRAP32(new->rapl_pkg_perf_status, old->rapl_pkg_perf_status);
- DELTA_WRAP32(new->rapl_dram_perf_status, old->rapl_dram_perf_status);
+ old->energy_pkg = new->energy_pkg - old->energy_pkg;
+ old->energy_cores = new->energy_cores - old->energy_cores;
+ old->energy_gfx = new->energy_gfx - old->energy_gfx;
+ old->energy_dram = new->energy_dram - old->energy_dram;
+ old->rapl_pkg_perf_status = new->rapl_pkg_perf_status - old->rapl_pkg_perf_status;
+ old->rapl_dram_perf_status = new->rapl_dram_perf_status - old->rapl_dram_perf_status;
for (i = 0, mp = sys.pp; mp; i++, mp = mp->next) {
if (mp->format == FORMAT_RAW)
@@ -3053,6 +3160,111 @@ void do_sleep(void)
}
}
+int get_msr_sum(int cpu, off_t offset, unsigned long long *msr)
+{
+ int ret, idx;
+ unsigned long long msr_cur, msr_last;
+
+ if (!per_cpu_msr_sum)
+ return 1;
+
+ idx = offset_to_idx(offset);
+ if (idx < 0)
+ return idx;
+ /* get_msr_sum() = sum + (get_msr() - last) */
+ ret = get_msr(cpu, offset, &msr_cur);
+ if (ret)
+ return ret;
+ msr_last = per_cpu_msr_sum[cpu].entries[idx].last;
+ DELTA_WRAP32(msr_cur, msr_last);
+ *msr = msr_last + per_cpu_msr_sum[cpu].entries[idx].sum;
+
+ return 0;
+}
+
+timer_t timerid;
+
+/* Timer callback, update the sum of MSRs periodically. */
+static int update_msr_sum(struct thread_data *t, struct core_data *c, struct pkg_data *p)
+{
+ int i, ret;
+ int cpu = t->cpu_id;
+
+ for (i = IDX_PKG_ENERGY; i < IDX_COUNT; i++) {
+ unsigned long long msr_cur, msr_last;
+ int offset;
+
+ if (!idx_valid(i))
+ continue;
+ offset = idx_to_offset(i);
+ if (offset < 0)
+ continue;
+ ret = get_msr(cpu, offset, &msr_cur);
+ if (ret) {
+ fprintf(outf, "Can not update msr(0x%x)\n", offset);
+ continue;
+ }
+
+ msr_last = per_cpu_msr_sum[cpu].entries[i].last;
+ per_cpu_msr_sum[cpu].entries[i].last = msr_cur & 0xffffffff;
+
+ DELTA_WRAP32(msr_cur, msr_last);
+ per_cpu_msr_sum[cpu].entries[i].sum += msr_last;
+ }
+ return 0;
+}
+
+static void
+msr_record_handler(union sigval v)
+{
+ for_all_cpus(update_msr_sum, EVEN_COUNTERS);
+}
+
+void msr_sum_record(void)
+{
+ struct itimerspec its;
+ struct sigevent sev;
+
+ per_cpu_msr_sum = calloc(topo.max_cpu_num + 1, sizeof(struct msr_sum_array));
+ if (!per_cpu_msr_sum) {
+ fprintf(outf, "Can not allocate memory for long time MSR.\n");
+ return;
+ }
+ /*
+ * Signal handler might be restricted, so use thread notifier instead.
+ */
+ memset(&sev, 0, sizeof(struct sigevent));
+ sev.sigev_notify = SIGEV_THREAD;
+ sev.sigev_notify_function = msr_record_handler;
+
+ sev.sigev_value.sival_ptr = &timerid;
+ if (timer_create(CLOCK_REALTIME, &sev, &timerid) == -1) {
+ fprintf(outf, "Can not create timer.\n");
+ goto release_msr;
+ }
+
+ its.it_value.tv_sec = 0;
+ its.it_value.tv_nsec = 1;
+ /*
+ * A wraparound time has been calculated early.
+ * Some sources state that the peak power for a
+ * microprocessor is usually 1.5 times the TDP rating,
+ * use 2 * TDP for safety.
+ */
+ its.it_interval.tv_sec = rapl_joule_counter_range / 2;
+ its.it_interval.tv_nsec = 0;
+
+ if (timer_settime(timerid, 0, &its, NULL) == -1) {
+ fprintf(outf, "Can not set timer.\n");
+ goto release_timer;
+ }
+ return;
+
+ release_timer:
+ timer_delete(timerid);
+ release_msr:
+ free(per_cpu_msr_sum);
+}
void turbostat_loop()
{
--
2.17.1
Enable the accumulated RAPL display by default.
Signed-off-by: Chen Yu <[email protected]>
---
tools/power/x86/turbostat/turbostat.c | 38 +++++++++++----------------
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 44fd3822a5fa..cc1661e753d2 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -1169,14 +1169,7 @@ int format_counters(struct thread_data *t, struct core_data *c,
}
}
- /*
- * If measurement interval exceeds minimum RAPL Joule Counter range,
- * indicate that results are suspect by printing "**" in fraction place.
- */
- if (interval_float < rapl_joule_counter_range)
- fmt8 = "%s%.2f";
- else
- fmt8 = "%6.0f**";
+ fmt8 = "%s%.2f";
if (DO_BIC(BIC_CorWatt) && (do_rapl & RAPL_PER_CORE_ENERGY))
outp += sprintf(outp, fmt8, (printed++ ? delim : ""), c->core_energy * rapl_energy_units / interval_float);
@@ -2069,39 +2062,39 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
p->sys_lpi = cpuidle_cur_sys_lpi_us;
if (do_rapl & RAPL_PKG) {
- if (get_msr(cpu, MSR_PKG_ENERGY_STATUS, &msr))
+ if (get_msr_sum(cpu, MSR_PKG_ENERGY_STATUS, &msr))
return -13;
- p->energy_pkg = msr & 0xFFFFFFFF;
+ p->energy_pkg = msr;
}
if (do_rapl & RAPL_CORES_ENERGY_STATUS) {
- if (get_msr(cpu, MSR_PP0_ENERGY_STATUS, &msr))
+ if (get_msr_sum(cpu, MSR_PP0_ENERGY_STATUS, &msr))
return -14;
- p->energy_cores = msr & 0xFFFFFFFF;
+ p->energy_cores = msr;
}
if (do_rapl & RAPL_DRAM) {
- if (get_msr(cpu, MSR_DRAM_ENERGY_STATUS, &msr))
+ if (get_msr_sum(cpu, MSR_DRAM_ENERGY_STATUS, &msr))
return -15;
- p->energy_dram = msr & 0xFFFFFFFF;
+ p->energy_dram = msr;
}
if (do_rapl & RAPL_GFX) {
- if (get_msr(cpu, MSR_PP1_ENERGY_STATUS, &msr))
+ if (get_msr_sum(cpu, MSR_PP1_ENERGY_STATUS, &msr))
return -16;
- p->energy_gfx = msr & 0xFFFFFFFF;
+ p->energy_gfx = msr;
}
if (do_rapl & RAPL_PKG_PERF_STATUS) {
- if (get_msr(cpu, MSR_PKG_PERF_STATUS, &msr))
+ if (get_msr_sum(cpu, MSR_PKG_PERF_STATUS, &msr))
return -16;
- p->rapl_pkg_perf_status = msr & 0xFFFFFFFF;
+ p->rapl_pkg_perf_status = msr;
}
if (do_rapl & RAPL_DRAM_PERF_STATUS) {
- if (get_msr(cpu, MSR_DRAM_PERF_STATUS, &msr))
+ if (get_msr_sum(cpu, MSR_DRAM_PERF_STATUS, &msr))
return -16;
- p->rapl_dram_perf_status = msr & 0xFFFFFFFF;
+ p->rapl_dram_perf_status = msr;
}
if (do_rapl & RAPL_AMD_F17H) {
- if (get_msr(cpu, MSR_PKG_ENERGY_STAT, &msr))
+ if (get_msr_sum(cpu, MSR_PKG_ENERGY_STAT, &msr))
return -13;
- p->energy_pkg = msr & 0xFFFFFFFF;
+ p->energy_pkg = msr;
}
if (DO_BIC(BIC_PkgTmp)) {
if (get_msr(cpu, MSR_IA32_PACKAGE_THERM_STATUS, &msr))
@@ -6076,6 +6069,7 @@ int main(int argc, char **argv)
return 0;
}
+ msr_sum_record();
/*
* if any params left, it must be a command to fork
*/
--
2.17.1
this cause turbostat not work on amd cpu.
root@localhost:~# /turbostat
turbostat version 20.09.30 - Len Brown <[email protected]>
CPUID(0): AuthenticAMD 0xd CPUID levels; 0x8000001f xlevels; family:model:stepping 0x17:18:1 (23:24:1)
CPUID(1): SSE3 MONITOR - - - TSC MSR - HT -
CPUID(6): APERF, No-TURBO, No-DTS, No-PTM, No-HWP, No-HWPnotify, No-HWPwindow, No-HWPepp, No-HWPpkg, No-EPB
CPUID(7): No-SGX
RAPL: 234 sec. Joule Counter Range, at 280 Watts
/dev/cpu_dma_latency: 8000 usec (constrained)
current_driver: acpi_idle
current_governor: menu
current_governor_ro: menu
cpu1: POLL: CPUIDLE CORE POLL IDLE
cpu1: C1: ACPI FFH MWAIT 0x0
cpu1: C2: ACPI IOPORT 0x414
cpu1: cpufreq driver: acpi-cpufreq
cpu1: cpufreq governor: powersave
cpufreq boost: 1
cpu0: MSR_RAPL_PWR_UNIT: 0x000a1003 (0.125000 Watts, 0.000015 Joules, 0.000977 sec.)
root@localhost:~#
On Mon, Mar 8, 2021 at 5:50 AM youling257 <[email protected]> wrote:
>
> this cause turbostat not work on amd cpu.
>
> root@localhost:~# /turbostat
> turbostat version 20.09.30 - Len Brown <[email protected]>
> CPUID(0): AuthenticAMD 0xd CPUID levels; 0x8000001f xlevels; family:model:stepping 0x17:18:1 (23:24:1)
There are already two fixes for this in the queue.
https://marc.info/?l=linux-pm&m=161382097503925&w=2
https://marc.info/?l=linux-pm&m=161141701219263&w=2
Hi,
On Mon, Mar 08, 2021 at 07:37:07AM -0800, Doug Smythies wrote:
> On Mon, Mar 8, 2021 at 5:50 AM youling257 <[email protected]> wrote:
> >
> > this cause turbostat not work on amd cpu.
> >
> > root@localhost:~# /turbostat
> > turbostat version 20.09.30 - Len Brown <[email protected]>
> > CPUID(0): AuthenticAMD 0xd CPUID levels; 0x8000001f xlevels; family:model:stepping 0x17:18:1 (23:24:1)
>
> There are already two fixes for this in the queue.
> https://marc.info/?l=linux-pm&m=161382097503925&w=2
> https://marc.info/?l=linux-pm&m=161141701219263&w=2
Thanks for reporting and pointing this out. I assume these two patches are both fixing the
same issue? It looks like these two patches should be merged into one:
1. Bingsong's patch access MSR_PKG_ENERGY_STAT only when RAPL_AMD_F17H is set,
which is consistent with the original context.
2. Bas Nieuwenhuizen's patch also fixes the issue in idx_valid()
in case RAPL_PKG was not set but with RAPL_AMD_F17H set.
thanks,
Chenyu
Hi Yu,
I am just resending your e-mail, adjusting the "To:" list to
include the 3 others that have submitted similar patches.
... Doug
On Mon, Mar 8, 2021 at 8:11 AM Chen Yu <[email protected]> wrote:
>
> Hi,
> On Mon, Mar 08, 2021 at 07:37:07AM -0800, Doug Smythies wrote:
> > On Mon, Mar 8, 2021 at 5:50 AM youling257 <[email protected]> wrote:
> > >
> > > this cause turbostat not work on amd cpu.
> > >
> > > root@localhost:~# /turbostat
> > > turbostat version 20.09.30 - Len Brown <[email protected]>
> > > CPUID(0): AuthenticAMD 0xd CPUID levels; 0x8000001f xlevels; family:model:stepping 0x17:18:1 (23:24:1)
> >
> > There are already two fixes for this in the queue.
> > https://marc.info/?l=linux-pm&m=161382097503925&w=2
> > https://marc.info/?l=linux-pm&m=161141701219263&w=2
> Thanks for reporting and pointing this out. I assume these two patches are both fixing the
> same issue? It looks like these two patches should be merged into one:
> 1. Bingsong's patch access MSR_PKG_ENERGY_STAT only when RAPL_AMD_F17H is set,
> which is consistent with the original context.
> 2. Bas Nieuwenhuizen's patch also fixes the issue in idx_valid()
> in case RAPL_PKG was not set but with RAPL_AMD_F17H set.
>
> thanks,
> Chenyu
On Wed, Mar 10, 2021 at 04:03:31PM -0800, Doug Smythies wrote:
> Hi Yu,
>
> I am just resending your e-mail, adjusting the "To:" list to
> include the 3 others that have submitted similar patches.
>
Thanks for adding the authors. I'll combine their patch with their credits and
send to Len for pulling.
thanks,
Chenyu
> ... Doug
>
> On Mon, Mar 8, 2021 at 8:11 AM Chen Yu <[email protected]> wrote:
> >
> > Hi,
> > On Mon, Mar 08, 2021 at 07:37:07AM -0800, Doug Smythies wrote:
> > > On Mon, Mar 8, 2021 at 5:50 AM youling257 <[email protected]> wrote:
> > > >
> > > > this cause turbostat not work on amd cpu.
> > > >
> > > > root@localhost:~# /turbostat
> > > > turbostat version 20.09.30 - Len Brown <[email protected]>
> > > > CPUID(0): AuthenticAMD 0xd CPUID levels; 0x8000001f xlevels; family:model:stepping 0x17:18:1 (23:24:1)
> > >
> > > There are already two fixes for this in the queue.
> > > https://marc.info/?l=linux-pm&m=161382097503925&w=2
> > > https://marc.info/?l=linux-pm&m=161141701219263&w=2
> > Thanks for reporting and pointing this out. I assume these two patches are both fixing the
> > same issue? It looks like these two patches should be merged into one:
> > 1. Bingsong's patch access MSR_PKG_ENERGY_STAT only when RAPL_AMD_F17H is set,
> > which is consistent with the original context.
> > 2. Bas Nieuwenhuizen's patch also fixes the issue in idx_valid()
> > in case RAPL_PKG was not set but with RAPL_AMD_F17H set.
> >
> > thanks,
> > Chenyu
Hi Youling, Bas, and Bingsong,
On Wed, Mar 10, 2021 at 04:03:31PM -0800, Doug Smythies wrote:
> Hi Yu,
>
> I am just resending your e-mail, adjusting the "To:" list to
> include the 3 others that have submitted similar patches.
>
> ... Doug
>
Could you please help check if the following combined patch works?
Thanks,
Chenyu
From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
From: Bas Nieuwenhuizen <[email protected]>
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 <[email protected]>
Co-developed-by: Bingsong Si <[email protected]>
Signed-off-by: Chen Yu <[email protected]>
---
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:
--
2.25.1
test this patch, turbostat can work.
2021-03-12 21:41 GMT+08:00, Chen Yu <[email protected]>:
> Hi Youling, Bas, and Bingsong,
> On Wed, Mar 10, 2021 at 04:03:31PM -0800, Doug Smythies wrote:
>> Hi Yu,
>>
>> I am just resending your e-mail, adjusting the "To:" list to
>> include the 3 others that have submitted similar patches.
>>
>> ... Doug
>>
> Could you please help check if the following combined patch works?
>
> Thanks,
> Chenyu
>
>
> From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
> From: Bas Nieuwenhuizen <[email protected]>
> 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 <[email protected]>
> Co-developed-by: Bingsong Si <[email protected]>
> Signed-off-by: Chen Yu <[email protected]>
> ---
> 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:
> --
> 2.25.1
>
>
Just resending to previously missed people who should also test this.
(See other e-mail: Re: turbostat: Fix Pkg Power on Zen)
On Sat, Mar 13, 2021 at 5:49 AM youling 257 <[email protected]> wrote:
>
> test this patch, turbostat can work.
>
> 2021-03-12 21:41 GMT+08:00, Chen Yu <[email protected]>:
> > Hi Youling, Bas, and Bingsong,
> > On Wed, Mar 10, 2021 at 04:03:31PM -0800, Doug Smythies wrote:
> >> Hi Yu,
> >>
> >> I am just resending your e-mail, adjusting the "To:" list to
> >> include the 3 others that have submitted similar patches.
> >>
> >> ... Doug
> >>
> > Could you please help check if the following combined patch works?
> >
> > Thanks,
> > Chenyu
> >
> >
> > From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
> > From: Bas Nieuwenhuizen <[email protected]>
> > 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 <[email protected]>
> > Co-developed-by: Bingsong Si <[email protected]>
> > Signed-off-by: Chen Yu <[email protected]>
> > ---
> > 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:
> > --
> > 2.25.1
> >
> >
On 24.03.21 15:44, Doug Smythies wrote:
> Just resending to previously missed people who should also test this.
> (See other e-mail: Re: turbostat: Fix Pkg Power on Zen)
Worked fine on my end (Zen 2), thanks.
>>> From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
>>> From: Bas Nieuwenhuizen <[email protected]>
>>> 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.
Hi Doug,
Am 24.03.21 um 15:44 schrieb Doug Smythies:
> Just resending to previously missed people who should also test this.
> (See other e-mail: Re: turbostat: Fix Pkg Power on Zen)
>
> On Sat, Mar 13, 2021 at 5:49 AM youling 257 <[email protected]> wrote:
>> test this patch, turbostat can work.
>>
>> 2021-03-12 21:41 GMT+08:00, Chen Yu <[email protected]>:
>>> [...]
>>> Could you please help check if the following combined patch works?
>>>
>>> Thanks,
>>> Chenyu
>>>
>>>
>>> From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
>>> From: Bas Nieuwenhuizen <[email protected]>
>>> 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 <[email protected]>
>>> Co-developed-by: Bingsong Si <[email protected]>
>>> Signed-off-by: Chen Yu <[email protected]>
>>> ---
>>> 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:
>>> --
>>> 2.25.1
Unsurprisingly, the patch from Bas works for me as well.
(Tested on a Zen 3 and an embedded Zen.)
Tested-by: Kurt Garloff <[email protected]>
Signed-off-by: Kurt Garloff <[email protected]>
Thanks,
--
Kurt Garloff <[email protected]>, Cologne, Germany
It works for me on Zen2.
> 2021年3月24日 下午10:44，Doug Smythies <[email protected]> 写道：
>
> Just resending to previously missed people who should also test this.
> (See other e-mail: Re: turbostat: Fix Pkg Power on Zen)
>
> On Sat, Mar 13, 2021 at 5:49 AM youling 257 <[email protected]> wrote:
>>
>> test this patch, turbostat can work.
>>
>> 2021-03-12 21:41 GMT+08:00, Chen Yu <[email protected]>:
>>> Hi Youling, Bas, and Bingsong,
>>> On Wed, Mar 10, 2021 at 04:03:31PM -0800, Doug Smythies wrote:
>>>> Hi Yu,
>>>>
>>>> I am just resending your e-mail, adjusting the "To:" list to
>>>> include the 3 others that have submitted similar patches.
>>>>
>>>> ... Doug
>>>>
>>> Could you please help check if the following combined patch works?
>>>
>>> Thanks,
>>> Chenyu
>>>
>>>
>>> From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
>>> From: Bas Nieuwenhuizen <[email protected]>
>>> 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 <[email protected]>
>>> Co-developed-by: Bingsong Si <[email protected]>
>>> Signed-off-by: Chen Yu <[email protected]>
>>> ---
>>> 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:
>>> --
>>> 2.25.1
>>>
>>>
>
Hi Bas Nieuwenhuizen,
On Fri, Mar 12, 2021 at 09:41:14PM +0800, Chen Yu wrote:
> Hi Youling, Bas, and Bingsong,
> On Wed, Mar 10, 2021 at 04:03:31PM -0800, Doug Smythies wrote:
> > Hi Yu,
> >
> > I am just resending your e-mail, adjusting the "To:" list to
> > include the 3 others that have submitted similar patches.
> >
> > ... Doug
> >
> Could you please help check if the following combined patch works?
>
> Thanks,
> Chenyu
>
>
> From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
> From: Bas Nieuwenhuizen <[email protected]>
> 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 <[email protected]>
> Co-developed-by: Bingsong Si <[email protected]>
> Signed-off-by: Chen Yu <[email protected]>
>
Can I add your SOB here if you are not object to it?
thanks,
Chenyu
(apologies for resend, seems my client switched to HTML before)
On Sat, Apr 24, 2021 at 3:40 AM Chen Yu <[email protected]> wrote:
>
> Hi Bas Nieuwenhuizen,
> On Fri, Mar 12, 2021 at 09:41:14PM +0800, Chen Yu wrote:
> > Hi Youling, Bas, and Bingsong,
> > On Wed, Mar 10, 2021 at 04:03:31PM -0800, Doug Smythies wrote:
> > > Hi Yu,
> > >
> > > I am just resending your e-mail, adjusting the "To:" list to
> > > include the 3 others that have submitted similar patches.
> > >
> > > ... Doug
> > >
> > Could you please help check if the following combined patch works?
> >
> > Thanks,
> > Chenyu
> >
> >
> > From 00e0622b1b693a5c7dc343aeb3aa51614a9e125e Mon Sep 17 00:00:00 2001
> > From: Bas Nieuwenhuizen <[email protected]>
> > 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 <[email protected]>
> > Co-developed-by: Bingsong Si <[email protected]>
> > Signed-off-by: Chen Yu <[email protected]>
> >
> Can I add your SOB here if you are not object to it?
Yes, seems fine by me. Thanks!
>
> thanks,
> Chenyu