tools/power turbostat: Fix TDP MSR bits and invalid parameters
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
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
--- tools/power/x86/turbostat/turbostat.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
tools/power/x86/turbostat/turbostat.c +10 -10
@@ -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
Len Brown
maintainerRe: [PATCH 1/2] tools/power turbostat: bugfix: TDP MSRs print bits fixing
Applied -- thanks!
-Len
Quoted 74 lines · Chen Yu
MSR_CONFIG_TDP_NOMINAL:
should print all 8 bits ofbase_ratio(bit 0:7)0xFF
MSR_CONFIG_TDP_LEVEL_1:
should print all 15 bits ofPKG_MIN_PWR_LVL1(bit 48:62)0x7FFFshould print all 15 bits ofPKG_MAX_PWR_LVL1(bit 32:46)0x7FFFshould print all 8 bits ofLVL1_RATIO(bit 16:23)0xFFshould print all 15 bits ofPKG_TDP_LVL1(bit 0:14)0x7FFFAnd the same modification to
MSR_CONFIG_TDP_LEVEL_2.
MSR_TURBO_ACTIVATION_RATIO:
should print all 8 bits ofMAX_NON_TURBO_RATIO(bit 0:7)0xFFSigned-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@@ -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
PATCH 2/2 tools/power turbostat: bugfix: fix several invalid parameters
In reply to Chen Yudump_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]
--- tools/power/x86/turbostat/turbostat.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
tools/power/x86/turbostat/turbostat.c +4 -4
@@ -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
maintainerRe: [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_infois 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_tdpandperf_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]
doubleget_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@@ -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