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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d6d4096c-2373-aafc-ed17-e44e351e66e1@bytedance.com>
Date:   Sat, 25 Feb 2023 22:15:02 +0000
From:   Usama Arif <usama.arif@...edance.com>
To:     David Woodhouse <dwmw2@...radead.org>,
        Brian Gerst <brgerst@...il.com>,
        Yuan Yao <yuan.yao@...ux.intel.com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: [External] [PATCH v2 0/5] x86-64: Remove global variables from
 boot



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!