Re: [Xen-devel] [RFC PATCH 04/31] cpufreq: make turbo settings to be configurable

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 9 Nov 2017, Oleksandr Tyshchenko wrote:
> From: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>
> 
> This settings is not needed for some architectures.
> So make it to be configurable and use it for x86
> architecture.
> 
> This is a rebased version of the original patch:
> https://lists.xen.org/archives/html/xen-devel/2014-11/msg00942.html
> 
> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  xen/arch/x86/Kconfig          |  1 +
>  xen/drivers/cpufreq/Kconfig   |  3 +++
>  xen/drivers/cpufreq/utility.c | 11 ++++++++++-
>  xen/drivers/pm/stat.c         |  6 ++++++
>  xen/include/xen/cpufreq.h     |  6 ++++++
>  5 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 86c8eca..c1eac1d 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -24,6 +24,7 @@ config X86
>       select NUMA
>       select VGA
>       select HAS_PM
> +     select HAS_CPU_TURBO
>  
>  config ARCH_DEFCONFIG
>       string
> diff --git a/xen/drivers/cpufreq/Kconfig b/xen/drivers/cpufreq/Kconfig
> index cce80f4..427ea2a 100644
> --- a/xen/drivers/cpufreq/Kconfig
> +++ b/xen/drivers/cpufreq/Kconfig
> @@ -1,3 +1,6 @@
>  
>  config HAS_CPUFREQ
>       bool
> +
> +config HAS_CPU_TURBO
> +     bool
> diff --git a/xen/drivers/cpufreq/utility.c b/xen/drivers/cpufreq/utility.c
> index a687e5a..25bf983 100644
> --- a/xen/drivers/cpufreq/utility.c
> +++ b/xen/drivers/cpufreq/utility.c
> @@ -209,7 +209,9 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy 
> *policy,
>  {
>      unsigned int min_freq = ~0;
>      unsigned int max_freq = 0;
> +#ifdef CONFIG_HAS_CPU_TURBO
>      unsigned int second_max_freq = 0;
> +#endif
>      unsigned int i;
>  
>      for (i=0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
> @@ -221,6 +223,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy 
> *policy,
>          if (freq > max_freq)
>              max_freq = freq;
>      }
> +#ifdef CONFIG_HAS_CPU_TURBO
>      for (i=0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
>          unsigned int freq = table[i].frequency;
>          if (freq == CPUFREQ_ENTRY_INVALID || freq == max_freq)
> @@ -234,9 +237,13 @@ int cpufreq_frequency_table_cpuinfo(struct 
> cpufreq_policy *policy,
>          printk("max_freq: %u    second_max_freq: %u\n",
>                 max_freq, second_max_freq);
>  
> +    policy->cpuinfo.second_max_freq = second_max_freq;
> +#else /* !CONFIG_HAS_CPU_TURBO */
> +    if (cpufreq_verbose)
> +        printk("max_freq: %u\n", max_freq);
> +#endif /* CONFIG_HAS_CPU_TURBO */
>      policy->min = policy->cpuinfo.min_freq = min_freq;
>      policy->max = policy->cpuinfo.max_freq = max_freq;
> -    policy->cpuinfo.second_max_freq = second_max_freq;
>  
>      if (policy->min == ~0)
>          return -EINVAL;
> @@ -390,6 +397,7 @@ int cpufreq_driver_getavg(unsigned int cpu, unsigned int 
> flag)
>      return policy->cur;
>  }
>  
> +#ifdef CONFIG_HAS_CPU_TURBO
>  int cpufreq_update_turbo(int cpuid, int new_state)
>  {
>      struct cpufreq_policy *policy;
> @@ -430,6 +438,7 @@ int cpufreq_get_turbo_status(int cpuid)
>      policy = per_cpu(cpufreq_cpu_policy, cpuid);
>      return policy && policy->turbo == CPUFREQ_TURBO_ENABLED;
>  }
> +#endif /* CONFIG_HAS_CPU_TURBO */
>  
>  /*********************************************************************
>   *                 POLICY                                            *

I am wondering if we need to go as far as #ifdef'ing
cpufreq_update_turbo. For the sake of reducing the number if #ifdef's,
would it be enough if we only make sure it is disabled?

In other words, I would keep the changes to stat.c but I would leave
utility.c and cpufreq.h pretty much untouched.


> diff --git a/xen/drivers/pm/stat.c b/xen/drivers/pm/stat.c
> index 2dbde1c..133e64d 100644
> --- a/xen/drivers/pm/stat.c
> +++ b/xen/drivers/pm/stat.c
> @@ -290,7 +290,11 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
>              &op->u.get_para.u.ondemand.sampling_rate,
>              &op->u.get_para.u.ondemand.up_threshold);
>      }
> +#ifdef CONFIG_HAS_CPU_TURBO
>      op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
> +#else
> +    op->u.get_para.turbo_enabled = 0;
> +#endif
>  
>      return ret;
>  }
> @@ -473,6 +477,7 @@ int do_pm_op(struct xen_sysctl_pm_op *op)
>          break;
>      }
>  
> +#ifdef CONFIG_HAS_CPU_TURBO
>      case XEN_SYSCTL_pm_op_enable_turbo:
>      {
>          ret = cpufreq_update_turbo(op->cpuid, CPUFREQ_TURBO_ENABLED);
> @@ -484,6 +489,7 @@ int do_pm_op(struct xen_sysctl_pm_op *op)
>          ret = cpufreq_update_turbo(op->cpuid, CPUFREQ_TURBO_DISABLED);
>          break;
>      }
> +#endif /* CONFIG_HAS_CPU_TURBO */
>  
>      default:
>          printk("not defined sub-hypercall @ do_pm_op\n");
> diff --git a/xen/include/xen/cpufreq.h b/xen/include/xen/cpufreq.h
> index 30c70c9..2e0c16a 100644
> --- a/xen/include/xen/cpufreq.h
> +++ b/xen/include/xen/cpufreq.h
> @@ -39,7 +39,9 @@ extern struct acpi_cpufreq_data *cpufreq_drv_data[NR_CPUS];
>  
>  struct cpufreq_cpuinfo {
>      unsigned int        max_freq;
> +#ifdef CONFIG_HAS_CPU_TURBO
>      unsigned int        second_max_freq;    /* P1 if Turbo Mode is on */
> +#endif
>      unsigned int        min_freq;
>      unsigned int        transition_latency; /* in 10^(-9) s = nanoseconds */
>  };
> @@ -72,9 +74,11 @@ struct cpufreq_policy {
>  
>      bool_t              resume; /* flag for cpufreq 1st run
>                                   * S3 wakeup, hotplug cpu, etc */
> +#ifdef CONFIG_HAS_CPU_TURBO
>      s8                  turbo;  /* tristate flag: 0 for unsupported
>                                   * -1 for disable, 1 for enabled
>                                   * See CPUFREQ_TURBO_* below for defines */
> +#endif
>      bool_t              aperf_mperf; /* CPU has APERF/MPERF MSRs */
>  };
>  DECLARE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_policy);
> @@ -138,8 +142,10 @@ extern int cpufreq_driver_getavg(unsigned int cpu, 
> unsigned int flag);
>  #define CPUFREQ_TURBO_UNSUPPORTED   0
>  #define CPUFREQ_TURBO_ENABLED       1
>  
> +#ifdef CONFIG_HAS_CPU_TURBO
>  extern int cpufreq_update_turbo(int cpuid, int new_state);
>  extern int cpufreq_get_turbo_status(int cpuid);
> +#endif
>  
>  static __inline__ int 
>  __cpufreq_governor(struct cpufreq_policy *policy, unsigned int event)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel