linux-kernel - Re: [PATCH 11/21] perf/x86/intel/lbr: Support LBR

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: <20200619192211.GA576871@hirez.programming.kicks-ass.net>
Date:   Fri, 19 Jun 2020 21:22:11 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Liang, Kan" <kan.liang@...ux.intel.com>
Cc:     mingo@...hat.com, acme@...nel.org, tglx@...utronix.de,
        bp@...en8.de, x86@...nel.org, linux-kernel@...r.kernel.org,
        mark.rutland@....com, alexander.shishkin@...ux.intel.com,
        jolsa@...hat.com, namhyung@...nel.org, dave.hansen@...el.com,
        yu-cheng.yu@...el.com, bigeasy@...utronix.de, gorcunov@...il.com,
        hpa@...or.com, alexey.budankov@...ux.intel.com, eranian@...gle.com,
        ak@...ux.intel.com, like.xu@...ux.intel.com,
        yao.jin@...ux.intel.com
Subject: Re: [PATCH 11/21] perf/x86/intel/lbr: Support LBR_CTL

On Fri, Jun 19, 2020 at 03:15:09PM -0400, Liang, Kan wrote:
> 
> 
> On 6/19/2020 2:40 PM, Peter Zijlstra wrote:
> > On Fri, Jun 19, 2020 at 07:03:59AM -0700, kan.liang@...ux.intel.com wrote:
> > > -	if (x86_pmu.extra_regs || x86_pmu.lbr_sel_map) {
> > > +	if (x86_pmu.extra_regs || x86_pmu.lbr_sel_map || x86_pmu.lbr_ctl_map) {
> > 
> > > +	union {
> > > +		u64		lbr_sel_mask;		   /* LBR_SELECT valid bits */
> > > +		u64		lbr_ctl_mask;		   /* LBR_CTL valid bits */
> > > +	};
> > 
> > This makes absolutely no sense. There is hoping the compiler realizes
> > how stupid that is and fixes it for you, but shees.
> > 
> 
> The lbr_ctl_map and the lbr_ctl_mask are two different things.
> 
> The lbr_ctl_map stores the mapping from PERF_SAMPLE_BRANCH_* to the
> corresponding filtering bits in LBR_CTL MSR. It is used to replace the old
> lbr_sel_map. The mapping information in the old lbr_sel_map is hard coded,
> and has a const type. But for arch LBR, the LBR filtering capabilities are
> enumerated from CPUID. We should not hard code the mapping. So I add a new
> variable lbr_ctl_map.
> 
>  	const int	*lbr_sel_map;		   /* lbr_select mappings */
> +	int		*lbr_ctl_map;		   /* LBR_CTL mappings */
> 
> 
> I think we cannot reuse the old lbr_sel_map for the lbr_ctl_map.

Of course you can, you just did it, they're the exact same variable, you
just got confused with all the naming nonsense. You then got further
confused and ended up writing code that checked if a variable was not 0
twice, just to make sure.