[PATCH v3 0/2] tools/power turbostat: Add support for AMD Fam 17h (Zen) RAPL

[PATCH v3 0/2] tools/power turbostat: Add support for AMD Fam 17h (Zen) RAPL

[Calvin Walton]

This is an updated version of my RAPL patchset for Zen CPUs, rebased on
top of the current state of kernel/git/lenb/linux.get turbostat branch.
(In particular, this fixes conflicts with the AMD APIC-id patch)

I've split it into two patches now. The first patch only adds Cores
power reporting, since I'm fairly confident that this'll work on all of
the Zen chips. I think this part is ready to apply now.

The second part is still a work in progress until I'm able to (or I'm
able to find someone else to) test the package power reporting on a
multi-die Zen chip.

Calvin Walton (2):
  tools/power turbostat: Add support for AMD Fam 17h (Zen) RAPL
  [WIP] tools/power turbostat: Also read package power on AMD F17h (Zen)

 tools/power/x86/turbostat/turbostat.c | 167 +++++++++++++++++++++-----
 1 file changed, 138 insertions(+), 29 deletions(-)

-- 
2.18.0

[PATCH v3 1/2] tools/power turbostat: Add support for AMD Fam 17h (Zen) RAPL

[Calvin Walton]

Based on the Open-Source Register Reference for AMD Family 17h
Processors Models 00h-2Fh:
https://support.amd.com/TechDocs/56255_OSRR.pdf

These processors report RAPL support in bit 14 of CPUID 0x80000007 EDX,
and the following MSRs are present:
0xc0010299 (RAPL_PWR_UNIT), like Intel's RAPL_POWER_UNIT
0xc001029a (CORE_ENERGY_STAT), kind of like Intel's PP0_ENERGY_STATUS
0xc001029b (PKG_ENERGY_STAT), like Intel's PKG_ENERGY_STATUS

A notable difference from the Intel implementation is that AMD reports
the "Cores" energy usage separately for each core, rather than a
per-package total. The code has been adjusted to handle either case in a
generic way.

I haven't yet enabled collection of package power, due to being unable
to test it on multi-node systems (TR, EPYC).

Signed-off-by: Calvin Walton <calvin.walton@kepstin.ca>
---
 tools/power/x86/turbostat/turbostat.c | 159 +++++++++++++++++++++-----
 1 file changed, 130 insertions(+), 29 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 6837a65409ba..89d4e2e75774 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -44,6 +44,7 @@
 #include <cpuid.h>
 #include <linux/capability.h>
 #include <errno.h>
+#include <math.h>
 
 char *proc_stat = "/proc/stat";
 FILE *outf;
@@ -141,9 +142,21 @@ unsigned int first_counter_read = 1;
 
 #define RAPL_CORES_ENERGY_STATUS	(1 << 9)
 					/* 0x639 MSR_PP0_ENERGY_STATUS */
+#define RAPL_PER_CORE_ENERGY	(1 << 10)
+					/* Indicates cores energy collection is per-core,
+					 * not per-package. */
+#define RAPL_AMD_F17H		(1 << 11)
+					/* 0xc0010299 MSR_RAPL_PWR_UNIT */
+					/* 0xc001029a MSR_CORE_ENERGY_STAT */
+					/* 0xc001029b MSR_PKG_ENERGY_STAT */
 #define RAPL_CORES (RAPL_CORES_ENERGY_STATUS | RAPL_CORES_POWER_LIMIT)
 #define	TJMAX_DEFAULT	100
 
+/* MSRs that are not yet in the kernel-provided header. */
+#define MSR_RAPL_PWR_UNIT	0xc0010299
+#define MSR_CORE_ENERGY_STAT	0xc001029a
+#define MSR_PKG_ENERGY_STAT	0xc001029b
+
 #define MAX(a, b) ((a) > (b) ? (a) : (b))
 
 /*
@@ -187,6 +200,7 @@ struct core_data {
 	unsigned long long c7;
 	unsigned long long mc6_us;	/* duplicate as per-core for now, even though per module */
 	unsigned int core_temp_c;
+	unsigned int core_energy;	/* MSR_CORE_ENERGY_STAT */
 	unsigned int core_id;
 	unsigned long long counter[MAX_ADDED_COUNTERS];
 } *core_even, *core_odd;
@@ -680,6 +694,14 @@ void print_header(char *delim)
 	if (DO_BIC(BIC_CoreTmp))
 		outp += sprintf(outp, "%sCoreTmp", (printed++ ? delim : ""));
 
+	if (do_rapl && !rapl_joules) {
+		if (DO_BIC(BIC_CorWatt) && (do_rapl & RAPL_PER_CORE_ENERGY))
+			outp += sprintf(outp, "%sCorWatt", (printed++ ? delim : ""));
+	} else if (do_rapl && rapl_joules) {
+		if (DO_BIC(BIC_Cor_J) && (do_rapl & RAPL_PER_CORE_ENERGY))
+			outp += sprintf(outp, "%sCor_J", (printed++ ? delim : ""));
+	}
+
 	for (mp = sys.cp; mp; mp = mp->next) {
 		if (mp->format == FORMAT_RAW) {
 			if (mp->width == 64)
@@ -734,7 +756,7 @@ void print_header(char *delim)
 	if (do_rapl && !rapl_joules) {
 		if (DO_BIC(BIC_PkgWatt))
 			outp += sprintf(outp, "%sPkgWatt", (printed++ ? delim : ""));
-		if (DO_BIC(BIC_CorWatt))
+		if (DO_BIC(BIC_CorWatt) && !(do_rapl & RAPL_PER_CORE_ENERGY))
 			outp += sprintf(outp, "%sCorWatt", (printed++ ? delim : ""));
 		if (DO_BIC(BIC_GFXWatt))
 			outp += sprintf(outp, "%sGFXWatt", (printed++ ? delim : ""));
@@ -747,7 +769,7 @@ void print_header(char *delim)
 	} else if (do_rapl && rapl_joules) {
 		if (DO_BIC(BIC_Pkg_J))
 			outp += sprintf(outp, "%sPkg_J", (printed++ ? delim : ""));
-		if (DO_BIC(BIC_Cor_J))
+		if (DO_BIC(BIC_Cor_J) && !(do_rapl & RAPL_PER_CORE_ENERGY))
 			outp += sprintf(outp, "%sCor_J", (printed++ ? delim : ""));
 		if (DO_BIC(BIC_GFX_J))
 			outp += sprintf(outp, "%sGFX_J", (printed++ ? delim : ""));
@@ -808,6 +830,7 @@ int dump_counters(struct thread_data *t, struct core_data *c,
 		outp += sprintf(outp, "c6: %016llX\n", c->c6);
 		outp += sprintf(outp, "c7: %016llX\n", c->c7);
 		outp += sprintf(outp, "DTS: %dC\n", c->core_temp_c);
+		outp += sprintf(outp, "Joules: %0X\n", c->core_energy);
 
 		for (i = 0, mp = sys.cp; mp; i++, mp = mp->next) {
 			outp += sprintf(outp, "cADDED [%d] msr0x%x: %08llX\n",
@@ -1033,6 +1056,20 @@ 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**";
+
+	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);
+	if (DO_BIC(BIC_Cor_J) && (do_rapl & RAPL_PER_CORE_ENERGY))
+		outp += sprintf(outp, fmt8, (printed++ ? delim : ""), c->core_energy * rapl_energy_units);
+
 	/* print per-package data only for 1st core in package */
 	if (!(t->flags & CPU_IS_FIRST_CORE_IN_PACKAGE))
 		goto done;
@@ -1085,18 +1122,9 @@ int format_counters(struct thread_data *t, struct core_data *c,
 	if (DO_BIC(BIC_SYS_LPI))
 		outp += sprintf(outp, "%s%.2f", (printed++ ? delim : ""), 100.0 * p->sys_lpi / 1000000.0 / interval_float);
 
-	/*
- 	 * 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**";
-
 	if (DO_BIC(BIC_PkgWatt))
 		outp += sprintf(outp, fmt8, (printed++ ? delim : ""), p->energy_pkg * rapl_energy_units / interval_float);
-	if (DO_BIC(BIC_CorWatt))
+	if (DO_BIC(BIC_CorWatt) && !(do_rapl & RAPL_PER_CORE_ENERGY))
 		outp += sprintf(outp, fmt8, (printed++ ? delim : ""), p->energy_cores * rapl_energy_units / interval_float);
 	if (DO_BIC(BIC_GFXWatt))
 		outp += sprintf(outp, fmt8, (printed++ ? delim : ""), p->energy_gfx * rapl_energy_units / interval_float);
@@ -1104,7 +1132,7 @@ int format_counters(struct thread_data *t, struct core_data *c,
 		outp += sprintf(outp, fmt8, (printed++ ? delim : ""), p->energy_dram * rapl_dram_energy_units / interval_float);
 	if (DO_BIC(BIC_Pkg_J))
 		outp += sprintf(outp, fmt8, (printed++ ? delim : ""), p->energy_pkg * rapl_energy_units);
-	if (DO_BIC(BIC_Cor_J))
+	if (DO_BIC(BIC_Cor_J) && !(do_rapl & RAPL_PER_CORE_ENERGY))
 		outp += sprintf(outp, fmt8, (printed++ ? delim : ""), p->energy_cores * rapl_energy_units);
 	if (DO_BIC(BIC_GFX_J))
 		outp += sprintf(outp, fmt8, (printed++ ? delim : ""), p->energy_gfx * rapl_energy_units);
@@ -1249,6 +1277,8 @@ delta_core(struct core_data *new, struct core_data *old)
 	old->core_temp_c = new->core_temp_c;
 	old->mc6_us = new->mc6_us - old->mc6_us;
 
+	DELTA_WRAP32(new->core_energy, old->core_energy);
+
 	for (i = 0, mp = sys.cp; mp; i++, mp = mp->next) {
 		if (mp->format == FORMAT_RAW)
 			old->counter[i] = new->counter[i];
@@ -1391,6 +1421,7 @@ void clear_counters(struct thread_data *t, struct core_data *c, struct pkg_data
 	c->c7 = 0;
 	c->mc6_us = 0;
 	c->core_temp_c = 0;
+	c->core_energy = 0;
 
 	p->pkg_wtd_core_c0 = 0;
 	p->pkg_any_core_c0 = 0;
@@ -1473,6 +1504,8 @@ int sum_counters(struct thread_data *t, struct core_data *c,
 
 	average.cores.core_temp_c = MAX(average.cores.core_temp_c, c->core_temp_c);
 
+	average.cores.core_energy += c->core_energy;
+
 	for (i = 0, mp = sys.cp; mp; i++, mp = mp->next) {
 		if (mp->format == FORMAT_RAW)
 			continue;
@@ -1845,6 +1878,12 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
 		c->core_temp_c = tcc_activation_temp - ((msr >> 16) & 0x7F);
 	}
 
+	if (do_rapl & RAPL_AMD_F17H) {
+		if (get_msr(cpu, MSR_CORE_ENERGY_STAT, &msr))
+			return -14;
+		c->core_energy = msr & 0xFFFFFFFF;
+	}
+
 	for (i = 0, mp = sys.cp; mp; i++, mp = mp->next) {
 		if (get_mp(cpu, mp, &c->counter[i]))
 			return -10;
@@ -3738,7 +3777,7 @@ int print_perf_limit(struct thread_data *t, struct core_data *c, struct pkg_data
 #define	RAPL_POWER_GRANULARITY	0x7FFF	/* 15 bit power granularity */
 #define	RAPL_TIME_GRANULARITY	0x3F /* 6 bit time granularity */
 
-double get_tdp(unsigned int model)
+double get_tdp_intel(unsigned int model)
 {
 	unsigned long long msr;
 
@@ -3755,6 +3794,16 @@ double get_tdp(unsigned int model)
 	}
 }
 
+double get_tdp_amd(unsigned int family)
+{
+	switch (family) {
+	case 0x17:
+	default:
+		/* This is the max stock TDP of HEDT/Server Fam17h chips */
+		return 250.0;
+	}
+}
+
 /*
  * rapl_dram_energy_units_probe()
  * Energy units are either hard-coded, or come from RAPL Energy Unit MSR.
@@ -3776,21 +3825,12 @@ rapl_dram_energy_units_probe(int  model, double rapl_energy_units)
 	}
 }
 
-
-/*
- * rapl_probe()
- *
- * sets do_rapl, rapl_power_units, rapl_energy_units, rapl_time_units
- */
-void rapl_probe(unsigned int family, unsigned int model)
+void rapl_probe_intel(unsigned int family, unsigned int model)
 {
 	unsigned long long msr;
 	unsigned int time_unit;
 	double tdp;
 
-	if (!genuine_intel)
-		return;
-
 	if (family != 6)
 		return;
 
@@ -3920,13 +3960,66 @@ void rapl_probe(unsigned int family, unsigned int model)
 
 	rapl_time_units = 1.0 / (1 << (time_unit));
 
-	tdp = get_tdp(model);
+	tdp = get_tdp_intel(model);
+
+	rapl_joule_counter_range = 0xFFFFFFFF * rapl_energy_units / tdp;
+	if (!quiet)
+		fprintf(outf, "RAPL: %.0f sec. Joule Counter Range, at %.0f Watts\n", rapl_joule_counter_range, tdp);
+}
+
+void rapl_probe_amd(unsigned int family, unsigned int model)
+{
+	unsigned long long msr;
+	unsigned int eax, ebx, ecx, edx;
+	unsigned int has_rapl = 0;
+	double tdp;
+
+	if (max_extended_level >= 0x80000007) {
+		__cpuid(0x80000007, eax, ebx, ecx, edx);
+		/* RAPL (Fam 17h) */
+		has_rapl = edx & (1 << 14);
+	}
+
+	if (!has_rapl)
+		return;
+
+	switch (family) {
+	case 0x17: /* Zen, Zen+ */
+		do_rapl = RAPL_AMD_F17H | RAPL_PER_CORE_ENERGY;
+		if (rapl_joules)
+			BIC_PRESENT(BIC_Cor_J);
+		else
+			BIC_PRESENT(BIC_CorWatt);
+		break;
+	default:
+		return;
+	}
+
+	if (get_msr(base_cpu, MSR_RAPL_PWR_UNIT, &msr))
+		return;
+
+	rapl_time_units = ldexp(1.0, -(msr >> 16 & 0xf));
+	rapl_energy_units = ldexp(1.0, -(msr >> 8 & 0x1f));
+	rapl_power_units = ldexp(1.0, -(msr & 0xf));
+
+	tdp = get_tdp_amd(model);
 
 	rapl_joule_counter_range = 0xFFFFFFFF * rapl_energy_units / tdp;
 	if (!quiet)
 		fprintf(outf, "RAPL: %.0f sec. Joule Counter Range, at %.0f Watts\n", rapl_joule_counter_range, tdp);
+}
 
-	return;
+/*
+ * rapl_probe()
+ *
+ * sets do_rapl, rapl_power_units, rapl_energy_units, rapl_time_units
+ */
+void rapl_probe(unsigned int family, unsigned int model)
+{
+	if (genuine_intel)
+		rapl_probe_intel(family, model);
+	if (authentic_amd)
+		rapl_probe_amd(family, model);
 }
 
 void perf_limit_reasons_probe(unsigned int family, unsigned int model)
@@ -4032,6 +4125,7 @@ void print_power_limit_msr(int cpu, unsigned long long msr, char *label)
 int print_rapl(struct thread_data *t, struct core_data *c, struct pkg_data *p)
 {
 	unsigned long long msr;
+	const char *msr_name;
 	int cpu;
 
 	if (!do_rapl)
@@ -4047,10 +4141,17 @@ int print_rapl(struct thread_data *t, struct core_data *c, struct pkg_data *p)
 		return -1;
 	}
 
-	if (get_msr(cpu, MSR_RAPL_POWER_UNIT, &msr))
-		return -1;
+	if (do_rapl & RAPL_AMD_F17H) {
+		msr_name = "MSR_RAPL_PWR_UNIT";
+		if (get_msr(cpu, MSR_RAPL_PWR_UNIT, &msr))
+			return -1;
+	} else {
+		msr_name = "MSR_RAPL_POWER_UNIT";
+		if (get_msr(cpu, MSR_RAPL_POWER_UNIT, &msr))
+			return -1;
+	}
 
-	fprintf(outf, "cpu%d: MSR_RAPL_POWER_UNIT: 0x%08llx (%f Watts, %f Joules, %f sec.)\n", cpu, msr,
+	fprintf(outf, "cpu%d: %s: 0x%08llx (%f Watts, %f Joules, %f sec.)\n", cpu, msr_name, msr,
 		rapl_power_units, rapl_energy_units, rapl_time_units);
 
 	if (do_rapl & RAPL_PKG_POWER_INFO) {
-- 
2.18.0

[PATCH v3 2/2] [WIP] tools/power turbostat: Also read package power on AMD F17h (Zen)

[Calvin Walton]

The package power can also be read from an MSR. It's not clear exactly
what is included, and whether it's aggregated over all nodes or
reported separately.

It does look like this is reported separately per CCX (I get a single
value on the Ryzen R7 1700), but it might be reported separately per-
die (node?) on larger processors. If that's the case, it would have to
be recorded per node and aggregated for the socket.

Note that although Zen has these MSRs reporting power, it looks like
the actual RAPL configuration (power limits, configured TDP) is done
through PCI configuration space. I have not yet found any public
documentation for this.

Signed-off-by: Calvin Walton <calvin.walton@kepstin.ca>
---
 tools/power/x86/turbostat/turbostat.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 89d4e2e75774..675c894b8595 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -1973,6 +1973,11 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
 			return -16;
 		p->rapl_dram_perf_status = msr & 0xFFFFFFFF;
 	}
+	if (do_rapl & RAPL_AMD_F17H) {
+		if (get_msr(cpu, MSR_PKG_ENERGY_STAT, &msr))
+			return -13;
+		p->energy_pkg = msr & 0xFFFFFFFF;
+	}
 	if (DO_BIC(BIC_PkgTmp)) {
 		if (get_msr(cpu, MSR_IA32_PACKAGE_THERM_STATUS, &msr))
 			return -17;
@@ -3986,10 +3991,13 @@ void rapl_probe_amd(unsigned int family, unsigned int model)
 	switch (family) {
 	case 0x17: /* Zen, Zen+ */
 		do_rapl = RAPL_AMD_F17H | RAPL_PER_CORE_ENERGY;
-		if (rapl_joules)
+		if (rapl_joules) {
+			BIC_PRESENT(BIC_Pkg_J);
 			BIC_PRESENT(BIC_Cor_J);
-		else
+		} else {
+			BIC_PRESENT(BIC_PkgWatt);
 			BIC_PRESENT(BIC_CorWatt);
+		}
 		break;
 	default:
 		return;
-- 
2.18.0

Re: [PATCH v3 0/2] tools/power turbostat: Add support for AMD Fam 17h (Zen) RAPL

[Len Brown]

Applied, thanks!

On Fri, Aug 17, 2018 at 12:35 PM Calvin Walton <calvin.walton@kepstin.ca> wrote:
>
> This is an updated version of my RAPL patchset for Zen CPUs, rebased on
> top of the current state of kernel/git/lenb/linux.get turbostat branch.
> (In particular, this fixes conflicts with the AMD APIC-id patch)
>
> I've split it into two patches now. The first patch only adds Cores
> power reporting, since I'm fairly confident that this'll work on all of
> the Zen chips. I think this part is ready to apply now.
>
> The second part is still a work in progress until I'm able to (or I'm
> able to find someone else to) test the package power reporting on a
> multi-die Zen chip.
>
> Calvin Walton (2):
>   tools/power turbostat: Add support for AMD Fam 17h (Zen) RAPL
>   [WIP] tools/power turbostat: Also read package power on AMD F17h (Zen)
>
>  tools/power/x86/turbostat/turbostat.c | 167 +++++++++++++++++++++-----
>  1 file changed, 138 insertions(+), 29 deletions(-)
>
> --
> 2.18.0
>


-- 
Len Brown, Intel Open Source Technology Center

Re: [PATCH v3 2/2] [WIP] tools/power turbostat: Also read package power on AMD F17h (Zen)

[Len Brown]

Hi Calvin,
I'm inclined to apply this -- because it will not break anything,
and it would at least enable testing by people, who have this hardware.

thoughts?
-Len

On Fri, Aug 17, 2018 at 12:35 PM Calvin Walton <calvin.walton@kepstin.ca> wrote:
>
> The package power can also be read from an MSR. It's not clear exactly
> what is included, and whether it's aggregated over all nodes or
> reported separately.
>
> It does look like this is reported separately per CCX (I get a single
> value on the Ryzen R7 1700), but it might be reported separately per-
> die (node?) on larger processors. If that's the case, it would have to
> be recorded per node and aggregated for the socket.
>
> Note that although Zen has these MSRs reporting power, it looks like
> the actual RAPL configuration (power limits, configured TDP) is done
> through PCI configuration space. I have not yet found any public
> documentation for this.
>
> Signed-off-by: Calvin Walton <calvin.walton@kepstin.ca>
> ---
>  tools/power/x86/turbostat/turbostat.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 89d4e2e75774..675c894b8595 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -1973,6 +1973,11 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
>                         return -16;
>                 p->rapl_dram_perf_status = msr & 0xFFFFFFFF;
>         }
> +       if (do_rapl & RAPL_AMD_F17H) {
> +               if (get_msr(cpu, MSR_PKG_ENERGY_STAT, &msr))
> +                       return -13;
> +               p->energy_pkg = msr & 0xFFFFFFFF;
> +       }
>         if (DO_BIC(BIC_PkgTmp)) {
>                 if (get_msr(cpu, MSR_IA32_PACKAGE_THERM_STATUS, &msr))
>                         return -17;
> @@ -3986,10 +3991,13 @@ void rapl_probe_amd(unsigned int family, unsigned int model)
>         switch (family) {
>         case 0x17: /* Zen, Zen+ */
>                 do_rapl = RAPL_AMD_F17H | RAPL_PER_CORE_ENERGY;
> -               if (rapl_joules)
> +               if (rapl_joules) {
> +                       BIC_PRESENT(BIC_Pkg_J);
>                         BIC_PRESENT(BIC_Cor_J);
> -               else
> +               } else {
> +                       BIC_PRESENT(BIC_PkgWatt);
>                         BIC_PRESENT(BIC_CorWatt);
> +               }
>                 break;
>         default:
>                 return;
> --
> 2.18.0
>


-- 
Len Brown, Intel Open Source Technology Center

Re: [PATCH v3 2/2] [WIP] tools/power turbostat: Also read package power on AMD F17h (Zen)

[Calvin Walton]

(Whoops, resending this message. Forgot that Gmail defaulted to
HTML...)

On Wed, 2019-03-20 at 19:36 -0400, Len Brown wrote:
> Hi Calvin,
> I'm inclined to apply this -- because it will not break anything,
> and it would at least enable testing by people, who have this
> hardware.

Hi Len,

By all means go ahead and apply the package power patch as well. I've
tested it on both Summit (single-die desktop) and Raven (desktop/laptop
apu) with good results. The worst case as it stands is that if the
multi-die packages (EPYC/TR) don't aggregate the value per package,
then package power will be under-reported.

(I previously wrote that this would be easier to test with the patch
applied, because the package power is reported with the -Dump option,
but I'm not sure that's actually the case - the value might still only
be collected once per package?)

Calvin.

> 
> On Fri, Aug 17, 2018 at 12:35 PM Calvin Walton <
> calvin.walton@kepstin.ca> wrote:
> > The package power can also be read from an MSR. It's not clear
> > exactly
> > what is included, and whether it's aggregated over all nodes or
> > reported separately.
> > 
> > It does look like this is reported separately per CCX (I get a
> > single
> > value on the Ryzen R7 1700), but it might be reported separately
> > per-
> > die (node?) on larger processors. If that's the case, it would have
> > to
> > be recorded per node and aggregated for the socket.
> > 
> > Note that although Zen has these MSRs reporting power, it looks
> > like
> > the actual RAPL configuration (power limits, configured TDP) is
> > done
> > through PCI configuration space. I have not yet found any public
> > documentation for this.
> > 
> > Signed-off-by: Calvin Walton <calvin.walton@kepstin.ca>
> > ---
> >  tools/power/x86/turbostat/turbostat.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/power/x86/turbostat/turbostat.c
> > b/tools/power/x86/turbostat/turbostat.c
> > index 89d4e2e75774..675c894b8595 100644
> > --- a/tools/power/x86/turbostat/turbostat.c
> > +++ b/tools/power/x86/turbostat/turbostat.c
> > @@ -1973,6 +1973,11 @@ int get_counters(struct thread_data *t,
> > struct core_data *c, struct pkg_data *p)
> >                         return -16;
> >                 p->rapl_dram_perf_status = msr & 0xFFFFFFFF;
> >         }
> > +       if (do_rapl & RAPL_AMD_F17H) {
> > +               if (get_msr(cpu, MSR_PKG_ENERGY_STAT, &msr))
> > +                       return -13;
> > +               p->energy_pkg = msr & 0xFFFFFFFF;
> > +       }
> >         if (DO_BIC(BIC_PkgTmp)) {
> >                 if (get_msr(cpu, MSR_IA32_PACKAGE_THERM_STATUS,
> > &msr))
> >                         return -17;
> > @@ -3986,10 +3991,13 @@ void rapl_probe_amd(unsigned int family,
> > unsigned int model)
> >         switch (family) {
> >         case 0x17: /* Zen, Zen+ */
> >                 do_rapl = RAPL_AMD_F17H | RAPL_PER_CORE_ENERGY;
> > -               if (rapl_joules)
> > +               if (rapl_joules) {
> > +                       BIC_PRESENT(BIC_Pkg_J);
> >                         BIC_PRESENT(BIC_Cor_J);
> > -               else
> > +               } else {
> > +                       BIC_PRESENT(BIC_PkgWatt);
> >                         BIC_PRESENT(BIC_CorWatt);
> > +               }
> >                 break;
> >         default:
> >                 return;
> > --
> > 2.18.0
> > 
> 
> 

Re: [PATCH v3 2/2] [WIP] tools/power turbostat: Also read package power on AMD F17h (Zen)

[Len Brown]

> (I previously wrote that this would be easier to test with the patch
> applied, because the package power is reported with the -Dump option,
> but I'm not sure that's actually the case - the value might still only
> be collected once per package?)

Right -- we'd have to move it to a per-core or per-CPU data structure
to capture it on a finer granularity than per package.

Of course, if you want to see if you get different values on the cpus,
you can always dump the raw values from each cpu using rdmsr(1).

Unfortunately, I don't have that AMD hardware -- somebody with an
interest in it can test it out.

thanks,
Len Brown, Intel Open Source Technology Center