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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJZ5v0jNSoL0ke1eT9PDQKqJ0qkVNsowaU5mrgGj2seSL654aA@mail.gmail.com>
Date:   Wed, 29 Nov 2023 17:42:04 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Wyes Karny <wyes.karny@....com>
Cc:     rafael@...nel.org, ray.huang@....com, viresh.kumar@...aro.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        perry.yuan@....com
Subject: Re: [PATCH v2] cpufreq/amd-pstate: Fix scaling_min_freq and
 scaling_max_freq update

On Fri, Nov 17, 2023 at 7:39 AM Wyes Karny <wyes.karny@....com> wrote:
>
> When amd_pstate is running, writing to scaling_min_freq and
> scaling_max_freq has no effect. These values are only passed to the
> policy level, but not to the platform level. This means that the
> platform does not know about the frequency limits set by the user. To
> fix this, update the min_perf and max_perf values at the platform level
> whenever the user changes the scaling_min_freq and scaling_max_freq
> values.
>
> Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> Acked-by: Huang Rui <ray.huang@....com>
> Signed-off-by: Wyes Karny <wyes.karny@....com>
> ---
> Changelog:
> v1 -> v2:
> - Rebase
> - Pick up ack from Ray
> - Add fixes tag
>
>  drivers/cpufreq/amd-pstate.c | 60 ++++++++++++++++++++++++++++--------
>  include/linux/amd-pstate.h   |  4 +++
>  2 files changed, 51 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 9a1e194d5cf8..4839cdd2715e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -307,11 +307,11 @@ static int pstate_init_perf(struct amd_cpudata *cpudata)
>                 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->nominal_perf, AMD_CPPC_NOMINAL_PERF(cap1));
>         WRITE_ONCE(cpudata->lowest_nonlinear_perf, AMD_CPPC_LOWNONLIN_PERF(cap1));
>         WRITE_ONCE(cpudata->lowest_perf, AMD_CPPC_LOWEST_PERF(cap1));
> -
> +       WRITE_ONCE(cpudata->min_limit_perf, AMD_CPPC_LOWEST_PERF(cap1));
>         return 0;
>  }
>
> @@ -329,11 +329,12 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)
>                 highest_perf = cppc_perf.highest_perf;
>
>         WRITE_ONCE(cpudata->highest_perf, highest_perf);
> -
> +       WRITE_ONCE(cpudata->max_limit_perf, highest_perf);
>         WRITE_ONCE(cpudata->nominal_perf, cppc_perf.nominal_perf);
>         WRITE_ONCE(cpudata->lowest_nonlinear_perf,
>                    cppc_perf.lowest_nonlinear_perf);
>         WRITE_ONCE(cpudata->lowest_perf, cppc_perf.lowest_perf);
> +       WRITE_ONCE(cpudata->min_limit_perf, cppc_perf.lowest_perf);
>
>         if (cppc_state == AMD_PSTATE_ACTIVE)
>                 return 0;
> @@ -432,6 +433,10 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
>         u64 prev = READ_ONCE(cpudata->cppc_req_cached);
>         u64 value = prev;
>
> +       min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf,
> +                       cpudata->max_limit_perf);
> +       max_perf = clamp_t(unsigned long, max_perf, cpudata->min_limit_perf,
> +                       cpudata->max_limit_perf);
>         des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
>
>         if ((cppc_state == AMD_PSTATE_GUIDED) && (gov_flags & CPUFREQ_GOV_DYNAMIC_SWITCHING)) {
> @@ -470,6 +475,22 @@ static int amd_pstate_verify(struct cpufreq_policy_data *policy)
>         return 0;
>  }
>
> +static int amd_pstate_update_min_max_limit(struct cpufreq_policy *policy)
> +{
> +       u32 max_limit_perf, min_limit_perf;
> +       struct amd_cpudata *cpudata = policy->driver_data;
> +
> +       max_limit_perf = div_u64(policy->max * cpudata->highest_perf, cpudata->max_freq);
> +       min_limit_perf = div_u64(policy->min * cpudata->highest_perf, cpudata->max_freq);
> +
> +       WRITE_ONCE(cpudata->max_limit_perf, max_limit_perf);
> +       WRITE_ONCE(cpudata->min_limit_perf, min_limit_perf);
> +       WRITE_ONCE(cpudata->max_limit_freq, policy->max);
> +       WRITE_ONCE(cpudata->min_limit_freq, policy->min);
> +
> +       return 0;
> +}
> +
>  static int amd_pstate_update_freq(struct cpufreq_policy *policy,
>                                   unsigned int target_freq, bool fast_switch)
>  {
> @@ -480,6 +501,9 @@ static int amd_pstate_update_freq(struct cpufreq_policy *policy,
>         if (!cpudata->max_freq)
>                 return -ENODEV;
>
> +       if (policy->min != cpudata->min_limit_freq || policy->max != cpudata->max_limit_freq)
> +               amd_pstate_update_min_max_limit(policy);
> +
>         cap_perf = READ_ONCE(cpudata->highest_perf);
>         min_perf = READ_ONCE(cpudata->lowest_perf);
>         max_perf = cap_perf;
> @@ -532,6 +556,10 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
>         struct amd_cpudata *cpudata = policy->driver_data;
>         unsigned int target_freq;
>
> +       if (policy->min != cpudata->min_limit_freq || policy->max != cpudata->max_limit_freq)
> +               amd_pstate_update_min_max_limit(policy);
> +
> +
>         cap_perf = READ_ONCE(cpudata->highest_perf);
>         lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
>         max_freq = READ_ONCE(cpudata->max_freq);
> @@ -745,6 +773,8 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>         /* Initial processor data capability frequencies */
>         cpudata->max_freq = max_freq;
>         cpudata->min_freq = min_freq;
> +       cpudata->max_limit_freq = max_freq;
> +       cpudata->min_limit_freq = min_freq;
>         cpudata->nominal_freq = nominal_freq;
>         cpudata->lowest_nonlinear_freq = lowest_nonlinear_freq;
>
> @@ -1183,16 +1213,25 @@ static int amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy)
>         return 0;
>  }
>
> -static void amd_pstate_epp_init(unsigned int cpu)
> +static void amd_pstate_epp_update_limit(struct cpufreq_policy *policy)
>  {
> -       struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
>         struct amd_cpudata *cpudata = policy->driver_data;
> -       u32 max_perf, min_perf;
> +       u32 max_perf, min_perf, min_limit_perf, max_limit_perf;
>         u64 value;
>         s16 epp;
>
>         max_perf = READ_ONCE(cpudata->highest_perf);
>         min_perf = READ_ONCE(cpudata->lowest_perf);
> +       max_limit_perf = div_u64(policy->max * cpudata->highest_perf, cpudata->max_freq);
> +       min_limit_perf = div_u64(policy->min * cpudata->highest_perf, cpudata->max_freq);
> +
> +       max_perf = clamp_t(unsigned long, max_perf, cpudata->min_limit_perf,
> +                       cpudata->max_limit_perf);
> +       min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf,
> +                       cpudata->max_limit_perf);
> +
> +       WRITE_ONCE(cpudata->max_limit_perf, max_limit_perf);
> +       WRITE_ONCE(cpudata->min_limit_perf, min_limit_perf);
>
>         value = READ_ONCE(cpudata->cppc_req_cached);
>
> @@ -1210,9 +1249,6 @@ static void amd_pstate_epp_init(unsigned int cpu)
>         value &= ~AMD_CPPC_DES_PERF(~0L);
>         value |= AMD_CPPC_DES_PERF(0);
>
> -       if (cpudata->epp_policy == cpudata->policy)
> -               goto skip_epp;
> -
>         cpudata->epp_policy = cpudata->policy;
>
>         /* Get BIOS pre-defined epp value */
> @@ -1222,7 +1258,7 @@ static void amd_pstate_epp_init(unsigned int cpu)
>                  * This return value can only be negative for shared_memory
>                  * systems where EPP register read/write not supported.
>                  */
> -               goto skip_epp;
> +               return;
>         }
>
>         if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> @@ -1236,8 +1272,6 @@ static void amd_pstate_epp_init(unsigned int cpu)
>
>         WRITE_ONCE(cpudata->cppc_req_cached, value);
>         amd_pstate_set_epp(cpudata, epp);
> -skip_epp:
> -       cpufreq_cpu_put(policy);
>  }
>
>  static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
> @@ -1252,7 +1286,7 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
>
>         cpudata->policy = policy->policy;
>
> -       amd_pstate_epp_init(policy->cpu);
> +       amd_pstate_epp_update_limit(policy);
>
>         return 0;
>  }
> diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> index 446394f84606..6ad02ad9c7b4 100644
> --- a/include/linux/amd-pstate.h
> +++ b/include/linux/amd-pstate.h
> @@ -70,6 +70,10 @@ struct amd_cpudata {
>         u32     nominal_perf;
>         u32     lowest_nonlinear_perf;
>         u32     lowest_perf;
> +       u32     min_limit_perf;
> +       u32     max_limit_perf;
> +       u32     min_limit_freq;
> +       u32     max_limit_freq;
>
>         u32     max_freq;
>         u32     min_freq;
> --

Applied as 6.7-rc material, thanks!

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ