On Fri, Nov 28, 2014 at 12:44 AM, Berthier, Emmanuel <emmanuel.berth...@intel.com> wrote: >> -----Original Message----- >> From: Andy Lutomirski [mailto:l...@amacapital.net] >> Sent: Thursday, November 27, 2014 10:56 PM >> To: Thomas Gleixner >> Cc: Berthier, Emmanuel; H. Peter Anvin; X86 ML; Jarzmik, Robert; LKML >> Subject: Re: [PATCH v2] [LBR] Dump LBRs on Exception >> >> On Thu, Nov 27, 2014 at 1:22 PM, Thomas Gleixner <t...@linutronix.de> >> wrote: >> >> /* >> >> * Exception entry points. >> >> */ >> >> @@ -1063,6 +1103,8 @@ ENTRY(\sym) >> >> subq $ORIG_RAX-R15, %rsp >> >> CFI_ADJUST_CFA_OFFSET ORIG_RAX-R15 >> >> >> >> + STOP_LBR >> > >> > We really cannot do this unconditionally for every exception. This >> > wants to be conditional, i.e. >> > >> > .if \stop_lbr >> > cond_stop_lbr >> > .endif >> > >> > So we can select which exceptions actually get that treatment. >> > do_page_fault is probably the only one which is interesting here. >> > >> > Now looking at your macro maze, I really wonder whether we can do it a >> > little bit less convoluted. We need to push/pop registers. error_entry >> > saves the registers already and has a (admitedly convoluted) >> > kernel/user space check. But we might be able to do something sane >> > there. Cc'ing Andy as he is the master of that universe. >> > >> >> Can one of you give me some context as to what this code is intended to do? >> I haven't followed the thread. >> >> In particular, knowing why this needs to be in asm instead of in C would be >> nice, because asm in entry_64.S has an amazing ability to have little bugs >> hiding for years. >> >> There's also the caveat that, especialy for the IST exceptions, you're >> running >> in a weird context in which lots of things that are usually safe are >> verboten. >> Page faults can be tricky too, though. >> >> --Andy > > Welcome Andy. > The global purpose of this patch is to disable/enable LBR during exception > handling and dump them later in the Panic process in order to get a small > instruction trace which could help in case of stack corruption by example. > This has to be done at the very early stage of exception handling as LBR > contains few records (8 or 16) and we do not want to flush useful ones (those > before the exception), so this code should avoid executing any > jump/branch/call before stopping the LBR. > > The proposed patch regarding asm code is as follow: > > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index > df088bb..f39cded 100644 > --- a/arch/x86/kernel/entry_64.S > +++ b/arch/x86/kernel/entry_64.S > @@ -1035,6 +1035,46 @@ apicinterrupt IRQ_WORK_VECTOR \ > irq_work_interrupt smp_irq_work_interrupt #endif > > +.macro STOP_LBR > +#ifdef CONFIG_LBR_DUMP_ON_EXCEPTION > + testl $3,CS+8(%rsp) /* Kernel Space? */ > + jz 1f > + testl $1, lbr_dump_on_exception
Is there a guarantee that, if lbr_dump_on_exception is true, then LBR is on? What happens if you schedule between stopping and resuming LBR? > + jz 1f > + push %rax > + push %rcx > + push %rdx > + movl $MSR_IA32_DEBUGCTLMSR, %ecx > + rdmsr > + and $~1, %eax /* Disable LBR recording */ > + wrmsr wrmsr is rather slow. Have you checked whether this is faster than just saving the LBR trace on exception entry? --Andy > + pop %rdx > + pop %rcx > + pop %rax > +1: > +#endif > +.endm > + > +.macro START_LBR > +#ifdef CONFIG_LBR_DUMP_ON_EXCEPTION > + testl $3,CS+8(%rsp) /* Kernel Space? */ > + jz 1f > + testl $1, lbr_dump_on_exception > + jz 1f > + push %rax > + push %rcx > + push %rdx > + movl $MSR_IA32_DEBUGCTLMSR, %ecx > + rdmsr > + or $1, %eax /* Enable LBR recording */ > + wrmsr > + pop %rdx > + pop %rcx > + pop %rax > +1: > +#endif > +.endm > + > /* > * Exception entry points. > */ > @@ -1063,6 +1103,8 @@ ENTRY(\sym) > subq $ORIG_RAX-R15, %rsp > CFI_ADJUST_CFA_OFFSET ORIG_RAX-R15 > > + STOP_LBR > + > .if \paranoid > call save_paranoid > .else > @@ -1094,6 +1136,8 @@ ENTRY(\sym) > > call \do_sym > > + START_LBR > + > .if \shift_ist != -1 > addq $EXCEPTION_STKSZ, INIT_TSS_IST(\shift_ist) > .endif > -- > 1.7.9.5 > > Thanks, > Emmanuel. -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
