All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.