| 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: <CABqG17j8gmewS5egmyMwHpWM3vL4ourE+4oqorQ+A8w7PAAyrQ@mail.gmail.com>
Date: Fri, 20 Dec 2024 15:39:09 +0530
From: Naresh Solanki <naresh.solanki@...ements.com>
To: Mario Limonciello <mario.limonciello@....com>
Cc: "Gautham R. Shenoy" <gautham.shenoy@....com>, Perry Yuan <perry.yuan@....com>,
Huang Rui <ray.huang@....com>, "Rafael J. Wysocki" <rafael@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] cpufreq/amd-pstate: Refactor max frequency calculation
Hi Mario
Based on linux-next:
2024-12-20 11:02:53,078 INFO: 🔋 Kernel sysfs files
+---------+----------------+----------------------+----------------+--------------------+--------------------+---------------------------------+------------+---------+
| CPU # | CPU Min Freq | CPU Nonlinear Freq | CPU Max Freq |
Scaling Min Freq | Scaling Max Freq | Energy Performance Preference
| Prefcore | Boost |
|---------+----------------+----------------------+----------------+--------------------+--------------------+---------------------------------+------------+---------|
| 0 | 599000 | 2000000 | 5095000 |
2000000 | 5095000 | performance
| 208 | 1 |
| 1 | 599000 | 2000000 | 5095000 |
2000000 | 5095000 | performance
| 196 | 1 |
| 2 | 599000 | 2000000 | 5095000 |
2000000 | 5095000 | performance
| 202 | 1 |
| 3 | 599000 | 2000000 | 5095000 |
2000000 | 5095000 | performance
| 208 | 1 |
| 4 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 5 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 6 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 7 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 8 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 9 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 10 | 599000 | 2000000 | 5095000 |
2000000 | 5095000 | performance
| 208 | 1 |
| 11 | 599000 | 2000000 | 5095000 |
2000000 | 5095000 | performance
| 196 | 1 |
| 12 | 599000 | 2000000 | 5095000 |
2000000 | 5095000 | performance
| 202 | 1 |
| 13 | 599000 | 2000000 | 5095000 |
2000000 | 5095000 | performance
| 208 | 1 |
| 14 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 15 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 16 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 17 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 18 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
| 19 | 599000 | 1401000 | 5095000 |
1401000 | 5095000 | performance
| 128 | 1 |
+---------+----------------+----------------------+----------------+--------------------+--------------------+---------------------------------+------------+---------+
Based on V3 changes:
+---------+----------------+----------------------+----------------+--------------------+--------------------+---------------------------------+------------+---------+
| CPU # | CPU Min Freq | CPU Nonlinear Freq | CPU Max Freq |
Scaling Min Freq | Scaling Max Freq | Energy Performance Preference
| Prefcore | Boost |
|---------+----------------+----------------------+----------------+--------------------+--------------------+---------------------------------+------------+---------|
| 0 | 599000 | 2002000 | 5096000 |
2002000 | 5096000 | performance
| 208 | 1 |
| 1 | 599000 | 2002000 | 5096000 |
2002000 | 5096000 | performance
| 196 | 1 |
| 2 | 599000 | 2002000 | 5096000 |
2002000 | 5096000 | performance
| 202 | 1 |
| 3 | 599000 | 2002000 | 5096000 |
2002000 | 5096000 | performance
| 208 | 1 |
| 4 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 5 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 6 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 7 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 8 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 9 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 10 | 599000 | 2002000 | 5096000 |
2002000 | 5096000 | performance
| 208 | 1 |
| 11 | 599000 | 2002000 | 5096000 |
2002000 | 5096000 | performance
| 196 | 1 |
| 12 | 599000 | 2002000 | 5096000 |
2002000 | 5096000 | performance
| 202 | 1 |
| 13 | 599000 | 2002000 | 5096000 |
2002000 | 5096000 | performance
| 208 | 1 |
| 14 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 15 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 16 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 17 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 18 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
| 19 | 599000 | 1404000 | 5096000 |
1404000 | 5096000 | performance
| 128 | 1 |
+---------+----------------+----------------------+----------------+--------------------+--------------------+---------------------------------+------------+---------+
Regards,
Naresh
Regards,
Naresh Solanki
9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email: naresh.solanki@...ements.com
Mobile: +91 9538631477
Sitz der Gesellschaft: Bochum
Handelsregister: Amtsgericht Bochum, HRB 17519
Geschäftsführung: Sebastian Deutsch, Eray Basar
Datenschutzhinweise nach Art. 13 DSGVO
On Fri, 20 Dec 2024 at 02:38, Mario Limonciello
<mario.limonciello@....com> wrote:
>
> On 12/19/2024 14:15, Naresh Solanki wrote:
> > Hi Mario,
> >
> > On Fri, 20 Dec 2024 at 01:40, Naresh Solanki
> > <naresh.solanki@...ements.com> wrote:
> >>
> >> Hi Mario,
> >>
> >> On Fri, 20 Dec 2024 at 01:02, Mario Limonciello
> >> <mario.limonciello@....com> wrote:
> >>>
> >>> On 12/19/2024 13:21, Naresh Solanki wrote:
> >>>> The previous approach introduced roundoff errors during division when
> >>>> calculating the boost ratio. This, in turn, affected the maximum
> >>>> frequency calculation, often resulting in reporting lower frequency
> >>>> values.
> >>>>
> >>>> For example, on the Glinda SoC based board with the following
> >>>> parameters:
> >>>>
> >>>> max_perf = 208
> >>>> nominal_perf = 100
> >>>> nominal_freq = 2600 MHz
> >>>>
> >>>> The Linux kernel previously calculated the frequency as:
> >>>> freq = ((max_perf * 1024 / nominal_perf) * nominal_freq) / 1024
> >>>> freq = 5405 MHz // Integer arithmetic.
> >>>>
> >>>> With the updated formula:
> >>>> freq = (max_perf * nominal_freq) / nominal_perf
> >>>> freq = 5408 MHz
> >>>>
> >>>> This change ensures more accurate frequency calculations by eliminating
> >>>> unnecessary shifts and divisions, thereby improving precision.
> >>>>
> >>>> Signed-off-by: Naresh Solanki <naresh.solanki@...ements.com>
> >>>
> >>> Thanks, this makes sense to me.
> >>>
> >>> But looking at it, we should have the same problem with lowest nonlinear
> >>> freq as it goes through the same conversion process. Would you mind
> >>> fixing that one too?
> >> Sure. Somehow my eyes missed that.
> >> Also observed that current calculations yields zero for lowest_nonlinear_freq.
> > Sorry I was wrong. it's not zero. Its roundoff version.
> >
> >> After fixing that, it reported frequency 2002 & 1404 depending on the core.
>
> Mmm, I wouldn't expect that. Is your APU heterogenous? Or is this a
> BIOS bug?
>
> Both with your v3 patch in place and your patch not in place can you
> send me the report generated from:
> https://gitlab.freedesktop.org/drm/amd/-/blob/master/scripts/amd-pstate-triage.py
>
> >>
> >> On a side note, I'm also observing that the highest_perf is set to 196 which
> >> should not have been the case as I do have cores with value 208.
> >> Seems like amd_get_boost_ratio_numerator needs some addressing for that.
>
> Ah this is something that is quite confusing about how AMD client CPUs
> behave. There is a feature called "Preferred cores" that uses the
> highest performance value to indicate rankings of the cores. This means
> that you can't use the value in this register to calculate the
> frequency, you have to use the pre-defined scaling factor.
>
> The scaling factor is listed in arch/x86/kernel/acpi/cppc.c and the
> correct one is fetched for this calculation.
>
> >>
> >> Regards,
> >> Naresh
> >>>
> >>> Gautham, Perry,
> >>>
> >>> Is there something non-obvious I'm missing about why it was done this
> >>> way? It looks like it's been there since the start.
> >>>
> >>>>
> >>>> Changes in V2:
> >>>> 1. Rebase on superm1.git/linux-next branch
> >>>> ---
> >>>> drivers/cpufreq/amd-pstate.c | 9 ++++-----
> >>>> 1 file changed, 4 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> >>>> index d7b1de97727a..02a851f93fd6 100644
> >>>> --- a/drivers/cpufreq/amd-pstate.c
> >>>> +++ b/drivers/cpufreq/amd-pstate.c
> >>>> @@ -908,9 +908,9 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
> >>>> {
> >>>> int ret;
> >>>> u32 min_freq, max_freq;
> >>>> - u32 nominal_perf, nominal_freq;
> >>>> + u32 highest_perf, nominal_perf, nominal_freq;
> >>>> u32 lowest_nonlinear_perf, lowest_nonlinear_freq;
> >>>> - u32 boost_ratio, lowest_nonlinear_ratio;
> >>>> + u32 lowest_nonlinear_ratio;
> >>>> struct cppc_perf_caps cppc_perf;
> >>>>
> >>>> ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
> >>>> @@ -927,10 +927,9 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
> >>>> else
> >>>> nominal_freq = cppc_perf.nominal_freq;
> >>>>
> >>>> + highest_perf = READ_ONCE(cpudata->highest_perf);
> >>>> nominal_perf = READ_ONCE(cpudata->nominal_perf);
> >>>> -
> >>>> - boost_ratio = div_u64(cpudata->highest_perf << SCHED_CAPACITY_SHIFT, nominal_perf);
> >>>> - max_freq = (nominal_freq * boost_ratio >> SCHED_CAPACITY_SHIFT);
> >>>> + max_freq = div_u64((u64)highest_perf * nominal_freq, nominal_perf);
> >>>>
> >>>> lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
> >>>> lowest_nonlinear_ratio = div_u64(lowest_nonlinear_perf << SCHED_CAPACITY_SHIFT,
> >>>
>
Powered by blists - more mailing lists