Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
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
Message-Id: <20200609004451.1296880-1-kevmitch@arista.com>
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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