[PATCH v2 0/5] x86-64: Remove global variables from boot

[PATCH v2 0/5] x86-64: Remove global variables from boot

[Brian Gerst]

This is based on the parallel boot v11 series.

Remove the global variables initial_gs, initial_stack, and
early_gdt_descr from the 64-bit boot code.  The stack, GDT, and GSBASE
can be determined from the CPU number.

v2: - Dropped first two patches which were merged into another patch in
      the parallel boot series.
    - Fixed a compile error in patch 1 found by the kernel test robot.
    - Split out the removal of STARTUP_SECONDARY into a separate patch
      and renumber the remaining flags.

Brian Gerst (5):
  x86/smpboot: Remove initial_stack on 64-bit
  x86/smpboot: Remove early_gdt_descr on 64-bit
  x86/smpboot: Remove initial_gs
  x86/smpboot: Simplify boot CPU setup
  x86/smpboot: Remove STARTUP_SECONDARY

 arch/x86/include/asm/processor.h |  6 +-
 arch/x86/include/asm/realmode.h  |  1 -
 arch/x86/include/asm/smp.h       |  5 +-
 arch/x86/kernel/acpi/sleep.c     |  5 +-
 arch/x86/kernel/head_64.S        | 99 ++++++++++++--------------------
 arch/x86/kernel/smpboot.c        |  6 +-
 arch/x86/xen/xen-head.S          |  2 +-
 7 files changed, 49 insertions(+), 75 deletions(-)

-- 
2.39.2

[PATCH v2 1/5] x86/smpboot: Remove initial_stack on 64-bit

[Brian Gerst]

Load RSP from current_task->thread.sp instead.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Tested-by: Usama Arif <usama.arif@bytedance.com>
Signed-off-by: Usama Arif <usama.arif@bytedance.com>
---
 arch/x86/include/asm/processor.h |  6 +++++-
 arch/x86/kernel/acpi/sleep.c     |  2 +-
 arch/x86/kernel/head_64.S        | 35 ++++++++++++++++++--------------
 arch/x86/xen/xen-head.S          |  2 +-
 4 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 8d73004e4cac..a1e4fa58b357 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -647,7 +647,11 @@ static inline void spin_lock_prefetch(const void *x)
 #define KSTK_ESP(task)		(task_pt_regs(task)->sp)
 
 #else
-#define INIT_THREAD { }
+extern unsigned long __end_init_task[];
+
+#define INIT_THREAD {							    \
+	.sp	= (unsigned long)&__end_init_task - sizeof(struct pt_regs), \
+}
 
 extern unsigned long KSTK_ESP(struct task_struct *task);
 
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 47e75c056cb5..008fda8b1982 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -113,7 +113,7 @@ int x86_acpi_suspend_lowlevel(void)
 	saved_magic = 0x12345678;
 #else /* CONFIG_64BIT */
 #ifdef CONFIG_SMP
-	initial_stack = (unsigned long)temp_stack + sizeof(temp_stack);
+	current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack);
 	early_gdt_descr.address =
 			(unsigned long)get_cpu_gdt_rw(smp_processor_id());
 	initial_gs = per_cpu_offset(smp_processor_id());
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index c32e5b06a9ce..f7905ba4b992 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -62,8 +62,8 @@ SYM_CODE_START_NOALIGN(startup_64)
 	 * tables and then reload them.
 	 */
 
-	/* Set up the stack for verify_cpu(), similar to initial_stack below */
-	leaq	(__end_init_task - FRAME_SIZE)(%rip), %rsp
+	/* Set up the stack for verify_cpu() */
+	leaq	(__end_init_task - PTREGS_SIZE)(%rip), %rsp
 
 	leaq	_text(%rip), %rdi
 
@@ -245,11 +245,11 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 #ifdef CONFIG_SMP
 	/*
 	 * Is this the boot CPU coming up? If so everything is available
-	 * in initial_gs, initial_stack and early_gdt_descr.
+	 * in initial_gs and early_gdt_descr.
 	 */
 	movl	smpboot_control(%rip), %edx
 	testl	$STARTUP_SECONDARY, %edx
-	jz	.Lsetup_cpu
+	jz	.Linit_cpu0_data
 
 	/*
 	 * For parallel boot, the APIC ID is retrieved from CPUID, and then
@@ -302,6 +302,10 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	hlt
 	jmp	1b
 
+.Linit_cpu0_data:
+	movq	__per_cpu_offset(%rip), %rdx
+	jmp	.Lsetup_cpu
+
 .Linit_cpu_data:
 	/* Get the per cpu offset for the given CPU# which is in ECX */
 	leaq	__per_cpu_offset(%rip), %rbx
@@ -314,13 +318,21 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	addq	%rbx, %rcx
 	movq	%rcx, early_gdt_descr_base(%rip)
 
-	/* Find the idle task stack */
-	movq	pcpu_hot + X86_current_task(%rbx), %rcx
-	movq	TASK_threadsp(%rcx), %rcx
-	movq	%rcx, initial_stack(%rip)
+	movq	%rbx, %rdx
+#else
+	xorl	%edx, %edx
 #endif /* CONFIG_SMP */
 
 .Lsetup_cpu:
+	/*
+	 * Setup a boot time stack - Any secondary CPU will have lost its stack
+	 * by now because the cr3-switch above unmaps the real-mode stack
+	 *
+	 * RDX contains the per-cpu offset
+	 */
+	movq	pcpu_hot + X86_current_task(%rdx), %rax
+	movq	TASK_threadsp(%rax), %rsp
+
 	/*
 	 * 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
@@ -355,12 +367,6 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	movl	initial_gs+4(%rip),%edx
 	wrmsr
 
-	/*
-	 * Setup a boot time stack - Any secondary CPU will have lost its stack
-	 * by now because the cr3-switch above unmaps the real-mode stack
-	 */
-	movq initial_stack(%rip), %rsp
-
 	/* Drop the realmode protection. For the boot CPU the pointer is NULL! */
 	movq	trampoline_lock(%rip), %rax
 	testq	%rax, %rax
@@ -517,7 +523,6 @@ SYM_DATA(initial_vc_handler,	.quad handle_vc_boot_ghcb)
  * The FRAME_SIZE gap is a convention which helps the in-kernel unwinder
  * reliably detect the end of the stack.
  */
-SYM_DATA(initial_stack, .quad init_thread_union + THREAD_SIZE - FRAME_SIZE)
 SYM_DATA(trampoline_lock, .quad 0);
 	__FINITDATA
 
diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index e36ea4268bd2..91f7a53519a7 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -49,7 +49,7 @@ SYM_CODE_START(startup_xen)
 	ANNOTATE_NOENDBR
 	cld
 
-	mov initial_stack(%rip), %rsp
+	leaq	(__end_init_task - PTREGS_SIZE)(%rip), %rsp
 
 	/* Set up %gs.
 	 *
-- 
2.39.2

[PATCH v2 2/5] x86/smpboot: Remove early_gdt_descr on 64-bit

[Brian Gerst]

Build the GDT descriptor on the stack instead.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Tested-by: Usama Arif <usama.arif@bytedance.com>
Signed-off-by: Usama Arif <usama.arif@bytedance.com>
---
 arch/x86/kernel/acpi/sleep.c |  2 --
 arch/x86/kernel/head_64.S    | 19 +++++++------------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 008fda8b1982..6538ddb55f28 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -114,8 +114,6 @@ int x86_acpi_suspend_lowlevel(void)
 #else /* CONFIG_64BIT */
 #ifdef CONFIG_SMP
 	current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack);
-	early_gdt_descr.address =
-			(unsigned long)get_cpu_gdt_rw(smp_processor_id());
 	initial_gs = per_cpu_offset(smp_processor_id());
 	/* Force the startup into boot mode */
 	saved_smpboot_ctrl = xchg(&smpboot_control, 0);
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index f7905ba4b992..0dd57d573a0e 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -245,7 +245,7 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 #ifdef CONFIG_SMP
 	/*
 	 * Is this the boot CPU coming up? If so everything is available
-	 * in initial_gs and early_gdt_descr.
+	 * in initial_gs.
 	 */
 	movl	smpboot_control(%rip), %edx
 	testl	$STARTUP_SECONDARY, %edx
@@ -313,11 +313,6 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	/* Save it for GS BASE setup */
 	movq	%rbx, initial_gs(%rip)
 
-	/* Calculate the GDT address */
-	movq	$gdt_page, %rcx
-	addq	%rbx, %rcx
-	movq	%rcx, early_gdt_descr_base(%rip)
-
 	movq	%rbx, %rdx
 #else
 	xorl	%edx, %edx
@@ -339,7 +334,12 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	 * addresses where we're currently running on. We have to do that here
 	 * because in 32bit we couldn't load a 64bit linear address.
 	 */
-	lgdt	early_gdt_descr(%rip)
+	subq	$16, %rsp
+	movw	$(GDT_SIZE-1), (%rsp)
+	leaq	gdt_page(%rdx), %rax
+	movq	%rax, 2(%rsp)
+	lgdt	(%rsp)
+	addq	$16, %rsp
 
 	/* set up data segments */
 	xorl %eax,%eax
@@ -754,11 +754,6 @@ SYM_DATA_END(level1_fixmap_pgt)
 
 	.data
 	.align 16
-
-SYM_DATA(early_gdt_descr,		.word GDT_ENTRIES*8-1)
-SYM_DATA_LOCAL(early_gdt_descr_base,	.quad INIT_PER_CPU_VAR(gdt_page))
-
-	.align 16
 SYM_DATA(smpboot_control,		.long 0)
 
 	.align 16
-- 
2.39.2

[PATCH v2 3/5] x86/smpboot: Remove initial_gs

[Brian Gerst]

Use the percpu offset directly to set GSBASE.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Tested-by: Usama Arif <usama.arif@bytedance.com>
Signed-off-by: Usama Arif <usama.arif@bytedance.com>
---
 arch/x86/include/asm/realmode.h |  1 -
 arch/x86/kernel/acpi/sleep.c    |  1 -
 arch/x86/kernel/head_64.S       | 34 ++++++++++-----------------------
 3 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/realmode.h b/arch/x86/include/asm/realmode.h
index f0357cfe2fb0..87e5482acd0d 100644
--- a/arch/x86/include/asm/realmode.h
+++ b/arch/x86/include/asm/realmode.h
@@ -60,7 +60,6 @@ extern struct real_mode_header *real_mode_header;
 extern unsigned char real_mode_blob_end[];
 
 extern unsigned long initial_code;
-extern unsigned long initial_gs;
 extern unsigned long initial_stack;
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 extern unsigned long initial_vc_handler;
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 6538ddb55f28..214dd4a79860 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -114,7 +114,6 @@ int x86_acpi_suspend_lowlevel(void)
 #else /* CONFIG_64BIT */
 #ifdef CONFIG_SMP
 	current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack);
-	initial_gs = per_cpu_offset(smp_processor_id());
 	/* Force the startup into boot mode */
 	saved_smpboot_ctrl = xchg(&smpboot_control, 0);
 #endif
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 0dd57d573a0e..9ed87ba0609f 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -67,18 +67,10 @@ SYM_CODE_START_NOALIGN(startup_64)
 
 	leaq	_text(%rip), %rdi
 
-	/*
-	 * initial_gs points to initial fixed_percpu_data struct with storage for
-	 * the stack protector canary. Global pointer fixups are needed at this
-	 * stage, so apply them as is done in fixup_pointer(), and initialize %gs
-	 * such that the canary can be accessed at %gs:40 for subsequent C calls.
-	 */
+	/* Setup GSBASE to allow stack canary access for C code */
 	movl	$MSR_GS_BASE, %ecx
-	movq	initial_gs(%rip), %rax
-	movq	$_text, %rdx
-	subq	%rdx, %rax
-	addq	%rdi, %rax
-	movq	%rax, %rdx
+	leaq	INIT_PER_CPU_VAR(fixed_percpu_data)(%rip), %rdx
+	movl	%edx, %eax
 	shrq	$32,  %rdx
 	wrmsr
 
@@ -243,10 +235,7 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	ANNOTATE_NOENDBR // above
 
 #ifdef CONFIG_SMP
-	/*
-	 * Is this the boot CPU coming up? If so everything is available
-	 * in initial_gs.
-	 */
+	/* Is this the boot CPU coming up? */
 	movl	smpboot_control(%rip), %edx
 	testl	$STARTUP_SECONDARY, %edx
 	jz	.Linit_cpu0_data
@@ -308,12 +297,7 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 
 .Linit_cpu_data:
 	/* Get the per cpu offset for the given CPU# which is in ECX */
-	leaq	__per_cpu_offset(%rip), %rbx
-	movq	(%rbx,%rcx,8), %rbx
-	/* Save it for GS BASE setup */
-	movq	%rbx, initial_gs(%rip)
-
-	movq	%rbx, %rdx
+	movq	__per_cpu_offset(,%rcx,8), %rdx
 #else
 	xorl	%edx, %edx
 #endif /* CONFIG_SMP */
@@ -363,8 +347,11 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	 * the per cpu areas are set up.
 	 */
 	movl	$MSR_GS_BASE,%ecx
-	movl	initial_gs(%rip),%eax
-	movl	initial_gs+4(%rip),%edx
+#ifndef CONFIG_SMP
+	leaq	INIT_PER_CPU_VAR(fixed_percpu_data)(%rip), %rdx
+#endif
+	movl	%edx, %eax
+	shrq	$32, %rdx
 	wrmsr
 
 	/* Drop the realmode protection. For the boot CPU the pointer is NULL! */
@@ -514,7 +501,6 @@ SYM_CODE_END(vc_boot_ghcb)
 	__REFDATA
 	.balign	8
 SYM_DATA(initial_code,	.quad x86_64_start_kernel)
-SYM_DATA(initial_gs,	.quad INIT_PER_CPU_VAR(fixed_percpu_data))
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 SYM_DATA(initial_vc_handler,	.quad handle_vc_boot_ghcb)
 #endif
-- 
2.39.2

[PATCH v2 4/5] x86/smpboot: Simplify boot CPU setup

[Brian Gerst]

Now that the per-cpu GSBASE, stack, and GDT descriptor can be derived
dynamically by CPU number, the boot CPU can use a fixed CPU number and
take the same path as secondary CPUs.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Tested-by: Usama Arif <usama.arif@bytedance.com>
Signed-off-by: Usama Arif <usama.arif@bytedance.com>
---
 arch/x86/kernel/head_64.S | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 9ed87ba0609f..8bd29ab523dd 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -235,11 +235,6 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	ANNOTATE_NOENDBR // above
 
 #ifdef CONFIG_SMP
-	/* Is this the boot CPU coming up? */
-	movl	smpboot_control(%rip), %edx
-	testl	$STARTUP_SECONDARY, %edx
-	jz	.Linit_cpu0_data
-
 	/*
 	 * 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
@@ -250,13 +245,13 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	 * Bit 29	STARTUP_APICID_CPUID_01 flag (use CPUID 0x01)
 	 * Bit 0-24	CPU# if STARTUP_APICID_CPUID_xx flags are not set
 	 */
-	testl	$STARTUP_APICID_CPUID_0B, %edx
+	movl	smpboot_control(%rip), %ecx
+	testl	$STARTUP_APICID_CPUID_0B, %ecx
 	jnz	.Luse_cpuid_0b
-	testl	$STARTUP_APICID_CPUID_01, %edx
+	testl	$STARTUP_APICID_CPUID_01, %ecx
 	jnz	.Luse_cpuid_01
-	andl	$0x0FFFFFFF, %edx
-	movl	%edx, %ecx
-	jmp	.Linit_cpu_data
+	andl	$0x0FFFFFFF, %ecx
+	jmp	.Lsetup_cpu
 
 .Luse_cpuid_01:
 	mov	$0x01, %eax
@@ -277,7 +272,7 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 
 .Lfind_cpunr:
 	cmpl	(%rbx,%rcx,4), %edx
-	jz	.Linit_cpu_data
+	jz	.Lsetup_cpu
 	inc	%ecx
 	cmpl	nr_cpu_ids(%rip), %ecx
 	jb	.Lfind_cpunr
@@ -291,18 +286,13 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	hlt
 	jmp	1b
 
-.Linit_cpu0_data:
-	movq	__per_cpu_offset(%rip), %rdx
-	jmp	.Lsetup_cpu
-
-.Linit_cpu_data:
+.Lsetup_cpu:
 	/* Get the per cpu offset for the given CPU# which is in ECX */
 	movq	__per_cpu_offset(,%rcx,8), %rdx
 #else
 	xorl	%edx, %edx
 #endif /* CONFIG_SMP */
 
-.Lsetup_cpu:
 	/*
 	 * Setup a boot time stack - Any secondary CPU will have lost its stack
 	 * by now because the cr3-switch above unmaps the real-mode stack
-- 
2.39.2

[PATCH v2 5/5] x86/smpboot: Remove STARTUP_SECONDARY

[Brian Gerst]

All CPUs now use the same setup path.  Remove STARTUP_SECONDARY and
renumber the remaining flags.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 arch/x86/include/asm/smp.h | 5 ++---
 arch/x86/kernel/head_64.S  | 5 ++---
 arch/x86/kernel/smpboot.c  | 6 +++---
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index b4b29e052b6e..97a36d029b0e 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -202,8 +202,7 @@ extern unsigned int smpboot_control;
 #endif /* !__ASSEMBLY__ */
 
 /* Control bits for startup_64 */
-#define STARTUP_SECONDARY	0x80000000
-#define STARTUP_APICID_CPUID_0B	0x40000000
-#define STARTUP_APICID_CPUID_01	0x20000000
+#define STARTUP_APICID_CPUID_0B	0x80000000
+#define STARTUP_APICID_CPUID_01	0x40000000
 
 #endif /* _ASM_X86_SMP_H */
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 8bd29ab523dd..f629bf74217b 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -240,9 +240,8 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
 	 * used to look up the CPU number.  For booting a single CPU, the
 	 * CPU number is encoded in smpboot_control.
 	 *
-	 * Bit 31	STARTUP_SECONDARY flag (checked above)
-	 * Bit 30	STARTUP_APICID_CPUID_0B flag (use CPUID 0x0b)
-	 * Bit 29	STARTUP_APICID_CPUID_01 flag (use CPUID 0x01)
+	 * 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
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index e1a2843c2841..c159a5c2df9f 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1140,7 +1140,7 @@ static int do_boot_cpu(int apicid, int cpu, struct task_struct *idle,
 		early_gdt_descr.address = (unsigned long)get_cpu_gdt_rw(cpu);
 		initial_stack  = idle->thread.sp;
 	} else if (!do_parallel_bringup) {
-		smpboot_control = STARTUP_SECONDARY | cpu;
+		smpboot_control = cpu;
 	}
 
 	/* Enable the espfix hack for this CPU */
@@ -1580,7 +1580,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 		 */
 		if (eax) {
 			pr_debug("Using CPUID 0xb for parallel CPU startup\n");
-			smpboot_control = STARTUP_SECONDARY | STARTUP_APICID_CPUID_0B;
+			smpboot_control = STARTUP_APICID_CPUID_0B;
 		} else {
 			pr_info("Disabling parallel bringup because CPUID 0xb looks untrustworthy\n");
 			do_parallel_bringup = false;
@@ -1588,7 +1588,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 	} else if (do_parallel_bringup) {
 		/* Without X2APIC, what's in CPUID 0x01 should suffice. */
 		pr_debug("Using CPUID 0x1 for parallel CPU startup\n");
-		smpboot_control = STARTUP_SECONDARY | STARTUP_APICID_CPUID_01;
+		smpboot_control = STARTUP_APICID_CPUID_01;
 	}
 
 	if (do_parallel_bringup) {
-- 
2.39.2

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[Usama Arif]

On 24/02/2023 15:42, Brian Gerst wrote:
> This is based on the parallel boot v11 series.
> 
> Remove the global variables initial_gs, initial_stack, and
> early_gdt_descr from the 64-bit boot code.  The stack, GDT, and GSBASE
> can be determined from the CPU number.
> 
> v2: - Dropped first two patches which were merged into another patch in
>        the parallel boot series.
>      - Fixed a compile error in patch 1 found by the kernel test robot.
>      - Split out the removal of STARTUP_SECONDARY into a separate patch
>        and renumber the remaining flags.
> 
> Brian Gerst (5):
>    x86/smpboot: Remove initial_stack on 64-bit
>    x86/smpboot: Remove early_gdt_descr on 64-bit
>    x86/smpboot: Remove initial_gs
>    x86/smpboot: Simplify boot CPU setup
>    x86/smpboot: Remove STARTUP_SECONDARY
> 
>   arch/x86/include/asm/processor.h |  6 +-
>   arch/x86/include/asm/realmode.h  |  1 -
>   arch/x86/include/asm/smp.h       |  5 +-
>   arch/x86/kernel/acpi/sleep.c     |  5 +-
>   arch/x86/kernel/head_64.S        | 99 ++++++++++++--------------------
>   arch/x86/kernel/smpboot.c        |  6 +-
>   arch/x86/xen/xen-head.S          |  2 +-
>   7 files changed, 49 insertions(+), 75 deletions(-)
> 

Its weird putting something in earlier patches like 
STARTUP_APICID_CPUID_*,STARTUP_SECONDARY.. and removing/changing it 
later on in *the same series*. Maybe better to keep this as a separate 
series from parallel smp? i.e. not include this in v12 and review it 
separately? Happy with whatever you and David decide..

Thanks,
Usama

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[Brian Gerst]

On Fri, Feb 24, 2023 at 3:40 PM Usama Arif <usama.arif@bytedance.com> wrote:
>
>
>
> On 24/02/2023 15:42, Brian Gerst wrote:
> > This is based on the parallel boot v11 series.
> >
> > Remove the global variables initial_gs, initial_stack, and
> > early_gdt_descr from the 64-bit boot code.  The stack, GDT, and GSBASE
> > can be determined from the CPU number.
> >
> > v2: - Dropped first two patches which were merged into another patch in
> >        the parallel boot series.
> >      - Fixed a compile error in patch 1 found by the kernel test robot.
> >      - Split out the removal of STARTUP_SECONDARY into a separate patch
> >        and renumber the remaining flags.
> >
> > Brian Gerst (5):
> >    x86/smpboot: Remove initial_stack on 64-bit
> >    x86/smpboot: Remove early_gdt_descr on 64-bit
> >    x86/smpboot: Remove initial_gs
> >    x86/smpboot: Simplify boot CPU setup
> >    x86/smpboot: Remove STARTUP_SECONDARY
> >
> >   arch/x86/include/asm/processor.h |  6 +-
> >   arch/x86/include/asm/realmode.h  |  1 -
> >   arch/x86/include/asm/smp.h       |  5 +-
> >   arch/x86/kernel/acpi/sleep.c     |  5 +-
> >   arch/x86/kernel/head_64.S        | 99 ++++++++++++--------------------
> >   arch/x86/kernel/smpboot.c        |  6 +-
> >   arch/x86/xen/xen-head.S          |  2 +-
> >   7 files changed, 49 insertions(+), 75 deletions(-)
> >
>
> Its weird putting something in earlier patches like
> STARTUP_APICID_CPUID_*,STARTUP_SECONDARY.. and removing/changing it
> later on in *the same series*. Maybe better to keep this as a separate
> series from parallel smp? i.e. not include this in v12 and review it
> separately? Happy with whatever you and David decide..
>
> Thanks,
> Usama

Removing the globals before the parallel boot series, would be the
best option IMO.  That would make the transition simpler.

--
Brian Gerst.

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[David Woodhouse]

On 24 February 2023 21:38:41 GMT, Brian Gerst <brgerst@gmail.com> wrote:
>On Fri, Feb 24, 2023 at 3:40 PM Usama Arif <usama.arif@bytedance.com> wrote:
>>
>>
>>
>> On 24/02/2023 15:42, Brian Gerst wrote:
>> > This is based on the parallel boot v11 series.
>> >
>> > Remove the global variables initial_gs, initial_stack, and
>> > early_gdt_descr from the 64-bit boot code.  The stack, GDT, and GSBASE
>> > can be determined from the CPU number.
>> >
>> > v2: - Dropped first two patches which were merged into another patch in
>> >        the parallel boot series.
>> >      - Fixed a compile error in patch 1 found by the kernel test robot.
>> >      - Split out the removal of STARTUP_SECONDARY into a separate patch
>> >        and renumber the remaining flags.
>> >
>> > Brian Gerst (5):
>> >    x86/smpboot: Remove initial_stack on 64-bit
>> >    x86/smpboot: Remove early_gdt_descr on 64-bit
>> >    x86/smpboot: Remove initial_gs
>> >    x86/smpboot: Simplify boot CPU setup
>> >    x86/smpboot: Remove STARTUP_SECONDARY
>> >
>> >   arch/x86/include/asm/processor.h |  6 +-
>> >   arch/x86/include/asm/realmode.h  |  1 -
>> >   arch/x86/include/asm/smp.h       |  5 +-
>> >   arch/x86/kernel/acpi/sleep.c     |  5 +-
>> >   arch/x86/kernel/head_64.S        | 99 ++++++++++++--------------------
>> >   arch/x86/kernel/smpboot.c        |  6 +-
>> >   arch/x86/xen/xen-head.S          |  2 +-
>> >   7 files changed, 49 insertions(+), 75 deletions(-)
>> >
>>
>> Its weird putting something in earlier patches like
>> STARTUP_APICID_CPUID_*,STARTUP_SECONDARY.. and removing/changing it
>> later on in *the same series*. Maybe better to keep this as a separate
>> series from parallel smp? i.e. not include this in v12 and review it
>> separately? Happy with whatever you and David decide..
>>
>> Thanks,
>> Usama
>
>Removing the globals before the parallel boot series, would be the
>best option IMO.  That would make the transition simpler.

Yeah, makes sense.. Let's do that.

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[David Woodhouse]

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

On Fri, 2023-02-24 at 16:38 -0500, Brian Gerst wrote:
> Removing the globals before the parallel boot series, would be the
> best option IMO.  That would make the transition simpler.

Looks like this:

https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/parallel-6.2-rc8-brfirst

Passes basic smoke testing in qemu, including suspend to RAM and
offlining CPU0.

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

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[Brian Gerst]

On Sat, Feb 25, 2023 at 5:20 AM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Fri, 2023-02-24 at 16:38 -0500, Brian Gerst wrote:
> > Removing the globals before the parallel boot series, would be the
> > best option IMO.  That would make the transition simpler.
>
> Looks like this:
>
> https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/parallel-6.2-rc8-brfirst
>
> Passes basic smoke testing in qemu, including suspend to RAM and
> offlining CPU0.

Looks good, thanks.

--
Brian Gerst

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[David Woodhouse]

On 25 February 2023 12:47:33 GMT, Brian Gerst <brgerst@gmail.com> wrote:
>On Sat, Feb 25, 2023 at 5:20 AM David Woodhouse <dwmw2@infradead.org> wrote:
>>
>> On Fri, 2023-02-24 at 16:38 -0500, Brian Gerst wrote:
>> > Removing the globals before the parallel boot series, would be the
>> > best option IMO.  That would make the transition simpler.
>>
>> Looks like this:
>>
>> https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/parallel-6.2-rc8-brfirst
>>
>> Passes basic smoke testing in qemu, including suspend to RAM and
>> offlining CPU0.
>
>Looks good, thanks.

Aside from the fact that I forgot to put the tr_lock into the other startup paths that load the realmode %esp. But I'll let Usama do that now.

I can probably work out how to test the AMD SEV path; is there a way I can test the startup_64 MADT one?

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[Usama Arif]

On 25/02/2023 12:55, David Woodhouse wrote:
> 
> 
> On 25 February 2023 12:47:33 GMT, Brian Gerst <brgerst@gmail.com> wrote:
>> On Sat, Feb 25, 2023 at 5:20 AM David Woodhouse <dwmw2@infradead.org> wrote:
>>>
>>> On Fri, 2023-02-24 at 16:38 -0500, Brian Gerst wrote:
>>>> Removing the globals before the parallel boot series, would be the
>>>> best option IMO.  That would make the transition simpler.
>>>
>>> Looks like this:
>>>
>>> https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/parallel-6.2-rc8-brfirst
>>>
>>> Passes basic smoke testing in qemu, including suspend to RAM and
>>> offlining CPU0.
>>
>> Looks good, thanks.
> 
> Aside from the fact that I forgot to put the tr_lock into the other startup paths that load the realmode %esp. But I'll let Usama do that now.
> 
Yeah looks good! I am testing with the macro diff for tr_lock from 
https://lore.kernel.org/all/05e27a7a-1faa-944e-2764-6ab4d620fb8f@bytedance.com/. 
If it all works, happy for me to send out v12 with it?

> I can probably work out how to test the AMD SEV path; is there a way I can test the startup_64 MADT one?

I guess the easiest way is to create a TDX VM on Sapphire Rapids which I 
believe mostly Intel people have access to right now? Maybe we can post 
v12 and someone from Intel could just quickly verify if it boots with 
it? I have added Yuan from the other thread in here who pointed it out 
initially.

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[David Woodhouse]

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

On Sat, 2023-02-25 at 13:33 +0000, Usama Arif wrote:
> 
> Yeah looks good! I am testing with the macro diff for tr_lock from 
> https://lore.kernel.org/all/05e27a7a-1faa-944e-2764-6ab4d620fb8f@bytedance.com/.
> If it all works, happy for me to send out v12 with it?

I moved the macro definition up a little to put it between the .code16
and the .align, pushed it out as a commit on top of the above branch.

We'll collapse it into the 'Support parallel startup' patch, yes?

> > I can probably work out how to test the AMD SEV path; is there a
> > way I can test the startup_64 MADT one?
> 
> I guess the easiest way is to create a TDX VM on Sapphire Rapids which I 
> believe mostly Intel people have access to right now? Maybe we can post 
> v12 and someone from Intel could just quickly verify if it boots with
> it? I have added Yuan from the other thread in here who pointed it out 
> initially.

Yeah, I should also be able to rustle up both SPR and SEV-SNP if I dig
around at work a little.

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

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[Usama Arif]

On 25/02/2023 13:52, David Woodhouse wrote:
> On Sat, 2023-02-25 at 13:33 +0000, Usama Arif wrote:
>>
>> Yeah looks good! I am testing with the macro diff for tr_lock from
>> https://lore.kernel.org/all/05e27a7a-1faa-944e-2764-6ab4d620fb8f@bytedance.com/.
>> If it all works, happy for me to send out v12 with it?
> 
> I moved the macro definition up a little to put it between the .code16
> and the .align, pushed it out as a commit on top of the above branch.
> 
> We'll collapse it into the 'Support parallel startup' patch, yes?
> 

Yes, collapsed with "Support parallel startup of secondary CPUs" patch. 
I think Thomas' solution to dealing with suspend might be better? So I 
was thinking of sending v12 on top of v6.2 release with the following 
diff over your branch (merged in the right commit ofcourse):

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index defe76ee9e64..97a36d029b0e 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -205,6 +205,4 @@ extern unsigned int smpboot_control;
  #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 5dcf5ca15383..214dd4a79860 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -58,6 +58,7 @@ asmlinkage acpi_status __visible 
x86_acpi_enter_sleep_state(u8 state)
   */
  int x86_acpi_suspend_lowlevel(void)
  {
+       unsigned int __maybe_unused saved_smpboot_ctrl;
         struct wakeup_header *header =
                 (struct wakeup_header *) 
__va(real_mode_header->wakeup_header);

@@ -113,16 +114,11 @@ int x86_acpi_suspend_lowlevel(void)
  #else /* CONFIG_64BIT */
  #ifdef CONFIG_SMP
         current->thread.sp = (unsigned long)temp_stack + 
sizeof(temp_stack);
-       /*
-        * 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();
+       /* Force the startup into boot mode */
+       saved_smpboot_ctrl = xchg(&smpboot_control, 0);
  #endif
         initial_code = (unsigned long)wakeup_long64;
-       saved_magic = 0x123456789abcdef0L;
+       saved_magic = 0x123456789abcdef0L;
  #endif /* CONFIG_64BIT */

         /*
@@ -132,6 +128,9 @@ int x86_acpi_suspend_lowlevel(void)
         pause_graph_tracing();
         do_suspend_lowlevel();
         unpause_graph_tracing();
+
+       if (IS_ENABLED(CONFIG_64BIT) && IS_ENABLED(CONFIG_SMP))
+               smpboot_control = saved_smpboot_ctrl;
         return 0;
  }


>>> I can probably work out how to test the AMD SEV path; is there a
>>> way I can test the startup_64 MADT one?
>>
>> I guess the easiest way is to create a TDX VM on Sapphire Rapids which I
>> believe mostly Intel people have access to right now? Maybe we can post
>> v12 and someone from Intel could just quickly verify if it boots with
>> it? I have added Yuan from the other thread in here who pointed it out
>> initially.
> 
> Yeah, I should also be able to rustle up both SPR and SEV-SNP if I dig
> around at work a little.

Sounds good!

Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[David Woodhouse]

On 25 February 2023 22:15:02 GMT, Usama Arif <usama.arif@bytedance.com> wrote:
>
>
>On 25/02/2023 13:52, David Woodhouse wrote:
>> On Sat, 2023-02-25 at 13:33 +0000, Usama Arif wrote:
>>> 
>>> Yeah looks good! I am testing with the macro diff for tr_lock from
>>> https://lore.kernel.org/all/05e27a7a-1faa-944e-2764-6ab4d620fb8f@bytedance.com/.
>>> If it all works, happy for me to send out v12 with it?
>> 
>> I moved the macro definition up a little to put it between the .code16
>> and the .align, pushed it out as a commit on top of the above branch.
>> 
>> We'll collapse it into the 'Support parallel startup' patch, yes?
>> 
>
>Yes, collapsed with "Support parallel startup of secondary CPUs" patch. I think Thomas' solution to dealing with suspend might be better? So I was thinking of sending v12 on top of v6.2 release with the following diff over your branch (merged in the right commit ofcourse):

Nah, I think I prefer it as I had it. The new comment says it all.

>-       /*
>-        * 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();

And since Brian's patches there is no "boot mode" any more.

>+       /* Force the startup into boot mode */