Use an atomic operation to clear pending event bits

Pending event bits are located in a shared memory and are potentially
accessed by multiple CPUs running dom0 and the guest VM.  It appears
that a failure to synchronize changes to this shared memory leads to
race conditions resulting in the guest missing out on notifications.

diff --git a/sys/dev/pv/xen.c b/sys/dev/pv/xen.c
index c12328c..8f4be60 100644 (file)
--- a/sys/dev/pv/xen.c
+++ b/sys/dev/pv/xen.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: xen.c,v 1.58 2016/08/01 14:37:39 mikeb Exp $  */
+/*     $OpenBSD: xen.c,v 1.59 2016/08/03 14:55:57 mikeb Exp $  */
 
 /*
  * Copyright (c) 2015 Mike Belopuhov
@@ -633,8 +633,8 @@ xen_intr(void)
                for (bit = 0; pending > 0; pending >>= 1, bit++) {
                        if ((pending & 1) == 0)
                                continue;
-                       sc->sc_ipg->evtchn_pending[row] &= ~(1 << bit);
-                       virtio_membar_producer();
+                       atomic_clearbit_ptr(&sc->sc_ipg->evtchn_pending[row],
+                           bit);
                        port = (row * LONG_BIT) + bit;
                        if ((xi = xen_lookup_intsrc(sc, port)) == NULL) {
                                printf("%s: unhandled interrupt on port %u\n",

diff --git a/sys/dev/pv/xenreg.h b/sys/dev/pv/xenreg.h
index e6cc818..b68ed76 100644 (file)
--- a/sys/dev/pv/xenreg.h
+++ b/sys/dev/pv/xenreg.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: xenreg.h,v 1.8 2016/01/18 19:06:48 mikeb Exp $        */
+/*     $OpenBSD: xenreg.h,v 1.9 2016/08/03 14:55:57 mikeb Exp $        */
 
 /*
  * Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -237,8 +237,8 @@ struct shared_info {
         * per-vcpu selector word to be set. Each bit in the selector covers a
         * 'C long' in the PENDING bitfield array.
         */
-       unsigned long evtchn_pending[sizeof(unsigned long) * 8];
-       unsigned long evtchn_mask[sizeof(unsigned long) * 8];
+       volatile unsigned long evtchn_pending[sizeof(unsigned long) * 8];
+       volatile unsigned long evtchn_mask[sizeof(unsigned long) * 8];
 
        /*
         * Wallclock time: updated only by control software. Guests should

diff --git a/sys/dev/pv/xenvar.h b/sys/dev/pv/xenvar.h
index d4e3fc5..a3cb403 100644 (file)
--- a/sys/dev/pv/xenvar.h
+++ b/sys/dev/pv/xenvar.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: xenvar.h,v 1.33 2016/07/29 21:27:43 mikeb Exp $       */
+/*     $OpenBSD: xenvar.h,v 1.34 2016/08/03 14:55:58 mikeb Exp $       */
 
 /*
  * Copyright (c) 2015 Mike Belopuhov
@@ -143,6 +143,18 @@ struct xs_transaction {
        struct xs_softc         *xst_sc;
 };
 
+static inline int
+atomic_clearbit_ptr(volatile void *ptr, int bit)
+{
+       int obit;
+
+       __asm__ __volatile__ ("lock btrl %2,%1; sbbl %0,%0" :
+           "=r" (obit), "=m" (*(volatile long *)ptr) : "Ir" (bit) :
+           "memory");
+
+       return (obit);
+}
+
 int    xs_cmd(struct xs_transaction *, int, const char *, struct iovec **,
            int *);
 void   xs_resfree(struct xs_transaction *, struct iovec *, int);