Re: [PATCH 00/43] x86 entry-stack and Kaiser series, 2017/11/24 version

From: Ingo Molnar <mingo@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: Dave Hansen <dave.hansen@linux.intel.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Borislav Petkov <bp@alien8.de>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 00/43] x86 entry-stack and Kaiser series, 2017/11/24 version
Date: Fri, 24 Nov 2017 14:55:30 +0100	[thread overview]
Message-ID: <20171124135530.gtz5hwqzddof4p4q@gmail.com> (raw)
In-Reply-To: <20171124091448.7649-1-mingo@kernel.org>


* Ingo Molnar <mingo@kernel.org> wrote:

> This is a linear series of patches of the latest entry-stack plus Kaiser
> bits from Andy Lutomirski (v3 series from today) and Dave Hansen
> (kaiser-414-tipwip-20171123 version), on top of latest tip:x86/urgent (12a78d43de76),
> plus fixes - for easier review.
> 
> The code should be the latest posted by Andy and Dave.
> 
> Any bugs caused by mis-merges, mis-backmerges or mis-fixes are mine.

There were some mis-merges in the assembly code, crashing the kernel on bootup 
with Kaiser enabled. Thomas helped find & fix them.

I've pushed out the latest to tip:WIP.x86/mm, the interdiff between the posted and 
the Git version can be found below.

Thanks,

	Ingo

===============>
 arch/x86/entry/entry_64.S        | 12 ++----------
 arch/x86/entry/entry_64_compat.S |  8 --------
 arch/x86/events/intel/ds.c       |  2 +-
 3 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 20be5e89a36a..4ac952080869 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -201,7 +201,6 @@ ENTRY(entry_SYSCALL_64)
 
 	swapgs
 	movq	%rsp, PER_CPU_VAR(rsp_scratch)
-
 	/*
 	 * The kernel CR3 is needed to map the process stack, but we
 	 * need a scratch register to be able to load CR3.  %rsp is
@@ -209,7 +208,6 @@ ENTRY(entry_SYSCALL_64)
 	 * %rsp will be look crazy here for a couple instructions.
 	 */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rsp
-
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
 
 	/* Construct struct pt_regs on stack */
@@ -259,9 +257,6 @@ GLOBAL(entry_SYSCALL_64_after_hwframe)
 	ja	1f				/* return -ENOSYS (already in pt_regs->ax) */
 	movq	%r10, %rcx
 
-	/* Must wait until we have the kernel CR3 to call C functions: */
-	TRACE_IRQS_OFF
-
 	/*
 	 * This call instruction is handled specially in stub_ptregs_64.
 	 * It might end up jumping to the slow path.  If it jumps, RAX
@@ -647,7 +642,6 @@ END(irq_entries_start)
 	testb	$3, CS-ORIG_RAX(%rsp)
 	jz	1f
 	SWAPGS
-	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 	call	switch_to_thread_stack
 1:
 
@@ -956,10 +950,9 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
 ENTRY(switch_to_thread_stack)
 	UNWIND_HINT_FUNC
 
+	pushq	%rdi
 	/* Need to switch before accessing the thread stack. */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
-
-	pushq	%rdi
 	movq	%rsp, %rdi
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
 	UNWIND_HINT sp_offset=16 sp_reg=ORC_REG_DI
@@ -1315,7 +1308,6 @@ ENTRY(error_entry)
 	 * from user mode due to an IRET fault.
 	 */
 	SWAPGS
-
 	/* We have user CR3.  Change to kernel CR3. */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 
@@ -1377,8 +1369,8 @@ ENTRY(error_entry)
 	 * We came from an IRET to user mode, so we have user
 	 * gsbase and CR3.  Switch to kernel gsbase and CR3:
 	 */
-	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 	SWAPGS
+	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 
 	/*
 	 * Pretend that the exception came from user mode: set up pt_regs
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 57cd353c0667..05238b29895e 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -319,14 +319,6 @@ ENTRY(entry_INT80_compat)
 	ASM_CLAC			/* Do this early to minimize exposure */
 	SWAPGS
 
-	/*
-	 * Must switch CR3 before thread stack is used.  %r8 itself
-	 * is not saved into pt_regs and is not preserved across
-	 * function calls (like TRACE_IRQS_OFF calls), thus should
-	 * be safe to use.
-	 */
-	SWITCH_TO_KERNEL_CR3 scratch_reg=%r8
-
 	/*
 	 * User tracing code (ptrace or signal handlers) might assume that
 	 * the saved RAX contains a 32-bit number when we're invoking a 32-bit
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 61388b01962d..b5cf473e443a 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/bitops.h>
 #include <linux/types.h>
+#include <linux/kaiser.h>
 #include <linux/slab.h>
 
-#include <linux/kaiser.h>
 #include <asm/perf_event.h>
 #include <asm/insn.h>
 
