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?

Reply via email to