[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <970faea15f724939dbae47d309bffcb92d12c6c2.camel@infradead.org>
Date: Sat, 04 Feb 2023 09:07:21 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: Kim Phillips <kim.phillips@....com>,
Usama Arif <usama.arif@...edance.com>, tglx@...utronix.de,
arjan@...ux.intel.com
Cc: 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, Mario Limonciello <Mario.Limonciello@....com>
Subject: Re: [PATCH v6 07/11] x86/smpboot: Disable parallel boot for AMD CPUs
On Fri, 2023-02-03 at 13:48 -0600, Kim Phillips wrote:
> +Mario
>
> Hi,
>
> On 2/2/23 3:56 PM, Usama Arif wrote:
> > From: David Woodhouse <dwmw@...zon.co.uk>
> >
> > Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
> > ---
>
> I'd like to nack this, but can't (and not because it doesn't have
> commit text):
>
> If I:
>
> - take dwmw2's parallel-6.2-rc6 branch (commit 459d1c46dbd1)
> - remove the set_cpu_bug(c, X86_BUG_NO_PARALLEL_BRINGUP) line from amd.c
>
> Then:
>
> - a Ryzen 3000 (Picasso A1/Zen+) notebook I have access to fails to boot.
> - Zen 2,3,4-based servers boot fine
> - a Zen1-based server doesn't boot.
>
> This is what's left on its serial port:
>
> [ 3.199633] smp: Bringing up secondary CPUs ...
> [ 3.200732] x86: Booting SMP configuration:
> [ 3.204242] .... node #0, CPUs: #1
> [ 3.204301] CPU 1 to 93/x86/cpu:kick in 63 21 -114014307645 0 . 0 0 0 0 . 0 114025055970
> [ 3.204478] ------------[ cut here ]------------
> [ 3.204481] WARNING: CPU: 0 PID: 1 at arch/x86/kernel/cpu/common.c:2122 cpu_init+0x2d/0x1f0
> [ 3.204490] Modules linked in:
> [ 3.204493] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.2.0-rc6+ #19
> [ 3.204496] Hardware name: AMD Corporation Speedway/Speedway, BIOS RSW1009C 07/27/2018
> [ 3.204498] RIP: 0010:cpu_init+0x2d/0x1f0
> [ 3.204502] Code: e5 41 56 41 55 41 54 53 65 48 8b 1c 25 80 2e 1f 00 65 44 8b 35 20 e4 39 55 48 8b 05 5d f7 51 02 44 89 f2 f0 48 0f ab 10 73 06 <0f> 0b eb 02 f3 90 48 8b 05 3e f7 51 02 48 0f a3 10 73 f1 45 85 f6
> [ 3.204504] RSP: 0000:ffffffffac803d70 EFLAGS: 00010083
> [ 3.204506] RAX: ffff8d293eef6e40 RBX: ffff8d1d40010000 RCX: 0000000000000008
> [ 3.204508] RDX: 0000000000000000 RSI: ffff8d1d1c40b048 RDI: ffffffffac566418
> [ 3.204509] RBP: ffffffffac803d90 R08: 00000000fffffe14 R09: ffff8d1d1c406078
> [ 3.204510] R10: ffffffffac803dc0 R11: 0000000000000000 R12: 0000000000000000
> [ 3.204511] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 3.204512] FS: 0000000000000000(0000) GS:ffff8d1d1c400000(0000) knlGS:0000000000000000
> [ 3.204514] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 3.204515] CR2: 0000000000000000 CR3: 0000800daec12000 CR4: 00000000003100a0
> [ 3.204517] Call Trace:
> [ 3.204519] ---[ end trace 0000000000000000 ]---
> [ 3.204580] [Firmware Bug]: CPU0: APIC id mismatch. Firmware: 0 APIC: 2
So this is where it all starts to go wrong, and we didn't really change
this in the less-tested later part of the series. So even though I'd
like to be sure you've tested with just '-part1' to be sure, I suspect
that isn't the issue.
The warning you highlighted the end of the end of the log is just
complaining that a given AP is *already* marked as initialized when
it's trying to come up, and that's just fallout of the fact that they
don't know which CPU they are. They *all* come up thinking they're
CPU#0.
So that's weird. Now, what we do in this series is stop *telling* the
AP which CPU# it is in a global variable as we bring them up one at a
time, and instead we let them get their own APIC ID from CPUID leaf 0xb
and look up their per-cpu data that way.
The commit message for 'Support parallel startup of secondary CPUs'
(currently commit 0f52d4eaaf0c) explains that in a bit more detail.
Please could you try parallel-6.2-rc6-part1 with something like this?
If I boot this in qemu with a weird topology to stop it being a 1:1
mapping, I do see a series of BbCcDdDeEeFfIgJh... showing the APIC ID
and CPU# that each AP finds as it starts up.
qemu-system-x86_64 -kernel arch/x86/boot/bzImage -smp 24,sockets=4,cores=3,threads=2 -append "console=ttyS0,115200 lpj=16528321 earlyprintk=ttyS0" -serial mon:stdio -display none -m 1G -accel kvm,kernel-irqchip=split
...
[ 0.570022] x86: Booting SMP configuration:
BbCc[ 0.570923] .... node #0, CPUs: #1 #2
[ 0.711468] Callback from call_rcu_tasks_rude() invoked.
DdEe[ 0.713316] #3 #4
[ 0.854459] Callback from call_rcu_tasks() invoked.
FfIgJhKiLjMkNlQmRnSoTpUqVrYsZt[u\v]w^x[ 0.856289] #5 #6 #7 #8 #9 #10 #11 #12 #13 #14 #15 #16 #17 #18 #19 #20 #21 #22 #23
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index d07f694691d2..c3219dc2a201 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -247,8 +247,8 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
* Is this the boot CPU coming up? If so everything is available
* in initial_gs, initial_stack and early_gdt_descr.
*/
- movl smpboot_control(%rip), %eax
- testl %eax, %eax
+ movl smpboot_control(%rip), %edx
+ testl %edx, %edx
jz .Lsetup_cpu
/*
@@ -259,30 +259,45 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL)
* Bit 30 STARTUP_SECONDARY flag
* Bit 31 STARTUP_PARALLEL flag (use CPUID 0x0b for APIC ID)
*/
- testl $STARTUP_PARALLEL, %eax
+ testl $STARTUP_PARALLEL, %edx
jnz .Luse_cpuid_0b
- andl $0x0FFFFFFF, %eax
+ andl $0x0FFFFFFF, %edx
jmp .Lsetup_AP
.Luse_cpuid_0b:
mov $0x0B, %eax
xorl %ecx, %ecx
cpuid
- mov %edx, %eax
+
+/* test hack: print 'a' + APICID */
.Lsetup_AP:
- /* EAX contains the APICID of the current CPU */
+ /* EDX contains the APICID of the current CPU */
+
+ /* Test hack: Print APIC ID and then CPU# when we find it. */
+ mov %edx, %ecx
+ mov %edx, %eax
+ addb $'A', %al
+ mov $0x3f8, %dx
+ outb %al, %dx
+ mov %ecx, %edx
+ mov $'a', %al
+
xorl %ecx, %ecx
leaq cpuid_to_apicid(%rip), %rbx
.Lfind_cpunr:
- cmpl (%rbx), %eax
+ cmpl (%rbx), %edx
jz .Linit_cpu_data
addq $4, %rbx
addq $8, %rcx
+ addb $1, %al
jmp .Lfind_cpunr
.Linit_cpu_data:
+ mov $0x3f8, %dx
+ outb %al, %dx
+
/* Get the per cpu offset */
leaq __per_cpu_offset(%rip), %rbx
addq %rcx, %rbx
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)
Powered by blists - more mailing lists