| 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: <2091c274-00b7-40ed-a27a-83850130df46@amd.com>
Date: Tue, 27 Aug 2024 13:36:47 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: "Gautham R. Shenoy" <gautham.shenoy@....com>,
Mario Limonciello <superm1@...nel.org>
Cc: Borislav Petkov <bp@...en8.de>, Perry Yuan <perry.yuan@....com>,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
"Rafael J . Wysocki" <rafael@...nel.org>,
"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<linux-kernel@...r.kernel.org>, "open list:ACPI"
<linux-acpi@...r.kernel.org>,
"open list:CPU FREQUENCY SCALING FRAMEWORK" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH 6/8] cpufreq: amd-pstate: Merge
amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()
On 8/27/2024 11:52, Gautham R. Shenoy wrote:
> Hello Mario,
>
>
> On Mon, Aug 26, 2024 at 04:13:56PM -0500, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@....com>
>>
>> The special case in amd_pstate_highest_perf_set() is the value used
>> for calculating the boost numerator. Merge this into
>> amd_get_boost_ratio_numerator() and then use that to calculate boost
>> ratio.
>>
>> This allows dropping more special casing of the highest perf value.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>> ---
>> arch/x86/kernel/acpi/cppc.c | 16 ++++++++++++
>> drivers/cpufreq/amd-pstate.c | 49 +++++++-----------------------------
>> 2 files changed, 25 insertions(+), 40 deletions(-)
>>
>> diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
>> index 729b35e84f5eb..44b13a4e28740 100644
>> --- a/arch/x86/kernel/acpi/cppc.c
>> +++ b/arch/x86/kernel/acpi/cppc.c
>> @@ -9,6 +9,7 @@
>> #include <asm/processor.h>
>> #include <asm/topology.h>
>>
>> +#define CPPC_HIGHEST_PERF_PERFORMANCE 196
>> #define CPPC_HIGHEST_PERF_PREFCORE 166
>>
>> enum amd_pref_core {
>> @@ -244,6 +245,21 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
>> *numerator = boost_numerator;
>> return 0;
>> }
>> +
>> + /*
>> + * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
>> + * the highest performance level is set to 196.
>> + * https://bugzilla.kernel.org/show_bug.cgi?id=218759
>> + */
>> + if (cpu_feature_enabled(X86_FEATURE_ZEN4)) {
>> + switch (boot_cpu_data.x86_model) {
>> + case 0x70 ... 0x7f:
>> + *numerator = CPPC_HIGHEST_PERF_PERFORMANCE;
>> + return 0;
>> + default:
>> + break;
>> + }
>> + }
>> *numerator = CPPC_HIGHEST_PERF_PREFCORE;
>>
>> return 0;
>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>> index ec32c830abc1d..75568d0f84623 100644
>> --- a/drivers/cpufreq/amd-pstate.c
>> +++ b/drivers/cpufreq/amd-pstate.c
>> @@ -52,8 +52,6 @@
>> #define AMD_PSTATE_TRANSITION_LATENCY 20000
>> #define AMD_PSTATE_TRANSITION_DELAY 1000
>> #define AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY 600
>> -#define CPPC_HIGHEST_PERF_PERFORMANCE 196
>> -#define CPPC_HIGHEST_PERF_DEFAULT 166
>>
>> #define AMD_CPPC_EPP_PERFORMANCE 0x00
>> #define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80
>> @@ -372,43 +370,17 @@ static inline int amd_pstate_enable(bool enable)
>> return static_call(amd_pstate_enable)(enable);
>> }
>>
>> -static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata)
>> -{
>> - struct cpuinfo_x86 *c = &cpu_data(0);
>> -
>> - /*
>> - * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
>> - * the highest performance level is set to 196.
>> - * https://bugzilla.kernel.org/show_bug.cgi?id=218759
>> - */
>> - if (c->x86 == 0x19 && (c->x86_model >= 0x70 && c->x86_model <= 0x7f))
>> - return CPPC_HIGHEST_PERF_PERFORMANCE;
>> -
>> - return CPPC_HIGHEST_PERF_DEFAULT;
>> -}
>> -
>> static int pstate_init_perf(struct amd_cpudata *cpudata)
>> {
>> u64 cap1;
>> - u32 highest_perf;
>>
>> int ret = rdmsrl_safe_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1,
>> &cap1);
>> if (ret)
>> return ret;
>>
>> - /* For platforms that do not support the preferred core feature, the
>> - * highest_pef may be configured with 166 or 255, to avoid max frequency
>> - * calculated wrongly. we take the AMD_CPPC_HIGHEST_PERF(cap1) value as
>> - * the default max perf.
>> - */
>> - if (cpudata->hw_prefcore)
>> - highest_perf = amd_pstate_highest_perf_set(cpudata);
>> - else
>> - highest_perf = AMD_CPPC_HIGHEST_PERF(cap1);
>> -
>> - WRITE_ONCE(cpudata->highest_perf, highest_perf);
>> - WRITE_ONCE(cpudata->max_limit_perf, highest_perf);
>> + WRITE_ONCE(cpudata->highest_perf, AMD_CPPC_HIGHEST_PERF(cap1));
>> + WRITE_ONCE(cpudata->max_limit_perf, AMD_CPPC_HIGHEST_PERF(cap1));
>
>
> So henceforth, cpudata->highest_perf is expected to cache the value of
> CPPC.highest_perf and not the boost_ratio_numerator. There are couple
> of user-visible changes due to this.
>
>
> 1. On platforms where preferred-core is supported, previously the
> sysfs file
> /sys/devices/system/cpu/cpuX/cpufreq/amd_pstate_highest_perf would
> report the boost_ratio_numerator. Henceforth it will report
> CPPC.highest_perf.
>
> I hope there are no userspace tools that compute the boost_ratio
> using the syfs amd_pstate_highest_perf/amd_pstate_nominal_perf.
>
> 2. The amd_pstate_prefcore_ranking and amd_pstate_highest_perf will
> show the same values on all platforms, and henceforth are
> redundant.
>
Good observations here. I'm not aware of any tools trying to replicate
this calculation.
With the redundancy I would actually argue we should just drop the sysfs
file 'amd_pstate_prefcore_ranking'.
Thoughts?
>
> Shouldn't this be documented?
I noticed amd_pstate_prefcore_ranking wasn't properly documented in
amd-pstate.rst in the first place. If the decision is not to drop the
sysfs file, then I'll add a section for it.
>
> The rest of the patch looks good to me.
>
>
>
> --
> Thanks and Regards
> gautham.
>
Powered by blists - more mailing lists