Re: [PATCH] tools/power/turbostat: Remove Package C6 Retention on Ice Lake Server

[Kin Cho <kin.cho@oracle.com>]

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);