linux-kernel - Re: [PATCH v3 08/10] perf/x86/rapl: Modify the generic variable names to *

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <bf76302d2d427522d4842cb1df8f58700cb669d4.camel@intel.com>
Date: Fri, 5 Jul 2024 02:18:30 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "peterz@...radead.org" <peterz@...radead.org>, "Dhananjay.Ugwekar@....com"
	<Dhananjay.Ugwekar@....com>
CC: "gautham.shenoy@....com" <gautham.shenoy@....com>,
	"alexander.shishkin@...ux.intel.com" <alexander.shishkin@...ux.intel.com>,
	"ananth.narayan@....com" <ananth.narayan@....com>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"ravi.bangoria@....com" <ravi.bangoria@....com>, "Hunter, Adrian"
	<adrian.hunter@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "mingo@...hat.com" <mingo@...hat.com>,
	"oleksandr@...alenko.name" <oleksandr@...alenko.name>, "irogers@...gle.com"
	<irogers@...gle.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"gustavoars@...nel.org" <gustavoars@...nel.org>, "kan.liang@...ux.intel.com"
	<kan.liang@...ux.intel.com>, "kees@...nel.org" <kees@...nel.org>,
	"sandipan.das@....com" <sandipan.das@....com>, "mark.rutland@....com"
	<mark.rutland@....com>, "linux-perf-users@...r.kernel.org"
	<linux-perf-users@...r.kernel.org>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>, "bp@...en8.de" <bp@...en8.de>,
	"acme@...nel.org" <acme@...nel.org>, "kprateek.nayak@....com"
	<kprateek.nayak@....com>, "jolsa@...nel.org" <jolsa@...nel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>, "x86@...nel.org"
	<x86@...nel.org>, "namhyung@...nel.org" <namhyung@...nel.org>
Subject: Re: [PATCH v3 08/10] perf/x86/rapl: Modify the generic variable names
 to *_pkg*

On Wed, 2024-07-03 at 12:01 +0530, Dhananjay Ugwekar wrote:
> Hi Rui,
> 
> On 7/3/2024 9:37 AM, Zhang, Rui wrote:
> > On Tue, 2024-07-02 at 15:50 +0530, Dhananjay Ugwekar wrote:
> > > 
> > > > 
> > > > For Intel products, we have
> > > > 1. Casecadelake-AP which has multi-die per package and has per-
> > > > die
> > > > RAPL
> > > > MSRs
> > > > 2. all other platforms which has single-die per package, so its
> > > > RAPL
> > > > MSRs can be considered as either package-scope or die-scope
> > > > This applies to Thermal MSRs as well.
> > > > 
> > > > so for these MSRs, we can treat them as
> > > > 1. always die-scope for all existing platforms
> > > > or
> > > > 2. package-scope with the exception of Casecadelake-ap
> > > > And current kernel code follows rule 1.
> > > > 
> > > > I propose we switch to rule 2 for these code because rule 1 can
> > > > be
> > > > broke on future multi-die systems (This is already true for
> > > > Thermal
> > > > MSRs).
> > > 
> > > I have a doubt about this, won't the future Intel multi-die
> > > systems 
> > > have die-scope for the RAPL PMU like Casecadelake-AP?
> > 
> > For future multi-die systems that I know, the RAPL is still package
> > scope 
> 
> I think in that case we can go with rule 2, it would be future proof
> for Intel systems. If you agree, I can make the change in next
> version.
> 
> Something like below?,
> 
> -#define rapl_pmu_is_pkg_scope()                         \
> -        (boot_cpu_data.x86_vendor == X86_VENDOR_AMD || 
> \                                                                    
>                                                                      
>                          
> -        boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
> 
> +#define rapl_pmu_is_die_scope()                        \
> +       (boot_cpu_data.x86_model_id == CASCADELAKE)
> 
sounds good to me. Just a reminder that using boot_cpu_data.vfm is a
better choice here.

And it would be great to get Peter' view on this.

thanks,
rui

> Regards,
> Dhananjay
> 
> but it is just lucky that RAPL control is not exposed via the
> > MSRs so rule 1 is not actually broke for RAPL PMU (while it is
> > indeed
> > broken for other drivers like thermal).
> > 
> > In short, if we stick with rule 1, the RAPL PMU still works.
> > Switching> to rule 2 to be consistent with the other drivers is
> > also a choice IMV.> 
> > thanks,
> > rui
> > > 
> > > If yes, then rule 1 above seems better.
> > > 
> > > Regards,
> > > Dhananjay
> > > 
> > > > 
> > > > In this sense, I think it is okay to call it pkg level rapl for
> > > > both
> > > > Intel and AMD.
> > > > 
> > > > thanks,
> > > > rui
> > > > 
> > 
>