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
Load RSP from current_task->thread.sp instead.
Signed-off-by: Brian Gerst <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Tested-by: Usama Arif <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
---
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
Use the percpu offset directly to set GSBASE.
Signed-off-by: Brian Gerst <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Tested-by: Usama Arif <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
---
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
Build the GDT descriptor on the stack instead.
Signed-off-by: Brian Gerst <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Tested-by: Usama Arif <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Tested-by: Usama Arif <[email protected]>
Signed-off-by: Usama Arif <[email protected]>
---
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
All CPUs now use the same setup path. Remove STARTUP_SECONDARY and
renumber the remaining flags.
Signed-off-by: Brian Gerst <[email protected]>
---
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
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
On Fri, Feb 24, 2023 at 3:40 PM Usama Arif <[email protected]> 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.
On 24 February 2023 21:38:41 GMT, Brian Gerst <[email protected]> wrote:
>On Fri, Feb 24, 2023 at 3:40 PM Usama Arif <[email protected]> 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.
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.
On Sat, Feb 25, 2023 at 5:20 AM David Woodhouse <[email protected]> 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
On 25 February 2023 12:47:33 GMT, Brian Gerst <[email protected]> wrote:
>On Sat, Feb 25, 2023 at 5:20 AM David Woodhouse <[email protected]> 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?
On 25/02/2023 12:55, David Woodhouse wrote:
>
>
> On 25 February 2023 12:47:33 GMT, Brian Gerst <[email protected]> wrote:
>> On Sat, Feb 25, 2023 at 5:20 AM David Woodhouse <[email protected]> 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/[email protected]/.
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.
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/[email protected]/.
> 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.
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/[email protected]/.
>> 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!
On 25 February 2023 22:15:02 GMT, Usama Arif <[email protected]> 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/[email protected]/.
>>> 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 */