On 21/02/2023 23:18, David Woodhouse wrote:
On Tue, 2023-02-21 at 22:41 +0100, Thomas Gleixner wrote:
@@ -57,6 +58,7 @@ asmlinkage acpi_status __visible x86_acp
*/
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);
@@ -115,7 +117,8 @@ int x86_acpi_suspend_lowlevel(void)
early_gdt_descr.address =
(unsigned long)get_cpu_gdt_rw(smp_processor_id());
initial_gs = per_cpu_offset(smp_processor_id());
- smpboot_control = 0;
+ /* Force the startup into boot mode */
+ saved_smpboot_ctrl = xchg(&smpboot_control, 0);
#endif
initial_code = (unsigned long)wakeup_long64;
saved_magic = 0x123456789abcdef0L;
@@ -128,6 +131,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;
}
But wait, why is this giving it a dedicated temp_stack anyway? Why
can't it use that CPU's idle thread stack like we usually do? I already
made idle_thread_get() accessible from here. So we could do this...
@@ -111,14 +112,16 @@ int x86_acpi_suspend_lowlevel(void)
saved_magic = 0x12345678;
#else /* CONFIG_64BIT */
#ifdef CONFIG_SMP
- initial_stack = (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 = 0;
+ if (!(smpboot_control & STARTUP_PARALLEL_MASK)) {
+ unsigned int cpu = smp_processor_id();
+ initial_stack = (unsigned long)idle_thread_get(cpu)->thread.sp;
+ early_gdt_descr.address = (unsigned long)get_cpu_gdt_rw(cpu);
+ initial_gs = per_cpu_offset(cpu);
+ smpboot_control = 0;
+ }
#endif
initial_code = (unsigned long)wakeup_long64;
But that's a whole bunch of pointless, because it can be even further
simplified to just let the find its own crap like the secondaries do,
except in the 'OMG CPUID won't tell me' case where it has to be told:
So how about we just do something more like this. I'd *quite* like to
put the actual handling of smpboot_control into a function we call in
smpboot.c. and that whole x86_acpi_suspend_lowlevel() function wants
all its horrid 64bit/smp ifdefs fixed up (and is there any reason
there's a generic part saving CR0 and IA32_MISC_ENABLE right in the
middle of some !CONFIG_64BIT parts? I don't see ordering constraints
there). But this should work, I think:
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 33c0d5fd8af6..72b9375fec7c 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -208,4 +208,6 @@ extern unsigned int smpboot_control;
#define STARTUP_APICID_CPUID_0B 0x40000000
#define STARTUP_APICID_CPUID_01 0x20000000
+#define STARTUP_PARALLEL_MASK 0x60000000
+
Probably could define STARTUP_PARALLEL_MASK as STARTUP_APICID_CPUID_0B |
STARTUP_APICID_CPUID_01 instead? otherwise if its a separate bit, it
needs to be set in native_smp_prepare_cpus as well for this to work?
#endif /* _ASM_X86_SMP_H */
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 06adf340a0f1..a1343a900caf 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -16,17 +16,14 @@
#include <asm/cacheflush.h>
#include <asm/realmode.h>
#include <asm/hypervisor.h>
-
+#include <asm/smp.h>
+#include <linux/smpboot.h>
#include <linux/ftrace.h>
#include "../../realmode/rm/wakeup.h"
#include "sleep.h"
unsigned long acpi_realmode_flags;
-#if defined(CONFIG_SMP) && defined(CONFIG_64BIT)
-static char temp_stack[4096];
-#endif
-
/**
* acpi_get_wakeup_address - provide physical address for S3 wakeup
*
@@ -111,14 +108,11 @@ int x86_acpi_suspend_lowlevel(void)
saved_magic = 0x12345678;
#else /* CONFIG_64BIT */
#ifdef CONFIG_SMP
- initial_stack = (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 = 0;
+ if (!(smpboot_control & STARTUP_PARALLEL_MASK))
+ smpboot_control = STARTUP_SECONDARY | cpu_physical_id(smp_processor_id());
#endif
initial_code = (unsigned long)wakeup_long64;
- saved_magic = 0x123456789abcdef0L;
+ saved_magic = 0x123456789abcdef0L;
#endif /* CONFIG_64BIT */
/*
