public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] x86/asm/entry/64: Add forgotten CFI annotation
@ 2015-03-27 10:36 Denys Vlasenko
  2015-03-27 10:36 ` [PATCH 2/4] x86/asm/entry/32: Update "interrupt off" comments Denys Vlasenko
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Denys Vlasenko @ 2015-03-27 10:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

This is a missing bit of the recent MOV-to-PUSH conversion.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/entry_64.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index bf9afad..a2ef30a 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -248,6 +248,7 @@ GLOBAL(system_call_after_swapgs)
 	pushq_cfi_reg	r10			/* pt_regs->r10 */
 	pushq_cfi_reg	r11			/* pt_regs->r11 */
 	sub	$(6*8),%rsp /* pt_regs->bp,bx,r12-15 not saved */
+	CFI_ADJUST_CFA_OFFSET 6*8
 
 	testl $_TIF_WORK_SYSCALL_ENTRY, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
 	jnz tracesys
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] x86/asm/entry/32: Update "interrupt off" comments
  2015-03-27 10:36 [PATCH 1/4] x86/asm/entry/64: Add forgotten CFI annotation Denys Vlasenko
@ 2015-03-27 10:36 ` Denys Vlasenko
  2015-03-28  8:39   ` [tip:x86/asm] " tip-bot for Denys Vlasenko
  2015-03-27 10:36 ` [PATCH 3/4] x86/asm/entry/32: Make register zero-extension more prominent Denys Vlasenko
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Denys Vlasenko @ 2015-03-27 10:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

The existing comment has proven to be not very clear.
Replace it with comment similar to one we now have in 64-bit syscall
entry point. (Three instances, one per 32-bit syscall entry).

In int80 entry point's CFI annotations, replace mysterious expressions
with numric constants. In this case, raw numbers look more understandable.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/ia32/ia32entry.S | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 5d2641c..7502ff0 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -112,13 +112,16 @@ ENTRY(ia32_sysenter_target)
 	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,0
 	CFI_REGISTER	rsp,rbp
-	SWAPGS_UNSAFE_STACK
-	movq	PER_CPU_VAR(cpu_tss + TSS_sp0), %rsp
+
 	/*
-	 * No need to follow this irqs on/off section: the syscall
-	 * disabled irqs, here we enable it straight after entry:
+	 * Interrupts are off on entry.
+	 * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
+	 * it is too small to ever cause noticeable irq latency.
 	 */
+	SWAPGS_UNSAFE_STACK
+	movq	PER_CPU_VAR(cpu_tss + TSS_sp0), %rsp
 	ENABLE_INTERRUPTS(CLBR_NONE)
+
 	/* Construct iret frame (ss,rsp,rflags,cs,rip) */
  	movl	%ebp,%ebp		/* zero extension */
 	pushq_cfi $__USER32_DS
@@ -314,15 +317,18 @@ ENTRY(ia32_cstar_target)
 	CFI_DEF_CFA	rsp,0
 	CFI_REGISTER	rip,rcx
 	/*CFI_REGISTER	rflags,r11*/
+
+	/*
+	 * Interrupts are off on entry.
+	 * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
+	 * it is too small to ever cause noticeable irq latency.
+	 */
 	SWAPGS_UNSAFE_STACK
 	movl	%esp,%r8d
 	CFI_REGISTER	rsp,r8
 	movq	PER_CPU_VAR(kernel_stack),%rsp
-	/*
-	 * No need to follow this irqs on/off section: the syscall
-	 * disabled irqs and here we enable it straight after entry:
-	 */
 	ENABLE_INTERRUPTS(CLBR_NONE)
+
 	ALLOC_PT_GPREGS_ON_STACK 6*8 /* 6*8: space for orig_ax and iret frame */
 	SAVE_C_REGS_EXCEPT_RCX_R891011
 	movl 	%eax,%eax	/* zero extension */
@@ -449,19 +455,22 @@ ia32_badarg:
 ENTRY(ia32_syscall)
 	CFI_STARTPROC32	simple
 	CFI_SIGNAL_FRAME
-	CFI_DEF_CFA	rsp,SS+8-RIP
-	/*CFI_REL_OFFSET	ss,SS-RIP*/
-	CFI_REL_OFFSET	rsp,RSP-RIP
-	/*CFI_REL_OFFSET	rflags,EFLAGS-RIP*/
-	/*CFI_REL_OFFSET	cs,CS-RIP*/
-	CFI_REL_OFFSET	rip,RIP-RIP
-	PARAVIRT_ADJUST_EXCEPTION_FRAME
-	SWAPGS
+	CFI_DEF_CFA	rsp,5*8
+	/*CFI_REL_OFFSET	ss,4*8 */
+	CFI_REL_OFFSET	rsp,3*8
+	/*CFI_REL_OFFSET	rflags,2*8 */
+	/*CFI_REL_OFFSET	cs,1*8 */
+	CFI_REL_OFFSET	rip,0*8
+
 	/*
-	 * No need to follow this irqs on/off section: the syscall
-	 * disabled irqs and here we enable it straight after entry:
+	 * Interrupts are off on entry.
+	 * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
+	 * it is too small to ever cause noticeable irq latency.
 	 */
+	PARAVIRT_ADJUST_EXCEPTION_FRAME
+	SWAPGS
 	ENABLE_INTERRUPTS(CLBR_NONE)
+
 	movl %eax,%eax
 	pushq_cfi %rax		/* store orig_ax */
 	cld
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] x86/asm/entry/32: Make register zero-extension more prominent
  2015-03-27 10:36 [PATCH 1/4] x86/asm/entry/64: Add forgotten CFI annotation Denys Vlasenko
  2015-03-27 10:36 ` [PATCH 2/4] x86/asm/entry/32: Update "interrupt off" comments Denys Vlasenko
@ 2015-03-27 10:36 ` Denys Vlasenko
  2015-03-28  8:40   ` [tip:x86/asm] " tip-bot for Denys Vlasenko
  2015-03-27 10:36 ` [PATCH 4/4] x86/asm/entry/32: Use PUSH instructions to build pt_regs on stack Denys Vlasenko
  2015-03-28  8:39 ` [tip:x86/asm] x86/asm/entry/64: Add missing CFI annotation tip-bot for Denys Vlasenko
  3 siblings, 1 reply; 10+ messages in thread
From: Denys Vlasenko @ 2015-03-27 10:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

It was mentioned that people keep trying to optimize them out,
introducing bugs. Make them more visible, and add "do not remove"
comment.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/ia32/ia32entry.S | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 7502ff0..dec8c1d 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -122,8 +122,11 @@ ENTRY(ia32_sysenter_target)
 	movq	PER_CPU_VAR(cpu_tss + TSS_sp0), %rsp
 	ENABLE_INTERRUPTS(CLBR_NONE)
 
+	/* Zero-extending 32-bit regs, do not remove */
+	movl	%ebp, %ebp
+	movl	%eax, %eax
+
 	/* Construct iret frame (ss,rsp,rflags,cs,rip) */
- 	movl	%ebp,%ebp		/* zero extension */
 	pushq_cfi $__USER32_DS
 	/*CFI_REL_OFFSET ss,0*/
 	pushq_cfi %rbp
@@ -134,7 +137,6 @@ ENTRY(ia32_sysenter_target)
 	CFI_REGISTER rip,r10
 	pushq_cfi $__USER32_CS
 	/*CFI_REL_OFFSET cs,0*/
-	movl	%eax, %eax
 	/* Store thread_info->sysenter_return in rip stack slot */
 	pushq_cfi %r10
 	CFI_REL_OFFSET rip,0
@@ -329,9 +331,11 @@ ENTRY(ia32_cstar_target)
 	movq	PER_CPU_VAR(kernel_stack),%rsp
 	ENABLE_INTERRUPTS(CLBR_NONE)
 
+	/* Zero-extending 32-bit regs, do not remove */
+	movl	%eax,%eax
+
 	ALLOC_PT_GPREGS_ON_STACK 6*8 /* 6*8: space for orig_ax and iret frame */
 	SAVE_C_REGS_EXCEPT_RCX_R891011
-	movl 	%eax,%eax	/* zero extension */
 	movq	%rax,ORIG_RAX(%rsp)
 	movq	%rcx,RIP(%rsp)
 	CFI_REL_OFFSET rip,RIP
@@ -471,7 +475,9 @@ ENTRY(ia32_syscall)
 	SWAPGS
 	ENABLE_INTERRUPTS(CLBR_NONE)
 
-	movl %eax,%eax
+	/* Zero-extending 32-bit regs, do not remove */
+	movl	%eax,%eax
+
 	pushq_cfi %rax		/* store orig_ax */
 	cld
 	/* note the registers are not zero extended to the sf.
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] x86/asm/entry/32: Use PUSH instructions to build pt_regs on stack
  2015-03-27 10:36 [PATCH 1/4] x86/asm/entry/64: Add forgotten CFI annotation Denys Vlasenko
  2015-03-27 10:36 ` [PATCH 2/4] x86/asm/entry/32: Update "interrupt off" comments Denys Vlasenko
  2015-03-27 10:36 ` [PATCH 3/4] x86/asm/entry/32: Make register zero-extension more prominent Denys Vlasenko
@ 2015-03-27 10:36 ` Denys Vlasenko
  2015-03-27 11:32   ` Ingo Molnar
  2015-03-28  8:39 ` [tip:x86/asm] x86/asm/entry/64: Add missing CFI annotation tip-bot for Denys Vlasenko
  3 siblings, 1 reply; 10+ messages in thread
From: Denys Vlasenko @ 2015-03-27 10:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Denys Vlasenko, Linus Torvalds, Steven Rostedt, Borislav Petkov,
	H. Peter Anvin, Andy Lutomirski, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	x86, linux-kernel

This mimics the recent similar 64-bit change.
Saves ~110 bytes of code.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---

Patch was run-tested on 32 and 64 bits, Intel and AMD CPU.

 arch/x86/ia32/ia32entry.S | 82 ++++++++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 36 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index dec8c1d..8d01cce 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -126,26 +126,27 @@ ENTRY(ia32_sysenter_target)
 	movl	%ebp, %ebp
 	movl	%eax, %eax
 
-	/* Construct iret frame (ss,rsp,rflags,cs,rip) */
-	pushq_cfi $__USER32_DS
-	/*CFI_REL_OFFSET ss,0*/
-	pushq_cfi %rbp
-	CFI_REL_OFFSET rsp,0
-	pushfq_cfi
-	/*CFI_REL_OFFSET rflags,0*/
-	movl	ASM_THREAD_INFO(TI_sysenter_return, %rsp, 3*8), %r10d
+	movl	ASM_THREAD_INFO(TI_sysenter_return, %rsp, 0), %r10d
 	CFI_REGISTER rip,r10
-	pushq_cfi $__USER32_CS
-	/*CFI_REL_OFFSET cs,0*/
-	/* Store thread_info->sysenter_return in rip stack slot */
-	pushq_cfi %r10
-	CFI_REL_OFFSET rip,0
-	/* Store orig_ax */
-	pushq_cfi %rax
-	/* Construct the rest of "struct pt_regs" */
+
+	/* Construct struct pt_regs on stack */
+	pushq_cfi	$__USER32_DS		/* pt_regs->ss */
+	pushq_cfi	%rbp			/* pt_regs->sp */
+	CFI_REL_OFFSET	rsp,0
+	pushfq_cfi				/* pt_regs->flags */
+	pushq_cfi	$__USER32_CS		/* pt_regs->cs */
+	pushq_cfi	%r10 /* pt_regs->ip = thread_info->sysenter_return */
+	CFI_REL_OFFSET	rip,0
+	pushq_cfi_reg	rax			/* pt_regs->orig_ax */
+	pushq_cfi_reg	rdi			/* pt_regs->di */
+	pushq_cfi_reg	rsi			/* pt_regs->si */
+	pushq_cfi_reg	rdx			/* pt_regs->dx */
+	pushq_cfi_reg	rcx			/* pt_regs->cx */
+	pushq_cfi_reg	rax			/* pt_regs->ax */
 	cld
-	ALLOC_PT_GPREGS_ON_STACK
-	SAVE_C_REGS_EXCEPT_R891011
+	sub	$(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
+	CFI_ADJUST_CFA_OFFSET 10*8
+
 	/*
 	 * no need to do an access_ok check here because rbp has been
 	 * 32bit zero extended
@@ -334,20 +335,24 @@ ENTRY(ia32_cstar_target)
 	/* Zero-extending 32-bit regs, do not remove */
 	movl	%eax,%eax
 
-	ALLOC_PT_GPREGS_ON_STACK 6*8 /* 6*8: space for orig_ax and iret frame */
-	SAVE_C_REGS_EXCEPT_RCX_R891011
-	movq	%rax,ORIG_RAX(%rsp)
-	movq	%rcx,RIP(%rsp)
-	CFI_REL_OFFSET rip,RIP
-	movq	%rbp,RCX(%rsp) /* this lies slightly to ptrace */
+	/* Construct struct pt_regs on stack */
+	pushq_cfi	$__USER32_DS		/* pt_regs->ss */
+	pushq_cfi	%r8			/* pt_regs->sp */
+	CFI_REL_OFFSET rsp,0
+	pushq_cfi	%r11			/* pt_regs->flags */
+	pushq_cfi	$__USER32_CS		/* pt_regs->cs */
+	pushq_cfi	%rcx			/* pt_regs->ip */
+	CFI_REL_OFFSET rip,0
+	pushq_cfi_reg	rax			/* pt_regs->orig_ax */
+	pushq_cfi_reg	rdi			/* pt_regs->di */
+	pushq_cfi_reg	rsi			/* pt_regs->si */
+	pushq_cfi_reg	rdx			/* pt_regs->dx */
+	pushq_cfi_reg	rbp			/* pt_regs->cx */
 	movl	%ebp,%ecx
-	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
-	/* iret stack frame is complete now */
+	pushq_cfi_reg	rax			/* pt_regs->ax */
+	sub	$(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
+	CFI_ADJUST_CFA_OFFSET 10*8
+
 	/*
 	 * no need to do an access_ok check here because r8 has been
 	 * 32bit zero extended
@@ -478,12 +483,17 @@ ENTRY(ia32_syscall)
 	/* Zero-extending 32-bit regs, do not remove */
 	movl	%eax,%eax
 
-	pushq_cfi %rax		/* store orig_ax */
+	/* Construct struct pt_regs on stack (iret frame is already on stack) */
+	pushq_cfi_reg	rax			/* pt_regs->orig_ax */
+	pushq_cfi_reg	rdi			/* pt_regs->di */
+	pushq_cfi_reg	rsi			/* pt_regs->si */
+	pushq_cfi_reg	rdx			/* pt_regs->dx */
+	pushq_cfi_reg	rcx			/* pt_regs->cx */
+	pushq_cfi_reg	rax			/* pt_regs->ax */
 	cld
-	/* note the registers are not zero extended to the sf.
-	   this could be a problem. */
-	ALLOC_PT_GPREGS_ON_STACK
-	SAVE_C_REGS_EXCEPT_R891011
+	sub	$(10*8),%rsp /* pt_regs->r8-11,bp,bx,r12-15 not saved */
+	CFI_ADJUST_CFA_OFFSET 10*8
+
 	orl $TS_COMPAT, ASM_THREAD_INFO(TI_status, %rsp, SIZEOF_PTREGS)
 	testl $_TIF_WORK_SYSCALL_ENTRY, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
 	jnz ia32_tracesys
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] x86/asm/entry/32: Use PUSH instructions to build pt_regs on stack
  2015-03-27 10:36 ` [PATCH 4/4] x86/asm/entry/32: Use PUSH instructions to build pt_regs on stack Denys Vlasenko
@ 2015-03-27 11:32   ` Ingo Molnar
  2015-03-27 12:09     ` Denys Vlasenko
  0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2015-03-27 11:32 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Linus Torvalds, Steven Rostedt, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel


* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> This mimics the recent similar 64-bit change.
> Saves ~110 bytes of code.

Would be nice to know whether you tested all the affected system entry 
variants, on both AMD and Intel CPUs?

that would be:

> @@ -126,26 +126,27 @@ ENTRY(ia32_sysenter_target)
> @@ -334,20 +335,24 @@ ENTRY(ia32_cstar_target)
> @@ -478,12 +483,17 @@ ENTRY(ia32_syscall)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] x86/asm/entry/32: Use PUSH instructions to build pt_regs on stack
  2015-03-27 11:32   ` Ingo Molnar
@ 2015-03-27 12:09     ` Denys Vlasenko
  2015-03-27 12:17       ` Ingo Molnar
  0 siblings, 1 reply; 10+ messages in thread
From: Denys Vlasenko @ 2015-03-27 12:09 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, Steven Rostedt, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel

On 03/27/2015 12:32 PM, Ingo Molnar wrote:
> 
> * Denys Vlasenko <dvlasenk@redhat.com> wrote:
> 
>> This mimics the recent similar 64-bit change.
>> Saves ~110 bytes of code.
> 
> Would be nice to know whether you tested all the affected system entry 
> variants, on both AMD and Intel CPUs?

Yes, I did. I also looked at the diff of entry_64.o disassembly, to have
a different view of the changes.

I know how fragile this stuff is.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] x86/asm/entry/32: Use PUSH instructions to build pt_regs on stack
  2015-03-27 12:09     ` Denys Vlasenko
@ 2015-03-27 12:17       ` Ingo Molnar
  0 siblings, 0 replies; 10+ messages in thread
From: Ingo Molnar @ 2015-03-27 12:17 UTC (permalink / raw)
  To: Denys Vlasenko
  Cc: Linus Torvalds, Steven Rostedt, Borislav Petkov, H. Peter Anvin,
	Andy Lutomirski, Oleg Nesterov, Frederic Weisbecker,
	Alexei Starovoitov, Will Drewry, Kees Cook, x86, linux-kernel


* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> On 03/27/2015 12:32 PM, Ingo Molnar wrote:
> > 
> > * Denys Vlasenko <dvlasenk@redhat.com> wrote:
> > 
> >> This mimics the recent similar 64-bit change.
> >> Saves ~110 bytes of code.
> > 
> > Would be nice to know whether you tested all the affected system entry 
> > variants, on both AMD and Intel CPUs?
> 
> Yes, I did. I also looked at the diff of entry_64.o disassembly, to have
> a different view of the changes.

That kind of information needs to be in the changelog.

> I know how fragile this stuff is.

yeah ...

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip:x86/asm] x86/asm/entry/64: Add missing CFI annotation
  2015-03-27 10:36 [PATCH 1/4] x86/asm/entry/64: Add forgotten CFI annotation Denys Vlasenko
                   ` (2 preceding siblings ...)
  2015-03-27 10:36 ` [PATCH 4/4] x86/asm/entry/32: Use PUSH instructions to build pt_regs on stack Denys Vlasenko
@ 2015-03-28  8:39 ` tip-bot for Denys Vlasenko
  3 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-28  8:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: oleg, ast, keescook, luto, dvlasenk, rostedt, hpa, bp, torvalds,
	mingo, fweisbec, linux-kernel, tglx, wad

Commit-ID:  27be87c5d53117f048d590d6fc6febb21176c3e9
Gitweb:     http://git.kernel.org/tip/27be87c5d53117f048d590d6fc6febb21176c3e9
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Fri, 27 Mar 2015 11:36:19 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 27 Mar 2015 12:27:57 +0100

x86/asm/entry/64: Add missing CFI annotation

This is a missing bit of the recent MOV-to-PUSH conversion.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1427452582-21624-1-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/entry_64.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index f85d2cc..dbfc8875 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -248,6 +248,7 @@ GLOBAL(system_call_after_swapgs)
 	pushq_cfi_reg	r10			/* pt_regs->r10 */
 	pushq_cfi_reg	r11			/* pt_regs->r11 */
 	sub	$(6*8),%rsp /* pt_regs->bp,bx,r12-15 not saved */
+	CFI_ADJUST_CFA_OFFSET 6*8
 
 	testl $_TIF_WORK_SYSCALL_ENTRY, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
 	jnz tracesys

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:x86/asm] x86/asm/entry/32: Update "interrupt off" comments
  2015-03-27 10:36 ` [PATCH 2/4] x86/asm/entry/32: Update "interrupt off" comments Denys Vlasenko
@ 2015-03-28  8:39   ` tip-bot for Denys Vlasenko
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-28  8:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, torvalds, rostedt, bp, fweisbec, ast, hpa, linux-kernel,
	luto, keescook, mingo, dvlasenk, oleg, wad

Commit-ID:  a232e3d558eef421fbb539ede5483dfb668e38f2
Gitweb:     http://git.kernel.org/tip/a232e3d558eef421fbb539ede5483dfb668e38f2
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Fri, 27 Mar 2015 11:36:20 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 27 Mar 2015 12:27:57 +0100

x86/asm/entry/32: Update "interrupt off" comments

The existing comment has proven to be not very clear.

Replace it with a comment similar to the one we now have in the 64-bit
syscall entry point. (Three instances, one per 32-bit syscall entry).

In the INT80 entry point's CFI annotations, replace mysterious
expressions with numric constants. In this case, raw numbers
look more understandable.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1427452582-21624-2-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/ia32/ia32entry.S | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 5d2641c..7502ff0 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -112,13 +112,16 @@ ENTRY(ia32_sysenter_target)
 	CFI_SIGNAL_FRAME
 	CFI_DEF_CFA	rsp,0
 	CFI_REGISTER	rsp,rbp
-	SWAPGS_UNSAFE_STACK
-	movq	PER_CPU_VAR(cpu_tss + TSS_sp0), %rsp
+
 	/*
-	 * No need to follow this irqs on/off section: the syscall
-	 * disabled irqs, here we enable it straight after entry:
+	 * Interrupts are off on entry.
+	 * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
+	 * it is too small to ever cause noticeable irq latency.
 	 */
+	SWAPGS_UNSAFE_STACK
+	movq	PER_CPU_VAR(cpu_tss + TSS_sp0), %rsp
 	ENABLE_INTERRUPTS(CLBR_NONE)
+
 	/* Construct iret frame (ss,rsp,rflags,cs,rip) */
  	movl	%ebp,%ebp		/* zero extension */
 	pushq_cfi $__USER32_DS
@@ -314,15 +317,18 @@ ENTRY(ia32_cstar_target)
 	CFI_DEF_CFA	rsp,0
 	CFI_REGISTER	rip,rcx
 	/*CFI_REGISTER	rflags,r11*/
+
+	/*
+	 * Interrupts are off on entry.
+	 * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
+	 * it is too small to ever cause noticeable irq latency.
+	 */
 	SWAPGS_UNSAFE_STACK
 	movl	%esp,%r8d
 	CFI_REGISTER	rsp,r8
 	movq	PER_CPU_VAR(kernel_stack),%rsp
-	/*
-	 * No need to follow this irqs on/off section: the syscall
-	 * disabled irqs and here we enable it straight after entry:
-	 */
 	ENABLE_INTERRUPTS(CLBR_NONE)
+
 	ALLOC_PT_GPREGS_ON_STACK 6*8 /* 6*8: space for orig_ax and iret frame */
 	SAVE_C_REGS_EXCEPT_RCX_R891011
 	movl 	%eax,%eax	/* zero extension */
@@ -449,19 +455,22 @@ ia32_badarg:
 ENTRY(ia32_syscall)
 	CFI_STARTPROC32	simple
 	CFI_SIGNAL_FRAME
-	CFI_DEF_CFA	rsp,SS+8-RIP
-	/*CFI_REL_OFFSET	ss,SS-RIP*/
-	CFI_REL_OFFSET	rsp,RSP-RIP
-	/*CFI_REL_OFFSET	rflags,EFLAGS-RIP*/
-	/*CFI_REL_OFFSET	cs,CS-RIP*/
-	CFI_REL_OFFSET	rip,RIP-RIP
-	PARAVIRT_ADJUST_EXCEPTION_FRAME
-	SWAPGS
+	CFI_DEF_CFA	rsp,5*8
+	/*CFI_REL_OFFSET	ss,4*8 */
+	CFI_REL_OFFSET	rsp,3*8
+	/*CFI_REL_OFFSET	rflags,2*8 */
+	/*CFI_REL_OFFSET	cs,1*8 */
+	CFI_REL_OFFSET	rip,0*8
+
 	/*
-	 * No need to follow this irqs on/off section: the syscall
-	 * disabled irqs and here we enable it straight after entry:
+	 * Interrupts are off on entry.
+	 * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
+	 * it is too small to ever cause noticeable irq latency.
 	 */
+	PARAVIRT_ADJUST_EXCEPTION_FRAME
+	SWAPGS
 	ENABLE_INTERRUPTS(CLBR_NONE)
+
 	movl %eax,%eax
 	pushq_cfi %rax		/* store orig_ax */
 	cld

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:x86/asm] x86/asm/entry/32: Make register zero-extension more prominent
  2015-03-27 10:36 ` [PATCH 3/4] x86/asm/entry/32: Make register zero-extension more prominent Denys Vlasenko
@ 2015-03-28  8:40   ` tip-bot for Denys Vlasenko
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Denys Vlasenko @ 2015-03-28  8:40 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: rostedt, fweisbec, bp, dvlasenk, linux-kernel, luto, oleg,
	torvalds, mingo, ast, wad, hpa, tglx, keescook

Commit-ID:  4ee8ec17ba00fce4af042543771f996fb9d98d34
Gitweb:     http://git.kernel.org/tip/4ee8ec17ba00fce4af042543771f996fb9d98d34
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Fri, 27 Mar 2015 11:36:21 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 27 Mar 2015 12:27:57 +0100

x86/asm/entry/32: Make register zero-extension more prominent

There are a couple of syscall argument zero-extension instructions in
the 32-bit compat entry code, and it was mentioned that people keep
trying to optimize them out, introducing bugs.

Make them more visible, and add a "do not remove" comment.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1427452582-21624-3-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/ia32/ia32entry.S | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 7502ff0..dec8c1d 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -122,8 +122,11 @@ ENTRY(ia32_sysenter_target)
 	movq	PER_CPU_VAR(cpu_tss + TSS_sp0), %rsp
 	ENABLE_INTERRUPTS(CLBR_NONE)
 
+	/* Zero-extending 32-bit regs, do not remove */
+	movl	%ebp, %ebp
+	movl	%eax, %eax
+
 	/* Construct iret frame (ss,rsp,rflags,cs,rip) */
- 	movl	%ebp,%ebp		/* zero extension */
 	pushq_cfi $__USER32_DS
 	/*CFI_REL_OFFSET ss,0*/
 	pushq_cfi %rbp
@@ -134,7 +137,6 @@ ENTRY(ia32_sysenter_target)
 	CFI_REGISTER rip,r10
 	pushq_cfi $__USER32_CS
 	/*CFI_REL_OFFSET cs,0*/
-	movl	%eax, %eax
 	/* Store thread_info->sysenter_return in rip stack slot */
 	pushq_cfi %r10
 	CFI_REL_OFFSET rip,0
@@ -329,9 +331,11 @@ ENTRY(ia32_cstar_target)
 	movq	PER_CPU_VAR(kernel_stack),%rsp
 	ENABLE_INTERRUPTS(CLBR_NONE)
 
+	/* Zero-extending 32-bit regs, do not remove */
+	movl	%eax,%eax
+
 	ALLOC_PT_GPREGS_ON_STACK 6*8 /* 6*8: space for orig_ax and iret frame */
 	SAVE_C_REGS_EXCEPT_RCX_R891011
-	movl 	%eax,%eax	/* zero extension */
 	movq	%rax,ORIG_RAX(%rsp)
 	movq	%rcx,RIP(%rsp)
 	CFI_REL_OFFSET rip,RIP
@@ -471,7 +475,9 @@ ENTRY(ia32_syscall)
 	SWAPGS
 	ENABLE_INTERRUPTS(CLBR_NONE)
 
-	movl %eax,%eax
+	/* Zero-extending 32-bit regs, do not remove */
+	movl	%eax,%eax
+
 	pushq_cfi %rax		/* store orig_ax */
 	cld
 	/* note the registers are not zero extended to the sf.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-03-28  8:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-27 10:36 [PATCH 1/4] x86/asm/entry/64: Add forgotten CFI annotation Denys Vlasenko
2015-03-27 10:36 ` [PATCH 2/4] x86/asm/entry/32: Update "interrupt off" comments Denys Vlasenko
2015-03-28  8:39   ` [tip:x86/asm] " tip-bot for Denys Vlasenko
2015-03-27 10:36 ` [PATCH 3/4] x86/asm/entry/32: Make register zero-extension more prominent Denys Vlasenko
2015-03-28  8:40   ` [tip:x86/asm] " tip-bot for Denys Vlasenko
2015-03-27 10:36 ` [PATCH 4/4] x86/asm/entry/32: Use PUSH instructions to build pt_regs on stack Denys Vlasenko
2015-03-27 11:32   ` Ingo Molnar
2015-03-27 12:09     ` Denys Vlasenko
2015-03-27 12:17       ` Ingo Molnar
2015-03-28  8:39 ` [tip:x86/asm] x86/asm/entry/64: Add missing CFI annotation tip-bot for Denys Vlasenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox