On Wed, Nov 22, 2017 at 5:55 AM, Josh Poimboeuf <jpoim...@redhat.com> wrote: > On Wed, Nov 22, 2017 at 08:39:07AM +0100, Ingo Molnar wrote: >> >> * Andy Lutomirski <l...@kernel.org> wrote: >> >> > This sets up stack switching, including for SYSCALL. I think it's >> > in decent shape. >> > >> > Known issues: >> > - I think we're going to want a way to turn the stack switching on and >> > off either at boot time or at runtime. It should be fairly >> > straightforward >> > to make it work. >> > >> > - I think the ORC unwinder isn't so good at dealing with stack overflows. >> > It bails too early (I think), resulting in lots of ? entries. This >> > isn't a regression with this series -- it's just something that could >> > be improved. >> >> Note that with the attached config on an Intel testbox I get the following >> new ORC >> unwinder warning during bootup: >> >> [ 12.200554] calling ghash_pclmulqdqni_mod_init+0x0/0x54 @ 1 >> [ 12.209536] WARNING: can't dereference registers at ffffc90004dfff60 for >> ip error_entry+0x7d/0xd0 >> [ 12.231388] initcall ghash_pclmulqdqni_mod_init+0x0/0x54 returned 0 after >> 23480 usecs > > After the stack switch in error_entry(), the pt_regs are at a different > offset than before, so they aren't where ORC expects them to be. > > I think the below patch should fix it, by popping the return address off > the stack instead of just copying it. This way we can avoid adding > another ORC annotation because pt_regs will be at the same offset both > before and after the sync_regs() call. > > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index 2fdd2127e8e9..8dad83724469 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -1291,8 +1291,8 @@ ENTRY(error_entry) > > .Lerror_entry_from_usermode_after_swapgs: > /* Put us onto the real thread stack. */ > - leaq 8(%rsp), %rdi /* pt_regs pointer */ > - movq (%rsp), %r12 > + popq %r12 /* function return address */ > + leaq (%rsp), %rdi /* pt_regs pointer */ > call sync_regs > movq %rax, %rsp /* switch stack */ > ENCODE_FRAME_POINTER
Ah, I see -- SAVE_C_REGS contains an UNWIND_HINT_REGS hint. I had thought I needed to fix it differently. Ingo, if you want to apply this, I attached it. I'll fold it in next time I resend the whole series.
From e4b840c5a2eee3610d870703d484263226df80cc Mon Sep 17 00:00:00 2001 Message-Id: <e4b840c5a2eee3610d870703d484263226df80cc.1511363929.git.l...@kernel.org> From: Andy Lutomirski <l...@kernel.org> Date: Wed, 22 Nov 2017 07:15:51 -0800 Subject: [PATCH] Fixup for "x86/asm/64: Use a percpu trampoline stack for IDT entries" One of my sync_regs calls was confusing objtool. I'll fold this in to the series next time I refresh it. Signed-off-by: Andy Lutomirski <l...@kernel.org> --- arch/x86/entry/entry_64.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index 2fdd2127e8e9..f1cef194dfba 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -1291,8 +1291,8 @@ ENTRY(error_entry) .Lerror_entry_from_usermode_after_swapgs: /* Put us onto the real thread stack. */ - leaq 8(%rsp), %rdi /* pt_regs pointer */ - movq (%rsp), %r12 + popq %r12 /* save return addr in %12 */ + movq %rsp, %rdi /* arg0 = pt_regs pointer */ call sync_regs movq %rax, %rsp /* switch stack */ ENCODE_FRAME_POINTER -- 2.13.6
