From: "Odzioba, Lukasz" <lukasz.odzioba@intel.com>
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>, "hpa@zytor.com" <hpa@zytor.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "bp@suse.de" <bp@suse.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "Liang, Kan" <kan.liang@intel.com>
Subject: RE: [PATCH 1/1] x86/perf/intel/cstate: add C-state residency events
 for Knights Landing
Thread-Topic: [PATCH 1/1] x86/perf/intel/cstate: add C-state residency
 events for Knights Landing
Thread-Index: AQHSHlwYAveGdmj9c0S7V3awLX0OqKCvp4mQ
Date: Wed, 19 Oct 2016 10:27:24 +0000
Message-ID: <D6EDEBF1F91015459DB866AC4EE162CC02499F87@IRSMSX103.ger.corp.intel.com>
References: <1475598386-19597-1-git-send-email-lukasz.odzioba@intel.com>
In-Reply-To: <1475598386-19597-1-git-send-email-lukasz.odzioba@intel.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3701
Lines: 100

On Tuesday, October 4, 2016 6:26 PM Odzioba, Lukasz wrote:
> Although KNL does support C1,C6,PC2,PC3,PC6 states, the patch only
> supports C6,PC2,PC3,PC6, because there is no counter for C1.
> C6 residency counter MSR on KNL has a different address than other
> platforms which is handled as a new quirk flag.
> 
> Signed-off-by: Lukasz Odzioba <lukasz.odzioba@intel.com>
> ---
> arch/x86/events/intel/cstate.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/events/intel/cstate.c b/arch/x86/events/intel/cstate.c
> index 3ca87b5..4f5ac72 100644
> --- a/arch/x86/events/intel/cstate.c
> +++ b/arch/x86/events/intel/cstate.c
> @@ -48,7 +48,8 @@
>  *			       Scope: Core
>  *	MSR_CORE_C6_RESIDENCY: CORE C6 Residency Counter
>  *			       perf code: 0x02
> - *			       Available model: SLM,AMT,NHM,WSM,SNB,IVB,HSW,BDW,SKL
> + *			       Available model: SLM,AMT,NHM,WSM,SNB,IVB,HSW,BDW
> + *						SKL,KNL
>   *			       Scope: Core
>   *	MSR_CORE_C7_RESIDENCY: CORE C7 Residency Counter
>   *			       perf code: 0x03
> @@ -56,15 +57,16 @@
>   *			       Scope: Core
>   *	MSR_PKG_C2_RESIDENCY:  Package C2 Residency Counter.
>   *			       perf code: 0x00
> - *			       Available model: SNB,IVB,HSW,BDW,SKL
> + *			       Available model: SNB,IVB,HSW,BDW,SKL,KNL
>   *			       Scope: Package (physical package)
>   *	MSR_PKG_C3_RESIDENCY:  Package C3 Residency Counter.
>   *			       perf code: 0x01
> - *			       Available model: NHM,WSM,SNB,IVB,HSW,BDW,SKL
> + *			       Available model: NHM,WSM,SNB,IVB,HSW,BDW,SKL,KNL
>   *			       Scope: Package (physical package)
>   *	MSR_PKG_C6_RESIDENCY:  Package C6 Residency Counter.
>   *			       perf code: 0x02
> - *			       Available model: SLM,AMT,NHM,WSM,SNB,IVB,HSW,BDW,SKL
> + *			       Available model: SLM,AMT,NHM,WSM,SNB,IVB,HSW,BDW
> + *						SKL,KNL
>   *			       Scope: Package (physical package)
>  *	MSR_PKG_C7_RESIDENCY:  Package C7 Residency Counter.
>   *			       perf code: 0x03
> @@ -118,6 +120,7 @@ struct cstate_model {
>  
>  /* Quirk flags */
>  #define SLM_PKG_C6_USE_C7_MSR	(1UL << 0)
> +#define KNL_CORE_C6_MSR		(1UL << 1)
>  
>  struct perf_cstate_msr {
>  	u64	msr;
> @@ -488,6 +491,18 @@ static const struct cstate_model slm_cstates __initconst = {
>  	.quirks			= SLM_PKG_C6_USE_C7_MSR,
>  };
>  
> +
> +static const struct cstate_model knl_cstates __initconst = {
> +	.core_events		= BIT(PERF_CSTATE_CORE_C6_RES),
> +
> +	.pkg_events		= BIT(PERF_CSTATE_PKG_C2_RES) |
> +				  BIT(PERF_CSTATE_PKG_C3_RES) |
> +				  BIT(PERF_CSTATE_PKG_C6_RES),
> +	.quirks			= KNL_CORE_C6_MSR,
> +};
> +
> +
> +
>  #define X86_CSTATES_MODEL(model, states)				\
>  	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long) &(states) }
>  
> @@ -523,6 +538,8 @@ static const struct x86_cpu_id intel_cstates_match[] __initconst = {
>  
>  	X86_CSTATES_MODEL(INTEL_FAM6_SKYLAKE_MOBILE,  snb_cstates),
>  	X86_CSTATES_MODEL(INTEL_FAM6_SKYLAKE_DESKTOP, snb_cstates),
> +
> +	X86_CSTATES_MODEL(INTEL_FAM6_XEON_PHI_KNL, knl_cstates),
> 	{ },
> };
>  MODULE_DEVICE_TABLE(x86cpu, intel_cstates_match);
> @@ -558,6 +575,11 @@ static int __init cstate_probe(const struct cstate_model *cm)
>  	if (cm->quirks & SLM_PKG_C6_USE_C7_MSR)
>  		pkg_msr[PERF_CSTATE_PKG_C6_RES].msr = MSR_PKG_C7_RESIDENCY;
>  
> +	/* KNL has different MSR for CORE C6 */
> +	if (cm->quirks & KNL_CORE_C6_MSR)
> +		pkg_msr[PERF_CSTATE_CORE_C6_RES].msr = MSR_KNL_CORE_C6_RESIDENCY;
> +
> +
>  	has_cstate_core = cstate_probe_msr(cm->core_events,
>  					   PERF_CSTATE_CORE_EVENT_MAX,
>  					   core_msr, core_events_attrs);
> -- 
> 1.8.3.1

Any comments would be appreciated  :)

Thanks,
Lukas