2023-03-28 19:59:07

by Usama Arif

[permalink] [raw]
Subject: [PATCH v17 0/8] Parallel CPU bringup for x86_64

This version changes the number of cpuhp_states reserved for parallel bringup
from 4 (CPUHP_BP_PARALLEL_DYN to CPUHP_BP_PARALLEL_DYN_END) to 1
CPUHP_BP_PARALLEL_STARTUP.

Thanks,
Usama

Changes across versions:
v2: Cut it back to just INIT/SIPI/SIPI in parallel for now, nothing more
v3: Clean up x2apic patch, add MTRR optimisation, lock topology update
in preparation for more parallelisation.
v4: Fixes to the real mode parallelisation patch spotted by SeanC, to
avoid scribbling on initial_gs in common_cpu_up(), and to allow all
24 bits of the physical X2APIC ID to be used. That patch still needs
a Signed-off-by from its original author, who once claimed not to
remember writing it at all. But now we've fixed it, hopefully he'll
admit it now :)
v5: rebase to v6.1 and remeasure performance, disable parallel bringup
for AMD CPUs.
v6: rebase to v6.2-rc6, disabled parallel boot on amd as a cpu bug and
reused timer calibration for secondary CPUs.
v7: [David Woodhouse] iterate over all possible CPUs to find any existing
cluster mask in alloc_clustermask. (patch 1/9)
Keep parallel AMD support enabled in AMD, using APIC ID in CPUID leaf
0x0B (for x2APIC mode) or CPUID leaf 0x01 where 8 bits are sufficient.
Included sanity checks for APIC id from 0x0B. (patch 6/9)
Removed patch for reusing timer calibration for secondary CPUs.
commit message and code improvements.
v8: Fix CPU0 hotplug by setting up the initial_gs, initial_stack and
early_gdt_descr.
Drop trampoline lock and bail if APIC ID not found in find_cpunr.
Code comments improved and debug prints added.
v9: Drop patch to avoid repeated saves of MTRR at boot time.
rebased and retested at v6.2-rc8.
added kernel doc for no_parallel_bringup and made do_parallel_bringup
__ro_after_init.
v10: Fixed suspend/resume not working with parallel smpboot.
rebased and retested to 6.2.
fixed checkpatch errors.
v11: Added patches from Brian Gerst to remove the global variables initial_gs,
initial_stack, and early_gdt_descr from the 64-bit boot code
(https://lore.kernel.org/all/[email protected]/).
v12: Fixed compilation errors, acquire tr_lock for every stack setup in
trampoline_64.S.
Rearranged commits for a cleaner git history.
v13: Fix build error with CONFIG_FORCE_NR_CPUS.
Commit message improved, typos fixed and extra comments added.
v14: Enable parallel bringup for SEV-ES guests.
v15: use vendor parallel_smp when platform has CC_ATTR_GUEST_STATE_ENCRYPT.
Call smpboot_restore_warm_reset_vector incase any of the steps in
native_cpu_up fail.
Reset stale stack and kasan unpoison in bringup_cpu
Release trampoline_lock a bit earlier.
v16: Roll back to CPUHP_OFFLINE on failure in parallel bringup case.
Release trampoline_lock earlier, just before setup_gdt.
Rebase to x86/apic (Linux 6.3-rc3).
v17: Change number of states for parallel bringup to one.

David Woodhouse (8):
cpu/hotplug: Move idle_thread_get() to <linux/smpboot.h>
cpu/hotplug: Reset task stack state in _cpu_up()
cpu/hotplug: Add CPUHP_BP_PARALLEL_STARTUP state before
CPUHP_BRINGUP_CPU
x86/smpboot: Split up native_cpu_up into separate phases and document
them
x86/smpboot: Support parallel startup of secondary CPUs
x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel
x86/smpboot: Serialize topology updates for secondary bringup
x86/smpboot: Allow parallel bringup for SEV-ES

.../admin-guide/kernel-parameters.txt | 3 +
arch/x86/coco/core.c | 5 +
arch/x86/include/asm/coco.h | 1 +
arch/x86/include/asm/cpu.h | 1 +
arch/x86/include/asm/realmode.h | 3 +
arch/x86/include/asm/sev-common.h | 3 +
arch/x86/include/asm/smp.h | 13 +-
arch/x86/include/asm/topology.h | 2 -
arch/x86/kernel/acpi/sleep.c | 9 +-
arch/x86/kernel/apic/apic.c | 2 +-
arch/x86/kernel/cpu/common.c | 6 +-
arch/x86/kernel/cpu/topology.c | 3 +-
arch/x86/kernel/head_64.S | 97 +++++
arch/x86/kernel/smpboot.c | 345 +++++++++++++-----
arch/x86/realmode/init.c | 3 +
arch/x86/realmode/rm/trampoline_64.S | 27 +-
arch/x86/xen/smp_pv.c | 4 +-
include/linux/cpuhotplug.h | 22 ++
include/linux/smpboot.h | 7 +
kernel/cpu.c | 50 ++-
kernel/smpboot.h | 2 -
21 files changed, 492 insertions(+), 116 deletions(-)

--
2.25.1


2023-03-28 19:59:17

by Usama Arif

[permalink] [raw]
Subject: [PATCH v17 1/8] cpu/hotplug: Move idle_thread_get() to <linux/smpboot.h>

From: David Woodhouse <[email protected]>

Instead of relying purely on the special-case wrapper in bringup_cpu()
to pass the idle thread to __cpu_up(), expose idle_thread_get() so that
the architecture code can obtain it directly when necessary.

This will be useful when the existing __cpu_up() is split into multiple
phases, only *one* of which will actually need the idle thread.

If the architecture code is to register its new pre-bringup states with
the cpuhp core, having a special-case wrapper to pass extra arguments is
non-trivial and it's easier just to let the arch register its function
pointer to be invoked with the standard API.

Signed-off-by: David Woodhouse <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
Tested-by: Paul E. McKenney <[email protected]>
Tested-by: Kim Phillips <[email protected]>
Tested-by: Oleksandr Natalenko <[email protected]>
Tested-by: Guilherme G. Piccoli <[email protected]>
Reviewed-by: Mark Rutland <[email protected]>
Tested-by: Mark Rutland <[email protected]> [arm64]
---
include/linux/smpboot.h | 7 +++++++
kernel/smpboot.h | 2 --
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/smpboot.h b/include/linux/smpboot.h
index 9d1bc65d226c..3862addcaa34 100644
--- a/include/linux/smpboot.h
+++ b/include/linux/smpboot.h
@@ -5,6 +5,13 @@
#include <linux/types.h>

struct task_struct;
+
+#ifdef CONFIG_GENERIC_SMP_IDLE_THREAD
+struct task_struct *idle_thread_get(unsigned int cpu);
+#else
+static inline struct task_struct *idle_thread_get(unsigned int cpu) { return NULL; }
+#endif
+
/* Cookie handed to the thread_fn*/
struct smpboot_thread_data;

diff --git a/kernel/smpboot.h b/kernel/smpboot.h
index 34dd3d7ba40b..60c609318ad6 100644
--- a/kernel/smpboot.h
+++ b/kernel/smpboot.h
@@ -5,11 +5,9 @@
struct task_struct;

#ifdef CONFIG_GENERIC_SMP_IDLE_THREAD
-struct task_struct *idle_thread_get(unsigned int cpu);
void idle_thread_set_boot_cpu(void);
void idle_threads_init(void);
#else
-static inline struct task_struct *idle_thread_get(unsigned int cpu) { return NULL; }
static inline void idle_thread_set_boot_cpu(void) { }
static inline void idle_threads_init(void) { }
#endif
--
2.25.1

2023-03-28 20:00:28

by Usama Arif

[permalink] [raw]
Subject: [PATCH v17 7/8] x86/smpboot: Serialize topology updates for secondary bringup

From: David Woodhouse <[email protected]>

The toplogy update is performed by the AP via smp_callin() after the BSP
has called do_wait_cpu_initialized(), setting the AP's bit in
cpu_callout_mask to allow it to proceed.

In preparation to enable further parallelism of AP bringup, add locking to
serialize the update even if multiple APs are (in future) permitted to
proceed through the next stages of bringup in parallel.

Without such ordering (and with that future extra parallelism), confusion
ensues:

[ 1.360149] x86: Booting SMP configuration:
[ 1.360221] .... node #0, CPUs: #1 #2 #3 #4 #5 #6 #7 #8 #9 #10 #11 #12 #13 #14 #15 #16 #17 #18 #19 #20 #21 #22 #23
[ 1.366225] .... node #1, CPUs: #24 #25 #26 #27 #28 #29 #30 #31 #32 #33 #34 #35 #36 #37 #38 #39 #40 #41 #42 #43 #44 #45 #46 #47
[ 1.370219] .... node #0, CPUs: #48 #49 #50 #51 #52 #53 #54 #55 #56 #57 #58 #59 #60 #61 #62 #63 #64 #65 #66 #67 #68 #69 #70 #71
[ 1.378226] .... node #1, CPUs: #72 #73 #74 #75 #76 #77 #78 #79 #80 #81 #82 #83 #84 #85 #86 #87 #88 #89 #90 #91 #92 #93 #94 #95
[ 1.382037] Brought 96 CPUs to x86/cpu:kick in 72232606 cycles
[ 0.104104] smpboot: CPU 26 Converting physical 0 to logical die 1
[ 0.104104] smpboot: CPU 27 Converting physical 1 to logical package 2
[ 0.104104] smpboot: CPU 24 Converting physical 1 to logical package 3
[ 0.104104] smpboot: CPU 27 Converting physical 0 to logical die 2
[ 0.104104] smpboot: CPU 25 Converting physical 1 to logical package 4
[ 1.385609] Brought 96 CPUs to x86/cpu:wait-init in 9269218 cycles
[ 1.395285] Brought CPUs online in 28930764 cycles
[ 1.395469] smp: Brought up 2 nodes, 96 CPUs
[ 1.395689] smpboot: Max logical packages: 2
[ 1.396222] smpboot: Total of 96 processors activated (576000.00 BogoMIPS)

Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
Tested-by: Paul E. McKenney <[email protected]>
Tested-by: Kim Phillips <[email protected]>
Tested-by: Oleksandr Natalenko <[email protected]>
Tested-by: Guilherme G. Piccoli <[email protected]>
---
arch/x86/include/asm/smp.h | 4 +-
arch/x86/include/asm/topology.h | 2 -
arch/x86/kernel/cpu/common.c | 6 +--
arch/x86/kernel/smpboot.c | 73 ++++++++++++++++++++-------------
arch/x86/xen/smp_pv.c | 4 +-
5 files changed, 48 insertions(+), 41 deletions(-)

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 1cf4f1e57570..defe76ee9e64 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -48,8 +48,6 @@ struct smp_ops {
};

/* Globals due to paravirt */
-extern void set_cpu_sibling_map(int cpu);
-
#ifdef CONFIG_SMP
extern struct smp_ops smp_ops;

@@ -137,7 +135,7 @@ void native_send_call_func_single_ipi(int cpu);
void x86_idle_thread_init(unsigned int cpu, struct task_struct *idle);

void smp_store_boot_cpu_info(void);
-void smp_store_cpu_info(int id);
+void smp_store_cpu_info(int id, bool force_single_core);

asmlinkage __visible void smp_reboot_interrupt(void);
__visible void smp_reschedule_interrupt(struct pt_regs *regs);
diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 458c891a8273..4bccbd949a99 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -136,8 +136,6 @@ static inline int topology_max_smt_threads(void)
return __max_smt_threads;
}

-int topology_update_package_map(unsigned int apicid, unsigned int cpu);
-int topology_update_die_map(unsigned int dieid, unsigned int cpu);
int topology_phys_to_logical_pkg(unsigned int pkg);
int topology_phys_to_logical_die(unsigned int die, unsigned int cpu);
bool topology_is_primary_thread(unsigned int cpu);
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8cd4126d8253..80a688295ffa 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1776,7 +1776,7 @@ static void generic_identify(struct cpuinfo_x86 *c)
* Validate that ACPI/mptables have the same information about the
* effective APIC id and update the package map.
*/
-static void validate_apic_and_package_id(struct cpuinfo_x86 *c)
+static void validate_apic_id(struct cpuinfo_x86 *c)
{
#ifdef CONFIG_SMP
unsigned int apicid, cpu = smp_processor_id();
@@ -1787,8 +1787,6 @@ static void validate_apic_and_package_id(struct cpuinfo_x86 *c)
pr_err(FW_BUG "CPU%u: APIC id mismatch. Firmware: %x APIC: %x\n",
cpu, apicid, c->initial_apicid);
}
- BUG_ON(topology_update_package_map(c->phys_proc_id, cpu));
- BUG_ON(topology_update_die_map(c->cpu_die_id, cpu));
#else
c->logical_proc_id = 0;
#endif
@@ -1979,7 +1977,7 @@ void identify_secondary_cpu(struct cpuinfo_x86 *c)
#ifdef CONFIG_X86_32
enable_sep_cpu();
#endif
- validate_apic_and_package_id(c);
+ validate_apic_id(c);
x86_spec_ctrl_setup_ap();
update_srbds_msr();

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 0003f5e1740c..05d202b7dcaa 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -180,16 +180,12 @@ static void smp_callin(void)
apic_ap_setup();

/*
- * Save our processor parameters. Note: this information
- * is needed for clock calibration.
- */
- smp_store_cpu_info(cpuid);
-
- /*
+ * Save our processor parameters and update topology.
+ * Note: this information is needed for clock calibration.
* The topology information must be up to date before
* calibrate_delay() and notify_cpu_starting().
*/
- set_cpu_sibling_map(raw_smp_processor_id());
+ smp_store_cpu_info(cpuid, false);

ap_init_aperfmperf();

@@ -243,6 +239,12 @@ static void notrace start_secondary(void *unused)
* its bit in cpu_callout_mask to release it.
*/
cpu_init_secondary();
+
+ /*
+ * Even though notify_cpu_starting() will do this, it does so too late
+ * as the AP may already have triggered lockdep splats by then. See
+ * commit 29368e093 ("x86/smpboot: Move rcu_cpu_starting() earlier").
+ */
rcu_cpu_starting(raw_smp_processor_id());
x86_cpuinit.early_percpu_clock_init();

@@ -352,7 +354,7 @@ EXPORT_SYMBOL(topology_phys_to_logical_die);
* @pkg: The physical package id as retrieved via CPUID
* @cpu: The cpu for which this is updated
*/
-int topology_update_package_map(unsigned int pkg, unsigned int cpu)
+static int topology_update_package_map(unsigned int pkg, unsigned int cpu)
{
int new;

@@ -375,7 +377,7 @@ int topology_update_package_map(unsigned int pkg, unsigned int cpu)
* @die: The die id as retrieved via CPUID
* @cpu: The cpu for which this is updated
*/
-int topology_update_die_map(unsigned int die, unsigned int cpu)
+static int topology_update_die_map(unsigned int die, unsigned int cpu)
{
int new;

@@ -406,25 +408,7 @@ void __init smp_store_boot_cpu_info(void)
c->initialized = true;
}

-/*
- * The bootstrap kernel entry code has set these up. Save them for
- * a given CPU
- */
-void smp_store_cpu_info(int id)
-{
- struct cpuinfo_x86 *c = &cpu_data(id);
-
- /* Copy boot_cpu_data only on the first bringup */
- if (!c->initialized)
- *c = boot_cpu_data;
- c->cpu_index = id;
- /*
- * During boot time, CPU0 has this setup already. Save the info when
- * bringing up AP or offlined CPU0.
- */
- identify_secondary_cpu(c);
- c->initialized = true;
-}
+static arch_spinlock_t topology_lock = __ARCH_SPIN_LOCK_UNLOCKED;

static bool
topology_same_node(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
@@ -630,7 +614,7 @@ static struct sched_domain_topology_level x86_topology[] = {
*/
static bool x86_has_numa_in_package;

-void set_cpu_sibling_map(int cpu)
+static void set_cpu_sibling_map(int cpu)
{
bool has_smt = smp_num_siblings > 1;
bool has_mp = has_smt || boot_cpu_data.x86_max_cores > 1;
@@ -709,6 +693,37 @@ void set_cpu_sibling_map(int cpu)
}
}

+/*
+ * The bootstrap kernel entry code has set these up. Save them for
+ * a given CPU
+ */
+void smp_store_cpu_info(int id, bool force_single_core)
+{
+ struct cpuinfo_x86 *c = &cpu_data(id);
+
+ /* Copy boot_cpu_data only on the first bringup */
+ if (!c->initialized)
+ *c = boot_cpu_data;
+ c->cpu_index = id;
+ /*
+ * During boot time, CPU0 has this setup already. Save the info when
+ * bringing up AP or offlined CPU0.
+ */
+ identify_secondary_cpu(c);
+
+ arch_spin_lock(&topology_lock);
+ BUG_ON(topology_update_package_map(c->phys_proc_id, id));
+ BUG_ON(topology_update_die_map(c->cpu_die_id, id));
+ c->initialized = true;
+
+ /* For Xen PV */
+ if (force_single_core)
+ c->x86_max_cores = 1;
+
+ set_cpu_sibling_map(id);
+ arch_spin_unlock(&topology_lock);
+}
+
/* maps the cpu to the sched domain representing multi-core */
const struct cpumask *cpu_coregroup_mask(int cpu)
{
diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index a9cf8c8fa074..ea6c60514092 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -71,9 +71,7 @@ static void cpu_bringup(void)
xen_enable_syscall();
}
cpu = smp_processor_id();
- smp_store_cpu_info(cpu);
- cpu_data(cpu).x86_max_cores = 1;
- set_cpu_sibling_map(cpu);
+ smp_store_cpu_info(cpu, true);

speculative_store_bypass_ht_init();

--
2.25.1

2023-03-28 20:00:46

by Usama Arif

[permalink] [raw]
Subject: [PATCH v17 8/8] x86/smpboot: Allow parallel bringup for SEV-ES

From: David Woodhouse <[email protected]>

Enable parallel bringup for SEV-ES guests. The APs can't actually
execute the CPUID instruction directly during early startup, but they
can make the GHCB call directly instead, just as the VC trap handler
would do.

Thanks to Sabin for talking me through the way this works.

Suggested-by: Sabin Rapan <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
Reviewed-by: Tom Lendacky <[email protected]>
---
arch/x86/coco/core.c | 5 ++++
arch/x86/include/asm/coco.h | 1 +
arch/x86/include/asm/sev-common.h | 3 +++
arch/x86/include/asm/smp.h | 5 +++-
arch/x86/kernel/head_64.S | 30 ++++++++++++++++++++++++
arch/x86/kernel/smpboot.c | 39 ++++++++++++++++++++++++++-----
6 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index 49b44f881484..0bab38efb15a 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -129,6 +129,11 @@ u64 cc_mkdec(u64 val)
}
EXPORT_SYMBOL_GPL(cc_mkdec);

+enum cc_vendor cc_get_vendor(void)
+{
+ return vendor;
+}
+
__init void cc_set_vendor(enum cc_vendor v)
{
vendor = v;
diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
index 3d98c3a60d34..0428d9712c96 100644
--- a/arch/x86/include/asm/coco.h
+++ b/arch/x86/include/asm/coco.h
@@ -12,6 +12,7 @@ enum cc_vendor {
};

void cc_set_vendor(enum cc_vendor v);
+enum cc_vendor cc_get_vendor(void);
void cc_set_mask(u64 mask);

#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index b63be696b776..0abf8a39cee1 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -70,6 +70,7 @@
/* GHCBData[63:12] */ \
(((u64)(v) & GENMASK_ULL(63, 12)) >> 12)

+#ifndef __ASSEMBLY__
/*
* SNP Page State Change Operation
*
@@ -161,6 +162,8 @@ struct snp_psc_desc {

#define GHCB_RESP_CODE(v) ((v) & GHCB_MSR_INFO_MASK)

+#endif /* __ASSEMBLY__ */
+
/*
* Error codes related to GHCB input that can be communicated back to the guest
* by setting the lower 32-bits of the GHCB SW_EXITINFO1 field to 2.
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index defe76ee9e64..1584f04a7007 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -204,7 +204,10 @@ extern unsigned int smpboot_control;
/* Control bits for startup_64 */
#define STARTUP_APICID_CPUID_0B 0x80000000
#define STARTUP_APICID_CPUID_01 0x40000000
+#define STARTUP_APICID_SEV_ES 0x20000000

-#define STARTUP_PARALLEL_MASK (STARTUP_APICID_CPUID_01 | STARTUP_APICID_CPUID_0B)
+#define STARTUP_PARALLEL_MASK (STARTUP_APICID_CPUID_01 | \
+ STARTUP_APICID_CPUID_0B | \
+ STARTUP_APICID_SEV_ES)

#endif /* _ASM_X86_SMP_H */
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index ff3a5f008d8a..9c38849fcac8 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -26,6 +26,7 @@
#include <asm/nospec-branch.h>
#include <asm/fixmap.h>
#include <asm/smp.h>
+#include <asm/sev-common.h>

/*
* We are not able to switch in one step to the final KERNEL ADDRESS SPACE
@@ -242,6 +243,7 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
*
* Bit 31 STARTUP_APICID_CPUID_0B flag (use CPUID 0x0b)
* Bit 30 STARTUP_APICID_CPUID_01 flag (use CPUID 0x01)
+ * Bit 29 STARTUP_APICID_SEV_ES flag (CPUID 0x0b via GHCB MSR)
* Bit 0-24 CPU# if STARTUP_APICID_CPUID_xx flags are not set
*/
movl smpboot_control(%rip), %ecx
@@ -249,6 +251,10 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
jnz .Luse_cpuid_0b
testl $STARTUP_APICID_CPUID_01, %ecx
jnz .Luse_cpuid_01
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+ testl $STARTUP_APICID_SEV_ES, %ecx
+ jnz .Luse_sev_cpuid_0b
+#endif
andl $0x0FFFFFFF, %ecx
jmp .Lsetup_cpu

@@ -259,6 +265,30 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
shr $24, %edx
jmp .Lsetup_AP

+#ifdef CONFIG_AMD_MEM_ENCRYPT
+.Luse_sev_cpuid_0b:
+ /* Set the GHCB MSR to request CPUID 0xB_EDX */
+ movl $MSR_AMD64_SEV_ES_GHCB, %ecx
+ movl $(GHCB_CPUID_REQ_EDX << 30) | GHCB_MSR_CPUID_REQ, %eax
+ movl $0x0B, %edx
+ wrmsr
+
+ /* Perform GHCB MSR protocol */
+ rep; vmmcall /* vmgexit */
+
+ /*
+ * Get the result. After the RDMSR:
+ * EAX should be 0xc0000005
+ * EDX should have the CPUID register value and since EDX
+ * is the target register, no need to move the result.
+ */
+ rdmsr
+ andl $GHCB_MSR_INFO_MASK, %eax
+ cmpl $GHCB_MSR_CPUID_RESP, %eax
+ jne 1f
+ jmp .Lsetup_AP
+#endif
+
.Luse_cpuid_0b:
mov $0x0B, %eax
xorl %ecx, %ecx
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 05d202b7dcaa..6ef040fd28a0 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -85,6 +85,7 @@
#include <asm/hw_irq.h>
#include <asm/stackprotector.h>
#include <asm/sev.h>
+#include <asm/coco.h>

/* representing HT siblings of each logical CPU */
DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_map);
@@ -1514,15 +1515,36 @@ void __init smp_prepare_cpus_common(void)
* We can do 64-bit AP bringup in parallel if the CPU reports its APIC
* ID in CPUID (either leaf 0x0B if we need the full APIC ID in X2APIC
* mode, or leaf 0x01 if 8 bits are sufficient). Otherwise it's too
- * hard. And not for SEV-ES guests because they can't use CPUID that
- * early.
+ * hard.
*/
static bool prepare_parallel_bringup(void)
{
- if (IS_ENABLED(CONFIG_X86_32) || cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
+ bool has_sev_es = false;
+
+ if (IS_ENABLED(CONFIG_X86_32))
return false;

- if (x2apic_mode) {
+ /*
+ * Encrypted guests other than SEV-ES (in the future) will need to
+ * implement an early way of finding the APIC ID, since they will
+ * presumably block direct CPUID too. Be kind to our future selves
+ * by warning here instead of just letting them break. Parallel
+ * startup doesn't have to be in the first round of enabling patches
+ * for any such technology.
+ */
+ if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
+ switch (cc_get_vendor()) {
+ case CC_VENDOR_AMD:
+ has_sev_es = true;
+ break;
+
+ default:
+ pr_info("Disabling parallel bringup due to guest state encryption\n");
+ return false;
+ }
+ }
+
+ if (x2apic_mode || has_sev_es) {
if (boot_cpu_data.cpuid_level < 0x0b)
return false;

@@ -1531,8 +1553,13 @@ static bool prepare_parallel_bringup(void)
return false;
}

- pr_debug("Using CPUID 0xb for parallel CPU startup\n");
- smpboot_control = STARTUP_APICID_CPUID_0B;
+ if (has_sev_es) {
+ pr_debug("Using SEV-ES CPUID 0xb for parallel CPU startup\n");
+ smpboot_control = STARTUP_APICID_SEV_ES;
+ } else {
+ pr_debug("Using CPUID 0xb for parallel CPU startup\n");
+ smpboot_control = STARTUP_APICID_CPUID_0B;
+ }
} else {
/* Without X2APIC, what's in CPUID 0x01 should suffice. */
if (boot_cpu_data.cpuid_level < 0x01)
--
2.25.1

2023-03-28 20:01:58

by Usama Arif

[permalink] [raw]
Subject: [PATCH v17 6/8] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel

From: David Woodhouse <[email protected]>

When the APs can find their own APIC ID without assistance, perform the
AP bringup in parallel.

Register a CPUHP_BP_PARALLEL_STARTUP stage "x86/cpu:kick" which just
calls do_boot_cpu() to deliver INIT/SIPI/SIPI to each AP in turn before
the normal native_cpu_up() does the rest of the hand-holding.

The APs will then take turns through the real mode code (which has its
own bitlock for exclusion) until they make it to their own stack, then
proceed through the first few lines of start_secondary() and execute
these parts in parallel:

start_secondary()
-> cr4_init()
-> (some 32-bit only stuff so not in the parallel cases)
-> cpu_init_secondary()
-> cpu_init_exception_handling()
-> cpu_init()
-> wait_for_master_cpu()

At this point they wait for the BSP to set their bit in cpu_callout_mask
(from do_wait_cpu_initialized()), and release them to continue through
the rest of cpu_init() and beyond.

This reduces the time taken for bringup on my 28-thread Haswell system
from about 120ms to 80ms. On a socket 96-thread Skylake it takes the
bringup time from 500ms to 100ms.

Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
Tested-by: Paul E. McKenney <[email protected]>
Tested-by: Kim Phillips <[email protected]>
Tested-by: Oleksandr Natalenko <[email protected]>
Tested-by: Guilherme G. Piccoli <[email protected]>
---
arch/x86/kernel/smpboot.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 45f3d08321fe..0003f5e1740c 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -57,6 +57,7 @@
#include <linux/pgtable.h>
#include <linux/overflow.h>
#include <linux/stackprotector.h>
+#include <linux/smpboot.h>

#include <asm/acpi.h>
#include <asm/cacheinfo.h>
@@ -993,7 +994,8 @@ static void announce_cpu(int cpu, int apicid)
node_width = num_digits(num_possible_nodes()) + 1; /* + '#' */

if (cpu == 1)
- printk(KERN_INFO "x86: Booting SMP configuration:\n");
+ printk(KERN_INFO "x86: Booting SMP configuration in %s:\n",
+ do_parallel_bringup ? "parallel" : "series");

if (system_state < SYSTEM_RUNNING) {
if (node != current_node) {
@@ -1326,9 +1328,12 @@ int native_cpu_up(unsigned int cpu, struct task_struct *tidle)
{
int ret;

- ret = do_cpu_up(cpu, tidle);
- if (ret)
- goto out;
+ /* If parallel AP bringup isn't enabled, perform the first steps now. */
+ if (!do_parallel_bringup) {
+ ret = do_cpu_up(cpu, tidle);
+ if (ret)
+ goto out;
+ }

ret = do_wait_cpu_initialized(cpu);
if (ret)
@@ -1348,6 +1353,12 @@ int native_cpu_up(unsigned int cpu, struct task_struct *tidle)
return ret;
}

+/* Bringup step one: Send INIT/SIPI to the target AP */
+static int native_cpu_kick(unsigned int cpu)
+{
+ return do_cpu_up(cpu, idle_thread_get(cpu));
+}
+
/**
* arch_disable_smp_support() - disables SMP support for x86 at runtime
*/
@@ -1516,6 +1527,8 @@ static bool prepare_parallel_bringup(void)
smpboot_control = STARTUP_APICID_CPUID_01;
}

+ cpuhp_setup_state_nocalls(CPUHP_BP_PARALLEL_STARTUP, "x86/cpu:kick",
+ native_cpu_kick, NULL);
return true;
}

--
2.25.1

2023-03-28 20:02:05

by Usama Arif

[permalink] [raw]
Subject: [PATCH v17 3/8] cpu/hotplug: Add CPUHP_BP_PARALLEL_STARTUP state before CPUHP_BRINGUP_CPU

From: David Woodhouse <[email protected]>

There is often significant latency in the early stages of CPU bringup,
and time is wasted by waking each CPU (e.g. with SIPI/INIT/INIT on x86)
and then waiting for it to make its way through hardware powerup and
through firmware before finally reaching the kernel entry point and
moving on through its startup.

Allow a platform to register a pre-bringup CPUHP state to which each
CPU can be stepped in parallel, thus absorbing some of that latency.

There is a subtlety here: even with an empty CPUHP_BP_PARALLEL_STARTUP
step, this means that *all* CPUs are brought through the prepare states
all the way to CPUHP_BP_PARALLEL_STARTUP before any of them are taken
to CPUHP_BRINGUP_CPU and then are allowed to run for themselves to
CPUHP_ONLINE.

So any combination of prepare/start calls which depend on A-B ordering
for each CPU in turn would explore horribly. As an example, the X2APIC
code prior to commit cefad862f238 ("x86/apic/x2apic: Allow CPU
cluster_mask to be populated in parallel") would allocate a new cluster
mask "just in case" and store it in a global variable in the prep stage,
then the AP would potentially consume that preallocated structure and set
the global pointer to NULL to be reallocated in CPUHP_X2APIC_PREPARE for
the next CPU. Which doesn't work at all if the prepare step is run for
all the CPUs first.

Any platform enabling the CPUHP_BP_PARALLEL_STARTUP step must be
reviewed and tested to ensure that such issues do not exist, and the
existing behaviour of each AP through to CPUHP_BP_PREPARE_DYN and then
immediately to CPUHP_BRINGUP_CPU and CPUHP_ONLINE only one at a time
does not change unless such a state is registered.

Note that this does *not* yet bring each AP to the CPUHP_BRINGUP_CPU
state at the same time, only to the new CPUHP_BP_PARALLEL_STARTUP state.
The final loop in bringup_nonboot_cpus() remains the same, bringing each
AP in turn from the CPUHP_BP_PARALLEL_STARTUP (or all the way from
CPUHP_OFFLINE) to CPUHP_BRINGUP_CPU and then waiting for that AP to do
its own processing and reach CPUHP_ONLINE before releasing the next.

Parallelising that part by bringing them all to CPUHP_BRINGUP_CPU and
then waiting for them all to run to CPUHP_ONLINE at the same time is a
more complicated exercise for the future.

Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
Tested-by: Paul E. McKenney <[email protected]>
Tested-by: Kim Phillips <[email protected]>
Tested-by: Oleksandr Natalenko <[email protected]>
Tested-by: Guilherme G. Piccoli <[email protected]>
Reviewed-by: Mark Rutland <[email protected]>
Tested-by: Mark Rutland <[email protected]> [arm64]
---
include/linux/cpuhotplug.h | 22 ++++++++++++++++++++++
kernel/cpu.c | 38 +++++++++++++++++++++++++++++++++++---
2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index c6fab004104a..84efd33ed3a3 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -133,6 +133,28 @@ enum cpuhp_state {
CPUHP_MIPS_SOC_PREPARE,
CPUHP_BP_PREPARE_DYN,
CPUHP_BP_PREPARE_DYN_END = CPUHP_BP_PREPARE_DYN + 20,
+ /*
+ * This is an optional state if the architecture supports parallel
+ * startup. It's used to start bringing the CPU online (e.g. send
+ * the startup IPI) so that the APs can run in parallel through
+ * the low level startup code instead of waking them one by one in
+ * CPUHP_BRINGUP_CPU. This avoids waiting for the AP to react and
+ * shortens the serialized phase of the bringup.
+ *
+ * If the architecture registers this state, all APs will be taken
+ * to it (and thus through all prior states) before any is taken
+ * to the subsequent CPUHP_BRINGUP_CPU state.
+ */
+ CPUHP_BP_PARALLEL_STARTUP,
+
+ /*
+ * This step brings the AP online and takes it to the point where it
+ * manages its own state from here on. For the time being, the rest
+ * of the AP bringup is fully serialized despite running on the AP.
+ * If the architecture doesn't use the CPUHP_BP_PARALLEL_STARTUP
+ * state, this step also does all the work of bringing the CPU
+ * online.
+ */
CPUHP_BRINGUP_CPU,

/*
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 43e0a77f21e8..3382273ea3f4 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1504,13 +1504,45 @@ int bringup_hibernate_cpu(unsigned int sleep_cpu)

void bringup_nonboot_cpus(unsigned int setup_max_cpus)
{
- unsigned int cpu;
+ unsigned int cpu, n = num_online_cpus();

+ /*
+ * On architectures which have setup the CPUHP_BP_PARALLEL_STARTUP
+ * state, this invokes all BP prepare states and the parallel
+ * startup state sends the startup IPI to each of the to be onlined
+ * APs. This avoids waiting for each AP to respond to the startup
+ * IPI in CPUHP_BRINGUP_CPU. The APs proceed through the low level
+ * bringup code and then wait for the control CPU to release them
+ * one by one for the final onlining procedure in the loop below.
+ *
+ * For architectures which do not support parallel bringup all
+ * states are fully serialized in the loop below.
+ */
+ if (!cpuhp_step_empty(true, cpuhp_get_step(CPUHP_BP_PARALLEL_STARTUP))) {
+ for_each_present_cpu(cpu) {
+ if (n++ >= setup_max_cpus)
+ break;
+ cpu_up(cpu, CPUHP_BP_PARALLEL_STARTUP);
+ }
+ }
+
+ /* Do the per CPU serialized bringup to ONLINE state */
for_each_present_cpu(cpu) {
if (num_online_cpus() >= setup_max_cpus)
break;
- if (!cpu_online(cpu))
- cpu_up(cpu, CPUHP_ONLINE);
+
+ if (!cpu_online(cpu)) {
+ struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu);
+ int ret = cpu_up(cpu, CPUHP_ONLINE);
+
+ /*
+ * Due to the above preparation loop a failed online attempt
+ * might have only rolled back to CPUHP_BP_PARALLEL_STARTUP. Do the
+ * remaining cleanups. NOOP for the non parallel case.
+ */
+ if (ret && can_rollback_cpu(st))
+ WARN_ON(cpuhp_invoke_callback_range(false, cpu, st, CPUHP_OFFLINE));
+ }
}
}

--
2.25.1

2023-03-28 20:02:15

by Usama Arif

[permalink] [raw]
Subject: [PATCH v17 2/8] cpu/hotplug: Reset task stack state in _cpu_up()

From: David Woodhouse <[email protected]>

Commit dce1ca0525bf ("sched/scs: Reset task stack state in bringup_cpu()")
ensured that the shadow call stack was reset and KASAN poisoning removed
from a CPU's stack each time that CPU is brought up, not just once.

This is not incorrect. However, with parallel bringup, an architecture
may obtain the idle thread for a new CPU from a pre-bringup stage, by
calling idle_thread_get() for itself. This would mean that the cleanup
in bringup_cpu() would be too late.

Move the SCS/KASAN cleanup to the generic _cpu_up() function instead,
which already ensures that the new CPU's stack is available, purely to
allow for early failure. This occurs when the CPU to be brought up is
in the CPUHP_OFFLINE state, which should correctly do the cleanup any
time the CPU has been taken down to the point where such is needed.

Signed-off-by: David Woodhouse <[email protected]>
Reviewed-by: Mark Rutland <[email protected]>
Tested-by: Mark Rutland <[email protected]> [arm64]
---
kernel/cpu.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 6c0a92ca6bb5..43e0a77f21e8 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -591,12 +591,6 @@ static int bringup_cpu(unsigned int cpu)
struct task_struct *idle = idle_thread_get(cpu);
int ret;

- /*
- * Reset stale stack state from the last time this CPU was online.
- */
- scs_task_reset(idle);
- kasan_unpoison_task_stack(idle);
-
/*
* Some architectures have to walk the irq descriptors to
* setup the vector space for the cpu which comes online.
@@ -1383,6 +1377,12 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target)
ret = PTR_ERR(idle);
goto out;
}
+
+ /*
+ * Reset stale stack state from the last time this CPU was online.
+ */
+ scs_task_reset(idle);
+ kasan_unpoison_task_stack(idle);
}

cpuhp_tasks_frozen = tasks_frozen;
--
2.25.1

2023-03-28 20:03:40

by Usama Arif

[permalink] [raw]
Subject: [PATCH v17 5/8] x86/smpboot: Support parallel startup of secondary CPUs

From: David Woodhouse <[email protected]>

Rework the real-mode startup code to allow for APs to be brought up in
parallel. This is in two parts:

1. Introduce a bit-spinlock to prevent them from all using the real
mode stack at the same time.

2. Avoid needing to use the global smpboot_control variable to pass
each AP its CPU#.

To achieve the latter, export the cpuid_to_apicid[] array so that each
AP can find its own CPU# by searching therein based on its APIC ID.

Introduce flags in the top bits of smpboot_control which indicate methods
by which an AP should find its CPU#. For a serialized bringup, the CPU#
is explicitly passed in the low bits of smpboot_control as before. For
parallel mode there are flags directing the AP to find its APIC ID in
CPUID leaf 0x0b (for X2APIC mode) or CPUID leaf 0x01 where 8 bits are
sufficient, then perform the cpuid_to_apicid[] lookup with that.

Parallel startup may be disabled by a command line option, and also if:
• AMD SEV-ES is in use, since the AP may not use CPUID that early.
• X2APIC is enabled, but CPUID leaf 0xb is not present and correct.
• X2APIC is not enabled but not even CPUID leaf 0x01 exists.

Aside from the fact that APs will now look up their CPU# via the
newly-exported cpuid_to_apicid[] table, there is no behavioural change
intended yet, since new parallel CPUHP states have not — yet — been
added.

[ tglx: Initial proof of concept patch with bitlock and APIC ID lookup ]
[ dwmw2: Rework and testing, commit message, CPUID 0x1 and CPU0 support ]
[ seanc: Fix stray override of initial_gs in common_cpu_up() ]
[ Oleksandr Natalenko: reported suspend/resume issue fixed in
x86_acpi_suspend_lowlevel ]
Co-developed-by: Thomas Gleixner <[email protected]>
Co-developed-by: Brian Gerst <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Brian Gerst <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
Tested-by: Paul E. McKenney <[email protected]>
Tested-by: Kim Phillips <[email protected]>
Tested-by: Oleksandr Natalenko <[email protected]>
Tested-by: Guilherme G. Piccoli <[email protected]>
---
.../admin-guide/kernel-parameters.txt | 3 +
arch/x86/include/asm/cpu.h | 1 +
arch/x86/include/asm/realmode.h | 3 +
arch/x86/include/asm/smp.h | 6 ++
arch/x86/kernel/acpi/sleep.c | 9 ++-
arch/x86/kernel/apic/apic.c | 2 +-
arch/x86/kernel/cpu/topology.c | 3 +-
arch/x86/kernel/head_64.S | 67 +++++++++++++++++++
arch/x86/kernel/smpboot.c | 50 +++++++++++++-
arch/x86/realmode/init.c | 3 +
arch/x86/realmode/rm/trampoline_64.S | 27 ++++++--
11 files changed, 165 insertions(+), 9 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 6221a1d057dd..424151f296ff 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3822,6 +3822,9 @@

nomodule Disable module load

+ no_parallel_bringup
+ [X86,SMP] Disable parallel bring-up of secondary cores.
+
nopat [X86] Disable PAT (page attribute table extension of
pagetables) support.

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 78796b98a544..ef8ba318dca1 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -97,5 +97,6 @@ static inline bool intel_cpu_signatures_match(unsigned int s1, unsigned int p1,
extern u64 x86_read_arch_cap_msr(void);
int intel_find_matching_signature(void *mc, unsigned int csig, int cpf);
int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type);
+int check_extended_topology_leaf(int leaf);

#endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/include/asm/realmode.h b/arch/x86/include/asm/realmode.h
index f6a1737c77be..87e5482acd0d 100644
--- a/arch/x86/include/asm/realmode.h
+++ b/arch/x86/include/asm/realmode.h
@@ -52,6 +52,7 @@ struct trampoline_header {
u64 efer;
u32 cr4;
u32 flags;
+ u32 lock;
#endif
};

@@ -64,6 +65,8 @@ extern unsigned long initial_stack;
extern unsigned long initial_vc_handler;
#endif

+extern u32 *trampoline_lock;
+
extern unsigned char real_mode_blob[];
extern unsigned char real_mode_relocs[];

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index bf2c51df9e0b..1cf4f1e57570 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -203,4 +203,10 @@ extern unsigned int smpboot_control;

#endif /* !__ASSEMBLY__ */

+/* Control bits for startup_64 */
+#define STARTUP_APICID_CPUID_0B 0x80000000
+#define STARTUP_APICID_CPUID_01 0x40000000
+
+#define STARTUP_PARALLEL_MASK (STARTUP_APICID_CPUID_01 | STARTUP_APICID_CPUID_0B)
+
#endif /* _ASM_X86_SMP_H */
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 1328c221af30..6dfecb27b846 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -16,6 +16,7 @@
#include <asm/cacheflush.h>
#include <asm/realmode.h>
#include <asm/hypervisor.h>
+#include <asm/smp.h>

#include <linux/ftrace.h>
#include "../../realmode/rm/wakeup.h"
@@ -127,7 +128,13 @@ int x86_acpi_suspend_lowlevel(void)
* value is in the actual %rsp register.
*/
current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack);
- smpboot_control = smp_processor_id();
+ /*
+ * Ensure the CPU knows which one it is when it comes back, if
+ * it isn't in parallel mode and expected to work that out for
+ * itself.
+ */
+ if (!(smpboot_control & STARTUP_PARALLEL_MASK))
+ smpboot_control = smp_processor_id();
#endif
initial_code = (unsigned long)wakeup_long64;
saved_magic = 0x123456789abcdef0L;
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 20d9a604da7c..ac1d7e5da1f2 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2377,7 +2377,7 @@ static int nr_logical_cpuids = 1;
/*
* Used to store mapping between logical CPU IDs and APIC IDs.
*/
-static int cpuid_to_apicid[] = {
+int cpuid_to_apicid[] = {
[0 ... NR_CPUS - 1] = -1,
};

diff --git a/arch/x86/kernel/cpu/topology.c b/arch/x86/kernel/cpu/topology.c
index 5e868b62a7c4..4373442e500a 100644
--- a/arch/x86/kernel/cpu/topology.c
+++ b/arch/x86/kernel/cpu/topology.c
@@ -9,6 +9,7 @@
#include <asm/apic.h>
#include <asm/memtype.h>
#include <asm/processor.h>
+#include <asm/cpu.h>

#include "cpu.h"

@@ -32,7 +33,7 @@ EXPORT_SYMBOL(__max_die_per_package);
/*
* Check if given CPUID extended topology "leaf" is implemented
*/
-static int check_extended_topology_leaf(int leaf)
+int check_extended_topology_leaf(int leaf)
{
unsigned int eax, ebx, ecx, edx;

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 6a8238702eab..ff3a5f008d8a 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -25,6 +25,7 @@
#include <asm/export.h>
#include <asm/nospec-branch.h>
#include <asm/fixmap.h>
+#include <asm/smp.h>

/*
* We are not able to switch in one step to the final KERNEL ADDRESS SPACE
@@ -234,8 +235,61 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
ANNOTATE_NOENDBR // above

#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.
+ *
+ * Bit 31 STARTUP_APICID_CPUID_0B flag (use CPUID 0x0b)
+ * Bit 30 STARTUP_APICID_CPUID_01 flag (use CPUID 0x01)
+ * Bit 0-24 CPU# if STARTUP_APICID_CPUID_xx flags are not set
+ */
movl smpboot_control(%rip), %ecx
+ testl $STARTUP_APICID_CPUID_0B, %ecx
+ jnz .Luse_cpuid_0b
+ testl $STARTUP_APICID_CPUID_01, %ecx
+ jnz .Luse_cpuid_01
+ andl $0x0FFFFFFF, %ecx
+ jmp .Lsetup_cpu
+
+.Luse_cpuid_01:
+ mov $0x01, %eax
+ cpuid
+ mov %ebx, %edx
+ shr $24, %edx
+ jmp .Lsetup_AP

+.Luse_cpuid_0b:
+ mov $0x0B, %eax
+ xorl %ecx, %ecx
+ cpuid
+
+.Lsetup_AP:
+ /* EDX contains the APIC ID of the current CPU */
+ xorq %rcx, %rcx
+ leaq cpuid_to_apicid(%rip), %rbx
+
+.Lfind_cpunr:
+ cmpl (%rbx,%rcx,4), %edx
+ jz .Lsetup_cpu
+ inc %ecx
+#ifdef CONFIG_FORCE_NR_CPUS
+ cmpl $NR_CPUS, %ecx
+#else
+ cmpl nr_cpu_ids(%rip), %ecx
+#endif
+ jb .Lfind_cpunr
+
+ /* APIC ID not found in the table. Drop the trampoline lock and bail. */
+ movq trampoline_lock(%rip), %rax
+ lock
+ btrl $0, (%rax)
+
+1: cli
+ hlt
+ jmp 1b
+
+.Lsetup_cpu:
/* Get the per cpu offset for the given CPU# which is in ECX */
movq __per_cpu_offset(,%rcx,8), %rdx
#else
@@ -251,6 +305,17 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
movq pcpu_hot + X86_current_task(%rdx), %rax
movq TASK_threadsp(%rax), %rsp

+ /*
+ * Now that this CPU is running on its own stack, drop the realmode
+ * protection. For the boot CPU the pointer is NULL!
+ */
+ movq trampoline_lock(%rip), %rax
+ testq %rax, %rax
+ jz .Lsetup_gdt
+ lock
+ btrl $0, (%rax)
+
+.Lsetup_gdt:
/*
* We must switch to a new descriptor in kernel space for the GDT
* because soon the kernel won't have access anymore to the userspace
@@ -435,6 +500,8 @@ SYM_DATA(initial_code, .quad x86_64_start_kernel)
#ifdef CONFIG_AMD_MEM_ENCRYPT
SYM_DATA(initial_vc_handler, .quad handle_vc_boot_ghcb)
#endif
+
+SYM_DATA(trampoline_lock, .quad 0);
__FINITDATA

__INIT
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 177ac98f5721..45f3d08321fe 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -798,6 +798,16 @@ static int __init cpu_init_udelay(char *str)
}
early_param("cpu_init_udelay", cpu_init_udelay);

+static bool do_parallel_bringup __ro_after_init = true;
+
+static int __init no_parallel_bringup(char *str)
+{
+ do_parallel_bringup = false;
+
+ return 0;
+}
+early_param("no_parallel_bringup", no_parallel_bringup);
+
static void __init smp_quirk_init_udelay(void)
{
/* if cmdline changed it from default, leave it alone */
@@ -1114,7 +1124,7 @@ 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 {
+ } else if (!do_parallel_bringup) {
smpboot_control = cpu;
}

@@ -1474,6 +1484,41 @@ void __init smp_prepare_cpus_common(void)
set_cpu_sibling_map(0);
}

+/*
+ * We can do 64-bit AP bringup in parallel if the CPU reports its APIC
+ * ID in CPUID (either leaf 0x0B if we need the full APIC ID in X2APIC
+ * mode, or leaf 0x01 if 8 bits are sufficient). Otherwise it's too
+ * hard. And not for SEV-ES guests because they can't use CPUID that
+ * early.
+ */
+static bool prepare_parallel_bringup(void)
+{
+ if (IS_ENABLED(CONFIG_X86_32) || cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
+ return false;
+
+ if (x2apic_mode) {
+ if (boot_cpu_data.cpuid_level < 0x0b)
+ return false;
+
+ if (check_extended_topology_leaf(0x0b) != 0) {
+ pr_info("Disabling parallel bringup because CPUID 0xb looks untrustworthy\n");
+ return false;
+ }
+
+ pr_debug("Using CPUID 0xb for parallel CPU startup\n");
+ smpboot_control = STARTUP_APICID_CPUID_0B;
+ } else {
+ /* Without X2APIC, what's in CPUID 0x01 should suffice. */
+ if (boot_cpu_data.cpuid_level < 0x01)
+ return false;
+
+ pr_debug("Using CPUID 0x1 for parallel CPU startup\n");
+ smpboot_control = STARTUP_APICID_CPUID_01;
+ }
+
+ return true;
+}
+
/*
* Prepare for SMP bootup.
* @max_cpus: configured maximum number of CPUs, It is a legacy parameter
@@ -1514,6 +1559,9 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)

speculative_store_bypass_ht_init();

+ if (do_parallel_bringup)
+ do_parallel_bringup = prepare_parallel_bringup();
+
snp_set_wakeup_secondary_cpu();
}

diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
index af565816d2ba..788e5559549f 100644
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -154,6 +154,9 @@ static void __init setup_real_mode(void)

trampoline_header->flags = 0;

+ trampoline_lock = &trampoline_header->lock;
+ *trampoline_lock = 0;
+
trampoline_pgd = (u64 *) __va(real_mode_header->trampoline_pgd);

/* Map the real mode stub as virtual == physical */
diff --git a/arch/x86/realmode/rm/trampoline_64.S b/arch/x86/realmode/rm/trampoline_64.S
index e38d61d6562e..2dfb1c400167 100644
--- a/arch/x86/realmode/rm/trampoline_64.S
+++ b/arch/x86/realmode/rm/trampoline_64.S
@@ -37,6 +37,24 @@
.text
.code16

+.macro LOAD_REALMODE_ESP
+ /*
+ * Make sure only one CPU fiddles with the realmode stack
+ */
+.Llock_rm\@:
+ btl $0, tr_lock
+ jnc 2f
+ pause
+ jmp .Llock_rm\@
+2:
+ lock
+ btsl $0, tr_lock
+ jc .Llock_rm\@
+
+ # Setup stack
+ movl $rm_stack_end, %esp
+.endm
+
.balign PAGE_SIZE
SYM_CODE_START(trampoline_start)
cli # We should be safe anyway
@@ -49,8 +67,7 @@ SYM_CODE_START(trampoline_start)
mov %ax, %es
mov %ax, %ss

- # Setup stack
- movl $rm_stack_end, %esp
+ LOAD_REALMODE_ESP

call verify_cpu # Verify the cpu supports long mode
testl %eax, %eax # Check for return code
@@ -93,8 +110,7 @@ SYM_CODE_START(sev_es_trampoline_start)
mov %ax, %es
mov %ax, %ss

- # Setup stack
- movl $rm_stack_end, %esp
+ LOAD_REALMODE_ESP

jmp .Lswitch_to_protected
SYM_CODE_END(sev_es_trampoline_start)
@@ -177,7 +193,7 @@ SYM_CODE_START(pa_trampoline_compat)
* In compatibility mode. Prep ESP and DX for startup_32, then disable
* paging and complete the switch to legacy 32-bit mode.
*/
- movl $rm_stack_end, %esp
+ LOAD_REALMODE_ESP
movw $__KERNEL_DS, %dx

movl $(CR0_STATE & ~X86_CR0_PG), %eax
@@ -241,6 +257,7 @@ SYM_DATA_START(trampoline_header)
SYM_DATA(tr_efer, .space 8)
SYM_DATA(tr_cr4, .space 4)
SYM_DATA(tr_flags, .space 4)
+ SYM_DATA(tr_lock, .space 4)
SYM_DATA_END(trampoline_header)

#include "trampoline_common.S"
--
2.25.1

2023-03-28 20:11:30

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v17 8/8] x86/smpboot: Allow parallel bringup for SEV-ES

On Tue, Mar 28, 2023 at 08:57:58PM +0100, Usama Arif wrote:
> From: David Woodhouse <[email protected]>
>
> Enable parallel bringup for SEV-ES guests. The APs can't actually
> execute the CPUID instruction directly during early startup, but they
> can make the GHCB call directly instead, just as the VC trap handler
> would do.
>
> Thanks to Sabin for talking me through the way this works.
>
> Suggested-by: Sabin Rapan <[email protected]>
> Signed-off-by: David Woodhouse <[email protected]>
> Signed-off-by: Usama Arif <[email protected]>
> Reviewed-by: Tom Lendacky <[email protected]>
> ---
> arch/x86/coco/core.c | 5 ++++
> arch/x86/include/asm/coco.h | 1 +
> arch/x86/include/asm/sev-common.h | 3 +++
> arch/x86/include/asm/smp.h | 5 +++-
> arch/x86/kernel/head_64.S | 30 ++++++++++++++++++++++++
> arch/x86/kernel/smpboot.c | 39 ++++++++++++++++++++++++++-----
> 6 files changed, 76 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
> index 49b44f881484..0bab38efb15a 100644
> --- a/arch/x86/coco/core.c
> +++ b/arch/x86/coco/core.c
> @@ -129,6 +129,11 @@ u64 cc_mkdec(u64 val)
> }
> EXPORT_SYMBOL_GPL(cc_mkdec);
>
> +enum cc_vendor cc_get_vendor(void)
> +{
> + return vendor;
> +}
> +
> __init void cc_set_vendor(enum cc_vendor v)
> {
> vendor = v;
> diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
> index 3d98c3a60d34..0428d9712c96 100644
> --- a/arch/x86/include/asm/coco.h
> +++ b/arch/x86/include/asm/coco.h
> @@ -12,6 +12,7 @@ enum cc_vendor {
> };
>
> void cc_set_vendor(enum cc_vendor v);
> +enum cc_vendor cc_get_vendor(void);
> void cc_set_mask(u64 mask);
>
> #ifdef CONFIG_ARCH_HAS_CC_PLATFORM

You don't need those hunks adding cc_set_vendor() anymore:

https://git.kernel.org/tip/5ae57743f578725a5dadb6f31d7798ee55e6e967

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-03-29 11:19:24

by Usama Arif

[permalink] [raw]
Subject: Re: [PATCH v17 2/8] cpu/hotplug: Reset task stack state in _cpu_up()



On 28/03/2023 20:57, Usama Arif wrote:
> From: David Woodhouse <[email protected]>
>
> Commit dce1ca0525bf ("sched/scs: Reset task stack state in bringup_cpu()")
> ensured that the shadow call stack was reset and KASAN poisoning removed
> from a CPU's stack each time that CPU is brought up, not just once.
>
> This is not incorrect. However, with parallel bringup, an architecture
> may obtain the idle thread for a new CPU from a pre-bringup stage, by
> calling idle_thread_get() for itself. This would mean that the cleanup
> in bringup_cpu() would be too late.
>
> Move the SCS/KASAN cleanup to the generic _cpu_up() function instead,
> which already ensures that the new CPU's stack is available, purely to
> allow for early failure. This occurs when the CPU to be brought up is
> in the CPUHP_OFFLINE state, which should correctly do the cleanup any
> time the CPU has been taken down to the point where such is needed.
>
> Signed-off-by: David Woodhouse <[email protected]>
> Reviewed-by: Mark Rutland <[email protected]>
> Tested-by: Mark Rutland <[email protected]> [arm64]

Forgot to include my sign-off. Thanks David for pointing it out.

Signed-off-by: Usama Arif <[email protected]>

> ---
> kernel/cpu.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 6c0a92ca6bb5..43e0a77f21e8 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -591,12 +591,6 @@ static int bringup_cpu(unsigned int cpu)
> struct task_struct *idle = idle_thread_get(cpu);
> int ret;
>
> - /*
> - * Reset stale stack state from the last time this CPU was online.
> - */
> - scs_task_reset(idle);
> - kasan_unpoison_task_stack(idle);
> -
> /*
> * Some architectures have to walk the irq descriptors to
> * setup the vector space for the cpu which comes online.
> @@ -1383,6 +1377,12 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target)
> ret = PTR_ERR(idle);
> goto out;
> }
> +
> + /*
> + * Reset stale stack state from the last time this CPU was online.
> + */
> + scs_task_reset(idle);
> + kasan_unpoison_task_stack(idle);
> }
>
> cpuhp_tasks_frozen = tasks_frozen;

2023-03-30 00:12:58

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v17 8/8] x86/smpboot: Allow parallel bringup for SEV-ES

On Tue, Mar 28 2023 at 22:07, Borislav Petkov wrote:
>> void cc_set_vendor(enum cc_vendor v);
>> +enum cc_vendor cc_get_vendor(void);
>> void cc_set_mask(u64 mask);
>>
>> #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
>
> You don't need those hunks adding cc_set_vendor() anymore:
>
> https://git.kernel.org/tip/5ae57743f578725a5dadb6f31d7798ee55e6e967

That's not really true. The series is based on the x86/apic branch as
the prerequites are in that brnach and that commit is in x86/sev.

That's an x86 maintainer issue to sort out, really.

Thanks,

tglx

2023-03-30 07:58:04

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v17 8/8] x86/smpboot: Allow parallel bringup for SEV-ES

On Thu, Mar 30, 2023 at 02:10:56AM +0200, Thomas Gleixner wrote:
> That's not really true. The series is based on the x86/apic branch as
> the prerequites are in that brnach and that commit is in x86/sev.
>
> That's an x86 maintainer issue to sort out, really.

Why do you think I left this note?

So that when you decide to pick up the rest of the parallel bringup
stuff, you can see it, find me on IRC and we figure out how to do the
tip patch tetris.

:-P

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-03-30 08:20:53

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v17 8/8] x86/smpboot: Allow parallel bringup for SEV-ES

On Thu, Mar 30 2023 at 02:10, Thomas Gleixner wrote:
> On Tue, Mar 28 2023 at 22:07, Borislav Petkov wrote:
>>> void cc_set_vendor(enum cc_vendor v);
>>> +enum cc_vendor cc_get_vendor(void);
>>> void cc_set_mask(u64 mask);
>>>
>>> #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
>>
>> You don't need those hunks adding cc_set_vendor() anymore:
>>
>> https://git.kernel.org/tip/5ae57743f578725a5dadb6f31d7798ee55e6e967
>
> That's not really true. The series is based on the x86/apic branch as
> the prerequites are in that brnach and that commit is in x86/sev.
>
> That's an x86 maintainer issue to sort out, really.

Aside of that, this needs a wrapper when CONFIG_ARCH_HAS_CC_PLATFORM is
not set to be usable outside of CC specific code...

Thanks,

tglx

2023-03-30 16:51:39

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v17 6/8] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel

On Tue, Mar 28 2023 at 20:57, Usama Arif wrote:
> The APs will then take turns through the real mode code (which has its
> own bitlock for exclusion) until they make it to their own stack, then
> proceed through the first few lines of start_secondary() and execute
> these parts in parallel:
>
> start_secondary()
> -> cr4_init()
> -> (some 32-bit only stuff so not in the parallel cases)
> -> cpu_init_secondary()
> -> cpu_init_exception_handling()
> -> cpu_init()
> -> wait_for_master_cpu()
>
> At this point they wait for the BSP to set their bit in cpu_callout_mask
> (from do_wait_cpu_initialized()), and release them to continue through
> the rest of cpu_init() and beyond.

That's actually broken on SMT enabled machines when microcode needs to
be updated.

Lets look at a 2 core, 4 thread system, where CPU0/2 and CPU1/3 are the
sibling pairs.

CPU 0: CPU1 CPU2 CPU3

for_each_present_cpu(cpu)
cpu_up(cpu, KICK_AP_ALIVE);
startup()
wait()

startup()
wait()

Release CPU1
load_ucode() startup()
wait()

So that violates the rules of microcode loading that the sibling must be
in a state where it does not execute anything which might be affected by
the microcode update. The fragile startup code does not really qualify
as such a state :)

Thanks,

tglx


2023-03-30 17:17:53

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v17 6/8] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel

On March 30, 2023 6:46:24 PM GMT+02:00, Thomas Gleixner <[email protected]> wrote:
>So that violates the rules of microcode loading that the sibling must be
>in a state where it does not execute anything which might be affected by
>the microcode update. The fragile startup code does not really qualify
>as such a state :)

Yeah I don't think we ever enforced this for early loading. The thread sync thing came with the late loading dance....


--
Sent from a small device: formatting sucks and brevity is inevitable.

2023-03-30 18:18:56

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v17 6/8] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel

On Thu, Mar 30 2023 at 19:05, Borislav Petkov wrote:

> On March 30, 2023 6:46:24 PM GMT+02:00, Thomas Gleixner <[email protected]> wrote:
>>So that violates the rules of microcode loading that the sibling must be
>>in a state where it does not execute anything which might be affected by
>>the microcode update. The fragile startup code does not really qualify
>>as such a state :)
>
> Yeah I don't think we ever enforced this for early loading.

We don't have to so far. CPU bringup is fully serialized so when the
first sibling comes up the other one is still in wait for SIPI lala
land. When the second comes up it will see that the microcode is already
up to date.

Thanks,

tglx

2023-03-31 11:42:55

by Usama Arif

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v17 6/8] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel



On 30/03/2023 19:17, Thomas Gleixner wrote:
> On Thu, Mar 30 2023 at 19:05, Borislav Petkov wrote:
>
>> On March 30, 2023 6:46:24 PM GMT+02:00, Thomas Gleixner <[email protected]> wrote:
>>> So that violates the rules of microcode loading that the sibling must be
>>> in a state where it does not execute anything which might be affected by
>>> the microcode update. The fragile startup code does not really qualify
>>> as such a state :)
>>
>> Yeah I don't think we ever enforced this for early loading.
>
> We don't have to so far. CPU bringup is fully serialized so when the
> first sibling comes up the other one is still in wait for SIPI lala
> land. When the second comes up it will see that the microcode is already
> up to date.
>

A simple solution is to serialize load_ucode_ap by acquiring a spinlock
at the start of ucode_cpu_init and releasing it at its end.

I guess if we had topology_sibling_cpumask initialized at this point we
could have a spinlock per core (not thread) and parallelize it, but
thats set much later in smp_callin.

I can include the below in next version if it makes sense?

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 80a688295ffa..b5e64628a975 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2155,10 +2155,13 @@ static inline void setup_getcpu(int cpu)
}

#ifdef CONFIG_X86_64
+static DEFINE_SPINLOCK(ucode_cpu_spinlock);
static inline void ucode_cpu_init(int cpu)
{
+ spin_lock(&ucode_cpu_spinlock);
if (cpu)
load_ucode_ap();
+ spin_unlock(&ucode_cpu_spinlock);
}