--=-=-=

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

> On Tue, Mar 14, 2006 at 05:56:51PM -0500, Jed Davis wrote:
>> 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?
>
> It doesn't look bad, but are you sure the sfence and mfence instructions
> are needed, and exists on older P6 CPUs ? If I understood it right and
> remmeber properly these are only needed when using SSE2 instructions (but
> I may either have misunderstood or misremember)

Further reading (including an Intel manual and Xen's (Linux-derived)
macros) suggest that I vastly overestimated the support for the fence
insns, and the locked memory reference thing should remain.

As for sfence: it's part of SSE, and according to Intel needed only if
memory is explicitly mapped as write-combining or accessed with a
special "non-temporal" store insn.  According to comments in Linux,
there exist non-Intel clones that do out-of-order store, and a
non-noop sfence is needed there, but they don't enable it by default.
It's lfence and mfence that are part of SSE2.

So, here's the patch without that mistake.

-- 
(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-fences2.diff

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	15 Mar 2006 21:30:27 -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	15 Mar 2006 21:30:28 -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	15 Mar 2006 21:30:29 -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);
 

--=-=-=--