[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a375b136-c4da-0676-7a67-d2308c012b8b@intel.com>
Date: Fri, 6 Jan 2023 06:50:45 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: "Zhang, Rui" <rui.zhang@...el.com>, "bp@...en8.de" <bp@...en8.de>
Cc: "ak@...ux.intel.com" <ak@...ux.intel.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>,
"peterz@...radead.org" <peterz@...radead.org>,
"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
"mingo@...hat.com" <mingo@...hat.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"kan.liang@...ux.intel.com" <kan.liang@...ux.intel.com>,
"Bityutskiy, Artem" <artem.bityutskiy@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake
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.
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?
Powered by blists - more mailing lists