[PATCH] tools/power turbostat: Fixed turbo ratio stats for Intel Xeon x200 family

[PATCH] tools/power turbostat: Fixed turbo ratio stats for Intel Xeon x200 family

[Hubert Chrzaniuk]

Algorithm for calculating turbo ratio limits for Intel Xeon x200 family
has some minor bugs. This patch fixes the routine.

Signed-off-by: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 0dac7e0..3a59aec 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -1330,12 +1330,13 @@ dump_knl_turbo_ratio_limits(void)
 
 	get_msr(base_cpu, MSR_NHM_TURBO_RATIO_LIMIT, &msr);
 
-	fprintf(stderr, "cpu%d: MSR_NHM_TURBO_RATIO_LIMIT: 0x%08llx\n",
+	fprintf(stderr, "cpu%d: MSR_TURBO_RATIO_LIMIT: 0x%08llx\n",
 		base_cpu, msr);
 
 	/**
 	 * Turbo encoding in KNL is as follows:
-	 * [7:0] -- Base value of number of active cores of bucket 1.
+	 * [0] -- Reserved
+	 * [7:1] -- Base value of number of active cores of bucket 1.
 	 * [15:8] -- Base value of freq ratio of bucket 1.
 	 * [20:16] -- +ve delta of number of active cores of bucket 2.
 	 * i.e. active cores of bucket 2 =
@@ -1354,8 +1355,8 @@ dump_knl_turbo_ratio_limits(void)
 	 * [60:56]-- +ve delta of number of active cores of bucket 7.
 	 * [63:61]-- -ve delta of freq ratio of bucket 7.
 	 */
-	cores = msr & 0xFF;
-	ratio = (msr >> 8) && 0xFF;
+	cores = (msr & 0xFF) >> 1;
+	ratio = (msr >> 8) & 0xFF;
 	if (ratio > 0)
 		fprintf(stderr,
 			"%d * %.0f = %.0f MHz max turbo %d active cores\n",
@@ -1363,8 +1364,8 @@ dump_knl_turbo_ratio_limits(void)
 
 	for (i = 16; i < 64; i = i + 8) {
 		delta_cores = (msr >> i) & 0x1F;
-		delta_ratio = (msr >> (i + 5)) && 0x7;
-		if (!delta_cores || !delta_ratio)
+		delta_ratio = (msr >> (i + 5)) & 0x7;
+		if (!delta_cores)
 			return;
 		cores = cores + delta_cores;
 		ratio = ratio - delta_ratio;
@@ -1889,6 +1890,7 @@ int has_nhm_turbo_ratio_limit(unsigned int family, unsigned int model)
 	/* Nehalem compatible, but do not include turbo-ratio limit support */
 	case 0x2E:	/* Nehalem-EX Xeon - Beckton */
 	case 0x2F:	/* Westmere-EX Xeon - Eagleton */
+	case 0x57:	/* PHI - Knights Landing (different MSR definition) */
 		return 0;
 	default:
 		return 1;
@@ -2599,7 +2601,7 @@ double slm_bclk(void)
 
 double discover_bclk(unsigned int family, unsigned int model)
 {
-	if (has_snb_msrs(family, model))
+	if (has_snb_msrs(family, model) || is_knl(family, model))
 		return 100.00;
 	else if (is_slm(family, model))
 		return slm_bclk();
-- 
1.8.3.1

Re: [PATCH] tools/power turbostat: Fixed turbo ratio stats for Intel Xeon x200 family

[Rafael J. Wysocki]

Hi,

On Tue, Feb 9, 2016 at 3:41 PM, Hubert Chrzaniuk
<hubert.chrzaniuk@intel.com> wrote:
> Algorithm for calculating turbo ratio limits for Intel Xeon x200 family
> has some minor bugs. This patch fixes the routine.

This is a bit laconic.  Please at least write what the bugs are here.

Plus the patch adds some KNL-specific checks that aren't directly
related to the fixes, so it looks like there should be two patches
rather?

Thanks,
Rafael


> Signed-off-by: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
> ---
>  tools/power/x86/turbostat/turbostat.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 0dac7e0..3a59aec 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -1330,12 +1330,13 @@ dump_knl_turbo_ratio_limits(void)
>
>         get_msr(base_cpu, MSR_NHM_TURBO_RATIO_LIMIT, &msr);
>
> -       fprintf(stderr, "cpu%d: MSR_NHM_TURBO_RATIO_LIMIT: 0x%08llx\n",
> +       fprintf(stderr, "cpu%d: MSR_TURBO_RATIO_LIMIT: 0x%08llx\n",
>                 base_cpu, msr);
>
>         /**
>          * Turbo encoding in KNL is as follows:
> -        * [7:0] -- Base value of number of active cores of bucket 1.
> +        * [0] -- Reserved
> +        * [7:1] -- Base value of number of active cores of bucket 1.
>          * [15:8] -- Base value of freq ratio of bucket 1.
>          * [20:16] -- +ve delta of number of active cores of bucket 2.
>          * i.e. active cores of bucket 2 =
> @@ -1354,8 +1355,8 @@ dump_knl_turbo_ratio_limits(void)
>          * [60:56]-- +ve delta of number of active cores of bucket 7.
>          * [63:61]-- -ve delta of freq ratio of bucket 7.
>          */
> -       cores = msr & 0xFF;
> -       ratio = (msr >> 8) && 0xFF;
> +       cores = (msr & 0xFF) >> 1;
> +       ratio = (msr >> 8) & 0xFF;
>         if (ratio > 0)
>                 fprintf(stderr,
>                         "%d * %.0f = %.0f MHz max turbo %d active cores\n",
> @@ -1363,8 +1364,8 @@ dump_knl_turbo_ratio_limits(void)
>
>         for (i = 16; i < 64; i = i + 8) {
>                 delta_cores = (msr >> i) & 0x1F;
> -               delta_ratio = (msr >> (i + 5)) && 0x7;
> -               if (!delta_cores || !delta_ratio)
> +               delta_ratio = (msr >> (i + 5)) & 0x7;
> +               if (!delta_cores)
>                         return;
>                 cores = cores + delta_cores;
>                 ratio = ratio - delta_ratio;
> @@ -1889,6 +1890,7 @@ int has_nhm_turbo_ratio_limit(unsigned int family, unsigned int model)
>         /* Nehalem compatible, but do not include turbo-ratio limit support */
>         case 0x2E:      /* Nehalem-EX Xeon - Beckton */
>         case 0x2F:      /* Westmere-EX Xeon - Eagleton */
> +       case 0x57:      /* PHI - Knights Landing (different MSR definition) */
>                 return 0;
>         default:
>                 return 1;
> @@ -2599,7 +2601,7 @@ double slm_bclk(void)
>
>  double discover_bclk(unsigned int family, unsigned int model)
>  {
> -       if (has_snb_msrs(family, model))
> +       if (has_snb_msrs(family, model) || is_knl(family, model))
>                 return 100.00;
>         else if (is_slm(family, model))
>                 return slm_bclk();
> --
> 1.8.3.1
>
> --
> 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

(unknown),

[Hubert Chrzaniuk]

>From Hubert Chrzaniuk <hubert.chrzaniuk@intel.com> # This line is ignored.
From: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
Subject: [PATCH v2] tools/power turbostat: Intel Xeon x200 bugfixes
In-Reply-To: <CAJZ5v0hyAeK9L+NcETZQXJKfmr+TX59bZ+759RbXz_we3wgzpg@mail.gmail.com>

> RJW:
> This is a bit laconic.  Please at least write what the bugs are here.

Summary of changes along with more detailed description has been added.

> RJW:
> Plus the patch adds some KNL-specific checks that aren't directly related
> to the fixes, so it looks like there should be two patches rather?

The change has been divided into two separate patches.

[PATCH 1/2] tools/power turbostat: Fixed turbo ratio stats for Intel Xeon x200 family

[Hubert Chrzaniuk]

Following changes have been made:
- changed MSR_NHM_TURBO_RATIO_LIMIT to MSR_TURBO_RATIO_LIMIT in debug print
  for consistency with Developer Manual
- updated definition of bitfields in MSR_TURBO_RATIO_LIMIT and appropriate
  parsing code
- added x200 to list of architectures that do not support Nahlem compatible
  definition of MSR_TURBO_RATIO_LIMIT register (x200 has the register but
  bits definition is custom)
- fixed typo in code that parses MSR_TURBO_RATIO_LIMIT
  (logical instead of bitwise operator)
- changed loop break condition because two concurrent turbo buckets may
  have zero delta ratio which would be still valid and no necessarily
  last bucket

Signed-off-by: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 0dac7e0..765c669 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -1330,12 +1330,13 @@ dump_knl_turbo_ratio_limits(void)
 
 	get_msr(base_cpu, MSR_NHM_TURBO_RATIO_LIMIT, &msr);
 
-	fprintf(stderr, "cpu%d: MSR_NHM_TURBO_RATIO_LIMIT: 0x%08llx\n",
+	fprintf(stderr, "cpu%d: MSR_TURBO_RATIO_LIMIT: 0x%08llx\n",
 		base_cpu, msr);
 
 	/**
 	 * Turbo encoding in KNL is as follows:
-	 * [7:0] -- Base value of number of active cores of bucket 1.
+	 * [0] -- Reserved
+	 * [7:1] -- Base value of number of active cores of bucket 1.
 	 * [15:8] -- Base value of freq ratio of bucket 1.
 	 * [20:16] -- +ve delta of number of active cores of bucket 2.
 	 * i.e. active cores of bucket 2 =
@@ -1354,8 +1355,8 @@ dump_knl_turbo_ratio_limits(void)
 	 * [60:56]-- +ve delta of number of active cores of bucket 7.
 	 * [63:61]-- -ve delta of freq ratio of bucket 7.
 	 */
-	cores = msr & 0xFF;
-	ratio = (msr >> 8) && 0xFF;
+	cores = (msr & 0xFF) >> 1;
+	ratio = (msr >> 8) & 0xFF;
 	if (ratio > 0)
 		fprintf(stderr,
 			"%d * %.0f = %.0f MHz max turbo %d active cores\n",
@@ -1363,8 +1364,8 @@ dump_knl_turbo_ratio_limits(void)
 
 	for (i = 16; i < 64; i = i + 8) {
 		delta_cores = (msr >> i) & 0x1F;
-		delta_ratio = (msr >> (i + 5)) && 0x7;
-		if (!delta_cores || !delta_ratio)
+		delta_ratio = (msr >> (i + 5)) & 0x7;
+		if (!delta_cores)
 			return;
 		cores = cores + delta_cores;
 		ratio = ratio - delta_ratio;
@@ -1889,6 +1890,7 @@ int has_nhm_turbo_ratio_limit(unsigned int family, unsigned int model)
 	/* Nehalem compatible, but do not include turbo-ratio limit support */
 	case 0x2E:	/* Nehalem-EX Xeon - Beckton */
 	case 0x2F:	/* Westmere-EX Xeon - Eagleton */
+	case 0x57:	/* PHI - Knights Landing (different MSR definition) */
 		return 0;
 	default:
 		return 1;
-- 
1.8.3.1

[PATCH 2/2] tools/power turbostat: changed bclk discovery path for Intel Xeon x200 Architecture

[Hubert Chrzaniuk]

x200 does not enable any way to programmatically obtain bus clock
speed. Bclk for the architecture has a fixed value of 100 MHz.
At the same time x200 cannot be included in has_snb_msrs since
it does not support C7 idle state.

Signed-off-by: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 765c669..3a59aec 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -2601,7 +2601,7 @@ double slm_bclk(void)
 
 double discover_bclk(unsigned int family, unsigned int model)
 {
-	if (has_snb_msrs(family, model))
+	if (has_snb_msrs(family, model) || is_knl(family, model))
 		return 100.00;
 	else if (is_slm(family, model))
 		return slm_bclk();
-- 
1.8.3.1

Re: [PATCH 1/2] tools/power turbostat: Fixed turbo ratio stats for Intel Xeon x200 family

[Len Brown]

Applied (replacing the previous version of this patch I had in my tree)

thanks,
-Len

On Wed, Feb 10, 2016 at 10:35 AM, Hubert Chrzaniuk
<hubert.chrzaniuk@intel.com> wrote:
> Following changes have been made:
> - changed MSR_NHM_TURBO_RATIO_LIMIT to MSR_TURBO_RATIO_LIMIT in debug print
>   for consistency with Developer Manual
> - updated definition of bitfields in MSR_TURBO_RATIO_LIMIT and appropriate
>   parsing code
> - added x200 to list of architectures that do not support Nahlem compatible
>   definition of MSR_TURBO_RATIO_LIMIT register (x200 has the register but
>   bits definition is custom)
> - fixed typo in code that parses MSR_TURBO_RATIO_LIMIT
>   (logical instead of bitwise operator)
> - changed loop break condition because two concurrent turbo buckets may
>   have zero delta ratio which would be still valid and no necessarily
>   last bucket
>
> Signed-off-by: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
> ---
>  tools/power/x86/turbostat/turbostat.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 0dac7e0..765c669 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -1330,12 +1330,13 @@ dump_knl_turbo_ratio_limits(void)
>
>         get_msr(base_cpu, MSR_NHM_TURBO_RATIO_LIMIT, &msr);
>
> -       fprintf(stderr, "cpu%d: MSR_NHM_TURBO_RATIO_LIMIT: 0x%08llx\n",
> +       fprintf(stderr, "cpu%d: MSR_TURBO_RATIO_LIMIT: 0x%08llx\n",
>                 base_cpu, msr);
>
>         /**
>          * Turbo encoding in KNL is as follows:
> -        * [7:0] -- Base value of number of active cores of bucket 1.
> +        * [0] -- Reserved
> +        * [7:1] -- Base value of number of active cores of bucket 1.
>          * [15:8] -- Base value of freq ratio of bucket 1.
>          * [20:16] -- +ve delta of number of active cores of bucket 2.
>          * i.e. active cores of bucket 2 =
> @@ -1354,8 +1355,8 @@ dump_knl_turbo_ratio_limits(void)
>          * [60:56]-- +ve delta of number of active cores of bucket 7.
>          * [63:61]-- -ve delta of freq ratio of bucket 7.
>          */
> -       cores = msr & 0xFF;
> -       ratio = (msr >> 8) && 0xFF;
> +       cores = (msr & 0xFF) >> 1;
> +       ratio = (msr >> 8) & 0xFF;
>         if (ratio > 0)
>                 fprintf(stderr,
>                         "%d * %.0f = %.0f MHz max turbo %d active cores\n",
> @@ -1363,8 +1364,8 @@ dump_knl_turbo_ratio_limits(void)
>
>         for (i = 16; i < 64; i = i + 8) {
>                 delta_cores = (msr >> i) & 0x1F;
> -               delta_ratio = (msr >> (i + 5)) && 0x7;
> -               if (!delta_cores || !delta_ratio)
> +               delta_ratio = (msr >> (i + 5)) & 0x7;
> +               if (!delta_cores)
>                         return;
>                 cores = cores + delta_cores;
>                 ratio = ratio - delta_ratio;
> @@ -1889,6 +1890,7 @@ int has_nhm_turbo_ratio_limit(unsigned int family, unsigned int model)
>         /* Nehalem compatible, but do not include turbo-ratio limit support */
>         case 0x2E:      /* Nehalem-EX Xeon - Beckton */
>         case 0x2F:      /* Westmere-EX Xeon - Eagleton */
> +       case 0x57:      /* PHI - Knights Landing (different MSR definition) */
>                 return 0;
>         default:
>                 return 1;
> --
> 1.8.3.1
>
> --
> 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

Re: [PATCH 2/2] tools/power turbostat: changed bclk discovery path for Intel Xeon x200 Architecture

[Len Brown]

Applied.

thanks,
-Len


On Wed, Feb 10, 2016 at 10:35 AM, Hubert Chrzaniuk
<hubert.chrzaniuk@intel.com> wrote:
> x200 does not enable any way to programmatically obtain bus clock
> speed. Bclk for the architecture has a fixed value of 100 MHz.
> At the same time x200 cannot be included in has_snb_msrs since
> it does not support C7 idle state.
>
> Signed-off-by: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
> ---
>  tools/power/x86/turbostat/turbostat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 765c669..3a59aec 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -2601,7 +2601,7 @@ double slm_bclk(void)
>
>  double discover_bclk(unsigned int family, unsigned int model)
>  {
> -       if (has_snb_msrs(family, model))
> +       if (has_snb_msrs(family, model) || is_knl(family, model))
>                 return 100.00;
>         else if (is_slm(family, model))
>                 return slm_bclk();
> --
> 1.8.3.1
>
> --
> 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