linux-kernel - Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake

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:   Sat, 7 Jan 2023 14:07:20 +0000
From:   "Zhang, Rui" <rui.zhang@...el.com>
To:     "Hansen, Dave" <dave.hansen@...el.com>,
        "bp@...en8.de" <bp@...en8.de>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "Bityutskiy, Artem" <artem.bityutskiy@...el.com>,
        "kan.liang@...ux.intel.com" <kan.liang@...ux.intel.com>
Subject: Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake

On Fri, 2023-01-06 at 06:50 -0800, Dave Hansen wrote:
> On 1/6/23 06:38, Zhang, Rui wrote:
> > My original proposal is that, instead of maintaining model lists in
> > a
> > series of different drivers, can we use feature flags instead, and
> > maintain them in a central place instead of different drivers. say,
> > something like
> > 
> > static const struct x86_cpu_id intel_pm_features[] __initconst = {
> >         X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L,           X86_FEATURE
> > _RAPL | X86_FEATURE_TCC_COOLING),
> >         X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X,           X86_FEATURE
> > _RAPL | X86_FEATURE_UNCORE_FREQ),
> >         ...
> >         X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE,           X86_FEATURE
> > _RAPL | X86_FEATURE_TCC_COOLING),
> >         X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X,    X86_FEATURE
> > _RAPL | X86_FEATURE_UNCORE_FREQ),
> >         ...
> >         {},
> > };
> > And then set the feature flags based on this, and make the drivers
> > test
> > the feature flags.
> 
> That works if you have very few features.  SKYLAKE_X looks to have on
> the order of 15 model-specific features, or at least references in
> the code.
> 
> That means that the
> 
> 	X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, ...
> 
> list goes on for 15 features.  It's even worse than that because
> you'd
> *like* to be able to scan up and down the list looking for, say, "all
> the CPUs that support RAPL".  But, if you do that, you actually need
> a
> table -- a really wide table -- for *all* the features and a column
> for
> each.

That's true.

> 
> What we have now isn't bad.  The only real way to fix this is to have
> the features enumerated *properly*, aka. architecturally.
> 
> I get it, Intel doesn't want to dedicate CPUID bits and architecture
> to
> one-offs.

> But, at the point that there are a dozen CPU models across
> three or four different CPU generations, it's time to revisit
> it.  Could
> you help our colleagues revisit it, please?

For this RAPL case, I think the biggest problem is the RAPL
*incompatibilities* between model variants as Ingo pointed out.
So a CPUID bit can not solve all the problems.

But given that the biggest inconsistency is the energy unit used on
different generations, I can also check with our colleagues if there is
a software visible way to get the "fixed" energy units rather than
hardcoding it in the driver using a model list.

thanks,
rui