Change in coreboot[master]: cpu/x86/lapic: Add lapic_send_ipi() helper

[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/+/55190

to review the following change.

Change subject: cpu/x86/lapic: Add lapic_send_ipi() helper
......................................................................

cpu/x86/lapic: Add lapic_send_ipi() helper

Change-Id: I7207a9aadd987b4307ce8b3dd8dbfd47d0a5768e
Signed-off-by: Arthur Heymans 
Signed-off-by: Kyösti Mälkki 
---
M src/cpu/x86/lapic/lapic_cpu_init.c
M src/cpu/x86/mp_init.c
M src/include/cpu/x86/lapic.h
3 files changed, 26 insertions(+), 40 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/55190/1

diff --git a/src/cpu/x86/lapic/lapic_cpu_init.c b/src/cpu/x86/lapic/lapic_cpu_init.c
index f73c4b2..5988919 100644
--- a/src/cpu/x86/lapic/lapic_cpu_init.c
+++ b/src/cpu/x86/lapic/lapic_cpu_init.c
@@ -103,14 +103,7 @@
    /*
     * Turn INIT on target chip
     */
-	lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(apicid));
-
-	/*
-	 * Send IPI
-	 */
-
-	lapic_write_around(LAPIC_ICR, LAPIC_INT_LEVELTRIG | LAPIC_INT_ASSERT
-				| LAPIC_DM_INIT);
+	lapic_send_ipi(LAPIC_INT_LEVELTRIG | LAPIC_INT_ASSERT | LAPIC_DM_INIT, apicid);

printk(BIOS_SPEW, "Waiting for send to finish...\n");
    timeout = 0;
@@ -136,11 +129,7 @@

printk(BIOS_SPEW, "Deasserting INIT.\n");

-	/* Target chip */
-	lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(apicid));
-
-	/* Send IPI */
-	lapic_write_around(LAPIC_ICR, LAPIC_INT_LEVELTRIG | LAPIC_DM_INIT);
+	lapic_send_ipi(LAPIC_INT_LEVELTRIG | LAPIC_DM_INIT, apicid);

printk(BIOS_SPEW, "Waiting for send to finish...\n");
    timeout = 0;
@@ -174,13 +163,7 @@
    	 * STARTUP IPI
    	 */

-		/* Target chip */
-		lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(apicid));
-
-		/* Boot on the stack */
-		/* Kick the second */
-		lapic_write_around(LAPIC_ICR, LAPIC_DM_STARTUP
-					| (AP_SIPI_VECTOR >> 12));
+		lapic_send_ipi(LAPIC_DM_STARTUP | (AP_SIPI_VECTOR >> 12), apicid);

/*
    	 * Give the other CPU some time to accept the IPI.
@@ -333,16 +316,12 @@
 {
    int timeout;
    unsigned long send_status;
-	unsigned long id;
-
-	id = lapicid();
+	unsigned long id = lapicid();

printk(BIOS_DEBUG, "CPU %ld going down...\n", id);

/* send an LAPIC INIT to myself */
-	lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(id));
-	lapic_write_around(LAPIC_ICR, LAPIC_INT_LEVELTRIG |
-				LAPIC_INT_ASSERT | LAPIC_DM_INIT);
+	lapic_send_ipi(LAPIC_INT_LEVELTRIG | LAPIC_INT_ASSERT | LAPIC_DM_INIT, id);

/* wait for the ipi send to finish */
    dprintk(BIOS_SPEW, "Waiting for send to finish...\n");
@@ -362,8 +341,7 @@
    dprintk(BIOS_SPEW, "Deasserting INIT.\n");

/* Deassert the LAPIC INIT */
-	lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(id));
-	lapic_write_around(LAPIC_ICR, LAPIC_INT_LEVELTRIG | LAPIC_DM_INIT);
+	lapic_send_ipi(LAPIC_INT_LEVELTRIG | LAPIC_DM_INIT, id);

dprintk(BIOS_SPEW, "Waiting for send to finish...\n");

diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c
index 893e8f1..87ddb0d 100644
--- a/src/cpu/x86/mp_init.c
+++ b/src/cpu/x86/mp_init.c
@@ -467,9 +467,7 @@
    }

/* Send INIT IPI to all but self. */
-	lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(0));
-	lapic_write_around(LAPIC_ICR, LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT |
-			   LAPIC_DM_INIT);
+	lapic_send_ipi(LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT | LAPIC_DM_INIT, 0);
    printk(BIOS_DEBUG, "Waiting for 10ms after sending INIT.\n");
    mdelay(10);

@@ -483,9 +481,8 @@
    	printk(BIOS_DEBUG, "done.\n");
    }

-	lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(0));
-	lapic_write_around(LAPIC_ICR, LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT |
-			   LAPIC_DM_STARTUP | sipi_vector);
+	lapic_send_ipi(LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT | LAPIC_DM_STARTUP | sipi_vector,
+		       0);
    printk(BIOS_DEBUG, "Waiting for 1st SIPI to complete...");
    if (apic_wait_timeout(10000 /* 10 ms */, 50 /* us */)) {
    	printk(BIOS_ERR, "timed out.\n");
@@ -509,9 +506,8 @@
    	printk(BIOS_DEBUG, "done.\n");
    }

-	lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(0));
-	lapic_write_around(LAPIC_ICR, LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT |
-			   LAPIC_DM_STARTUP | sipi_vector);
+	lapic_send_ipi(LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT | LAPIC_DM_STARTUP | sipi_vector,
+		       0);
    printk(BIOS_DEBUG, "Waiting for 2nd SIPI to complete...");
    if (apic_wait_timeout(10000 /* 10 ms */, 50 /* us */)) {
    	printk(BIOS_ERR, "timed out.\n");
@@ -689,8 +685,7 @@
    	printk(BIOS_DEBUG, "done.\n");
    }

-	lapic_write_around(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(lapicid()));
-	lapic_write_around(LAPIC_ICR, LAPIC_INT_ASSERT | LAPIC_DM_SMI);
+	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");
    else
diff --git a/src/include/cpu/x86/lapic.h b/src/include/cpu/x86/lapic.h
index 7eba327..e297e3b7 100644
--- a/src/include/cpu/x86/lapic.h
+++ b/src/include/cpu/x86/lapic.h
@@ -29,10 +29,15 @@
    	      : : "memory", "cc");
 }

+static __always_inline void xapic_send_ipi(uint32_t icrlow, uint32_t apicid)
+{
+	xapic_write_atomic(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(apicid));
+	xapic_write_atomic(LAPIC_ICR, icrlow);
+}
+
 #define lapic_read_around(x) lapic_read(x)
 #define lapic_write_around(x, y) xapic_write_atomic((x), (y))

-
 static __always_inline uint32_t x2apic_read(unsigned int reg)
 {
    uint32_t value, index;
@@ -111,6 +116,14 @@
    }
 }

+static __always_inline void lapic_send_ipi(uint32_t icrlow, uint32_t apicid)
+{
+	if (is_x2apic_mode())
+		x2apic_send_ipi(icrlow, apicid);
+	else
+		xapic_send_ipi(icrlow, apicid);
+}
+
 static __always_inline void lapic_wait_icr_idle(void)
 {
    do { } while (lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY);