Message-ID: <d6d4096c-2373-aafc-ed17-e44e351e66e1@bytedance.com>
Date:   Sat, 25 Feb 2023 22:15:02 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from
 boot
Content-Language: en-US
To:     David Woodhouse <dwmw2@infradead.org>,
        Brian Gerst <brgerst@gmail.com>,
        Yuan Yao <yuan.yao@linux.intel.com>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Andy Lutomirski <luto@kernel.org>,
        Ingo Molnar <mingo@kernel.org>
References: <20230224154235.277350-1-brgerst@gmail.com>
 <cfdb9c5a-4723-d920-511e-b57ae6c492f7@bytedance.com>
 <CAMzpN2hvPHWYOeyzfpRmk39XYwCrSJx0UyqxE48F1TjTNyoKAg@mail.gmail.com>
 <ca5788873c373249983ab6ac9ee173b12293641e.camel@infradead.org>
 <CAMzpN2ixYTKQaNoizkeNBrHQH_2SN9OMexnK9t3oVMDDCMwbOA@mail.gmail.com>
 <E940A2E7-6FAC-4E87-96DE-596BA3F505AF@infradead.org>
 <671dc8ad-2721-a3a3-7aa2-0e2558d5c953@bytedance.com>
 <1c64afc8769c222e27ee22a3758b6c32348949d0.camel@infradead.org>
From:   Usama Arif <usama.arif@bytedance.com>
In-Reply-To: <1c64afc8769c222e27ee22a3758b6c32348949d0.camel@infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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@bytedance.com/.
>> 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!