* [PATCH 0/2] tools/power turbostat: Fix TDP MSR bits and invalid parameters
@ 2015-12-13 13:09 Chen Yu
2015-12-13 13:09 ` [PATCH 1/2] tools/power turbostat: bugfix: TDP MSRs print bits fixing Chen Yu
2015-12-13 13:09 ` [PATCH 2/2] tools/power turbostat: bugfix: fix several invalid parameters Chen Yu
0 siblings, 2 replies; 5+ messages in thread
From: Chen Yu @ 2015-12-13 13:09 UTC (permalink / raw)
To: len.brown; +Cc: rjw, srinivas.pandruvada, rui.zhang, linux-pm, linux-kernel
[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(-)
--
1.8.4.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] tools/power turbostat: bugfix: TDP MSRs print bits fixing
2015-12-13 13:09 [PATCH 0/2] tools/power turbostat: Fix TDP MSR bits and invalid parameters Chen Yu
@ 2015-12-13 13:09 ` Chen Yu
2016-03-13 7:38 ` Len Brown
2015-12-13 13:09 ` [PATCH 2/2] tools/power turbostat: bugfix: fix several invalid parameters Chen Yu
1 sibling, 1 reply; 5+ messages in thread
From: Chen Yu @ 2015-12-13 13:09 UTC (permalink / raw)
To: len.brown; +Cc: rjw, srinivas.pandruvada, rui.zhang, linux-pm, linux-kernel
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 <yu.c.chen@intel.com>
---
tools/power/x86/turbostat/turbostat.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 0dac7e0..9854929 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] tools/power turbostat: bugfix: fix several invalid parameters
2015-12-13 13:09 [PATCH 0/2] tools/power turbostat: Fix TDP MSR bits and invalid parameters Chen Yu
2015-12-13 13:09 ` [PATCH 1/2] tools/power turbostat: bugfix: TDP MSRs print bits fixing Chen Yu
@ 2015-12-13 13:09 ` Chen Yu
2016-03-13 7:28 ` Len Brown
1 sibling, 1 reply; 5+ messages in thread
From: Chen Yu @ 2015-12-13 13:09 UTC (permalink / raw)
To: len.brown; +Cc: rjw, srinivas.pandruvada, rui.zhang, linux-pm, linux-kernel
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 <yu.c.chen@intel.com>
---
tools/power/x86/turbostat/turbostat.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 9854929..47068de 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] tools/power turbostat: bugfix: fix several invalid parameters
2015-12-13 13:09 ` [PATCH 2/2] tools/power turbostat: bugfix: fix several invalid parameters Chen Yu
@ 2016-03-13 7:28 ` Len Brown
0 siblings, 0 replies; 5+ messages in thread
From: Len Brown @ 2016-03-13 7:28 UTC (permalink / raw)
To: Chen Yu
Cc: Brown, Len, Rafael J. Wysocki, Pandruvada, Srinivas, Zhang, Rui,
Linux PM list, linux-kernel@vger.kernel.org
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
On Sun, Dec 13, 2015 at 8:09 AM, Chen Yu <yu.c.chen@intel.com> wrote:
> 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 <yu.c.chen@intel.com>
> ---
> tools/power/x86/turbostat/turbostat.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 9854929..47068de 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Len Brown, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] tools/power turbostat: bugfix: TDP MSRs print bits fixing
2015-12-13 13:09 ` [PATCH 1/2] tools/power turbostat: bugfix: TDP MSRs print bits fixing Chen Yu
@ 2016-03-13 7:38 ` Len Brown
0 siblings, 0 replies; 5+ messages in thread
From: Len Brown @ 2016-03-13 7:38 UTC (permalink / raw)
To: Chen Yu
Cc: Brown, Len, Rafael J. Wysocki, Pandruvada, Srinivas, Zhang, Rui,
Linux PM list, linux-kernel@vger.kernel.org
Applied -- thanks!
-Len
On Sun, Dec 13, 2015 at 8:09 AM, Chen Yu <yu.c.chen@intel.com> wrote:
> 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 <yu.c.chen@intel.com>
> ---
> tools/power/x86/turbostat/turbostat.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 0dac7e0..9854929 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Len Brown, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-03-13 7:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-13 13:09 [PATCH 0/2] tools/power turbostat: Fix TDP MSR bits and invalid parameters Chen Yu
2015-12-13 13:09 ` [PATCH 1/2] tools/power turbostat: bugfix: TDP MSRs print bits fixing Chen Yu
2016-03-13 7:38 ` Len Brown
2015-12-13 13:09 ` [PATCH 2/2] tools/power turbostat: bugfix: fix several invalid parameters Chen Yu
2016-03-13 7:28 ` Len Brown
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).