Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 15 Apr 2015 16:52:35 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r281560 - in stable/10/sys: amd64/amd64 i386/i386 x86/x86
Message-ID:  <201504151652.t3FGqZjl071938@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jhb
Date: Wed Apr 15 16:52:34 2015
New Revision: 281560
URL: https://svnweb.freebsd.org/changeset/base/281560

Log:
  MFC 278325,280866:
  Revert the IPI startup sequence to match what is described in the
  Intel Multiprocessor Specification v1.4.  The Intel SDM claims that
  
  278325:
  Revert the IPI startup sequence to match what is described in the
  Intel Multiprocessor Specification v1.4.  The Intel SDM claims that
  the INIT IPIs here are invalid, but other systems follow the MP
  spec instead.
  
  While here, fix the IPI wait routine to accept a timeout in microseconds
  instead of a raw spin count, and don't spin forever during AP startup.
  Instead, panic if a STARTUP IPI is not delivered after 20 us.
  
  280866:
  Wait 100 microseconds for a local APIC to dispatch each startup-related IPI
  rather than 20.  The MP 1.4 specification states in Appendix B.2:
  
    "A period of 20 microseconds should be sufficient for IPI dispatch to
     complete under normal operating conditions".
  
  (Note that this appears to be separate from the 10 millisecond (INIT) and
  200 microsecond (STARTUP) waits after the IPIs are dispatched.)  The
  Intel SDM is silent on this issue as far as I can tell.
  
  At least some hardware requires 60 microseconds as noted in the PR, so
  bump this to 100 to be on the safe side.
  
  PR:		196542, 197756

Modified:
  stable/10/sys/amd64/amd64/mp_machdep.c
  stable/10/sys/i386/i386/mp_machdep.c
  stable/10/sys/x86/x86/local_apic.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/amd64/amd64/mp_machdep.c
==============================================================================
--- stable/10/sys/amd64/amd64/mp_machdep.c	Wed Apr 15 16:22:05 2015	(r281559)
+++ stable/10/sys/amd64/amd64/mp_machdep.c	Wed Apr 15 16:52:34 2015	(r281560)
@@ -1067,14 +1067,27 @@ ipi_startup(int apic_id, int vector)
 {
 
 	/*
+	 * This attempts to follow the algorithm described in the
+	 * Intel Multiprocessor Specification v1.4 in section B.4.
+	 * For each IPI, we allow the local APIC ~20us to deliver the
+	 * IPI.  If that times out, we panic.
+	 */
+
+	/*
 	 * first we do an INIT IPI: this INIT IPI might be run, resetting
 	 * and running the target CPU. OR this INIT IPI might be latched (P5
 	 * bug), CPU waiting for STARTUP IPI. OR this INIT IPI might be
 	 * ignored.
 	 */
-	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_EDGE |
+	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_LEVEL |
 	    APIC_LEVEL_ASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_INIT, apic_id);
-	lapic_ipi_wait(-1);
+	lapic_ipi_wait(100);
+
+	/* Explicitly deassert the INIT IPI. */
+	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_LEVEL |
+	    APIC_LEVEL_DEASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_INIT,
+	    apic_id);
+
 	DELAY(10000);		/* wait ~10mS */
 
 	/*
@@ -1086,9 +1099,11 @@ ipi_startup(int apic_id, int vector)
 	 * will run.
 	 */
 	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_EDGE |
-	    APIC_LEVEL_DEASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_STARTUP |
+	    APIC_LEVEL_ASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_STARTUP |
 	    vector, apic_id);
-	lapic_ipi_wait(-1);
+	if (!lapic_ipi_wait(100))
+		panic("Failed to deliver first STARTUP IPI to APIC %d",
+		    apic_id);
 	DELAY(200);		/* wait ~200uS */
 
 	/*
@@ -1098,9 +1113,12 @@ ipi_startup(int apic_id, int vector)
 	 * recognized after hardware RESET or INIT IPI.
 	 */
 	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_EDGE |
-	    APIC_LEVEL_DEASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_STARTUP |
+	    APIC_LEVEL_ASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_STARTUP |
 	    vector, apic_id);
-	lapic_ipi_wait(-1);
+	if (!lapic_ipi_wait(100))
+		panic("Failed to deliver second STARTUP IPI to APIC %d",
+		    apic_id);
+
 	DELAY(200);		/* wait ~200uS */
 }
 

Modified: stable/10/sys/i386/i386/mp_machdep.c
==============================================================================
--- stable/10/sys/i386/i386/mp_machdep.c	Wed Apr 15 16:22:05 2015	(r281559)
+++ stable/10/sys/i386/i386/mp_machdep.c	Wed Apr 15 16:52:34 2015	(r281560)
@@ -1140,14 +1140,27 @@ ipi_startup(int apic_id, int vector)
 {
 
 	/*
+	 * This attempts to follow the algorithm described in the
+	 * Intel Multiprocessor Specification v1.4 in section B.4.
+	 * For each IPI, we allow the local APIC ~20us to deliver the
+	 * IPI.  If that times out, we panic.
+	 */
+
+	/*
 	 * first we do an INIT IPI: this INIT IPI might be run, resetting
 	 * and running the target CPU. OR this INIT IPI might be latched (P5
 	 * bug), CPU waiting for STARTUP IPI. OR this INIT IPI might be
 	 * ignored.
 	 */
-	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_EDGE |
+	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_LEVEL |
 	    APIC_LEVEL_ASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_INIT, apic_id);
-	lapic_ipi_wait(-1);
+	lapic_ipi_wait(100);
+
+	/* Explicitly deassert the INIT IPI. */
+	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_LEVEL |
+	    APIC_LEVEL_DEASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_INIT,
+	    apic_id);
+
 	DELAY(10000);		/* wait ~10mS */
 
 	/*
@@ -1159,9 +1172,11 @@ ipi_startup(int apic_id, int vector)
 	 * will run.
 	 */
 	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_EDGE |
-	    APIC_LEVEL_DEASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_STARTUP |
+	    APIC_LEVEL_ASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_STARTUP |
 	    vector, apic_id);
-	lapic_ipi_wait(-1);
+	if (!lapic_ipi_wait(100))
+		panic("Failed to deliver first STARTUP IPI to APIC %d",
+		    apic_id);
 	DELAY(200);		/* wait ~200uS */
 
 	/*
@@ -1171,9 +1186,12 @@ ipi_startup(int apic_id, int vector)
 	 * recognized after hardware RESET or INIT IPI.
 	 */
 	lapic_ipi_raw(APIC_DEST_DESTFLD | APIC_TRIGMOD_EDGE |
-	    APIC_LEVEL_DEASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_STARTUP |
+	    APIC_LEVEL_ASSERT | APIC_DESTMODE_PHY | APIC_DELMODE_STARTUP |
 	    vector, apic_id);
-	lapic_ipi_wait(-1);
+	if (!lapic_ipi_wait(100))
+		panic("Failed to deliver second STARTUP IPI to APIC %d",
+		    apic_id);
+
 	DELAY(200);		/* wait ~200uS */
 }
 

Modified: stable/10/sys/x86/x86/local_apic.c
==============================================================================
--- stable/10/sys/x86/x86/local_apic.c	Wed Apr 15 16:22:05 2015	(r281559)
+++ stable/10/sys/x86/x86/local_apic.c	Wed Apr 15 16:52:34 2015	(r281560)
@@ -1384,22 +1384,22 @@ SYSINIT(apic_setup_io, SI_SUB_INTR, SI_O
 int
 lapic_ipi_wait(int delay)
 {
-	int x, incr;
+	int x;
 
 	/*
-	 * Wait delay loops for IPI to be sent.  This is highly bogus
-	 * since this is sensitive to CPU clock speed.  If delay is
+	 * Wait delay microseconds for IPI to be sent.  If delay is
 	 * -1, we wait forever.
 	 */
 	if (delay == -1) {
-		incr = 0;
-		delay = 1;
-	} else
-		incr = 1;
-	for (x = 0; x < delay; x += incr) {
+		while ((lapic->icr_lo & APIC_DELSTAT_MASK) != APIC_DELSTAT_IDLE)
+			ia32_pause();
+		return (1);
+	}
+
+	for (x = 0; x < delay; x += 5) {
 		if ((lapic->icr_lo & APIC_DELSTAT_MASK) == APIC_DELSTAT_IDLE)
 			return (1);
-		ia32_pause();
+		DELAY(5);
 	}
 	return (0);
 }
@@ -1433,9 +1433,9 @@ lapic_ipi_raw(register_t icrlo, u_int de
 	intr_restore(saveintr);
 }
 
-#define	BEFORE_SPIN	1000000
+#define	BEFORE_SPIN	50000
 #ifdef DETECT_DEADLOCK
-#define	AFTER_SPIN	1000
+#define	AFTER_SPIN	50
 #endif
 
 void
@@ -1446,7 +1446,7 @@ lapic_ipi_vectored(u_int vector, int des
 	KASSERT((vector & ~APIC_VECTOR_MASK) == 0,
 	    ("%s: invalid vector %d", __func__, vector));
 
-	icrlo = APIC_DESTMODE_PHY | APIC_TRIGMOD_EDGE;
+	icrlo = APIC_DESTMODE_PHY | APIC_TRIGMOD_EDGE | APIC_LEVEL_ASSERT;
 
 	/*
 	 * IPI_STOP_HARD is just a "fake" vector used to send a NMI.
@@ -1454,9 +1454,9 @@ lapic_ipi_vectored(u_int vector, int des
 	 * the vector.
 	 */
 	if (vector == IPI_STOP_HARD)
-		icrlo |= APIC_DELMODE_NMI | APIC_LEVEL_ASSERT;
+		icrlo |= APIC_DELMODE_NMI;
 	else
-		icrlo |= vector | APIC_DELMODE_FIXED | APIC_LEVEL_DEASSERT;
+		icrlo |= vector | APIC_DELMODE_FIXED;
 	destfield = 0;
 	switch (dest) {
 	case APIC_IPI_DEST_SELF:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201504151652.t3FGqZjl071938>