* [PATCH 2/5] x86: open-code register save/restore in trace_hardirqs thunks
2014-08-01 14:48 [PATCH 1/5] x86: entry_64.S: delete unused code Denys Vlasenko
@ 2014-08-01 14:48 ` Denys Vlasenko
2014-08-01 14:48 ` [PATCH 3/5] x86: entry_64.S: fold SAVE_ARGS_IRQ macro into its sole user Denys Vlasenko
` (3 subsequent siblings)
4 siblings, 0 replies; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-01 14:48 UTC (permalink / raw)
To: linux-kernel
Cc: Denys Vlasenko, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
Frederic Weisbecker, X86 ML, Alexei Starovoitov, Will Drewry,
Kees Cook
This is a preparatory patch for change in "struct pt_regs"
handling in entry_64.S.
trace_hardirqs thunks were (ab)using a part of pt_regs
handling code, namely SAVE_ARGS/RESTORE_ARGS macros,
to save/restore registers across C function calls.
Since SAVE_ARGS is going to be changed, open-code
register saving/restoring here.
Incidentally, this removes a bit of dead code:
one SAVE_ARGS was just to emit a CFI annotation,
but it also generated unreachable assembly insns.
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Oleg Nesterov <oleg@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: X86 ML <x86@kernel.org>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: linux-kernel@vger.kernel.org
---
arch/x86/lib/thunk_64.S | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/arch/x86/lib/thunk_64.S b/arch/x86/lib/thunk_64.S
index 92d9fea..c1c9131 100644
--- a/arch/x86/lib/thunk_64.S
+++ b/arch/x86/lib/thunk_64.S
@@ -16,10 +16,20 @@
\name:
CFI_STARTPROC
- /* this one pushes 9 elems, the next one would be %rIP */
- SAVE_ARGS
+ subq $9*8, %rsp
+ CFI_ADJUST_CFA_OFFSET 9*8
+ movq_cfi rdi, 8*8
+ movq_cfi rsi, 7*8
+ movq_cfi rdx, 6*8
+ movq_cfi rcx, 5*8
+ movq_cfi rax, 4*8
+ movq_cfi r8, 3*8
+ movq_cfi r9, 2*8
+ movq_cfi r10, 1*8
+ movq_cfi r11, 0*8
.if \put_ret_addr_in_rdi
+ /* 9*8(%rsp) is return addr on stack */
movq_cfi_restore 9*8, rdi
.endif
@@ -38,11 +48,20 @@
THUNK lockdep_sys_exit_thunk,lockdep_sys_exit
#endif
- /* SAVE_ARGS below is used only for the .cfi directives it contains. */
CFI_STARTPROC
- SAVE_ARGS
+ CFI_ADJUST_CFA_OFFSET 9*8
restore:
- RESTORE_ARGS
+ movq_cfi_restore 0*8, r11
+ movq_cfi_restore 1*8, r10
+ movq_cfi_restore 2*8, r9
+ movq_cfi_restore 3*8, r8
+ movq_cfi_restore 4*8, rax
+ movq_cfi_restore 5*8, rcx
+ movq_cfi_restore 6*8, rdx
+ movq_cfi_restore 7*8, rsi
+ movq_cfi_restore 8*8, rdi
+ addq 9*8, %rsp
+ CFI_ADJUST_CFA_OFFSET -9*8
ret
CFI_ENDPROC
_ASM_NOKPROBE(restore)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 3/5] x86: entry_64.S: fold SAVE_ARGS_IRQ macro into its sole user
2014-08-01 14:48 [PATCH 1/5] x86: entry_64.S: delete unused code Denys Vlasenko
2014-08-01 14:48 ` [PATCH 2/5] x86: open-code register save/restore in trace_hardirqs thunks Denys Vlasenko
@ 2014-08-01 14:48 ` Denys Vlasenko
2014-08-01 18:30 ` Frederic Weisbecker
2014-08-01 14:48 ` [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs" Denys Vlasenko
` (2 subsequent siblings)
4 siblings, 1 reply; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-01 14:48 UTC (permalink / raw)
To: linux-kernel
Cc: Denys Vlasenko, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
Frederic Weisbecker, X86 ML, Alexei Starovoitov, Will Drewry,
Kees Cook
No code changes.
This is a preparatory patch for change in "struct pt_regs" handling.
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Oleg Nesterov <oleg@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: X86 ML <x86@kernel.org>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: linux-kernel@vger.kernel.org
---
arch/x86/kernel/entry_64.S | 88 ++++++++++++++++++++++------------------------
1 file changed, 42 insertions(+), 46 deletions(-)
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index dbfd037..37f7d95 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -218,51 +218,6 @@ ENDPROC(native_usergs_sysret64)
CFI_REL_OFFSET r15, R15+\offset
.endm
-/* save partial stack frame */
- .macro SAVE_ARGS_IRQ
- cld
- /* start from rbp in pt_regs and jump over */
- movq_cfi rdi, (RDI-RBP)
- movq_cfi rsi, (RSI-RBP)
- movq_cfi rdx, (RDX-RBP)
- movq_cfi rcx, (RCX-RBP)
- movq_cfi rax, (RAX-RBP)
- movq_cfi r8, (R8-RBP)
- movq_cfi r9, (R9-RBP)
- movq_cfi r10, (R10-RBP)
- movq_cfi r11, (R11-RBP)
-
- /* Save rbp so that we can unwind from get_irq_regs() */
- movq_cfi rbp, 0
-
- /* Save previous stack value */
- movq %rsp, %rsi
-
- leaq -RBP(%rsp),%rdi /* arg1 for handler */
- testl $3, CS-RBP(%rsi)
- je 1f
- SWAPGS
- /*
- * irq_count is used to check if a CPU is already on an interrupt stack
- * or not. While this is essentially redundant with preempt_count it is
- * a little cheaper to use a separate counter in the PDA (short of
- * moving irq_enter into assembly, which would be too much work)
- */
-1: incl PER_CPU_VAR(irq_count)
- cmovzq PER_CPU_VAR(irq_stack_ptr),%rsp
- CFI_DEF_CFA_REGISTER rsi
-
- /* Store previous stack value */
- pushq %rsi
- CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
- 0x77 /* DW_OP_breg7 */, 0, \
- 0x06 /* DW_OP_deref */, \
- 0x08 /* DW_OP_const1u */, SS+8-RBP, \
- 0x22 /* DW_OP_plus */
- /* We entered an interrupt context - irqs are off: */
- TRACE_IRQS_OFF
- .endm
-
ENTRY(save_paranoid)
XCPT_FRAME 1 RDI+8
cld
@@ -731,7 +686,48 @@ END(interrupt)
/* reserve pt_regs for scratch regs and rbp */
subq $ORIG_RAX-RBP, %rsp
CFI_ADJUST_CFA_OFFSET ORIG_RAX-RBP
- SAVE_ARGS_IRQ
+ cld
+ /* start from rbp in pt_regs and jump over */
+ movq_cfi rdi, (RDI-RBP)
+ movq_cfi rsi, (RSI-RBP)
+ movq_cfi rdx, (RDX-RBP)
+ movq_cfi rcx, (RCX-RBP)
+ movq_cfi rax, (RAX-RBP)
+ movq_cfi r8, (R8-RBP)
+ movq_cfi r9, (R9-RBP)
+ movq_cfi r10, (R10-RBP)
+ movq_cfi r11, (R11-RBP)
+
+ /* Save rbp so that we can unwind from get_irq_regs() */
+ movq_cfi rbp, 0
+
+ /* Save previous stack value */
+ movq %rsp, %rsi
+
+ leaq -RBP(%rsp),%rdi /* arg1 for handler */
+ testl $3, CS-RBP(%rsi)
+ je 1f
+ SWAPGS
+ /*
+ * irq_count is used to check if a CPU is already on an interrupt stack
+ * or not. While this is essentially redundant with preempt_count it is
+ * a little cheaper to use a separate counter in the PDA (short of
+ * moving irq_enter into assembly, which would be too much work)
+ */
+1: incl PER_CPU_VAR(irq_count)
+ cmovzq PER_CPU_VAR(irq_stack_ptr),%rsp
+ CFI_DEF_CFA_REGISTER rsi
+
+ /* Store previous stack value */
+ pushq %rsi
+ CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
+ 0x77 /* DW_OP_breg7 */, 0, \
+ 0x06 /* DW_OP_deref */, \
+ 0x08 /* DW_OP_const1u */, SS+8-RBP, \
+ 0x22 /* DW_OP_plus */
+ /* We entered an interrupt context - irqs are off: */
+ TRACE_IRQS_OFF
+
call \func
.endm
--
1.8.1.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 3/5] x86: entry_64.S: fold SAVE_ARGS_IRQ macro into its sole user
2014-08-01 14:48 ` [PATCH 3/5] x86: entry_64.S: fold SAVE_ARGS_IRQ macro into its sole user Denys Vlasenko
@ 2014-08-01 18:30 ` Frederic Weisbecker
0 siblings, 0 replies; 41+ messages in thread
From: Frederic Weisbecker @ 2014-08-01 18:30 UTC (permalink / raw)
To: Denys Vlasenko
Cc: linux-kernel, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
X86 ML, Alexei Starovoitov, Will Drewry, Kees Cook
On Fri, Aug 01, 2014 at 04:48:16PM +0200, Denys Vlasenko wrote:
> No code changes.
>
> This is a preparatory patch for change in "struct pt_regs" handling.
>
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
One macro dissolved == one Hop-o'-My-Thumb pebble less to drop before taking
a jump on the editor.
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 14:48 [PATCH 1/5] x86: entry_64.S: delete unused code Denys Vlasenko
2014-08-01 14:48 ` [PATCH 2/5] x86: open-code register save/restore in trace_hardirqs thunks Denys Vlasenko
2014-08-01 14:48 ` [PATCH 3/5] x86: entry_64.S: fold SAVE_ARGS_IRQ macro into its sole user Denys Vlasenko
@ 2014-08-01 14:48 ` Denys Vlasenko
2014-08-01 17:04 ` Andy Lutomirski
` (5 more replies)
2014-08-01 14:48 ` [PATCH 5/5] x86: mass removal of ARGOFFSET Denys Vlasenko
2014-08-01 18:00 ` [PATCH 1/5] x86: entry_64.S: delete unused code Frederic Weisbecker
4 siblings, 6 replies; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-01 14:48 UTC (permalink / raw)
To: linux-kernel
Cc: Denys Vlasenko, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
Frederic Weisbecker, X86 ML, Alexei Starovoitov, Will Drewry,
Kees Cook
64-bit code was using six stack slots fewer by not saving/restoring
registers which a callee-preserved according to C ABI,
and not allocating space for them.
Only when syscall needed a complete "struct pt_regs",
the complete area was allocated and filled in.
This proved to be a source of significant obfuscation and subtle bugs.
For example, stub_fork had to pop the return address,
extend the struct, save registers, and push return address back. Ugly.
ia32_ptregs_common pops return address and "returns" via jmp insn,
throwing a wrench into CPU return stack cache.
This patch changes code to always allocate a complete "struct pt_regs".
The saving of registers is still done lazily.
Macros which manipulate "struct pt_regs" on stack are reworked:
ALLOC_PTREGS_ON_STACK allocates the structure.
SAVE_C_REGS saves to it those registers which are clobbered by C code.
SAVE_EXTRA_REGS saves to it all other registers.
Corresponding RESTORE_* and REMOVE_PTREGS_FROM_STACK macros reverse it.
ia32_ptregs_common, stub_fork and friends lost their ugly dance with
return pointer.
LOAD_ARGS32 in ia32entry.S now uses a symbolic stack offsets
instead of magic numbers.
Misleading and slightly wrong comments in "struct pt_regs" are fixed
(four instances).
Patch was run-tested: 64-bit executables, 32-bit executables,
strace works.
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Oleg Nesterov <oleg@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: X86 ML <x86@kernel.org>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: linux-kernel@vger.kernel.org
---
arch/x86/ia32/ia32entry.S | 47 +++----
arch/x86/include/asm/calling.h | 224 ++++++++++++++++-----------------
arch/x86/include/asm/irqflags.h | 4 +-
arch/x86/include/asm/ptrace.h | 13 +-
arch/x86/include/uapi/asm/ptrace-abi.h | 16 ++-
arch/x86/include/uapi/asm/ptrace.h | 13 +-
arch/x86/kernel/entry_64.S | 132 ++++++++-----------
arch/x86/kernel/preempt.S | 16 ++-
8 files changed, 232 insertions(+), 233 deletions(-)
diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 4299eb0..ef9ee16 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -62,12 +62,12 @@
*/
.macro LOAD_ARGS32 offset, _r9=0
.if \_r9
- movl \offset+16(%rsp),%r9d
+ movl \offset+R9(%rsp),%r9d
.endif
- movl \offset+40(%rsp),%ecx
- movl \offset+48(%rsp),%edx
- movl \offset+56(%rsp),%esi
- movl \offset+64(%rsp),%edi
+ movl \offset+RCX(%rsp),%ecx
+ movl \offset+RDX(%rsp),%edx
+ movl \offset+RSI(%rsp),%esi
+ movl \offset+RDI(%rsp),%edi
movl %eax,%eax /* zero extension */
.endm
@@ -144,7 +144,8 @@ ENTRY(ia32_sysenter_target)
CFI_REL_OFFSET rip,0
pushq_cfi %rax
cld
- SAVE_ARGS 0,1,0
+ ALLOC_PTREGS_ON_STACK
+ SAVE_C_REGS_EXCEPT_R891011
/* no need to do an access_ok check here because rbp has been
32bit zero extended */
ASM_STAC
@@ -172,7 +173,8 @@ sysexit_from_sys_call:
andl $~0x200,EFLAGS-R11(%rsp)
movl RIP-R11(%rsp),%edx /* User %eip */
CFI_REGISTER rip,rdx
- RESTORE_ARGS 0,24,0,0,0,0
+ RESTORE_RSI_RDI
+ REMOVE_PTREGS_FROM_STACK 8*3
xorq %r8,%r8
xorq %r9,%r9
xorq %r10,%r10
@@ -240,13 +242,13 @@ sysenter_tracesys:
testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
jz sysenter_auditsys
#endif
- SAVE_REST
+ SAVE_EXTRA_REGS
CLEAR_RREGS
movq $-ENOSYS,RAX(%rsp)/* ptrace can change this for a bad syscall */
movq %rsp,%rdi /* &pt_regs -> arg1 */
call syscall_trace_enter
LOAD_ARGS32 ARGOFFSET /* reload args from stack in case ptrace changed it */
- RESTORE_REST
+ RESTORE_EXTRA_REGS
cmpq $(IA32_NR_syscalls-1),%rax
ja int_ret_from_sys_call /* sysenter_tracesys has set RAX(%rsp) */
jmp sysenter_do_call
@@ -288,7 +290,8 @@ ENTRY(ia32_cstar_target)
* disabled irqs and here we enable it straight after entry:
*/
ENABLE_INTERRUPTS(CLBR_NONE)
- SAVE_ARGS 8,0,0
+ ALLOC_PTREGS_ON_STACK 8
+ SAVE_C_REGS_EXCEPT_RCX_R891011
movl %eax,%eax /* zero extension */
movq %rax,ORIG_RAX-ARGOFFSET(%rsp)
movq %rcx,RIP-ARGOFFSET(%rsp)
@@ -325,7 +328,7 @@ cstar_dispatch:
jnz sysretl_audit
sysretl_from_sys_call:
andl $~TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
- RESTORE_ARGS 0,-ARG_SKIP,0,0,0
+ RESTORE_RSI_RDI_RDX
movl RIP-ARGOFFSET(%rsp),%ecx
CFI_REGISTER rip,rcx
movl EFLAGS-ARGOFFSET(%rsp),%r11d
@@ -356,13 +359,13 @@ cstar_tracesys:
jz cstar_auditsys
#endif
xchgl %r9d,%ebp
- SAVE_REST
+ SAVE_EXTRA_REGS
CLEAR_RREGS 0, r9
movq $-ENOSYS,RAX(%rsp) /* ptrace can change this for a bad syscall */
movq %rsp,%rdi /* &pt_regs -> arg1 */
call syscall_trace_enter
LOAD_ARGS32 ARGOFFSET, 1 /* reload args from stack in case ptrace changed it */
- RESTORE_REST
+ RESTORE_EXTRA_REGS
xchgl %ebp,%r9d
cmpq $(IA32_NR_syscalls-1),%rax
ja int_ret_from_sys_call /* cstar_tracesys has set RAX(%rsp) */
@@ -417,7 +420,8 @@ ENTRY(ia32_syscall)
cld
/* note the registers are not zero extended to the sf.
this could be a problem. */
- SAVE_ARGS 0,1,0
+ ALLOC_PTREGS_ON_STACK
+ SAVE_C_REGS_EXCEPT_R891011
orl $TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
jnz ia32_tracesys
@@ -430,16 +434,16 @@ ia32_sysret:
movq %rax,RAX-ARGOFFSET(%rsp)
ia32_ret_from_sys_call:
CLEAR_RREGS -ARGOFFSET
- jmp int_ret_from_sys_call
+ jmp int_ret_from_sys_call
-ia32_tracesys:
- SAVE_REST
+ia32_tracesys:
+ SAVE_EXTRA_REGS
CLEAR_RREGS
movq $-ENOSYS,RAX(%rsp) /* ptrace can change this for a bad syscall */
movq %rsp,%rdi /* &pt_regs -> arg1 */
call syscall_trace_enter
LOAD_ARGS32 ARGOFFSET /* reload args from stack in case ptrace changed it */
- RESTORE_REST
+ RESTORE_EXTRA_REGS
cmpq $(IA32_NR_syscalls-1),%rax
ja int_ret_from_sys_call /* ia32_tracesys has set RAX(%rsp) */
jmp ia32_do_call
@@ -475,7 +479,6 @@ GLOBAL(stub32_clone)
ALIGN
ia32_ptregs_common:
- popq %r11
CFI_ENDPROC
CFI_STARTPROC32 simple
CFI_SIGNAL_FRAME
@@ -490,9 +493,9 @@ ia32_ptregs_common:
/* CFI_REL_OFFSET rflags,EFLAGS-ARGOFFSET*/
CFI_REL_OFFSET rsp,RSP-ARGOFFSET
/* CFI_REL_OFFSET ss,SS-ARGOFFSET*/
- SAVE_REST
+ SAVE_EXTRA_REGS 8
call *%rax
- RESTORE_REST
- jmp ia32_sysret /* misbalances the return cache */
+ RESTORE_EXTRA_REGS 8
+ ret
CFI_ENDPROC
END(ia32_ptregs_common)
diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
index e176cea..10aff1e 100644
--- a/arch/x86/include/asm/calling.h
+++ b/arch/x86/include/asm/calling.h
@@ -52,142 +52,132 @@ For 32-bit we have the following conventions - kernel is built with
/*
* 64-bit system call stack frame layout defines and helpers,
- * for assembly code:
+ * for assembly code.
*/
-#define R15 0
-#define R14 8
-#define R13 16
-#define R12 24
-#define RBP 32
-#define RBX 40
-
-/* arguments: interrupts/non tracing syscalls only save up to here: */
-#define R11 48
-#define R10 56
-#define R9 64
-#define R8 72
-#define RAX 80
-#define RCX 88
-#define RDX 96
-#define RSI 104
-#define RDI 112
-#define ORIG_RAX 120 /* + error_code */
-/* end of arguments */
-
-/* cpu exception frame or undefined in case of fast syscall: */
-#define RIP 128
-#define CS 136
-#define EFLAGS 144
-#define RSP 152
-#define SS 160
-
-#define ARGOFFSET R11
-
- .macro SAVE_ARGS addskip=0, save_rcx=1, save_r891011=1
- subq $9*8+\addskip, %rsp
- CFI_ADJUST_CFA_OFFSET 9*8+\addskip
- movq_cfi rdi, 8*8
- movq_cfi rsi, 7*8
- movq_cfi rdx, 6*8
-
- .if \save_rcx
- movq_cfi rcx, 5*8
- .endif
-
- movq_cfi rax, 4*8
+/* The layout forms the "struct pt_regs" on the stack: */
+/*
+ * C ABI says these regs are callee-preserved. They aren't saved on kernel entry
+ * unless syscall needs a complete, fully filled "struct pt_regs".
+ */
+#define R15 0*8
+#define R14 1*8
+#define R13 2*8
+#define R12 3*8
+#define RBP 4*8
+#define RBX 5*8
+/* These regs are callee-clobbered. Always saved on kernel entry. */
+#define R11 6*8
+#define R10 7*8
+#define R9 8*8
+#define R8 9*8
+#define RAX 10*8
+#define RCX 11*8
+#define RDX 12*8
+#define RSI 13*8
+#define RDI 14*8
+/*
+ * On syscall entry, this is syscall#. On CPU exception, this is error code.
+ * On hw interrupt, it's IRQ number:
+ */
+#define ORIG_RAX 15*8
+/* Return frame for iretq */
+#define RIP 16*8
+#define CS 17*8
+#define EFLAGS 18*8
+#define RSP 19*8
+#define SS 20*8
+
+#define ARGOFFSET 0
+
+ .macro ALLOC_PTREGS_ON_STACK addskip=0
+ subq $15*8+\addskip, %rsp
+ CFI_ADJUST_CFA_OFFSET 15*8+\addskip
+ .endm
- .if \save_r891011
- movq_cfi r8, 3*8
- movq_cfi r9, 2*8
- movq_cfi r10, 1*8
- movq_cfi r11, 0*8
+ .macro SAVE_C_REGS_HELPER rcx=1 r8plus=1
+ movq_cfi rdi, 14*8
+ movq_cfi rsi, 13*8
+ movq_cfi rdx, 12*8
+ .if \rcx
+ movq_cfi rcx, 11*8
.endif
-
+ movq_cfi rax, 10*8
+ .if \r8plus
+ movq_cfi r8, 9*8
+ movq_cfi r9, 8*8
+ movq_cfi r10, 7*8
+ movq_cfi r11, 6*8
+ .endif
+ .endm
+ .macro SAVE_C_REGS
+ SAVE_C_REGS_HELPER 1, 1
+ .endm
+ .macro SAVE_C_REGS_EXCEPT_R891011
+ SAVE_C_REGS_HELPER 1, 0
+ .endm
+ .macro SAVE_C_REGS_EXCEPT_RCX_R891011
+ SAVE_C_REGS_HELPER 0, 0
.endm
-#define ARG_SKIP (9*8)
+ .macro SAVE_EXTRA_REGS offset=0
+ movq_cfi rbx, 5*8+\offset
+ movq_cfi rbp, 4*8+\offset
+ movq_cfi r12, 3*8+\offset
+ movq_cfi r13, 2*8+\offset
+ movq_cfi r14, 1*8+\offset
+ movq_cfi r15, 0*8+\offset
+ .endm
- .macro RESTORE_ARGS rstor_rax=1, addskip=0, rstor_rcx=1, rstor_r11=1, \
- rstor_r8910=1, rstor_rdx=1
- .if \rstor_r11
- movq_cfi_restore 0*8, r11
- .endif
+ .macro RESTORE_EXTRA_REGS offset=0
+ movq_cfi_restore 0*8+\offset, r15
+ movq_cfi_restore 1*8+\offset, r14
+ movq_cfi_restore 2*8+\offset, r13
+ movq_cfi_restore 3*8+\offset, r12
+ movq_cfi_restore 4*8+\offset, rbp
+ movq_cfi_restore 5*8+\offset, rbx
+ .endm
- .if \rstor_r8910
- movq_cfi_restore 1*8, r10
- movq_cfi_restore 2*8, r9
- movq_cfi_restore 3*8, r8
+ .macro RESTORE_C_REGS_HELPER rax=1, rcx=1, r11=1, r8910=1, rdx=1
+ .if \r11
+ movq_cfi_restore 6*8, r11
.endif
-
- .if \rstor_rax
- movq_cfi_restore 4*8, rax
+ .if \r8910
+ movq_cfi_restore 7*8, r10
+ movq_cfi_restore 8*8, r9
+ movq_cfi_restore 9*8, r8
.endif
-
- .if \rstor_rcx
- movq_cfi_restore 5*8, rcx
+ .if \rax
+ movq_cfi_restore 10*8, rax
.endif
-
- .if \rstor_rdx
- movq_cfi_restore 6*8, rdx
+ .if \rcx
+ movq_cfi_restore 11*8, rcx
.endif
-
- movq_cfi_restore 7*8, rsi
- movq_cfi_restore 8*8, rdi
-
- .if ARG_SKIP+\addskip > 0
- addq $ARG_SKIP+\addskip, %rsp
- CFI_ADJUST_CFA_OFFSET -(ARG_SKIP+\addskip)
+ .if \rdx
+ movq_cfi_restore 12*8, rdx
.endif
+ movq_cfi_restore 13*8, rsi
+ movq_cfi_restore 14*8, rdi
.endm
-
- .macro LOAD_ARGS offset, skiprax=0
- movq \offset(%rsp), %r11
- movq \offset+8(%rsp), %r10
- movq \offset+16(%rsp), %r9
- movq \offset+24(%rsp), %r8
- movq \offset+40(%rsp), %rcx
- movq \offset+48(%rsp), %rdx
- movq \offset+56(%rsp), %rsi
- movq \offset+64(%rsp), %rdi
- .if \skiprax
- .else
- movq \offset+72(%rsp), %rax
- .endif
+ .macro RESTORE_C_REGS
+ RESTORE_C_REGS_HELPER 1,1,1,1,1
.endm
-
-#define REST_SKIP (6*8)
-
- .macro SAVE_REST
- subq $REST_SKIP, %rsp
- CFI_ADJUST_CFA_OFFSET REST_SKIP
- movq_cfi rbx, 5*8
- movq_cfi rbp, 4*8
- movq_cfi r12, 3*8
- movq_cfi r13, 2*8
- movq_cfi r14, 1*8
- movq_cfi r15, 0*8
+ .macro RESTORE_C_REGS_EXCEPT_RAX
+ RESTORE_C_REGS_HELPER 0,1,1,1,1
.endm
-
- .macro RESTORE_REST
- movq_cfi_restore 0*8, r15
- movq_cfi_restore 1*8, r14
- movq_cfi_restore 2*8, r13
- movq_cfi_restore 3*8, r12
- movq_cfi_restore 4*8, rbp
- movq_cfi_restore 5*8, rbx
- addq $REST_SKIP, %rsp
- CFI_ADJUST_CFA_OFFSET -(REST_SKIP)
+ .macro RESTORE_C_REGS_EXCEPT_RCX
+ RESTORE_C_REGS_HELPER 1,0,1,1,1
.endm
-
- .macro SAVE_ALL
- SAVE_ARGS
- SAVE_REST
+ .macro RESTORE_RSI_RDI
+ RESTORE_C_REGS_HELPER 0,0,0,0,0
+ .endm
+ .macro RESTORE_RSI_RDI_RDX
+ RESTORE_C_REGS_HELPER 0,0,0,0,1
.endm
- .macro RESTORE_ALL addskip=0
- RESTORE_REST
- RESTORE_ARGS 1, \addskip
+ .macro REMOVE_PTREGS_FROM_STACK addskip=0
+ addq $15*8+\addskip, %rsp
+ CFI_ADJUST_CFA_OFFSET -(15*8+\addskip)
.endm
.macro icebp
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index bba3cf8..6f98c16 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -171,9 +171,9 @@ static inline int arch_irqs_disabled(void)
#define ARCH_LOCKDEP_SYS_EXIT_IRQ \
TRACE_IRQS_ON; \
sti; \
- SAVE_REST; \
+ SAVE_EXTRA_REGS; \
LOCKDEP_SYS_EXIT; \
- RESTORE_REST; \
+ RESTORE_EXTRA_REGS; \
cli; \
TRACE_IRQS_OFF;
diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
index 6205f0c..c822b35 100644
--- a/arch/x86/include/asm/ptrace.h
+++ b/arch/x86/include/asm/ptrace.h
@@ -31,13 +31,17 @@ struct pt_regs {
#else /* __i386__ */
struct pt_regs {
+/*
+ * C ABI says these regs are callee-preserved. They aren't saved on kernel entry
+ * unless syscall needs a complete, fully filled "struct pt_regs".
+ */
unsigned long r15;
unsigned long r14;
unsigned long r13;
unsigned long r12;
unsigned long bp;
unsigned long bx;
-/* arguments: non interrupts/non tracing syscalls only save up to here*/
+/* These regs are callee-clobbered. Always saved on kernel entry. */
unsigned long r11;
unsigned long r10;
unsigned long r9;
@@ -47,9 +51,12 @@ struct pt_regs {
unsigned long dx;
unsigned long si;
unsigned long di;
+/*
+ * On syscall entry, this is syscall#. On CPU exception, this is error code.
+ * On hw interrupt, it's IRQ number:
+ */
unsigned long orig_ax;
-/* end of arguments */
-/* cpu exception frame or undefined */
+/* Return frame for iretq */
unsigned long ip;
unsigned long cs;
unsigned long flags;
diff --git a/arch/x86/include/uapi/asm/ptrace-abi.h b/arch/x86/include/uapi/asm/ptrace-abi.h
index 7b0a55a..580aee3 100644
--- a/arch/x86/include/uapi/asm/ptrace-abi.h
+++ b/arch/x86/include/uapi/asm/ptrace-abi.h
@@ -25,13 +25,17 @@
#else /* __i386__ */
#if defined(__ASSEMBLY__) || defined(__FRAME_OFFSETS)
+/*
+ * C ABI says these regs are callee-preserved. They aren't saved on kernel entry
+ * unless syscall needs a complete, fully filled "struct pt_regs".
+ */
#define R15 0
#define R14 8
#define R13 16
#define R12 24
#define RBP 32
#define RBX 40
-/* arguments: interrupts/non tracing syscalls only save up to here*/
+/* These regs are callee-clobbered. Always saved on kernel entry. */
#define R11 48
#define R10 56
#define R9 64
@@ -41,15 +45,17 @@
#define RDX 96
#define RSI 104
#define RDI 112
-#define ORIG_RAX 120 /* = ERROR */
-/* end of arguments */
-/* cpu exception frame or undefined in case of fast syscall. */
+/*
+ * On syscall entry, this is syscall#. On CPU exception, this is error code.
+ * On hw interrupt, it's IRQ number:
+ */
+#define ORIG_RAX 120
+/* Return frame for iretq */
#define RIP 128
#define CS 136
#define EFLAGS 144
#define RSP 152
#define SS 160
-#define ARGOFFSET R11
#endif /* __ASSEMBLY__ */
/* top of stack page */
diff --git a/arch/x86/include/uapi/asm/ptrace.h b/arch/x86/include/uapi/asm/ptrace.h
index ac4b9aa..bc16115 100644
--- a/arch/x86/include/uapi/asm/ptrace.h
+++ b/arch/x86/include/uapi/asm/ptrace.h
@@ -41,13 +41,17 @@ struct pt_regs {
#ifndef __KERNEL__
struct pt_regs {
+/*
+ * C ABI says these regs are callee-preserved. They aren't saved on kernel entry
+ * unless syscall needs a complete, fully filled "struct pt_regs".
+ */
unsigned long r15;
unsigned long r14;
unsigned long r13;
unsigned long r12;
unsigned long rbp;
unsigned long rbx;
-/* arguments: non interrupts/non tracing syscalls only save up to here*/
+/* These regs are callee-clobbered. Always saved on kernel entry. */
unsigned long r11;
unsigned long r10;
unsigned long r9;
@@ -57,9 +61,12 @@ struct pt_regs {
unsigned long rdx;
unsigned long rsi;
unsigned long rdi;
+/*
+ * On syscall entry, this is syscall#. On CPU exception, this is error code.
+ * On hw interrupt, it's IRQ number:
+ */
unsigned long orig_rax;
-/* end of arguments */
-/* cpu exception frame or undefined */
+/* Return frame for iretq */
unsigned long rip;
unsigned long cs;
unsigned long eflags;
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 37f7d95..b3c3ebb 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -26,12 +26,6 @@
* Some macro usage:
* - CFI macros are used to generate dwarf2 unwind information for better
* backtraces. They don't change any code.
- * - SAVE_ALL/RESTORE_ALL - Save/restore all registers
- * - SAVE_ARGS/RESTORE_ARGS - Save/restore registers that C functions modify.
- * There are unfortunately lots of special cases where some registers
- * not touched. The macro is a big mess that should be cleaned up.
- * - SAVE_REST/RESTORE_REST - Handle the registers not saved by SAVE_ARGS.
- * Gives a full stack frame.
* - ENTRY/END Define functions in the symbol table.
* - FIXUP_TOP_OF_STACK/RESTORE_TOP_OF_STACK - Fix up the hardware stack
* frame that is otherwise undefined after a SYSCALL
@@ -264,7 +258,7 @@ ENTRY(ret_from_fork)
GET_THREAD_INFO(%rcx)
- RESTORE_REST
+ RESTORE_EXTRA_REGS
testl $3, CS-ARGOFFSET(%rsp) # from kernel_thread?
jz 1f
@@ -276,12 +270,10 @@ ENTRY(ret_from_fork)
jmp ret_from_sys_call # go to the SYSRET fastpath
1:
- subq $REST_SKIP, %rsp # leave space for volatiles
- CFI_ADJUST_CFA_OFFSET REST_SKIP
movq %rbp, %rdi
call *%rbx
movl $0, RAX(%rsp)
- RESTORE_REST
+ RESTORE_EXTRA_REGS
jmp int_ret_from_sys_call
CFI_ENDPROC
END(ret_from_fork)
@@ -339,7 +331,8 @@ GLOBAL(system_call_after_swapgs)
* and short:
*/
ENABLE_INTERRUPTS(CLBR_NONE)
- SAVE_ARGS 8,0
+ ALLOC_PTREGS_ON_STACK 8
+ SAVE_C_REGS
movq %rax,ORIG_RAX-ARGOFFSET(%rsp)
movq %rcx,RIP-ARGOFFSET(%rsp)
CFI_REL_OFFSET rip,RIP-ARGOFFSET
@@ -375,9 +368,9 @@ sysret_check:
* sysretq will re-enable interrupts:
*/
TRACE_IRQS_ON
+ RESTORE_C_REGS_EXCEPT_RCX
movq RIP-ARGOFFSET(%rsp),%rcx
CFI_REGISTER rip,rcx
- RESTORE_ARGS 1,-ARG_SKIP,0
/*CFI_REGISTER rflags,r11*/
movq PER_CPU_VAR(old_rsp), %rsp
USERGS_SYSRET64
@@ -429,7 +422,7 @@ auditsys:
movq %rax,%rsi /* 2nd arg: syscall number */
movl $AUDIT_ARCH_X86_64,%edi /* 1st arg: audit arch */
call __audit_syscall_entry
- LOAD_ARGS 0 /* reload call-clobbered registers */
+ RESTORE_C_REGS /* reload call-clobbered registers */
jmp system_call_fastpath
/*
@@ -453,7 +446,7 @@ tracesys:
testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
jz auditsys
#endif
- SAVE_REST
+ SAVE_EXTRA_REGS
movq $-ENOSYS,RAX(%rsp) /* ptrace can change this for a bad syscall */
FIXUP_TOP_OF_STACK %rdi
movq %rsp,%rdi
@@ -463,8 +456,8 @@ tracesys:
* We don't reload %rax because syscall_trace_enter() returned
* the value it wants us to use in the table lookup.
*/
- LOAD_ARGS ARGOFFSET, 1
- RESTORE_REST
+ RESTORE_C_REGS_EXCEPT_RAX
+ RESTORE_EXTRA_REGS
#if __SYSCALL_MASK == ~0
cmpq $__NR_syscall_max,%rax
#else
@@ -515,7 +508,7 @@ int_very_careful:
TRACE_IRQS_ON
ENABLE_INTERRUPTS(CLBR_NONE)
int_check_syscall_exit_work:
- SAVE_REST
+ SAVE_EXTRA_REGS
/* Check for syscall exit trace */
testl $_TIF_WORK_SYSCALL_EXIT,%edx
jz int_signal
@@ -534,7 +527,7 @@ int_signal:
call do_notify_resume
1: movl $_TIF_WORK_MASK,%edi
int_restore_rest:
- RESTORE_REST
+ RESTORE_EXTRA_REGS
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
jmp int_with_check
@@ -544,15 +537,12 @@ END(system_call)
.macro FORK_LIKE func
ENTRY(stub_\func)
CFI_STARTPROC
- popq %r11 /* save return address */
- PARTIAL_FRAME 0
- SAVE_REST
- pushq %r11 /* put it back on stack */
+ DEFAULT_FRAME 0, 8 /* offset 8: return address */
+ SAVE_EXTRA_REGS 8
FIXUP_TOP_OF_STACK %r11, 8
- DEFAULT_FRAME 0 8 /* offset 8: return address */
call sys_\func
RESTORE_TOP_OF_STACK %r11, 8
- ret $REST_SKIP /* pop extended registers */
+ ret
CFI_ENDPROC
END(stub_\func)
.endm
@@ -560,7 +550,7 @@ END(stub_\func)
.macro FIXED_FRAME label,func
ENTRY(\label)
CFI_STARTPROC
- PARTIAL_FRAME 0 8 /* offset 8: return address */
+ DEFAULT_FRAME 0, 8 /* offset 8: return address */
FIXUP_TOP_OF_STACK %r11, 8-ARGOFFSET
call \func
RESTORE_TOP_OF_STACK %r11, 8-ARGOFFSET
@@ -577,12 +567,12 @@ END(\label)
ENTRY(stub_execve)
CFI_STARTPROC
addq $8, %rsp
- PARTIAL_FRAME 0
- SAVE_REST
+ DEFAULT_FRAME 0
+ SAVE_EXTRA_REGS
FIXUP_TOP_OF_STACK %r11
call sys_execve
movq %rax,RAX(%rsp)
- RESTORE_REST
+ RESTORE_EXTRA_REGS
jmp int_ret_from_sys_call
CFI_ENDPROC
END(stub_execve)
@@ -594,12 +584,12 @@ END(stub_execve)
ENTRY(stub_rt_sigreturn)
CFI_STARTPROC
addq $8, %rsp
- PARTIAL_FRAME 0
- SAVE_REST
+ DEFAULT_FRAME 0
+ SAVE_EXTRA_REGS
FIXUP_TOP_OF_STACK %r11
call sys_rt_sigreturn
movq %rax,RAX(%rsp) # fixme, this could be done at the higher layer
- RESTORE_REST
+ RESTORE_EXTRA_REGS
jmp int_ret_from_sys_call
CFI_ENDPROC
END(stub_rt_sigreturn)
@@ -608,12 +598,12 @@ END(stub_rt_sigreturn)
ENTRY(stub_x32_rt_sigreturn)
CFI_STARTPROC
addq $8, %rsp
- PARTIAL_FRAME 0
- SAVE_REST
+ DEFAULT_FRAME 0
+ SAVE_EXTRA_REGS
FIXUP_TOP_OF_STACK %r11
call sys32_x32_rt_sigreturn
movq %rax,RAX(%rsp) # fixme, this could be done at the higher layer
- RESTORE_REST
+ RESTORE_EXTRA_REGS
jmp int_ret_from_sys_call
CFI_ENDPROC
END(stub_x32_rt_sigreturn)
@@ -621,13 +611,13 @@ END(stub_x32_rt_sigreturn)
ENTRY(stub_x32_execve)
CFI_STARTPROC
addq $8, %rsp
- PARTIAL_FRAME 0
- SAVE_REST
+ DEFAULT_FRAME 0
+ SAVE_EXTRA_REGS
FIXUP_TOP_OF_STACK %r11
call compat_sys_execve
RESTORE_TOP_OF_STACK %r11
movq %rax,RAX(%rsp)
- RESTORE_REST
+ RESTORE_EXTRA_REGS
jmp int_ret_from_sys_call
CFI_ENDPROC
END(stub_x32_execve)
@@ -683,51 +673,31 @@ END(interrupt)
/* 0(%rsp): ~(interrupt number) */
.macro interrupt func
- /* reserve pt_regs for scratch regs and rbp */
- subq $ORIG_RAX-RBP, %rsp
- CFI_ADJUST_CFA_OFFSET ORIG_RAX-RBP
- cld
- /* start from rbp in pt_regs and jump over */
- movq_cfi rdi, (RDI-RBP)
- movq_cfi rsi, (RSI-RBP)
- movq_cfi rdx, (RDX-RBP)
- movq_cfi rcx, (RCX-RBP)
- movq_cfi rax, (RAX-RBP)
- movq_cfi r8, (R8-RBP)
- movq_cfi r9, (R9-RBP)
- movq_cfi r10, (R10-RBP)
- movq_cfi r11, (R11-RBP)
-
- /* Save rbp so that we can unwind from get_irq_regs() */
- movq_cfi rbp, 0
-
- /* Save previous stack value */
- movq %rsp, %rsi
-
- leaq -RBP(%rsp),%rdi /* arg1 for handler */
- testl $3, CS-RBP(%rsi)
+ ALLOC_PTREGS_ON_STACK
+ SAVE_C_REGS
+ movq %rsp, %rdi /* arg1 for handler */
+ testl $3, CS(%rsp)
je 1f
SWAPGS
- /*
+1: /*
* irq_count is used to check if a CPU is already on an interrupt stack
* or not. While this is essentially redundant with preempt_count it is
* a little cheaper to use a separate counter in the PDA (short of
* moving irq_enter into assembly, which would be too much work)
*/
-1: incl PER_CPU_VAR(irq_count)
+ incl PER_CPU_VAR(irq_count)
cmovzq PER_CPU_VAR(irq_stack_ptr),%rsp
- CFI_DEF_CFA_REGISTER rsi
+ CFI_DEF_CFA_REGISTER rdi
/* Store previous stack value */
- pushq %rsi
+ pushq %rdi
CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
0x77 /* DW_OP_breg7 */, 0, \
0x06 /* DW_OP_deref */, \
- 0x08 /* DW_OP_const1u */, SS+8-RBP, \
+ 0x08 /* DW_OP_const1u */, SS+8, \
0x22 /* DW_OP_plus */
/* We entered an interrupt context - irqs are off: */
TRACE_IRQS_OFF
-
call \func
.endm
@@ -749,10 +719,9 @@ ret_from_intr:
/* Restore saved previous stack */
popq %rsi
- CFI_DEF_CFA rsi,SS+8-RBP /* reg/off reset after def_cfa_expr */
- leaq ARGOFFSET-RBP(%rsi), %rsp
+ CFI_DEF_CFA rsi,SS+8 /* reg/off reset after def_cfa_expr */
+ movq %rsi, %rsp
CFI_DEF_CFA_REGISTER rsp
- CFI_ADJUST_CFA_OFFSET RBP-ARGOFFSET
exit_intr:
GET_THREAD_INFO(%rcx)
@@ -789,7 +758,8 @@ retint_restore_args: /* return to kernel space */
*/
TRACE_IRQS_IRETQ
restore_args:
- RESTORE_ARGS 1,8,1
+ RESTORE_C_REGS
+ REMOVE_PTREGS_FROM_STACK 8
irq_return:
/*
@@ -876,12 +846,12 @@ retint_signal:
jz retint_swapgs
TRACE_IRQS_ON
ENABLE_INTERRUPTS(CLBR_NONE)
- SAVE_REST
+ SAVE_EXTRA_REGS
movq $-1,ORIG_RAX(%rsp)
xorl %esi,%esi # oldset
movq %rsp,%rdi # &pt_regs
call do_notify_resume
- RESTORE_REST
+ RESTORE_EXTRA_REGS
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
GET_THREAD_INFO(%rcx)
@@ -1256,7 +1226,9 @@ ENTRY(xen_failsafe_callback)
addq $0x30,%rsp
CFI_ADJUST_CFA_OFFSET -0x30
pushq_cfi $-1 /* orig_ax = -1 => not a system call */
- SAVE_ALL
+ ALLOC_PTREGS_ON_STACK
+ SAVE_C_REGS
+ SAVE_EXTRA_REGS
jmp error_exit
CFI_ENDPROC
END(xen_failsafe_callback)
@@ -1313,11 +1285,15 @@ ENTRY(paranoid_exit)
paranoid_swapgs:
TRACE_IRQS_IRETQ 0
SWAPGS_UNSAFE_STACK
- RESTORE_ALL 8
+ RESTORE_EXTRA_REGS
+ RESTORE_C_REGS
+ REMOVE_PTREGS_FROM_STACK 8
jmp irq_return
paranoid_restore:
TRACE_IRQS_IRETQ_DEBUG 0
- RESTORE_ALL 8
+ RESTORE_EXTRA_REGS
+ RESTORE_C_REGS
+ REMOVE_PTREGS_FROM_STACK 8
jmp irq_return
paranoid_userspace:
GET_THREAD_INFO(%rcx)
@@ -1412,7 +1388,7 @@ END(error_entry)
ENTRY(error_exit)
DEFAULT_FRAME
movl %ebx,%eax
- RESTORE_REST
+ RESTORE_EXTRA_REGS
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
GET_THREAD_INFO(%rcx)
@@ -1671,8 +1647,10 @@ end_repeat_nmi:
nmi_swapgs:
SWAPGS_UNSAFE_STACK
nmi_restore:
+ RESTORE_EXTRA_REGS
+ RESTORE_C_REGS
/* Pop the extra iret frame at once */
- RESTORE_ALL 6*8
+ REMOVE_PTREGS_FROM_STACK 6*8
/* Clear the NMI executing stack variable */
movq $0, 5*8(%rsp)
diff --git a/arch/x86/kernel/preempt.S b/arch/x86/kernel/preempt.S
index ca7f0d5..673da2f 100644
--- a/arch/x86/kernel/preempt.S
+++ b/arch/x86/kernel/preempt.S
@@ -6,9 +6,13 @@
ENTRY(___preempt_schedule)
CFI_STARTPROC
- SAVE_ALL
+ ALLOC_PTREGS_ON_STACK
+ SAVE_C_REGS
+ SAVE_EXTRA_REGS
call preempt_schedule
- RESTORE_ALL
+ RESTORE_EXTRA_REGS
+ RESTORE_C_REGS
+ REMOVE_PTREGS_FROM_STACK
ret
CFI_ENDPROC
@@ -16,9 +20,13 @@ ENTRY(___preempt_schedule)
ENTRY(___preempt_schedule_context)
CFI_STARTPROC
- SAVE_ALL
+ ALLOC_PTREGS_ON_STACK
+ SAVE_C_REGS
+ SAVE_EXTRA_REGS
call preempt_schedule_context
- RESTORE_ALL
+ RESTORE_EXTRA_REGS
+ RESTORE_C_REGS
+ REMOVE_PTREGS_FROM_STACK
ret
CFI_ENDPROC
--
1.8.1.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 14:48 ` [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs" Denys Vlasenko
@ 2014-08-01 17:04 ` Andy Lutomirski
2014-08-04 14:28 ` Denys Vlasenko
2014-08-01 18:09 ` Alexei Starovoitov
` (4 subsequent siblings)
5 siblings, 1 reply; 41+ messages in thread
From: Andy Lutomirski @ 2014-08-01 17:04 UTC (permalink / raw)
To: Denys Vlasenko
Cc: linux-kernel@vger.kernel.org, Oleg Nesterov, H. Peter Anvin,
Frederic Weisbecker, X86 ML, Alexei Starovoitov, Will Drewry,
Kees Cook
On Fri, Aug 1, 2014 at 7:48 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
> 64-bit code was using six stack slots fewer by not saving/restoring
> registers which a callee-preserved according to C ABI,
> and not allocating space for them
This is great.
Next up: remove FIXUP/RESTORE_TOP_OF_STACK? :) Maybe I'll give that a shot.
--Andy
.
>
> Only when syscall needed a complete "struct pt_regs",
> the complete area was allocated and filled in.
>
> This proved to be a source of significant obfuscation and subtle bugs.
> For example, stub_fork had to pop the return address,
> extend the struct, save registers, and push return address back. Ugly.
> ia32_ptregs_common pops return address and "returns" via jmp insn,
> throwing a wrench into CPU return stack cache.
>
> This patch changes code to always allocate a complete "struct pt_regs".
> The saving of registers is still done lazily.
>
> Macros which manipulate "struct pt_regs" on stack are reworked:
> ALLOC_PTREGS_ON_STACK allocates the structure.
> SAVE_C_REGS saves to it those registers which are clobbered by C code.
> SAVE_EXTRA_REGS saves to it all other registers.
> Corresponding RESTORE_* and REMOVE_PTREGS_FROM_STACK macros reverse it.
>
> ia32_ptregs_common, stub_fork and friends lost their ugly dance with
> return pointer.
>
> LOAD_ARGS32 in ia32entry.S now uses a symbolic stack offsets
> instead of magic numbers.
>
> Misleading and slightly wrong comments in "struct pt_regs" are fixed
> (four instances).
>
> Patch was run-tested: 64-bit executables, 32-bit executables,
> strace works.
>
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: X86 ML <x86@kernel.org>
> CC: Alexei Starovoitov <ast@plumgrid.com>
> CC: Will Drewry <wad@chromium.org>
> CC: Kees Cook <keescook@chromium.org>
> CC: linux-kernel@vger.kernel.org
> ---
> arch/x86/ia32/ia32entry.S | 47 +++----
> arch/x86/include/asm/calling.h | 224 ++++++++++++++++-----------------
> arch/x86/include/asm/irqflags.h | 4 +-
> arch/x86/include/asm/ptrace.h | 13 +-
> arch/x86/include/uapi/asm/ptrace-abi.h | 16 ++-
> arch/x86/include/uapi/asm/ptrace.h | 13 +-
> arch/x86/kernel/entry_64.S | 132 ++++++++-----------
> arch/x86/kernel/preempt.S | 16 ++-
> 8 files changed, 232 insertions(+), 233 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
> index 4299eb0..ef9ee16 100644
> --- a/arch/x86/ia32/ia32entry.S
> +++ b/arch/x86/ia32/ia32entry.S
> @@ -62,12 +62,12 @@
> */
> .macro LOAD_ARGS32 offset, _r9=0
> .if \_r9
> - movl \offset+16(%rsp),%r9d
> + movl \offset+R9(%rsp),%r9d
> .endif
> - movl \offset+40(%rsp),%ecx
> - movl \offset+48(%rsp),%edx
> - movl \offset+56(%rsp),%esi
> - movl \offset+64(%rsp),%edi
> + movl \offset+RCX(%rsp),%ecx
> + movl \offset+RDX(%rsp),%edx
> + movl \offset+RSI(%rsp),%esi
> + movl \offset+RDI(%rsp),%edi
> movl %eax,%eax /* zero extension */
> .endm
>
> @@ -144,7 +144,8 @@ ENTRY(ia32_sysenter_target)
> CFI_REL_OFFSET rip,0
> pushq_cfi %rax
> cld
> - SAVE_ARGS 0,1,0
> + ALLOC_PTREGS_ON_STACK
> + SAVE_C_REGS_EXCEPT_R891011
> /* no need to do an access_ok check here because rbp has been
> 32bit zero extended */
> ASM_STAC
> @@ -172,7 +173,8 @@ sysexit_from_sys_call:
> andl $~0x200,EFLAGS-R11(%rsp)
> movl RIP-R11(%rsp),%edx /* User %eip */
> CFI_REGISTER rip,rdx
> - RESTORE_ARGS 0,24,0,0,0,0
> + RESTORE_RSI_RDI
> + REMOVE_PTREGS_FROM_STACK 8*3
> xorq %r8,%r8
> xorq %r9,%r9
> xorq %r10,%r10
> @@ -240,13 +242,13 @@ sysenter_tracesys:
> testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
> jz sysenter_auditsys
> #endif
> - SAVE_REST
> + SAVE_EXTRA_REGS
> CLEAR_RREGS
> movq $-ENOSYS,RAX(%rsp)/* ptrace can change this for a bad syscall */
> movq %rsp,%rdi /* &pt_regs -> arg1 */
> call syscall_trace_enter
> LOAD_ARGS32 ARGOFFSET /* reload args from stack in case ptrace changed it */
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> cmpq $(IA32_NR_syscalls-1),%rax
> ja int_ret_from_sys_call /* sysenter_tracesys has set RAX(%rsp) */
> jmp sysenter_do_call
> @@ -288,7 +290,8 @@ ENTRY(ia32_cstar_target)
> * disabled irqs and here we enable it straight after entry:
> */
> ENABLE_INTERRUPTS(CLBR_NONE)
> - SAVE_ARGS 8,0,0
> + ALLOC_PTREGS_ON_STACK 8
> + SAVE_C_REGS_EXCEPT_RCX_R891011
> movl %eax,%eax /* zero extension */
> movq %rax,ORIG_RAX-ARGOFFSET(%rsp)
> movq %rcx,RIP-ARGOFFSET(%rsp)
> @@ -325,7 +328,7 @@ cstar_dispatch:
> jnz sysretl_audit
> sysretl_from_sys_call:
> andl $~TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
> - RESTORE_ARGS 0,-ARG_SKIP,0,0,0
> + RESTORE_RSI_RDI_RDX
> movl RIP-ARGOFFSET(%rsp),%ecx
> CFI_REGISTER rip,rcx
> movl EFLAGS-ARGOFFSET(%rsp),%r11d
> @@ -356,13 +359,13 @@ cstar_tracesys:
> jz cstar_auditsys
> #endif
> xchgl %r9d,%ebp
> - SAVE_REST
> + SAVE_EXTRA_REGS
> CLEAR_RREGS 0, r9
> movq $-ENOSYS,RAX(%rsp) /* ptrace can change this for a bad syscall */
> movq %rsp,%rdi /* &pt_regs -> arg1 */
> call syscall_trace_enter
> LOAD_ARGS32 ARGOFFSET, 1 /* reload args from stack in case ptrace changed it */
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> xchgl %ebp,%r9d
> cmpq $(IA32_NR_syscalls-1),%rax
> ja int_ret_from_sys_call /* cstar_tracesys has set RAX(%rsp) */
> @@ -417,7 +420,8 @@ ENTRY(ia32_syscall)
> cld
> /* note the registers are not zero extended to the sf.
> this could be a problem. */
> - SAVE_ARGS 0,1,0
> + ALLOC_PTREGS_ON_STACK
> + SAVE_C_REGS_EXCEPT_R891011
> orl $TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
> testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
> jnz ia32_tracesys
> @@ -430,16 +434,16 @@ ia32_sysret:
> movq %rax,RAX-ARGOFFSET(%rsp)
> ia32_ret_from_sys_call:
> CLEAR_RREGS -ARGOFFSET
> - jmp int_ret_from_sys_call
> + jmp int_ret_from_sys_call
>
> -ia32_tracesys:
> - SAVE_REST
> +ia32_tracesys:
> + SAVE_EXTRA_REGS
> CLEAR_RREGS
> movq $-ENOSYS,RAX(%rsp) /* ptrace can change this for a bad syscall */
> movq %rsp,%rdi /* &pt_regs -> arg1 */
> call syscall_trace_enter
> LOAD_ARGS32 ARGOFFSET /* reload args from stack in case ptrace changed it */
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> cmpq $(IA32_NR_syscalls-1),%rax
> ja int_ret_from_sys_call /* ia32_tracesys has set RAX(%rsp) */
> jmp ia32_do_call
> @@ -475,7 +479,6 @@ GLOBAL(stub32_clone)
>
> ALIGN
> ia32_ptregs_common:
> - popq %r11
> CFI_ENDPROC
> CFI_STARTPROC32 simple
> CFI_SIGNAL_FRAME
> @@ -490,9 +493,9 @@ ia32_ptregs_common:
> /* CFI_REL_OFFSET rflags,EFLAGS-ARGOFFSET*/
> CFI_REL_OFFSET rsp,RSP-ARGOFFSET
> /* CFI_REL_OFFSET ss,SS-ARGOFFSET*/
> - SAVE_REST
> + SAVE_EXTRA_REGS 8
> call *%rax
> - RESTORE_REST
> - jmp ia32_sysret /* misbalances the return cache */
> + RESTORE_EXTRA_REGS 8
> + ret
> CFI_ENDPROC
> END(ia32_ptregs_common)
> diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
> index e176cea..10aff1e 100644
> --- a/arch/x86/include/asm/calling.h
> +++ b/arch/x86/include/asm/calling.h
> @@ -52,142 +52,132 @@ For 32-bit we have the following conventions - kernel is built with
>
> /*
> * 64-bit system call stack frame layout defines and helpers,
> - * for assembly code:
> + * for assembly code.
> */
>
> -#define R15 0
> -#define R14 8
> -#define R13 16
> -#define R12 24
> -#define RBP 32
> -#define RBX 40
> -
> -/* arguments: interrupts/non tracing syscalls only save up to here: */
> -#define R11 48
> -#define R10 56
> -#define R9 64
> -#define R8 72
> -#define RAX 80
> -#define RCX 88
> -#define RDX 96
> -#define RSI 104
> -#define RDI 112
> -#define ORIG_RAX 120 /* + error_code */
> -/* end of arguments */
> -
> -/* cpu exception frame or undefined in case of fast syscall: */
> -#define RIP 128
> -#define CS 136
> -#define EFLAGS 144
> -#define RSP 152
> -#define SS 160
> -
> -#define ARGOFFSET R11
> -
> - .macro SAVE_ARGS addskip=0, save_rcx=1, save_r891011=1
> - subq $9*8+\addskip, %rsp
> - CFI_ADJUST_CFA_OFFSET 9*8+\addskip
> - movq_cfi rdi, 8*8
> - movq_cfi rsi, 7*8
> - movq_cfi rdx, 6*8
> -
> - .if \save_rcx
> - movq_cfi rcx, 5*8
> - .endif
> -
> - movq_cfi rax, 4*8
> +/* The layout forms the "struct pt_regs" on the stack: */
> +/*
> + * C ABI says these regs are callee-preserved. They aren't saved on kernel entry
> + * unless syscall needs a complete, fully filled "struct pt_regs".
> + */
> +#define R15 0*8
> +#define R14 1*8
> +#define R13 2*8
> +#define R12 3*8
> +#define RBP 4*8
> +#define RBX 5*8
> +/* These regs are callee-clobbered. Always saved on kernel entry. */
> +#define R11 6*8
> +#define R10 7*8
> +#define R9 8*8
> +#define R8 9*8
> +#define RAX 10*8
> +#define RCX 11*8
> +#define RDX 12*8
> +#define RSI 13*8
> +#define RDI 14*8
> +/*
> + * On syscall entry, this is syscall#. On CPU exception, this is error code.
> + * On hw interrupt, it's IRQ number:
> + */
> +#define ORIG_RAX 15*8
> +/* Return frame for iretq */
> +#define RIP 16*8
> +#define CS 17*8
> +#define EFLAGS 18*8
> +#define RSP 19*8
> +#define SS 20*8
> +
> +#define ARGOFFSET 0
> +
> + .macro ALLOC_PTREGS_ON_STACK addskip=0
> + subq $15*8+\addskip, %rsp
> + CFI_ADJUST_CFA_OFFSET 15*8+\addskip
> + .endm
>
> - .if \save_r891011
> - movq_cfi r8, 3*8
> - movq_cfi r9, 2*8
> - movq_cfi r10, 1*8
> - movq_cfi r11, 0*8
> + .macro SAVE_C_REGS_HELPER rcx=1 r8plus=1
> + movq_cfi rdi, 14*8
> + movq_cfi rsi, 13*8
> + movq_cfi rdx, 12*8
> + .if \rcx
> + movq_cfi rcx, 11*8
> .endif
> -
> + movq_cfi rax, 10*8
> + .if \r8plus
> + movq_cfi r8, 9*8
> + movq_cfi r9, 8*8
> + movq_cfi r10, 7*8
> + movq_cfi r11, 6*8
> + .endif
> + .endm
> + .macro SAVE_C_REGS
> + SAVE_C_REGS_HELPER 1, 1
> + .endm
> + .macro SAVE_C_REGS_EXCEPT_R891011
> + SAVE_C_REGS_HELPER 1, 0
> + .endm
> + .macro SAVE_C_REGS_EXCEPT_RCX_R891011
> + SAVE_C_REGS_HELPER 0, 0
> .endm
>
> -#define ARG_SKIP (9*8)
> + .macro SAVE_EXTRA_REGS offset=0
> + movq_cfi rbx, 5*8+\offset
> + movq_cfi rbp, 4*8+\offset
> + movq_cfi r12, 3*8+\offset
> + movq_cfi r13, 2*8+\offset
> + movq_cfi r14, 1*8+\offset
> + movq_cfi r15, 0*8+\offset
> + .endm
>
> - .macro RESTORE_ARGS rstor_rax=1, addskip=0, rstor_rcx=1, rstor_r11=1, \
> - rstor_r8910=1, rstor_rdx=1
> - .if \rstor_r11
> - movq_cfi_restore 0*8, r11
> - .endif
> + .macro RESTORE_EXTRA_REGS offset=0
> + movq_cfi_restore 0*8+\offset, r15
> + movq_cfi_restore 1*8+\offset, r14
> + movq_cfi_restore 2*8+\offset, r13
> + movq_cfi_restore 3*8+\offset, r12
> + movq_cfi_restore 4*8+\offset, rbp
> + movq_cfi_restore 5*8+\offset, rbx
> + .endm
>
> - .if \rstor_r8910
> - movq_cfi_restore 1*8, r10
> - movq_cfi_restore 2*8, r9
> - movq_cfi_restore 3*8, r8
> + .macro RESTORE_C_REGS_HELPER rax=1, rcx=1, r11=1, r8910=1, rdx=1
> + .if \r11
> + movq_cfi_restore 6*8, r11
> .endif
> -
> - .if \rstor_rax
> - movq_cfi_restore 4*8, rax
> + .if \r8910
> + movq_cfi_restore 7*8, r10
> + movq_cfi_restore 8*8, r9
> + movq_cfi_restore 9*8, r8
> .endif
> -
> - .if \rstor_rcx
> - movq_cfi_restore 5*8, rcx
> + .if \rax
> + movq_cfi_restore 10*8, rax
> .endif
> -
> - .if \rstor_rdx
> - movq_cfi_restore 6*8, rdx
> + .if \rcx
> + movq_cfi_restore 11*8, rcx
> .endif
> -
> - movq_cfi_restore 7*8, rsi
> - movq_cfi_restore 8*8, rdi
> -
> - .if ARG_SKIP+\addskip > 0
> - addq $ARG_SKIP+\addskip, %rsp
> - CFI_ADJUST_CFA_OFFSET -(ARG_SKIP+\addskip)
> + .if \rdx
> + movq_cfi_restore 12*8, rdx
> .endif
> + movq_cfi_restore 13*8, rsi
> + movq_cfi_restore 14*8, rdi
> .endm
> -
> - .macro LOAD_ARGS offset, skiprax=0
> - movq \offset(%rsp), %r11
> - movq \offset+8(%rsp), %r10
> - movq \offset+16(%rsp), %r9
> - movq \offset+24(%rsp), %r8
> - movq \offset+40(%rsp), %rcx
> - movq \offset+48(%rsp), %rdx
> - movq \offset+56(%rsp), %rsi
> - movq \offset+64(%rsp), %rdi
> - .if \skiprax
> - .else
> - movq \offset+72(%rsp), %rax
> - .endif
> + .macro RESTORE_C_REGS
> + RESTORE_C_REGS_HELPER 1,1,1,1,1
> .endm
> -
> -#define REST_SKIP (6*8)
> -
> - .macro SAVE_REST
> - subq $REST_SKIP, %rsp
> - CFI_ADJUST_CFA_OFFSET REST_SKIP
> - movq_cfi rbx, 5*8
> - movq_cfi rbp, 4*8
> - movq_cfi r12, 3*8
> - movq_cfi r13, 2*8
> - movq_cfi r14, 1*8
> - movq_cfi r15, 0*8
> + .macro RESTORE_C_REGS_EXCEPT_RAX
> + RESTORE_C_REGS_HELPER 0,1,1,1,1
> .endm
> -
> - .macro RESTORE_REST
> - movq_cfi_restore 0*8, r15
> - movq_cfi_restore 1*8, r14
> - movq_cfi_restore 2*8, r13
> - movq_cfi_restore 3*8, r12
> - movq_cfi_restore 4*8, rbp
> - movq_cfi_restore 5*8, rbx
> - addq $REST_SKIP, %rsp
> - CFI_ADJUST_CFA_OFFSET -(REST_SKIP)
> + .macro RESTORE_C_REGS_EXCEPT_RCX
> + RESTORE_C_REGS_HELPER 1,0,1,1,1
> .endm
> -
> - .macro SAVE_ALL
> - SAVE_ARGS
> - SAVE_REST
> + .macro RESTORE_RSI_RDI
> + RESTORE_C_REGS_HELPER 0,0,0,0,0
> + .endm
> + .macro RESTORE_RSI_RDI_RDX
> + RESTORE_C_REGS_HELPER 0,0,0,0,1
> .endm
>
> - .macro RESTORE_ALL addskip=0
> - RESTORE_REST
> - RESTORE_ARGS 1, \addskip
> + .macro REMOVE_PTREGS_FROM_STACK addskip=0
> + addq $15*8+\addskip, %rsp
> + CFI_ADJUST_CFA_OFFSET -(15*8+\addskip)
> .endm
>
> .macro icebp
> diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
> index bba3cf8..6f98c16 100644
> --- a/arch/x86/include/asm/irqflags.h
> +++ b/arch/x86/include/asm/irqflags.h
> @@ -171,9 +171,9 @@ static inline int arch_irqs_disabled(void)
> #define ARCH_LOCKDEP_SYS_EXIT_IRQ \
> TRACE_IRQS_ON; \
> sti; \
> - SAVE_REST; \
> + SAVE_EXTRA_REGS; \
> LOCKDEP_SYS_EXIT; \
> - RESTORE_REST; \
> + RESTORE_EXTRA_REGS; \
> cli; \
> TRACE_IRQS_OFF;
>
> diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
> index 6205f0c..c822b35 100644
> --- a/arch/x86/include/asm/ptrace.h
> +++ b/arch/x86/include/asm/ptrace.h
> @@ -31,13 +31,17 @@ struct pt_regs {
> #else /* __i386__ */
>
> struct pt_regs {
> +/*
> + * C ABI says these regs are callee-preserved. They aren't saved on kernel entry
> + * unless syscall needs a complete, fully filled "struct pt_regs".
> + */
> unsigned long r15;
> unsigned long r14;
> unsigned long r13;
> unsigned long r12;
> unsigned long bp;
> unsigned long bx;
> -/* arguments: non interrupts/non tracing syscalls only save up to here*/
> +/* These regs are callee-clobbered. Always saved on kernel entry. */
> unsigned long r11;
> unsigned long r10;
> unsigned long r9;
> @@ -47,9 +51,12 @@ struct pt_regs {
> unsigned long dx;
> unsigned long si;
> unsigned long di;
> +/*
> + * On syscall entry, this is syscall#. On CPU exception, this is error code.
> + * On hw interrupt, it's IRQ number:
> + */
> unsigned long orig_ax;
> -/* end of arguments */
> -/* cpu exception frame or undefined */
> +/* Return frame for iretq */
> unsigned long ip;
> unsigned long cs;
> unsigned long flags;
> diff --git a/arch/x86/include/uapi/asm/ptrace-abi.h b/arch/x86/include/uapi/asm/ptrace-abi.h
> index 7b0a55a..580aee3 100644
> --- a/arch/x86/include/uapi/asm/ptrace-abi.h
> +++ b/arch/x86/include/uapi/asm/ptrace-abi.h
> @@ -25,13 +25,17 @@
> #else /* __i386__ */
>
> #if defined(__ASSEMBLY__) || defined(__FRAME_OFFSETS)
> +/*
> + * C ABI says these regs are callee-preserved. They aren't saved on kernel entry
> + * unless syscall needs a complete, fully filled "struct pt_regs".
> + */
> #define R15 0
> #define R14 8
> #define R13 16
> #define R12 24
> #define RBP 32
> #define RBX 40
> -/* arguments: interrupts/non tracing syscalls only save up to here*/
> +/* These regs are callee-clobbered. Always saved on kernel entry. */
> #define R11 48
> #define R10 56
> #define R9 64
> @@ -41,15 +45,17 @@
> #define RDX 96
> #define RSI 104
> #define RDI 112
> -#define ORIG_RAX 120 /* = ERROR */
> -/* end of arguments */
> -/* cpu exception frame or undefined in case of fast syscall. */
> +/*
> + * On syscall entry, this is syscall#. On CPU exception, this is error code.
> + * On hw interrupt, it's IRQ number:
> + */
> +#define ORIG_RAX 120
> +/* Return frame for iretq */
> #define RIP 128
> #define CS 136
> #define EFLAGS 144
> #define RSP 152
> #define SS 160
> -#define ARGOFFSET R11
> #endif /* __ASSEMBLY__ */
>
> /* top of stack page */
> diff --git a/arch/x86/include/uapi/asm/ptrace.h b/arch/x86/include/uapi/asm/ptrace.h
> index ac4b9aa..bc16115 100644
> --- a/arch/x86/include/uapi/asm/ptrace.h
> +++ b/arch/x86/include/uapi/asm/ptrace.h
> @@ -41,13 +41,17 @@ struct pt_regs {
> #ifndef __KERNEL__
>
> struct pt_regs {
> +/*
> + * C ABI says these regs are callee-preserved. They aren't saved on kernel entry
> + * unless syscall needs a complete, fully filled "struct pt_regs".
> + */
> unsigned long r15;
> unsigned long r14;
> unsigned long r13;
> unsigned long r12;
> unsigned long rbp;
> unsigned long rbx;
> -/* arguments: non interrupts/non tracing syscalls only save up to here*/
> +/* These regs are callee-clobbered. Always saved on kernel entry. */
> unsigned long r11;
> unsigned long r10;
> unsigned long r9;
> @@ -57,9 +61,12 @@ struct pt_regs {
> unsigned long rdx;
> unsigned long rsi;
> unsigned long rdi;
> +/*
> + * On syscall entry, this is syscall#. On CPU exception, this is error code.
> + * On hw interrupt, it's IRQ number:
> + */
> unsigned long orig_rax;
> -/* end of arguments */
> -/* cpu exception frame or undefined */
> +/* Return frame for iretq */
> unsigned long rip;
> unsigned long cs;
> unsigned long eflags;
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 37f7d95..b3c3ebb 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -26,12 +26,6 @@
> * Some macro usage:
> * - CFI macros are used to generate dwarf2 unwind information for better
> * backtraces. They don't change any code.
> - * - SAVE_ALL/RESTORE_ALL - Save/restore all registers
> - * - SAVE_ARGS/RESTORE_ARGS - Save/restore registers that C functions modify.
> - * There are unfortunately lots of special cases where some registers
> - * not touched. The macro is a big mess that should be cleaned up.
> - * - SAVE_REST/RESTORE_REST - Handle the registers not saved by SAVE_ARGS.
> - * Gives a full stack frame.
> * - ENTRY/END Define functions in the symbol table.
> * - FIXUP_TOP_OF_STACK/RESTORE_TOP_OF_STACK - Fix up the hardware stack
> * frame that is otherwise undefined after a SYSCALL
> @@ -264,7 +258,7 @@ ENTRY(ret_from_fork)
>
> GET_THREAD_INFO(%rcx)
>
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
>
> testl $3, CS-ARGOFFSET(%rsp) # from kernel_thread?
> jz 1f
> @@ -276,12 +270,10 @@ ENTRY(ret_from_fork)
> jmp ret_from_sys_call # go to the SYSRET fastpath
>
> 1:
> - subq $REST_SKIP, %rsp # leave space for volatiles
> - CFI_ADJUST_CFA_OFFSET REST_SKIP
> movq %rbp, %rdi
> call *%rbx
> movl $0, RAX(%rsp)
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> jmp int_ret_from_sys_call
> CFI_ENDPROC
> END(ret_from_fork)
> @@ -339,7 +331,8 @@ GLOBAL(system_call_after_swapgs)
> * and short:
> */
> ENABLE_INTERRUPTS(CLBR_NONE)
> - SAVE_ARGS 8,0
> + ALLOC_PTREGS_ON_STACK 8
> + SAVE_C_REGS
> movq %rax,ORIG_RAX-ARGOFFSET(%rsp)
> movq %rcx,RIP-ARGOFFSET(%rsp)
> CFI_REL_OFFSET rip,RIP-ARGOFFSET
> @@ -375,9 +368,9 @@ sysret_check:
> * sysretq will re-enable interrupts:
> */
> TRACE_IRQS_ON
> + RESTORE_C_REGS_EXCEPT_RCX
> movq RIP-ARGOFFSET(%rsp),%rcx
> CFI_REGISTER rip,rcx
> - RESTORE_ARGS 1,-ARG_SKIP,0
> /*CFI_REGISTER rflags,r11*/
> movq PER_CPU_VAR(old_rsp), %rsp
> USERGS_SYSRET64
> @@ -429,7 +422,7 @@ auditsys:
> movq %rax,%rsi /* 2nd arg: syscall number */
> movl $AUDIT_ARCH_X86_64,%edi /* 1st arg: audit arch */
> call __audit_syscall_entry
> - LOAD_ARGS 0 /* reload call-clobbered registers */
> + RESTORE_C_REGS /* reload call-clobbered registers */
> jmp system_call_fastpath
>
> /*
> @@ -453,7 +446,7 @@ tracesys:
> testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
> jz auditsys
> #endif
> - SAVE_REST
> + SAVE_EXTRA_REGS
> movq $-ENOSYS,RAX(%rsp) /* ptrace can change this for a bad syscall */
> FIXUP_TOP_OF_STACK %rdi
> movq %rsp,%rdi
> @@ -463,8 +456,8 @@ tracesys:
> * We don't reload %rax because syscall_trace_enter() returned
> * the value it wants us to use in the table lookup.
> */
> - LOAD_ARGS ARGOFFSET, 1
> - RESTORE_REST
> + RESTORE_C_REGS_EXCEPT_RAX
> + RESTORE_EXTRA_REGS
> #if __SYSCALL_MASK == ~0
> cmpq $__NR_syscall_max,%rax
> #else
> @@ -515,7 +508,7 @@ int_very_careful:
> TRACE_IRQS_ON
> ENABLE_INTERRUPTS(CLBR_NONE)
> int_check_syscall_exit_work:
> - SAVE_REST
> + SAVE_EXTRA_REGS
> /* Check for syscall exit trace */
> testl $_TIF_WORK_SYSCALL_EXIT,%edx
> jz int_signal
> @@ -534,7 +527,7 @@ int_signal:
> call do_notify_resume
> 1: movl $_TIF_WORK_MASK,%edi
> int_restore_rest:
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> DISABLE_INTERRUPTS(CLBR_NONE)
> TRACE_IRQS_OFF
> jmp int_with_check
> @@ -544,15 +537,12 @@ END(system_call)
> .macro FORK_LIKE func
> ENTRY(stub_\func)
> CFI_STARTPROC
> - popq %r11 /* save return address */
> - PARTIAL_FRAME 0
> - SAVE_REST
> - pushq %r11 /* put it back on stack */
> + DEFAULT_FRAME 0, 8 /* offset 8: return address */
> + SAVE_EXTRA_REGS 8
> FIXUP_TOP_OF_STACK %r11, 8
> - DEFAULT_FRAME 0 8 /* offset 8: return address */
> call sys_\func
> RESTORE_TOP_OF_STACK %r11, 8
> - ret $REST_SKIP /* pop extended registers */
> + ret
> CFI_ENDPROC
> END(stub_\func)
> .endm
> @@ -560,7 +550,7 @@ END(stub_\func)
> .macro FIXED_FRAME label,func
> ENTRY(\label)
> CFI_STARTPROC
> - PARTIAL_FRAME 0 8 /* offset 8: return address */
> + DEFAULT_FRAME 0, 8 /* offset 8: return address */
> FIXUP_TOP_OF_STACK %r11, 8-ARGOFFSET
> call \func
> RESTORE_TOP_OF_STACK %r11, 8-ARGOFFSET
> @@ -577,12 +567,12 @@ END(\label)
> ENTRY(stub_execve)
> CFI_STARTPROC
> addq $8, %rsp
> - PARTIAL_FRAME 0
> - SAVE_REST
> + DEFAULT_FRAME 0
> + SAVE_EXTRA_REGS
> FIXUP_TOP_OF_STACK %r11
> call sys_execve
> movq %rax,RAX(%rsp)
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> jmp int_ret_from_sys_call
> CFI_ENDPROC
> END(stub_execve)
> @@ -594,12 +584,12 @@ END(stub_execve)
> ENTRY(stub_rt_sigreturn)
> CFI_STARTPROC
> addq $8, %rsp
> - PARTIAL_FRAME 0
> - SAVE_REST
> + DEFAULT_FRAME 0
> + SAVE_EXTRA_REGS
> FIXUP_TOP_OF_STACK %r11
> call sys_rt_sigreturn
> movq %rax,RAX(%rsp) # fixme, this could be done at the higher layer
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> jmp int_ret_from_sys_call
> CFI_ENDPROC
> END(stub_rt_sigreturn)
> @@ -608,12 +598,12 @@ END(stub_rt_sigreturn)
> ENTRY(stub_x32_rt_sigreturn)
> CFI_STARTPROC
> addq $8, %rsp
> - PARTIAL_FRAME 0
> - SAVE_REST
> + DEFAULT_FRAME 0
> + SAVE_EXTRA_REGS
> FIXUP_TOP_OF_STACK %r11
> call sys32_x32_rt_sigreturn
> movq %rax,RAX(%rsp) # fixme, this could be done at the higher layer
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> jmp int_ret_from_sys_call
> CFI_ENDPROC
> END(stub_x32_rt_sigreturn)
> @@ -621,13 +611,13 @@ END(stub_x32_rt_sigreturn)
> ENTRY(stub_x32_execve)
> CFI_STARTPROC
> addq $8, %rsp
> - PARTIAL_FRAME 0
> - SAVE_REST
> + DEFAULT_FRAME 0
> + SAVE_EXTRA_REGS
> FIXUP_TOP_OF_STACK %r11
> call compat_sys_execve
> RESTORE_TOP_OF_STACK %r11
> movq %rax,RAX(%rsp)
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> jmp int_ret_from_sys_call
> CFI_ENDPROC
> END(stub_x32_execve)
> @@ -683,51 +673,31 @@ END(interrupt)
>
> /* 0(%rsp): ~(interrupt number) */
> .macro interrupt func
> - /* reserve pt_regs for scratch regs and rbp */
> - subq $ORIG_RAX-RBP, %rsp
> - CFI_ADJUST_CFA_OFFSET ORIG_RAX-RBP
> - cld
> - /* start from rbp in pt_regs and jump over */
> - movq_cfi rdi, (RDI-RBP)
> - movq_cfi rsi, (RSI-RBP)
> - movq_cfi rdx, (RDX-RBP)
> - movq_cfi rcx, (RCX-RBP)
> - movq_cfi rax, (RAX-RBP)
> - movq_cfi r8, (R8-RBP)
> - movq_cfi r9, (R9-RBP)
> - movq_cfi r10, (R10-RBP)
> - movq_cfi r11, (R11-RBP)
> -
> - /* Save rbp so that we can unwind from get_irq_regs() */
> - movq_cfi rbp, 0
> -
> - /* Save previous stack value */
> - movq %rsp, %rsi
> -
> - leaq -RBP(%rsp),%rdi /* arg1 for handler */
> - testl $3, CS-RBP(%rsi)
> + ALLOC_PTREGS_ON_STACK
> + SAVE_C_REGS
> + movq %rsp, %rdi /* arg1 for handler */
> + testl $3, CS(%rsp)
> je 1f
> SWAPGS
> - /*
> +1: /*
> * irq_count is used to check if a CPU is already on an interrupt stack
> * or not. While this is essentially redundant with preempt_count it is
> * a little cheaper to use a separate counter in the PDA (short of
> * moving irq_enter into assembly, which would be too much work)
> */
> -1: incl PER_CPU_VAR(irq_count)
> + incl PER_CPU_VAR(irq_count)
> cmovzq PER_CPU_VAR(irq_stack_ptr),%rsp
> - CFI_DEF_CFA_REGISTER rsi
> + CFI_DEF_CFA_REGISTER rdi
>
> /* Store previous stack value */
> - pushq %rsi
> + pushq %rdi
> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
> 0x77 /* DW_OP_breg7 */, 0, \
> 0x06 /* DW_OP_deref */, \
> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
> + 0x08 /* DW_OP_const1u */, SS+8, \
> 0x22 /* DW_OP_plus */
> /* We entered an interrupt context - irqs are off: */
> TRACE_IRQS_OFF
> -
> call \func
> .endm
>
> @@ -749,10 +719,9 @@ ret_from_intr:
>
> /* Restore saved previous stack */
> popq %rsi
> - CFI_DEF_CFA rsi,SS+8-RBP /* reg/off reset after def_cfa_expr */
> - leaq ARGOFFSET-RBP(%rsi), %rsp
> + CFI_DEF_CFA rsi,SS+8 /* reg/off reset after def_cfa_expr */
> + movq %rsi, %rsp
> CFI_DEF_CFA_REGISTER rsp
> - CFI_ADJUST_CFA_OFFSET RBP-ARGOFFSET
>
> exit_intr:
> GET_THREAD_INFO(%rcx)
> @@ -789,7 +758,8 @@ retint_restore_args: /* return to kernel space */
> */
> TRACE_IRQS_IRETQ
> restore_args:
> - RESTORE_ARGS 1,8,1
> + RESTORE_C_REGS
> + REMOVE_PTREGS_FROM_STACK 8
>
> irq_return:
> /*
> @@ -876,12 +846,12 @@ retint_signal:
> jz retint_swapgs
> TRACE_IRQS_ON
> ENABLE_INTERRUPTS(CLBR_NONE)
> - SAVE_REST
> + SAVE_EXTRA_REGS
> movq $-1,ORIG_RAX(%rsp)
> xorl %esi,%esi # oldset
> movq %rsp,%rdi # &pt_regs
> call do_notify_resume
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> DISABLE_INTERRUPTS(CLBR_NONE)
> TRACE_IRQS_OFF
> GET_THREAD_INFO(%rcx)
> @@ -1256,7 +1226,9 @@ ENTRY(xen_failsafe_callback)
> addq $0x30,%rsp
> CFI_ADJUST_CFA_OFFSET -0x30
> pushq_cfi $-1 /* orig_ax = -1 => not a system call */
> - SAVE_ALL
> + ALLOC_PTREGS_ON_STACK
> + SAVE_C_REGS
> + SAVE_EXTRA_REGS
> jmp error_exit
> CFI_ENDPROC
> END(xen_failsafe_callback)
> @@ -1313,11 +1285,15 @@ ENTRY(paranoid_exit)
> paranoid_swapgs:
> TRACE_IRQS_IRETQ 0
> SWAPGS_UNSAFE_STACK
> - RESTORE_ALL 8
> + RESTORE_EXTRA_REGS
> + RESTORE_C_REGS
> + REMOVE_PTREGS_FROM_STACK 8
> jmp irq_return
> paranoid_restore:
> TRACE_IRQS_IRETQ_DEBUG 0
> - RESTORE_ALL 8
> + RESTORE_EXTRA_REGS
> + RESTORE_C_REGS
> + REMOVE_PTREGS_FROM_STACK 8
> jmp irq_return
> paranoid_userspace:
> GET_THREAD_INFO(%rcx)
> @@ -1412,7 +1388,7 @@ END(error_entry)
> ENTRY(error_exit)
> DEFAULT_FRAME
> movl %ebx,%eax
> - RESTORE_REST
> + RESTORE_EXTRA_REGS
> DISABLE_INTERRUPTS(CLBR_NONE)
> TRACE_IRQS_OFF
> GET_THREAD_INFO(%rcx)
> @@ -1671,8 +1647,10 @@ end_repeat_nmi:
> nmi_swapgs:
> SWAPGS_UNSAFE_STACK
> nmi_restore:
> + RESTORE_EXTRA_REGS
> + RESTORE_C_REGS
> /* Pop the extra iret frame at once */
> - RESTORE_ALL 6*8
> + REMOVE_PTREGS_FROM_STACK 6*8
>
> /* Clear the NMI executing stack variable */
> movq $0, 5*8(%rsp)
> diff --git a/arch/x86/kernel/preempt.S b/arch/x86/kernel/preempt.S
> index ca7f0d5..673da2f 100644
> --- a/arch/x86/kernel/preempt.S
> +++ b/arch/x86/kernel/preempt.S
> @@ -6,9 +6,13 @@
>
> ENTRY(___preempt_schedule)
> CFI_STARTPROC
> - SAVE_ALL
> + ALLOC_PTREGS_ON_STACK
> + SAVE_C_REGS
> + SAVE_EXTRA_REGS
> call preempt_schedule
> - RESTORE_ALL
> + RESTORE_EXTRA_REGS
> + RESTORE_C_REGS
> + REMOVE_PTREGS_FROM_STACK
> ret
> CFI_ENDPROC
>
> @@ -16,9 +20,13 @@ ENTRY(___preempt_schedule)
>
> ENTRY(___preempt_schedule_context)
> CFI_STARTPROC
> - SAVE_ALL
> + ALLOC_PTREGS_ON_STACK
> + SAVE_C_REGS
> + SAVE_EXTRA_REGS
> call preempt_schedule_context
> - RESTORE_ALL
> + RESTORE_EXTRA_REGS
> + RESTORE_C_REGS
> + REMOVE_PTREGS_FROM_STACK
> ret
> CFI_ENDPROC
>
> --
> 1.8.1.4
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 17:04 ` Andy Lutomirski
@ 2014-08-04 14:28 ` Denys Vlasenko
2014-08-04 14:47 ` Oleg Nesterov
2014-08-04 21:03 ` Andy Lutomirski
0 siblings, 2 replies; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-04 14:28 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Denys Vlasenko, linux-kernel@vger.kernel.org, Oleg Nesterov,
H. Peter Anvin, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On Fri, Aug 1, 2014 at 7:04 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Aug 1, 2014 at 7:48 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
>> 64-bit code was using six stack slots fewer by not saving/restoring
>> registers which a callee-preserved according to C ABI,
>> and not allocating space for them
>
> This is great.
>
> Next up: remove FIXUP/RESTORE_TOP_OF_STACK? :) Maybe I'll give that a shot.
I'm yet at the stage "what that stuff does anyway?" and at
"why do we need percpu old_rsp thingy?" in particular.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-04 14:28 ` Denys Vlasenko
@ 2014-08-04 14:47 ` Oleg Nesterov
2014-08-04 15:34 ` Oleg Nesterov
2014-08-04 21:03 ` Andy Lutomirski
1 sibling, 1 reply; 41+ messages in thread
From: Oleg Nesterov @ 2014-08-04 14:47 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Andy Lutomirski, Denys Vlasenko, linux-kernel@vger.kernel.org,
H. Peter Anvin, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On 08/04, Denys Vlasenko wrote:
>
> "why do we need percpu old_rsp thingy?" in particular.
See thread_struct->usersp, current_user_stack_pointer().
Oleg.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-04 14:47 ` Oleg Nesterov
@ 2014-08-04 15:34 ` Oleg Nesterov
0 siblings, 0 replies; 41+ messages in thread
From: Oleg Nesterov @ 2014-08-04 15:34 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Andy Lutomirski, Denys Vlasenko, linux-kernel@vger.kernel.org,
H. Peter Anvin, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On 08/04, Oleg Nesterov wrote:
>
> On 08/04, Denys Vlasenko wrote:
> >
> > "why do we need percpu old_rsp thingy?" in particular.
>
> See thread_struct->usersp, current_user_stack_pointer().
Btw, perhaps it makes sense to document that task_pt_regs(current)->sp
is not current_user_stack_pointer() inside system_call_fastpath.
Oleg.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-04 14:28 ` Denys Vlasenko
2014-08-04 14:47 ` Oleg Nesterov
@ 2014-08-04 21:03 ` Andy Lutomirski
2014-08-04 21:23 ` Borislav Petkov
2014-08-05 10:35 ` Denys Vlasenko
1 sibling, 2 replies; 41+ messages in thread
From: Andy Lutomirski @ 2014-08-04 21:03 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Denys Vlasenko, linux-kernel@vger.kernel.org, Oleg Nesterov,
H. Peter Anvin, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On Mon, Aug 4, 2014 at 11:28 PM, Denys Vlasenko
<vda.linux@googlemail.com> wrote:
> On Fri, Aug 1, 2014 at 7:04 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Fri, Aug 1, 2014 at 7:48 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
>>> 64-bit code was using six stack slots fewer by not saving/restoring
>>> registers which a callee-preserved according to C ABI,
>>> and not allocating space for them
>>
>> This is great.
>>
>> Next up: remove FIXUP/RESTORE_TOP_OF_STACK? :) Maybe I'll give that a shot.
>
> I'm yet at the stage "what that stuff does anyway?" and at
> "why do we need percpu old_rsp thingy?" in particular.
On x86_64, the syscall instruction has no effect on rsp. That means
that the entry point starts out with no stack. There are no free
registers whatsoever at the entry point.
That means that the entry code needs to do swapgs, stash rsp somewhere
relative to gs, and then load the kernel's rsp. old_rsp is the spot
used for this.
Now the kernel does an optimization that is, I think, very much not
worth it. The kernel doesn't bother sticking the old rsp value into
pt_regs (saving two instructions on fast path entries) and doesn't
initialize the SS, CS, RCX, and EFLAGS fields in pt_regs, saving four
more instructions.
To make this optimization work, the whole FIXUP/RESTORE_TOP_OF_STACK
dance is needed, and there's the usersp crap in the context switch
code, and current_user_stack_pointer(), and probably even more crap
that I haven't noticed. And I sure hope that nothing in the *compat*
syscall path touches current_user_stack_pointer(), because the compat
code doesn't seem to use old_rsp.
I think this should all be ripped out. The only real difficulty will
be that the sysret code needs to restore rsp itself, so the sysret
path will end up needing two more instructions. Removing all of the
TOP_OF_STACK stuff will add ten instructions to fast path syscalls,
and I wouldn't be surprised if this adds considerably fewer than ten
cycles on any modern chip.
(It's too bad that there's no unlocked xchg; this could be faster if
we had one. It's also too bad that the syscall ABI didn't choose some
register to unconditionally set to zero, which would have given us the
single scratch register we'd need to avoid this whole mess in the
first place.)
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-04 21:03 ` Andy Lutomirski
@ 2014-08-04 21:23 ` Borislav Petkov
2014-08-05 10:35 ` Denys Vlasenko
1 sibling, 0 replies; 41+ messages in thread
From: Borislav Petkov @ 2014-08-04 21:23 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Denys Vlasenko, Denys Vlasenko, linux-kernel@vger.kernel.org,
Oleg Nesterov, H. Peter Anvin, Frederic Weisbecker, X86 ML,
Alexei Starovoitov, Will Drewry, Kees Cook
On Tue, Aug 05, 2014 at 06:03:24AM +0900, Andy Lutomirski wrote:
> (It's too bad that there's no unlocked xchg; this could be faster if
> we had one.
There is - just put both operands in registers. :-)
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-04 21:03 ` Andy Lutomirski
2014-08-04 21:23 ` Borislav Petkov
@ 2014-08-05 10:35 ` Denys Vlasenko
2014-08-05 14:53 ` Andy Lutomirski
1 sibling, 1 reply; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-05 10:35 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Denys Vlasenko, linux-kernel@vger.kernel.org, Oleg Nesterov,
H. Peter Anvin, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On Mon, Aug 4, 2014 at 11:03 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> Next up: remove FIXUP/RESTORE_TOP_OF_STACK? :) Maybe I'll give that a shot.
>>
>> I'm yet at the stage "what that stuff does anyway?" and at
>> "why do we need percpu old_rsp thingy?" in particular.
>
> On x86_64, the syscall instruction has no effect on rsp. That means
> that the entry point starts out with no stack. There are no free
> registers whatsoever at the entry point.
>
> That means that the entry code needs to do swapgs, stash rsp somewhere
> relative to gs, and then load the kernel's rsp. old_rsp is the spot
> used for this.
>
> Now the kernel does an optimization that is, I think, very much not
> worth it. The kernel doesn't bother sticking the old rsp value into
> pt_regs (saving two instructions on fast path entries) and doesn't
> initialize the SS, CS, RCX, and EFLAGS fields in pt_regs, saving four
> more instructions.
>
> To make this optimization work, the whole FIXUP/RESTORE_TOP_OF_STACK
> dance is needed, and there's the usersp crap in the context switch
> code, and current_user_stack_pointer(), and probably even more crap
> that I haven't noticed. And I sure hope that nothing in the *compat*
> syscall path touches current_user_stack_pointer(), because the compat
> code doesn't seem to use old_rsp.
>
> I think this should all be ripped out. The only real difficulty will
> be that the sysret code needs to restore rsp itself, so the sysret
> path will end up needing two more instructions. Removing all of the
> TOP_OF_STACK stuff will add ten instructions to fast path syscalls,
> and I wouldn't be surprised if this adds considerably fewer than ten
> cycles on any modern chip.
Something like this on the fast path? -
SWAPGS_UNSAFE_STACK
movq %rsp,PER_CPU_VAR(old_rsp)
movq PER_CPU_VAR(kernel_stack),%rsp
ENABLE_INTERRUPTS(CLBR_NONE)
ALLOC_PTREGS_ON_STACK 8 /* +8: space for orig_ax */
SAVE_C_REGS
movq %rax,ORIG_RAX(%rsp)
movq %rcx,RIP(%rsp)
+ movq %r11,EFLAGS(%rsp)
+ movq PER_CPU_VAR(old_rsp),%rcx
+ movq %rcx,RSP(%rsp)
...
- RESTORE_C_REGS_EXCEPT_RCX
+ RESTORE_C_REGS_EXCEPT_RCX_R11
movq RIP(%rsp),%rcx
+ movq EFLAGS(%rsp), %r11
- movq PER_CPU_VAR(old_rsp), %rsp
+ movq RSP(%rsp), %rsp
USERGS_SYSRET64
Looks like only 3 additional insns (unfortunately, one is memory read).
Do we need to save rsc and r11 in "struct pt_regs" in their
"standard" slots, though? If we don't, we can drop two insns
(SAVE_C_REGS -> SAVE_C_REGS_EXCEPT_RCX_R11).
Then old_rsp can be nuked everywhere else,
RESTORE_TOP_OF_STACK can be nuked, and
FIXUP_TOP_OF_STACK can be reduced to merely:
movq $__USER_DS,SS(%rsp)
movq $__USER_CS,CS(%rsp)
(BTW, why currently it does "movq $-1,RCX+\offset(%rsp)?)
--
vda
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-05 10:35 ` Denys Vlasenko
@ 2014-08-05 14:53 ` Andy Lutomirski
2014-08-05 15:17 ` Denys Vlasenko
2014-08-07 9:54 ` Denys Vlasenko
0 siblings, 2 replies; 41+ messages in thread
From: Andy Lutomirski @ 2014-08-05 14:53 UTC (permalink / raw)
To: Denys Vlasenko
Cc: X86 ML, Frederic Weisbecker, Denys Vlasenko, Oleg Nesterov,
linux-kernel@vger.kernel.org, H. Peter Anvin, Alexei Starovoitov,
Kees Cook, Will Drewry
On Aug 5, 2014 7:36 PM, "Denys Vlasenko" <vda.linux@googlemail.com> wrote:
>
> On Mon, Aug 4, 2014 at 11:03 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> >>> Next up: remove FIXUP/RESTORE_TOP_OF_STACK? :) Maybe I'll give that a shot.
> >>
> >> I'm yet at the stage "what that stuff does anyway?" and at
> >> "why do we need percpu old_rsp thingy?" in particular.
> >
> > On x86_64, the syscall instruction has no effect on rsp. That means
> > that the entry point starts out with no stack. There are no free
> > registers whatsoever at the entry point.
> >
> > That means that the entry code needs to do swapgs, stash rsp somewhere
> > relative to gs, and then load the kernel's rsp. old_rsp is the spot
> > used for this.
> >
> > Now the kernel does an optimization that is, I think, very much not
> > worth it. The kernel doesn't bother sticking the old rsp value into
> > pt_regs (saving two instructions on fast path entries) and doesn't
> > initialize the SS, CS, RCX, and EFLAGS fields in pt_regs, saving four
> > more instructions.
> >
> > To make this optimization work, the whole FIXUP/RESTORE_TOP_OF_STACK
> > dance is needed, and there's the usersp crap in the context switch
> > code, and current_user_stack_pointer(), and probably even more crap
> > that I haven't noticed. And I sure hope that nothing in the *compat*
> > syscall path touches current_user_stack_pointer(), because the compat
> > code doesn't seem to use old_rsp.
> >
> > I think this should all be ripped out. The only real difficulty will
> > be that the sysret code needs to restore rsp itself, so the sysret
> > path will end up needing two more instructions. Removing all of the
> > TOP_OF_STACK stuff will add ten instructions to fast path syscalls,
> > and I wouldn't be surprised if this adds considerably fewer than ten
> > cycles on any modern chip.
>
> Something like this on the fast path? -
>
> SWAPGS_UNSAFE_STACK
> movq %rsp,PER_CPU_VAR(old_rsp)
> movq PER_CPU_VAR(kernel_stack),%rsp
> ENABLE_INTERRUPTS(CLBR_NONE)
> ALLOC_PTREGS_ON_STACK 8 /* +8: space for orig_ax */
> SAVE_C_REGS
> movq %rax,ORIG_RAX(%rsp)
> movq %rcx,RIP(%rsp)
> + movq %r11,EFLAGS(%rsp)
> + movq PER_CPU_VAR(old_rsp),%rcx
> + movq %rcx,RSP(%rsp)
> ...
> - RESTORE_C_REGS_EXCEPT_RCX
> + RESTORE_C_REGS_EXCEPT_RCX_R11
> movq RIP(%rsp),%rcx
> + movq EFLAGS(%rsp), %r11
> - movq PER_CPU_VAR(old_rsp), %rsp
> + movq RSP(%rsp), %rsp
> USERGS_SYSRET64
The sysret code still needs the inverse, right? ptrace can change
RSP. And, if that happens, then all the context switch code can go,
as can the usersp thread info slot.
>
> Looks like only 3 additional insns (unfortunately, one is memory read).
The store forwarding buffer should handle that one, I think.
> Do we need to save rsc and r11 in "struct pt_regs" in their
> "standard" slots, though?
ptrace probably wants it.
> If we don't, we can drop two insns
> (SAVE_C_REGS -> SAVE_C_REGS_EXCEPT_RCX_R11).
>
> Then old_rsp can be nuked everywhere else,
> RESTORE_TOP_OF_STACK can be nuked, and
> FIXUP_TOP_OF_STACK can be reduced to merely:
>
> movq $__USER_DS,SS(%rsp)
> movq $__USER_CS,CS(%rsp)
Mmm, right. That's probably better than doing this on the fast path.
>
> (BTW, why currently it does "movq $-1,RCX+\offset(%rsp)?)
I would argue this is a bug. (In fact, I have a patch floating around
to fix it. The current code is glitchy in a visible-to-user-space
way.) We should put rcx into both RIP and RCX, since the sysret path
will implicitly do that, and we should restore the same register
values in the iret and sysret paths.
--Andy
>
> --
> vda
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-05 14:53 ` Andy Lutomirski
@ 2014-08-05 15:17 ` Denys Vlasenko
2014-08-05 23:02 ` Andy Lutomirski
2014-08-07 9:54 ` Denys Vlasenko
1 sibling, 1 reply; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-05 15:17 UTC (permalink / raw)
To: Andy Lutomirski, Denys Vlasenko
Cc: X86 ML, Frederic Weisbecker, Oleg Nesterov,
linux-kernel@vger.kernel.org, H. Peter Anvin, Alexei Starovoitov,
Kees Cook, Will Drewry
On 08/05/2014 04:53 PM, Andy Lutomirski wrote:
> On Aug 5, 2014 7:36 PM, "Denys Vlasenko" <vda.linux@googlemail.com> wrote:
>>
>> On Mon, Aug 4, 2014 at 11:03 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>> Next up: remove FIXUP/RESTORE_TOP_OF_STACK? :) Maybe I'll give that a shot.
>>>>
>>>> I'm yet at the stage "what that stuff does anyway?" and at
>>>> "why do we need percpu old_rsp thingy?" in particular.
>>>
>>> On x86_64, the syscall instruction has no effect on rsp. That means
>>> that the entry point starts out with no stack. There are no free
>>> registers whatsoever at the entry point.
>>>
>>> That means that the entry code needs to do swapgs, stash rsp somewhere
>>> relative to gs, and then load the kernel's rsp. old_rsp is the spot
>>> used for this.
>>>
>>> Now the kernel does an optimization that is, I think, very much not
>>> worth it. The kernel doesn't bother sticking the old rsp value into
>>> pt_regs (saving two instructions on fast path entries) and doesn't
>>> initialize the SS, CS, RCX, and EFLAGS fields in pt_regs, saving four
>>> more instructions.
>>>
>>> To make this optimization work, the whole FIXUP/RESTORE_TOP_OF_STACK
>>> dance is needed, and there's the usersp crap in the context switch
>>> code, and current_user_stack_pointer(), and probably even more crap
>>> that I haven't noticed. And I sure hope that nothing in the *compat*
>>> syscall path touches current_user_stack_pointer(), because the compat
>>> code doesn't seem to use old_rsp.
>>>
>>> I think this should all be ripped out. The only real difficulty will
>>> be that the sysret code needs to restore rsp itself, so the sysret
>>> path will end up needing two more instructions. Removing all of the
>>> TOP_OF_STACK stuff will add ten instructions to fast path syscalls,
>>> and I wouldn't be surprised if this adds considerably fewer than ten
>>> cycles on any modern chip.
>>
>> Something like this on the fast path? -
>>
>> SWAPGS_UNSAFE_STACK
>> movq %rsp,PER_CPU_VAR(old_rsp)
>> movq PER_CPU_VAR(kernel_stack),%rsp
>> ENABLE_INTERRUPTS(CLBR_NONE)
>> ALLOC_PTREGS_ON_STACK 8 /* +8: space for orig_ax */
>> SAVE_C_REGS
>> movq %rax,ORIG_RAX(%rsp)
>> movq %rcx,RIP(%rsp)
>> + movq %r11,EFLAGS(%rsp)
>> + movq PER_CPU_VAR(old_rsp),%rcx
>> + movq %rcx,RSP(%rsp)
>> ...
>> - RESTORE_C_REGS_EXCEPT_RCX
>> + RESTORE_C_REGS_EXCEPT_RCX_R11
>> movq RIP(%rsp),%rcx
>> + movq EFLAGS(%rsp), %r11
>> - movq PER_CPU_VAR(old_rsp), %rsp
>> + movq RSP(%rsp), %rsp
>> USERGS_SYSRET64
>
> The sysret code still needs the inverse, right?
The part after "..." in my skecth is the sysret code.
> ptrace can change RSP.
By writing to pt_regs->rsp, yes. And the above code
will pick it up - we read RSP(%rsp).
>> Do we need to save rcx and r11 in "struct pt_regs" in their
>> "standard" slots, though?
>
> ptrace probably wants it.
Let's see.
They don't contain any useful information:
With current code,
pt_regs->r11 is the same as pt_regs->rflags,
pt_regs->rcx is the same as pt_regs->rip (modulo weird store of -1).
So reading them by ptrace is... weird - just read
pt_regs->rflags or pt_regs->rip instead!
If ptrace is active, we'll return via iretq.
If ptrace wrote to these pt_regs members, on return
to userspace current code restores modified values.
My proposed change does not change this.
So, only ptrace reads of rcx and r11 will be affected.
Hmm. Maybe we can fill them only on "tracesys:" codepath?
>> Then old_rsp can be nuked everywhere else,
>> RESTORE_TOP_OF_STACK can be nuked, and
>> FIXUP_TOP_OF_STACK can be reduced to merely:
>>
>> movq $__USER_DS,SS(%rsp)
>> movq $__USER_CS,CS(%rsp)
>
> Mmm, right. That's probably better than doing this on the fast path.
>
>>
>> (BTW, why currently it does "movq $-1,RCX+\offset(%rsp)?)
>
> I would argue this is a bug.
Agree.
> (In fact, I have a patch floating around
> to fix it. The current code is glitchy in a visible-to-user-space
> way.) We should put rcx into both RIP and RCX, since the sysret path
> will implicitly do that, and we should restore the same register
> values in the iret and sysret paths.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-05 15:17 ` Denys Vlasenko
@ 2014-08-05 23:02 ` Andy Lutomirski
0 siblings, 0 replies; 41+ messages in thread
From: Andy Lutomirski @ 2014-08-05 23:02 UTC (permalink / raw)
To: Denys Vlasenko
Cc: X86 ML, Frederic Weisbecker, Oleg Nesterov,
linux-kernel@vger.kernel.org, H. Peter Anvin, Denys Vlasenko,
Alexei Starovoitov, Will Drewry, Kees Cook
On Aug 6, 2014 12:17 AM, "Denys Vlasenko" <dvlasenk@redhat.com> wrote:
>
> On 08/05/2014 04:53 PM, Andy Lutomirski wrote:
> > On Aug 5, 2014 7:36 PM, "Denys Vlasenko" <vda.linux@googlemail.com> wrote:
> >>
> >> On Mon, Aug 4, 2014 at 11:03 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> >>>>> Next up: remove FIXUP/RESTORE_TOP_OF_STACK? :) Maybe I'll give that a shot.
> >>>>
> >>>> I'm yet at the stage "what that stuff does anyway?" and at
> >>>> "why do we need percpu old_rsp thingy?" in particular.
> >>>
> >>> On x86_64, the syscall instruction has no effect on rsp. That means
> >>> that the entry point starts out with no stack. There are no free
> >>> registers whatsoever at the entry point.
> >>>
> >>> That means that the entry code needs to do swapgs, stash rsp somewhere
> >>> relative to gs, and then load the kernel's rsp. old_rsp is the spot
> >>> used for this.
> >>>
> >>> Now the kernel does an optimization that is, I think, very much not
> >>> worth it. The kernel doesn't bother sticking the old rsp value into
> >>> pt_regs (saving two instructions on fast path entries) and doesn't
> >>> initialize the SS, CS, RCX, and EFLAGS fields in pt_regs, saving four
> >>> more instructions.
> >>>
> >>> To make this optimization work, the whole FIXUP/RESTORE_TOP_OF_STACK
> >>> dance is needed, and there's the usersp crap in the context switch
> >>> code, and current_user_stack_pointer(), and probably even more crap
> >>> that I haven't noticed. And I sure hope that nothing in the *compat*
> >>> syscall path touches current_user_stack_pointer(), because the compat
> >>> code doesn't seem to use old_rsp.
> >>>
> >>> I think this should all be ripped out. The only real difficulty will
> >>> be that the sysret code needs to restore rsp itself, so the sysret
> >>> path will end up needing two more instructions. Removing all of the
> >>> TOP_OF_STACK stuff will add ten instructions to fast path syscalls,
> >>> and I wouldn't be surprised if this adds considerably fewer than ten
> >>> cycles on any modern chip.
> >>
> >> Something like this on the fast path? -
> >>
> >> SWAPGS_UNSAFE_STACK
> >> movq %rsp,PER_CPU_VAR(old_rsp)
> >> movq PER_CPU_VAR(kernel_stack),%rsp
> >> ENABLE_INTERRUPTS(CLBR_NONE)
> >> ALLOC_PTREGS_ON_STACK 8 /* +8: space for orig_ax */
> >> SAVE_C_REGS
> >> movq %rax,ORIG_RAX(%rsp)
> >> movq %rcx,RIP(%rsp)
> >> + movq %r11,EFLAGS(%rsp)
> >> + movq PER_CPU_VAR(old_rsp),%rcx
> >> + movq %rcx,RSP(%rsp)
> >> ...
> >> - RESTORE_C_REGS_EXCEPT_RCX
> >> + RESTORE_C_REGS_EXCEPT_RCX_R11
> >> movq RIP(%rsp),%rcx
> >> + movq EFLAGS(%rsp), %r11
> >> - movq PER_CPU_VAR(old_rsp), %rsp
> >> + movq RSP(%rsp), %rsp
> >> USERGS_SYSRET64
> >
> > The sysret code still needs the inverse, right?
>
> The part after "..." in my skecth is the sysret code.
Right :)
>
> > ptrace can change RSP.
>
> By writing to pt_regs->rsp, yes. And the above code
> will pick it up - we read RSP(%rsp).
Also correct -- nice :)
>
> >> Do we need to save rcx and r11 in "struct pt_regs" in their
> >> "standard" slots, though?
> >
> > ptrace probably wants it.
>
> Let's see.
> They don't contain any useful information:
> With current code,
> pt_regs->r11 is the same as pt_regs->rflags,
> pt_regs->rcx is the same as pt_regs->rip (modulo weird store of -1).
> So reading them by ptrace is... weird - just read
> pt_regs->rflags or pt_regs->rip instead!
I'm unconvinced. This is too complicated.
>
> If ptrace is active, we'll return via iretq.
> If ptrace wrote to these pt_regs members, on return
> to userspace current code restores modified values.
> My proposed change does not change this.
>
> So, only ptrace reads of rcx and r11 will be affected.
> Hmm. Maybe we can fill them only on "tracesys:" codepath?
It's not just tracesys -- there's FORK_LIKE, the various pt_regs
stubs, and exit work, too. Setting these values up early is faster,
anyway -- no loads are needed, only stores, and the stores will
presumably be combined with the rest of the frame setup, so no
additional memory bandwidth should be needed.
--Andy
>
> >> Then old_rsp can be nuked everywhere else,
> >> RESTORE_TOP_OF_STACK can be nuked, and
> >> FIXUP_TOP_OF_STACK can be reduced to merely:
> >>
> >> movq $__USER_DS,SS(%rsp)
> >> movq $__USER_CS,CS(%rsp)
> >
> > Mmm, right. That's probably better than doing this on the fast path.
> >
> >>
> >> (BTW, why currently it does "movq $-1,RCX+\offset(%rsp)?)
> >
> > I would argue this is a bug.
>
> Agree.
>
> > (In fact, I have a patch floating around
> > to fix it. The current code is glitchy in a visible-to-user-space
> > way.) We should put rcx into both RIP and RCX, since the sysret path
> > will implicitly do that, and we should restore the same register
> > values in the iret and sysret paths.
>
--Andy
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-05 14:53 ` Andy Lutomirski
2014-08-05 15:17 ` Denys Vlasenko
@ 2014-08-07 9:54 ` Denys Vlasenko
1 sibling, 0 replies; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-07 9:54 UTC (permalink / raw)
To: Andy Lutomirski
Cc: X86 ML, Frederic Weisbecker, Denys Vlasenko, Oleg Nesterov,
linux-kernel@vger.kernel.org, H. Peter Anvin, Alexei Starovoitov,
Kees Cook, Will Drewry
On Tue, Aug 5, 2014 at 4:53 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Aug 5, 2014 7:36 PM, "Denys Vlasenko" <vda.linux@googlemail.com> wrote:
>> Then old_rsp can be nuked everywhere else,
>> RESTORE_TOP_OF_STACK can be nuked, and
>> FIXUP_TOP_OF_STACK can be reduced to merely:
>>
>> movq $__USER_DS,SS(%rsp)
>> movq $__USER_CS,CS(%rsp)
>
> Mmm, right. That's probably better than doing this on the fast path.
I measured it. These particular insns have a nasty huge encoding in x86 -
12 bytes each, no less!
48 c7 84 24 88 00 00 00 33 00 00 00 movq $0x33,0x88(%rsp)
48 c7 84 24 a0 00 00 00 2b 00 00 00 movq $0x2b,0xa0(%rsp)
With them in hot path, I measure 55.6 ns per syscall,
with them moved out of hot path it's 54.8 ns.
This is 2.70GHz CPU, so it must be 2 CPU cycles.
--
vda
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 14:48 ` [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs" Denys Vlasenko
2014-08-01 17:04 ` Andy Lutomirski
@ 2014-08-01 18:09 ` Alexei Starovoitov
2014-08-01 18:30 ` Oleg Nesterov
` (3 subsequent siblings)
5 siblings, 0 replies; 41+ messages in thread
From: Alexei Starovoitov @ 2014-08-01 18:09 UTC (permalink / raw)
To: Denys Vlasenko
Cc: LKML, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
Frederic Weisbecker, X86 ML, Will Drewry, Kees Cook
On Fri, Aug 1, 2014 at 7:48 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
> 64-bit code was using six stack slots fewer by not saving/restoring
> registers which a callee-preserved according to C ABI,
> and not allocating space for them.
>
> Only when syscall needed a complete "struct pt_regs",
> the complete area was allocated and filled in.
>
> This proved to be a source of significant obfuscation and subtle bugs.
> For example, stub_fork had to pop the return address,
> extend the struct, save registers, and push return address back. Ugly.
> ia32_ptregs_common pops return address and "returns" via jmp insn,
> throwing a wrench into CPU return stack cache.
>
> This patch changes code to always allocate a complete "struct pt_regs".
> The saving of registers is still done lazily.
>
> Macros which manipulate "struct pt_regs" on stack are reworked:
> ALLOC_PTREGS_ON_STACK allocates the structure.
> SAVE_C_REGS saves to it those registers which are clobbered by C code.
> SAVE_EXTRA_REGS saves to it all other registers.
> Corresponding RESTORE_* and REMOVE_PTREGS_FROM_STACK macros reverse it.
Looks like a nice cleanup at the cost of extra 48 byte stack gap for fast path.
I'm guessing the gap shouldn't affect performance, but would be nice
to see performance numbers before/after.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 14:48 ` [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs" Denys Vlasenko
2014-08-01 17:04 ` Andy Lutomirski
2014-08-01 18:09 ` Alexei Starovoitov
@ 2014-08-01 18:30 ` Oleg Nesterov
2014-08-01 18:35 ` H. Peter Anvin
` (2 subsequent siblings)
5 siblings, 0 replies; 41+ messages in thread
From: Oleg Nesterov @ 2014-08-01 18:30 UTC (permalink / raw)
To: Denys Vlasenko
Cc: linux-kernel, H. Peter Anvin, Andy Lutomirski,
Frederic Weisbecker, X86 ML, Alexei Starovoitov, Will Drewry,
Kees Cook
On 08/01, Denys Vlasenko wrote:
>
> This patch changes code to always allocate a complete "struct pt_regs".
> The saving of registers is still done lazily.
I obviously like this change very much. Unfortunately I can only ack the
intent ;)
I really hope that maintainers will take a closer look.
Oleg.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 14:48 ` [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs" Denys Vlasenko
` (2 preceding siblings ...)
2014-08-01 18:30 ` Oleg Nesterov
@ 2014-08-01 18:35 ` H. Peter Anvin
2014-08-01 22:11 ` Denys Vlasenko
2014-08-01 22:52 ` Frederic Weisbecker
2014-08-01 23:19 ` Frederic Weisbecker
5 siblings, 1 reply; 41+ messages in thread
From: H. Peter Anvin @ 2014-08-01 18:35 UTC (permalink / raw)
To: Denys Vlasenko, linux-kernel
Cc: Oleg Nesterov, Andy Lutomirski, Frederic Weisbecker, X86 ML,
Alexei Starovoitov, Will Drewry, Kees Cook
On 08/01/2014 07:48 AM, Denys Vlasenko wrote:
>
> Patch was run-tested: 64-bit executables, 32-bit executables,
> strace works.
>
Could you please try to see if there is a measurable change in the
latency of a trivial syscall?
-hpa
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 18:35 ` H. Peter Anvin
@ 2014-08-01 22:11 ` Denys Vlasenko
2014-08-01 22:13 ` H. Peter Anvin
0 siblings, 1 reply; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-01 22:11 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Denys Vlasenko, Linux Kernel Mailing List, Oleg Nesterov,
Andy Lutomirski, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On Fri, Aug 1, 2014 at 8:35 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 08/01/2014 07:48 AM, Denys Vlasenko wrote:
>>
>> Patch was run-tested: 64-bit executables, 32-bit executables,
>> strace works.
>>
>
> Could you please try to see if there is a measurable change in the
> latency of a trivial syscall?
Will do.
Something along the lines of "how long does it take to execute two
gazillions of getppid()?"
I'll send v2 shortly - I just figured out what was this mysterious
dance with allocating "not-quite-full struct pt_regs" on interrupt path:
It is intended to make nested interrupts to have better
backtraces. I broke that by this patch, v2 will have this feature retained.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 22:11 ` Denys Vlasenko
@ 2014-08-01 22:13 ` H. Peter Anvin
2014-08-02 21:14 ` Andy Lutomirski
0 siblings, 1 reply; 41+ messages in thread
From: H. Peter Anvin @ 2014-08-01 22:13 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Denys Vlasenko, Linux Kernel Mailing List, Oleg Nesterov,
Andy Lutomirski, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On 08/01/2014 03:11 PM, Denys Vlasenko wrote:
>>
>> Could you please try to see if there is a measurable change in the
>> latency of a trivial syscall?
>
> Will do.
> Something along the lines of "how long does it take to execute two
> gazillions of getppid()?"
>
Something like that, yes, but you have to run enough data points so you
can determine if the difference is statistically significant or just noise.
-hpa
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 22:13 ` H. Peter Anvin
@ 2014-08-02 21:14 ` Andy Lutomirski
2014-08-02 21:23 ` H. Peter Anvin
0 siblings, 1 reply; 41+ messages in thread
From: Andy Lutomirski @ 2014-08-02 21:14 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Denys Vlasenko, Denys Vlasenko, Linux Kernel Mailing List,
Oleg Nesterov, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On Fri, Aug 1, 2014 at 3:13 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 08/01/2014 03:11 PM, Denys Vlasenko wrote:
>>>
>>> Could you please try to see if there is a measurable change in the
>>> latency of a trivial syscall?
>>
>> Will do.
>> Something along the lines of "how long does it take to execute two
>> gazillions of getppid()?"
>>
>
> Something like that, yes, but you have to run enough data points so you
> can determine if the difference is statistically significant or just noise.
Denys, if you want to avoid five minutes of programming, you can build this:
https://gitorious.org/linux-test-utils/linux-clock-tests/
and run timing_test_64 10 getpid
It doesn't do real statistics, but it seems to get quite stable results.
--Andy
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-02 21:14 ` Andy Lutomirski
@ 2014-08-02 21:23 ` H. Peter Anvin
2014-08-02 21:38 ` Andy Lutomirski
0 siblings, 1 reply; 41+ messages in thread
From: H. Peter Anvin @ 2014-08-02 21:23 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Denys Vlasenko, Denys Vlasenko, Linux Kernel Mailing List,
Oleg Nesterov, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
It would be nice to automate running a T-test on it.
On August 2, 2014 2:14:50 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On Fri, Aug 1, 2014 at 3:13 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 08/01/2014 03:11 PM, Denys Vlasenko wrote:
>>>>
>>>> Could you please try to see if there is a measurable change in the
>>>> latency of a trivial syscall?
>>>
>>> Will do.
>>> Something along the lines of "how long does it take to execute two
>>> gazillions of getppid()?"
>>>
>>
>> Something like that, yes, but you have to run enough data points so
>you
>> can determine if the difference is statistically significant or just
>noise.
>
>Denys, if you want to avoid five minutes of programming, you can build
>this:
>
>https://gitorious.org/linux-test-utils/linux-clock-tests/
>
>and run timing_test_64 10 getpid
>
>It doesn't do real statistics, but it seems to get quite stable
>results.
>
>--Andy
--
Sent from my mobile phone. Please pardon brevity and lack of formatting.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-02 21:23 ` H. Peter Anvin
@ 2014-08-02 21:38 ` Andy Lutomirski
0 siblings, 0 replies; 41+ messages in thread
From: Andy Lutomirski @ 2014-08-02 21:38 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Denys Vlasenko, Denys Vlasenko, Linux Kernel Mailing List,
Oleg Nesterov, Frederic Weisbecker, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On Sat, Aug 2, 2014 at 2:23 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> It would be nice to automate running a T-test on it.
I'll see if I can do something like that. Using the t statistic seems
like overkill here, though -- timing_test_64 runs millions of
iterations, so even if I batch them, I'll end up with n ~ 1000 (or
even larger), at which point plain old Gaussians should be fine.
--Andy
>
> On August 2, 2014 2:14:50 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>>On Fri, Aug 1, 2014 at 3:13 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 08/01/2014 03:11 PM, Denys Vlasenko wrote:
>>>>>
>>>>> Could you please try to see if there is a measurable change in the
>>>>> latency of a trivial syscall?
>>>>
>>>> Will do.
>>>> Something along the lines of "how long does it take to execute two
>>>> gazillions of getppid()?"
>>>>
>>>
>>> Something like that, yes, but you have to run enough data points so
>>you
>>> can determine if the difference is statistically significant or just
>>noise.
>>
>>Denys, if you want to avoid five minutes of programming, you can build
>>this:
>>
>>https://gitorious.org/linux-test-utils/linux-clock-tests/
>>
>>and run timing_test_64 10 getpid
>>
>>It doesn't do real statistics, but it seems to get quite stable
>>results.
>>
>>--Andy
>
> --
> Sent from my mobile phone. Please pardon brevity and lack of formatting.
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 14:48 ` [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs" Denys Vlasenko
` (3 preceding siblings ...)
2014-08-01 18:35 ` H. Peter Anvin
@ 2014-08-01 22:52 ` Frederic Weisbecker
2014-08-01 23:19 ` Frederic Weisbecker
5 siblings, 0 replies; 41+ messages in thread
From: Frederic Weisbecker @ 2014-08-01 22:52 UTC (permalink / raw)
To: Denys Vlasenko
Cc: linux-kernel, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
X86 ML, Alexei Starovoitov, Will Drewry, Kees Cook
On Fri, Aug 01, 2014 at 04:48:17PM +0200, Denys Vlasenko wrote:
> 64-bit code was using six stack slots fewer by not saving/restoring
> registers which a callee-preserved according to C ABI,
> and not allocating space for them.
>
> Only when syscall needed a complete "struct pt_regs",
> the complete area was allocated and filled in.
>
> This proved to be a source of significant obfuscation and subtle bugs.
> For example, stub_fork had to pop the return address,
> extend the struct, save registers, and push return address back. Ugly.
> ia32_ptregs_common pops return address and "returns" via jmp insn,
> throwing a wrench into CPU return stack cache.
>
> This patch changes code to always allocate a complete "struct pt_regs".
> The saving of registers is still done lazily.
>
> Macros which manipulate "struct pt_regs" on stack are reworked:
> ALLOC_PTREGS_ON_STACK allocates the structure.
> SAVE_C_REGS saves to it those registers which are clobbered by C code.
> SAVE_EXTRA_REGS saves to it all other registers.
> Corresponding RESTORE_* and REMOVE_PTREGS_FROM_STACK macros reverse it.
>
> ia32_ptregs_common, stub_fork and friends lost their ugly dance with
> return pointer.
>
> LOAD_ARGS32 in ia32entry.S now uses a symbolic stack offsets
> instead of magic numbers.
>
> Misleading and slightly wrong comments in "struct pt_regs" are fixed
> (four instances).
>
> Patch was run-tested: 64-bit executables, 32-bit executables,
> strace works.
>
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: X86 ML <x86@kernel.org>
> CC: Alexei Starovoitov <ast@plumgrid.com>
> CC: Will Drewry <wad@chromium.org>
> CC: Kees Cook <keescook@chromium.org>
> CC: linux-kernel@vger.kernel.org
> ---
> arch/x86/ia32/ia32entry.S | 47 +++----
> arch/x86/include/asm/calling.h | 224 ++++++++++++++++-----------------
> arch/x86/include/asm/irqflags.h | 4 +-
> arch/x86/include/asm/ptrace.h | 13 +-
> arch/x86/include/uapi/asm/ptrace-abi.h | 16 ++-
> arch/x86/include/uapi/asm/ptrace.h | 13 +-
> arch/x86/kernel/entry_64.S | 132 ++++++++-----------
> arch/x86/kernel/preempt.S | 16 ++-
> 8 files changed, 232 insertions(+), 233 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
> index 4299eb0..ef9ee16 100644
> --- a/arch/x86/ia32/ia32entry.S
> +++ b/arch/x86/ia32/ia32entry.S
> @@ -62,12 +62,12 @@
> */
> .macro LOAD_ARGS32 offset, _r9=0
> .if \_r9
> - movl \offset+16(%rsp),%r9d
> + movl \offset+R9(%rsp),%r9d
> .endif
> - movl \offset+40(%rsp),%ecx
> - movl \offset+48(%rsp),%edx
> - movl \offset+56(%rsp),%esi
> - movl \offset+64(%rsp),%edi
> + movl \offset+RCX(%rsp),%ecx
> + movl \offset+RDX(%rsp),%edx
> + movl \offset+RSI(%rsp),%esi
> + movl \offset+RDI(%rsp),%edi
> movl %eax,%eax /* zero extension */
> .endm
>
> @@ -144,7 +144,8 @@ ENTRY(ia32_sysenter_target)
> CFI_REL_OFFSET rip,0
> pushq_cfi %rax
> cld
> - SAVE_ARGS 0,1,0
> + ALLOC_PTREGS_ON_STACK
> + SAVE_C_REGS_EXCEPT_R891011
> /* no need to do an access_ok check here because rbp has been
> 32bit zero extended */
> ASM_STAC
> @@ -172,7 +173,8 @@ sysexit_from_sys_call:
> andl $~0x200,EFLAGS-R11(%rsp)
> movl RIP-R11(%rsp),%edx /* User %eip */
> CFI_REGISTER rip,rdx
> - RESTORE_ARGS 0,24,0,0,0,0
> + RESTORE_RSI_RDI
I heard there will be a v2 so I'll probably wait for it to review this patch
which really requires 0db where I sit.
But the macro names like above look much clearer as well!
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 14:48 ` [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs" Denys Vlasenko
` (4 preceding siblings ...)
2014-08-01 22:52 ` Frederic Weisbecker
@ 2014-08-01 23:19 ` Frederic Weisbecker
2014-08-04 3:03 ` Denys Vlasenko
5 siblings, 1 reply; 41+ messages in thread
From: Frederic Weisbecker @ 2014-08-01 23:19 UTC (permalink / raw)
To: Denys Vlasenko
Cc: linux-kernel, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
X86 ML, Alexei Starovoitov, Will Drewry, Kees Cook
On Fri, Aug 01, 2014 at 04:48:17PM +0200, Denys Vlasenko wrote:
>
> /* 0(%rsp): ~(interrupt number) */
> .macro interrupt func
> - /* reserve pt_regs for scratch regs and rbp */
> - subq $ORIG_RAX-RBP, %rsp
> - CFI_ADJUST_CFA_OFFSET ORIG_RAX-RBP
> - cld
> - /* start from rbp in pt_regs and jump over */
> - movq_cfi rdi, (RDI-RBP)
> - movq_cfi rsi, (RSI-RBP)
> - movq_cfi rdx, (RDX-RBP)
> - movq_cfi rcx, (RCX-RBP)
> - movq_cfi rax, (RAX-RBP)
> - movq_cfi r8, (R8-RBP)
> - movq_cfi r9, (R9-RBP)
> - movq_cfi r10, (R10-RBP)
> - movq_cfi r11, (R11-RBP)
> -
> - /* Save rbp so that we can unwind from get_irq_regs() */
> - movq_cfi rbp, 0
Hmm SAVEE_C_REGS below doesn't seem to save rbp like we did before.
Perhaps it's implicitely saved somewhere?
> -
> - /* Save previous stack value */
> - movq %rsp, %rsi
Also rsp isn't saved in %rsi like before. Maybe
that's because we already save it in rdi? Makes sense since
now arg1 == rsp. More on that later.
> -
> - leaq -RBP(%rsp),%rdi /* arg1 for handler */
> - testl $3, CS-RBP(%rsi)
> + ALLOC_PTREGS_ON_STACK
> + SAVE_C_REGS
> + movq %rsp, %rdi /* arg1 for handler */
> + testl $3, CS(%rsp)
> je 1f
> SWAPGS
> - /*
> +1: /*
> * irq_count is used to check if a CPU is already on an interrupt stack
> * or not. While this is essentially redundant with preempt_count it is
> * a little cheaper to use a separate counter in the PDA (short of
> * moving irq_enter into assembly, which would be too much work)
> */
> -1: incl PER_CPU_VAR(irq_count)
> + incl PER_CPU_VAR(irq_count)
> cmovzq PER_CPU_VAR(irq_stack_ptr),%rsp
> - CFI_DEF_CFA_REGISTER rsi
> + CFI_DEF_CFA_REGISTER rdi
>
> /* Store previous stack value */
> - pushq %rsi
> + pushq %rdi
So you push rdi instead...
> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
> 0x77 /* DW_OP_breg7 */, 0, \
> 0x06 /* DW_OP_deref */, \
> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
> + 0x08 /* DW_OP_const1u */, SS+8, \
> 0x22 /* DW_OP_plus */
> /* We entered an interrupt context - irqs are off: */
> TRACE_IRQS_OFF
> -
> call \func
> .endm
>
> @@ -749,10 +719,9 @@ ret_from_intr:
>
> /* Restore saved previous stack */
> popq %rsi
And then you pop to rsi. Ok that indeed works but perhaps we should keep it symetrical
just for clarity? Any reason why we can't reuse rdi here?
> - CFI_DEF_CFA rsi,SS+8-RBP /* reg/off reset after def_cfa_expr */
> - leaq ARGOFFSET-RBP(%rsi), %rsp
> + CFI_DEF_CFA rsi,SS+8 /* reg/off reset after def_cfa_expr */
> + movq %rsi, %rsp
> CFI_DEF_CFA_REGISTER rsp
> - CFI_ADJUST_CFA_OFFSET RBP-ARGOFFSET
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-01 23:19 ` Frederic Weisbecker
@ 2014-08-04 3:03 ` Denys Vlasenko
2014-08-04 7:57 ` Borislav Petkov
2014-08-11 0:46 ` Frederic Weisbecker
0 siblings, 2 replies; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-04 3:03 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Denys Vlasenko, Linux Kernel Mailing List, Oleg Nesterov,
H. Peter Anvin, Andy Lutomirski, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On Sat, Aug 2, 2014 at 1:19 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Fri, Aug 01, 2014 at 04:48:17PM +0200, Denys Vlasenko wrote:
>>
>> /* 0(%rsp): ~(interrupt number) */
>> .macro interrupt func
>> - /* reserve pt_regs for scratch regs and rbp */
>> - subq $ORIG_RAX-RBP, %rsp
>> - CFI_ADJUST_CFA_OFFSET ORIG_RAX-RBP
>> - cld
>> - /* start from rbp in pt_regs and jump over */
>> - movq_cfi rdi, (RDI-RBP)
>> - movq_cfi rsi, (RSI-RBP)
>> - movq_cfi rdx, (RDX-RBP)
>> - movq_cfi rcx, (RCX-RBP)
>> - movq_cfi rax, (RAX-RBP)
>> - movq_cfi r8, (R8-RBP)
>> - movq_cfi r9, (R9-RBP)
>> - movq_cfi r10, (R10-RBP)
>> - movq_cfi r11, (R11-RBP)
>> -
>> - /* Save rbp so that we can unwind from get_irq_regs() */
>> - movq_cfi rbp, 0
>
> Hmm SAVEE_C_REGS below doesn't seem to save rbp like we did before.
> Perhaps it's implicitely saved somewhere?
>
>> -
>> - /* Save previous stack value */
>> - movq %rsp, %rsi
>
> Also rsp isn't saved in %rsi like before. Maybe
> that's because we already save it in rdi? Makes sense since
> now arg1 == rsp. More on that later.
>
>> -
>> - leaq -RBP(%rsp),%rdi /* arg1 for handler */
>> - testl $3, CS-RBP(%rsi)
>> + ALLOC_PTREGS_ON_STACK
>> + SAVE_C_REGS
>> + movq %rsp, %rdi /* arg1 for handler */
>> + testl $3, CS(%rsp)
>> je 1f
>> SWAPGS
>> - /*
>> +1: /*
>> * irq_count is used to check if a CPU is already on an interrupt stack
>> * or not. While this is essentially redundant with preempt_count it is
>> * a little cheaper to use a separate counter in the PDA (short of
>> * moving irq_enter into assembly, which would be too much work)
>> */
>> -1: incl PER_CPU_VAR(irq_count)
>> + incl PER_CPU_VAR(irq_count)
>> cmovzq PER_CPU_VAR(irq_stack_ptr),%rsp
>> - CFI_DEF_CFA_REGISTER rsi
>> + CFI_DEF_CFA_REGISTER rdi
>>
>> /* Store previous stack value */
>> - pushq %rsi
>> + pushq %rdi
>
> So you push rdi instead...
>
>> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
>> 0x77 /* DW_OP_breg7 */, 0, \
>> 0x06 /* DW_OP_deref */, \
>> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
>> + 0x08 /* DW_OP_const1u */, SS+8, \
>> 0x22 /* DW_OP_plus */
>> /* We entered an interrupt context - irqs are off: */
>> TRACE_IRQS_OFF
>> -
>> call \func
>> .endm
>>
>> @@ -749,10 +719,9 @@ ret_from_intr:
>>
>> /* Restore saved previous stack */
>> popq %rsi
>
> And then you pop to rsi. Ok that indeed works but perhaps we should keep it symetrical
> just for clarity? Any reason why we can't reuse rdi here?
I changed this entire area in v2: basically, I will not change the logic,
but will add comments explaining what are we doing here, and why.
(Some minor code changes will be done, not affecting the logic).
While we are at it, what this CFI_ESCAPE thing does here?
As usual, it has no comment :/
--
vda
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-04 3:03 ` Denys Vlasenko
@ 2014-08-04 7:57 ` Borislav Petkov
2014-08-11 0:46 ` Frederic Weisbecker
1 sibling, 0 replies; 41+ messages in thread
From: Borislav Petkov @ 2014-08-04 7:57 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Frederic Weisbecker, Denys Vlasenko, Linux Kernel Mailing List,
Oleg Nesterov, H. Peter Anvin, Andy Lutomirski, X86 ML,
Alexei Starovoitov, Will Drewry, Kees Cook
On Mon, Aug 04, 2014 at 05:03:42AM +0200, Denys Vlasenko wrote:
> I changed this entire area in v2: basically, I will not change the
> logic, but will add comments explaining what are we doing here, and
> why.
Very good idea! This file needs some good commenting.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-04 3:03 ` Denys Vlasenko
2014-08-04 7:57 ` Borislav Petkov
@ 2014-08-11 0:46 ` Frederic Weisbecker
2014-08-11 8:40 ` Jan Beulich
1 sibling, 1 reply; 41+ messages in thread
From: Frederic Weisbecker @ 2014-08-11 0:46 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Denys Vlasenko, Linux Kernel Mailing List, Oleg Nesterov,
H. Peter Anvin, Andy Lutomirski, X86 ML, Alexei Starovoitov,
Will Drewry, Kees Cook
On Mon, Aug 04, 2014 at 05:03:42AM +0200, Denys Vlasenko wrote:
> On Sat, Aug 2, 2014 at 1:19 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
> >> 0x77 /* DW_OP_breg7 */, 0, \
> >> 0x06 /* DW_OP_deref */, \
> >> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
> >> + 0x08 /* DW_OP_const1u */, SS+8, \
> >> 0x22 /* DW_OP_plus */
> >> /* We entered an interrupt context - irqs are off: */
> >> TRACE_IRQS_OFF
> >> -
> >> call \func
> >> .endm
> >>
> >> @@ -749,10 +719,9 @@ ret_from_intr:
> >>
> >> /* Restore saved previous stack */
> >> popq %rsi
> >
> > And then you pop to rsi. Ok that indeed works but perhaps we should keep it symetrical
> > just for clarity? Any reason why we can't reuse rdi here?
>
> I changed this entire area in v2: basically, I will not change the logic,
> but will add comments explaining what are we doing here, and why.
> (Some minor code changes will be done, not affecting the logic).
>
> While we are at it, what this CFI_ESCAPE thing does here?
> As usual, it has no comment :/
I don't know, only Jan Beulich understands those CFI black magic.
BTW he doesn't appears to be Cc, we should add him.
>
> --
> vda
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 0:46 ` Frederic Weisbecker
@ 2014-08-11 8:40 ` Jan Beulich
2014-08-11 9:07 ` Andy Lutomirski
2014-08-11 13:26 ` Denys Vlasenko
0 siblings, 2 replies; 41+ messages in thread
From: Jan Beulich @ 2014-08-11 8:40 UTC (permalink / raw)
To: Frederic Weisbecker, Denys Vlasenko
Cc: Andy Lutomirski, Kees Cook, Will Drewry, X86 ML,
Alexei Starovoitov, Denys Vlasenko, Oleg Nesterov,
Linux Kernel Mailing List, H. Peter Anvin
>>> On 11.08.14 at 02:46, <fweisbec@gmail.com> wrote:
> On Mon, Aug 04, 2014 at 05:03:42AM +0200, Denys Vlasenko wrote:
>> On Sat, Aug 2, 2014 at 1:19 AM, Frederic Weisbecker <fweisbec@gmail.com>
> wrote:
>> >> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
>> >> 0x77 /* DW_OP_breg7 */, 0, \
>> >> 0x06 /* DW_OP_deref */, \
>> >> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
>> >> + 0x08 /* DW_OP_const1u */, SS+8, \
>> >> 0x22 /* DW_OP_plus */
>> >> /* We entered an interrupt context - irqs are off: */
>> >> TRACE_IRQS_OFF
>> >> -
>> >> call \func
>> >> .endm
>> >>
>> >> @@ -749,10 +719,9 @@ ret_from_intr:
>> >>
>> >> /* Restore saved previous stack */
>> >> popq %rsi
>> >
>> > And then you pop to rsi. Ok that indeed works but perhaps we should keep it symetrical
>> > just for clarity? Any reason why we can't reuse rdi here?
>>
>> I changed this entire area in v2: basically, I will not change the logic,
>> but will add comments explaining what are we doing here, and why.
>> (Some minor code changes will be done, not affecting the logic).
>>
>> While we are at it, what this CFI_ESCAPE thing does here?
>> As usual, it has no comment :/
Each of its lines has a comment; with other CFI annotations not
each having comments, I don't see what else is needed here.
> I don't know, only Jan Beulich understands those CFI black magic.
That would be very said if true.
In any case: This needs to be a CFI_ESCAPE because there's no
other way I know of to emit the DW_CFA_def_cfa_expression.
And the change to it looks correct to me.
Jan
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 8:40 ` Jan Beulich
@ 2014-08-11 9:07 ` Andy Lutomirski
2014-08-11 9:31 ` Jan Beulich
2014-08-11 13:26 ` Denys Vlasenko
1 sibling, 1 reply; 41+ messages in thread
From: Andy Lutomirski @ 2014-08-11 9:07 UTC (permalink / raw)
To: Jan Beulich
Cc: Frederic Weisbecker, Denys Vlasenko, Kees Cook, Will Drewry,
X86 ML, Alexei Starovoitov, Denys Vlasenko, Oleg Nesterov,
Linux Kernel Mailing List, H. Peter Anvin
On Mon, Aug 11, 2014 at 5:40 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 11.08.14 at 02:46, <fweisbec@gmail.com> wrote:
>> On Mon, Aug 04, 2014 at 05:03:42AM +0200, Denys Vlasenko wrote:
>>> On Sat, Aug 2, 2014 at 1:19 AM, Frederic Weisbecker <fweisbec@gmail.com>
>> wrote:
>>> >> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
>>> >> 0x77 /* DW_OP_breg7 */, 0, \
>>> >> 0x06 /* DW_OP_deref */, \
>>> >> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
>>> >> + 0x08 /* DW_OP_const1u */, SS+8, \
>>> >> 0x22 /* DW_OP_plus */
>>> >> /* We entered an interrupt context - irqs are off: */
>>> >> TRACE_IRQS_OFF
>>> >> -
>>> >> call \func
>>> >> .endm
>>> >>
>>> >> @@ -749,10 +719,9 @@ ret_from_intr:
>>> >>
>>> >> /* Restore saved previous stack */
>>> >> popq %rsi
>>> >
>>> > And then you pop to rsi. Ok that indeed works but perhaps we should keep it symetrical
>>> > just for clarity? Any reason why we can't reuse rdi here?
>>>
>>> I changed this entire area in v2: basically, I will not change the logic,
>>> but will add comments explaining what are we doing here, and why.
>>> (Some minor code changes will be done, not affecting the logic).
>>>
>>> While we are at it, what this CFI_ESCAPE thing does here?
>>> As usual, it has no comment :/
>
> Each of its lines has a comment; with other CFI annotations not
> each having comments, I don't see what else is needed here.
>
>> I don't know, only Jan Beulich understands those CFI black magic.
>
> That would be very said if true.
>
> In any case: This needs to be a CFI_ESCAPE because there's no
> other way I know of to emit the DW_CFA_def_cfa_expression.
> And the change to it looks correct to me.
>
How does one test the entry CFI annotations? The best that I know of
is to single-step through using gdb attached to qemu and see whether
backtraces seem to work.
--Andy
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 9:07 ` Andy Lutomirski
@ 2014-08-11 9:31 ` Jan Beulich
0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2014-08-11 9:31 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Kees Cook, Will Drewry, Frederic Weisbecker, Denys Vlasenko,
X86 ML, Alexei Starovoitov, Denys Vlasenko, Oleg Nesterov,
Linux Kernel Mailing List, H. Peter Anvin
>>> On 11.08.14 at 11:07, <luto@amacapital.net> wrote:
> How does one test the entry CFI annotations? The best that I know of
> is to single-step through using gdb attached to qemu and see whether
> backtraces seem to work.
Or have the kernel generate a backtrace from a suitable location and
check that the backtrace is correct (of course provided you have Dwarf
stack unwinding support in your kernel).
Jan
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 8:40 ` Jan Beulich
2014-08-11 9:07 ` Andy Lutomirski
@ 2014-08-11 13:26 ` Denys Vlasenko
2014-08-11 14:17 ` Jan Beulich
1 sibling, 1 reply; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-11 13:26 UTC (permalink / raw)
To: Jan Beulich, Frederic Weisbecker, Denys Vlasenko
Cc: Andy Lutomirski, Kees Cook, Will Drewry, X86 ML,
Alexei Starovoitov, Oleg Nesterov, Linux Kernel Mailing List,
H. Peter Anvin
On 08/11/2014 10:40 AM, Jan Beulich wrote:
>>>>> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
>>>>> 0x77 /* DW_OP_breg7 */, 0, \
>>>>> 0x06 /* DW_OP_deref */, \
>>>>> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
>>>>> + 0x08 /* DW_OP_const1u */, SS+8, \
>>>>> 0x22 /* DW_OP_plus */
>>>>> /* We entered an interrupt context - irqs are off: */
>>>>> TRACE_IRQS_OFF
...
...
>>> While we are at it, what this CFI_ESCAPE thing does here?
>>> As usual, it has no comment :/
>
> Each of its lines has a comment; with other CFI annotations not
> each having comments, I don't see what else is needed here.
The existing comments explain what every byte means.
They are useful if CFI-literate reader wants to check correctness
of the encoding of this annotation.
There is no overall comment what this CFI annotation
*achieves*. In human language, what do we say
to DWARF decoder here?
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 13:26 ` Denys Vlasenko
@ 2014-08-11 14:17 ` Jan Beulich
2014-08-11 14:53 ` H. Peter Anvin
0 siblings, 1 reply; 41+ messages in thread
From: Jan Beulich @ 2014-08-11 14:17 UTC (permalink / raw)
To: Denys Vlasenko, Denys Vlasenko
Cc: Andy Lutomirski, Kees Cook, Will Drewry, Frederic Weisbecker,
X86 ML, Alexei Starovoitov, Oleg Nesterov,
Linux Kernel Mailing List, H. Peter Anvin
>>> On 11.08.14 at 15:26, <dvlasenk@redhat.com> wrote:
> On 08/11/2014 10:40 AM, Jan Beulich wrote:
>>>>>> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
>>>>>> 0x77 /* DW_OP_breg7 */, 0, \
>>>>>> 0x06 /* DW_OP_deref */, \
>>>>>> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
>>>>>> + 0x08 /* DW_OP_const1u */, SS+8, \
>>>>>> 0x22 /* DW_OP_plus */
>>>>>> /* We entered an interrupt context - irqs are off: */
>>>>>> TRACE_IRQS_OFF
> ...
> ...
>>>> While we are at it, what this CFI_ESCAPE thing does here?
>>>> As usual, it has no comment :/
>>
>> Each of its lines has a comment; with other CFI annotations not
>> each having comments, I don't see what else is needed here.
>
> The existing comments explain what every byte means.
> They are useful if CFI-literate reader wants to check correctness
> of the encoding of this annotation.
>
> There is no overall comment what this CFI annotation
> *achieves*. In human language, what do we say
> to DWARF decoder here?
Short answer: DW_CFA_def_cfa_expression.
Longer response: Just like I said before, what you're asking for is
identical to ask for each other CFI annotation to get a comment
associated to tell you what it's doing, which I don't think you
really mean to ask for. (Our main problem here is that we can't
specify expressions with the .cfi_* gas directives, and hence have
to resort to .cfi_escape.)
Jan
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 14:17 ` Jan Beulich
@ 2014-08-11 14:53 ` H. Peter Anvin
2014-08-11 15:08 ` Jan Beulich
0 siblings, 1 reply; 41+ messages in thread
From: H. Peter Anvin @ 2014-08-11 14:53 UTC (permalink / raw)
To: Jan Beulich, Denys Vlasenko, Denys Vlasenko
Cc: Andy Lutomirski, Kees Cook, Will Drewry, Frederic Weisbecker,
X86 ML, Alexei Starovoitov, Oleg Nesterov,
Linux Kernel Mailing List
On 08/11/2014 07:17 AM, Jan Beulich wrote:
>>
>> The existing comments explain what every byte means.
>> They are useful if CFI-literate reader wants to check correctness
>> of the encoding of this annotation.
>>
>> There is no overall comment what this CFI annotation
>> *achieves*. In human language, what do we say
>> to DWARF decoder here?
>
> Short answer: DW_CFA_def_cfa_expression.
>
> Longer response: Just like I said before, what you're asking for is
> identical to ask for each other CFI annotation to get a comment
> associated to tell you what it's doing, which I don't think you
> really mean to ask for. (Our main problem here is that we can't
> specify expressions with the .cfi_* gas directives, and hence have
> to resort to .cfi_escape.)
>
No, in *human language*. What does the DW_CFA_def_cfa_expression
actually aim to accomplish? If you don't know the innards of the DWARF
spec, the whole thing might as well be Hungarian.
-hpa
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 14:53 ` H. Peter Anvin
@ 2014-08-11 15:08 ` Jan Beulich
2014-08-11 15:13 ` H. Peter Anvin
0 siblings, 1 reply; 41+ messages in thread
From: Jan Beulich @ 2014-08-11 15:08 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Andy Lutomirski, Kees Cook, Will Drewry, Frederic Weisbecker,
Denys Vlasenko, X86 ML, Alexei Starovoitov, Denys Vlasenko,
Oleg Nesterov, Linux Kernel Mailing List
>>> On 11.08.14 at 16:53, <hpa@zytor.com> wrote:
> On 08/11/2014 07:17 AM, Jan Beulich wrote:
>>>
>>> The existing comments explain what every byte means.
>>> They are useful if CFI-literate reader wants to check correctness
>>> of the encoding of this annotation.
>>>
>>> There is no overall comment what this CFI annotation
>>> *achieves*. In human language, what do we say
>>> to DWARF decoder here?
>>
>> Short answer: DW_CFA_def_cfa_expression.
>>
>> Longer response: Just like I said before, what you're asking for is
>> identical to ask for each other CFI annotation to get a comment
>> associated to tell you what it's doing, which I don't think you
>> really mean to ask for. (Our main problem here is that we can't
>> specify expressions with the .cfi_* gas directives, and hence have
>> to resort to .cfi_escape.)
>>
>
> No, in *human language*. What does the DW_CFA_def_cfa_expression
> actually aim to accomplish? If you don't know the innards of the DWARF
> spec, the whole thing might as well be Hungarian.
Just like the other DW_CFA_def_cfa_* ones it sets the current
frame address (CFA), just not via one of the pre-canned shortcuts,
but via an expression (in the case here de-referencing the stack
pointer to read the top of stack, and then adding the necessary
offset). So it indeed is similar enough to other .cfi_* annotations we
use without further comments.
And btw., Hungarian isn't _that_ bad.
Jan
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 15:08 ` Jan Beulich
@ 2014-08-11 15:13 ` H. Peter Anvin
2014-08-12 9:31 ` Denys Vlasenko
0 siblings, 1 reply; 41+ messages in thread
From: H. Peter Anvin @ 2014-08-11 15:13 UTC (permalink / raw)
To: Jan Beulich
Cc: Andy Lutomirski, Kees Cook, Will Drewry, Frederic Weisbecker,
Denys Vlasenko, X86 ML, Alexei Starovoitov, Denys Vlasenko,
Oleg Nesterov, Linux Kernel Mailing List
On 08/11/2014 08:08 AM, Jan Beulich wrote:
>>
>> No, in *human language*. What does the DW_CFA_def_cfa_expression
>> actually aim to accomplish? If you don't know the innards of the DWARF
>> spec, the whole thing might as well be Hungarian.
>
> Just like the other DW_CFA_def_cfa_* ones it sets the current
> frame address (CFA), just not via one of the pre-canned shortcuts,
> but via an expression (in the case here de-referencing the stack
> pointer to read the top of stack, and then adding the necessary
> offset). So it indeed is similar enough to other .cfi_* annotations we
> use without further comments.
>
Actually, what you had inside the parenteses there is actually a
half-decent comment. I'm going to pretend the rest of this wasn't posted.
-hpa
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-11 15:13 ` H. Peter Anvin
@ 2014-08-12 9:31 ` Denys Vlasenko
2014-08-12 9:50 ` Jan Beulich
0 siblings, 1 reply; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-12 9:31 UTC (permalink / raw)
To: H. Peter Anvin, Jan Beulich
Cc: Andy Lutomirski, Kees Cook, Will Drewry, Frederic Weisbecker,
Denys Vlasenko, X86 ML, Alexei Starovoitov, Oleg Nesterov,
Linux Kernel Mailing List
On 08/11/2014 05:13 PM, H. Peter Anvin wrote:
> On 08/11/2014 08:08 AM, Jan Beulich wrote:
>>> No, in *human language*. What does the DW_CFA_def_cfa_expression
>>> actually aim to accomplish? If you don't know the innards of the DWARF
>>> spec, the whole thing might as well be Hungarian.
>>
>> Just like the other DW_CFA_def_cfa_* ones it sets the current
>> frame address (CFA), just not via one of the pre-canned shortcuts,
>> but via an expression (in the case here de-referencing the stack
>> pointer to read the top of stack, and then adding the necessary
>> offset). So it indeed is similar enough to other .cfi_* annotations we
>> use without further comments.
>>
>
> Actually, what you had inside the parenteses there is actually a
> half-decent comment. I'm going to pretend the rest of this wasn't posted.
Jan, Pater, does this look correct _and_ human-understandable?
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -652,10 +652,14 @@ END(interrupt)
cmovzq PER_CPU_VAR(irq_stack_ptr),%rsp
CFI_DEF_CFA_REGISTER rsi
pushq %rsi
+ /*
+ * For debugger:
+ * "CFA (Current Frame Address) is the value on stack + offset"
+ */
CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
- 0x77 /* DW_OP_breg7 */, 0, \
+ 0x77 /* DW_OP_breg7 (rsp) */, 0, \
0x06 /* DW_OP_deref */, \
- 0x08 /* DW_OP_const1u */, SS+8-RBP, \
+ 0x08 /* DW_OP_const1u */, SIZEOF_PTREGS-RBP, \
0x22 /* DW_OP_plus */
/* We entered an interrupt context - irqs are off: */
TRACE_IRQS_OFF
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs"
2014-08-12 9:31 ` Denys Vlasenko
@ 2014-08-12 9:50 ` Jan Beulich
0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2014-08-12 9:50 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Andy Lutomirski, Kees Cook, Will Drewry, Frederic Weisbecker,
Denys Vlasenko, X86 ML, Alexei Starovoitov, Oleg Nesterov,
Linux Kernel Mailing List, H. Peter Anvin
>>> On 12.08.14 at 11:31, <dvlasenk@redhat.com> wrote:
> Jan, Pater, does this look correct _and_ human-understandable?
>
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -652,10 +652,14 @@ END(interrupt)
> cmovzq PER_CPU_VAR(irq_stack_ptr),%rsp
> CFI_DEF_CFA_REGISTER rsi
> pushq %rsi
> + /*
> + * For debugger:
> + * "CFA (Current Frame Address) is the value on stack + offset"
> + */
> CFI_ESCAPE 0x0f /* DW_CFA_def_cfa_expression */, 6, \
> - 0x77 /* DW_OP_breg7 */, 0, \
> + 0x77 /* DW_OP_breg7 (rsp) */, 0, \
I'm fine with the changes above.
> 0x06 /* DW_OP_deref */, \
> - 0x08 /* DW_OP_const1u */, SS+8-RBP, \
> + 0x08 /* DW_OP_const1u */, SIZEOF_PTREGS-RBP, \
But I'd rather not see this one alone changed - either change all
similar uses of SS, or leave the one here alone.
Jan
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 5/5] x86: mass removal of ARGOFFSET
2014-08-01 14:48 [PATCH 1/5] x86: entry_64.S: delete unused code Denys Vlasenko
` (2 preceding siblings ...)
2014-08-01 14:48 ` [PATCH 4/5] x86: entry_64.S: always allocate complete "struct pt_regs" Denys Vlasenko
@ 2014-08-01 14:48 ` Denys Vlasenko
2014-08-01 18:00 ` [PATCH 1/5] x86: entry_64.S: delete unused code Frederic Weisbecker
4 siblings, 0 replies; 41+ messages in thread
From: Denys Vlasenko @ 2014-08-01 14:48 UTC (permalink / raw)
To: linux-kernel
Cc: Denys Vlasenko, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
Frederic Weisbecker, X86 ML, Alexei Starovoitov, Will Drewry,
Kees Cook
ARGOFFSET is zero now, removing it changes no code.
A few macros lost "offset" parameter, since it is always zero now too.
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Oleg Nesterov <oleg@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: X86 ML <x86@kernel.org>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: linux-kernel@vger.kernel.org
---
arch/x86/ia32/ia32entry.S | 136 ++++++++++++++++++++---------------------
arch/x86/include/asm/calling.h | 2 -
arch/x86/kernel/entry_64.S | 62 +++++++++----------
3 files changed, 99 insertions(+), 101 deletions(-)
diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index ef9ee16..4e51c7c 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -41,13 +41,13 @@
movl %edx,%edx /* zero extension */
.endm
- /* clobbers %eax */
- .macro CLEAR_RREGS offset=0, _r9=rax
+ /* clobbers %eax */
+ .macro CLEAR_RREGS _r9=rax
xorl %eax,%eax
- movq %rax,\offset+R11(%rsp)
- movq %rax,\offset+R10(%rsp)
- movq %\_r9,\offset+R9(%rsp)
- movq %rax,\offset+R8(%rsp)
+ movq %rax,R11(%rsp)
+ movq %rax,R10(%rsp)
+ movq %\_r9,R9(%rsp)
+ movq %rax,R8(%rsp)
.endm
/*
@@ -60,14 +60,14 @@
* If it's -1 to make us punt the syscall, then (u32)-1 is still
* an appropriately invalid value.
*/
- .macro LOAD_ARGS32 offset, _r9=0
+ .macro LOAD_ARGS32 _r9=0
.if \_r9
- movl \offset+R9(%rsp),%r9d
+ movl R9(%rsp),%r9d
.endif
- movl \offset+RCX(%rsp),%ecx
- movl \offset+RDX(%rsp),%edx
- movl \offset+RSI(%rsp),%esi
- movl \offset+RDI(%rsp),%edi
+ movl RCX(%rsp),%ecx
+ movl RDX(%rsp),%edx
+ movl RSI(%rsp),%esi
+ movl RDI(%rsp),%edi
movl %eax,%eax /* zero extension */
.endm
@@ -152,8 +152,8 @@ ENTRY(ia32_sysenter_target)
1: movl (%rbp),%ebp
_ASM_EXTABLE(1b,ia32_badarg)
ASM_CLAC
- orl $TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
- testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ orl $TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP)
+ testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP)
CFI_REMEMBER_STATE
jnz sysenter_tracesys
cmpq $(IA32_NR_syscalls-1),%rax
@@ -162,13 +162,13 @@ sysenter_do_call:
IA32_ARG_FIXUP
sysenter_dispatch:
call *ia32_sys_call_table(,%rax,8)
- movq %rax,RAX-ARGOFFSET(%rsp)
+ movq %rax,RAX(%rsp)
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
- testl $_TIF_ALLWORK_MASK,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ testl $_TIF_ALLWORK_MASK,TI_flags+THREAD_INFO(%rsp,RIP)
jnz sysexit_audit
sysexit_from_sys_call:
- andl $~TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ andl $~TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP)
/* clear IF, that popfq doesn't enable interrupts early */
andl $~0x200,EFLAGS-R11(%rsp)
movl RIP-R11(%rsp),%edx /* User %eip */
@@ -195,18 +195,18 @@ sysexit_from_sys_call:
movl %eax,%esi /* 2nd arg: syscall number */
movl $AUDIT_ARCH_I386,%edi /* 1st arg: audit arch */
call __audit_syscall_entry
- movl RAX-ARGOFFSET(%rsp),%eax /* reload syscall number */
+ movl RAX(%rsp),%eax /* reload syscall number */
cmpq $(IA32_NR_syscalls-1),%rax
ja ia32_badsys
movl %ebx,%edi /* reload 1st syscall arg */
- movl RCX-ARGOFFSET(%rsp),%esi /* reload 2nd syscall arg */
- movl RDX-ARGOFFSET(%rsp),%edx /* reload 3rd syscall arg */
- movl RSI-ARGOFFSET(%rsp),%ecx /* reload 4th syscall arg */
- movl RDI-ARGOFFSET(%rsp),%r8d /* reload 5th syscall arg */
+ movl RCX(%rsp),%esi /* reload 2nd syscall arg */
+ movl RDX(%rsp),%edx /* reload 3rd syscall arg */
+ movl RSI(%rsp),%ecx /* reload 4th syscall arg */
+ movl RDI(%rsp),%r8d /* reload 5th syscall arg */
.endm
.macro auditsys_exit exit
- testl $(_TIF_ALLWORK_MASK & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ testl $(_TIF_ALLWORK_MASK & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP)
jnz ia32_ret_from_sys_call
TRACE_IRQS_ON
ENABLE_INTERRUPTS(CLBR_NONE)
@@ -217,13 +217,13 @@ sysexit_from_sys_call:
1: setbe %al /* 1 if error, 0 if not */
movzbl %al,%edi /* zero-extend that into %edi */
call __audit_syscall_exit
- movq RAX-ARGOFFSET(%rsp),%rax /* reload syscall return value */
+ movq RAX(%rsp),%rax /* reload syscall return value */
movl $(_TIF_ALLWORK_MASK & ~_TIF_SYSCALL_AUDIT),%edi
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
- testl %edi,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ testl %edi,TI_flags+THREAD_INFO(%rsp,RIP)
jz \exit
- CLEAR_RREGS -ARGOFFSET
+ CLEAR_RREGS
jmp int_with_check
.endm
@@ -239,7 +239,7 @@ sysexit_audit:
sysenter_tracesys:
#ifdef CONFIG_AUDITSYSCALL
- testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP)
jz sysenter_auditsys
#endif
SAVE_EXTRA_REGS
@@ -247,7 +247,7 @@ sysenter_tracesys:
movq $-ENOSYS,RAX(%rsp)/* ptrace can change this for a bad syscall */
movq %rsp,%rdi /* &pt_regs -> arg1 */
call syscall_trace_enter
- LOAD_ARGS32 ARGOFFSET /* reload args from stack in case ptrace changed it */
+ LOAD_ARGS32 /* reload args from stack in case ptrace changed it */
RESTORE_EXTRA_REGS
cmpq $(IA32_NR_syscalls-1),%rax
ja int_ret_from_sys_call /* sysenter_tracesys has set RAX(%rsp) */
@@ -293,17 +293,17 @@ ENTRY(ia32_cstar_target)
ALLOC_PTREGS_ON_STACK 8
SAVE_C_REGS_EXCEPT_RCX_R891011
movl %eax,%eax /* zero extension */
- movq %rax,ORIG_RAX-ARGOFFSET(%rsp)
- movq %rcx,RIP-ARGOFFSET(%rsp)
- CFI_REL_OFFSET rip,RIP-ARGOFFSET
- movq %rbp,RCX-ARGOFFSET(%rsp) /* this lies slightly to ptrace */
+ movq %rax,ORIG_RAX(%rsp)
+ movq %rcx,RIP(%rsp)
+ CFI_REL_OFFSET rip,RIP
+ movq %rbp,RCX(%rsp) /* this lies slightly to ptrace */
movl %ebp,%ecx
- movq $__USER32_CS,CS-ARGOFFSET(%rsp)
- movq $__USER32_DS,SS-ARGOFFSET(%rsp)
- movq %r11,EFLAGS-ARGOFFSET(%rsp)
- /*CFI_REL_OFFSET rflags,EFLAGS-ARGOFFSET*/
- movq %r8,RSP-ARGOFFSET(%rsp)
- CFI_REL_OFFSET rsp,RSP-ARGOFFSET
+ movq $__USER32_CS,CS(%rsp)
+ movq $__USER32_DS,SS(%rsp)
+ movq %r11,EFLAGS(%rsp)
+ /*CFI_REL_OFFSET rflags,EFLAGS*/
+ movq %r8,RSP(%rsp)
+ CFI_REL_OFFSET rsp,RSP
/* no need to do an access_ok check here because r8 has been
32bit zero extended */
/* hardware stack frame is complete now */
@@ -311,8 +311,8 @@ ENTRY(ia32_cstar_target)
1: movl (%r8),%r9d
_ASM_EXTABLE(1b,ia32_badarg)
ASM_CLAC
- orl $TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
- testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ orl $TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP)
+ testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP)
CFI_REMEMBER_STATE
jnz cstar_tracesys
cmpq $IA32_NR_syscalls-1,%rax
@@ -321,32 +321,32 @@ cstar_do_call:
IA32_ARG_FIXUP 1
cstar_dispatch:
call *ia32_sys_call_table(,%rax,8)
- movq %rax,RAX-ARGOFFSET(%rsp)
+ movq %rax,RAX(%rsp)
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
- testl $_TIF_ALLWORK_MASK,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ testl $_TIF_ALLWORK_MASK,TI_flags+THREAD_INFO(%rsp,RIP)
jnz sysretl_audit
sysretl_from_sys_call:
- andl $~TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ andl $~TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP)
RESTORE_RSI_RDI_RDX
- movl RIP-ARGOFFSET(%rsp),%ecx
+ movl RIP(%rsp),%ecx
CFI_REGISTER rip,rcx
- movl EFLAGS-ARGOFFSET(%rsp),%r11d
+ movl EFLAGS(%rsp),%r11d
/*CFI_REGISTER rflags,r11*/
xorq %r10,%r10
xorq %r9,%r9
xorq %r8,%r8
TRACE_IRQS_ON
- movl RSP-ARGOFFSET(%rsp),%esp
+ movl RSP(%rsp),%esp
CFI_RESTORE rsp
USERGS_SYSRET32
#ifdef CONFIG_AUDITSYSCALL
cstar_auditsys:
CFI_RESTORE_STATE
- movl %r9d,R9-ARGOFFSET(%rsp) /* register to be clobbered by call */
+ movl %r9d,R9(%rsp) /* register to be clobbered by call */
auditsys_entry_common
- movl R9-ARGOFFSET(%rsp),%r9d /* reload 6th syscall arg */
+ movl R9(%rsp),%r9d /* reload 6th syscall arg */
jmp cstar_dispatch
sysretl_audit:
@@ -355,16 +355,16 @@ sysretl_audit:
cstar_tracesys:
#ifdef CONFIG_AUDITSYSCALL
- testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP)
jz cstar_auditsys
#endif
xchgl %r9d,%ebp
SAVE_EXTRA_REGS
- CLEAR_RREGS 0, r9
+ CLEAR_RREGS r9
movq $-ENOSYS,RAX(%rsp) /* ptrace can change this for a bad syscall */
movq %rsp,%rdi /* &pt_regs -> arg1 */
call syscall_trace_enter
- LOAD_ARGS32 ARGOFFSET, 1 /* reload args from stack in case ptrace changed it */
+ LOAD_ARGS32 1 /* reload args from stack in case ptrace changed it */
RESTORE_EXTRA_REGS
xchgl %ebp,%r9d
cmpq $(IA32_NR_syscalls-1),%rax
@@ -422,8 +422,8 @@ ENTRY(ia32_syscall)
this could be a problem. */
ALLOC_PTREGS_ON_STACK
SAVE_C_REGS_EXCEPT_R891011
- orl $TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP-ARGOFFSET)
- testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ orl $TS_COMPAT,TI_status+THREAD_INFO(%rsp,RIP)
+ testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP)
jnz ia32_tracesys
cmpq $(IA32_NR_syscalls-1),%rax
ja ia32_badsys
@@ -431,9 +431,9 @@ ia32_do_call:
IA32_ARG_FIXUP
call *ia32_sys_call_table(,%rax,8) # xxx: rip relative
ia32_sysret:
- movq %rax,RAX-ARGOFFSET(%rsp)
+ movq %rax,RAX(%rsp)
ia32_ret_from_sys_call:
- CLEAR_RREGS -ARGOFFSET
+ CLEAR_RREGS
jmp int_ret_from_sys_call
ia32_tracesys:
@@ -442,7 +442,7 @@ ia32_tracesys:
movq $-ENOSYS,RAX(%rsp) /* ptrace can change this for a bad syscall */
movq %rsp,%rdi /* &pt_regs -> arg1 */
call syscall_trace_enter
- LOAD_ARGS32 ARGOFFSET /* reload args from stack in case ptrace changed it */
+ LOAD_ARGS32 /* reload args from stack in case ptrace changed it */
RESTORE_EXTRA_REGS
cmpq $(IA32_NR_syscalls-1),%rax
ja int_ret_from_sys_call /* ia32_tracesys has set RAX(%rsp) */
@@ -450,7 +450,7 @@ ia32_tracesys:
END(ia32_syscall)
ia32_badsys:
- movq $0,ORIG_RAX-ARGOFFSET(%rsp)
+ movq $0,ORIG_RAX(%rsp)
movq $-ENOSYS,%rax
jmp ia32_sysret
@@ -482,17 +482,17 @@ ia32_ptregs_common:
CFI_ENDPROC
CFI_STARTPROC32 simple
CFI_SIGNAL_FRAME
- CFI_DEF_CFA rsp,SS+8-ARGOFFSET
- CFI_REL_OFFSET rax,RAX-ARGOFFSET
- CFI_REL_OFFSET rcx,RCX-ARGOFFSET
- CFI_REL_OFFSET rdx,RDX-ARGOFFSET
- CFI_REL_OFFSET rsi,RSI-ARGOFFSET
- CFI_REL_OFFSET rdi,RDI-ARGOFFSET
- CFI_REL_OFFSET rip,RIP-ARGOFFSET
-/* CFI_REL_OFFSET cs,CS-ARGOFFSET*/
-/* CFI_REL_OFFSET rflags,EFLAGS-ARGOFFSET*/
- CFI_REL_OFFSET rsp,RSP-ARGOFFSET
-/* CFI_REL_OFFSET ss,SS-ARGOFFSET*/
+ CFI_DEF_CFA rsp,SS+8
+ CFI_REL_OFFSET rax,RAX
+ CFI_REL_OFFSET rcx,RCX
+ CFI_REL_OFFSET rdx,RDX
+ CFI_REL_OFFSET rsi,RSI
+ CFI_REL_OFFSET rdi,RDI
+ CFI_REL_OFFSET rip,RIP
+/* CFI_REL_OFFSET cs,CS*/
+/* CFI_REL_OFFSET rflags,EFLAGS*/
+ CFI_REL_OFFSET rsp,RSP
+/* CFI_REL_OFFSET ss,SS*/
SAVE_EXTRA_REGS 8
call *%rax
RESTORE_EXTRA_REGS 8
diff --git a/arch/x86/include/asm/calling.h b/arch/x86/include/asm/calling.h
index 10aff1e..31d0ac8 100644
--- a/arch/x86/include/asm/calling.h
+++ b/arch/x86/include/asm/calling.h
@@ -88,8 +88,6 @@ For 32-bit we have the following conventions - kernel is built with
#define RSP 19*8
#define SS 20*8
-#define ARGOFFSET 0
-
.macro ALLOC_PTREGS_ON_STACK addskip=0
subq $15*8+\addskip, %rsp
CFI_ADJUST_CFA_OFFSET 15*8+\addskip
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index b3c3ebb..ec68a07 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -76,7 +76,7 @@ ENDPROC(native_usergs_sysret64)
#endif /* CONFIG_PARAVIRT */
-.macro TRACE_IRQS_IRETQ offset=ARGOFFSET
+.macro TRACE_IRQS_IRETQ offset=0
#ifdef CONFIG_TRACE_IRQFLAGS
bt $9,EFLAGS-\offset(%rsp) /* interrupts off? */
jnc 1f
@@ -110,7 +110,7 @@ ENDPROC(native_usergs_sysret64)
call debug_stack_reset
.endm
-.macro TRACE_IRQS_IRETQ_DEBUG offset=ARGOFFSET
+.macro TRACE_IRQS_IRETQ_DEBUG offset=0
bt $9,EFLAGS-\offset(%rsp) /* interrupts off? */
jnc 1f
TRACE_IRQS_ON_DEBUG
@@ -187,16 +187,16 @@ ENDPROC(native_usergs_sysret64)
* frame that enables calling into C.
*/
.macro PARTIAL_FRAME start=1 offset=0
- XCPT_FRAME \start, ORIG_RAX+\offset-ARGOFFSET
- CFI_REL_OFFSET rdi, RDI+\offset-ARGOFFSET
- CFI_REL_OFFSET rsi, RSI+\offset-ARGOFFSET
- CFI_REL_OFFSET rdx, RDX+\offset-ARGOFFSET
- CFI_REL_OFFSET rcx, RCX+\offset-ARGOFFSET
- CFI_REL_OFFSET rax, RAX+\offset-ARGOFFSET
- CFI_REL_OFFSET r8, R8+\offset-ARGOFFSET
- CFI_REL_OFFSET r9, R9+\offset-ARGOFFSET
- CFI_REL_OFFSET r10, R10+\offset-ARGOFFSET
- CFI_REL_OFFSET r11, R11+\offset-ARGOFFSET
+ XCPT_FRAME \start, ORIG_RAX+\offset
+ CFI_REL_OFFSET rdi, RDI+\offset
+ CFI_REL_OFFSET rsi, RSI+\offset
+ CFI_REL_OFFSET rdx, RDX+\offset
+ CFI_REL_OFFSET rcx, RCX+\offset
+ CFI_REL_OFFSET rax, RAX+\offset
+ CFI_REL_OFFSET r8, R8+\offset
+ CFI_REL_OFFSET r9, R9+\offset
+ CFI_REL_OFFSET r10, R10+\offset
+ CFI_REL_OFFSET r11, R11+\offset
.endm
/*
@@ -260,13 +260,13 @@ ENTRY(ret_from_fork)
RESTORE_EXTRA_REGS
- testl $3, CS-ARGOFFSET(%rsp) # from kernel_thread?
+ testl $3, CS(%rsp) # from kernel_thread?
jz 1f
testl $_TIF_IA32, TI_flags(%rcx) # 32-bit compat task needs IRET
jnz int_ret_from_sys_call
- RESTORE_TOP_OF_STACK %rdi, -ARGOFFSET
+ RESTORE_TOP_OF_STACK %rdi
jmp ret_from_sys_call # go to the SYSRET fastpath
1:
@@ -333,10 +333,10 @@ GLOBAL(system_call_after_swapgs)
ENABLE_INTERRUPTS(CLBR_NONE)
ALLOC_PTREGS_ON_STACK 8
SAVE_C_REGS
- movq %rax,ORIG_RAX-ARGOFFSET(%rsp)
- movq %rcx,RIP-ARGOFFSET(%rsp)
- CFI_REL_OFFSET rip,RIP-ARGOFFSET
- testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ movq %rax,ORIG_RAX(%rsp)
+ movq %rcx,RIP(%rsp)
+ CFI_REL_OFFSET rip,RIP
+ testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP)
jnz tracesys
system_call_fastpath:
#if __SYSCALL_MASK == ~0
@@ -348,7 +348,7 @@ system_call_fastpath:
ja badsys
movq %r10,%rcx
call *sys_call_table(,%rax,8) # XXX: rip relative
- movq %rax,RAX-ARGOFFSET(%rsp)
+ movq %rax,RAX(%rsp)
/*
* Syscall return path ending with SYSRET (fast path)
* Has incomplete stack frame and undefined top of stack.
@@ -360,7 +360,7 @@ sysret_check:
LOCKDEP_SYS_EXIT
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
- movl TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET),%edx
+ movl TI_flags+THREAD_INFO(%rsp,RIP),%edx
andl %edi,%edx
jnz sysret_careful
CFI_REMEMBER_STATE
@@ -369,7 +369,7 @@ sysret_check:
*/
TRACE_IRQS_ON
RESTORE_C_REGS_EXCEPT_RCX
- movq RIP-ARGOFFSET(%rsp),%rcx
+ movq RIP(%rsp),%rcx
CFI_REGISTER rip,rcx
/*CFI_REGISTER rflags,r11*/
movq PER_CPU_VAR(old_rsp), %rsp
@@ -401,11 +401,11 @@ sysret_signal:
* These all wind up with the iret return path anyway,
* so just join that path right now.
*/
- FIXUP_TOP_OF_STACK %r11, -ARGOFFSET
+ FIXUP_TOP_OF_STACK %r11
jmp int_check_syscall_exit_work
badsys:
- movq $-ENOSYS,RAX-ARGOFFSET(%rsp)
+ movq $-ENOSYS,RAX(%rsp)
jmp ret_from_sys_call
#ifdef CONFIG_AUDITSYSCALL
@@ -431,7 +431,7 @@ auditsys:
* masked off.
*/
sysret_audit:
- movq RAX-ARGOFFSET(%rsp),%rsi /* second arg, syscall return value */
+ movq RAX(%rsp),%rsi /* second arg, syscall return value */
cmpq $-MAX_ERRNO,%rsi /* is it < -MAX_ERRNO? */
setbe %al /* 1 if so, 0 if not */
movzbl %al,%edi /* zero-extend that into %edi */
@@ -443,7 +443,7 @@ sysret_audit:
/* Do syscall tracing */
tracesys:
#ifdef CONFIG_AUDITSYSCALL
- testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+ testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP)
jz auditsys
#endif
SAVE_EXTRA_REGS
@@ -467,7 +467,7 @@ tracesys:
ja int_ret_from_sys_call /* RAX(%rsp) set to -ENOSYS above */
movq %r10,%rcx /* fixup for C */
call *sys_call_table(,%rax,8)
- movq %rax,RAX-ARGOFFSET(%rsp)
+ movq %rax,RAX(%rsp)
/* Use IRET because user could have changed frame */
/*
@@ -551,9 +551,9 @@ END(stub_\func)
ENTRY(\label)
CFI_STARTPROC
DEFAULT_FRAME 0, 8 /* offset 8: return address */
- FIXUP_TOP_OF_STACK %r11, 8-ARGOFFSET
+ FIXUP_TOP_OF_STACK %r11, 8
call \func
- RESTORE_TOP_OF_STACK %r11, 8-ARGOFFSET
+ RESTORE_TOP_OF_STACK %r11, 8
ret
CFI_ENDPROC
END(\label)
@@ -711,7 +711,7 @@ common_interrupt:
ASM_CLAC
addq $-0x80,(%rsp) /* Adjust vector to [-256,-1] range */
interrupt do_IRQ
- /* 0(%rsp): old_rsp-ARGOFFSET */
+ /* 0(%rsp): old_rsp */
ret_from_intr:
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
@@ -725,7 +725,7 @@ ret_from_intr:
exit_intr:
GET_THREAD_INFO(%rcx)
- testl $3,CS-ARGOFFSET(%rsp)
+ testl $3,CS(%rsp)
je retint_kernel
/* Interrupt came from user space */
@@ -863,7 +863,7 @@ retint_signal:
ENTRY(retint_kernel)
cmpl $0,PER_CPU_VAR(__preempt_count)
jnz retint_restore_args
- bt $9,EFLAGS-ARGOFFSET(%rsp) /* interrupts off? */
+ bt $9,EFLAGS(%rsp) /* interrupts off? */
jnc retint_restore_args
call preempt_schedule_irq
jmp exit_intr
--
1.8.1.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 1/5] x86: entry_64.S: delete unused code
2014-08-01 14:48 [PATCH 1/5] x86: entry_64.S: delete unused code Denys Vlasenko
` (3 preceding siblings ...)
2014-08-01 14:48 ` [PATCH 5/5] x86: mass removal of ARGOFFSET Denys Vlasenko
@ 2014-08-01 18:00 ` Frederic Weisbecker
4 siblings, 0 replies; 41+ messages in thread
From: Frederic Weisbecker @ 2014-08-01 18:00 UTC (permalink / raw)
To: Denys Vlasenko
Cc: linux-kernel, Oleg Nesterov, H. Peter Anvin, Andy Lutomirski,
X86 ML, Alexei Starovoitov, Will Drewry, Kees Cook
On Fri, Aug 01, 2014 at 04:48:14PM +0200, Denys Vlasenko wrote:
> A define, two macros and an unreferenced bit of assembly are gone.
>
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
^ permalink raw reply [flat|nested] 41+ messages in thread