linux-kernel - Re: [PATCH] x86: intel_epb: Allow model specific normal EPB value

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]
Date:   Tue, 4 Jan 2022 16:26:39 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86: intel_epb: Allow model specific normal EPB value

On Mon, Dec 13, 2021 at 11:00 AM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Sat, Dec 11, 2021 at 5:33 PM Srinivas Pandruvada
> <srinivas.pandruvada@...ux.intel.com> wrote:
> >
> > The current EPB "normal" is defined as 6 and set whenever power-up EPB
> > value is 0. This setting resulted in the desired out of box power and
> > performance for several CPU generations. But this value is not suitable
> > for AlderLake mobile CPUs, as this resulted in higher uncore power.
> > Since EPB is model specific, this is not unreasonable to have different
> > behavior.
> >
> > Allow a capability where "normal" EPB can be redefined. For AlderLake
> > mobile CPUs this desired normal value is 7.
> >
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

And since there don't seem to be any concerns regarding this, I'm
going to apply it.

> > ---
> >  arch/x86/kernel/cpu/intel_epb.c | 45 +++++++++++++++++++++++----------
> >  1 file changed, 32 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/intel_epb.c b/arch/x86/kernel/cpu/intel_epb.c
> > index f4dd73396f28..fbaf12e43f41 100644
> > --- a/arch/x86/kernel/cpu/intel_epb.c
> > +++ b/arch/x86/kernel/cpu/intel_epb.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/syscore_ops.h>
> >  #include <linux/pm.h>
> >
> > +#include <asm/cpu_device_id.h>
> >  #include <asm/cpufeature.h>
> >  #include <asm/msr.h>
> >
> > @@ -58,6 +59,22 @@ static DEFINE_PER_CPU(u8, saved_epb);
> >  #define EPB_SAVED      0x10ULL
> >  #define MAX_EPB                EPB_MASK
> >
> > +enum energy_perf_value_index {
> > +       EPB_INDEX_PERFORMANCE,
> > +       EPB_INDEX_BALANCE_PERFORMANCE,
> > +       EPB_INDEX_NORMAL,
> > +       EPB_INDEX_BALANCE_POWERSAVE,
> > +       EPB_INDEX_POWERSAVE,
> > +};
> > +
> > +static u8 energ_perf_values[] = {
> > +       [EPB_INDEX_PERFORMANCE] = ENERGY_PERF_BIAS_PERFORMANCE,
> > +       [EPB_INDEX_BALANCE_PERFORMANCE] = ENERGY_PERF_BIAS_BALANCE_PERFORMANCE,
> > +       [EPB_INDEX_NORMAL] = ENERGY_PERF_BIAS_NORMAL,
> > +       [EPB_INDEX_BALANCE_POWERSAVE] = ENERGY_PERF_BIAS_BALANCE_POWERSAVE,
> > +       [EPB_INDEX_POWERSAVE] = ENERGY_PERF_BIAS_POWERSAVE,
> > +};
> > +
> >  static int intel_epb_save(void)
> >  {
> >         u64 epb;
> > @@ -90,7 +107,7 @@ static void intel_epb_restore(void)
> >                  */
> >                 val = epb & EPB_MASK;
> >                 if (val == ENERGY_PERF_BIAS_PERFORMANCE) {
> > -                       val = ENERGY_PERF_BIAS_NORMAL;
> > +                       val = energ_perf_values[EPB_INDEX_NORMAL];
> >                         pr_warn_once("ENERGY_PERF_BIAS: Set to 'normal', was 'performance'\n");
> >                 }
> >         }
> > @@ -103,18 +120,11 @@ static struct syscore_ops intel_epb_syscore_ops = {
> >  };
> >
> >  static const char * const energy_perf_strings[] = {
> > -       "performance",
> > -       "balance-performance",
> > -       "normal",
> > -       "balance-power",
> > -       "power"
> > -};
> > -static const u8 energ_perf_values[] = {
> > -       ENERGY_PERF_BIAS_PERFORMANCE,
> > -       ENERGY_PERF_BIAS_BALANCE_PERFORMANCE,
> > -       ENERGY_PERF_BIAS_NORMAL,
> > -       ENERGY_PERF_BIAS_BALANCE_POWERSAVE,
> > -       ENERGY_PERF_BIAS_POWERSAVE
> > +       [EPB_INDEX_PERFORMANCE] = "performance",
> > +       [EPB_INDEX_BALANCE_PERFORMANCE] = "balance-performance",
> > +       [EPB_INDEX_NORMAL] = "normal",
> > +       [EPB_INDEX_BALANCE_POWERSAVE] = "balance-power",
> > +       [EPB_INDEX_POWERSAVE] = "power",
> >  };
> >
> >  static ssize_t energy_perf_bias_show(struct device *dev,
> > @@ -193,13 +203,22 @@ static int intel_epb_offline(unsigned int cpu)
> >         return 0;
> >  }
> >
> > +static const struct x86_cpu_id intel_epb_normal[] = {
> > +       X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, 7),
> > +       {}
> > +};
> > +
> >  static __init int intel_epb_init(void)
> >  {
> > +       const struct x86_cpu_id *id = x86_match_cpu(intel_epb_normal);
> >         int ret;
> >
> >         if (!boot_cpu_has(X86_FEATURE_EPB))
> >                 return -ENODEV;
> >
> > +       if (id)
> > +               energ_perf_values[EPB_INDEX_NORMAL] = id->driver_data;
> > +
> >         ret = cpuhp_setup_state(CPUHP_AP_X86_INTEL_EPB_ONLINE,
> >                                 "x86/intel/epb:online", intel_epb_online,
> >                                 intel_epb_offline);
> > --
> > 2.31.1
> >