| Message ID | 20210504145234.4103405-1-artem.bityutskiy@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | tools/power/turbostat: Remove Package C6 Retention on Ice Lake Server | expand |
Hi, This patch (25368d7cefcd87a94ccabcc6f9f31796607bbe4e) has affected Sapphire Rapids support. Specifically Pkg%pc2 and Pkg%pc6 are now missing on Sapphire Rapids (with package C6 retention). See below for a diff of turbostat --list output with and without 25368d7. -kin --- turbostat-0304_115100-v5.12-10-g25368d7cefcd 2022-03-04 11:51:00.184727582 -0800 +++ turbostat-0304_124741-v5.12-10-g25368d7cefcd--25368d7 2022-03-04 12:47:41.543801798 -0800 @@ -1,2 +1,2 @@ turbostat version 21.03.12 - Len Brown <lenb@kernel.org> -usec,Time_Of_Day_Seconds,Package,Core,CPU,APIC,X2APIC,Avg_MHz,Busy%,Bzy_MHz,TSC_MHz,IPC,IRQ,SMI,POLL,C1ACPI,C2ACPI,POLL%,C1ACPI%,C2ACPI%,CPU%c1,CPU%c6,CoreTmp,PkgTmp,PkgWatt,RAMWatt,PKG_%,RAM_% +usec,Time_Of_Day_Seconds,Package,Core,CPU,APIC,X2APIC,Avg_MHz,Busy%,Bzy_MHz,TSC_MHz,IPC,IRQ,SMI,POLL,C1ACPI,C2ACPI,POLL%,C1ACPI%,C2ACPI%,CPU%c1,CPU%c6,CoreTmp,PkgTmp,Pkg%pc2,Pkg%pc6,PkgWatt,RAMWatt,PKG_%,RAM_% On 5/4/21 7:52 AM, Artem Bityutskiy wrote: > From: Chen Yu <yu.c.chen@intel.com> > > Currently the turbostat treats ICX the same way as SKX and shares the > code among them. But one difference is that ICX does not support Package > C6 Retention, unlike SKX and CLX. > > So this patch: > > 1. Splitting SKX and ICX in turbostat. > 2. Removing Package C6 Rentention for ICX. > > And after this split, it would be easier to cutomize Ice Lake Server > in turbostat in the future. > > Suggested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > Signed-off-by: Chen Yu <yu.c.chen@intel.com> > Reviewed-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > Tested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > --- > tools/power/x86/turbostat/turbostat.c | 36 ++++++++++++++++++++++----- > 1 file changed, 30 insertions(+), 6 deletions(-) > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > index ace100dd5a83..81afaff81f43 100644 > --- a/tools/power/x86/turbostat/turbostat.c > +++ b/tools/power/x86/turbostat/turbostat.c > @@ -2259,7 +2259,7 @@ int amt_pkg_cstate_limits[16] = {PCLUNL, PCL__1, PCL__2, PCLRSV, PCLRSV, PCLRSV, > int phi_pkg_cstate_limits[16] = {PCL__0, PCL__2, PCL_6N, PCL_6R, PCLRSV, PCLRSV, PCLRSV, PCLUNL, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV}; > int glm_pkg_cstate_limits[16] = {PCLUNL, PCL__1, PCL__3, PCL__6, PCL__7, PCL_7S, PCL__8, PCL__9, PCL_10, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV}; > int skx_pkg_cstate_limits[16] = {PCL__0, PCL__2, PCL_6N, PCL_6R, PCLRSV, PCLRSV, PCLRSV, PCLUNL, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV}; > - > +int icx_pkg_cstate_limits[16] = {PCL__0, PCL__2, PCL__6, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLUNL, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV}; > > static void > calculate_tsc_tweak() > @@ -2374,6 +2374,7 @@ int has_turbo_ratio_group_limits(int family, int model) > switch (model) { > case INTEL_FAM6_ATOM_GOLDMONT: > case INTEL_FAM6_SKYLAKE_X: > + case INTEL_FAM6_ICELAKE_X: > case INTEL_FAM6_ATOM_GOLDMONT_D: > case INTEL_FAM6_ATOM_TREMONT_D: > return 1; > @@ -3613,6 +3614,10 @@ int probe_nhm_msrs(unsigned int family, unsigned int model) > pkg_cstate_limits = skx_pkg_cstate_limits; > has_misc_feature_control = 1; > break; > + case INTEL_FAM6_ICELAKE_X: /* ICX */ > + pkg_cstate_limits = icx_pkg_cstate_limits; > + has_misc_feature_control = 1; > + break; > case INTEL_FAM6_ATOM_SILVERMONT: /* BYT */ > no_MSR_MISC_PWR_MGMT = 1; > case INTEL_FAM6_ATOM_SILVERMONT_D: /* AVN */ > @@ -3701,6 +3706,20 @@ int is_skx(unsigned int family, unsigned int model) > } > return 0; > } > + > +int is_icx(unsigned int family, unsigned int model) > +{ > + > + if (!genuine_intel) > + return 0; > + > + switch (model) { > + case INTEL_FAM6_ICELAKE_X: > + return 1; > + } > + return 0; > +} > + > int is_ehl(unsigned int family, unsigned int model) > { > if (!genuine_intel) > @@ -3803,6 +3822,7 @@ int has_glm_turbo_ratio_limit(unsigned int family, unsigned int model) > switch (model) { > case INTEL_FAM6_ATOM_GOLDMONT: > case INTEL_FAM6_SKYLAKE_X: > + case INTEL_FAM6_ICELAKE_X: > return 1; > default: > return 0; > @@ -3828,6 +3848,7 @@ int has_config_tdp(unsigned int family, unsigned int model) > case INTEL_FAM6_SKYLAKE_L: /* SKL */ > case INTEL_FAM6_CANNONLAKE_L: /* CNL */ > case INTEL_FAM6_SKYLAKE_X: /* SKX */ > + case INTEL_FAM6_ICELAKE_X: /* ICX */ > > case INTEL_FAM6_XEON_PHI_KNL: /* Knights Landing */ > return 1; > @@ -4358,6 +4379,7 @@ void rapl_probe_intel(unsigned int family, unsigned int model) > case INTEL_FAM6_HASWELL_X: /* HSX */ > case INTEL_FAM6_BROADWELL_X: /* BDX */ > case INTEL_FAM6_SKYLAKE_X: /* SKX */ > + case INTEL_FAM6_ICELAKE_X: /* ICX */ > case INTEL_FAM6_XEON_PHI_KNL: /* KNL */ > do_rapl = RAPL_PKG | RAPL_DRAM | RAPL_DRAM_POWER_INFO | RAPL_DRAM_PERF_STATUS | RAPL_PKG_PERF_STATUS | RAPL_PKG_POWER_INFO; > BIC_PRESENT(BIC_PKG__); > @@ -4514,7 +4536,8 @@ void perf_limit_reasons_probe(unsigned int family, unsigned int model) > > void automatic_cstate_conversion_probe(unsigned int family, unsigned int model) > { > - if (is_skx(family, model) || is_bdx(family, model)) > + if (is_skx(family, model) || is_bdx(family, model) || > + is_icx(family, model)) > has_automatic_cstate_conversion = 1; > } > > @@ -4729,6 +4752,7 @@ int has_snb_msrs(unsigned int family, unsigned int model) > case INTEL_FAM6_SKYLAKE_L: /* SKL */ > case INTEL_FAM6_CANNONLAKE_L: /* CNL */ > case INTEL_FAM6_SKYLAKE_X: /* SKX */ > + case INTEL_FAM6_ICELAKE_X: /* ICX */ > case INTEL_FAM6_ATOM_GOLDMONT: /* BXT */ > case INTEL_FAM6_ATOM_GOLDMONT_PLUS: > case INTEL_FAM6_ATOM_GOLDMONT_D: /* DNV */ > @@ -5067,10 +5091,9 @@ unsigned int intel_model_duplicates(unsigned int model) > case INTEL_FAM6_ATOM_TREMONT_L: > return INTEL_FAM6_ATOM_TREMONT; > > - case INTEL_FAM6_ICELAKE_X: > case INTEL_FAM6_ICELAKE_D: > case INTEL_FAM6_SAPPHIRERAPIDS_X: > - return INTEL_FAM6_SKYLAKE_X; > + return INTEL_FAM6_ICELAKE_X; > } > return model; > } > @@ -5180,8 +5203,9 @@ void process_cpuid() > edx_flags & (1 << 28) ? "HT" : "-", > edx_flags & (1 << 29) ? "TM" : "-"); > } > - if (genuine_intel) > + if (genuine_intel) { > model = intel_model_duplicates(model); > + } > > if (!(edx_flags & (1 << 5))) > errx(1, "CPUID: no MSR"); > @@ -5359,7 +5383,7 @@ void process_cpuid() > BIC_NOT_PRESENT(BIC_Pkgpc7); > use_c1_residency_msr = 1; > } > - if (is_skx(family, model)) { > + if (is_skx(family, model) || is_icx(family, model)) { > BIC_NOT_PRESENT(BIC_CPU_c3); > BIC_NOT_PRESENT(BIC_Pkgpc3); > BIC_NOT_PRESENT(BIC_CPU_c7);
Hi Artem, Chen, The patch "turbostat: fix PC6 displaying on some systems" fixes the issue on our Sapphire Rapids system. On this system I also checked MAX_PKG_C_STATE, and it is 3. Thanks, -kin On 3/6/22 6:35 AM, Artem Bityutskiy wrote: > On Sat, 2022-03-05 at 17:20 +0800, Chen Yu wrote: >> Hi Cho, >> On Fri, Mar 04, 2022 at 01:35:17PM -0800, Kin Cho wrote: >>> Hi, >>> >>> This patch (25368d7cefcd87a94ccabcc6f9f31796607bbe4e) has affected >>> Sapphire >>> Rapids support. >>> Specifically Pkg%pc2 and Pkg%pc6 are now missing on Sapphire Rapids >>> (with >>> package C6 retention). >>> See below for a diff of turbostat --list output with and without >>> 25368d7. >>> >> My guess is that the max limited package c-state exposed might be '3' >> rather than '2'. >> Artem has previously found this issue on ICX and there is a patch: >> https://patchwork.kernel.org/project/linux-pm/patch/20211004105224.3145916-1-dedekind1@gmail.com/ >> <https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-pm/patch/20211004105224.3145916-1-dedekind1@gmail.com/__;!!ACWV5N9M2RV99hQ!c44scI9uwc_oZRla1MmeuL9IJVimON-bjStsp8_FIh3icFJoju9aMrt6LpMCcA$> >> would you please help check if that helps? > > Yes, this patch fixes the problem. > > Rafael, would you please take a look and possibly pick it? Thanks! > > Artem. >
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index ace100dd5a83..81afaff81f43 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -2259,7 +2259,7 @@ int amt_pkg_cstate_limits[16] = {PCLUNL, PCL__1, PCL__2, PCLRSV, PCLRSV, PCLRSV, int phi_pkg_cstate_limits[16] = {PCL__0, PCL__2, PCL_6N, PCL_6R, PCLRSV, PCLRSV, PCLRSV, PCLUNL, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV}; int glm_pkg_cstate_limits[16] = {PCLUNL, PCL__1, PCL__3, PCL__6, PCL__7, PCL_7S, PCL__8, PCL__9, PCL_10, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV}; int skx_pkg_cstate_limits[16] = {PCL__0, PCL__2, PCL_6N, PCL_6R, PCLRSV, PCLRSV, PCLRSV, PCLUNL, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV}; - +int icx_pkg_cstate_limits[16] = {PCL__0, PCL__2, PCL__6, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLUNL, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV, PCLRSV}; static void calculate_tsc_tweak() @@ -2374,6 +2374,7 @@ int has_turbo_ratio_group_limits(int family, int model) switch (model) { case INTEL_FAM6_ATOM_GOLDMONT: case INTEL_FAM6_SKYLAKE_X: + case INTEL_FAM6_ICELAKE_X: case INTEL_FAM6_ATOM_GOLDMONT_D: case INTEL_FAM6_ATOM_TREMONT_D: return 1; @@ -3613,6 +3614,10 @@ int probe_nhm_msrs(unsigned int family, unsigned int model) pkg_cstate_limits = skx_pkg_cstate_limits; has_misc_feature_control = 1; break; + case INTEL_FAM6_ICELAKE_X: /* ICX */ + pkg_cstate_limits = icx_pkg_cstate_limits; + has_misc_feature_control = 1; + break; case INTEL_FAM6_ATOM_SILVERMONT: /* BYT */ no_MSR_MISC_PWR_MGMT = 1; case INTEL_FAM6_ATOM_SILVERMONT_D: /* AVN */ @@ -3701,6 +3706,20 @@ int is_skx(unsigned int family, unsigned int model) } return 0; } + +int is_icx(unsigned int family, unsigned int model) +{ + + if (!genuine_intel) + return 0; + + switch (model) { + case INTEL_FAM6_ICELAKE_X: + return 1; + } + return 0; +} + int is_ehl(unsigned int family, unsigned int model) { if (!genuine_intel) @@ -3803,6 +3822,7 @@ int has_glm_turbo_ratio_limit(unsigned int family, unsigned int model) switch (model) { case INTEL_FAM6_ATOM_GOLDMONT: case INTEL_FAM6_SKYLAKE_X: + case INTEL_FAM6_ICELAKE_X: return 1; default: return 0; @@ -3828,6 +3848,7 @@ int has_config_tdp(unsigned int family, unsigned int model) case INTEL_FAM6_SKYLAKE_L: /* SKL */ case INTEL_FAM6_CANNONLAKE_L: /* CNL */ case INTEL_FAM6_SKYLAKE_X: /* SKX */ + case INTEL_FAM6_ICELAKE_X: /* ICX */ case INTEL_FAM6_XEON_PHI_KNL: /* Knights Landing */ return 1; @@ -4358,6 +4379,7 @@ void rapl_probe_intel(unsigned int family, unsigned int model) case INTEL_FAM6_HASWELL_X: /* HSX */ case INTEL_FAM6_BROADWELL_X: /* BDX */ case INTEL_FAM6_SKYLAKE_X: /* SKX */ + case INTEL_FAM6_ICELAKE_X: /* ICX */ case INTEL_FAM6_XEON_PHI_KNL: /* KNL */ do_rapl = RAPL_PKG | RAPL_DRAM | RAPL_DRAM_POWER_INFO | RAPL_DRAM_PERF_STATUS | RAPL_PKG_PERF_STATUS | RAPL_PKG_POWER_INFO; BIC_PRESENT(BIC_PKG__); @@ -4514,7 +4536,8 @@ void perf_limit_reasons_probe(unsigned int family, unsigned int model) void automatic_cstate_conversion_probe(unsigned int family, unsigned int model) { - if (is_skx(family, model) || is_bdx(family, model)) + if (is_skx(family, model) || is_bdx(family, model) || + is_icx(family, model)) has_automatic_cstate_conversion = 1; } @@ -4729,6 +4752,7 @@ int has_snb_msrs(unsigned int family, unsigned int model) case INTEL_FAM6_SKYLAKE_L: /* SKL */ case INTEL_FAM6_CANNONLAKE_L: /* CNL */ case INTEL_FAM6_SKYLAKE_X: /* SKX */ + case INTEL_FAM6_ICELAKE_X: /* ICX */ case INTEL_FAM6_ATOM_GOLDMONT: /* BXT */ case INTEL_FAM6_ATOM_GOLDMONT_PLUS: case INTEL_FAM6_ATOM_GOLDMONT_D: /* DNV */ @@ -5067,10 +5091,9 @@ unsigned int intel_model_duplicates(unsigned int model) case INTEL_FAM6_ATOM_TREMONT_L: return INTEL_FAM6_ATOM_TREMONT; - case INTEL_FAM6_ICELAKE_X: case INTEL_FAM6_ICELAKE_D: case INTEL_FAM6_SAPPHIRERAPIDS_X: - return INTEL_FAM6_SKYLAKE_X; + return INTEL_FAM6_ICELAKE_X; } return model; } @@ -5180,8 +5203,9 @@ void process_cpuid() edx_flags & (1 << 28) ? "HT" : "-", edx_flags & (1 << 29) ? "TM" : "-"); } - if (genuine_intel) + if (genuine_intel) { model = intel_model_duplicates(model); + } if (!(edx_flags & (1 << 5))) errx(1, "CPUID: no MSR"); @@ -5359,7 +5383,7 @@ void process_cpuid() BIC_NOT_PRESENT(BIC_Pkgpc7); use_c1_residency_msr = 1; } - if (is_skx(family, model)) { + if (is_skx(family, model) || is_icx(family, model)) { BIC_NOT_PRESENT(BIC_CPU_c3); BIC_NOT_PRESENT(BIC_Pkgpc3); BIC_NOT_PRESENT(BIC_CPU_c7);