Messages in this thread | | | Date | Fri, 6 Jan 2023 06:50:45 -0800 | Subject | Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake | From | Dave Hansen <> |
| |
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?
| |