All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Mitchell <kevmitch@arista.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>,
	Kevin Mitchell <kevmitch@arista.com>,
	stable@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Tony Luck <tony.luck@intel.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	David Rientjes <rientjes@google.com>,
	Dou Liyang <douly.fnst@cn.fujitsu.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] x86/mpparse: avoid overwriting boot_cpu_physical_apicid
Date: Mon,  8 Jun 2020 17:44:48 -0700	[thread overview]
Message-ID: <20200609004451.1296880-1-kevmitch@arista.com> (raw)

When booting with ACPI unavailable or disabled, get_smp_config() ends up
calling MP_processor_info() for each CPU found in the MPS
table. Previously, this resulted in boot_cpu_physical_apicid getting
unconditionally overwritten by the apicid of whatever processor had the
CPU_BOOTPROCESSOR flag. This occurred even if boot_cpu_physical_apicid
had already been more reliably determined in register_lapic_address() by
calling read_apic_id() from the actual boot processor.

Ordinariliy, this is not a problem because the boot processor really is
the one with the CPU_BOOTPROCESSOR flag. However, kexec is an exception
in which the kernel may be booted from any processor regardless of the
MPS table contents. In this case, boot_cpu_physical_apicid may not
indicate the actual boot processor.

This was particularly problematic when the second kernel was booted with
NR_CPUS fewer than the number of physical processors. It's the job of
generic_processor_info() to decide which CPUs to bring up in this case.
That obviously must include the real boot processor which it takes care
to save a slot for. It relies upon the contents of
boot_cpu_physical_apicid to do this, which if incorrect, may result in
the boot processor getting left out.

This condition can be discovered by smp_sanity_check() and rectified by
adding the boot processor to the phys_cpu_present_map with the warning
"weird, boot CPU (#%d) not listed by the BIOS". However, commit
3e730dad3b6da ("x86/apic: Unify interrupt mode setup for SMP-capable
system") caused setup_local_APIC() to be called before this could happen
resulting in a BUG_ON(!apic->apic_id_registered()):

[    0.655452] ------------[ cut here ]------------
[    0.660610] Kernel BUG at setup_local_APIC+0x74/0x280 [verbose debug info unavailable]
[    0.669466] invalid opcode: 0000 [#1] SMP
[    0.673948] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.19.109.Ar-16509018.eostrunkkernel419 #1
[    0.683670] Hardware name: Quanta Quanta LY6 (1LY6UZZ0FBC), BIOS 1.0.6.0-e7d6a55 11/26/2015
[    0.693007] RIP: 0010:setup_local_APIC+0x74/0x280
[    0.698264] Code: 80 e4 fe bf f0 00 00 00 89 c6 48 8b 05 0f 1a 8e 00 ff 50 10 e8 12 53 fd ff 48 8b 05 00 1a 8e 00 ff 90 a0 00 00 00 85 c0 75 02 <0f> 0b 48 8b 05 ed 19 8e 00 41 be 00 02 00 00 ff 90 b0 00 00 00 48
[    0.719251] RSP: 0000:ffffffff81a03e20 EFLAGS: 00010246
[    0.725091] RAX: 0000000000000000 RBX: 0000000000000003 RCX: 0000000000000000
[    0.733066] RDX: 0000000000000000 RSI: 000000000000000f RDI: 0000000000000020
[    0.741041] RBP: ffffffff81a03e98 R08: 0000000000000002 R09: 0000000000000000
[    0.749014] R10: ffffffff81a204e0 R11: ffffffff81b50ea7 R12: 0000000000000000
[    0.756989] R13: ffffffff81aef920 R14: ffffffff81af60a0 R15: 0000000000000000
[    0.764965] FS:  0000000000000000(0000) GS:ffff888036800000(0000) knlGS:0000000000000000
[    0.774007] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.780427] CR2: ffff888035c01000 CR3: 0000000035a08000 CR4: 00000000000006b0
[    0.788401] Call Trace:
[    0.791137]  ? amd_iommu_prepare+0x15/0x2a
[    0.795717]  apic_bsp_setup+0x55/0x75
[    0.799808]  apic_intr_mode_init+0x169/0x16e
[    0.804579]  x86_late_time_init+0x10/0x17
[    0.809062]  start_kernel+0x37e/0x3fe
[    0.813154]  x86_64_start_reservations+0x2a/0x2c
[    0.818316]  x86_64_start_kernel+0x72/0x75
[    0.822886]  secondary_startup_64+0xa4/0xb0
[    0.827564] ---[ end trace 237b64da0fd9b22e ]---

This change avoids these issues by only setting boot_cpu_physical_apicid
from the MPS table if it is not already set, which can occur in the
construct_default_ISA_mptable() path. Otherwise,
boot_cpu_physical_apicid will already have been set in
register_lapic_address() and should therefore remain untouched.

Looking through all the places where boot_cpu_physical_apicid is
accessed, nearly all of them assume that boot_cpu_physical_apicid should
match read_apic_id() on the booting processor. The only place that might
intend to use the BSP apicid listed in the MPS table is amd_numa_init(),
which explicitly requires boot_cpu_physical_apicid to be the lowest
apicid of all processors. Ironically, due to the early exit short
circuit in early_get_smp_config(), it instead gets
boot_cpu_physical_apicid = read_apic_id() rather than the MPS table
BSP. The behaviour of amd_numa_init() is therefore unaffected by this
change.

Fixes: 3e730dad3b6da ("x86/apic: Unify interrupt mode setup for SMP-capable system")
Signed-off-by: Kevin Mitchell <kevmitch@arista.com>
Cc: <stable@vger.kernel.org>
---
 arch/x86/kernel/mpparse.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/mpparse.c b/arch/x86/kernel/mpparse.c
index afac7ccce72f..6f22f09bfe11 100644
--- a/arch/x86/kernel/mpparse.c
+++ b/arch/x86/kernel/mpparse.c
@@ -64,7 +64,8 @@ static void __init MP_processor_info(struct mpc_cpu *m)
 
 	if (m->cpuflag & CPU_BOOTPROCESSOR) {
 		bootup_cpu = " (Bootup-CPU)";
-		boot_cpu_physical_apicid = m->apicid;
+		if (boot_cpu_physical_apicid == -1U)
+			boot_cpu_physical_apicid = m->apicid;
 	}
 
 	pr_info("Processor #%d%s\n", m->apicid, bootup_cpu);
-- 
2.26.2


             reply	other threads:[~2020-06-09  0:46 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-09  0:44 Kevin Mitchell [this message]
2020-06-09  0:44 ` [PATCH 2/2] x86/apic: remove boot_cpu_physical_apicid hacks Kevin Mitchell
2022-05-21  0:32 ` [PATCH 1/2] x86/mpparse: avoid overwriting boot_cpu_physical_apicid Kevin Mitchell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200609004451.1296880-1-kevmitch@arista.com \
    --to=kevmitch@arista.com \
    --cc=bp@alien8.de \
    --cc=d.hatayama@jp.fujitsu.com \
    --cc=douly.fnst@cn.fujitsu.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=wangkefeng.wang@huawei.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.