Re: [PATCH 3/4] x86: open-code register save/restore in trace_hardirqs thunks

From: Denys Vlasenko
Date: Fri Jan 09 2015 - 13:54:33 EST


Hi Borislav, thank you for testing and finding it.

On Fri, Jan 9, 2015 at 1:19 PM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> Hmm, this patch breaks booting my kvm guest: it stops booting at some
> point and restarts itself after a couple of seconds.
>
> The monitor says rIP points to ffffffff8167ae30 which is this:
>
> ffffffff8167ae30 <async_page_fault>:
> ffffffff8167ae30: ff 15 fa 62 31 00 callq *0x3162fa(%rip) # ffffffff81991130 <pv_irq_ops+0x30>
> ffffffff8167ae36: 48 83 ec 78 sub $0x78,%rsp
> ffffffff8167ae3a: e8 d1 01 00 00 callq ffffffff8167b010 <error_entry>
> ffffffff8167ae3f: 48 89 e7 mov %rsp,%rdi
> ffffffff8167ae42: 48 8b 74 24 78 mov 0x78(%rsp),%rsi
> ffffffff8167ae47: 48 c7 44 24 78 ff ff movq $0xffffffffffffffff,0x78(%rsp)
> ffffffff8167ae4e: ff ff
> ffffffff8167ae50: e8 9b 9e 9c ff callq ffffffff81044cf0 <do_async_page_fault>
> ffffffff8167ae55: e9 76 02 00 00 jmpq ffffffff8167b0d0 <error_exit>
> ffffffff8167ae5a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)

I just looked at disassembly of thunk_64.o
before and after the patch. Here's what I see:

Before:

Disassembly of section .text:
0000000000000000 <restore-0x30>:
0: 48 83 ec 48 sub $0x48,%rsp
4: 48 89 7c 24 40 mov %rdi,0x40(%rsp)
9: 48 89 74 24 38 mov %rsi,0x38(%rsp)
e: 48 89 54 24 30 mov %rdx,0x30(%rsp)
13: 48 89 4c 24 28 mov %rcx,0x28(%rsp)
18: 48 89 44 24 20 mov %rax,0x20(%rsp)
1d: 4c 89 44 24 18 mov %r8,0x18(%rsp)
22: 4c 89 4c 24 10 mov %r9,0x10(%rsp)
27: 4c 89 54 24 08 mov %r10,0x8(%rsp)
2c: 4c 89 1c 24 mov %r11,(%rsp)
0000000000000030 <restore>:
30: 4c 8b 1c 24 mov (%rsp),%r11
34: 4c 8b 54 24 08 mov 0x8(%rsp),%r10
39: 4c 8b 4c 24 10 mov 0x10(%rsp),%r9
3e: 4c 8b 44 24 18 mov 0x18(%rsp),%r8
43: 48 8b 44 24 20 mov 0x20(%rsp),%rax
48: 48 8b 4c 24 28 mov 0x28(%rsp),%rcx
4d: 48 8b 54 24 30 mov 0x30(%rsp),%rdx
52: 48 8b 74 24 38 mov 0x38(%rsp),%rsi
57: 48 8b 7c 24 40 mov 0x40(%rsp),%rdi
5c: 48 83 c4 48 add $0x48,%rsp
60: c3 retq

After:

Disassembly of section .text:
0000000000000000 <restore>:
0: 4c 8b 1c 24 mov (%rsp),%r11
4: 4c 8b 54 24 08 mov 0x8(%rsp),%r10
9: 4c 8b 4c 24 10 mov 0x10(%rsp),%r9
e: 4c 8b 44 24 18 mov 0x18(%rsp),%r8
13: 48 8b 44 24 20 mov 0x20(%rsp),%rax
18: 48 8b 4c 24 28 mov 0x28(%rsp),%rcx
1d: 48 8b 54 24 30 mov 0x30(%rsp),%rdx
22: 48 8b 74 24 38 mov 0x38(%rsp),%rsi
27: 48 8b 7c 24 40 mov 0x40(%rsp),%rdi
2c: 48 03 24 25 48 00 00 add 0x48,%rsp
33: 00
34: c3 retq

IOW, my patch, on the level of generated assembly, results only in removal
of unreachable "SAVE_ARGS" thing.

I looked into git history all the way back to 2005. The part

+ /* SAVE_ARGS below is used only for the .cfi directives it contains. */
+ CFI_STARTPROC
+ SAVE_ARGS
+restore:

was there in the very first git commit.

I don't see how this SAVE_ARGS can affect anything. It *is* unreachable,
right?

Does kvm guest code really parse and use CFI data in its operation?
That's the only way the breakage can be explained.

In order to narrow it down, can you, instead of my patch, try
just deleting this one line, and see whether breakage appears?

Thanks!
--
vda
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/