[src/trunk]: src/sys/arch/arm/broadcom Jared points out that interrupt_distri...

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

details:   https://anonhg.NetBSD.org/src/rev/f7faa46ba2ba
branches:  trunk
changeset: 465559:f7faa46ba2ba
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Thu Nov 28 15:35:51 2019 +0000

description:
Jared points out that interrupt_distribute(9) assumes that any interrupt
handle can be used as an input to the MD interrupt_distribute implementation
so we are forced to return the handle we got back from intr_establish().
Upshot is that the input to bcm2835_icu_fdt_disestablish() is ambiguous for
shared IRQs, rendering them un-disestablishable.

While here, make sure to actually bump the intr_refcnt, and add an
assertion on the value we get back from bcm2835_icu_fdt_decode_irq().

diffstat:

 sys/arch/arm/broadcom/bcm2835_intr.c |  53 ++++++++++++++++++++++++++---------
 1 files changed, 39 insertions(+), 14 deletions(-)

diffs (94 lines):

diff -r 588c6e4724c4 -r f7faa46ba2ba sys/arch/arm/broadcom/bcm2835_intr.c
--- a/sys/arch/arm/broadcom/bcm2835_intr.c      Thu Nov 28 14:21:25 2019 +0000
+++ b/sys/arch/arm/broadcom/bcm2835_intr.c      Thu Nov 28 15:35:51 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: bcm2835_intr.c,v 1.25 2019/11/28 01:08:06 thorpej Exp $        */
+/*     $NetBSD: bcm2835_intr.c,v 1.26 2019/11/28 15:35:51 thorpej Exp $        */
 
 /*-
  * Copyright (c) 2012, 2015, 2019 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: bcm2835_intr.c,v 1.25 2019/11/28 01:08:06 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: bcm2835_intr.c,v 1.26 2019/11/28 15:35:51 thorpej Exp $");
 
 #define _INTR_PRIVATE
 
@@ -473,6 +473,8 @@
        if (irq == -1)
                return NULL;
 
+       KASSERT(irq < BCM2835_NIRQ);
+
        firq = sc->sc_irq[irq];
        if (firq == NULL) {
                firq = kmem_alloc(sizeof(*firq), KM_SLEEP);
@@ -517,31 +519,54 @@
        firqh->ih_irq = firq;
        firqh->ih_fn = func;
        firqh->ih_arg = arg;
+
+       firq->intr_refcnt++;
        TAILQ_INSERT_TAIL(&firq->intr_handlers, firqh, ih_next);
 
-       return firqh;
+       /*
+        * XXX interrupt_distribute(9) assumes that any interrupt
+        * handle can be used as an input to the MD interrupt_distribute
+        * implementationm, so we are forced to return the handle
+        * we got back from intr_establish().  Upshot is that the
+        * input to bcm2835_icu_fdt_disestablish() is ambiguous for
+        * shared IRQs, rendering them un-disestablishable.
+        */
+
+       return firq->intr_ih;
 }
 
 static void
 bcm2835_icu_fdt_disestablish(device_t dev, void *ih)
 {
        struct bcm2835icu_softc * const sc = device_private(dev);
-       struct bcm2835icu_irqhandler *firqh = ih;
-       struct bcm2835icu_irq *firq = firqh->ih_irq;
+       struct bcm2835icu_irqhandler *firqh;
+       struct bcm2835icu_irq *firq;
+       u_int n;
 
-       KASSERT(firq->intr_refcnt > 0);
+       for (n = 0; n < BCM2835_NIRQ; n++) {
+               firq = sc->sc_irq[n];
+               if (firq == NULL || firq->intr_ih != ih)
+                       continue;
+
+               KASSERT(firq->intr_refcnt > 0);
 
-       /* XXX */
-       if (firq->intr_refcnt > 1)
-               panic("%s: cannot disestablish shared irq", __func__);
+               /* XXX see above */
+               if (firq->intr_refcnt > 1)
+                       panic("%s: cannot disestablish shared irq", __func__);
 
-       intr_disestablish(firq->intr_ih);
+               intr_disestablish(firq->intr_ih);
 
-       TAILQ_REMOVE(&firq->intr_handlers, firqh, ih_next);
-       kmem_free(firqh, sizeof(*firqh));
+               firqh = TAILQ_FIRST(&firq->intr_handlers);
+               TAILQ_REMOVE(&firq->intr_handlers, firqh, ih_next);
+               kmem_free(firqh, sizeof(*firqh));
 
-       sc->sc_irq[firq->intr_irq] = NULL;
-       kmem_free(firq, sizeof(*firq));
+               sc->sc_irq[firq->intr_irq] = NULL;
+               kmem_free(firq, sizeof(*firq));
+
+               return;
+       }
+
+       panic("%s: interrupt not established", __func__);
 }
 
 static int