* [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
* 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
* [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
end of thread, other threads:[~2016-03-13 7:39 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