All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V7 0/6] x86/entry: Clean up entry code
@ 2022-05-03  3:21 Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 1/6] x86/traps: Move pt_regs only in fixup_bad_iret() Lai Jiangshan
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Lai Jiangshan @ 2022-05-03  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, Juergen Gross, x86, Lai Jiangshan

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

This patchset moves the stack-switch code to the place where
error_entry() return, unravels error_entry() from XENpv and makes
entry_INT80_compat() uses idtentry macro.

This patchset is highly related to XENpv, because it does the extra
cleanup to convert SWAPGS to swapgs after major cleanup is done.

The patches are the 7th version to pick patches from the patchset
https://lore.kernel.org/lkml/20211126101209.8613-1-jiangshanlai@gmail.com/
which converts ASM code to C code.  These patches are prepared for that
purpose.  But this patchset has it own value: it simplifies the stack
switch, avoids leaving the old stack inside a function call, and
separates XENpv code with native code without adding new code.

Peter said in V3:
>	So AFAICT these patches are indeed correct.
>
>	I'd love for some of the other x86 people to also look at this,
>	but a tentative ACK on this.

[V6]: https://lore.kernel.org/lkml/20220421141055.316239-1-jiangshanlai@gmail.com/
[V5]: https://lore.kernel.org/lkml/20220412121541.4595-1-jiangshanlai@gmail.com/
[V4]: https://lore.kernel.org/lkml/20220318143016.124387-1-jiangshanlai@gmail.com/
[V3]: https://lore.kernel.org/lkml/20220315073949.7541-1-jiangshanlai@gmail.com/
[V2]: https://lore.kernel.org/lkml/20220303035434.20471-1-jiangshanlai@gmail.com/
[V1]: https://lore.kernel.org/lkml/20211208110833.65366-1-jiangshanlai@gmail.com/


Changed from V6:
	(no code changed)

	Update the changelog of patch3 and patch6

	Drop patch7 and patch8 of the V6.  The patch7/8 are nice cleanup
	patches but they are not required to convert error_entry() to C.
	The changelog of patch8 is also updated locally to avoid ambiguity
	that tglx questioned, and it will be sent separately after PeterZ's
	change to entry_INT80_compat() merged to avoid confliction.

Changed from V5:
	Add a new ASM function to wrap PUSH_AND_CLEAR_REGS rather than
	inline it in the marco idtentry to reduce text size.

	Remove the branch in sync_regs() (pick it from V1 and update
	the changelog a little)

Changed from V4:
	Update changelog largely of patch 1 and patch 2
	Update changelog slightly of other patches
	Unbreak the line of fixup_bad_iret() in patch1

	Add Reviewed-by from Juergen Gross <jgross@suse.com> in patch 6
	since he gave the Reviewed-by in one of the squashed patches.

Changed from V3:
	Only reorder the int80 thing as the last patch to make patches
	ordering more natural. (Both orders are correct)

Other interactions in V3:
	Peter raised several questions and I think I answered them and I
	don't think the code need to be updated unless I missed some
	points. (Except reordering the patches)

	Josh asked to remove UNWIND_HINT_REGS in patch5, but I think
	UNWIND_HINT_REGS is old code before this patchset and I don't
	want to do a cleanup that is not relate to preparing converting
	ASM code C code in this patchset.  He also asked to remove
	ENCODE_FRAME_POINTER in xenpv case, and I think it just
	complicates the code for just optimizing out a single assignment
	to %rbp.  I would not always stick to these reasons of mine,
	but I just keep the code unchanged since he hasn't emphasized it
	again nor other people has requested it.

Changed from V2:
	Make the patch of folding int80 thing as the first patch
	Add more changelog in "Switch the stack after error_entry() returns"

Changed from V1
	Squash cleanup patches converting SWAPGS to swapgs into one patch

	Use my official email address (Ant Group).  The work is backed
	by my company and I was incorrectly misunderstood that
	XXX@linux.alibaba.com is the only portal for opensource work
	in the corporate group.


Lai Jiangshan (6):
  x86/traps: Move pt_regs only in fixup_bad_iret()
  x86/entry: Switch the stack after error_entry() returns
  x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry()
  x86/entry: Move cld to the start of idtentry macro
  x86/entry: Don't call error_entry() for XENPV
  x86/entry: Convert SWAPGS to swapgs and remove the definition of
    SWAPGS

 arch/x86/entry/entry_64.S        | 53 ++++++++++++++++++++++----------
 arch/x86/entry/entry_64_compat.S |  2 +-
 arch/x86/include/asm/irqflags.h  |  8 -----
 arch/x86/include/asm/traps.h     |  2 +-
 arch/x86/kernel/traps.c          | 18 ++++-------
 5 files changed, 44 insertions(+), 39 deletions(-)

-- 
2.19.1.6.gb485710b


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

* [PATCH V7 1/6] x86/traps: Move pt_regs only in fixup_bad_iret()
  2022-05-03  3:21 [PATCH V7 0/6] x86/entry: Clean up entry code Lai Jiangshan
@ 2022-05-03  3:21 ` Lai Jiangshan
  2022-05-03 19:01   ` [tip: x86/asm] x86/traps: Use pt_regs directly " tip-bot2 for Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 2/6] x86/entry: Switch the stack after error_entry() returns Lai Jiangshan
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Lai Jiangshan @ 2022-05-03  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, Juergen Gross, x86, Lai Jiangshan, Ingo Molnar,
	Dave Hansen, H. Peter Anvin, Kirill A. Shutemov, Fenghua Yu,
	Chang S. Bae

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

Always stash the address error_entry() is going to return to, in %r12
and get rid of the void *error_entry_ret; slot in struct bad_iret_stack
which was supposed to account for it and pt_regs pushed on the stack.

After this, both fixup_bad_iret() and sync_regs() can work on a
struct pt_regs pointer directly.

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/entry/entry_64.S    |  5 ++++-
 arch/x86/include/asm/traps.h |  2 +-
 arch/x86/kernel/traps.c      | 18 ++++++------------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 73d958522b6a..ecbfca3cc18c 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1061,9 +1061,12 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * Pretend that the exception came from user mode: set up pt_regs
 	 * as if we faulted immediately after IRET.
 	 */
-	mov	%rsp, %rdi
+	popq	%r12				/* save return addr in %12 */
+	movq	%rsp, %rdi			/* arg0 = pt_regs pointer */
 	call	fixup_bad_iret
 	mov	%rax, %rsp
+	ENCODE_FRAME_POINTER
+	pushq	%r12
 	jmp	.Lerror_entry_from_usermode_after_swapgs
 SYM_CODE_END(error_entry)
 
diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 35317c5c551d..47ecfff2c83d 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -13,7 +13,7 @@
 #ifdef CONFIG_X86_64
 asmlinkage __visible notrace struct pt_regs *sync_regs(struct pt_regs *eregs);
 asmlinkage __visible notrace
-struct bad_iret_stack *fixup_bad_iret(struct bad_iret_stack *s);
+struct pt_regs *fixup_bad_iret(struct pt_regs *bad_regs);
 void __init trap_init(void);
 asmlinkage __visible noinstr struct pt_regs *vc_switch_off_ist(struct pt_regs *eregs);
 #endif
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index a4e2efde5d1f..111b18d57a54 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -898,13 +898,7 @@ asmlinkage __visible noinstr struct pt_regs *vc_switch_off_ist(struct pt_regs *r
 }
 #endif
 
-struct bad_iret_stack {
-	void *error_entry_ret;
-	struct pt_regs regs;
-};
-
-asmlinkage __visible noinstr
-struct bad_iret_stack *fixup_bad_iret(struct bad_iret_stack *s)
+asmlinkage __visible noinstr struct pt_regs *fixup_bad_iret(struct pt_regs *bad_regs)
 {
 	/*
 	 * This is called from entry_64.S early in handling a fault
@@ -914,19 +908,19 @@ struct bad_iret_stack *fixup_bad_iret(struct bad_iret_stack *s)
 	 * just below the IRET frame) and we want to pretend that the
 	 * exception came from the IRET target.
 	 */
-	struct bad_iret_stack tmp, *new_stack =
-		(struct bad_iret_stack *)__this_cpu_read(cpu_tss_rw.x86_tss.sp0) - 1;
+	struct pt_regs tmp, *new_stack =
+		(struct pt_regs *)__this_cpu_read(cpu_tss_rw.x86_tss.sp0) - 1;
 
 	/* Copy the IRET target to the temporary storage. */
-	__memcpy(&tmp.regs.ip, (void *)s->regs.sp, 5*8);
+	__memcpy(&tmp.ip, (void *)bad_regs->sp, 5*8);
 
 	/* Copy the remainder of the stack from the current stack. */
-	__memcpy(&tmp, s, offsetof(struct bad_iret_stack, regs.ip));
+	__memcpy(&tmp, bad_regs, offsetof(struct pt_regs, ip));
 
 	/* Update the entry stack */
 	__memcpy(new_stack, &tmp, sizeof(tmp));
 
-	BUG_ON(!user_mode(&new_stack->regs));
+	BUG_ON(!user_mode(new_stack));
 	return new_stack;
 }
 #endif
-- 
2.19.1.6.gb485710b


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

* [PATCH V7 2/6] x86/entry: Switch the stack after error_entry() returns
  2022-05-03  3:21 [PATCH V7 0/6] x86/entry: Clean up entry code Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 1/6] x86/traps: Move pt_regs only in fixup_bad_iret() Lai Jiangshan
@ 2022-05-03  3:21 ` Lai Jiangshan
  2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 3/6] x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry() Lai Jiangshan
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Lai Jiangshan @ 2022-05-03  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, Juergen Gross, x86, Lai Jiangshan, Ingo Molnar,
	Dave Hansen, H. Peter Anvin

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

error_entry() calls sync_regs(), and fixup_bad_iret() before sync_regs()
if it is a fault from bad IRET, to copy the pt_regs to the kernel stack
and switches the kernel stack directly after sync_regs().

But error_entry() itself is also a function call, so the code has to
stash the address error_entry() is going to return to, in %r12 and
makes the work complicated.

Move the code of switching stack after error_entry() and get rid of the
need to handle the return address.

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/entry/entry_64.S | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ecbfca3cc18c..ca3e99e08a44 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -326,6 +326,8 @@ SYM_CODE_END(ret_from_fork)
 .macro idtentry_body cfunc has_error_code:req
 
 	call	error_entry
+	movq	%rax, %rsp			/* switch to the task stack if from userspace */
+	ENCODE_FRAME_POINTER
 	UNWIND_HINT_REGS
 
 	movq	%rsp, %rdi			/* pt_regs pointer into 1st argument*/
@@ -1002,14 +1004,10 @@ SYM_CODE_START_LOCAL(error_entry)
 	/* We have user CR3.  Change to kernel CR3. */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 
+	leaq	8(%rsp), %rdi			/* arg0 = pt_regs pointer */
 .Lerror_entry_from_usermode_after_swapgs:
 	/* Put us onto the real thread stack. */
-	popq	%r12				/* save return addr in %12 */
-	movq	%rsp, %rdi			/* arg0 = pt_regs pointer */
 	call	sync_regs
-	movq	%rax, %rsp			/* switch stack */
-	ENCODE_FRAME_POINTER
-	pushq	%r12
 	RET
 
 	/*
@@ -1041,6 +1039,7 @@ SYM_CODE_START_LOCAL(error_entry)
 	 */
 .Lerror_entry_done_lfence:
 	FENCE_SWAPGS_KERNEL_ENTRY
+	leaq	8(%rsp), %rax			/* return pt_regs pointer */
 	RET
 
 .Lbstep_iret:
@@ -1061,12 +1060,9 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * Pretend that the exception came from user mode: set up pt_regs
 	 * as if we faulted immediately after IRET.
 	 */
-	popq	%r12				/* save return addr in %12 */
-	movq	%rsp, %rdi			/* arg0 = pt_regs pointer */
+	leaq	8(%rsp), %rdi			/* arg0 = pt_regs pointer */
 	call	fixup_bad_iret
-	mov	%rax, %rsp
-	ENCODE_FRAME_POINTER
-	pushq	%r12
+	mov	%rax, %rdi
 	jmp	.Lerror_entry_from_usermode_after_swapgs
 SYM_CODE_END(error_entry)
 
-- 
2.19.1.6.gb485710b


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

* [PATCH V7 3/6] x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry()
  2022-05-03  3:21 [PATCH V7 0/6] x86/entry: Clean up entry code Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 1/6] x86/traps: Move pt_regs only in fixup_bad_iret() Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 2/6] x86/entry: Switch the stack after error_entry() returns Lai Jiangshan
@ 2022-05-03  3:21 ` Lai Jiangshan
  2022-05-03  7:23   ` Juergen Gross
  2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 4/6] x86/entry: Move cld to the start of idtentry macro Lai Jiangshan
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 15+ messages in thread
From: Lai Jiangshan @ 2022-05-03  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, Juergen Gross, x86, Lai Jiangshan, Ingo Molnar,
	Dave Hansen, H. Peter Anvin

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

The macro idtentry calls error_entry() unconditionally even on XENPV.
But the code XENPV needs in error_entry() is PUSH_AND_CLEAR_REGS only.

And PUSH_AND_CLEAR_REGS in error_entry() makes the stack not return to
its original place when the function returns, which means it is not
possible to convert it to a C function.

Move PUSH_AND_CLEAR_REGS out of error_entry(), add a function to wrap
PUSH_AND_CLEAR_REGS and call it before error_entry().

It will allow for error_entry() to be not called on XENPV and for
error_entry() to be converted to C code.

Cc: Juergen Gross <jgross@suse.com>
Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/entry/entry_64.S | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ca3e99e08a44..b1cef3b0a7ab 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -318,6 +318,14 @@ SYM_CODE_END(ret_from_fork)
 #endif
 .endm
 
+/* Save all registers in pt_regs */
+SYM_CODE_START_LOCAL(push_and_clear_regs)
+	UNWIND_HINT_FUNC
+	PUSH_AND_CLEAR_REGS save_ret=1
+	ENCODE_FRAME_POINTER 8
+	RET
+SYM_CODE_END(push_and_clear_regs)
+
 /**
  * idtentry_body - Macro to emit code calling the C function
  * @cfunc:		C function to be called
@@ -325,6 +333,9 @@ SYM_CODE_END(ret_from_fork)
  */
 .macro idtentry_body cfunc has_error_code:req
 
+	call push_and_clear_regs
+	UNWIND_HINT_REGS
+
 	call	error_entry
 	movq	%rax, %rsp			/* switch to the task stack if from userspace */
 	ENCODE_FRAME_POINTER
@@ -985,13 +996,11 @@ SYM_CODE_START_LOCAL(paranoid_exit)
 SYM_CODE_END(paranoid_exit)
 
 /*
- * Save all registers in pt_regs, and switch GS if needed.
+ * Switch GS and CR3 if needed.
  */
 SYM_CODE_START_LOCAL(error_entry)
 	UNWIND_HINT_FUNC
 	cld
-	PUSH_AND_CLEAR_REGS save_ret=1
-	ENCODE_FRAME_POINTER 8
 	testb	$3, CS+8(%rsp)
 	jz	.Lerror_kernelspace
 
-- 
2.19.1.6.gb485710b


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

* [PATCH V7 4/6] x86/entry: Move cld to the start of idtentry macro
  2022-05-03  3:21 [PATCH V7 0/6] x86/entry: Clean up entry code Lai Jiangshan
                   ` (2 preceding siblings ...)
  2022-05-03  3:21 ` [PATCH V7 3/6] x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry() Lai Jiangshan
@ 2022-05-03  3:21 ` Lai Jiangshan
  2022-05-03 19:01   ` [tip: x86/asm] x86/entry: Move CLD to the start of the " tip-bot2 for Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 5/6] x86/entry: Don't call error_entry() for XENPV Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 6/6] x86/entry: Convert SWAPGS to swapgs and remove the definition of SWAPGS Lai Jiangshan
  5 siblings, 1 reply; 15+ messages in thread
From: Lai Jiangshan @ 2022-05-03  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, Juergen Gross, x86, Lai Jiangshan, Ingo Molnar,
	Dave Hansen, H. Peter Anvin

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

Make it next to CLAC

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/entry/entry_64.S | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index b1cef3b0a7ab..ab6ab6d3dab5 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -371,6 +371,7 @@ SYM_CODE_START(\asmsym)
 	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
 	ENDBR
 	ASM_CLAC
+	cld
 
 	.if \has_error_code == 0
 		pushq	$-1			/* ORIG_RAX: no syscall to restart */
@@ -439,6 +440,7 @@ SYM_CODE_START(\asmsym)
 	UNWIND_HINT_IRET_REGS
 	ENDBR
 	ASM_CLAC
+	cld
 
 	pushq	$-1			/* ORIG_RAX: no syscall to restart */
 
@@ -495,6 +497,7 @@ SYM_CODE_START(\asmsym)
 	UNWIND_HINT_IRET_REGS
 	ENDBR
 	ASM_CLAC
+	cld
 
 	/*
 	 * If the entry is from userspace, switch stacks and treat it as
@@ -557,6 +560,7 @@ SYM_CODE_START(\asmsym)
 	UNWIND_HINT_IRET_REGS offset=8
 	ENDBR
 	ASM_CLAC
+	cld
 
 	/* paranoid_entry returns GS information for paranoid_exit in EBX. */
 	call	paranoid_entry
@@ -882,7 +886,6 @@ SYM_CODE_END(xen_failsafe_callback)
  */
 SYM_CODE_START_LOCAL(paranoid_entry)
 	UNWIND_HINT_FUNC
-	cld
 	PUSH_AND_CLEAR_REGS save_ret=1
 	ENCODE_FRAME_POINTER 8
 
@@ -1000,7 +1003,6 @@ SYM_CODE_END(paranoid_exit)
  */
 SYM_CODE_START_LOCAL(error_entry)
 	UNWIND_HINT_FUNC
-	cld
 	testb	$3, CS+8(%rsp)
 	jz	.Lerror_kernelspace
 
@@ -1134,6 +1136,7 @@ SYM_CODE_START(asm_exc_nmi)
 	 */
 
 	ASM_CLAC
+	cld
 
 	/* Use %rdx as our temp variable throughout */
 	pushq	%rdx
@@ -1153,7 +1156,6 @@ SYM_CODE_START(asm_exc_nmi)
 	 */
 
 	swapgs
-	cld
 	FENCE_SWAPGS_USER_ENTRY
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
 	movq	%rsp, %rdx
-- 
2.19.1.6.gb485710b


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

* [PATCH V7 5/6] x86/entry: Don't call error_entry() for XENPV
  2022-05-03  3:21 [PATCH V7 0/6] x86/entry: Clean up entry code Lai Jiangshan
                   ` (3 preceding siblings ...)
  2022-05-03  3:21 ` [PATCH V7 4/6] x86/entry: Move cld to the start of idtentry macro Lai Jiangshan
@ 2022-05-03  3:21 ` Lai Jiangshan
  2022-05-03  7:24   ` Juergen Gross
  2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
  2022-05-03  3:21 ` [PATCH V7 6/6] x86/entry: Convert SWAPGS to swapgs and remove the definition of SWAPGS Lai Jiangshan
  5 siblings, 2 replies; 15+ messages in thread
From: Lai Jiangshan @ 2022-05-03  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, Juergen Gross, x86, Lai Jiangshan, Ingo Molnar,
	Dave Hansen, H. Peter Anvin

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

When in XENPV, it is already in the task stack, and it can't fault for
native_iret() nor native_load_gs_index() since XENPV uses its own pvops
for IRET and load_gs_index().  And it doesn't need to switch the CR3.

So there is no reason to call error_entry() in XENPV.

Cc: Juergen Gross <jgross@suse.com>
Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/entry/entry_64.S | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ab6ab6d3dab5..062aa9d95961 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -336,8 +336,17 @@ SYM_CODE_END(push_and_clear_regs)
 	call push_and_clear_regs
 	UNWIND_HINT_REGS
 
-	call	error_entry
-	movq	%rax, %rsp			/* switch to the task stack if from userspace */
+	/*
+	 * Call error_entry() and switch to the task stack if from userspace.
+	 *
+	 * When in XENPV, it is already in the task stack, and it can't fault
+	 * for native_iret() nor native_load_gs_index() since XENPV uses its
+	 * own pvops for IRET and load_gs_index().  And it doesn't need to
+	 * switch the CR3.  So it can skip invoking error_entry().
+	 */
+	ALTERNATIVE "call error_entry; movq %rax, %rsp", \
+		"", X86_FEATURE_XENPV
+
 	ENCODE_FRAME_POINTER
 	UNWIND_HINT_REGS
 
-- 
2.19.1.6.gb485710b


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

* [PATCH V7 6/6] x86/entry: Convert SWAPGS to swapgs and remove the definition of SWAPGS
  2022-05-03  3:21 [PATCH V7 0/6] x86/entry: Clean up entry code Lai Jiangshan
                   ` (4 preceding siblings ...)
  2022-05-03  3:21 ` [PATCH V7 5/6] x86/entry: Don't call error_entry() for XENPV Lai Jiangshan
@ 2022-05-03  3:21 ` Lai Jiangshan
  2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
  5 siblings, 1 reply; 15+ messages in thread
From: Lai Jiangshan @ 2022-05-03  3:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, Juergen Gross, x86, Lai Jiangshan, Ingo Molnar,
	Dave Hansen, H. Peter Anvin, Kirill A. Shutemov

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

XENPV doesn't use swapgs_restore_regs_and_return_to_usermode(),
error_entry() and the code between entry_SYSENTER_compat() and
entry_SYSENTER_compat_after_hwframe.

Change the PV-compatible SWAPGS to the ASM instruction swapgs in these
places.

Also remove the definition of SWAPGS since there is no user of the
SWAPGS anymore.

Reviewed-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/entry/entry_64.S        | 6 +++---
 arch/x86/entry/entry_64_compat.S | 2 +-
 arch/x86/include/asm/irqflags.h  | 8 --------
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 062aa9d95961..312186612f4e 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1019,7 +1019,7 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * We entered from user mode or we're pretending to have entered
 	 * from user mode due to an IRET fault.
 	 */
-	SWAPGS
+	swapgs
 	FENCE_SWAPGS_USER_ENTRY
 	/* We have user CR3.  Change to kernel CR3. */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
@@ -1051,7 +1051,7 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * gsbase and proceed.  We'll fix up the exception and land in
 	 * .Lgs_change's error handler with kernel gsbase.
 	 */
-	SWAPGS
+	swapgs
 
 	/*
 	 * Issue an LFENCE to prevent GS speculation, regardless of whether it is a
@@ -1072,7 +1072,7 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * We came from an IRET to user mode, so we have user
 	 * gsbase and CR3.  Switch to kernel gsbase and CR3:
 	 */
-	SWAPGS
+	swapgs
 	FENCE_SWAPGS_USER_ENTRY
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 4fdb007cddbd..c5aeb0819707 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -50,7 +50,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
 	UNWIND_HINT_EMPTY
 	ENDBR
 	/* Interrupts are off on entry. */
-	SWAPGS
+	swapgs
 
 	pushq	%rax
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index 111104d1c2cd..7793e52d6237 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -137,14 +137,6 @@ static __always_inline void arch_local_irq_restore(unsigned long flags)
 	if (!arch_irqs_disabled_flags(flags))
 		arch_local_irq_enable();
 }
-#else
-#ifdef CONFIG_X86_64
-#ifdef CONFIG_XEN_PV
-#define SWAPGS	ALTERNATIVE "swapgs", "", X86_FEATURE_XENPV
-#else
-#define SWAPGS	swapgs
-#endif
-#endif
 #endif /* !__ASSEMBLY__ */
 
 #endif
-- 
2.19.1.6.gb485710b


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

* Re: [PATCH V7 3/6] x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry()
  2022-05-03  3:21 ` [PATCH V7 3/6] x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry() Lai Jiangshan
@ 2022-05-03  7:23   ` Juergen Gross
  2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
  1 sibling, 0 replies; 15+ messages in thread
From: Juergen Gross @ 2022-05-03  7:23 UTC (permalink / raw)
  To: Lai Jiangshan, linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, x86, Lai Jiangshan, Ingo Molnar, Dave Hansen,
	H. Peter Anvin


[-- Attachment #1.1.1: Type: text/plain, Size: 852 bytes --]

On 03.05.22 05:21, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> The macro idtentry calls error_entry() unconditionally even on XENPV.
> But the code XENPV needs in error_entry() is PUSH_AND_CLEAR_REGS only.
> 
> And PUSH_AND_CLEAR_REGS in error_entry() makes the stack not return to
> its original place when the function returns, which means it is not
> possible to convert it to a C function.
> 
> Move PUSH_AND_CLEAR_REGS out of error_entry(), add a function to wrap
> PUSH_AND_CLEAR_REGS and call it before error_entry().
> 
> It will allow for error_entry() to be not called on XENPV and for
> error_entry() to be converted to C code.
> 
> Cc: Juergen Gross <jgross@suse.com>
> Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

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

* Re: [PATCH V7 5/6] x86/entry: Don't call error_entry() for XENPV
  2022-05-03  3:21 ` [PATCH V7 5/6] x86/entry: Don't call error_entry() for XENPV Lai Jiangshan
@ 2022-05-03  7:24   ` Juergen Gross
  2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
  1 sibling, 0 replies; 15+ messages in thread
From: Juergen Gross @ 2022-05-03  7:24 UTC (permalink / raw)
  To: Lai Jiangshan, linux-kernel
  Cc: Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Andy Lutomirski,
	Thomas Gleixner, x86, Lai Jiangshan, Ingo Molnar, Dave Hansen,
	H. Peter Anvin


[-- Attachment #1.1.1: Type: text/plain, Size: 545 bytes --]

On 03.05.22 05:21, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> When in XENPV, it is already in the task stack, and it can't fault for
> native_iret() nor native_load_gs_index() since XENPV uses its own pvops
> for IRET and load_gs_index().  And it doesn't need to switch the CR3.
> 
> So there is no reason to call error_entry() in XENPV.
> 
> Cc: Juergen Gross <jgross@suse.com>
> Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

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

* [tip: x86/asm] x86/entry: Convert SWAPGS to swapgs and remove the definition of SWAPGS
  2022-05-03  3:21 ` [PATCH V7 6/6] x86/entry: Convert SWAPGS to swapgs and remove the definition of SWAPGS Lai Jiangshan
@ 2022-05-03 19:01   ` tip-bot2 for Lai Jiangshan
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot2 for Lai Jiangshan @ 2022-05-03 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Lai Jiangshan, Borislav Petkov, Juergen Gross, x86, linux-kernel

The following commit has been merged into the x86/asm branch of tip:

Commit-ID:     c89191ce67efa4e5353db6a67f7287c28e673740
Gitweb:        https://git.kernel.org/tip/c89191ce67efa4e5353db6a67f7287c28e673740
Author:        Lai Jiangshan <jiangshan.ljs@antgroup.com>
AuthorDate:    Tue, 03 May 2022 11:21:07 +08:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 03 May 2022 12:26:08 +02:00

x86/entry: Convert SWAPGS to swapgs and remove the definition of SWAPGS

XENPV doesn't use swapgs_restore_regs_and_return_to_usermode(),
error_entry() and the code between entry_SYSENTER_compat() and
entry_SYSENTER_compat_after_hwframe.

Change the PV-compatible SWAPGS to the ASM instruction swapgs in these
places.

Also remove the definition of SWAPGS since no more users.

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
Link: https://lore.kernel.org/r/20220503032107.680190-7-jiangshanlai@gmail.com
---
 arch/x86/entry/entry_64.S        | 6 +++---
 arch/x86/entry/entry_64_compat.S | 2 +-
 arch/x86/include/asm/irqflags.h  | 8 --------
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 062aa9d..3121866 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1019,7 +1019,7 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * We entered from user mode or we're pretending to have entered
 	 * from user mode due to an IRET fault.
 	 */
-	SWAPGS
+	swapgs
 	FENCE_SWAPGS_USER_ENTRY
 	/* We have user CR3.  Change to kernel CR3. */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
@@ -1051,7 +1051,7 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * gsbase and proceed.  We'll fix up the exception and land in
 	 * .Lgs_change's error handler with kernel gsbase.
 	 */
-	SWAPGS
+	swapgs
 
 	/*
 	 * Issue an LFENCE to prevent GS speculation, regardless of whether it is a
@@ -1072,7 +1072,7 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * We came from an IRET to user mode, so we have user
 	 * gsbase and CR3.  Switch to kernel gsbase and CR3:
 	 */
-	SWAPGS
+	swapgs
 	FENCE_SWAPGS_USER_ENTRY
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 4fdb007..c5aeb08 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -50,7 +50,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
 	UNWIND_HINT_EMPTY
 	ENDBR
 	/* Interrupts are off on entry. */
-	SWAPGS
+	swapgs
 
 	pushq	%rax
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index 111104d..7793e52 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -137,14 +137,6 @@ static __always_inline void arch_local_irq_restore(unsigned long flags)
 	if (!arch_irqs_disabled_flags(flags))
 		arch_local_irq_enable();
 }
-#else
-#ifdef CONFIG_X86_64
-#ifdef CONFIG_XEN_PV
-#define SWAPGS	ALTERNATIVE "swapgs", "", X86_FEATURE_XENPV
-#else
-#define SWAPGS	swapgs
-#endif
-#endif
 #endif /* !__ASSEMBLY__ */
 
 #endif

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

* [tip: x86/asm] x86/entry: Don't call error_entry() for XENPV
  2022-05-03  3:21 ` [PATCH V7 5/6] x86/entry: Don't call error_entry() for XENPV Lai Jiangshan
  2022-05-03  7:24   ` Juergen Gross
@ 2022-05-03 19:01   ` tip-bot2 for Lai Jiangshan
  1 sibling, 0 replies; 15+ messages in thread
From: tip-bot2 for Lai Jiangshan @ 2022-05-03 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Lai Jiangshan, Borislav Petkov, Juergen Gross, x86, linux-kernel

The following commit has been merged into the x86/asm branch of tip:

Commit-ID:     64cbd0acb58203fb769ed2f4eab526d43e243847
Gitweb:        https://git.kernel.org/tip/64cbd0acb58203fb769ed2f4eab526d43e243847
Author:        Lai Jiangshan <jiangshan.ljs@antgroup.com>
AuthorDate:    Tue, 03 May 2022 11:21:06 +08:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 03 May 2022 12:21:35 +02:00

x86/entry: Don't call error_entry() for XENPV

XENPV guests enter already on the task stack and they can't fault for
native_iret() nor native_load_gs_index() since they use their own pvop
for IRET and load_gs_index(). A CR3 switch is not needed either.

So there is no reason to call error_entry() in XENPV.

  [ bp: Massage commit message. ]

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
Link: https://lore.kernel.org/r/20220503032107.680190-6-jiangshanlai@gmail.com
---
 arch/x86/entry/entry_64.S | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ab6ab6d..062aa9d 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -336,8 +336,17 @@ SYM_CODE_END(push_and_clear_regs)
 	call push_and_clear_regs
 	UNWIND_HINT_REGS
 
-	call	error_entry
-	movq	%rax, %rsp			/* switch to the task stack if from userspace */
+	/*
+	 * Call error_entry() and switch to the task stack if from userspace.
+	 *
+	 * When in XENPV, it is already in the task stack, and it can't fault
+	 * for native_iret() nor native_load_gs_index() since XENPV uses its
+	 * own pvops for IRET and load_gs_index().  And it doesn't need to
+	 * switch the CR3.  So it can skip invoking error_entry().
+	 */
+	ALTERNATIVE "call error_entry; movq %rax, %rsp", \
+		"", X86_FEATURE_XENPV
+
 	ENCODE_FRAME_POINTER
 	UNWIND_HINT_REGS
 

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

* [tip: x86/asm] x86/entry: Move CLD to the start of the idtentry macro
  2022-05-03  3:21 ` [PATCH V7 4/6] x86/entry: Move cld to the start of idtentry macro Lai Jiangshan
@ 2022-05-03 19:01   ` tip-bot2 for Lai Jiangshan
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot2 for Lai Jiangshan @ 2022-05-03 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Peter Zijlstra, Lai Jiangshan, Borislav Petkov, x86, linux-kernel

The following commit has been merged into the x86/asm branch of tip:

Commit-ID:     c64cc2802a784ecfd25d39945e57e7a147854a5b
Gitweb:        https://git.kernel.org/tip/c64cc2802a784ecfd25d39945e57e7a147854a5b
Author:        Lai Jiangshan <jiangshan.ljs@antgroup.com>
AuthorDate:    Thu, 21 Apr 2022 22:10:51 +08:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 03 May 2022 12:17:16 +02:00

x86/entry: Move CLD to the start of the idtentry macro

Move it after CLAC.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lore.kernel.org/r/20220503032107.680190-5-jiangshanlai@gmail.com
---
 arch/x86/entry/entry_64.S | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index b1cef3b..ab6ab6d 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -371,6 +371,7 @@ SYM_CODE_START(\asmsym)
 	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
 	ENDBR
 	ASM_CLAC
+	cld
 
 	.if \has_error_code == 0
 		pushq	$-1			/* ORIG_RAX: no syscall to restart */
@@ -439,6 +440,7 @@ SYM_CODE_START(\asmsym)
 	UNWIND_HINT_IRET_REGS
 	ENDBR
 	ASM_CLAC
+	cld
 
 	pushq	$-1			/* ORIG_RAX: no syscall to restart */
 
@@ -495,6 +497,7 @@ SYM_CODE_START(\asmsym)
 	UNWIND_HINT_IRET_REGS
 	ENDBR
 	ASM_CLAC
+	cld
 
 	/*
 	 * If the entry is from userspace, switch stacks and treat it as
@@ -557,6 +560,7 @@ SYM_CODE_START(\asmsym)
 	UNWIND_HINT_IRET_REGS offset=8
 	ENDBR
 	ASM_CLAC
+	cld
 
 	/* paranoid_entry returns GS information for paranoid_exit in EBX. */
 	call	paranoid_entry
@@ -882,7 +886,6 @@ SYM_CODE_END(xen_failsafe_callback)
  */
 SYM_CODE_START_LOCAL(paranoid_entry)
 	UNWIND_HINT_FUNC
-	cld
 	PUSH_AND_CLEAR_REGS save_ret=1
 	ENCODE_FRAME_POINTER 8
 
@@ -1000,7 +1003,6 @@ SYM_CODE_END(paranoid_exit)
  */
 SYM_CODE_START_LOCAL(error_entry)
 	UNWIND_HINT_FUNC
-	cld
 	testb	$3, CS+8(%rsp)
 	jz	.Lerror_kernelspace
 
@@ -1134,6 +1136,7 @@ SYM_CODE_START(asm_exc_nmi)
 	 */
 
 	ASM_CLAC
+	cld
 
 	/* Use %rdx as our temp variable throughout */
 	pushq	%rdx
@@ -1153,7 +1156,6 @@ SYM_CODE_START(asm_exc_nmi)
 	 */
 
 	swapgs
-	cld
 	FENCE_SWAPGS_USER_ENTRY
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
 	movq	%rsp, %rdx

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

* [tip: x86/asm] x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry()
  2022-05-03  3:21 ` [PATCH V7 3/6] x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry() Lai Jiangshan
  2022-05-03  7:23   ` Juergen Gross
@ 2022-05-03 19:01   ` tip-bot2 for Lai Jiangshan
  1 sibling, 0 replies; 15+ messages in thread
From: tip-bot2 for Lai Jiangshan @ 2022-05-03 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Lai Jiangshan, Borislav Petkov, Juergen Gross, x86, linux-kernel

The following commit has been merged into the x86/asm branch of tip:

Commit-ID:     ee774dac0da1543376a69fd90840af6aa86879b3
Gitweb:        https://git.kernel.org/tip/ee774dac0da1543376a69fd90840af6aa86879b3
Author:        Lai Jiangshan <jiangshan.ljs@antgroup.com>
AuthorDate:    Thu, 21 Apr 2022 22:10:50 +08:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 03 May 2022 11:42:59 +02:00

x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry()

The macro idtentry() (through idtentry_body()) calls error_entry()
unconditionally even on XENPV. But XENPV needs to only push and clear
regs.

PUSH_AND_CLEAR_REGS in error_entry() makes the stack not return to its
original place when the function returns, which means it is not possible
to convert it to a C function.

Carve out PUSH_AND_CLEAR_REGS out of error_entry() and into a separate
function and call it before error_entry() in order to avoid calling
error_entry() on XENPV.

It will also allow for error_entry() to be converted to C code that can
use inlined sync_regs() and save a function call.

  [ bp: Massage commit message. ]

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
Link: https://lore.kernel.org/r/20220503032107.680190-4-jiangshanlai@gmail.com
---
 arch/x86/entry/entry_64.S | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ca3e99e..b1cef3b 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -318,6 +318,14 @@ SYM_CODE_END(ret_from_fork)
 #endif
 .endm
 
+/* Save all registers in pt_regs */
+SYM_CODE_START_LOCAL(push_and_clear_regs)
+	UNWIND_HINT_FUNC
+	PUSH_AND_CLEAR_REGS save_ret=1
+	ENCODE_FRAME_POINTER 8
+	RET
+SYM_CODE_END(push_and_clear_regs)
+
 /**
  * idtentry_body - Macro to emit code calling the C function
  * @cfunc:		C function to be called
@@ -325,6 +333,9 @@ SYM_CODE_END(ret_from_fork)
  */
 .macro idtentry_body cfunc has_error_code:req
 
+	call push_and_clear_regs
+	UNWIND_HINT_REGS
+
 	call	error_entry
 	movq	%rax, %rsp			/* switch to the task stack if from userspace */
 	ENCODE_FRAME_POINTER
@@ -985,13 +996,11 @@ SYM_CODE_START_LOCAL(paranoid_exit)
 SYM_CODE_END(paranoid_exit)
 
 /*
- * Save all registers in pt_regs, and switch GS if needed.
+ * Switch GS and CR3 if needed.
  */
 SYM_CODE_START_LOCAL(error_entry)
 	UNWIND_HINT_FUNC
 	cld
-	PUSH_AND_CLEAR_REGS save_ret=1
-	ENCODE_FRAME_POINTER 8
 	testb	$3, CS+8(%rsp)
 	jz	.Lerror_kernelspace
 

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

* [tip: x86/asm] x86/entry: Switch the stack after error_entry() returns
  2022-05-03  3:21 ` [PATCH V7 2/6] x86/entry: Switch the stack after error_entry() returns Lai Jiangshan
@ 2022-05-03 19:01   ` tip-bot2 for Lai Jiangshan
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot2 for Lai Jiangshan @ 2022-05-03 19:01 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Lai Jiangshan, Borislav Petkov, x86, linux-kernel

The following commit has been merged into the x86/asm branch of tip:

Commit-ID:     520a7e80c96d655fbe4650d9cc985bd9d0443389
Gitweb:        https://git.kernel.org/tip/520a7e80c96d655fbe4650d9cc985bd9d0443389
Author:        Lai Jiangshan <jiangshan.ljs@antgroup.com>
AuthorDate:    Thu, 21 Apr 2022 22:10:49 +08:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 03 May 2022 11:35:34 +02:00

x86/entry: Switch the stack after error_entry() returns

error_entry() calls fixup_bad_iret() before sync_regs() if it is a fault
from a bad IRET, to copy pt_regs to the kernel stack. It switches to the
kernel stack directly after sync_regs().

But error_entry() itself is also a function call, so it has to stash
the address it is going to return to, in %r12 which is unnecessarily
complicated.

Move the stack switching after error_entry() and get rid of the need to
handle the return address.

  [ bp: Massage commit message. ]

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lore.kernel.org/r/20220503032107.680190-3-jiangshanlai@gmail.com
---
 arch/x86/entry/entry_64.S | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ecbfca3..ca3e99e 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -326,6 +326,8 @@ SYM_CODE_END(ret_from_fork)
 .macro idtentry_body cfunc has_error_code:req
 
 	call	error_entry
+	movq	%rax, %rsp			/* switch to the task stack if from userspace */
+	ENCODE_FRAME_POINTER
 	UNWIND_HINT_REGS
 
 	movq	%rsp, %rdi			/* pt_regs pointer into 1st argument*/
@@ -1002,14 +1004,10 @@ SYM_CODE_START_LOCAL(error_entry)
 	/* We have user CR3.  Change to kernel CR3. */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
 
+	leaq	8(%rsp), %rdi			/* arg0 = pt_regs pointer */
 .Lerror_entry_from_usermode_after_swapgs:
 	/* Put us onto the real thread stack. */
-	popq	%r12				/* save return addr in %12 */
-	movq	%rsp, %rdi			/* arg0 = pt_regs pointer */
 	call	sync_regs
-	movq	%rax, %rsp			/* switch stack */
-	ENCODE_FRAME_POINTER
-	pushq	%r12
 	RET
 
 	/*
@@ -1041,6 +1039,7 @@ SYM_CODE_START_LOCAL(error_entry)
 	 */
 .Lerror_entry_done_lfence:
 	FENCE_SWAPGS_KERNEL_ENTRY
+	leaq	8(%rsp), %rax			/* return pt_regs pointer */
 	RET
 
 .Lbstep_iret:
@@ -1061,12 +1060,9 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * Pretend that the exception came from user mode: set up pt_regs
 	 * as if we faulted immediately after IRET.
 	 */
-	popq	%r12				/* save return addr in %12 */
-	movq	%rsp, %rdi			/* arg0 = pt_regs pointer */
+	leaq	8(%rsp), %rdi			/* arg0 = pt_regs pointer */
 	call	fixup_bad_iret
-	mov	%rax, %rsp
-	ENCODE_FRAME_POINTER
-	pushq	%r12
+	mov	%rax, %rdi
 	jmp	.Lerror_entry_from_usermode_after_swapgs
 SYM_CODE_END(error_entry)
 

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

* [tip: x86/asm] x86/traps: Use pt_regs directly in fixup_bad_iret()
  2022-05-03  3:21 ` [PATCH V7 1/6] x86/traps: Move pt_regs only in fixup_bad_iret() Lai Jiangshan
@ 2022-05-03 19:01   ` tip-bot2 for Lai Jiangshan
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot2 for Lai Jiangshan @ 2022-05-03 19:01 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Lai Jiangshan, Borislav Petkov, x86, linux-kernel

The following commit has been merged into the x86/asm branch of tip:

Commit-ID:     0aca53c6b522f8d6e2681ca875acbbe105f5fdcf
Gitweb:        https://git.kernel.org/tip/0aca53c6b522f8d6e2681ca875acbbe105f5fdcf
Author:        Lai Jiangshan <jiangshan.ljs@antgroup.com>
AuthorDate:    Thu, 21 Apr 2022 22:10:48 +08:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 03 May 2022 11:18:59 +02:00

x86/traps: Use pt_regs directly in fixup_bad_iret()

Always stash the address error_entry() is going to return to, in %r12
and get rid of the void *error_entry_ret; slot in struct bad_iret_stack
which was supposed to account for it and pt_regs pushed on the stack.

After this, both fixup_bad_iret() and sync_regs() can work on a struct
pt_regs pointer directly.

  [ bp: Rewrite commit message, touch ups. ]

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lore.kernel.org/r/20220503032107.680190-2-jiangshanlai@gmail.com
---
 arch/x86/entry/entry_64.S    |  5 ++++-
 arch/x86/include/asm/traps.h |  2 +-
 arch/x86/kernel/traps.c      | 19 +++++++------------
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 73d9585..ecbfca3 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1061,9 +1061,12 @@ SYM_CODE_START_LOCAL(error_entry)
 	 * Pretend that the exception came from user mode: set up pt_regs
 	 * as if we faulted immediately after IRET.
 	 */
-	mov	%rsp, %rdi
+	popq	%r12				/* save return addr in %12 */
+	movq	%rsp, %rdi			/* arg0 = pt_regs pointer */
 	call	fixup_bad_iret
 	mov	%rax, %rsp
+	ENCODE_FRAME_POINTER
+	pushq	%r12
 	jmp	.Lerror_entry_from_usermode_after_swapgs
 SYM_CODE_END(error_entry)
 
diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 35317c5..47ecfff 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -13,7 +13,7 @@
 #ifdef CONFIG_X86_64
 asmlinkage __visible notrace struct pt_regs *sync_regs(struct pt_regs *eregs);
 asmlinkage __visible notrace
-struct bad_iret_stack *fixup_bad_iret(struct bad_iret_stack *s);
+struct pt_regs *fixup_bad_iret(struct pt_regs *bad_regs);
 void __init trap_init(void);
 asmlinkage __visible noinstr struct pt_regs *vc_switch_off_ist(struct pt_regs *eregs);
 #endif
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 1563fb9..4167215 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -892,14 +892,10 @@ sync:
 }
 #endif
 
-struct bad_iret_stack {
-	void *error_entry_ret;
-	struct pt_regs regs;
-};
-
-asmlinkage __visible noinstr
-struct bad_iret_stack *fixup_bad_iret(struct bad_iret_stack *s)
+asmlinkage __visible noinstr struct pt_regs *fixup_bad_iret(struct pt_regs *bad_regs)
 {
+	struct pt_regs tmp, *new_stack;
+
 	/*
 	 * This is called from entry_64.S early in handling a fault
 	 * caused by a bad iret to user mode.  To handle the fault
@@ -908,19 +904,18 @@ struct bad_iret_stack *fixup_bad_iret(struct bad_iret_stack *s)
 	 * just below the IRET frame) and we want to pretend that the
 	 * exception came from the IRET target.
 	 */
-	struct bad_iret_stack tmp, *new_stack =
-		(struct bad_iret_stack *)__this_cpu_read(cpu_tss_rw.x86_tss.sp0) - 1;
+	new_stack = (struct pt_regs *)__this_cpu_read(cpu_tss_rw.x86_tss.sp0) - 1;
 
 	/* Copy the IRET target to the temporary storage. */
-	__memcpy(&tmp.regs.ip, (void *)s->regs.sp, 5*8);
+	__memcpy(&tmp.ip, (void *)bad_regs->sp, 5*8);
 
 	/* Copy the remainder of the stack from the current stack. */
-	__memcpy(&tmp, s, offsetof(struct bad_iret_stack, regs.ip));
+	__memcpy(&tmp, bad_regs, offsetof(struct pt_regs, ip));
 
 	/* Update the entry stack */
 	__memcpy(new_stack, &tmp, sizeof(tmp));
 
-	BUG_ON(!user_mode(&new_stack->regs));
+	BUG_ON(!user_mode(new_stack));
 	return new_stack;
 }
 #endif

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

end of thread, other threads:[~2022-05-03 19:01 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-03  3:21 [PATCH V7 0/6] x86/entry: Clean up entry code Lai Jiangshan
2022-05-03  3:21 ` [PATCH V7 1/6] x86/traps: Move pt_regs only in fixup_bad_iret() Lai Jiangshan
2022-05-03 19:01   ` [tip: x86/asm] x86/traps: Use pt_regs directly " tip-bot2 for Lai Jiangshan
2022-05-03  3:21 ` [PATCH V7 2/6] x86/entry: Switch the stack after error_entry() returns Lai Jiangshan
2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
2022-05-03  3:21 ` [PATCH V7 3/6] x86/entry: Move PUSH_AND_CLEAR_REGS out of error_entry() Lai Jiangshan
2022-05-03  7:23   ` Juergen Gross
2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
2022-05-03  3:21 ` [PATCH V7 4/6] x86/entry: Move cld to the start of idtentry macro Lai Jiangshan
2022-05-03 19:01   ` [tip: x86/asm] x86/entry: Move CLD to the start of the " tip-bot2 for Lai Jiangshan
2022-05-03  3:21 ` [PATCH V7 5/6] x86/entry: Don't call error_entry() for XENPV Lai Jiangshan
2022-05-03  7:24   ` Juergen Gross
2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan
2022-05-03  3:21 ` [PATCH V7 6/6] x86/entry: Convert SWAPGS to swapgs and remove the definition of SWAPGS Lai Jiangshan
2022-05-03 19:01   ` [tip: x86/asm] " tip-bot2 for Lai Jiangshan

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.