Subject: Re: lfence?
To: None <port-xen@NetBSD.org>
From: Jed Davis <jdev@panix.com>
List: port-xen
Date: 03/14/2006 17:56:51
--=-=-=

Manuel Bouyer <bouyer@antioche.eu.org> writes:

> Yes, this can be an issue. I've run in such issues in other parts of
> the kernel already.

Does the attached patch look reasonable?

-- 
(let ((C call-with-current-continuation)) (apply (lambda (x y) (x y)) (map
((lambda (r) ((C C) (lambda (s) (r (lambda l (apply (s s) l))))))  (lambda
(f) (lambda (l) (if (null? l) C (lambda (k) (display (car l)) ((f (cdr l))
(C k)))))))    '((#\J #\d #\D #\v #\s) (#\e #\space #\a #\i #\newline)))))

--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=xen-fences1.diff
Content-Description: Add missing memory barriers.

Index: arch/xen/include/cpufunc.h
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/include/cpufunc.h,v
retrieving revision 1.11
diff -u -p -r1.11 cpufunc.h
--- arch/xen/include/cpufunc.h	6 Mar 2006 19:54:14 -0000	1.11
+++ arch/xen/include/cpufunc.h	14 Mar 2006 22:40:53 -0000
@@ -62,9 +62,9 @@ x86_lfence(void)
 {
 
 	/*
-	 * XXX it's better to use real lfence insn if available.
+	 * Xen requires a P6, so we can use an actual lfence.
 	 */
-	__asm volatile("lock; addl $0, 0(%%esp)" : : : "memory");
+	__asm volatile("lfence" : : : "memory");
 }
 
 static __inline void
@@ -72,9 +72,20 @@ x86_sfence(void)
 {
 
 	/*
-	 * nothing to do at the CPU level, just put a barrier for compiler
+	 * Xen requires a P6, so we can use an actual sfence.
+	 * Allegedly, CPUs now exist where sfence isn't a no-op.
 	 */
-	__insn_barrier();
+	__asm volatile("sfence" : : : "memory");
+}
+
+static __inline void
+x86_mfence(void)
+{
+
+	/*
+	 * Xen requires a P6, so we can use an actual mfence.
+	 */
+	__asm volatile("mfence" : : : "memory");
 }
 
 #ifdef _KERNEL
Index: arch/xen/xen/clock.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/clock.c,v
retrieving revision 1.18
diff -u -p -r1.18 clock.c
--- arch/xen/xen/clock.c	7 Mar 2006 23:08:14 -0000	1.18
+++ arch/xen/xen/clock.c	14 Mar 2006 22:40:53 -0000
@@ -82,29 +82,29 @@ get_time_values_from_xen(void)
 	uint32_t tversion;
 	do {
 		tversion = t->version;
-		__insn_barrier();
+		x86_lfence();
 		shadow_tsc_stamp = t->tsc_timestamp;
 		shadow_system_time = t->system_time;
-		__insn_barrier();
+		x86_lfence();
 	} while ((t->version & 1) || (tversion != t->version));
 	do {
 		tversion = HYPERVISOR_shared_info->wc_version;
-		__insn_barrier();
+		x86_lfence();
 		shadow_tv.tv_sec = HYPERVISOR_shared_info->wc_sec;
 		shadow_tv.tv_usec = HYPERVISOR_shared_info->wc_nsec;
-		__insn_barrier();
+		x86_lfence();
 	} while ((HYPERVISOR_shared_info->wc_version & 1) ||
 	    (tversion != HYPERVISOR_shared_info->wc_version));
 	shadow_tv.tv_usec = shadow_tv.tv_usec / 1000;
 #else /* XEN3 */
 	do {
 		shadow_time_version = HYPERVISOR_shared_info->time_version2;
-		__insn_barrier();
+		x86_lfence();
 		shadow_tv.tv_sec = HYPERVISOR_shared_info->wc_sec;
 		shadow_tv.tv_usec = HYPERVISOR_shared_info->wc_usec;
 		shadow_tsc_stamp = HYPERVISOR_shared_info->tsc_timestamp;
 		shadow_system_time = HYPERVISOR_shared_info->system_time;
-		__insn_barrier();
+		x86_lfence();
 	} while (shadow_time_version != HYPERVISOR_shared_info->time_version1);
 #endif
 }
Index: arch/xen/xen/ctrl_if.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/ctrl_if.c,v
retrieving revision 1.12
diff -u -p -r1.12 ctrl_if.c
--- arch/xen/xen/ctrl_if.c	15 Jan 2006 22:09:52 -0000	1.12
+++ arch/xen/xen/ctrl_if.c	14 Mar 2006 22:40:54 -0000
@@ -136,7 +136,7 @@ __ctrl_if_rxmsg_deferred(void *unused)
 	while (1) {
 		s = splsoftnet();
 		dp = ctrl_if_rxmsg_deferred_prod;
-		__insn_barrier(); /* Ensure we see all requests up to 'dp'. */
+		x86_lfence(); /* Ensure we see all requests up to 'dp'. */
 		if (ctrl_if_rxmsg_deferred_cons == dp) {
 			tsleep(&ctrl_if_rxmsg_deferred_cons, PRIBIO,
 			    "rxdef", 0);
Index: arch/xen/xen/if_xennet.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/if_xennet.c,v
retrieving revision 1.43
diff -u -p -r1.43 if_xennet.c
--- arch/xen/xen/if_xennet.c	6 Mar 2006 22:04:18 -0000	1.43
+++ arch/xen/xen/if_xennet.c	14 Mar 2006 22:40:54 -0000
@@ -550,7 +550,7 @@ xennet_interface_status_change(netif_fe_
 		 * we've probably just requeued some packets.
 		 */
 		sc->sc_backend_state = BEST_CONNECTED;
-		__insn_barrier();
+		x86_sfence();
 		hypervisor_notify_via_evtchn(status->evtchn);  
 		network_tx_buf_gc(sc);
 

--=-=-=--