RE: [Xen-devel] [PATCH] [retry 1] 1/3 Refactor Xen support for Intel Turbo boost

["Yu, Ke" <ke.yu@xxxxxxxxx>]

Overall It looks fine. Still some small comments.

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx [mailto:xen-devel-
> bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Mark Langsdorf
> Sent: Wednesday, March 31, 2010 8:25 AM
> To: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: [Xen-devel] [PATCH] [retry 1] 1/3 Refactor Xen support for Intel
> Turbo boost
> 
> diff -r ebd84be3420a -r 9da598418e6d
> xen/arch/x86/acpi/cpufreq/cpufreq.c
> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c     Tue Mar 30 18:31:39 2010
> +0100
> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c     Tue Mar 30 23:49:53
> 2010 -0500
> @@ -410,6 +410,10 @@
>          return -ENODEV;
>      }
> 
> +    if (policy->turbo == -1)
> +        if (target_freq > policy->cpuinfo.second_max_freq)
> +            target_freq = policy->cpuinfo.second_max_freq;
> +

use CPUFREQ_TURBO_DISABLED for better readability.

>      perf = data->acpi_data;
>      result = cpufreq_frequency_table_target(policy,
>                                              data->freq_table,
> @@ -610,12 +614,19 @@
>          break;
>      }
> 
> -    /* Check for APERF/MPERF support in hardware */
> +    /* Check for APERF/MPERF support in hardware
> +     * also check for boost support */
>      if (c->x86_vendor == X86_VENDOR_INTEL && c->cpuid_level >= 6) {
>          unsigned int ecx;
> +        unsigned int eax;
>          ecx = cpuid_ecx(6);
>          if (ecx & CPUID_6_ECX_APERFMPERF_CAPABILITY)
>              acpi_cpufreq_driver.getavg = get_measured_perf;
> +        eax = cpuid_eax(6);
> +        if ( eax & 0x2 ) {
> +            policy->turbo = 1;
> +            printk(XENLOG_INFO "Turbo Mode detected and enabled!\n");
> +        }
>      }

Use CPUFREQ_TURBO_ENABLED

> diff -r ebd84be3420a -r 9da598418e6d
> xen/include/acpi/cpufreq/processor_perf.h
> --- a/xen/include/acpi/cpufreq/processor_perf.h       Tue Mar 30 18:31:39
> 2010 +0100
> +++ b/xen/include/acpi/cpufreq/processor_perf.h       Tue Mar 30 23:49:53
> 2010 -0500
> @@ -9,6 +9,7 @@
>  int get_cpu_id(u8);
>  int powernow_cpufreq_init(void);
>  unsigned int powernow_register_driver(void);
> +unsigned int get_measured_perf(unsigned int cpu, unsigned int flag);
> 
>  void cpufreq_residency_update(unsigned int, uint8_t);
>  void cpufreq_statistic_update(unsigned int, uint8_t, uint8_t);
> diff -r ebd84be3420a -r 9da598418e6d xen/include/public/sysctl.h
> --- a/xen/include/public/sysctl.h     Tue Mar 30 18:31:39 2010 +0100
> +++ b/xen/include/public/sysctl.h     Tue Mar 30 23:49:53 2010 -0500
> @@ -298,7 +298,6 @@
> 
>      uint32_t sampling_rate;
>      uint32_t up_threshold;
> -    uint32_t turbo_enabled;
>  };
>  typedef struct xen_ondemand xen_ondemand_t;
> 
> @@ -328,6 +327,7 @@
>      char scaling_governor[CPUFREQ_NAME_LEN];
>      uint32_t scaling_max_freq;
>      uint32_t scaling_min_freq;
> +    int32_t turbo_enabled;
> 
>      /* for specific governor */
>      union {
> 

I would like to change this as following, to keep hypercall interface backward 
compatible.  And adding padding allows future extension.

diff -r 91232efdcfdc xen/include/public/sysctl.h
--- a/xen/include/public/sysctl.h       Tue Mar 30 08:36:41 2010 +0100
+++ b/xen/include/public/sysctl.h       Wed Mar 31 15:58:48 2010 +0800
@@ -333,7 +333,10 @@ struct xen_get_cpufreq_para {
     union {
         struct  xen_userspace userspace;
         struct  xen_ondemand ondemand;
+        uint8_t pad[32];
     } u;
+
+    int32_t turbo_enabled;
 };

And you may want to change "struct xc_get_cpufreq_para" as well.

Best Regards
Ke

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel