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, [email protected] 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.
