| Message ID | 20190605135830.23941-1-prarit@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Len Brown |
| Headers | show |
| Series | turbostat: Fix Haswell Core systems | expand |
Hi, > On Haswell (model 0x3C) turbostat fails with > > turbostat: cpu0: msr offset 0x630 read failed: Input/output error > > because Haswell Core does not have C8-C10. > > Output C8-C10 only on Haswell ULT. has_hsw_msrs() uses the model number returned by intel_model_duplicates(), but commit f5a4c76ad7de added code to return INTEL_FAM6_HASWELL_CORE for INTEL_FAM6_HASWELL_ULT there. So I think Haswell UTL also doesn't output C8-C10 with your patch. Something similar to the below patch is needed to differentiate between INTEL_FAM6_HASWELL_CORE and INTEL_FAM6_HASWELL_ULT. I don't have a Haswell-ULT machine, so I've only tested it on a Haswell-DT CPU. Best regards. > Fixes: f5a4c76ad7de ("tools/power turbostat: consolidate duplicate model > numbers") > Cc: Len Brown <len.brown@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 c7727be9719f..ec8797005731 100644 > --- a/tools/power/x86/turbostat/turbostat.c > +++ b/tools/power/x86/turbostat/turbostat.c > @@ -4296,7 +4296,7 @@ int has_hsw_msrs(unsigned int family, unsigned int model) > return 0; > > switch (model) { > - case INTEL_FAM6_HASWELL_CORE: > + case INTEL_FAM6_HASWELL_ULT: > case INTEL_FAM6_BROADWELL_CORE: /* BDW */ > case INTEL_FAM6_SKYLAKE_MOBILE: /* SKL */ > case INTEL_FAM6_CANNONLAKE_MOBILE: /* CNL */ > -- > 2.21.0 diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 75fc4fb..5830552 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -3209,6 +3209,7 @@ int probe_nhm_msrs(unsigned int family, unsigned int model) break; case INTEL_FAM6_HASWELL_CORE: /* HSW */ case INTEL_FAM6_HASWELL_X: /* HSX */ + case INTEL_FAM6_HASWELL_ULT: /* HSW */ case INTEL_FAM6_HASWELL_GT3E: /* HSW */ case INTEL_FAM6_BROADWELL_CORE: /* BDW */ case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ @@ -3405,6 +3406,7 @@ int has_config_tdp(unsigned int family, unsigned int model) case INTEL_FAM6_IVYBRIDGE: /* IVB */ case INTEL_FAM6_HASWELL_CORE: /* HSW */ case INTEL_FAM6_HASWELL_X: /* HSX */ + case INTEL_FAM6_HASWELL_ULT: /* HSW */ case INTEL_FAM6_HASWELL_GT3E: /* HSW */ case INTEL_FAM6_BROADWELL_CORE: /* BDW */ case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ @@ -3841,6 +3843,7 @@ void rapl_probe_intel(unsigned int family, unsigned int model) case INTEL_FAM6_SANDYBRIDGE: case INTEL_FAM6_IVYBRIDGE: case INTEL_FAM6_HASWELL_CORE: /* HSW */ + case INTEL_FAM6_HASWELL_ULT: /* HSW */ case INTEL_FAM6_HASWELL_GT3E: /* HSW */ case INTEL_FAM6_BROADWELL_CORE: /* BDW */ case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ @@ -4032,6 +4035,7 @@ void perf_limit_reasons_probe(unsigned int family, unsigned int model) switch (model) { case INTEL_FAM6_HASWELL_CORE: /* HSW */ + case INTEL_FAM6_HASWELL_ULT: /* HSW */ case INTEL_FAM6_HASWELL_GT3E: /* HSW */ do_gfx_perf_limit_reasons = 1; case INTEL_FAM6_HASWELL_X: /* HSX */ @@ -4251,6 +4255,7 @@ int has_snb_msrs(unsigned int family, unsigned int model) case INTEL_FAM6_IVYBRIDGE_X: /* IVB Xeon */ case INTEL_FAM6_HASWELL_CORE: /* HSW */ case INTEL_FAM6_HASWELL_X: /* HSW */ + case INTEL_FAM6_HASWELL_ULT: /* HSW */ case INTEL_FAM6_HASWELL_GT3E: /* HSW */ case INTEL_FAM6_BROADWELL_CORE: /* BDW */ case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ @@ -4284,7 +4289,7 @@ int has_hsw_msrs(unsigned int family, unsigned int model) return 0; switch (model) { - case INTEL_FAM6_HASWELL_CORE: + case INTEL_FAM6_HASWELL_ULT: /* HSW */ case INTEL_FAM6_BROADWELL_CORE: /* BDW */ case INTEL_FAM6_SKYLAKE_MOBILE: /* SKL */ case INTEL_FAM6_CANNONLAKE_MOBILE: /* CNL */ @@ -4568,9 +4573,6 @@ unsigned int intel_model_duplicates(unsigned int model) case INTEL_FAM6_XEON_PHI_KNM: return INTEL_FAM6_XEON_PHI_KNL; - case INTEL_FAM6_HASWELL_ULT: - return INTEL_FAM6_HASWELL_CORE; - case INTEL_FAM6_BROADWELL_X: case INTEL_FAM6_BROADWELL_XEON_D: /* BDX-DE */ return INTEL_FAM6_BROADWELL_X;
On 6/9/19 11:14 PM, Kosuke Tatsukawa wrote: > Hi, > >> On Haswell (model 0x3C) turbostat fails with >> >> turbostat: cpu0: msr offset 0x630 read failed: Input/output error >> >> because Haswell Core does not have C8-C10. >> >> Output C8-C10 only on Haswell ULT. > > has_hsw_msrs() uses the model number returned by intel_model_duplicates(), > but commit f5a4c76ad7de added code to return INTEL_FAM6_HASWELL_CORE for > INTEL_FAM6_HASWELL_ULT there. So I think Haswell UTL also doesn't > output C8-C10 with your patch. > > Something similar to the below patch is needed to differentiate between > INTEL_FAM6_HASWELL_CORE and INTEL_FAM6_HASWELL_ULT. I don't have a > Haswell-ULT machine, so I've only tested it on a Haswell-DT CPU. > Kosuke, my apologies this was in my junk folder and I just found it. Yes. I agree with your patch below. I will resubmit. OK to add Signed-off-by from you? P. > Best regards. > >> Fixes: f5a4c76ad7de ("tools/power turbostat: consolidate duplicate model >> numbers") >> Cc: Len Brown <len.brown@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 c7727be9719f..ec8797005731 100644 >> --- a/tools/power/x86/turbostat/turbostat.c >> +++ b/tools/power/x86/turbostat/turbostat.c >> @@ -4296,7 +4296,7 @@ int has_hsw_msrs(unsigned int family, unsigned int model) >> return 0; >> >> switch (model) { >> - case INTEL_FAM6_HASWELL_CORE: >> + case INTEL_FAM6_HASWELL_ULT: >> case INTEL_FAM6_BROADWELL_CORE: /* BDW */ >> case INTEL_FAM6_SKYLAKE_MOBILE: /* SKL */ >> case INTEL_FAM6_CANNONLAKE_MOBILE: /* CNL */ >> -- >> 2.21.0 > > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > index 75fc4fb..5830552 100644 > --- a/tools/power/x86/turbostat/turbostat.c > +++ b/tools/power/x86/turbostat/turbostat.c > @@ -3209,6 +3209,7 @@ int probe_nhm_msrs(unsigned int family, unsigned int model) > break; > case INTEL_FAM6_HASWELL_CORE: /* HSW */ > case INTEL_FAM6_HASWELL_X: /* HSX */ > + case INTEL_FAM6_HASWELL_ULT: /* HSW */ > case INTEL_FAM6_HASWELL_GT3E: /* HSW */ > case INTEL_FAM6_BROADWELL_CORE: /* BDW */ > case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ > @@ -3405,6 +3406,7 @@ int has_config_tdp(unsigned int family, unsigned int model) > case INTEL_FAM6_IVYBRIDGE: /* IVB */ > case INTEL_FAM6_HASWELL_CORE: /* HSW */ > case INTEL_FAM6_HASWELL_X: /* HSX */ > + case INTEL_FAM6_HASWELL_ULT: /* HSW */ > case INTEL_FAM6_HASWELL_GT3E: /* HSW */ > case INTEL_FAM6_BROADWELL_CORE: /* BDW */ > case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ > @@ -3841,6 +3843,7 @@ void rapl_probe_intel(unsigned int family, unsigned int model) > case INTEL_FAM6_SANDYBRIDGE: > case INTEL_FAM6_IVYBRIDGE: > case INTEL_FAM6_HASWELL_CORE: /* HSW */ > + case INTEL_FAM6_HASWELL_ULT: /* HSW */ > case INTEL_FAM6_HASWELL_GT3E: /* HSW */ > case INTEL_FAM6_BROADWELL_CORE: /* BDW */ > case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ > @@ -4032,6 +4035,7 @@ void perf_limit_reasons_probe(unsigned int family, unsigned int model) > > switch (model) { > case INTEL_FAM6_HASWELL_CORE: /* HSW */ > + case INTEL_FAM6_HASWELL_ULT: /* HSW */ > case INTEL_FAM6_HASWELL_GT3E: /* HSW */ > do_gfx_perf_limit_reasons = 1; > case INTEL_FAM6_HASWELL_X: /* HSX */ > @@ -4251,6 +4255,7 @@ int has_snb_msrs(unsigned int family, unsigned int model) > case INTEL_FAM6_IVYBRIDGE_X: /* IVB Xeon */ > case INTEL_FAM6_HASWELL_CORE: /* HSW */ > case INTEL_FAM6_HASWELL_X: /* HSW */ > + case INTEL_FAM6_HASWELL_ULT: /* HSW */ > case INTEL_FAM6_HASWELL_GT3E: /* HSW */ > case INTEL_FAM6_BROADWELL_CORE: /* BDW */ > case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ > @@ -4284,7 +4289,7 @@ int has_hsw_msrs(unsigned int family, unsigned int model) > return 0; > > switch (model) { > - case INTEL_FAM6_HASWELL_CORE: > + case INTEL_FAM6_HASWELL_ULT: /* HSW */ > case INTEL_FAM6_BROADWELL_CORE: /* BDW */ > case INTEL_FAM6_SKYLAKE_MOBILE: /* SKL */ > case INTEL_FAM6_CANNONLAKE_MOBILE: /* CNL */ > @@ -4568,9 +4573,6 @@ unsigned int intel_model_duplicates(unsigned int model) > case INTEL_FAM6_XEON_PHI_KNM: > return INTEL_FAM6_XEON_PHI_KNL; > > - case INTEL_FAM6_HASWELL_ULT: > - return INTEL_FAM6_HASWELL_CORE; > - > case INTEL_FAM6_BROADWELL_X: > case INTEL_FAM6_BROADWELL_XEON_D: /* BDX-DE */ > return INTEL_FAM6_BROADWELL_X; >
> On 6/9/19 11:14 PM, Kosuke Tatsukawa wrote: >> Hi, >> >>> On Haswell (model 0x3C) turbostat fails with >>> >>> turbostat: cpu0: msr offset 0x630 read failed: Input/output error >>> >>> because Haswell Core does not have C8-C10. >>> >>> Output C8-C10 only on Haswell ULT. >> >> has_hsw_msrs() uses the model number returned by intel_model_duplicates(), >> but commit f5a4c76ad7de added code to return INTEL_FAM6_HASWELL_CORE for >> INTEL_FAM6_HASWELL_ULT there. So I think Haswell UTL also doesn't >> output C8-C10 with your patch. >> >> Something similar to the below patch is needed to differentiate between >> INTEL_FAM6_HASWELL_CORE and INTEL_FAM6_HASWELL_ULT. I don't have a >> Haswell-ULT machine, so I've only tested it on a Haswell-DT CPU. >> > > Kosuke, my apologies this was in my junk folder and I just found it. > > Yes. I agree with your patch below. I will resubmit. OK to add Signed-off-by > from you? Prarit, yes, that's OK with me. Best regards. > P. > >> Best regards. >> >>> Fixes: f5a4c76ad7de ("tools/power turbostat: consolidate duplicate model >>> numbers") >>> Cc: Len Brown <len.brown@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 c7727be9719f..ec8797005731 100644 >>> --- a/tools/power/x86/turbostat/turbostat.c >>> +++ b/tools/power/x86/turbostat/turbostat.c >>> @@ -4296,7 +4296,7 @@ int has_hsw_msrs(unsigned int family, unsigned int model) >>> return 0; >>> >>> switch (model) { >>> - case INTEL_FAM6_HASWELL_CORE: >>> + case INTEL_FAM6_HASWELL_ULT: >>> case INTEL_FAM6_BROADWELL_CORE: /* BDW */ >>> case INTEL_FAM6_SKYLAKE_MOBILE: /* SKL */ >>> case INTEL_FAM6_CANNONLAKE_MOBILE: /* CNL */ >>> -- >>> 2.21.0 >> >> >> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c >> index 75fc4fb..5830552 100644 >> --- a/tools/power/x86/turbostat/turbostat.c >> +++ b/tools/power/x86/turbostat/turbostat.c >> @@ -3209,6 +3209,7 @@ int probe_nhm_msrs(unsigned int family, unsigned int model) >> break; >> case INTEL_FAM6_HASWELL_CORE: /* HSW */ >> case INTEL_FAM6_HASWELL_X: /* HSX */ >> + case INTEL_FAM6_HASWELL_ULT: /* HSW */ >> case INTEL_FAM6_HASWELL_GT3E: /* HSW */ >> case INTEL_FAM6_BROADWELL_CORE: /* BDW */ >> case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ >> @@ -3405,6 +3406,7 @@ int has_config_tdp(unsigned int family, unsigned int model) >> case INTEL_FAM6_IVYBRIDGE: /* IVB */ >> case INTEL_FAM6_HASWELL_CORE: /* HSW */ >> case INTEL_FAM6_HASWELL_X: /* HSX */ >> + case INTEL_FAM6_HASWELL_ULT: /* HSW */ >> case INTEL_FAM6_HASWELL_GT3E: /* HSW */ >> case INTEL_FAM6_BROADWELL_CORE: /* BDW */ >> case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ >> @@ -3841,6 +3843,7 @@ void rapl_probe_intel(unsigned int family, unsigned int model) >> case INTEL_FAM6_SANDYBRIDGE: >> case INTEL_FAM6_IVYBRIDGE: >> case INTEL_FAM6_HASWELL_CORE: /* HSW */ >> + case INTEL_FAM6_HASWELL_ULT: /* HSW */ >> case INTEL_FAM6_HASWELL_GT3E: /* HSW */ >> case INTEL_FAM6_BROADWELL_CORE: /* BDW */ >> case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ >> @@ -4032,6 +4035,7 @@ void perf_limit_reasons_probe(unsigned int family, unsigned int model) >> >> switch (model) { >> case INTEL_FAM6_HASWELL_CORE: /* HSW */ >> + case INTEL_FAM6_HASWELL_ULT: /* HSW */ >> case INTEL_FAM6_HASWELL_GT3E: /* HSW */ >> do_gfx_perf_limit_reasons = 1; >> case INTEL_FAM6_HASWELL_X: /* HSX */ >> @@ -4251,6 +4255,7 @@ int has_snb_msrs(unsigned int family, unsigned int model) >> case INTEL_FAM6_IVYBRIDGE_X: /* IVB Xeon */ >> case INTEL_FAM6_HASWELL_CORE: /* HSW */ >> case INTEL_FAM6_HASWELL_X: /* HSW */ >> + case INTEL_FAM6_HASWELL_ULT: /* HSW */ >> case INTEL_FAM6_HASWELL_GT3E: /* HSW */ >> case INTEL_FAM6_BROADWELL_CORE: /* BDW */ >> case INTEL_FAM6_BROADWELL_GT3E: /* BDW */ >> @@ -4284,7 +4289,7 @@ int has_hsw_msrs(unsigned int family, unsigned int model) >> return 0; >> >> switch (model) { >> - case INTEL_FAM6_HASWELL_CORE: >> + case INTEL_FAM6_HASWELL_ULT: /* HSW */ >> case INTEL_FAM6_BROADWELL_CORE: /* BDW */ >> case INTEL_FAM6_SKYLAKE_MOBILE: /* SKL */ >> case INTEL_FAM6_CANNONLAKE_MOBILE: /* CNL */ >> @@ -4568,9 +4573,6 @@ unsigned int intel_model_duplicates(unsigned int model) >> case INTEL_FAM6_XEON_PHI_KNM: >> return INTEL_FAM6_XEON_PHI_KNL; >> >> - case INTEL_FAM6_HASWELL_ULT: >> - return INTEL_FAM6_HASWELL_CORE; >> - >> case INTEL_FAM6_BROADWELL_X: >> case INTEL_FAM6_BROADWELL_XEON_D: /* BDX-DE */ >> return INTEL_FAM6_BROADWELL_X; >> --- Kosuke TATSUKAWA | 1st Platform Software Division | NEC Solution Innovators | tatsu@ab.jp.nec.com
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index c7727be9719f..ec8797005731 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -4296,7 +4296,7 @@ int has_hsw_msrs(unsigned int family, unsigned int model) return 0; switch (model) { - case INTEL_FAM6_HASWELL_CORE: + case INTEL_FAM6_HASWELL_ULT: case INTEL_FAM6_BROADWELL_CORE: /* BDW */ case INTEL_FAM6_SKYLAKE_MOBILE: /* SKL */ case INTEL_FAM6_CANNONLAKE_MOBILE: /* CNL */
On Haswell (model 0x3C) turbostat fails with turbostat: cpu0: msr offset 0x630 read failed: Input/output error because Haswell Core does not have C8-C10. Output C8-C10 only on Haswell ULT. Fixes: f5a4c76ad7de ("tools/power turbostat: consolidate duplicate model numbers") Cc: Len Brown <len.brown@intel.com> --- tools/power/x86/turbostat/turbostat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)