Re: [patch 00/37] cpu/hotplug, x86: Reworked parallel CPU bringup
From: Thomas Gleixner
Date: Thu Apr 20 2023 - 15:10:52 EST
On Thu, Apr 20 2023 at 18:47, Paul Menzel wrote:
> Am 20.04.23 um 17:57 schrieb Thomas Gleixner:
> I quickly applied it on top of your branch, but I am getting:
As I said it was untested. I was traveling and did not have access to a
machine to even build it completely. Fixed up and tested version below.
Thanks,
tglx
---
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -138,7 +138,8 @@
#define APIC_EILVT_MASKED (1 << 16)
#define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
-#define APIC_BASE_MSR 0x800
+#define APIC_BASE_MSR 0x800
+#define APIC_X2APIC_ID_MSR 0x802
#define XAPIC_ENABLE (1UL << 11)
#define X2APIC_ENABLE (1UL << 10)
@@ -162,6 +163,7 @@
#define APIC_CPUID(apicid) ((apicid) & XAPIC_DEST_CPUS_MASK)
#define NUM_APIC_CLUSTERS ((BAD_APICID + 1) >> XAPIC_DEST_CPUS_SHIFT)
+#ifndef __ASSEMBLY__
/*
* the local APIC register structure, memory mapped. Not terribly well
* tested, but we might eventually use this one in the future - the
@@ -435,4 +437,5 @@ enum apic_delivery_modes {
APIC_DELIVERY_MODE_EXTINT = 7,
};
+#endif /* !__ASSEMBLY__ */
#endif /* _ASM_X86_APICDEF_H */
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -195,14 +195,13 @@ extern void nmi_selftest(void);
#endif
extern unsigned int smpboot_control;
+extern unsigned long apic_mmio_base;
#endif /* !__ASSEMBLY__ */
/* Control bits for startup_64 */
-#define STARTUP_APICID_CPUID_1F 0x80000000
-#define STARTUP_APICID_CPUID_0B 0x40000000
-#define STARTUP_APICID_CPUID_01 0x20000000
-#define STARTUP_APICID_SEV_ES 0x10000000
+#define STARTUP_READ_APICID 0x80000000
+#define STARTUP_APICID_SEV_ES 0x40000000
/* Top 8 bits are reserved for control */
#define STARTUP_PARALLEL_MASK 0xFF000000
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -101,6 +101,8 @@ static int apic_extnmi __ro_after_init =
*/
static bool virt_ext_dest_id __ro_after_init;
+unsigned long apic_mmio_base __ro_after_init;
+
/*
* Map cpu index to physical APIC ID
*/
@@ -2164,6 +2166,7 @@ void __init register_lapic_address(unsig
if (!x2apic_mode) {
set_fixmap_nocache(FIX_APIC_BASE, address);
+ apic_mmio_base = APIC_BASE;
apic_printk(APIC_VERBOSE, "mapped APIC to %16lx (%16lx)\n",
APIC_BASE, address);
}
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -24,8 +24,10 @@
#include "../entry/calling.h"
#include <asm/export.h>
#include <asm/nospec-branch.h>
+#include <asm/apicdef.h>
#include <asm/fixmap.h>
#include <asm/smp.h>
+
#include <asm/sev-common.h>
/*
@@ -237,37 +239,25 @@ SYM_INNER_LABEL(secondary_startup_64_no_
#ifdef CONFIG_SMP
/*
- * For parallel boot, the APIC ID is retrieved from CPUID, and then
- * used to look up the CPU number. For booting a single CPU, the
- * CPU number is encoded in smpboot_control.
+ * For parallel boot, the APIC ID is either retrieved the APIC or
+ * from CPUID, and then used to look up the CPU number.
+ * For booting a single CPU, the CPU number is encoded in
+ * smpboot_control.
*
- * Bit 31 STARTUP_APICID_CPUID_1F flag (use CPUID 0x1f)
- * Bit 30 STARTUP_APICID_CPUID_0B flag (use CPUID 0x0b)
- * Bit 29 STARTUP_APICID_CPUID_01 flag (use CPUID 0x01)
- * Bit 28 STARTUP_APICID_SEV_ES flag (CPUID 0x0b via GHCB MSR)
+ * Bit 31 STARTUP_APICID_READ (Read APICID from APIC)
+ * Bit 30 STARTUP_APICID_SEV_ES flag (CPUID 0x0b via GHCB MSR)
* Bit 0-23 CPU# if STARTUP_APICID_CPUID_xx flags are not set
*/
movl smpboot_control(%rip), %ecx
+ testl $STARTUP_READ_APICID, %ecx
+ jnz .Lread_apicid
#ifdef CONFIG_AMD_MEM_ENCRYPT
testl $STARTUP_APICID_SEV_ES, %ecx
jnz .Luse_sev_cpuid_0b
#endif
- testl $STARTUP_APICID_CPUID_1F, %ecx
- jnz .Luse_cpuid_1f
- testl $STARTUP_APICID_CPUID_0B, %ecx
- jnz .Luse_cpuid_0b
- testl $STARTUP_APICID_CPUID_01, %ecx
- jnz .Luse_cpuid_01
andl $(~STARTUP_PARALLEL_MASK), %ecx
jmp .Lsetup_cpu
-.Luse_cpuid_01:
- mov $0x01, %eax
- cpuid
- mov %ebx, %edx
- shr $24, %edx
- jmp .Lsetup_AP
-
#ifdef CONFIG_AMD_MEM_ENCRYPT
.Luse_sev_cpuid_0b:
/* Set the GHCB MSR to request CPUID 0x0B_EDX */
@@ -292,24 +282,30 @@ SYM_INNER_LABEL(secondary_startup_64_no_
jmp .Lsetup_AP
#endif
-.Luse_cpuid_0b:
- mov $0x0B, %eax
- xorl %ecx, %ecx
- cpuid
+.Lread_apicid:
+ mov $MSR_IA32_APICBASE, %ecx
+ rdmsr
+ testl $X2APIC_ENABLE, %eax
+ jnz .Lread_apicid_msr
+
+ /* Read the APIC ID from the fix-mapped MMIO space. */
+ movq apic_mmio_base(%rip), %rcx
+ addq $APIC_ID, %rcx
+ movl (%rcx), %eax
+ shr $24, %eax
jmp .Lsetup_AP
-.Luse_cpuid_1f:
- mov $0x1f, %eax
- xorl %ecx, %ecx
- cpuid
+.Lread_apicid_msr:
+ mov $APIC_X2APIC_ID_MSR, %ecx
+ rdmsr
.Lsetup_AP:
- /* EDX contains the APIC ID of the current CPU */
+ /* EAX contains the APIC ID of the current CPU */
xorq %rcx, %rcx
leaq cpuid_to_apicid(%rip), %rbx
.Lfind_cpunr:
- cmpl (%rbx,%rcx,4), %edx
+ cmpl (%rbx,%rcx,4), %eax
jz .Lsetup_cpu
inc %ecx
#ifdef CONFIG_FORCE_NR_CPUS
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1253,41 +1253,22 @@ bool __init arch_cpuhp_init_parallel_bri
return false;
}
- /* Encrypted guests require special CPUID handling. */
+ /* Encrypted guests require special handling. */
if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
switch (cc_get_vendor()) {
case CC_VENDOR_AMD:
ctrl = STARTUP_APICID_SEV_ES;
if (topology_extended_leaf == 0x0b)
- goto setup;
+ break;
fallthrough;
default:
pr_info("Parallel CPU startup disabled due to guest state encryption\n");
return false;
}
+ } else {
+ ctrl = STARTUP_READ_APICID;
}
- switch (topology_extended_leaf) {
- case 0x0b:
- ctrl = STARTUP_APICID_CPUID_0B;
- break;
- case 0x1f:
- ctrl = STARTUP_APICID_CPUID_1F;
- break;
- case 0x00:
- /* For !x2APIC mode 8 bits from leaf 0x01 are sufficient. */
- if (!x2apic_mode) {
- ctrl = STARTUP_APICID_CPUID_01;
- break;
- }
- fallthrough;
- default:
- pr_info("Parallel CPU startup disabled. Unsupported topology leaf %u\n",
- topology_extended_leaf);
- return false;
- }
-
-setup:
pr_debug("Parallel CPU startup enabled: 0x%08x\n", ctrl);
smpboot_control = ctrl;
return true;