| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aac036a17b1bcbabe8ee5a7c69fb2dfbc546d06e.camel@infradead.org>
Date: Tue, 21 Feb 2023 23:18:40 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: Thomas Gleixner <tglx@...utronix.de>,
Oleksandr Natalenko <oleksandr@...alenko.name>
Cc: Kim Phillips <kim.phillips@....com>,
Usama Arif <usama.arif@...edance.com>, arjan@...ux.intel.com,
mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
hpa@...or.com, x86@...nel.org, pbonzini@...hat.com,
paulmck@...nel.org, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, rcu@...r.kernel.org, mimoja@...oja.de,
hewenliang4@...wei.com, thomas.lendacky@....com, seanjc@...gle.com,
pmenzel@...gen.mpg.de, fam.zheng@...edance.com,
punit.agrawal@...edance.com, simon.evans@...edance.com,
liangma@...ngbit.com,
"Limonciello, Mario" <Mario.Limonciello@....com>,
Piotr Gorski <piotrgorski@...hyos.org>
Subject: Re: [PATCH v9 0/8] Parallel CPU bringup for x86_64
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
+
#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 */
/*
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)
Powered by blists - more mailing lists