[src/trunk]: src/sys/arch/xen/xen Fix theoretical race in xen_evtchn_trymask.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/3b05b2a93d2b
branches:  trunk
changeset: 357435:3b05b2a93d2b
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Fri Nov 10 19:24:17 2017 +0000

description:
Fix theoretical race in xen_evtchn_trymask.

This now matches ioapic_trymask:

- Caller masks CPU interrupts while the PIC's interrupts are unmasked.
- Trymask masks the PIC's interrupts, and tests whether any were
  pending that we missed because the CPU interrupts are masked.

  ==> If there were any pending, trymask unmasks PIC's interrupts and
      returns false.

  ==> If not, interrupts are masked on the PIC and CPU and there were
      none pending and trymask returns true.

No functional change right now because cpu_intr_redistribute is
currently disabled on Xen.

ok cherry

diffstat:

 sys/arch/xen/xen/evtchn.c |  41 +++++++++++++++++++++++++++++------------
 1 files changed, 29 insertions(+), 12 deletions(-)

diffs (68 lines):

diff -r dc76ab479588 -r 3b05b2a93d2b sys/arch/xen/xen/evtchn.c
--- a/sys/arch/xen/xen/evtchn.c Fri Nov 10 18:08:11 2017 +0000
+++ b/sys/arch/xen/xen/evtchn.c Fri Nov 10 19:24:17 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: evtchn.c,v 1.74 2017/11/04 10:26:14 cherry Exp $       */
+/*     $NetBSD: evtchn.c,v 1.75 2017/11/10 19:24:17 riastradh Exp $    */
 
 /*
  * Copyright (c) 2006 Manuel Bouyer.
@@ -54,7 +54,7 @@
 
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: evtchn.c,v 1.74 2017/11/04 10:26:14 cherry Exp $");
+__KERNEL_RCSID(0, "$NetBSD: evtchn.c,v 1.75 2017/11/10 19:24:17 riastradh Exp $");
 
 #include "opt_xen.h"
 #include "isa.h"
@@ -505,22 +505,39 @@
        xen_atomic_clear_bit(&ci->ci_evtmask[0], evtchn);
 }
 
+/*
+ * xen_evtchn_trymask(pic, pin)
+ *
+ *     If there are interrupts pending on the bus-shared pic, return
+ *     false.  Otherwise, mask interrupts on the bus-shared pic and
+ *     return true.
+ */
 static bool
 xen_evtchn_trymask(struct pic *pic, int pin)
 {
-       volatile shared_info_t *s = HYPERVISOR_shared_info;
+       volatile struct shared_info *s = HYPERVISOR_shared_info;
+       unsigned long masked __diagused;
+
+       /* Mask it.  */
+       masked = xen_atomic_test_and_set_bit(&s->evtchn_mask[0], pin);
 
-       /* Already masked! */
-       if (xen_atomic_test_bit(&s->evtchn_mask[0], pin))
-               return true;
-       
-       /* Pending - bail! */
-       if (xen_atomic_test_bit(&s->evtchn_pending[0], pin))
+       /*
+        * Caller is responsible for calling trymask only when the
+        * interrupt pin is not masked, and for serializing calls to
+        * trymask.
+        */
+       KASSERT(!masked);
+
+       /*
+        * Check whether there were any interrupts pending when we
+        * masked it.  If there were, unmask and abort.
+        */
+       if (xen_atomic_test_bit(&s->evtchn_pending[0], pin)) {
+               xen_atomic_clear_bit(&s->evtchn_mask[0], pin);
                return false;
+       }
 
-       /* XXX: There's a race here - anything we can do about this ? */
-       /* Mask it */
-       xen_atomic_set_bit(&s->evtchn_mask[0], pin);
+       /* Success: masked, not pending.  */
        return true;
 }