On 21/02/2023 19:10, David Woodhouse wrote:
On Tue, 2023-02-21 at 13:14 +0100, Oleksandr Natalenko wrote:
With this in place:
```
early_gdt_descr.address = (unsigned long)get_cpu_gdt_rw(0);
initial_gs = per_cpu_offset(0);
smpboot_control = 0;
```
the resume does not work.
Yeah, I think it's always running on CPU0 after the other CPUs are
taken down anyway.
We definitely *do* need to clear smpboot_control because we absolutely
want it using the temp_stack we explicitly put into initial_stack, not
finding its own idle thread.
The problem was that it was never being restored to STARTUP_SECONDARY
in the parallel modes, because that's a one-time setup in
native_smp_prepare_cpus(). So we can just restore it in
arch_thaw_secondary_cpus_begin() too, by working this into patch 6 of
the series.
(Usama, I think my tree is fairly out of date now so I'll let you do
that? Thanks!).
Sounds good! Will send out the next revision with below diff, checkpatch
fixes and rebased to 6.2 (testing it now). The below fix looks good!
Oleksandr, would you mind testing out suspend/resume with the below diff
on your AMD machine just to make sure it fixes the issue before I send
out the next revision with it. Thanks!
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 50621793671d..3db77a257ae2 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1571,6 +1571,17 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
void arch_thaw_secondary_cpus_begin(void)
{
+ /*
+ * On suspend, smpboot_control will have been zeroed to allow the
+ * boot CPU to use explicitly passed values including a temporary
+ * stack. Since native_smp_prepare_cpus() won't be called again,
+ * restore the appropriate value for the parallel startup modes.
+ */
+ if (do_parallel_bringup) {
+ smpboot_control = STARTUP_SECONDARY |
+ (x2apic_mode ? STARTUP_APICID_CPUID_0B : STARTUP_APICID_CPUID_01);
+ }
+
set_cache_aps_delayed_init(true);
}
