lkml.org 
[lkml]   [2019]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [RFC][PATCH] tracing/x86: Save CR2 before tracing irqsoff on error_entry
On Thu, Mar 21, 2019 at 11:10 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 21 Mar 2019 11:05:06 -0700
> Andy Lutomirski <luto@amacapital.net> wrote:
>
> > In the long run, I think the right solution is to rewrite even more of
> > this mess in C. We really ought to be able to put the IRQ flag
> > tracing and the context tracking into C code.
>
> And once we do that, we can work on getting the irq tracing
> incorporated into a jump_label type that we could possibly enable
> lockdep at start up, and then disable it later, even on production
> systems! That is, to be able to turn it off and bring the system back
> up to full speed.
>
> It would also allow for irq tracing too.
>
> Some inquiring minds have been asking about this ;-)
>

Well, here's pass zero at this. Untested, because it obviously
doesn't work. Here are just a few things that are almost certainly
wrong with it

- The assertion that entries return with IRQs off will blow up,
because a bunch of the entries can return with IRQs on. Needs fixing.

- native_load_gs_index needs to be re-added as a static inline
somewhere or maybe even as a real function because of PV. Sigh.

- The IRQ tracing needs to be re-added.

- Some real semantics need to be defined for precisely what code is
responsible for tracing.

- We need some asm-callable assertions to check the following
conditions as appropriate:

(a) that IRQ flags are currently traced as off.
(b) that IRQ flags are currently traced to match the IRET frame.
(c) that our context tracking is currently in good shape. I'm not
100% sure how to define this.


- We need to do some serious don't-instrument-me stuff to all the C
entries, since we're now in an awful context when calling them.

- I haven't touched IRQs at all.
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 1f0efdb7b629..0ae05724eafc 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -167,15 +167,11 @@ GLOBAL(entry_SYSCALL_64_after_hwframe)

PUSH_AND_CLEAR_REGS rax=$-ENOSYS

- TRACE_IRQS_OFF
-
/* IRQs are off. */
movq %rax, %rdi
movq %rsp, %rsi
call do_syscall_64 /* returns with IRQs disabled */

- TRACE_IRQS_IRETQ /* we're about to change IF */
-
/*
* Try to use SYSRET instead of IRET if we're returning to
* a completely clean 64-bit userspace context. If we're not,
@@ -342,7 +338,6 @@ ENTRY(ret_from_fork)
UNWIND_HINT_REGS
movq %rsp, %rdi
call syscall_return_slowpath /* returns with IRQs disabled */
- TRACE_IRQS_ON /* user mode is traced as IRQS on */
jmp swapgs_restore_regs_and_return_to_usermode

1:
@@ -595,7 +590,6 @@ ret_from_intr:
GLOBAL(retint_user)
mov %rsp,%rdi
call prepare_exit_to_usermode
- TRACE_IRQS_IRETQ

GLOBAL(swapgs_restore_regs_and_return_to_usermode)
#ifdef CONFIG_DEBUG_ENTRY
@@ -651,10 +645,6 @@ retint_kernel:
jmp 0b
1:
#endif
- /*
- * The iretq could re-enable interrupts:
- */
- TRACE_IRQS_IRETQ

GLOBAL(restore_regs_and_return_to_kernel)
#ifdef CONFIG_DEBUG_ENTRY
@@ -879,15 +869,10 @@ apicinterrupt IRQ_WORK_VECTOR irq_work_interrupt smp_irq_work_interrupt
* @paranoid == 2 is special: the stub will never switch stacks. This is for
* #DF: if the thread stack is somehow unusable, we'll still get a useful OOPS.
*/
-.macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
+.macro idtentry sym do_sym has_error_code:req paranoid=0
ENTRY(\sym)
UNWIND_HINT_IRET_REGS offset=\has_error_code*8

- /* Sanity check */
- .if \shift_ist != -1 && \paranoid == 0
- .error "using shift_ist requires paranoid=1"
- .endif
-
ASM_CLAC

.if \has_error_code == 0
@@ -907,14 +892,6 @@ ENTRY(\sym)
UNWIND_HINT_REGS
/* returned flag: ebx=0: need swapgs on exit, ebx=1: don't need it */

- .if \paranoid
- .if \shift_ist != -1
- TRACE_IRQS_OFF_DEBUG /* reload IDT in case of recursion */
- .else
- TRACE_IRQS_OFF
- .endif
- .endif
-
movq %rsp, %rdi /* pt_regs pointer */

.if \has_error_code
@@ -924,17 +901,9 @@ ENTRY(\sym)
xorl %esi, %esi /* no error code */
.endif

- .if \shift_ist != -1
- subq $EXCEPTION_STKSZ, CPU_TSS_IST(\shift_ist)
- .endif
-
call \do_sym

- .if \shift_ist != -1
- addq $EXCEPTION_STKSZ, CPU_TSS_IST(\shift_ist)
- .endif
-
- /* these procedures expect "no swapgs" flag in ebx */
+ /* paranoid_exit expects "no swapgs" flag in ebx */
.if \paranoid
jmp paranoid_exit
.else
@@ -986,18 +955,14 @@ idtentry simd_coprocessor_error do_simd_coprocessor_error has_error_code=0
* Reload gs selector with exception handling
* edi: new selector
*/
-ENTRY(native_load_gs_index)
+ENTRY(native_load_gs_index_irqs_off)
+ DEBUG_ENTRY_ASSERT_IRQS_OFF
FRAME_BEGIN
- pushfq
- DISABLE_INTERRUPTS(CLBR_ANY & ~CLBR_RDI)
- TRACE_IRQS_OFF
SWAPGS
.Lgs_change:
movl %edi, %gs
2: ALTERNATIVE "", "mfence", X86_BUG_SWAPGS_FENCE
SWAPGS
- TRACE_IRQS_FLAGS (%rsp)
- popfq
FRAME_END
ret
ENDPROC(native_load_gs_index)
@@ -1243,17 +1208,7 @@ ENTRY(error_entry)
ENCODE_FRAME_POINTER
pushq %r12

- /*
- * We need to tell lockdep that IRQs are off. We can't do this until
- * we fix gsbase, and we should do it before enter_from_user_mode
- * (which can take locks).
- */
- TRACE_IRQS_OFF
- CALL_enter_from_user_mode
- ret
-
.Lerror_entry_done:
- TRACE_IRQS_OFF
ret

/*
@@ -1306,8 +1261,7 @@ END(error_entry)

ENTRY(error_exit)
UNWIND_HINT_REGS
- DISABLE_INTERRUPTS(CLBR_ANY)
- TRACE_IRQS_OFF
+ DEBUG_ENTRY_ASSERT_IRQS_OFF
testb $3, CS(%rsp)
jz retint_kernel
jmp retint_user
\
 
 \ /
  Last update: 2019-03-21 19:28    [W:0.205 / U:0.956 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site