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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d6ae35f1e72d4bb8e02f44fe6d9369f1a5f21619.camel@intel.com>
Date:   Fri, 6 Jan 2023 14:38:10 +0000
From:   "Zhang, Rui" <rui.zhang@...el.com>
To:     "bp@...en8.de" <bp@...en8.de>
CC:     "Hansen, Dave" <dave.hansen@...el.com>,
        "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

Hi, Boris,

On Fri, 2023-01-06 at 11:39 +0100, Borislav Petkov wrote:
> On Fri, Jan 06, 2023 at 06:05:41AM +0000, Zhang, Rui wrote:
> > But I still have a question.
> > Take RAPL feature for example, the feature is not architectural,
> > although 80% of the platforms may follow the same behavior, but
> > there
> > are still cases that behave differently. And so far, there are 8
> > different behaviors based on different models.
> > 
> > In this case, can we have several different flags for the RAPL
> > feature
> > and make the RAPL driver probe on different RAPL flags? Or else, a
> > model list is still needed.
> 
> Well, you asked about detecting CPUs supporting RAPL.
> 
> Now you're asking about different RAPL "subfunctionality" or
> whatever.
> 
Sorry that I was not clear enough.

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.

The goal of this is to do model list update in one place instead of 4
or more different drivers when a new model comes.

If yes, then, the second question is that,  there are cases like RAPL
which has model specific behavior. To make the driver totally clean, I'
m wondering if we can have different flags for one feature so that we
don't need to maintain the exceptions in the driver. Say, something
like

static const struct x86_cpu_id intel_pm_features[] __initconst = {
     
   X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L,           X86_FEATURE_RAPL_DEF
AULT | X86_FEATURE_TCC_COOLING),
        X86_MATCH_INTEL_FAM6_MODEL(SKYL
AKE_X,           X86_FEATURE_RAPL_FIX_DRAM | X86_FEATURE_UNCORE_FREQ),
 
... 
     
  X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE,           X86_FEATURE_RAPL_DEFA
ULT | X86_FEATURE_TCC_COOLING),
        X86_MATCH_INTEL_FAM6_MODEL(SAPPH
IRERAPIDS_X,    X86_FEATURE_RAPL_FIX_PSYS | X86_FEATURE_UNCORE_FREQ),
  
...
        {},
};

> You could do the synthetic flag for feature detection because
> apparently giving
> it a CPUID flag is soo expensive (/sarcastic eyeroll) and then you
> can pick
> apart subfeatures in the RAPL code and do flags there, away from the
> x86 arch
> code because no one should see that.

I got your point.

thanks,
rui