[PATCH v4 03/11] x86/intel_pstate: add new policy fields and a new driver interface

[PATCH v4 03/11] x86/intel_pstate: add new policy fields and a new driver interface

[Wei Wang]

In order to better support future Intel processors, intel_pstate
changes to use percentage values to tune P-states. The setpolicy
driver interface is used to configure the intel_pstate internal
policy. The __cpufreq_set_policy needs to be intercepted to use
the setpolicy driver if it exists.

The perf_limts struct is included in the per-CPU policy struct, so
that each CPU can be managed individually by the xenpm tool.

The internal_governor struct is used by the driver that implements
its own internal governor, without going through the old acpi
governor framework.

v4 changes:
1) "data->limits = policy->limits", also moved it outside the if{};
2) introduced a new struct "internal_governor", which describes the
   internal governors, to "struct cpufreq_policy";
3) removed the "policy" field in "struct cpufreq_policy" - it's not
   used anymore;
4) renamed "CPUFREQ_POLICY_xx" to "INTERNAL_GOV_xx". "INTERNAL_GOV_xx"
   are macros for internal governors. Future other new internally
   implemented governors are expected to be added here;
5) no_turbo and turbo_disabled, were changed to be the "bool" type.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 xen/drivers/cpufreq/utility.c      |  3 +++
 xen/include/acpi/cpufreq/cpufreq.h | 27 +++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/xen/drivers/cpufreq/utility.c b/xen/drivers/cpufreq/utility.c
index 519f862..53879fe 100644
--- a/xen/drivers/cpufreq/utility.c
+++ b/xen/drivers/cpufreq/utility.c
@@ -456,6 +456,9 @@ int __cpufreq_set_policy(struct cpufreq_policy *data,
 
     data->min = policy->min;
     data->max = policy->max;
+    data->limits = policy->limits;
+    if (cpufreq_driver->setpolicy)
+        return cpufreq_driver->setpolicy(data);
 
     if (policy->governor != data->governor) {
         /* save old, working values */
diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h
index f96c3e4..af37e90 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -41,6 +41,24 @@ struct cpufreq_cpuinfo {
     unsigned int        transition_latency; /* in 10^(-9) s = nanoseconds */
 };
 
+struct perf_limits {
+    bool_t no_turbo;
+    bool_t turbo_disabled;
+    uint32_t turbo_pct;
+    uint32_t max_perf_pct; /* max performance in percentage */
+    uint32_t min_perf_pct; /* min performance in percentage */
+    uint32_t max_perf;
+    uint32_t min_perf;
+    uint32_t max_policy_pct;
+    uint32_t min_policy_pct;
+};
+
+struct internal_governor {
+    char *avail_gov;
+    uint32_t gov_num;
+    uint32_t cur_gov;
+};
+
 struct cpufreq_policy {
     cpumask_var_t       cpus;          /* affected CPUs */
     unsigned int        shared_type;   /* ANY or ALL affected CPUs
@@ -52,7 +70,9 @@ struct cpufreq_policy {
     unsigned int        max;    /* in kHz */
     unsigned int        cur;    /* in kHz, only needed if cpufreq
                                  * governors are used */
+    struct perf_limits  limits;
     struct cpufreq_governor     *governor;
+    struct internal_governor    *internal_gov;
 
     bool_t              resume; /* flag for cpufreq 1st run
                                  * S3 wakeup, hotplug cpu, etc */
@@ -87,6 +107,12 @@ struct cpufreq_freqs {
  *                          CPUFREQ GOVERNORS                        *
  *********************************************************************/
 
+/* Please add internal governors here */
+#define INTERNAL_GOV_POWERSAVE        (1)
+#define INTERNAL_GOV_PERFORMANCE      (2)
+#define INTERNAL_GOV_USERSPACE        (3)
+#define INTERNAL_GOV_ONDEMAND         (4)
+
 #define CPUFREQ_GOV_START  1
 #define CPUFREQ_GOV_STOP   2
 #define CPUFREQ_GOV_LIMITS 3
@@ -145,6 +171,7 @@ struct cpufreq_driver {
     char   name[CPUFREQ_NAME_LEN];
     int    (*init)(struct cpufreq_policy *policy);
     int    (*verify)(struct cpufreq_policy *policy);
+    int    (*setpolicy)(struct cpufreq_policy *policy);
     int    (*update)(int cpuid, struct cpufreq_policy *policy);
     int    (*target)(struct cpufreq_policy *policy,
                      unsigned int target_freq,
-- 
1.9.1

Re: [PATCH v4 03/11] x86/intel_pstate: add new policy fields and a new driver interface

[Jan Beulich]

>>> On 25.06.15 at 13:15, <wei.w.wang@intel.com> wrote:
> --- a/xen/include/acpi/cpufreq/cpufreq.h
> +++ b/xen/include/acpi/cpufreq/cpufreq.h
> @@ -41,6 +41,24 @@ struct cpufreq_cpuinfo {
>      unsigned int        transition_latency; /* in 10^(-9) s = nanoseconds */
>  };
>  
> +struct perf_limits {
> +    bool_t no_turbo;
> +    bool_t turbo_disabled;

Considering that struct cpufreq_policy already has a turbo field,
and without seeing how the fields get initialized or used, I can't
really judge whether they're rightfully being added here. If you
want to keep them, please defer their addition to this structure
until the patch actually using them. (I wonder whether that
wouldn't be better for most/all of the other fields too.)

> +struct internal_governor {
> +    char *avail_gov;
> +    uint32_t gov_num;
> +    uint32_t cur_gov;

Same here - how should a reviewer of this patch judge whether
these fields make sens when they aren't being used anywhere?
I can somehow guess the meaning of the last field...

Jan