RE: [PATCH v2] [LBR] Dump LBRs on Exception

["Berthier, Emmanuel" <emmanuel.berthier@intel.com>]

> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Thursday, November 27, 2014 10:23 PM
> To: Berthier, Emmanuel
> Cc: H. Peter Anvin; x86@kernel.org; Jarzmik, Robert; LKML; Andy Lutomirski
> Subject: Re: [PATCH v2] [LBR] Dump LBRs on Exception
> 
> On Thu, 27 Nov 2014, Emmanuel Berthier wrote:
> > diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > index 45fa730..0a69365 100644
> > --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > @@ -4,7 +4,7 @@
> >  #include <asm/perf_event.h>
> >  #include <asm/msr.h>
> >  #include <asm/insn.h>
> > -
> 
> This newline is intentional to seperate asm includes from the local one.

Got it.

> >  static void __intel_pmu_lbr_enable(void)  {
> >  	u64 debugctl;
> >  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> >
> > +	lbr_set_used_by_perf(true);
> 
> This cannot work.
> 
> CPU0					CPU1
> 
> __intel_pmu_lbr_enable()
>    lbr_set_used_by_perf(true);
> 					__intel_pmu_lbr_disable()
> 					  lbr_set_used_by_perf(false);
> 
> This is a per cpu property.
> 
> And there is more to that. Let's look at a single CPU.
> 
> lbr for oops is enabled
> 
> context switch()
>    __intel_pmu_lbr_enable()	-> LBR used by perf, oops dumper disabled
> 
> context switch()
>    __intel_pmu_lbr_disable()	-> LBR not longer used by perf, oops
>    				   dumper enabled
> 
> So after that context switch we crash in the kernel and LBR is empty because
> we did disable it at the context switch.
> 
> So you need per cpu state, which handles the LBR dumper state:
> 
> #define LBR_OOPS_DISABLED 0x01
> #define LBR_PERF_USAGE	  0x02
> 
> DEFINE_PER_CPU(unsigned long, lbr_dump_state) = LBR_OOPS_DISABLED;
> 
> lbr_perf_enable()
> 	this_cpu_add(lbr_dump_state, LBR_PERF_USAGE);
> 
> lbr_perf_disable()
> 	if (!this_cpu_sub_return(lbr_dump_state, LBR_PERF_USAGE))
> 	   	enable_lbr_oops();
> 
> Now of course you need to handle this in the exception path per cpu as well.

Agree, I will do that.

Thx.
Emmanuel.