Re: [PATCH v15 09/12] x86/smpboot: Support parallel startup of secondary CPUs

From: David Woodhouse
Date: Tue Mar 21 2023 - 15:14:22 EST


On Tue, 2023-03-21 at 14:28 -0400, Brian Gerst wrote:
>
> > @@ -264,6 +318,14 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
> >          lgdt    (%rsp)
> >          addq    $16, %rsp
> >
> > +       /* Drop the realmode protection. For the boot CPU the pointer is NULL! */
> > +       movq    trampoline_lock(%rip), %rax
> > +       testq   %rax, %rax
> > +       jz      .Lsetup_data_segments
> > +       lock
> > +       btrl    $0, (%rax)
> > +
> > +.Lsetup_data_segments:
> >          /* set up data segments */
> >          xorl %eax,%eax
> >          movl %eax,%ds
>
> This can still go earlier, right after "movq TASK_threadsp(%rax),
> %rsp".  The GDT descriptor is placed on the idle thread stack, so it's
> safe to drop the lock before it.


--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -335,6 +335,17 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
movq pcpu_hot + X86_current_task(%rdx), %rax
movq TASK_threadsp(%rax), %rsp

+ /*
+ * Now that this CPU is running on its own stack, drop the realmode
+ * protection. For the boot CPU the pointer is NULL!
+ */
+ movq trampoline_lock(%rip), %rax
+ testq %rax, %rax
+ jz .Lsetup_gdt
+ lock
+ btrl $0, (%rax)
+
+.Lsetup_gdt:
/*
* We must switch to a new descriptor in kernel space for the GDT
* because soon the kernel won't have access anymore to the userspace
@@ -377,14 +388,6 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
shrq $32, %rdx
wrmsr

- /* Drop the realmode protection. For the boot CPU the pointer is NULL! */
- movq trampoline_lock(%rip), %rax
- testq %rax, %rax
- jz .Lsetup_idt
- lock
- btrl $0, (%rax)
-
-.Lsetup_idt:
/* Setup and Load IDT */
pushq %rsi
call early_setup_idt

Attachment: smime.p7s
Description: S/MIME cryptographic signature