From b7d69d88044d88c06a301f099aedeb1eaaf90aa7 Mon Sep 17 00:00:00 2001 From: Travis Geiselbrecht Date: Mon, 22 Sep 2025 20:57:30 -0700 Subject: [PATCH] [arch][x86] handle the local apic of the boot cpu not being 0 I have a bulldozer machine here that curiously starts the APIC IDs for the cpus at 16 and counts up. This is a problem since the current code assumes that the boot cpu is 0, and would try to start itself (apic id 16) later because it thought it was the first secondary. Fix this by re-reading the APIC id on the boot cpu and patching the percpu structure a bit into boot. Kinda a hack but avoids having to detect the APIC, find the type of ID to read, etc. Also means that practically speaking the system is using the full 32bit APIC IDs if that feature is present, since now the local apic id is entirely read from the local apic as it should be (if present). Fixes #475 --- arch/x86/32/start.S | 2 +- arch/x86/64/start.S | 2 +- arch/x86/lapic.c | 29 ++++++++++++++++------------- arch/x86/mp.c | 2 -- platform/pc/mp.c | 4 +--- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/arch/x86/32/start.S b/arch/x86/32/start.S index 9464fe71..1e3735ab 100644 --- a/arch/x86/32/start.S +++ b/arch/x86/32/start.S @@ -178,7 +178,7 @@ main_lk: call setup_idt /* set up the percpu data structure pointer for the boot cpu */ - /* NOTE: assumes the local apic is 0, which is probably not a safe assumption. */ + /* NOTE: sets the first cpu as APIC id 0 for now, which may not be correct. Fixed later in boot. */ pushl $0 pushl $0 call x86_configure_percpu_early diff --git a/arch/x86/64/start.S b/arch/x86/64/start.S index ba506787..de36a1fd 100644 --- a/arch/x86/64/start.S +++ b/arch/x86/64/start.S @@ -216,7 +216,7 @@ highaddr: call setup_idt /* set up the percpu data structure pointer for the boot cpu */ - /* NOTE: assumes the local apic is 0, which is probably not a safe assumption. */ + /* NOTE: sets the first cpu as APIC id 0 for now, which may not be correct. Fixed later in boot. */ xor %edi, %edi xor %esi, %esi call x86_configure_percpu_early diff --git a/arch/x86/lapic.c b/arch/x86/lapic.c index 525ae13c..ff470613 100644 --- a/arch/x86/lapic.c +++ b/arch/x86/lapic.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -213,6 +214,7 @@ void lapic_init(void) { lapic_present = x86_feature_test(X86_FEATURE_APIC); } +// run on the boot cpu after vm is initialized static void lapic_init_postvm(uint level) { if (!lapic_present) { return; @@ -224,31 +226,32 @@ static void lapic_init_postvm(uint level) { uint64_t apic_base = read_msr(X86_MSR_IA32_APIC_BASE); LTRACEF("raw apic base msr %#llx\n", apic_base); - // make sure it's enabled - if ((apic_base & (1u<<11)) == 0) { - dprintf(INFO, "X86: enabling lapic\n"); - apic_base |= (1u<<11); - write_msr(X86_MSR_IA32_APIC_BASE, apic_base); + // check for X2APIC feature + if (x86_feature_test(X86_FEATURE_X2APIC)) { + lapic_x2apic = true; + dprintf(INFO, "X86: local apic supports x2APIC mode\n"); } dprintf(INFO, "X86: lapic physical address %#llx\n", apic_base & ~0xfff); - // see if x2APIC mode is supported and enable - if (x86_feature_test(X86_FEATURE_X2APIC)) { - lapic_x2apic = true; - dprintf(INFO, "X86: local apic supports x2APIC mode\n"); - - write_msr(X86_MSR_IA32_APIC_BASE, apic_base | (1u<<10)); - } + // make sure the apic is enabled on the first cpu + lapic_enable_on_local_cpu(); // map the lapic into the kernel since it's not guaranteed that the physmap covers it - if (!lapic_mmio) { + if (!lapic_x2apic) { LTRACEF("mapping lapic into kernel\n"); status_t err = vmm_alloc_physical(vmm_get_kernel_aspace(), "lapic", PAGE_SIZE, (void **)&lapic_mmio, 0, apic_base & ~0xfff, /* vmm_flags */ 0, ARCH_MMU_FLAG_UNCACHED_DEVICE); ASSERT(err == NO_ERROR); + ASSERT(lapic_mmio != NULL); } + // semi-hack: re-read the APIC id of the boot cpu make sure the pecpu struct is correct + // before we go any further, in case the boot cpu's apic id is not 0. + x86_percpu_t *percpu = x86_get_percpu(); + percpu->apic_id = lapic_get_apic_id(); + dprintf(INFO, "X86: boot cpu apic id %u\n", percpu->apic_id); + // Read the local apic id and version and features uint32_t id = lapic_read(LAPIC_ID); uint32_t version = lapic_read(LAPIC_VERSION); diff --git a/arch/x86/mp.c b/arch/x86/mp.c index 63987f23..0449f398 100644 --- a/arch/x86/mp.c +++ b/arch/x86/mp.c @@ -85,8 +85,6 @@ status_t arch_mp_send_ipi(mp_cpu_mask_t target, mp_ipi_t ipi) { return NO_ERROR; } -void arch_mp_init_percpu(void) {} - void x86_secondary_entry(uint cpu_num) { // Read the local apic id from the local apic. // NOTE: assumes a local apic is present but since this is a secondary cpu, diff --git a/platform/pc/mp.c b/platform/pc/mp.c index 149e4656..adf74bdd 100644 --- a/platform/pc/mp.c +++ b/platform/pc/mp.c @@ -112,9 +112,7 @@ static void local_apic_callback(const void *_entry, size_t entry_len, void *cook return; } - // TODO: read the current APIC id and skip it, instead of assuming 0 is the boot cpu - // read BSP from X86_IA32_APIC_BASE_MSR bit 8? - if (entry->apic_id == 0) { + if (entry->apic_id == x86_get_apic_id()) { // skip the boot cpu return; }