Skip to content

Commit

Permalink
x86/apic: Consolidate boot_cpu_physical_apicid initialization sites
Browse files Browse the repository at this point in the history
boot_cpu_physical_apicid is written in random places and in the last
consequence filled with the APIC ID read from the local APIC. That causes
it to have inconsistent state when the MPTABLE is broken. As a consequence
tons of moronic checks are sprinkled all over the place.

Consolidate the code and read it exactly once when either X2APIC mode is
detected early or when the APIC mapping is established.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Michael Kelley <mikelley@microsoft.com>
Tested-by: Sohil Mehta <sohil.mehta@intel.com>
Tested-by: Juergen Gross <jgross@suse.com> # Xen PV (dom0 and unpriv. guest)
  • Loading branch information
Thomas Gleixner authored and Dave Hansen committed Aug 9, 2023
1 parent 1d90c9f commit d10a904
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 74 deletions.
2 changes: 1 addition & 1 deletion arch/x86/include/asm/apic.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ static inline int x2apic_enabled(void)
#else /* !CONFIG_X86_X2APIC */
static inline void x2apic_setup(void) { }
static inline int x2apic_enabled(void) { return 0; }

static inline u32 native_apic_msr_read(u32 reg) { BUG(); }
#define x2apic_mode (0)
#define x2apic_supported() (0)
#endif /* !CONFIG_X86_X2APIC */
Expand Down
102 changes: 32 additions & 70 deletions arch/x86/kernel/apic/apic.c
Original file line number Diff line number Diff line change
Expand Up @@ -1318,8 +1318,7 @@ static int __init __apic_intr_mode_select(void)
if (!boot_cpu_has(X86_FEATURE_APIC) &&
APIC_INTEGRATED(boot_cpu_apic_version)) {
apic_is_disabled = true;
pr_err(FW_BUG "Local APIC %d not detected, force emulation\n",
boot_cpu_physical_apicid);
pr_err(FW_BUG "Local APIC not detected, force emulation\n");
return APIC_PIC;
}
#endif
Expand All @@ -1340,12 +1339,6 @@ static int __init __apic_intr_mode_select(void)
pr_info("APIC: SMP mode deactivated\n");
return APIC_SYMMETRIC_IO_NO_ROUTING;
}

if (read_apic_id() != boot_cpu_physical_apicid) {
panic("Boot APIC ID in local APIC unexpected (%d vs %d)",
read_apic_id(), boot_cpu_physical_apicid);
/* Or can we switch back to PIC here? */
}
#endif

return APIC_SYMMETRIC_IO;
Expand Down Expand Up @@ -1741,6 +1734,23 @@ void apic_ap_setup(void)
end_local_APIC_setup();
}

static __init void apic_read_boot_cpu_id(bool x2apic)
{
/*
* This can be invoked from check_x2apic() before the APIC has been
* selected. But that code knows for sure that the BIOS enabled
* X2APIC.
*/
if (x2apic) {
boot_cpu_physical_apicid = native_apic_msr_read(APIC_ID);
boot_cpu_apic_version = GET_APIC_VERSION(native_apic_msr_read(APIC_LVR));
} else {
boot_cpu_physical_apicid = read_apic_id();
boot_cpu_apic_version = GET_APIC_VERSION(apic_read(APIC_LVR));
}
}


#ifdef CONFIG_X86_X2APIC
int x2apic_mode;
EXPORT_SYMBOL_GPL(x2apic_mode);
Expand Down Expand Up @@ -1921,6 +1931,7 @@ void __init check_x2apic(void)
x2apic_state = X2APIC_ON_LOCKED;
else
x2apic_state = X2APIC_ON;
apic_read_boot_cpu_id(true);
} else if (!boot_cpu_has(X86_FEATURE_X2APIC)) {
x2apic_state = X2APIC_DISABLED;
}
Expand Down Expand Up @@ -2109,15 +2120,11 @@ static int __init detect_init_APIC(void)
*/
void __init init_apic_mappings(void)
{
unsigned int new_apicid;

if (apic_validate_deadline_timer())
pr_info("TSC deadline timer available\n");

if (x2apic_mode) {
boot_cpu_physical_apicid = read_apic_id();
if (x2apic_mode)
return;
}

/* If no local APIC can be found return early */
if (!smp_found_config && detect_init_APIC()) {
Expand All @@ -2134,39 +2141,19 @@ void __init init_apic_mappings(void)
if (!acpi_lapic && !smp_found_config)
register_lapic_address(apic_phys);
}

/*
* Fetch the APIC ID of the BSP in case we have a
* default configuration (or the MP table is broken).
*/
new_apicid = read_apic_id();
if (boot_cpu_physical_apicid != new_apicid) {
boot_cpu_physical_apicid = new_apicid;
/*
* yeah -- we lie about apic_version
* in case if apic was disabled via boot option
* but it's not a problem for SMP compiled kernel
* since apic_intr_mode_select is prepared for such
* a case and disable smp mode
*/
boot_cpu_apic_version = GET_APIC_VERSION(apic_read(APIC_LVR));
}
}

void __init register_lapic_address(unsigned long address)
{
mp_lapic_addr = address;

if (!x2apic_mode) {
set_fixmap_nocache(FIX_APIC_BASE, address);
apic_mmio_base = APIC_BASE;
apic_printk(APIC_VERBOSE, "mapped APIC to %16lx (%16lx)\n",
APIC_BASE, address);
}
if (boot_cpu_physical_apicid == -1U) {
boot_cpu_physical_apicid = read_apic_id();
boot_cpu_apic_version = GET_APIC_VERSION(apic_read(APIC_LVR));
}
if (x2apic_mode)
return;

set_fixmap_nocache(FIX_APIC_BASE, address);
apic_mmio_base = APIC_BASE;
apic_printk(APIC_VERBOSE, "mapped APIC to %16lx (%16lx)\n", APIC_BASE, address);
apic_read_boot_cpu_id(false);
}

/*
Expand Down Expand Up @@ -2446,31 +2433,15 @@ int generic_processor_info(int apicid, int version)
phys_cpu_present_map);

/*
* boot_cpu_physical_apicid is designed to have the apicid
* returned by read_apic_id(), i.e, the apicid of the
* currently booting-up processor. However, on some platforms,
* it is temporarily modified by the apicid reported as BSP
* through MP table. Concretely:
*
* - arch/x86/kernel/mpparse.c: MP_processor_info()
* - arch/x86/mm/amdtopology.c: amd_numa_init()
*
* This function is executed with the modified
* boot_cpu_physical_apicid. So, disabled_cpu_apicid kernel
* parameter doesn't work to disable APs on kdump 2nd kernel.
*
* Since fixing handling of boot_cpu_physical_apicid requires
* another discussion and tests on each platform, we leave it
* for now and here we use read_apic_id() directly in this
* function, generic_processor_info().
* boot_cpu_physical_apicid is guaranteed to contain the boot CPU
* APIC ID read from the local APIC when this function is invoked.
*/
if (disabled_cpu_apicid != BAD_APICID &&
disabled_cpu_apicid != read_apic_id() &&
if (disabled_cpu_apicid != boot_cpu_physical_apicid &&
disabled_cpu_apicid == apicid) {
int thiscpu = num_processors + disabled_cpus;

pr_warn("APIC: Disabling requested cpu."
" Processor %d/0x%x ignored.\n", thiscpu, apicid);
pr_warn("APIC: Disabling requested cpu. Processor %d/0x%x ignored.\n",
thiscpu, apicid);

disabled_cpus++;
return -ENODEV;
Expand Down Expand Up @@ -2626,15 +2597,6 @@ static void __init apic_bsp_up_setup(void)
{
#ifdef CONFIG_X86_64
apic_write(APIC_ID, apic->set_apic_id(boot_cpu_physical_apicid));
#else
/*
* Hack: In case of kdump, after a crash, kernel might be booting
* on a cpu with non-zero lapic id. But boot_cpu_physical_apicid
* might be zero if read from MP tables. Get it from LAPIC.
*/
# ifdef CONFIG_CRASH_DUMP
boot_cpu_physical_apicid = read_apic_id();
# endif
#endif
physid_set_mask_of_physid(boot_cpu_physical_apicid, &phys_cpu_present_map);
}
Expand Down
4 changes: 1 addition & 3 deletions arch/x86/kernel/mpparse.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,8 @@ static void __init MP_processor_info(struct mpc_cpu *m)

apicid = m->apicid;

if (m->cpuflag & CPU_BOOTPROCESSOR) {
if (m->cpuflag & CPU_BOOTPROCESSOR)
bootup_cpu = " (Bootup-CPU)";
boot_cpu_physical_apicid = m->apicid;
}

pr_info("Processor #%d%s\n", m->apicid, bootup_cpu);
generic_processor_info(apicid, m->apicver);
Expand Down

0 comments on commit d10a904

Please sign in to comment.