Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 6 Sep 2014 22:17:55 +0000 (UTC)
From:      Marcel Moolenaar <marcel@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: r271211 - in stable/10/sys/ia64: ia64 include
Message-ID:  <201409062217.s86MHtgJ058741@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: marcel
Date: Sat Sep  6 22:17:54 2014
New Revision: 271211
URL: http://svnweb.freebsd.org/changeset/base/271211

Log:
  Fix the PCPU access macros. It was found that the PCPU pointer, when
  held in register r13, is used outside the bounds of critical_enter()
  and critical_exit() by virtue of optimizations performed by the
  compiler. The net effect being that address computations of fields
  in the PCPU structure could be relative to the PCPU structure of the
  CPU on which the address computation was performed and not related
  to the CPU that executes the actual load or store operation.
  The typical failure mode being that the per-CPU cache of UMA got
  corrupted due to accesses from other CPUs.
  
  Adding more volatile decorating to the register expression does not
  help. The thinking being that volatile is assumed to work on memory
  references and not register references. Thus, the fix is to perform
  the address computation using a volatile inline assembly statement.
  
  Additionally, since the reference is fundamentally non-atomic on ia64
  by virtue of have a distinct address computation followed by the
  actual load or store operation, it is required to wrap the entire
  PCPU access in a critical section.
  
  With PCPU_GET and friends requiring curthread now that they're in a
  critical section, low-level use of these macros in functions like
  cpu_switch() is not possible anymore. Consequently, a second order
  set of changes is needed to avoid using PCPU_GET and friends where
  curthread is either not set yet, or in the process of being changed.
  In those cases, explicit dereferencing of pcpup is needed. In those
  cases it is also possible to do that.
  
  This is a direct commit to stable/10.
  
  Approved by:	re@ (marius)

Modified:
  stable/10/sys/ia64/ia64/highfp.c
  stable/10/sys/ia64/ia64/machdep.c
  stable/10/sys/ia64/ia64/mp_machdep.c
  stable/10/sys/ia64/ia64/xtrace.c
  stable/10/sys/ia64/include/pcpu.h

Modified: stable/10/sys/ia64/ia64/highfp.c
==============================================================================
--- stable/10/sys/ia64/ia64/highfp.c	Sat Sep  6 20:16:45 2014	(r271210)
+++ stable/10/sys/ia64/ia64/highfp.c	Sat Sep  6 22:17:54 2014	(r271211)
@@ -171,8 +171,8 @@ ia64_highfp_save_ipi(void)
 		td->td_pcb->pcb_fpcpu = NULL;
 		PCPU_SET(fpcurthread, NULL);
 	}
+	wakeup(PCPU_PTR(fpcurthread));
 	mtx_unlock_spin(&ia64_highfp_mtx);
-	wakeup(&PCPU_GET(fpcurthread));
 
 	return ((td != NULL) ? 1 : 0);
 }

Modified: stable/10/sys/ia64/ia64/machdep.c
==============================================================================
--- stable/10/sys/ia64/ia64/machdep.c	Sat Sep  6 20:16:45 2014	(r271210)
+++ stable/10/sys/ia64/ia64/machdep.c	Sat Sep  6 22:17:54 2014	(r271211)
@@ -458,7 +458,7 @@ cpu_switch(struct thread *old, struct th
 #ifdef COMPAT_FREEBSD32
 	ia32_savectx(oldpcb);
 #endif
-	if (PCPU_GET(fpcurthread) == old)
+	if (pcpup->pc_fpcurthread == old)
 		old->td_frame->tf_special.psr |= IA64_PSR_DFH;
 	if (!savectx(oldpcb)) {
 		newpcb = new->td_pcb;
@@ -472,13 +472,13 @@ cpu_switch(struct thread *old, struct th
 			cpu_spinwait();
 #endif
 
-		PCPU_SET(curthread, new);
+		pcpup->pc_curthread = new;
 
 #ifdef COMPAT_FREEBSD32
 		ia32_restorectx(newpcb);
 #endif
 
-		if (PCPU_GET(fpcurthread) == new)
+		if (pcpup->pc_fpcurthread == new)
 			new->td_frame->tf_special.psr &= ~IA64_PSR_DFH;
 		restorectx(newpcb);
 		/* We should not get here. */
@@ -500,7 +500,7 @@ cpu_throw(struct thread *old __unused, s
 		cpu_spinwait();
 #endif
 
-	PCPU_SET(curthread, new);
+	pcpup->pc_curthread = new;
 
 #ifdef COMPAT_FREEBSD32
 	ia32_restorectx(newpcb);
@@ -833,7 +833,7 @@ ia64_init(void)
 	pcpu_init(pcpup, 0, sizeof(pcpu0));
 	dpcpu_init(ia64_physmem_alloc(DPCPU_SIZE, PAGE_SIZE), 0);
 	cpu_pcpu_setup(pcpup, ~0U, ia64_get_lid());
-	PCPU_SET(curthread, &thread0);
+	pcpup->pc_curthread = &thread0;
 
 	/*
 	 * Initialize the console before we print anything out.

Modified: stable/10/sys/ia64/ia64/mp_machdep.c
==============================================================================
--- stable/10/sys/ia64/ia64/mp_machdep.c	Sat Sep  6 20:16:45 2014	(r271210)
+++ stable/10/sys/ia64/ia64/mp_machdep.c	Sat Sep  6 22:17:54 2014	(r271211)
@@ -222,7 +222,7 @@ ia64_ap_startup(void)
 
 	ia64_ap_state.as_trace = 0x108;
 
-	vhpt = PCPU_GET(md.vhpt);
+	vhpt = pcpup->pc_md.vhpt;
 	map_vhpt(vhpt);
 	ia64_set_pta(vhpt + (1 << 8) + (pmap_vhpt_log2size << 2) + 1);
 	ia64_srlz_i();
@@ -246,8 +246,8 @@ ia64_ap_startup(void)
 		cpu_spinwait();
 
 	/* Initialize curthread. */
-	KASSERT(PCPU_GET(idlethread) != NULL, ("no idle thread"));
-	PCPU_SET(curthread, PCPU_GET(idlethread));
+	KASSERT(pcpup->pc_idlethread != NULL, ("no idle thread"));
+	pcpup->pc_curthread = pcpup->pc_idlethread;
 
 	pmap_invalidate_all();
 

Modified: stable/10/sys/ia64/ia64/xtrace.c
==============================================================================
--- stable/10/sys/ia64/ia64/xtrace.c	Sat Sep  6 20:16:45 2014	(r271210)
+++ stable/10/sys/ia64/ia64/xtrace.c	Sat Sep  6 22:17:54 2014	(r271211)
@@ -94,7 +94,7 @@ ia64_xtrace_init_common(vm_paddr_t pa)
 	__asm __volatile("mov   psr.l=%0" :: "r" (psr));
 	ia64_srlz_i();
 
-	PCPU_SET(md.xtrace_tail, ia64_xtrace_base);
+	pcpup->pc_md.xtrace_tail = ia64_xtrace_base;
 	ia64_set_k3(ia64_xtrace_base);
 }
 
@@ -119,7 +119,7 @@ ia64_xtrace_init_ap(void *buf)
 		ia64_set_k3(0);
 		return;
 	}
-	PCPU_SET(md.xtrace_buffer, buf);
+	pcpup->pc_md.xtrace_buffer = buf;
 	pa = ia64_tpa((uintptr_t)buf);
 	ia64_xtrace_init_common(pa);
 }
@@ -140,7 +140,7 @@ ia64_xtrace_init_bsp(void)
 		ia64_set_k3(0);
 		return;
 	}
-	PCPU_SET(md.xtrace_buffer, buf);
+	pcpup->pc_md.xtrace_buffer = buf;
 	pa = IA64_RR_MASK((uintptr_t)buf);
 	ia64_xtrace_init_common(pa);
 }

Modified: stable/10/sys/ia64/include/pcpu.h
==============================================================================
--- stable/10/sys/ia64/include/pcpu.h	Sat Sep  6 20:16:45 2014	(r271210)
+++ stable/10/sys/ia64/include/pcpu.h	Sat Sep  6 22:17:54 2014	(r271211)
@@ -31,6 +31,7 @@
 #define	_MACHINE_PCPU_H_
 
 #include <sys/sysctl.h>
+#include <sys/systm.h>
 #include <machine/pcb.h>
 
 struct pcpu_stats {
@@ -85,16 +86,48 @@ __curthread(void)
 }
 #define	curthread	(__curthread())
 
-#define	PCPU_GET(member)	(pcpup->pc_ ## member)
+#define	__pcpu_offset(name)	__offsetof(struct pcpu, name)
+#define	__pcpu_type(name)	__typeof(((struct pcpu *)0)->name)
+
+#define	PCPU_ADD(name, val)					\
+    do {							\
+	__pcpu_type(pc_ ## name) *nmp;				\
+	critical_enter();					\
+	__asm __volatile("add %0=%1,r13;;" :			\
+	    "=r"(nmp) : "i"(__pcpu_offset(pc_ ## name)));	\
+	*nmp += val;						\
+	critical_exit();					\
+    } while (0)
+
+#define	PCPU_GET(name)						\
+    ({	__pcpu_type(pc_ ## name) *nmp;				\
+	__pcpu_type(pc_ ## name) res;				\
+	critical_enter();					\
+	__asm __volatile("add %0=%1,r13;;" :			\
+	    "=r"(nmp) : "i"(__pcpu_offset(pc_ ## name)));	\
+	res = *nmp;						\
+	critical_exit();					\
+	res;							\
+    })
 
-/*
- * XXX The implementation of this operation should be made atomic
- * with respect to preemption.
- */
-#define	PCPU_ADD(member, value)	(pcpup->pc_ ## member += (value))
 #define	PCPU_INC(member)	PCPU_ADD(member, 1)
-#define	PCPU_PTR(member)	(&pcpup->pc_ ## member)
-#define	PCPU_SET(member,value)	(pcpup->pc_ ## member = (value))
+
+#define	PCPU_PTR(name)						\
+    ({	__pcpu_type(pc_ ## name) *nmp;				\
+	__asm __volatile("add %0=%1,r13;;" :			\
+	    "=r"(nmp) : "i"(__pcpu_offset(pc_ ## name)));	\
+	nmp;							\
+    })
+
+#define	PCPU_SET(name, val)					\
+    do {							\
+	__pcpu_type(pc_ ## name) *nmp;				\
+	critical_enter();					\
+	__asm __volatile("add %0=%1,r13;;" :			\
+	    "=r"(nmp) : "i"(__pcpu_offset(pc_ ## name)));	\
+	*nmp = val;						\
+	critical_exit();					\
+    } while (0)
 
 #endif	/* _KERNEL */
 



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