Re: lfence?

[Jed Davis <jdev%panix.com@localhost>]

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

> On Tue, Mar 14, 2006 at 05:56:51PM -0500, Jed Davis wrote:
>> Manuel Bouyer <bouyer%antioche.eu.org@localhost> 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)))))

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);