1. Patchew
  2. linux
  3. [PATCH v2 0/7] x86: retbleed=stuff fixes
  4. View patch

[PATCH v2 1/7] x86/boot: Remove verify_cpu() from secondary_startup_64()

Peter Zijlstra posted 7 patches 2 years, 8 months ago
[PATCH v2 1/7] x86/boot: Remove verify_cpu() from secondary_startup_64()
Posted by Peter Zijlstra 2 years, 8 months ago 
The boot trampolines from trampoline_64.S have code flow like:

  16bit BIOS			SEV-ES				64bit EFI

  trampoline_start()		sev_es_trampoline_start()	trampoline_start_64()
    verify_cpu()			  |				|
  switch_to_protected:    <---------------'				v
       |							pa_trampoline_compat()
       v								|
  startup_32()		<-----------------------------------------------'
       |
       v
  startup_64()
       |
       v
  tr_start() := head_64.S:secondary_startup_64()

Since AP bringup always goes through the 16bit BIOS path (EFI doesn't
touch the APs), there is already a verify_cpu() invocation.

Removing the verify_cpu() invocation from secondary_startup_64()
renders the whole secondary_startup_64_no_verify() thing moot, so
remove that too.

Cc: jroedel@suse.de
Cc: hpa@zytor.com
Fixes: e81dc127ef69 ("x86/callthunks: Add call patching for call depth tracking")
Reported-by: Joan Bruguera <joanbrugueram@gmail.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/realmode.h |    1 -
 arch/x86/kernel/head_64.S       |   16 ----------------
 arch/x86/realmode/init.c        |    6 ------
 3 files changed, 23 deletions(-)

--- a/arch/x86/include/asm/realmode.h
+++ b/arch/x86/include/asm/realmode.h
@@ -73,7 +73,6 @@ extern unsigned char startup_32_smp[];
 extern unsigned char boot_gdt[];
 #else
 extern unsigned char secondary_startup_64[];
-extern unsigned char secondary_startup_64_no_verify[];
 #endif
 
 static inline size_t real_mode_size_needed(void)
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -143,22 +143,6 @@ SYM_CODE_START(secondary_startup_64)
 	 * after the boot processor executes this code.
 	 */
 
-	/* Sanitize CPU configuration */
-	call verify_cpu
-
-	/*
-	 * The secondary_startup_64_no_verify entry point is only used by
-	 * SEV-ES guests. In those guests the call to verify_cpu() would cause
-	 * #VC exceptions which can not be handled at this stage of secondary
-	 * CPU bringup.
-	 *
-	 * All non SEV-ES systems, especially Intel systems, need to execute
-	 * verify_cpu() above to make sure NX is enabled.
-	 */
-SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
-	UNWIND_HINT_EMPTY
-	ANNOTATE_NOENDBR
-
 	/*
 	 * Retrieve the modifier (SME encryption mask if SME is active) to be
 	 * added to the initial pgdir entry that will be programmed into CR3.
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -74,12 +74,6 @@ static void __init sme_sev_setup_real_mo
 		th->flags |= TH_FLAGS_SME_ACTIVE;
 
 	if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
-		/*
-		 * Skip the call to verify_cpu() in secondary_startup_64 as it
-		 * will cause #VC exceptions when the AP can't handle them yet.
-		 */
-		th->start = (u64) secondary_startup_64_no_verify;
-
 		if (sev_es_setup_ap_jump_table(real_mode_header))
 			panic("Failed to get/update SEV-ES AP Jump Table");
 	}
Re: [PATCH v2 1/7] x86/boot: Remove verify_cpu() from secondary_startup_64()
Posted by Peter Zijlstra 2 years, 7 months ago 
On Mon, Jan 16, 2023 at 03:25:34PM +0100, Peter Zijlstra wrote:
> The boot trampolines from trampoline_64.S have code flow like:
> 
>   16bit BIOS			SEV-ES				64bit EFI
> 
>   trampoline_start()		sev_es_trampoline_start()	trampoline_start_64()
>     verify_cpu()			  |				|
>   switch_to_protected:    <---------------'				v
>        |							pa_trampoline_compat()
>        v								|
>   startup_32()		<-----------------------------------------------'
>        |
>        v
>   startup_64()
>        |
>        v
>   tr_start() := head_64.S:secondary_startup_64()
> 
> Since AP bringup always goes through the 16bit BIOS path (EFI doesn't
> touch the APs), there is already a verify_cpu() invocation.

So supposedly TDX/ACPI-6.4 comes in on trampoline_startup64() for APs --
can any of the TDX capable folks tell me if we need verify_cpu() on
these?

Aside from checking for LM, it seems to clear XD_DISABLE on Intel and
force enable SSE on AMD/K7. Surely none of that is needed for these
shiny new chips?

I mean, I can hack up a patch that adds verify_cpu() to the 64bit entry
point, but it seems really sad to need that on modern systems.
Re: [PATCH v2 1/7] x86/boot: Remove verify_cpu() from secondary_startup_64()
Posted by H. Peter Anvin 2 years, 7 months ago 
On January 18, 2023 1:45:44 AM PST, Peter Zijlstra <peterz@infradead.org> wrote:
>On Mon, Jan 16, 2023 at 03:25:34PM +0100, Peter Zijlstra wrote:
>> The boot trampolines from trampoline_64.S have code flow like:
>> 
>>   16bit BIOS			SEV-ES				64bit EFI
>> 
>>   trampoline_start()		sev_es_trampoline_start()	trampoline_start_64()
>>     verify_cpu()			  |				|
>>   switch_to_protected:    <---------------'				v
>>        |							pa_trampoline_compat()
>>        v								|
>>   startup_32()		<-----------------------------------------------'
>>        |
>>        v
>>   startup_64()
>>        |
>>        v
>>   tr_start() := head_64.S:secondary_startup_64()
>> 
>> Since AP bringup always goes through the 16bit BIOS path (EFI doesn't
>> touch the APs), there is already a verify_cpu() invocation.
>
>So supposedly TDX/ACPI-6.4 comes in on trampoline_startup64() for APs --
>can any of the TDX capable folks tell me if we need verify_cpu() on
>these?
>
>Aside from checking for LM, it seems to clear XD_DISABLE on Intel and
>force enable SSE on AMD/K7. Surely none of that is needed for these
>shiny new chips?
>
>I mean, I can hack up a patch that adds verify_cpu() to the 64bit entry
>point, but it seems really sad to need that on modern systems.

Sad, perhaps, but really better for orthogonality – fewer special cases.
Re: [PATCH v2 1/7] x86/boot: Remove verify_cpu() from secondary_startup_64()
Posted by Peter Zijlstra 2 years, 7 months ago 
On Thu, Jan 19, 2023 at 11:35:06AM -0800, H. Peter Anvin wrote:
> On January 18, 2023 1:45:44 AM PST, Peter Zijlstra <peterz@infradead.org> wrote:
> >On Mon, Jan 16, 2023 at 03:25:34PM +0100, Peter Zijlstra wrote:
> >> The boot trampolines from trampoline_64.S have code flow like:
> >> 
> >>   16bit BIOS			SEV-ES				64bit EFI
> >> 
> >>   trampoline_start()		sev_es_trampoline_start()	trampoline_start_64()
> >>     verify_cpu()			  |				|
> >>   switch_to_protected:    <---------------'				v
> >>        |							pa_trampoline_compat()
> >>        v								|
> >>   startup_32()		<-----------------------------------------------'
> >>        |
> >>        v
> >>   startup_64()
> >>        |
> >>        v
> >>   tr_start() := head_64.S:secondary_startup_64()
> >> 
> >> Since AP bringup always goes through the 16bit BIOS path (EFI doesn't
> >> touch the APs), there is already a verify_cpu() invocation.
> >
> >So supposedly TDX/ACPI-6.4 comes in on trampoline_startup64() for APs --
> >can any of the TDX capable folks tell me if we need verify_cpu() on
> >these?
> >
> >Aside from checking for LM, it seems to clear XD_DISABLE on Intel and
> >force enable SSE on AMD/K7. Surely none of that is needed for these
> >shiny new chips?
> >
> >I mean, I can hack up a patch that adds verify_cpu() to the 64bit entry
> >point, but it seems really sad to need that on modern systems.
> 
> Sad, perhaps, but really better for orthogonality – fewer special cases.

I'd argue more, but whatever. XD_DISABLE is an abomination and 64bit
entry points should care about it just as much as having LM. And this
way we have 2/3 instead of 1/3 entry points do 'special' nonsense.

I ended up with this trainwreck, it adds verify_cpu to
pa_trampoline_compat() because for some raisin it doesn't want to
assemble when placed in trampoline_start64().

Is this really what we want?

---

--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -689,9 +689,14 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lno_longmo
 	jmp     1b
 SYM_FUNC_END(.Lno_longmode)
 
-	.globl	verify_cpu
 #include "../../kernel/verify_cpu.S"
 
+	.globl	verify_cpu
+SYM_FUNC_START_LOCAL(verify_cpu)
+	VERIFY_CPU
+	RET
+SYM_FUNC_END(verify_cpu)
+
 	.data
 SYM_DATA_START_LOCAL(gdt64)
 	.word	gdt_end - gdt - 1
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -321,6 +321,11 @@ SYM_FUNC_END(startup_32_smp)
 
 #include "verify_cpu.S"
 
+SYM_FUNC_START_LOCAL(verify_cpu)
+	VERIFY_CPU
+	RET
+SYM_FUNC_END(verify_cpu)
+
 __INIT
 SYM_FUNC_START(early_idt_handler_array)
 	# 36(%esp) %eflags
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -345,6 +345,12 @@ SYM_CODE_START(secondary_startup_64)
 SYM_CODE_END(secondary_startup_64)
 
 #include "verify_cpu.S"
+
+SYM_FUNC_START_LOCAL(verify_cpu)
+	VERIFY_CPU
+	RET
+SYM_FUNC_END(verify_cpu)
+
 #include "sev_verify_cbit.S"
 
 #ifdef CONFIG_HOTPLUG_CPU
--- a/arch/x86/kernel/verify_cpu.S
+++ b/arch/x86/kernel/verify_cpu.S
@@ -31,7 +31,7 @@
 #include <asm/cpufeatures.h>
 #include <asm/msr-index.h>
 
-SYM_FUNC_START_LOCAL(verify_cpu)
+.macro VERIFY_CPU
 	pushf				# Save caller passed flags
 	push	$0			# Kill any dangerous flags
 	popf
@@ -46,31 +46,31 @@ SYM_FUNC_START_LOCAL(verify_cpu)
 	pushfl
 	popl	%eax
 	cmpl	%eax,%ebx
-	jz	.Lverify_cpu_no_longmode	# cpu has no cpuid
+	jz	.Lverify_cpu_no_longmode_\@	# cpu has no cpuid
 #endif
 
 	movl	$0x0,%eax		# See if cpuid 1 is implemented
 	cpuid
 	cmpl	$0x1,%eax
-	jb	.Lverify_cpu_no_longmode	# no cpuid 1
+	jb	.Lverify_cpu_no_longmode_\@	# no cpuid 1
 
 	xor	%di,%di
 	cmpl	$0x68747541,%ebx	# AuthenticAMD
-	jnz	.Lverify_cpu_noamd
+	jnz	.Lverify_cpu_noamd_\@
 	cmpl	$0x69746e65,%edx
-	jnz	.Lverify_cpu_noamd
+	jnz	.Lverify_cpu_noamd_\@
 	cmpl	$0x444d4163,%ecx
-	jnz	.Lverify_cpu_noamd
+	jnz	.Lverify_cpu_noamd_\@
 	mov	$1,%di			# cpu is from AMD
-	jmp	.Lverify_cpu_check
+	jmp	.Lverify_cpu_check_\@
 
-.Lverify_cpu_noamd:
+.Lverify_cpu_noamd_\@:
 	cmpl	$0x756e6547,%ebx        # GenuineIntel?
-	jnz	.Lverify_cpu_check
+	jnz	.Lverify_cpu_check_\@
 	cmpl	$0x49656e69,%edx
-	jnz	.Lverify_cpu_check
+	jnz	.Lverify_cpu_check_\@
 	cmpl	$0x6c65746e,%ecx
-	jnz	.Lverify_cpu_check
+	jnz	.Lverify_cpu_check_\@
 
 	# only call IA32_MISC_ENABLE when:
 	# family > 6 || (family == 6 && model >= 0xd)
@@ -81,60 +81,62 @@ SYM_FUNC_START_LOCAL(verify_cpu)
 	andl	$0x0ff00f00, %eax	# mask family and extended family
 	shrl	$8, %eax
 	cmpl	$6, %eax
-	ja	.Lverify_cpu_clear_xd	# family > 6, ok
-	jb	.Lverify_cpu_check	# family < 6, skip
+	ja	.Lverify_cpu_clear_xd_\@	# family > 6, ok
+	jb	.Lverify_cpu_check_\@	# family < 6, skip
 
 	andl	$0x000f00f0, %ecx	# mask model and extended model
 	shrl	$4, %ecx
 	cmpl	$0xd, %ecx
-	jb	.Lverify_cpu_check	# family == 6, model < 0xd, skip
+	jb	.Lverify_cpu_check_\@	# family == 6, model < 0xd, skip
 
-.Lverify_cpu_clear_xd:
+.Lverify_cpu_clear_xd_\@:
 	movl	$MSR_IA32_MISC_ENABLE, %ecx
 	rdmsr
 	btrl	$2, %edx		# clear MSR_IA32_MISC_ENABLE_XD_DISABLE
-	jnc	.Lverify_cpu_check	# only write MSR if bit was changed
+	jnc	.Lverify_cpu_check_\@	# only write MSR if bit was changed
 	wrmsr
 
-.Lverify_cpu_check:
+.Lverify_cpu_check_\@:
 	movl    $0x1,%eax		# Does the cpu have what it takes
 	cpuid
 	andl	$REQUIRED_MASK0,%edx
 	xorl	$REQUIRED_MASK0,%edx
-	jnz	.Lverify_cpu_no_longmode
+	jnz	.Lverify_cpu_no_longmode_\@
 
 	movl    $0x80000000,%eax	# See if extended cpuid is implemented
 	cpuid
 	cmpl    $0x80000001,%eax
-	jb      .Lverify_cpu_no_longmode	# no extended cpuid
+	jb      .Lverify_cpu_no_longmode_\@	# no extended cpuid
 
 	movl    $0x80000001,%eax	# Does the cpu have what it takes
 	cpuid
 	andl    $REQUIRED_MASK1,%edx
 	xorl    $REQUIRED_MASK1,%edx
-	jnz     .Lverify_cpu_no_longmode
+	jnz     .Lverify_cpu_no_longmode_\@
 
-.Lverify_cpu_sse_test:
+.Lverify_cpu_sse_test_\@:
 	movl	$1,%eax
 	cpuid
 	andl	$SSE_MASK,%edx
 	cmpl	$SSE_MASK,%edx
-	je	.Lverify_cpu_sse_ok
+	je	.Lverify_cpu_sse_ok_\@
 	test	%di,%di
-	jz	.Lverify_cpu_no_longmode	# only try to force SSE on AMD
+	jz	.Lverify_cpu_no_longmode_\@	# only try to force SSE on AMD
 	movl	$MSR_K7_HWCR,%ecx
 	rdmsr
 	btr	$15,%eax		# enable SSE
 	wrmsr
 	xor	%di,%di			# don't loop
-	jmp	.Lverify_cpu_sse_test	# try again
+	jmp	.Lverify_cpu_sse_test_\@	# try again
 
-.Lverify_cpu_no_longmode:
+.Lverify_cpu_no_longmode_\@:
 	popf				# Restore caller passed flags
 	movl $1,%eax
-	RET
-.Lverify_cpu_sse_ok:
+	jmp	.Lverify_cpu_ret_\@
+
+.Lverify_cpu_sse_ok_\@:
 	popf				# Restore caller passed flags
 	xorl %eax, %eax
-	RET
-SYM_FUNC_END(verify_cpu)
+
+.Lverify_cpu_ret_\@:
+.endm
--- a/arch/x86/realmode/rm/trampoline_64.S
+++ b/arch/x86/realmode/rm/trampoline_64.S
@@ -34,6 +34,8 @@
 #include <asm/realmode.h>
 #include "realmode.h"
 
+#include "../kernel/verify_cpu.S"
+
 	.text
 	.code16
 
@@ -52,7 +54,8 @@ SYM_CODE_START(trampoline_start)
 	# Setup stack
 	movl	$rm_stack_end, %esp
 
-	call	verify_cpu		# Verify the cpu supports long mode
+	VERIFY_CPU			# Verify the cpu supports long mode
+
 	testl   %eax, %eax		# Check for return code
 	jnz	no_longmode
 
@@ -100,8 +103,6 @@ SYM_CODE_START(sev_es_trampoline_start)
 SYM_CODE_END(sev_es_trampoline_start)
 #endif	/* CONFIG_AMD_MEM_ENCRYPT */
 
-#include "../kernel/verify_cpu.S"
-
 	.section ".text32","ax"
 	.code32
 	.balign 4
@@ -180,6 +181,8 @@ SYM_CODE_START(pa_trampoline_compat)
 	movl	$rm_stack_end, %esp
 	movw	$__KERNEL_DS, %dx
 
+	VERIFY_CPU
+
 	movl	$(CR0_STATE & ~X86_CR0_PG), %eax
 	movl	%eax, %cr0
 	ljmpl   $__KERNEL32_CS, $pa_startup_32
Re: [PATCH v2 1/7] x86/boot: Remove verify_cpu() from secondary_startup_64()
Posted by kirill.shutemov@linux.intel.com 2 years, 7 months ago 
On Wed, Jan 18, 2023 at 10:45:44AM +0100, Peter Zijlstra wrote:
> On Mon, Jan 16, 2023 at 03:25:34PM +0100, Peter Zijlstra wrote:
> > The boot trampolines from trampoline_64.S have code flow like:
> > 
> >   16bit BIOS			SEV-ES				64bit EFI
> > 
> >   trampoline_start()		sev_es_trampoline_start()	trampoline_start_64()
> >     verify_cpu()			  |				|
> >   switch_to_protected:    <---------------'				v
> >        |							pa_trampoline_compat()
> >        v								|
> >   startup_32()		<-----------------------------------------------'
> >        |
> >        v
> >   startup_64()
> >        |
> >        v
> >   tr_start() := head_64.S:secondary_startup_64()
> > 
> > Since AP bringup always goes through the 16bit BIOS path (EFI doesn't
> > touch the APs), there is already a verify_cpu() invocation.
> 
> So supposedly TDX/ACPI-6.4 comes in on trampoline_startup64() for APs --
> can any of the TDX capable folks tell me if we need verify_cpu() on
> these?
> 
> Aside from checking for LM, it seems to clear XD_DISABLE on Intel and
> force enable SSE on AMD/K7. Surely none of that is needed for these
> shiny new chips?

TDX has no XD_DISABLE set and it doesn't allow to write to
IA32_MISC_ENABLE MSR (triggers #VE), so we should be safe.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v2 1/7] x86/boot: Remove verify_cpu() from secondary_startup_64()
Posted by Ingo Molnar 2 years, 8 months ago 
* Peter Zijlstra <peterz@infradead.org> wrote:

> The boot trampolines from trampoline_64.S have code flow like:
> 
>   16bit BIOS			SEV-ES				64bit EFI
> 
>   trampoline_start()		sev_es_trampoline_start()	trampoline_start_64()
>     verify_cpu()			  |				|
>   switch_to_protected:    <---------------'				v
>        |							pa_trampoline_compat()
>        v								|
>   startup_32()		<-----------------------------------------------'
>        |
>        v
>   startup_64()
>        |
>        v
>   tr_start() := head_64.S:secondary_startup_64()

oh ... this nice flow chart should move into a prominent C comment I think, 
it's far too good to be forgotten in an Git commit changelog.

> Since AP bringup always goes through the 16bit BIOS path (EFI doesn't
> touch the APs), there is already a verify_cpu() invocation.
> 
> Removing the verify_cpu() invocation from secondary_startup_64()
> renders the whole secondary_startup_64_no_verify() thing moot, so
> remove that too.
> 
> Cc: jroedel@suse.de
> Cc: hpa@zytor.com
> Fixes: e81dc127ef69 ("x86/callthunks: Add call patching for call depth tracking")
> Reported-by: Joan Bruguera <joanbrugueram@gmail.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.