Re: [PATCH v4 06/11][RESEND] x86/intel_pstate: APERF/MPERF feature detect

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 29/06/15 07:42, Wei Wang wrote:
> Add support to detect the APERF/MPERF feature. Also, remove the identical
> code in cpufreq.c and powernow.c. This patch is independent of the
> earlier patches.
>
> Resend changes:
> 1) defined macros for 0x1 and CPUID leaf5;
> 2) added a statement stating that this patch is independent of the
> previous ones.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
>  xen/arch/x86/acpi/cpufreq/cpufreq.c  | 6 ++----
>  xen/arch/x86/acpi/cpufreq/powernow.c | 6 ++----
>  xen/arch/x86/cpu/common.c            | 4 ++++
>  xen/include/asm-x86/cpufeature.h     | 4 ++++
>  4 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> index fa3678d..643c405 100644
> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> @@ -51,7 +51,6 @@ enum {
>  };
>  
>  #define INTEL_MSR_RANGE         (0xffffull)
> -#define CPUID_6_ECX_APERFMPERF_CAPABILITY       (0x1)
>  
>  struct acpi_cpufreq_data *cpufreq_drv_data[NR_CPUS];
>  
> @@ -352,10 +351,9 @@ static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
>  static void feature_detect(void *info)
>  {
>      struct cpufreq_policy *policy = info;
> -    unsigned int eax, ecx;
> +    unsigned int eax;
>  
> -    ecx = cpuid_ecx(6);
> -    if (ecx & CPUID_6_ECX_APERFMPERF_CAPABILITY) {
> +    if (boot_cpu_has(X86_FEATURE_APERFMPERF)) {
>          policy->aperf_mperf = 1;
>          acpi_cpufreq_driver.getavg = get_measured_perf;
>      }
> diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c
> index 2c9fea2..b5b752c 100644
> --- a/xen/arch/x86/acpi/cpufreq/powernow.c
> +++ b/xen/arch/x86/acpi/cpufreq/powernow.c
> @@ -38,7 +38,6 @@
>  #include <acpi/acpi.h>
>  #include <acpi/cpufreq/cpufreq.h>
>  
> -#define CPUID_6_ECX_APERFMPERF_CAPABILITY       (0x1)
>  #define CPUID_FREQ_VOLT_CAPABILITIES    0x80000007
>  #define CPB_CAPABLE             0x00000200
>  #define USE_HW_PSTATE           0x00000080
> @@ -212,10 +211,9 @@ static int powernow_cpufreq_verify(struct cpufreq_policy *policy)
>  static void feature_detect(void *info)
>  {
>      struct cpufreq_policy *policy = info;
> -    unsigned int ecx, edx;
> +    unsigned int edx;
>  
> -    ecx = cpuid_ecx(6);
> -    if (ecx & CPUID_6_ECX_APERFMPERF_CAPABILITY) {
> +    if (boot_cpu_has(X86_FEATURE_APERFMPERF)) {
>          policy->aperf_mperf = 1;
>          powernow_cpufreq_driver.getavg = get_measured_perf;
>      }
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index e105aeb..d2272bc 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -238,6 +238,10 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
>  	if ( cpu_has(c, X86_FEATURE_CLFLSH) )
>  		c->x86_clflush_size = ((ebx >> 8) & 0xff) * 8;
>  
> +	if ((c->cpuid_level > CPUID_PM_LEAF) &&

Xen bracket style please (as visible in the leading and trailing context).

> +		(cpuid_ecx(CPUID_PM_LEAF) & CPUID6_ECX_APERFMPERF_CAPABILITY))
> +		set_bit(X86_FEATURE_APERFMPERF, c->x86_capability);
> +
>  	/* AMD-defined flags: level 0x80000001 */
>  	c->extended_cpuid_level = cpuid_eax(0x80000000);
>  	if ( (c->extended_cpuid_level & 0xffff0000) == 0x80000000 ) {
> diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
> index 7963a3a..530256b 100644
> --- a/xen/include/asm-x86/cpufeature.h
> +++ b/xen/include/asm-x86/cpufeature.h
> @@ -69,6 +69,7 @@
>  #define X86_FEATURE_XTOPOLOGY    (3*32+13) /* cpu topology enum extensions */
>  #define X86_FEATURE_CPUID_FAULTING (3*32+14) /* cpuid faulting */
>  #define X86_FEATURE_CLFLUSH_MONITOR (3*32+15) /* clflush reqd with monitor */
> +#define X86_FEATURE_APERFMPERF (3*32+28) /* APERFMPERF */
>  
>  /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
>  #define X86_FEATURE_XMM3	(4*32+ 0) /* Streaming SIMD Extensions-3 */
> @@ -164,6 +165,9 @@
>  #define CPUID5_ECX_EXTENSIONS_SUPPORTED 0x1
>  #define CPUID5_ECX_INTERRUPT_BREAK      0x2
>  
> +#define CPUID_PM_LEAF                     6
> +#define CPUID6_ECX_APERFMPERF_CAPABILITY 0x1
> +
>  #define cpu_has_vme		0
>  #define cpu_has_de		1
>  #define cpu_has_pse		1

Please introduce "cpu_has_aperfmperf" and use it.

~Andrew