linux-kernel - Re: [PATCH] cpufreq: intel_pstate: Use the latest guaranteed freq during verify

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: <CAJZ5v0gBwVm4p-haqfKFL25RGXPteeem0ake2UoaT9pm=17BoA@mail.gmail.com>
Date:   Thu, 17 Dec 2020 20:02:07 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cpufreq: intel_pstate: Use the latest guaranteed freq
 during verify

On Thu, Dec 17, 2020 at 6:29 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Thu, Dec 17, 2020 at 6:09 PM Srinivas Pandruvada
> <srinivas.pandruvada@...ux.intel.com> wrote:
> >
> > On Thu, 2020-12-17 at 16:24 +0100, Rafael J. Wysocki wrote:
> > > On Thu, Dec 17, 2020 at 4:21 PM Srinivas Pandruvada
> > > <srinivas.pandruvada@...ux.intel.com> wrote:

[cut]

> > > > > Well, would something like the patch below work?
> > > > >
> > > > > ---
> > > > >  drivers/cpufreq/intel_pstate.c |   16 +++++++++++++---
> > > > >  1 file changed, 13 insertions(+), 3 deletions(-)
> > > > >
> > > > > Index: linux-pm/drivers/cpufreq/intel_pstate.c
> > > > > =================================================================
> > > > > ==
> > > > > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> > > > > +++ linux-pm/drivers/cpufreq/intel_pstate.c
> > > > > @@ -2207,9 +2207,9 @@ static void intel_pstate_update_perf_lim
> > > > >                                             unsigned int
> > > > > policy_min,
> > > > >                                             unsigned int
> > > > > policy_max)
> > > > >  {
> > > > > -       int max_freq = intel_pstate_get_max_freq(cpu);
> > > > >         int32_t max_policy_perf, min_policy_perf;
> > > > >         int max_state, turbo_max;
> > > > > +       int max_freq;
> > > > >
> > > > >         /*
> > > > >          * HWP needs some special consideration, because on BDX
> > > > > the
> > > > > @@ -2223,6 +2223,7 @@ static void intel_pstate_update_perf_lim
> > > > >                         cpu->pstate.max_pstate : cpu-
> > > > > > pstate.turbo_pstate;
> > > > >                 turbo_max = cpu->pstate.turbo_pstate;
> > > > >         }
> > > > > +       max_freq = max_state * cpu->pstate.scaling;
> > > > >
> > > > >         max_policy_perf = max_state * policy_max / max_freq;
> > > > >         if (policy_max == policy_min) {
> > > > > @@ -2325,9 +2326,18 @@ static void intel_pstate_adjust_policy_m
> > > > >  static void intel_pstate_verify_cpu_policy(struct cpudata *cpu,
> > > > >                                            struct
> > > > > cpufreq_policy_data
> > > > > *policy)
> > > > >  {
> > > > > +       int max_freq;
> > > > > +
> > > > >         update_turbo_state();
> > > > > -       cpufreq_verify_within_limits(policy, policy-
> > > > > > cpuinfo.min_freq,
> > > > > -
> > > > > intel_pstate_get_max_freq(cpu));
> > > > > +       if (hwp_active) {
> > > > > +               int max_state, turbo_max;
> > > > > +
> > > > > +               intel_pstate_get_hwp_max(cpu->cpu, &turbo_max,
> > > > > &max_state);
> > > > > +               max_freq = max_state * cpu->pstate.scaling;
> > > > > +       } else {
> > > > > +               max_freq = intel_pstate_get_max_freq(cpu);
> > > > > +       }
> > > > > +       cpufreq_verify_within_limits(policy, policy-
> > > > > > cpuinfo.min_freq, max_freq);
> > > > >
> > > > >         intel_pstate_adjust_policy_max(cpu, policy);
> > > > >  }
> > > > >
> > > > Should work.
> > > >  I will test this patch and let you know once I get the system.
> > >
> > > Please do, thank you!
> >
> > This works. Please check if you can submit a change for this.
>
> I can do that, but I'm going to borrow some changelog pieces from the
> $subject patch.
>
> Will submit shortly.

Well, this only fixes the setting of the policy max limit AFAICS, but
pstate.max_pstate is used in computations in some places, so it looks
like it needs to be updated every time HWP_CAP is read, or do I
confuse things?

And if pstate.max_pstate needs to be updated then, shouldn't
pstate.turbo_pstate be updated then too (because it may change too as
a result of ISS updates)?