[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMzpN2ijBZV0d+FjXpi8iP9rQ2gFgQ-9TtkZMB4xDizox1yW3g@mail.gmail.com>
Date: Tue, 21 Mar 2023 17:02:27 -0400
From: Brian Gerst <brgerst@...il.com>
To: David Woodhouse <dwmw2@...radead.org>
Cc: Usama Arif <usama.arif@...edance.com>, tglx@...utronix.de,
kim.phillips@....com, piotrgorski@...hyos.org,
oleksandr@...alenko.name, arjan@...ux.intel.com, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com,
x86@...nel.org, pbonzini@...hat.com, paulmck@...nel.org,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
rcu@...r.kernel.org, mimoja@...oja.de, hewenliang4@...wei.com,
thomas.lendacky@....com, seanjc@...gle.com, pmenzel@...gen.mpg.de,
fam.zheng@...edance.com, punit.agrawal@...edance.com,
simon.evans@...edance.com, liangma@...ngbit.com,
gpiccoli@...lia.com
Subject: Re: [PATCH v15 09/12] x86/smpboot: Support parallel startup of
secondary CPUs
On Tue, Mar 21, 2023 at 3:12 PM David Woodhouse <dwmw2@...radead.org> wrote:
>
> 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
>
Looks good, thanks.
Powered by blists - more mailing lists