Messages in this thread Patch in this message | | | Subject | Re: [External] Re: [PATCH v9 0/8] Parallel CPU bringup for x86_64 | From | David Woodhouse <> | Date | Wed, 22 Feb 2023 08:19:27 +0000 |
| |
On Wed, 2023-02-22 at 00:00 +0000, Usama Arif wrote: > > > 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?
It is CPUID_0B | CPUID_01, unless I was being really stupid last night. Sips coffee... yep, 6 = 4 | 2. I could have made that more obvious with a bit more typing, I suppose.
However, I don't think this approach is correct. The idle thread stacks for the other CPUs are *unused* because those CPUs are offline. So we can use them with impunity, and when those CPUs come back online they'll call into cpu_startup_entry(CPUHP_AP_ONLINE_IDLE) using the correct stack.
But the BSP/CPU0 is different. It hasn't actually been taken offline, and its idle thread context is still in cpu_startup_entry(CPUHP_ONLINE) which got called from rest_init().
In testing I probably got away with it because we're only using the *top* of the stack, don't use anything of the red zone, and thus don't actually bother the true idle thread which is never going to return. But I don't think it's correct; we really ought to have that temp_stack unless we're going to refactor the wakeup_64 code to *become* the idle thread just as startup_secondary() does, and *schedule* to the context that was saved in the suspend code. That might be an interesting cleanup, and let us use the normal __switch_to() to save and restore a bunch of context which is currently done by hand.
But not today. [unhandled content-type:application/pkcs7-signature] | |