Re: [PATCH v3 6/9] x86/smpboot: Support parallel startup of secondary CPUs

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3866 bytes --]

On Thu, 2021-12-16 at 13:00 -0600, Tom Lendacky wrote:
> On 12/16/21 12:24 PM, David Woodhouse wrote:
> > On Thu, 2021-12-16 at 08:24 -0600, Tom Lendacky wrote:
> > 
> > > This will break an SEV-ES guest because CPUID will generate a #VC and a
> > > #VC handler has not been established yet.
> > > 
> > > I guess for now, you can probably just not enable parallel startup for
> > > SEV-ES guests.
> > 
> > OK, thanks. I'll expand it to allow 24 bits of (physical) APIC ID then,
> > since it's no longer limited to CPUs without X2APIC. Then we can
> > refrain from doing parallel bringup for SEV-ES guests, as you suggest.
> > 
> > What precisely is the check I should be using for that?
> 
> Calling cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT) will return true for 
> an SEV-ES guest.

Thanks. Incremental patch (which I'll roll into Thomas's patch) looks a
bit like this. Testing it now...


diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 0b6012fd3e55..1ac33ce1d60e 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -199,7 +199,6 @@ extern unsigned int smpboot_control;
 #endif /* !__ASSEMBLY__ */
 
 /* Control bits for startup_64 */
-#define	STARTUP_USE_APICID	0x10000
-#define	STARTUP_USE_CPUID_0B	0x20000
+#define	STARTUP_PARALLEL	0x80000000
 
 #endif /* _ASM_X86_SMP_H */
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 0249212e23d2..3e4c3c416bce 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -189,11 +189,10 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	 * Secondary CPUs find out the offsets via the APIC ID. For parallel
 	 * boot the APIC ID is retrieved from CPUID, otherwise it's encoded
 	 * in smpboot_control:
-	 * Bit 0-15	APICID if STARTUP_USE_CPUID_0B is not set
-	 * Bit 16 	Secondary boot flag
-	 * Bit 17	Parallel boot flag
+	 * Bit 0-30	APIC ID if STARTUP_PARALLEL is not set
+	 * Bit 31	Parallel boot flag (use CPUID leaf 0x0b for APIC ID).
 	 */
-	testl	$STARTUP_USE_CPUID_0B, %eax
+	testl	$STARTUP_PARALLEL, %eax
 	jz	.Lsetup_AP
 
 	mov	$0x0B, %eax
@@ -203,7 +202,6 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 
 .Lsetup_AP:
 	/* EAX contains the APICID of the current CPU */
-	andl	$0xFFFF, %eax
 	xorl	%ecx, %ecx
 	leaq	cpuid_to_apicid(%rip), %rbx
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 725fede281ac..acfb22ce8d4f 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1125,13 +1125,10 @@ static int do_boot_cpu(int apicid, int cpu, struct task_struct *idle,
 	if (IS_ENABLED(CONFIG_X86_32)) {
 		early_gdt_descr.address = (unsigned long)get_cpu_gdt_rw(cpu);
 		initial_stack  = idle->thread.sp;
-	} else if (boot_cpu_data.cpuid_level < 0x0B) {
-		/* Anything with X2APIC should have CPUID leaf 0x0B */
-		if (WARN_ON_ONCE(x2apic_mode) && apicid > 0xffff)
-			return -EIO;
-		smpboot_control = apicid | STARTUP_USE_APICID;
+	} else if (do_parallel_bringup) {
+		smpboot_control = STARTUP_PARALLEL;
 	} else {
-		smpboot_control = STARTUP_USE_CPUID_0B;
+		smpboot_control = apicid;
 	}
 
 	/* Enable the espfix hack for this CPU */
@@ -1553,9 +1550,11 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 
 	/*
 	 * We can do 64-bit AP bringup in parallel if the CPU reports its
-	 * APIC ID in CPUID leaf 0x0B. Otherwise it's too hard.
+	 * APIC ID in CPUID leaf 0x0B. Otherwise it's too hard. And not
+	 * for SEV-ES guests because they can't use CPUID that early.
 	 */
-	if (IS_ENABLED(CONFIG_X86_32) || boot_cpu_data.cpuid_level < 0x0B)
+	if (IS_ENABLED(CONFIG_X86_32) || boot_cpu_data.cpuid_level < 0x0B ||
+	    cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
 		do_parallel_bringup = false;
 
 	if (do_parallel_bringup)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]