From: Peter Zijlstra <peterz@infradead.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
"H. Peter Anvin" <hpa@zytor.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
Andy Lutomirski <luto@amacapital.net>,
Joel Fernandes <joel@joelfernandes.org>,
He Zhe <zhe.he@windriver.com>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [RFC][PATCH] tracing/x86: Save CR2 before tracing irqsoff on error_entry
Date: Thu, 21 Mar 2019 11:45:17 +0100 [thread overview]
Message-ID: <20190321104517.GM6521@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20190321090241.GL6521@hirez.programming.kicks-ass.net>
On Thu, Mar 21, 2019 at 10:02:41AM +0100, Peter Zijlstra wrote:
> On Thu, Mar 21, 2019 at 09:33:17AM +0100, Peter Zijlstra wrote:
>
> I'm thinking this problem wasn't new.
>
> > idtentry page_fault do_page_fault has_error_code=1
> > call error_entry
> > TRACE_IRQS_OFF
> > call trace_hardirqs_off*
> > <tracer stuff>
> > <fault> # modifies CR2
>
> CALL_enter_from_user_mode
> __context_tracking_exit()
> trace_user_exit(0)
> #PF
>
> > call do_page_fault
> > address = read_cr2(); /* whoopsie */
>
> And that also isn't fixed by your patch.
>
> I'm trying to make idtentry put cr2 in rdx, such that do_page_fault()
> takes address as a 3rd argument, but I'm still fighting that context
> tracking nonsense.
>
Something a little like so; completely untested and obviously needs
32bit changes too.
---
arch/x86/entry/entry_64.S | 28 ++++++++++++++++------------
arch/x86/mm/fault.c | 3 +--
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 1f0efdb7b629..776dbe7ba72e 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -879,7 +879,7 @@ 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 shift_ist=-1 read_cr2=0
ENTRY(\sym)
UNWIND_HINT_IRET_REGS offset=\has_error_code*8
@@ -901,18 +901,28 @@ ENTRY(\sym)
.if \paranoid
call paranoid_entry
+ /* returned flag: ebx=0: need swapgs on exit, ebx=1: don't need it */
.else
call error_entry
+ /* returned flag: ebx=1: CALL_enter_from_user_mode, ebx=0: don't need it */
.endif
UNWIND_HINT_REGS
- /* returned flag: ebx=0: need swapgs on exit, ebx=1: don't need it */
- .if \paranoid
+ .if \read_cr2
+ mov %cr2, %rdx /* XXX paravirt crap */
+ .endif
+
.if \shift_ist != -1
TRACE_IRQS_OFF_DEBUG /* reload IDT in case of recursion */
.else
TRACE_IRQS_OFF
.endif
+
+ .if \paranoid == 0
+ testl %ebx, %ebx
+ jz .Lno_context_tracking_\@
+ CALL_enter_from_user_mode
+.Lno_context_tracking_\@:
.endif
movq %rsp, %rdi /* pt_regs pointer */
@@ -1140,7 +1150,7 @@ idtentry xenint3 do_int3 has_error_code=0
#endif
idtentry general_protection do_general_protection has_error_code=1
-idtentry page_fault do_page_fault has_error_code=1
+idtentry page_fault do_page_fault has_error_code=1 read_cr2=1
#ifdef CONFIG_KVM_GUEST
idtentry async_page_fault do_async_page_fault has_error_code=1
@@ -1243,17 +1253,11 @@ 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
+ mov $1, %ebx /* CALL_enter_from_user_mode */
ret
.Lerror_entry_done:
- TRACE_IRQS_OFF
+ xorl %ebx, %ebx
ret
/*
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 667f1da36208..aac7a74869a2 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1560,9 +1560,8 @@ trace_page_fault_entries(unsigned long address, struct pt_regs *regs,
* exception_{enter,exit}() contains all sorts of tracepoints.
*/
dotraplinkage void notrace
-do_page_fault(struct pt_regs *regs, unsigned long error_code)
+do_page_fault(struct pt_regs *regs, unsigned long error_code, unsigned long address)
{
- unsigned long address = read_cr2(); /* Get the faulting address */
enum ctx_state prev_state;
prev_state = exception_enter();
next prev parent reply other threads:[~2019-03-21 10:45 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-21 2:15 [RFC][PATCH] tracing/x86: Save CR2 before tracing irqsoff on error_entry Steven Rostedt
2019-03-21 8:33 ` Peter Zijlstra
2019-03-21 9:02 ` Peter Zijlstra
2019-03-21 10:45 ` Peter Zijlstra [this message]
2019-03-21 13:32 ` Steven Rostedt
2019-03-21 13:55 ` Steven Rostedt
2019-03-21 17:23 ` Linus Torvalds
2019-03-21 17:22 ` Peter Zijlstra
2019-03-21 18:05 ` Andy Lutomirski
2019-03-21 18:10 ` Steven Rostedt
2019-03-21 18:27 ` Andy Lutomirski
2019-03-21 20:50 ` Peter Zijlstra
2019-03-22 2:52 ` Andy Lutomirski
2019-03-21 18:28 ` Peter Zijlstra
2019-03-21 18:55 ` Steven Rostedt
2019-03-21 19:31 ` Peter Zijlstra
2019-03-21 19:50 ` Steven Rostedt
2019-03-21 20:03 ` Peter Zijlstra
2019-03-21 20:11 ` Steven Rostedt
2019-03-21 18:18 ` Linus Torvalds
2019-03-21 18:20 ` Andy Lutomirski
2019-03-21 18:25 ` Linus Torvalds
2019-03-21 18:37 ` Peter Zijlstra
2019-03-21 18:39 ` Andy Lutomirski
2019-03-21 20:00 ` Andrew Cooper
2019-03-21 20:35 ` Steven Rostedt
2019-03-21 18:38 ` Andy Lutomirski
2019-03-21 18:42 ` Peter Zijlstra
2019-03-21 18:22 ` hpa
2019-03-22 5:54 ` Juergen Gross
2019-03-21 18:27 ` Peter Zijlstra
2019-03-21 18:28 ` Andy Lutomirski
2019-03-21 18:33 ` Peter Zijlstra
2019-03-21 13:04 ` Steven Rostedt
2019-04-17 1:52 ` He Zhe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190321104517.GM6521@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mingo@kernel.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=zhe.he@windriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.