From: David Woodhouse <dwmw2@infradead.org>
To: Thomas Gleixner <tglx@linutronix.de>,
Usama Arif <usama.arif@bytedance.com>,
kim.phillips@amd.com, brgerst@gmail.com
Cc: piotrgorski@cachyos.org, oleksandr@natalenko.name,
arjan@linux.intel.com, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org,
pbonzini@redhat.com, paulmck@kernel.org,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
rcu@vger.kernel.org, mimoja@mimoja.de, hewenliang4@huawei.com,
thomas.lendacky@amd.com, seanjc@google.com,
pmenzel@molgen.mpg.de, fam.zheng@bytedance.com,
punit.agrawal@bytedance.com, simon.evans@bytedance.com,
liangma@liangbit.com
Subject: Re: [PATCH v12 06/11] x86/smpboot: Remove initial_stack on 64-bit
Date: Tue, 28 Feb 2023 17:09:20 +0000 [thread overview]
Message-ID: <c6863590f5fbf139f6aec50d0f3bc8e8b00cfcaf.camel@infradead.org> (raw)
In-Reply-To: <87k001n4xo.ffs@tglx>
[-- Attachment #1: Type: text/plain, Size: 9349 bytes --]
On Tue, 2023-02-28 at 17:13 +0100, Thomas Gleixner wrote:
> On Sun, Feb 26 2023 at 11:07, Usama Arif wrote:
> > From: Brian Gerst <brgerst@gmail.com>
> >
> > Eliminating global variables from the CPU startup path in order to
> > simplify
> > it and facilitate parallel startup.
>
> As this patch is now part of the parallel boot series and actually
> introduces smpboot_control, the above is neither accurate nor useful.
Better commit message, add a comment where we abuse current->thread.sp
in the sleep path. Didn't remove the {} which would be added back in
the very next patch. Pushed to my tree for Usama's next round.
From b5b9a2ab146cfd840f115b0455930767b6f6fcea Mon Sep 17 00:00:00 2001
From: Brian Gerst <brgerst@gmail.com>
Date: Fri, 24 Feb 2023 10:42:31 -0500
Subject: [PATCH 06/11] x86/smpboot: Remove initial_stack on 64-bit
In order to facilitate parallel startup, start to eliminate global variables
from the CPU startup path.
However, we start by introducing one more: smpboot_control. For now this
merely holds the CPU# of the CPU which is coming up. That CPU can then
find its own per-cpu data, and everything else it needs can be found from
there, allowing the other global variables to be removed.
First to be removed is initial_stack. Each CPU can load %rsp from its
current_task->thread.sp instead. That is already set up with the correct
idle thread for APs. Set up the .sp field in INIT_THREAD on x86 so that
the BSP also finds a suitable stack pointer in the static per-cpu data
when coming up on first boot.
On resume from S3, the CPU needs a temporary stack because its idle task
is already active. Instead of setting initial_stack, the sleep code can
simply set its own current->thread.sp to point to the temporary stack.
The true stack pointer will get restored with the rest of the CPU
context in do_suspend_lowlevel().
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>
Tested-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
---
arch/x86/include/asm/processor.h | 6 ++++-
arch/x86/include/asm/smp.h | 5 +++-
arch/x86/kernel/acpi/sleep.c | 17 +++++++++++--
arch/x86/kernel/asm-offsets.c | 1 +
arch/x86/kernel/head_64.S | 43 +++++++++++++++++++++-----------
arch/x86/kernel/smpboot.c | 7 +++++-
arch/x86/xen/xen-head.S | 2 +-
7 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 4e35c66edeb7..bdde7316e75b 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -648,7 +648,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/include/asm/smp.h b/arch/x86/include/asm/smp.h
index b4dbb20dab1a..bf2c51df9e0b 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -199,5 +199,8 @@ extern void nmi_selftest(void);
#define nmi_selftest() do { } while (0)
#endif
-#endif /* __ASSEMBLY__ */
+extern unsigned int smpboot_control;
+
+#endif /* !__ASSEMBLY__ */
+
#endif /* _ASM_X86_SMP_H */
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 3b7f4cdbf2e0..23e44416dc04 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -111,13 +111,26 @@ int x86_acpi_suspend_lowlevel(void)
saved_magic = 0x12345678;
#else /* CONFIG_64BIT */
#ifdef CONFIG_SMP
- initial_stack = (unsigned long)temp_stack + sizeof(temp_stack);
+ /*
+ * As each CPU starts up, it will find its own stack pointer
+ * from its current_task->thread.sp. Typically that will be
+ * the idle thread for a newly-started AP, or even the boot
+ * CPU which will find it set to &init_task in the static
+ * per-cpu data.
+ *
+ * Make the resuming CPU use the temporary stack at startup
+ * by setting current->thread.sp to point to that. The true
+ * %rsp will be restored with the rest of the CPU context,
+ * by do_suspend_lowlevel().
+ */
+ 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());
+ smpboot_control = smp_processor_id();
#endif
initial_code = (unsigned long)wakeup_long64;
- saved_magic = 0x123456789abcdef0L;
+ saved_magic = 0x123456789abcdef0L;
#endif /* CONFIG_64BIT */
/*
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 82c783da16a8..797ae1a15c91 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -108,6 +108,7 @@ static void __used common(void)
OFFSET(TSS_sp1, tss_struct, x86_tss.sp1);
OFFSET(TSS_sp2, tss_struct, x86_tss.sp2);
OFFSET(X86_top_of_stack, pcpu_hot, top_of_stack);
+ OFFSET(X86_current_task, pcpu_hot, current_task);
#ifdef CONFIG_CALL_DEPTH_TRACKING
OFFSET(X86_call_depth, pcpu_hot, call_depth);
#endif
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 222efd4a09bc..5a2417d788d1 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -61,8 +61,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
@@ -241,6 +241,24 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
UNWIND_HINT_EMPTY
ANNOTATE_NOENDBR // above
+#ifdef CONFIG_SMP
+ movl smpboot_control(%rip), %ecx
+
+ /* 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 */
+
+ /*
+ * 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
@@ -275,12 +293,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
-
/* Setup and Load IDT */
pushq %rsi
call early_setup_idt
@@ -372,7 +384,11 @@ SYM_CODE_END(secondary_startup_64)
SYM_CODE_START(start_cpu0)
ANNOTATE_NOENDBR
UNWIND_HINT_EMPTY
- movq initial_stack(%rip), %rsp
+
+ /* Find the idle task stack */
+ movq PER_CPU_VAR(pcpu_hot) + X86_current_task, %rcx
+ movq TASK_threadsp(%rcx), %rsp
+
jmp .Ljump_to_C_code
SYM_CODE_END(start_cpu0)
#endif
@@ -420,12 +436,6 @@ 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
-
-/*
- * 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)
__FINITDATA
__INIT
@@ -660,6 +670,9 @@ SYM_DATA_END(level1_fixmap_pgt)
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
/* This must match the first entry in level2_kernel_pgt */
SYM_DATA(phys_base, .quad 0x0)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b18c1385e181..62e3bf37f0b8 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1112,7 +1112,12 @@ static int do_boot_cpu(int apicid, int cpu, struct task_struct *idle,
idle->thread.sp = (unsigned long)task_pt_regs(idle);
early_gdt_descr.address = (unsigned long)get_cpu_gdt_rw(cpu);
initial_code = (unsigned long)start_secondary;
- initial_stack = idle->thread.sp;
+
+ if (IS_ENABLED(CONFIG_X86_32)) {
+ initial_stack = idle->thread.sp;
+ } else {
+ smpboot_control = cpu;
+ }
/* Enable the espfix hack for this CPU */
init_espfix_ap(cpu);
diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index ffaa62167f6e..6bd391476656 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.0
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
next prev parent reply other threads:[~2023-02-28 17:10 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-26 11:07 [PATCH v12 00/11] Parallel CPU bringup for x86_64 Usama Arif
2023-02-26 11:07 ` [PATCH v12 01/11] x86/apic/x2apic: Allow CPU cluster_mask to be populated in parallel Usama Arif
2023-02-26 11:07 ` [PATCH v12 02/11] cpu/hotplug: Move idle_thread_get() to <linux/smpboot.h> Usama Arif
2023-02-26 11:07 ` [PATCH v12 03/11] cpu/hotplug: Add dynamic parallel bringup states before CPUHP_BRINGUP_CPU Usama Arif
2023-02-26 11:07 ` [PATCH v12 04/11] x86/smpboot: Reference count on smpboot_setup_warm_reset_vector() Usama Arif
2023-02-26 11:07 ` [PATCH v12 05/11] x86/smpboot: Split up native_cpu_up into separate phases and document them Usama Arif
2023-02-26 11:07 ` [PATCH v12 06/11] x86/smpboot: Remove initial_stack on 64-bit Usama Arif
2023-02-28 16:13 ` Thomas Gleixner
2023-02-28 16:25 ` David Woodhouse
2023-02-28 20:32 ` Thomas Gleixner
2023-02-28 17:09 ` David Woodhouse [this message]
2023-02-28 20:17 ` Thomas Gleixner
2023-02-28 20:43 ` David Woodhouse
2023-03-01 10:18 ` Thomas Gleixner
2023-03-01 10:42 ` David Woodhouse
2023-02-28 22:22 ` Brian Gerst
2023-02-26 11:07 ` [PATCH v12 07/11] x86/smpboot: Remove early_gdt_descr " Usama Arif
2023-02-28 21:01 ` Thomas Gleixner
2023-02-28 21:02 ` Arjan van de Ven
2023-03-01 20:32 ` Josh Triplett
2023-03-01 22:16 ` Paul E. McKenney
2023-03-01 22:25 ` Josh Triplett
2023-03-02 0:28 ` Paul E. McKenney
2023-03-02 1:05 ` Randy Dunlap
2023-03-02 1:37 ` Paul E. McKenney
2023-02-28 21:57 ` David Woodhouse
2023-02-28 22:41 ` David Woodhouse
2023-02-28 22:48 ` Brian Gerst
2023-02-28 23:42 ` David Woodhouse
2023-03-01 0:02 ` H. Peter Anvin
2023-03-01 7:39 ` David Woodhouse
2023-03-01 0:03 ` Brian Gerst
2023-03-01 10:27 ` Thomas Gleixner
2023-02-28 22:09 ` Brian Gerst
2023-02-26 11:07 ` [PATCH v12 08/11] x86/smpboot: Remove initial_gs Usama Arif
2023-02-26 11:08 ` [PATCH v12 09/11] x86/smpboot: Support parallel startup of secondary CPUs Usama Arif
2023-02-26 11:08 ` [PATCH v12 10/11] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel Usama Arif
2023-02-26 11:08 ` [PATCH v12 11/11] x86/smpboot: Serialize topology updates for secondary bringup Usama Arif
2023-02-26 18:31 ` [PATCH v12 00/11] Parallel CPU bringup for x86_64 Oleksandr Natalenko
2023-02-26 20:59 ` [External] " Usama Arif
2023-02-27 6:13 ` David Woodhouse
2023-02-27 6:25 ` Usama Arif
2023-02-27 6:14 ` Usama Arif
2023-02-27 15:29 ` David Woodhouse
2023-02-27 16:32 ` Usama Arif
2023-02-27 21:39 ` Guilherme G. Piccoli
2023-02-28 9:07 ` David Woodhouse
2023-02-28 10:13 ` Paul Menzel
2023-02-28 12:04 ` Guilherme G. Piccoli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c6863590f5fbf139f6aec50d0f3bc8e8b00cfcaf.camel@infradead.org \
--to=dwmw2@infradead.org \
--cc=arjan@linux.intel.com \
--cc=bp@alien8.de \
--cc=brgerst@gmail.com \
--cc=dave.hansen@linux.intel.com \
--cc=fam.zheng@bytedance.com \
--cc=hewenliang4@huawei.com \
--cc=hpa@zytor.com \
--cc=kim.phillips@amd.com \
--cc=kvm@vger.kernel.org \
--cc=liangma@liangbit.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mimoja@mimoja.de \
--cc=mingo@redhat.com \
--cc=oleksandr@natalenko.name \
--cc=paulmck@kernel.org \
--cc=pbonzini@redhat.com \
--cc=piotrgorski@cachyos.org \
--cc=pmenzel@molgen.mpg.de \
--cc=punit.agrawal@bytedance.com \
--cc=rcu@vger.kernel.org \
--cc=seanjc@google.com \
--cc=simon.evans@bytedance.com \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
--cc=usama.arif@bytedance.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®