Attention is currently required from: Arthur Heymans. Hello Arthur Heymans,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/55197
to review the following change.
Change subject: cpu/x86/mp_init: Support both xapic and x2apic with common code ......................................................................
cpu/x86/mp_init: Support both xapic and x2apic with common code
Change-Id: I7264f39143cc6edb7a9687d0bd763cb2703a8265 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/x86/mp_init.c 1 file changed, 14 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/55197/1
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index ad45ee9..d7c2f7f 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -435,27 +435,8 @@
printk(BIOS_DEBUG, "Attempting to start %d APs\n", ap_count);
- if (is_x2apic_mode()) { - x2apic_send_ipi(LAPIC_DM_INIT | LAPIC_INT_LEVELTRIG | - LAPIC_INT_ASSERT | LAPIC_DEST_ALLBUT, 0); - mdelay(10); - x2apic_send_ipi(LAPIC_DM_STARTUP | LAPIC_INT_LEVELTRIG | - LAPIC_DEST_ALLBUT | sipi_vector, 0); - - /* Wait for CPUs to check in up to 200 us. */ - wait_for_aps(num_aps, ap_count, 200 /* us */, 15 /* us */); - - x2apic_send_ipi(LAPIC_DM_STARTUP | LAPIC_INT_LEVELTRIG | - LAPIC_DEST_ALLBUT | sipi_vector, 0); - - /* Wait for CPUs to check in. */ - if (wait_for_aps(num_aps, ap_count, 100000 /* 100 ms */, 50 /* us */)) { - printk(BIOS_ERR, "Not all APs checked in: %d/%d.\n", - atomic_read(num_aps), ap_count); - return -1; - } - return 0; - } + int x2apic_mode = is_x2apic_mode(); + printk(BIOS_DEBUG, "Starting CPUs in %s mode\n", x2apic_mode ? "x2apic" : "xapic");
if (lapic_busy()) { printk(BIOS_DEBUG, "Waiting for ICR not to be busy..."); @@ -671,11 +652,6 @@
void smm_initiate_relocation_parallel(void) { - if (is_x2apic_mode()) { - x2apic_send_ipi(LAPIC_DM_SMI | LAPIC_INT_LEVELTRIG, lapicid()); - return; - } - if (lapic_busy()) { printk(BIOS_DEBUG, "Waiting for ICR not to be busy..."); if (apic_wait_timeout(1000 /* 1 ms */, 50)) { @@ -685,11 +661,19 @@ printk(BIOS_DEBUG, "done.\n"); }
- lapic_send_ipi(LAPIC_INT_ASSERT | LAPIC_DM_SMI, lapicid()); - if (apic_wait_timeout(1000 /* 1 ms */, 100 /* us */)) - printk(BIOS_DEBUG, "SMI Relocation timed out.\n"); + /* FIXME: Why the difference? */ + if (is_x2apic_mode()) + lapic_send_ipi(LAPIC_DM_SMI | LAPIC_INT_LEVELTRIG, lapicid()); else - printk(BIOS_DEBUG, "Relocation complete.\n"); + lapic_send_ipi(LAPIC_INT_ASSERT | LAPIC_DM_SMI, lapicid()); + + if (lapic_busy()) { + if (apic_wait_timeout(1000 /* 1 ms */, 100 /* us */)) { + printk(BIOS_DEBUG, "SMI Relocation timed out.\n"); + return; + } + } + printk(BIOS_DEBUG, "Relocation complete.\n"); }
DECLARE_SPIN_LOCK(smm_relocation_lock);