On Wed, Jun 01, 2016 at 07:42:02PM -0700, David Carrillo-Cisneros wrote: > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > index a5e52ad4..1ce172d 100644 > --- a/arch/x86/events/intel/core.c > +++ b/arch/x86/events/intel/core.c > @@ -3309,6 +3309,7 @@ static void intel_snb_check_microcode(void) > static bool check_msr(unsigned long msr, u64 mask) > { > u64 val_old, val_new, val_tmp; > + u64 (*wr_quirk)(u64); > > /* > * Read the current value, change it and read it back to see if it > @@ -3322,13 +3323,30 @@ static bool check_msr(unsigned long msr, u64 mask) > * Only change the bits which can be updated by wrmsrl. > */ > val_tmp = val_old ^ mask; > + > + /* Use wr quirk for lbr msr's. */ > + if ((x86_pmu.lbr_from <= msr && > + msr < x86_pmu.lbr_from + x86_pmu.lbr_nr) || > + (x86_pmu.lbr_to <= msr && > + msr < x86_pmu.lbr_to + x86_pmu.lbr_nr)) > + wr_quirk = lbr_from_signext_quirk_wr;
This is unreadable code.. static inline bool within(unsigned long msr, unsigned long base, unsigned long size) { return base <= msr && msr < base + size; } static inline bool is_lbr_msr(unsigned long msr) { if (within(msr, x86_pmu.lbr_from, x86_pmu.lbr_nr)) return true; if (within(msr, x86_pmu.lbr_to, x86_pmu.lbr_nr)) return true; return false; } Yes, its a few more lines, but its bloody obvious what it does at any time of the day. Also, seeing how the write quirk is for the LBR_FROM only, wth are you testing against _TO too?