[coreboot-gerrit] Change in coreboot[master]: cpu/x86/mp_init: Support both xapic and x2apic with common code

[Kyösti Mälkki (Code Review)]

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 
Signed-off-by: Kyösti Mälkki 
---
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);