Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 7 May 2018 12:22:26 +0000 (UTC)
From:      Andriy Gapon <avg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r333321 - head/sys/x86/x86
Message-ID:  <201805071222.w47CMQCE054194@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: avg
Date: Mon May  7 12:22:25 2018
New Revision: 333321
URL: https://svnweb.freebsd.org/changeset/base/333321

Log:
  x86 cpususpend_handler: call wbinvd after setting suspend state bits
  
  Without a subsequent wbinvd the changes to suspended_cpus (and
  resuming_cpus) can be lost at least on AMD systems that use MOESI cache
  coherency protocol.  That can happen because one of APs ends up as an
  Owner of the corresponding cache line(s) and the changes may never reach
  the main memory before the AP is reset.
  
  While here, move clearing of suspended_cpus a little bit earlier as the
  fact of returning from savectx (with zero return value) means that the
  CPU has fully restored it execution context.
  
  Also, rework the comment that describes the need for resuming_cpus.
  
  This change fixed suspend to RAM a previously broken AMD-based system.
  
  Reviewed by:	kib
  Discussed with:	bde
  MFC after:	3 weeks
  Differential Revision: https://reviews.freebsd.org/D15295

Modified:
  head/sys/x86/x86/mp_x86.c

Modified: head/sys/x86/x86/mp_x86.c
==============================================================================
--- head/sys/x86/x86/mp_x86.c	Mon May  7 09:42:35 2018	(r333320)
+++ head/sys/x86/x86/mp_x86.c	Mon May  7 12:22:25 2018	(r333321)
@@ -1425,16 +1425,36 @@ cpususpend_handler(void)
 #else
 		npxsuspend(susppcbs[cpu]->sp_fpususpend);
 #endif
-		wbinvd();
-		CPU_SET_ATOMIC(cpu, &suspended_cpus);
 		/*
-		 * Hack for xen, which does not use resumectx() so never
-		 * uses the next clause: set resuming_cpus early so that
-		 * resume_cpus() can wait on the same bitmap for acpi and
-		 * xen.  resuming_cpus now means eventually_resumable_cpus.
+		 * suspended_cpus is cleared shortly after each AP is restarted
+		 * by a Startup IPI, so that the BSP can proceed to restarting
+		 * the next AP.
+		 *
+		 * resuming_cpus gets cleared when the AP completes
+		 * initialization after having been released by the BSP.
+		 * resuming_cpus is probably not the best name for the
+		 * variable, because it is actually a set of processors that
+		 * haven't resumed yet and haven't necessarily started resuming.
+		 *
+		 * Note that suspended_cpus is meaningful only for ACPI suspend
+		 * as it's not really used for Xen suspend since the APs are
+		 * automatically restored to the running state and the correct
+		 * context.  For the same reason resumectx is never called in
+		 * that case.
 		 */
+		CPU_SET_ATOMIC(cpu, &suspended_cpus);
 		CPU_SET_ATOMIC(cpu, &resuming_cpus);
+
+		/*
+		 * Invalidate the cache after setting the global status bits.
+		 * The last AP to set its bit may end up being an Owner of the
+		 * corresponding cache line in MOESI protocol.  The AP may be
+		 * stopped before the cache line is written to the main memory.
+		 */
+		wbinvd();
 	} else {
+		/* Indicate that we have restarted and restored the context. */
+		CPU_CLR_ATOMIC(cpu, &suspended_cpus);
 #ifdef __amd64__
 		fpuresume(susppcbs[cpu]->sp_fpususpend);
 #else
@@ -1444,9 +1464,6 @@ cpususpend_handler(void)
 		initializecpu();
 		PCPU_SET(switchtime, 0);
 		PCPU_SET(switchticks, ticks);
-
-		/* Indicate that we are resuming */
-		CPU_CLR_ATOMIC(cpu, &suspended_cpus);
 	}
 
 	/* Wait for resume directive */



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