| 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
| ||
|
Date: Thu, 31 Aug 2023 18:19:18 +0800
From: Tony W Wang-oc <TonyWWang-oc@...oxin.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
CC: <viresh.kumar@...aro.org>, <linux-pm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <CobeChen@...oxin.com>,
<TimGuo@...oxin.com>, <LeoLiu-oc@...oxin.com>,
<LindaChai@...oxin.com>
Subject: Re: [PATCH v2] cpufreq: ACPI: add ITMT support when CPPC enabled
On 8/23/23 04:01, Rafael J. Wysocki wrote:
> On Tue, Aug 8, 2023 at 1:13 PM Tony W Wang-oc <TonyWWang-oc@...oxin.com> wrote:
>>
>> The _CPC method can get per-core highest frequency.
>
> Well, not exactly. A more precise way to say this would be "The
> per-core highest frequency can be obtained via CPPC."
>
Thanks for your reply, will rewrite the commit in next version.
>> The highest frequency may varies between cores which mean cores can
>
> "may vary" and "which means"
>
>> running at different max frequency, so can use it as a core priority
>
> "can run", but it would be better to say "may run".
>
>> and give a hint to scheduler in order to put critical task to the
>> higher priority core.
>
> Well, roughly speaking ...
>
> You should really talk about ITMT and how it can be hooked up to this.
>
Ok, Got it.
>> Signed-off-by: Tony W Wang-oc <TonyWWang-oc@...oxin.com>
>> ---
>> v1->v2: Fix build errors reported by kernel test robot
>>
>> arch/x86/kernel/itmt.c | 2 ++
>> drivers/cpufreq/acpi-cpufreq.c | 59 ++++++++++++++++++++++++++++++----
>> 2 files changed, 54 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/kernel/itmt.c b/arch/x86/kernel/itmt.c
>> index ee4fe8cdb857..b49ac8ecbbd6 100644
>> --- a/arch/x86/kernel/itmt.c
>> +++ b/arch/x86/kernel/itmt.c
>> @@ -122,6 +122,7 @@ int sched_set_itmt_support(void)
>>
>> return 0;
>> }
>> +EXPORT_SYMBOL_GPL(sched_set_itmt_support);
>
> This requires an ACK from the x86 maintainers.
>
>>
>> /**
>> * sched_clear_itmt_support() - Revoke platform's support of ITMT
>> @@ -181,3 +182,4 @@ void sched_set_itmt_core_prio(int prio, int cpu)
>> {
>> per_cpu(sched_core_priority, cpu) = prio;
>> }
>> +EXPORT_SYMBOL_GPL(sched_set_itmt_core_prio);
>
> And same here.
>
>> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
>> index b2f05d27167e..5733323e04ac 100644
>> --- a/drivers/cpufreq/acpi-cpufreq.c
>> +++ b/drivers/cpufreq/acpi-cpufreq.c
>> @@ -628,28 +628,35 @@ static int acpi_cpufreq_blacklist(struct cpuinfo_x86 *c)
>> #endif
>>
>> #ifdef CONFIG_ACPI_CPPC_LIB
>> -static u64 get_max_boost_ratio(unsigned int cpu)
>> +static void cpufreq_get_core_perf(int cpu, u64 *highest_perf, u64 *nominal_perf)
>
> This is not a cpufreq core function, so please use a different prefix
> in its name.
>
Ok. Will remove the prefix of "cpufreq_".
>> {
>> struct cppc_perf_caps perf_caps;
>> - u64 highest_perf, nominal_perf;
>> int ret;
>>
>> if (acpi_pstate_strict)
>> - return 0;
>> + return;
>>
>> ret = cppc_get_perf_caps(cpu, &perf_caps);
>> if (ret) {
>> pr_debug("CPU%d: Unable to get performance capabilities (%d)\n",
>> cpu, ret);
>> - return 0;
>> + return;
>> }
>>
>> if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
>> - highest_perf = amd_get_highest_perf();
>> + *highest_perf = amd_get_highest_perf();
>> else
>> - highest_perf = perf_caps.highest_perf;
>> + *highest_perf = perf_caps.highest_perf;
>> +
>> + *nominal_perf = perf_caps.nominal_perf;
>> + return;
>> +}
>>
>> - nominal_perf = perf_caps.nominal_perf;
>> +static u64 get_max_boost_ratio(unsigned int cpu)
>> +{
>> + u64 highest_perf, nominal_perf;
>> +
>> + cpufreq_get_core_perf(cpu, &highest_perf, &nominal_perf);
>>
>> if (!highest_perf || !nominal_perf) {
>> pr_debug("CPU%d: highest or nominal performance missing\n", cpu);
>> @@ -663,8 +670,44 @@ static u64 get_max_boost_ratio(unsigned int cpu)
>>
>> return div_u64(highest_perf << SCHED_CAPACITY_SHIFT, nominal_perf);
>> }
>> +
>> +static void cpufreq_sched_itmt_work_fn(struct work_struct *work)
>
> A similar comment applies here.
>
>> +{
>> + sched_set_itmt_support();
>> +}
>> +
>> +static DECLARE_WORK(sched_itmt_work, cpufreq_sched_itmt_work_fn);
>> +
>> +static void cpufreq_set_itmt_prio(int cpu)
>> +{
>> + u64 highest_perf, nominal_perf;
>> + static u32 max_highest_perf = 0, min_highest_perf = U32_MAX;
>> +
>> + cpufreq_get_core_perf(cpu, &highest_perf, &nominal_perf);
>> +
>> + sched_set_itmt_core_prio(highest_perf, cpu);
>> +
>> + if (max_highest_perf <= min_highest_perf) {
>> + if (highest_perf > max_highest_perf)
>> + max_highest_perf = highest_perf;
>> +
>> + if (highest_perf < min_highest_perf)
>> + min_highest_perf = highest_perf;
>> +
>> + if (max_highest_perf > min_highest_perf) {
>> + /*
>> + * This code can be run during CPU online under the
>> + * CPU hotplug locks, so sched_set_itmt_support()
>> + * cannot be called from here. Queue up a work item
>> + * to invoke it.
>> + */
>> + schedule_work(&sched_itmt_work);
>> + }
>
> This potentially runs before ITMT priorities are set for all CPUs.
> Isn't it a problem?
>
Yes, you are right.
Will use schedule_delayed_work(&sched_itmt_work, msecs_to_jiffies(500))
to fix this.
Sincerely.
TonyWWang-oc
>> + }
>> +}
>> #else
>> static inline u64 get_max_boost_ratio(unsigned int cpu) { return 0; }
>> +static void cpufreq_set_itmt_prio(int cpu) { return; }
>> #endif
>>
>> static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>> @@ -870,6 +913,8 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>> /* notify BIOS that we exist */
>> acpi_processor_notify_smm(THIS_MODULE);
>>
>> + cpufreq_set_itmt_prio(cpu);
>> +
>> pr_debug("CPU%u - ACPI performance management activated.\n", cpu);
>> for (i = 0; i < perf->state_count; i++)
>> pr_debug(" %cP%d: %d MHz, %d mW, %d uS\n",
>> --
Powered by blists - more mailing lists