tools/power turbostat: Fix TDP MSR bits and invalid parameters

PATCH In review Chen Yu 2 participants 5 messages Dec 13, 2015 – Mar 13, 2016 Turbostat utility

Share & tools

Chen Yu

author

2015-12-13 13:04 UTC #0 lore raw

cover tools/power turbostat: Fix TDP MSR bits and invalid parameters

[PATCH 1/2] is to fix the TDP MSRs bits calculation, which might bring incorrect information to user.
[PATCH 2/2] is to fix invalid parameters passing to dump_cstate_pstate_config_info and also fix some compiling problems.

Chen Yu (2):
tools/power turbostat: bugfix: TDP MSRs print bits fixing tools/power turbostat: bugfix: fix several invalid parameters

 tools/power/x86/turbostat/turbostat.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Signature · 1 line

1.8.4.2

Chen Yu

author

2015-12-13 13:04 UTC #1 lore raw

PATCH 1/2 tools/power turbostat: bugfix: TDP MSRs print bits fixing

MSR_CONFIG_TDP_NOMINAL:
should print all 8 bits of base_ratio (bit 0:7) 0xFF

MSR_CONFIG_TDP_LEVEL_1:
should print all 15 bits of PKG_MIN_PWR_LVL1 (bit 48:62) 0x7FFF should print all 15 bits of PKG_MAX_PWR_LVL1 (bit 32:46) 0x7FFF should print all 8 bits of LVL1_RATIO (bit 16:23) 0xFF should print all 15 bits of PKG_TDP_LVL1 (bit 0:14) 0x7FFF

And the same modification to MSR_CONFIG_TDP_LEVEL_2.

MSR_TURBO_ACTIVATION_RATIO:
should print all 8 bits of MAX_NON_TURBO_RATIO (bit 0:7) 0xFF

Signed-off-by Chen Yu <[email protected]>

---
 tools/power/x86/turbostat/turbostat.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

tools/power/x86/turbostat/turbostat.c +10 ~~-10~~

index 0dac7e0..9854929 100644
@@ -1409,25 +1409,25 @@ dump_config_tdp(void)
 
 	get_msr(base_cpu, MSR_CONFIG_TDP_NOMINAL, &msr);
 	fprintf(stderr, "cpu%d: MSR_CONFIG_TDP_NOMINAL: 0x%08llx", base_cpu, msr);
-	fprintf(stderr, " (base_ratio=%d)\n", (unsigned int)msr & 0xEF);
+	fprintf(stderr, " (base_ratio=%d)\n", (unsigned int)msr & 0xFF);
 
 	get_msr(base_cpu, MSR_CONFIG_TDP_LEVEL_1, &msr);
 	fprintf(stderr, "cpu%d: MSR_CONFIG_TDP_LEVEL_1: 0x%08llx (", base_cpu, msr);
 	if (msr) {
-		fprintf(stderr, "PKG_MIN_PWR_LVL1=%d ", (unsigned int)(msr >> 48) & 0xEFFF);
-		fprintf(stderr, "PKG_MAX_PWR_LVL1=%d ", (unsigned int)(msr >> 32) & 0xEFFF);
-		fprintf(stderr, "LVL1_RATIO=%d ", (unsigned int)(msr >> 16) & 0xEF);
-		fprintf(stderr, "PKG_TDP_LVL1=%d", (unsigned int)(msr) & 0xEFFF);
+		fprintf(stderr, "PKG_MIN_PWR_LVL1=%d ", (unsigned int)(msr >> 48) & 0x7FFF);
+		fprintf(stderr, "PKG_MAX_PWR_LVL1=%d ", (unsigned int)(msr >> 32) & 0x7FFF);
+		fprintf(stderr, "LVL1_RATIO=%d ", (unsigned int)(msr >> 16) & 0xFF);
+		fprintf(stderr, "PKG_TDP_LVL1=%d", (unsigned int)(msr) & 0x7FFF);
 	}
 	fprintf(stderr, ")\n");
 
 	get_msr(base_cpu, MSR_CONFIG_TDP_LEVEL_2, &msr);
 	fprintf(stderr, "cpu%d: MSR_CONFIG_TDP_LEVEL_2: 0x%08llx (", base_cpu, msr);
 	if (msr) {
-		fprintf(stderr, "PKG_MIN_PWR_LVL2=%d ", (unsigned int)(msr >> 48) & 0xEFFF);
-		fprintf(stderr, "PKG_MAX_PWR_LVL2=%d ", (unsigned int)(msr >> 32) & 0xEFFF);
-		fprintf(stderr, "LVL2_RATIO=%d ", (unsigned int)(msr >> 16) & 0xEF);
-		fprintf(stderr, "PKG_TDP_LVL2=%d", (unsigned int)(msr) & 0xEFFF);
+		fprintf(stderr, "PKG_MIN_PWR_LVL2=%d ", (unsigned int)(msr >> 48) & 0x7FFF);
+		fprintf(stderr, "PKG_MAX_PWR_LVL2=%d ", (unsigned int)(msr >> 32) & 0x7FFF);
+		fprintf(stderr, "LVL2_RATIO=%d ", (unsigned int)(msr >> 16) & 0xFF);
+		fprintf(stderr, "PKG_TDP_LVL2=%d", (unsigned int)(msr) & 0x7FFF);
 	}
 	fprintf(stderr, ")\n");

@@ -1440,7 +1440,7 @@ dump_config_tdp(void)
 	
 	get_msr(base_cpu, MSR_TURBO_ACTIVATION_RATIO, &msr);
 	fprintf(stderr, "cpu%d: MSR_TURBO_ACTIVATION_RATIO: 0x%08llx (", base_cpu, msr);
-	fprintf(stderr, "MAX_NON_TURBO_RATIO=%d", (unsigned int)(msr) & 0x7F);
+	fprintf(stderr, "MAX_NON_TURBO_RATIO=%d", (unsigned int)(msr) & 0xFF);
 	fprintf(stderr, " lock=%d", (unsigned int)(msr >> 31) & 1);
 	fprintf(stderr, ")\n");
 }

Signature · 1 line

1.8.4.2

Chen Yu

author

2015-12-13 13:04 UTC #3 lore raw

PATCH 2/2 tools/power turbostat: bugfix: fix several invalid parameters

In reply to Chen Yu

dump_cstate_pstate_config_info is invoked with no parameters, which might cause this function to misjudge the processor family and model. Fix this by passing family and model to it.

Besides, fix the compiling warning caused by missing type declaration in dump_cstate_pstate_config_info, get_tdp and perf_limit_reasons_probe:

turbostat.c: In function ‘dump_cstate_pstate_config_info’:
turbostat.c:1973:1: warning: type of ‘family’ defaults to ‘int’ [-Wimplicit-int]
dump_cstate_pstate_config_info(family, model)
^
turbostat.c:1973:1: warning: type of ‘model’ defaults to ‘int’ [-Wimplicit-int] turbostat.c: In function ‘get_tdp’:
turbostat.c:2145:8: warning: type of ‘model’ defaults to ‘int’ [-Wimplicit-int]
double get_tdp(model)

        ^
turbostat.c: In function ‘perf_limit_reasons_probe’:
turbostat.c:2259:6: warning: type of ‘family’ defaults to ‘int’ [-Wimplicit-int]
 void perf_limit_reasons_probe(family, model)
      ^

turbostat.c:2259:6: warning: type of ‘model’ defaults to ‘int’ [-Wimplicit-int]

Signed-off-by Chen Yu <[email protected]>

---
 tools/power/x86/turbostat/turbostat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

tools/power/x86/turbostat/turbostat.c +4 -4

index 9854929..47068de 100644
@@ -1970,7 +1970,7 @@ int has_config_tdp(unsigned int family, unsigned int model)
 }
 
 static void
-dump_cstate_pstate_config_info(family, model)
+dump_cstate_pstate_config_info(unsigned int family, unsigned int model)
 {
 	if (!do_nhm_platform_info)
 		return;

@@ -2142,7 +2142,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(model)
+double get_tdp(unsigned int model)
 {
 	unsigned long long msr;

@@ -2256,7 +2256,7 @@ void rapl_probe(unsigned int family, unsigned int model)
 	return;
 }
 
-void perf_limit_reasons_probe(family, model)
+void perf_limit_reasons_probe(unsigned int family, unsigned int model)
 {
 	if (!genuine_intel)
 		return;

@@ -2792,7 +2792,7 @@ void process_cpuid()
 	perf_limit_reasons_probe(family, model);
 
 	if (debug)
-		dump_cstate_pstate_config_info();
+		dump_cstate_pstate_config_info(family, model);
 
 	if (has_skl_msrs(family, model))
 		calculate_tsc_tweak();

Signature · 1 line

1.8.4.2

Len Brown

maintainer

2016-03-13 07:28 UTC #4 lore raw

Re: [PATCH 2/2] tools/power turbostat: bugfix: fix several invalid parameters

Thanks for the patch Yu.

As it turns out, I already had a fix for this in my tree. I think the compiler complained about these programming errors when I compiled on a machine different from my standard desktop.

If you know what compiler options cause it to check
for stuff like this by default, I'd welcome a patch to the Makefile to enable them.

thanks,
-Len

Quoted 74 lines · Chen Yu

dump_cstate_pstate_config_info is invoked with no parameters, which might cause this function to misjudge the processor family and model. Fix this by passing family and model to it.
Besides, fix the compiling warning caused by missing type declaration in dump_cstate_pstate_config_info, get_tdp and perf_limit_reasons_probe:
turbostat.c: In function ‘dump_cstate_pstate_config_info’:
turbostat.c:1973:1: warning: type of ‘family’ defaults to ‘int’ [-Wimplicit-int]
dump_cstate_pstate_config_info(family, model)
^
turbostat.c:1973:1: warning: type of ‘model’ defaults to ‘int’ [-Wimplicit-int] turbostat.c: In function ‘get_tdp’:
turbostat.c:2145:8: warning: type of ‘model’ defaults to ‘int’ [-Wimplicit-int]
double get_tdp(model)
        ^
turbostat.c: In function ‘perf_limit_reasons_probe’:
turbostat.c:2259:6: warning: type of ‘family’ defaults to ‘int’ [-Wimplicit-int]
 void perf_limit_reasons_probe(family, model)
      ^
turbostat.c:2259:6: warning: type of ‘model’ defaults to ‘int’ [-Wimplicit-int]
Signed-off-by Chen Yu <[email protected]>
---
 tools/power/x86/turbostat/turbostat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
tools/power/x86/turbostat/turbostat.c +4 -5
index 9854929..47068de 100644
@@ -1970,7 +1970,7 @@ int has_config_tdp(unsigned int family, unsigned int model)
 }
 
 static void
-dump_cstate_pstate_config_info(family, model)
+dump_cstate_pstate_config_info(unsigned int family, unsigned int model)
 {
        if (!do_nhm_platform_info)
                return;
@@ -2142,7 +2142,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(model)
+double get_tdp(unsigned int model)
 {
        unsigned long long msr;
 
@@ -2256,7 +2256,7 @@ void rapl_probe(unsigned int family, unsigned int model)
        return;
 }
 
-void perf_limit_reasons_probe(family, model)
+void perf_limit_reasons_probe(unsigned int family, unsigned int model)
 {
        if (!genuine_intel)
                return;
@@ -2792,7 +2792,7 @@ void process_cpuid()
        perf_limit_reasons_probe(family, model);
 
        if (debug)
-               dump_cstate_pstate_config_info();
+               dump_cstate_pstate_config_info(family, model);
 
        if (has_skl_msrs(family, model))
                calculate_tsc_tweak();
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line

Len Brown, Intel Open Source Technology Center

Len Brown

maintainer

2016-03-13 07:38 UTC #2 lore raw

Re: [PATCH 1/2] tools/power turbostat: bugfix: TDP MSRs print bits fixing

In reply to Chen Yu

Applied -- thanks!

-Len

Quoted 74 lines · Chen Yu

MSR_CONFIG_TDP_NOMINAL:
should print all 8 bits of base_ratio (bit 0:7) 0xFF

And the same modification to MSR_CONFIG_TDP_LEVEL_2.

MSR_TURBO_ACTIVATION_RATIO:
should print all 8 bits of MAX_NON_TURBO_RATIO (bit 0:7) 0xFF

Signed-off-by Chen Yu <[email protected]>

---
 tools/power/x86/turbostat/turbostat.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

tools/power/x86/turbostat/turbostat.c +10 ~~-11~~

index 0dac7e0..9854929 100644
@@ -1409,25 +1409,25 @@ dump_config_tdp(void)
 
        get_msr(base_cpu, MSR_CONFIG_TDP_NOMINAL, &msr);
        fprintf(stderr, "cpu%d: MSR_CONFIG_TDP_NOMINAL: 0x%08llx", base_cpu, msr);
-       fprintf(stderr, " (base_ratio=%d)\n", (unsigned int)msr & 0xEF);
+       fprintf(stderr, " (base_ratio=%d)\n", (unsigned int)msr & 0xFF);
 
        get_msr(base_cpu, MSR_CONFIG_TDP_LEVEL_1, &msr);
        fprintf(stderr, "cpu%d: MSR_CONFIG_TDP_LEVEL_1: 0x%08llx (", base_cpu, msr);
        if (msr) {
-               fprintf(stderr, "PKG_MIN_PWR_LVL1=%d ", (unsigned int)(msr >> 48) & 0xEFFF);
-               fprintf(stderr, "PKG_MAX_PWR_LVL1=%d ", (unsigned int)(msr >> 32) & 0xEFFF);
-               fprintf(stderr, "LVL1_RATIO=%d ", (unsigned int)(msr >> 16) & 0xEF);
-               fprintf(stderr, "PKG_TDP_LVL1=%d", (unsigned int)(msr) & 0xEFFF);
+               fprintf(stderr, "PKG_MIN_PWR_LVL1=%d ", (unsigned int)(msr >> 48) & 0x7FFF);
+               fprintf(stderr, "PKG_MAX_PWR_LVL1=%d ", (unsigned int)(msr >> 32) & 0x7FFF);
+               fprintf(stderr, "LVL1_RATIO=%d ", (unsigned int)(msr >> 16) & 0xFF);
+               fprintf(stderr, "PKG_TDP_LVL1=%d", (unsigned int)(msr) & 0x7FFF);
        }
        fprintf(stderr, ")\n");
 
        get_msr(base_cpu, MSR_CONFIG_TDP_LEVEL_2, &msr);
        fprintf(stderr, "cpu%d: MSR_CONFIG_TDP_LEVEL_2: 0x%08llx (", base_cpu, msr);
        if (msr) {
-               fprintf(stderr, "PKG_MIN_PWR_LVL2=%d ", (unsigned int)(msr >> 48) & 0xEFFF);
-               fprintf(stderr, "PKG_MAX_PWR_LVL2=%d ", (unsigned int)(msr >> 32) & 0xEFFF);
-               fprintf(stderr, "LVL2_RATIO=%d ", (unsigned int)(msr >> 16) & 0xEF);
-               fprintf(stderr, "PKG_TDP_LVL2=%d", (unsigned int)(msr) & 0xEFFF);
+               fprintf(stderr, "PKG_MIN_PWR_LVL2=%d ", (unsigned int)(msr >> 48) & 0x7FFF);
+               fprintf(stderr, "PKG_MAX_PWR_LVL2=%d ", (unsigned int)(msr >> 32) & 0x7FFF);
+               fprintf(stderr, "LVL2_RATIO=%d ", (unsigned int)(msr >> 16) & 0xFF);
+               fprintf(stderr, "PKG_TDP_LVL2=%d", (unsigned int)(msr) & 0x7FFF);
        }
        fprintf(stderr, ")\n");

@@ -1440,7 +1440,7 @@ dump_config_tdp(void)
 
        get_msr(base_cpu, MSR_TURBO_ACTIVATION_RATIO, &msr);
        fprintf(stderr, "cpu%d: MSR_TURBO_ACTIVATION_RATIO: 0x%08llx (", base_cpu, msr);
-       fprintf(stderr, "MAX_NON_TURBO_RATIO=%d", (unsigned int)(msr) & 0x7F);
+       fprintf(stderr, "MAX_NON_TURBO_RATIO=%d", (unsigned int)(msr) & 0xFF);
        fprintf(stderr, " lock=%d", (unsigned int)(msr >> 31) & 1);
        fprintf(stderr, ")\n");
 }
--

1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line

Len Brown, Intel Open Source Technology Center