On Thu, 2023-02-09 at 12:53 +0100, Thomas Gleixner wrote:
> On Thu, Feb 09 2023 at 11:03, David Woodhouse wrote:
> > This one also fixes it for me. If we're happy with this approach, I'll
> > work it into Thomas's original patch (and hopefully eventually he'll be
> > happy enough with it and the commit message that he'll give us his
> > Signed-off-by for it.)
>
> I'm happy enough by now, but I'm not sure how much of the original patch
> is still left. Also you did the heavy lifting of making it work and
> writing the nice changelog. So please make this:
>
> From: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
>
> Co-developed-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Signed-off-by: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
Thanks. I'll flip that to the Amazon address, of course. It's useless
for actual email (until I apply that LART some more) but I should still
use it for that.
I think I'm going to make one more change to that as I review it; in
the "should never happen" case of not finding the APIC ID in the
cpuid_to_apicid[] array it would just keep searching for ever. I don't
know if there's a better thing to do other than just dropping the
trampoline lock and 1:cli;hlt;jmp 1b but at least it's *attempting* to
handle the failure.
Patch below, and I'll update the tree shortly. There's a "what if
there's noise in the top 32 bits of %rcx" fix in there too.
> > I could probably add a Co-developed-by: tglx for that first x2apic
> > patch in the series too, but then it would *also* need his SoB and I
> > didn't want to be owed two, so I just pasted his suggested code and
> > didn't credit him.
>
> That's what Suggested-by: is for. For that I don't owe you anything. :)
Well, I didn't think that was really for suggestions which come in
'diff -up' form, but OK :)
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -280,15 +280,25 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
cpuid
.Lsetup_AP:
- /* EDX contains the APICID of the current CPU */
- xorl %ecx, %ecx
+ /* EDX contains the APIC ID of the current CPU */
+ xorq %rcx, %rcx
leaq cpuid_to_apicid(%rip), %rbx
.Lfind_cpunr:
cmpl (%rbx,%rcx,4), %edx
jz .Linit_cpu_data
- inc %rcx
- jmp .Lfind_cpunr
+ inc %ecx
+ cmpl nr_cpu_ids(%rip), %ecx
+ jb .Lfind_cpunr
+
+ /* APIC ID not found in the table. Drop the trampoline lock and bail. */
+ movq trampoline_lock(%rip), %rax
+ lock
+ btrl $0, (%rax)
+
+1: cli
+ hlt
+ jmp 1b
.Linit_cpu_data:
/* Get the per cpu offset for the given CPU# which is in ECX */
@@ -303,9 +313,7 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
movq %rcx, early_gdt_descr_base(%rip)
/* Find the idle task stack */
- movq $idle_threads, %rcx
- addq %rbx, %rcx
- movq (%rcx), %rcx
+ movq idle_threads(%rbx), %rcx
movq TASK_threadsp(%rcx), %rcx
movq %rcx, initial_stack(%rip)
#endif /* CONFIG_SMP */
@@ -450,7 +458,14 @@ SYM_CODE_END(secondary_startup_64)
SYM_CODE_START(start_cpu0)
ANNOTATE_NOENDBR
UNWIND_HINT_EMPTY
- movq initial_stack(%rip), %rsp
+ /* Load the per-cpu base for CPU#0 */
+ leaq __per_cpu_offset(%rip), %rbx
+ movq (%rbx), %rbx
+
+ /* Find the idle task stack */
+ movq idle_threads(%rbx), %rcx
+ movq TASK_threadsp(%rcx), %rsp
+
jmp .Ljump_to_C_code
SYM_CODE_END(start_cpu0)
#endif
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
