linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3][v3] tools/power turbostat: Enable accumulated energy consumption for long time sampling
@ 2020-04-18  8:31 Chen Yu
  2020-04-18  8:31 ` [PATCH 1/3][v3] tools/power turbostat: Make the energy variable to be 64 bit Chen Yu
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Chen Yu @ 2020-04-18  8:31 UTC (permalink / raw)
  To: linux-pm
  Cc: Len Brown, Rafael J. Wysocki, Doug Smythies, linux-kernel, Chen Yu

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3][v3] tools/power turbostat: Make the energy variable to be 64 bit
  2020-04-18  8:31 [PATCH 0/3][v3] tools/power turbostat: Enable accumulated energy consumption for long time sampling Chen Yu
@ 2020-04-18  8:31 ` Chen Yu
  2020-04-18  8:31 ` [PATCH 2/3][v3] tools/power turbostat: Introduce functions to accumulate RAPL consumption Chen Yu
  2020-04-18  8:32 ` [PATCH 3/3][v3] tools/power turbostat: Enable accumulate RAPL display Chen Yu
  2 siblings, 0 replies; 17+ messages in thread
From: Chen Yu @ 2020-04-18  8:31 UTC (permalink / raw)
  To: linux-pm
  Cc: Len Brown, Rafael J. Wysocki, Doug Smythies, linux-kernel, Chen Yu

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 <yu.c.chen@intel.com>
---
 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/3][v3] tools/power turbostat: Introduce functions to accumulate RAPL consumption
  2020-04-18  8:31 [PATCH 0/3][v3] tools/power turbostat: Enable accumulated energy consumption for long time sampling Chen Yu
  2020-04-18  8:31 ` [PATCH 1/3][v3] tools/power turbostat: Make the energy variable to be 64 bit Chen Yu
@ 2020-04-18  8:31 ` Chen Yu
  2020-04-18  8:32 ` [PATCH 3/3][v3] tools/power turbostat: Enable accumulate RAPL display Chen Yu
  2 siblings, 0 replies; 17+ messages in thread
From: Chen Yu @ 2020-04-18  8:31 UTC (permalink / raw)
  To: linux-pm
  Cc: Len Brown, Rafael J. Wysocki, Doug Smythies, linux-kernel, Chen Yu

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 <aaron.lwe@gmail.com>
Reviewed-by: Doug Smythies <dsmythies@telus.net>
Tested-by: Doug Smythies <dsmythies@telus.net>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/3][v3] tools/power turbostat: Enable accumulate RAPL display
  2020-04-18  8:31 [PATCH 0/3][v3] tools/power turbostat: Enable accumulated energy consumption for long time sampling Chen Yu
  2020-04-18  8:31 ` [PATCH 1/3][v3] tools/power turbostat: Make the energy variable to be 64 bit Chen Yu
  2020-04-18  8:31 ` [PATCH 2/3][v3] tools/power turbostat: Introduce functions to accumulate RAPL consumption Chen Yu
@ 2020-04-18  8:32 ` Chen Yu
  2021-03-08 13:49   ` [3/3,v3] " youling257
  2 siblings, 1 reply; 17+ messages in thread
From: Chen Yu @ 2020-04-18  8:32 UTC (permalink / raw)
  To: linux-pm
  Cc: Len Brown, Rafael J. Wysocki, Doug Smythies, linux-kernel, Chen Yu

Enable the accumulated RAPL display by default.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2020-04-18  8:32 ` [PATCH 3/3][v3] tools/power turbostat: Enable accumulate RAPL display Chen Yu
@ 2021-03-08 13:49   ` youling257
  2021-03-08 15:37     ` Doug Smythies
  0 siblings, 1 reply; 17+ messages in thread
From: youling257 @ 2021-03-08 13:49 UTC (permalink / raw)
  To: yu.c.chen; +Cc: lenb, rjw, dsmythies, linux-kernel

this cause turbostat not work on amd cpu.


root@localhost:~# /turbostat
turbostat version 20.09.30 - Len Brown <lenb@kernel.org>
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:~#

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-08 13:49   ` [3/3,v3] " youling257
@ 2021-03-08 15:37     ` Doug Smythies
  2021-03-08 16:15       ` Chen Yu
  0 siblings, 1 reply; 17+ messages in thread
From: Doug Smythies @ 2021-03-08 15:37 UTC (permalink / raw)
  To: youling257; +Cc: yu.c.chen, lenb, rjw, linux-kernel, dsmythies

On Mon, Mar 8, 2021 at 5:50 AM youling257 <youling257@gmail.com> wrote:
>
> this cause turbostat not work on amd cpu.
>
> root@localhost:~# /turbostat
> turbostat version 20.09.30 - Len Brown <lenb@kernel.org>
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-08 15:37     ` Doug Smythies
@ 2021-03-08 16:15       ` Chen Yu
  2021-03-11  0:03         ` Doug Smythies
  0 siblings, 1 reply; 17+ messages in thread
From: Chen Yu @ 2021-03-08 16:15 UTC (permalink / raw)
  To: Doug Smythies, youling257
  Cc: youling257, lenb, rjw, linux-kernel, Len Brown, Chen Yu

Hi,
On Mon, Mar 08, 2021 at 07:37:07AM -0800, Doug Smythies wrote:
> On Mon, Mar 8, 2021 at 5:50 AM youling257 <youling257@gmail.com> wrote:
> >
> > this cause turbostat not work on amd cpu.
> >
> > root@localhost:~# /turbostat
> > turbostat version 20.09.30 - Len Brown <lenb@kernel.org>
> > 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-08 16:15       ` Chen Yu
@ 2021-03-11  0:03         ` Doug Smythies
  2021-03-12 12:17           ` Chen Yu
  2021-03-12 13:41           ` Chen Yu
  0 siblings, 2 replies; 17+ messages in thread
From: Doug Smythies @ 2021-03-11  0:03 UTC (permalink / raw)
  To: Chen Yu, bas, erwanaliasr1, owen.si
  Cc: youling257, lenb, rjw, linux-kernel, Len Brown, dsmythies

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 <yu.c.chen@intel.com> 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 <youling257@gmail.com> wrote:
> > >
> > > this cause turbostat not work on amd cpu.
> > >
> > > root@localhost:~# /turbostat
> > > turbostat version 20.09.30 - Len Brown <lenb@kernel.org>
> > > 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-11  0:03         ` Doug Smythies
@ 2021-03-12 12:17           ` Chen Yu
  2021-03-12 13:41           ` Chen Yu
  1 sibling, 0 replies; 17+ messages in thread
From: Chen Yu @ 2021-03-12 12:17 UTC (permalink / raw)
  To: Doug Smythies
  Cc: bas, erwanaliasr1, owen.si, youling257, lenb, rjw, linux-kernel,
	Len Brown

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 <yu.c.chen@intel.com> 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 <youling257@gmail.com> wrote:
> > > >
> > > > this cause turbostat not work on amd cpu.
> > > >
> > > > root@localhost:~# /turbostat
> > > > turbostat version 20.09.30 - Len Brown <lenb@kernel.org>
> > > > 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-11  0:03         ` Doug Smythies
  2021-03-12 12:17           ` Chen Yu
@ 2021-03-12 13:41           ` Chen Yu
  2021-03-13 13:49             ` youling 257
  2021-04-24  1:43             ` Chen Yu
  1 sibling, 2 replies; 17+ messages in thread
From: Chen Yu @ 2021-03-12 13:41 UTC (permalink / raw)
  To: Doug Smythies, bas, Bingsong Si, youling257
  Cc: bas, erwanaliasr1, owen.si, youling257, lenb, rjw, linux-kernel,
	Len Brown, Zhang Rui

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 <bas@basnieuwenhuizen.nl>
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@gmail.com>
Co-developed-by: Bingsong Si <owen.si@ucloud.cn>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-12 13:41           ` Chen Yu
@ 2021-03-13 13:49             ` youling 257
  2021-03-24 14:44               ` Doug Smythies
  2021-04-24  1:43             ` Chen Yu
  1 sibling, 1 reply; 17+ messages in thread
From: youling 257 @ 2021-03-13 13:49 UTC (permalink / raw)
  To: Chen Yu
  Cc: Doug Smythies, bas, Bingsong Si, erwanaliasr1, lenb, rjw,
	linux-kernel, Len Brown, Zhang Rui

test this patch, turbostat can work.

2021-03-12 21:41 GMT+08:00, Chen Yu <yu.c.chen@intel.com>:
> 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 <bas@basnieuwenhuizen.nl>
> 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@gmail.com>
> Co-developed-by: Bingsong Si <owen.si@ucloud.cn>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  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
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-13 13:49             ` youling 257
@ 2021-03-24 14:44               ` Doug Smythies
  2021-03-24 15:55                 ` Christian Kastner
                                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Doug Smythies @ 2021-03-24 14:44 UTC (permalink / raw)
  To: Salvatore Bonaccorso, Christian Kastner, Kurt Garloff
  Cc: Chen Yu, Bas Nieuwenhuizen, Bingsong Si, erwanaliasr1, Len Brown,
	rjw, Linux Kernel Mailing List, Len Brown, Zhang Rui,
	youling 257, dsmythies

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 <youling257@gmail.com> wrote:
>
> test this patch, turbostat can work.
>
> 2021-03-12 21:41 GMT+08:00, Chen Yu <yu.c.chen@intel.com>:
> > 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 <bas@basnieuwenhuizen.nl>
> > 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@gmail.com>
> > Co-developed-by: Bingsong Si <owen.si@ucloud.cn>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> >  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
> >
> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-24 14:44               ` Doug Smythies
@ 2021-03-24 15:55                 ` Christian Kastner
  2021-03-25 12:12                 ` Kurt Garloff
  2021-03-26  2:14                 ` sibingsong
  2 siblings, 0 replies; 17+ messages in thread
From: Christian Kastner @ 2021-03-24 15:55 UTC (permalink / raw)
  To: Doug Smythies
  Cc: Linux Kernel Mailing List, Kurt Garloff, Salvatore Bonaccorso

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 <bas@basnieuwenhuizen.nl>
>>> 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.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-24 14:44               ` Doug Smythies
  2021-03-24 15:55                 ` Christian Kastner
@ 2021-03-25 12:12                 ` Kurt Garloff
  2021-03-26  2:14                 ` sibingsong
  2 siblings, 0 replies; 17+ messages in thread
From: Kurt Garloff @ 2021-03-25 12:12 UTC (permalink / raw)
  To: Doug Smythies, Salvatore Bonaccorso, Christian Kastner
  Cc: Chen Yu, Bas Nieuwenhuizen, Bingsong Si, erwanaliasr1, Len Brown,
	rjw, Linux Kernel Mailing List, Len Brown, Zhang Rui,
	youling 257

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 <youling257@gmail.com> wrote:
>> test this patch, turbostat can work.
>>
>> 2021-03-12 21:41 GMT+08:00, Chen Yu <yu.c.chen@intel.com>:
>>> [...]
>>> 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 <bas@basnieuwenhuizen.nl>
>>> 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@gmail.com>
>>> Co-developed-by: Bingsong Si <owen.si@ucloud.cn>
>>> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
>>> ---
>>>  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 <kurt@garloff.de>
Signed-off-by: Kurt Garloff <kurt@garloff.de>

Thanks,

-- 
Kurt Garloff <kurt@garloff.de>, Cologne, Germany



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-24 14:44               ` Doug Smythies
  2021-03-24 15:55                 ` Christian Kastner
  2021-03-25 12:12                 ` Kurt Garloff
@ 2021-03-26  2:14                 ` sibingsong
  2 siblings, 0 replies; 17+ messages in thread
From: sibingsong @ 2021-03-26  2:14 UTC (permalink / raw)
  To: Doug Smythies
  Cc: Salvatore Bonaccorso, Christian Kastner, Kurt Garloff, Chen Yu,
	Bas Nieuwenhuizen, erwanaliasr1, Len Brown, rjw,
	Linux Kernel Mailing List, Len Brown, Zhang Rui, youling 257

It works for me on Zen2.

> 2021年3月24日 下午10:44,Doug Smythies <dsmythies@telus.net> 写道:
> 
> 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 <youling257@gmail.com> wrote:
>> 
>> test this patch, turbostat can work.
>> 
>> 2021-03-12 21:41 GMT+08:00, Chen Yu <yu.c.chen@intel.com>:
>>> 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 <bas@basnieuwenhuizen.nl>
>>> 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@gmail.com>
>>> Co-developed-by: Bingsong Si <owen.si@ucloud.cn>
>>> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
>>> ---
>>> 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
>>> 
>>> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-03-12 13:41           ` Chen Yu
  2021-03-13 13:49             ` youling 257
@ 2021-04-24  1:43             ` Chen Yu
  2021-04-24 10:50               ` Bas Nieuwenhuizen
  1 sibling, 1 reply; 17+ messages in thread
From: Chen Yu @ 2021-04-24  1:43 UTC (permalink / raw)
  To: Doug Smythies, bas, Bingsong Si, youling257
  Cc: erwanaliasr1, lenb, rjw, linux-kernel, Len Brown, Zhang Rui

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 <bas@basnieuwenhuizen.nl>
> 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@gmail.com>
> Co-developed-by: Bingsong Si <owen.si@ucloud.cn>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
>
Can I add your SOB here if you are not object to it?

thanks,
Chenyu

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [3/3,v3] tools/power turbostat: Enable accumulate RAPL display
  2021-04-24  1:43             ` Chen Yu
@ 2021-04-24 10:50               ` Bas Nieuwenhuizen
  0 siblings, 0 replies; 17+ messages in thread
From: Bas Nieuwenhuizen @ 2021-04-24 10:50 UTC (permalink / raw)
  To: Chen Yu
  Cc: Doug Smythies, Bingsong Si, youling257, erwanaliasr1, lenb, rjw,
	LKML, Len Brown, Zhang Rui

(apologies for resend, seems my client switched to HTML before)

On Sat, Apr 24, 2021 at 3:40 AM Chen Yu <yu.c.chen@intel.com> 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 <bas@basnieuwenhuizen.nl>
> > 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@gmail.com>
> > Co-developed-by: Bingsong Si <owen.si@ucloud.cn>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> >
> Can I add your SOB here if you are not object to it?

Yes, seems fine by me. Thanks!

>
> thanks,
> Chenyu

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-04-24 10:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-18  8:31 [PATCH 0/3][v3] tools/power turbostat: Enable accumulated energy consumption for long time sampling Chen Yu
2020-04-18  8:31 ` [PATCH 1/3][v3] tools/power turbostat: Make the energy variable to be 64 bit Chen Yu
2020-04-18  8:31 ` [PATCH 2/3][v3] tools/power turbostat: Introduce functions to accumulate RAPL consumption Chen Yu
2020-04-18  8:32 ` [PATCH 3/3][v3] tools/power turbostat: Enable accumulate RAPL display Chen Yu
2021-03-08 13:49   ` [3/3,v3] " youling257
2021-03-08 15:37     ` Doug Smythies
2021-03-08 16:15       ` Chen Yu
2021-03-11  0:03         ` Doug Smythies
2021-03-12 12:17           ` Chen Yu
2021-03-12 13:41           ` Chen Yu
2021-03-13 13:49             ` youling 257
2021-03-24 14:44               ` Doug Smythies
2021-03-24 15:55                 ` Christian Kastner
2021-03-25 12:12                 ` Kurt Garloff
2021-03-26  2:14                 ` sibingsong
2021-04-24  1:43             ` Chen Yu
2021-04-24 10:50               ` Bas Nieuwenhuizen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).