| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CALYGNiP1-=Rk85Ek1LXkiP8X5eC0kgbhNZYYFLimYx-k+Gcukg@mail.gmail.com>
Date: Fri, 17 Jul 2015 10:10:54 +0300
From: Konstantin Khlebnikov <koct9i@...il.com>
To: ethan zhao <ethan.zhao@...cle.com>
Cc: Konstantin Khlebnikov <khlebnikov@...dex-team.ru>,
linux-pm@...r.kernel.org, "Rafael J. Wysocki" <rjw@...ysocki.net>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-acpi@...r.kernel.org,
Kristen Carlson Accardi <kristen@...ux.intel.com>,
Len Brown <lenb@...nel.org>
Subject: Re: [PATCH RFC] intel_pstate: play well with frequency limits set by acpi
On Fri, Jul 17, 2015 at 9:00 AM, ethan zhao <ethan.zhao@...cle.com> wrote:
>
> On 2015/7/17 2:17, Konstantin Khlebnikov wrote:
>>
>> IPMI can control CPU P-states remotely: configuration is reported via
>> common ACPI interface (_PPC/_PSS/etc). This patch adds required minimal
>> support in intel_pstate to receive and use these P-state limits.
>>
>> * ignore limit of top state in _PPC: it lower than turbo boost frequency
>> * register intel_pstate in acpi-processor to get states from _PSS
>> * link acpi_processor_get_bios_limit: this adds attribute "bios_limit"
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
>> ---
>> drivers/acpi/processor_perflib.c | 3 +-
>> drivers/cpufreq/intel_pstate.c | 57
>> ++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/processor_perflib.c
>> b/drivers/acpi/processor_perflib.c
>> index cfc8aba72f86..781e328c9d5f 100644
>> --- a/drivers/acpi/processor_perflib.c
>> +++ b/drivers/acpi/processor_perflib.c
>> @@ -98,7 +98,8 @@ static int acpi_processor_ppc_notifier(struct
>> notifier_block *nb,
>> ppc = (unsigned int)pr->performance_platform_limit;
>> - if (ppc >= pr->performance->state_count)
>> + /* Ignore limit of top state: it lower than turbo boost frequency
>> */
>> + if (!ppc || ppc >= pr->performance->state_count)
>
> Perhaps the !ppc is wrong if we check it against ACPI spec.
> Zero value of ppc means:
>
> "0 – States 0 through Nth state are available (all states available)"
Yep, also states are ordered by power - state0 must must have highest
performance.
This code below clamps range of available frequences to 0.._PSS[_PPC].frequency.
So if _PPC = 0 then there should be no limit but then turbo is enabled frequency
of state0 is actually could be lower than effective turbo frequency:
for hardware I have it's _PSS[0] = 2601 Mhz, PSS[1] = 2600 Mhz while turbo
state is 3400 Mhz.
>
>
>> goto out;
>> cpufreq_verify_within_limits(policy, 0,
>> diff --git a/drivers/cpufreq/intel_pstate.c
>> b/drivers/cpufreq/intel_pstate.c
>> index 15ada47bb720..4a34ddf4fa73 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -26,6 +26,7 @@
>> #include <linux/fs.h>
>> #include <linux/debugfs.h>
>> #include <linux/acpi.h>
>> +#include <acpi/processor.h>
>> #include <linux/vmalloc.h>
>> #include <trace/events/power.h>
>> @@ -113,6 +114,9 @@ struct cpudata {
>> u64 prev_mperf;
>> u64 prev_tsc;
>> struct sample sample;
>> +#ifdef CONFIG_ACPI_PROCESSOR
>> + struct acpi_processor_performance acpi_data;
>> +#endif
>> };
>> static struct cpudata **all_cpu_data;
>> @@ -145,6 +149,7 @@ static int hwp_active;
>> struct perf_limits {
>> int no_turbo;
>> + int no_acpi;
>> int turbo_disabled;
>> int max_perf_pct;
>> int min_perf_pct;
>> @@ -158,6 +163,7 @@ struct perf_limits {
>> static struct perf_limits limits = {
>> .no_turbo = 0,
>> + .no_acpi = !IS_ENABLED(CONFIG_ACPI_PROCESSOR),
>> .turbo_disabled = 0,
>> .max_perf_pct = 100,
>> .max_perf = int_tofp(1),
>> @@ -449,6 +455,18 @@ static ssize_t store_min_perf_pct(struct kobject *a,
>> struct attribute *b,
>> return count;
>> }
>> +static ssize_t store_no_acpi(struct kobject *a, struct attribute *b,
>> + const char *buf, size_t count)
>> +{
>> +#ifdef CONFIG_ACPI_PROCESSOR
>> + return kstrtouint(buf, 0, &limits.no_acpi) ?: count;
>> +#else
>> + return -ENODEV;
>> +#endif
>> +}
>> +show_one(no_acpi, no_acpi);
>> +define_one_global_rw(no_acpi);
>> +
>> show_one(max_perf_pct, max_perf_pct);
>> show_one(min_perf_pct, min_perf_pct);
>> @@ -460,6 +478,7 @@ define_one_global_ro(num_pstates);
>> static struct attribute *intel_pstate_attributes[] = {
>> &no_turbo.attr,
>> + &no_acpi.attr,
>> &max_perf_pct.attr,
>> &min_perf_pct.attr,
>> &turbo_pct.attr,
>> @@ -1049,6 +1068,38 @@ static int intel_pstate_cpu_init(struct
>> cpufreq_policy *policy)
>> policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
>> cpumask_set_cpu(policy->cpu, policy->cpus);
>> +#ifdef CONFIG_ACPI_PROCESSOR
>> + if (!limits.no_acpi) {
>> + /*
>> + * Minimum necessary to get acpi_processor_ppc_notifier()
>> and
>> + * acpi_processor_get_bios_limit() working.
>> + */
>> + if (!zalloc_cpumask_var(&cpu->acpi_data.shared_cpu_map,
>> + GFP_KERNEL))
>> + rc = -ENOMEM;
>> + else
>> + rc = acpi_processor_register_performance(
>> + &cpu->acpi_data, policy->cpu);
>> + if (rc) {
>> + pr_err("intel_pstate: acpi init failed: %d\n",
>> rc);
>> + free_cpumask_var(cpu->acpi_data.shared_cpu_map);
>> + limits.no_acpi = 1;
>> + }
>> + }
>> +#endif
>> + return 0;
>> +}
>> +
>> +static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
>> +{
>> +#ifdef CONFIG_ACPI_PROCESSOR
>> + struct cpudata *cpu = all_cpu_data[policy->cpu];
>> +
>> + if (cpu->acpi_data.state_count)
>> + acpi_processor_unregister_performance(&cpu->acpi_data,
>> + policy->cpu);
>> + free_cpumask_var(cpu->acpi_data.shared_cpu_map);
>> +#endif
>> return 0;
>> }
>> @@ -1057,7 +1108,11 @@ static struct cpufreq_driver intel_pstate_driver
>> = {
>> .verify = intel_pstate_verify_policy,
>> .setpolicy = intel_pstate_set_policy,
>> .get = intel_pstate_get,
>> +#ifdef CONFIG_ACPI_PROCESSOR
>> + .bios_limit = acpi_processor_get_bios_limit,
>> +#endif
>> .init = intel_pstate_cpu_init,
>> + .exit = intel_pstate_cpu_exit,
>> .stop_cpu = intel_pstate_stop_cpu,
>> .name = "intel_pstate",
>> };
>> @@ -1286,6 +1341,8 @@ static int __init intel_pstate_setup(char *str)
>> force_load = 1;
>> if (!strcmp(str, "hwp_only"))
>> hwp_only = 1;
>> + if (!strcmp(str, "no_acpi"))
>> + limits.no_acpi = 1;
>> return 0;
>> }
>> early_param("intel_pstate", intel_pstate_setup);
>>
>
>
> Thanks,
> Ethan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists