Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from boot

[Usama Arif]

On 25/02/2023 13:52, David Woodhouse wrote:
> On Sat, 2023-02-25 at 13:33 +0000, Usama Arif wrote:
> > Yeah looks good! I am testing with the macro diff for tr_lock from
> > https://lore.kernel.org/all/05e27a7a-1faa-944e-2764-6ab4d620fb8f@xxxxxxxxxxxxx/.
> > If it all works, happy for me to send out v12 with it?
>
> I moved the macro definition up a little to put it between the .code16
> and the .align, pushed it out as a commit on top of the above branch.
>
> We'll collapse it into the 'Support parallel startup' patch, yes?

Yes, collapsed with "Support parallel startup of secondary CPUs" patch. 
I think Thomas' solution to dealing with suspend might be better? So I 
was thinking of sending v12 on top of v6.2 release with the following 
diff over your branch (merged in the right commit ofcourse):

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index defe76ee9e64..97a36d029b0e 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -205,6 +205,4 @@ extern unsigned int smpboot_control;
 #define STARTUP_APICID_CPUID_0B        0x80000000
 #define STARTUP_APICID_CPUID_01        0x40000000

-#define STARTUP_PARALLEL_MASK (STARTUP_APICID_CPUID_01 | 
STARTUP_APICID_CPUID_0B)
-
 #endif /* _ASM_X86_SMP_H */
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 5dcf5ca15383..214dd4a79860 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -58,6 +58,7 @@ asmlinkage acpi_status __visible 
x86_acpi_enter_sleep_state(u8 state)
  */
 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);

@@ -113,16 +114,11 @@ int x86_acpi_suspend_lowlevel(void)
 #else /* CONFIG_64BIT */
 #ifdef CONFIG_SMP
        current->thread.sp = (unsigned long)temp_stack + 
sizeof(temp_stack);
-       /*
-        * Ensure the CPU knows which one it is when it comes back, if
-        * it isn't in parallel mode and expected to work that out for
-        * itself.
-        */
-       if (!(smpboot_control & STARTUP_PARALLEL_MASK))
-               smpboot_control = smp_processor_id();
+       /* Force the startup into boot mode */
+       saved_smpboot_ctrl = xchg(&smpboot_control, 0);
 #endif
        initial_code = (unsigned long)wakeup_long64;
-       saved_magic = 0x123456789abcdef0L;
+       saved_magic = 0x123456789abcdef0L;
 #endif /* CONFIG_64BIT */

        /*
@@ -132,6 +128,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;
 }


> > > I can probably work out how to test the AMD SEV path; is there a
> > > way I can test the startup_64 MADT one?
> >
> > I guess the easiest way is to create a TDX VM on Sapphire Rapids which I
> > believe mostly Intel people have access to right now? Maybe we can post
> > v12 and someone from Intel could just quickly verify if it boots with
> > it? I have added Yuan from the other thread in here who pointed it out
> > initially.
>
> Yeah, I should also be able to rustle up both SPR and SEV-SNP if I dig
> around at work a little.

Sounds good!