Hi Stefano
On Sat, Dec 2, 2017 at 3:06 AM, Stefano Stabellini
<[email protected]> wrote:
> On Thu, 9 Nov 2017, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Dmytryshyn <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Oleksandr Tyshchenko <[email protected]>
>> CC: Jan Beulich <[email protected]>
>> CC: Andrew Cooper <[email protected]>
>> CC: Stefano Stabellini <[email protected]>
>> CC: Julien Grall <[email protected]>
>> ---
>> 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.
Yes. I was thinking about dropping this patch at all. If platform
doesn't support CPU Boost, the platform
driver should just inform framework about that (policy->turbo =
CPUFREQ_TURBO_UNSUPPORTED).
That's all.
cpufreq_update_turbo() will return -EOPNOTSUPP if someone tries to
enable/disable turbo mode.
cpufreq_get_turbo_status() will return that turbo mode "is not enabled".
Another question is second_max_freq. As I understand, it is highest
non-turbo frequency calculated by framework to limit target frequency
when
turbo mode "is disabled". And Xen assumes that second_max_freq is
always P1 if turbo mode is on.
But, there might be a case when a few highest frequencies are
turbo-frequencies. So, I propose to add an extra flag for handling
that.
So, each CPUFreq driver responsibility will be to mark
turbo-frequency(ies) for the framework to properly calculate
second_max_freq.
Something like that:
diff --git a/xen/drivers/cpufreq/utility.c b/xen/drivers/cpufreq/utility.c
index 25bf983..122a88b 100644
--- a/xen/drivers/cpufreq/utility.c
+++ b/xen/drivers/cpufreq/utility.c
@@ -226,7 +226,8 @@ int cpufreq_frequency_table_cpuinfo(struct
cpufreq_policy *policy,
#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)
+ if ((freq == CPUFREQ_ENTRY_INVALID) ||
+ (table[i].flags & CPUFREQ_BOOST_FREQ))
continue;
if (freq > second_max_freq)
second_max_freq = freq;
diff --git a/xen/include/xen/cpufreq.h b/xen/include/xen/cpufreq.h
index 2e0c16a..77b29da 100644
--- a/xen/include/xen/cpufreq.h
+++ b/xen/include/xen/cpufreq.h
@@ -204,7 +204,11 @@ void cpufreq_verify_within_limits(struct
cpufreq_policy *policy,
#define CPUFREQ_ENTRY_INVALID ~0
#define CPUFREQ_TABLE_END ~1
+/* Special Values of .flags field */
+#define CPUFREQ_BOOST_FREQ (1 << 0)
+
struct cpufreq_frequency_table {
+ unsigned int flags;
unsigned int index; /* any */
unsigned int frequency; /* kHz - doesn't need to be in ascending
* order */
Both existing on x86 CPUFreq drivers just need to mark P0 frequency as
a turbo-frequency if turbo mode "is supported". Am I correct?
And the most important question is how to recognize in Xen on ARM
(using SCPI protocol) which frequencies are turbo-frequencies
actually? I couldn't find any information regarding that in protocol
description.
For DT-based CPUFreq it is not an issue, since there is a specific
property "turbo-mode" to mark corresponding OPPs. [1].
But neither SCPI DT bindings [2] nor the SCPI protocol itself [3]
mentions about it. Perhaps, additional command should be added to pass
such info.
[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/opp/opp.txt
[2]
http://elixir.free-electrons.com/linux/v4.15-rc1/source/Documentation/devicetree/bindings/arm/arm,scpi.txt
[3]
http://infocenter.arm.com/help/topic/com.arm.doc.dui0922g/scp_message_interface_v1_2_DUI0922G_en.pdf
>
>
>> 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)
--
Regards,
Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
