Fix bugs in interrupts allocation on ARM64

Separate interrupt descriptors lookup from allocation. It was possible
to perform config on non-existing interrupt simply by allocating spurious
descriptor.
Must lock the interrupt descriptors table lookup to avoid mismatches.
This ought to prevent trouble while setting up new interrupt
and dispatching existing one.
Use spin mutex rather than sleep mutex. This is mainly due to lock in
arm_dispatch_intr.
This should be eventually changed to a lock-less solution without
walking through a linked list on each interrupt.

Reviewed by:   andrew, wma
Obtained from: Semihalf
Sponsored by:  Cavium
Differential Revision: https://reviews.freebsd.org/D5121

1 files changed, 36 insertions, 29 deletions

diff --git a/sys/arm64/arm64/intr_machdep.c b/sys/arm64/arm64/intr_machdep.c
index 3389c69..70902ce 100644
--- a/sys/arm64/arm64/intr_machdep.c
+++ b/sys/arm64/arm64/intr_machdep.c
@@ -105,7 +105,7 @@ static void
 intr_init(void *dummy __unused)
 {
 
-	mtx_init(&intr_list_lock, "intr sources lock", NULL, MTX_DEF);
+	mtx_init(&intr_list_lock, "intr sources lock", NULL, MTX_SPIN);
 }
 SYSINIT(intr_init, SI_SUB_INTR, SI_ORDER_FIRST, intr_init, NULL);
 
@@ -123,33 +123,44 @@ intrcnt_setname(const char *name, u_int idx)
 }
 
 /*
+ * Find the interrupt descriptor in the list
+ * based on the hardware IRQ number.
+ */
+static __inline struct arm64_intr_entry *
+intr_lookup_locked(u_int hw_irq)
+{
+	struct arm64_intr_entry *intr;
+
+	mtx_assert(&intr_list_lock, MA_OWNED);
+	SLIST_FOREACH(intr, &irq_slist_head, entries) {
+		if (intr->i_hw_irq == hw_irq)
+			return (intr);
+	}
+	return (NULL);
+}
+/*
  * Get intr structure for the given interrupt number.
  * Allocate one if this is the first time.
- * (Similar to ppc's intr_lookup() but without actual
- * lookup since irq number is an index in arm64_intrs[]).
  */
 static struct arm64_intr_entry *
-intr_acquire(u_int hw_irq)
+intr_allocate(u_int hw_irq)
 {
 	struct arm64_intr_entry *intr;
 
-	mtx_lock(&intr_list_lock);
-
-	SLIST_FOREACH(intr, &irq_slist_head, entries) {
-		if (intr->i_hw_irq == hw_irq) {
-			break;
-		}
-	}
+	/* Check if already allocated */
+	mtx_lock_spin(&intr_list_lock);
+	intr = intr_lookup_locked(hw_irq);
+	mtx_unlock_spin(&intr_list_lock);
 	if (intr != NULL)
-		goto out;
+		return (intr);
 
 	/* Do not alloc another intr when max number of IRQs has been reached */
 	if (intrcntidx >= NIRQS)
-		goto out;
+		return (NULL);
 
 	intr = malloc(sizeof(*intr), M_INTR, M_NOWAIT);
 	if (intr == NULL)
-		goto out;
+		return (NULL);
 
 	intr->i_event = NULL;
 	intr->i_handlers = 0;
@@ -158,9 +169,10 @@ intr_acquire(u_int hw_irq)
 	intr->i_cntidx = atomic_fetchadd_int(&intrcntidx, 1);
 	intr->i_cntp = &intrcnt[intr->i_cntidx];
 	intr->i_hw_irq = hw_irq;
+	mtx_lock_spin(&intr_list_lock);
 	SLIST_INSERT_HEAD(&irq_slist_head, intr, entries);
-out:
-	mtx_unlock(&intr_list_lock);
+	mtx_unlock_spin(&intr_list_lock);
+
 	return intr;
 }
 
@@ -312,7 +324,7 @@ arm_setup_intr(const char *name, driver_filter_t *filt, driver_intr_t handler,
 	struct arm64_intr_entry *intr;
 	int error;
 
-	intr = intr_acquire(hw_irq);
+	intr = intr_allocate(hw_irq);
 	if (intr == NULL)
 		return (ENOMEM);
 
@@ -336,7 +348,6 @@ arm_setup_intr(const char *name, driver_filter_t *filt, driver_intr_t handler,
 	    intr_priority(flags), flags, cookiep);
 
 	if (!error) {
-		mtx_lock(&intr_list_lock);
 		intrcnt_setname(intr->i_event->ie_fullname, intr->i_cntidx);
 		intr->i_handlers++;
 
@@ -349,7 +360,6 @@ arm_setup_intr(const char *name, driver_filter_t *filt, driver_intr_t handler,
 
 			PIC_UNMASK(root_pic, intr->i_hw_irq);
 		}
-		mtx_unlock(&intr_list_lock);
 	}
 
 	return (error);
@@ -364,12 +374,10 @@ arm_teardown_intr(void *cookie)
 	intr = intr_handler_source(cookie);
 	error = intr_event_remove_handler(cookie);
 	if (!error) {
-		mtx_lock(&intr_list_lock);
 		intr->i_handlers--;
 		if (intr->i_handlers == 0)
 			PIC_MASK(root_pic, intr->i_hw_irq);
 		intrcnt_setname(intr->i_event->ie_fullname, intr->i_cntidx);
-		mtx_unlock(&intr_list_lock);
 	}
 
 	return (error);
@@ -380,9 +388,11 @@ arm_config_intr(u_int hw_irq, enum intr_trigger trig, enum intr_polarity pol)
 {
 	struct arm64_intr_entry *intr;
 
-	intr = intr_acquire(hw_irq);
+	mtx_lock_spin(&intr_list_lock);
+	intr = intr_lookup_locked(hw_irq);
+	mtx_unlock_spin(&intr_list_lock);
 	if (intr == NULL)
-		return (ENOMEM);
+		return (EINVAL);
 
 	intr->i_trig = trig;
 	intr->i_pol = pol;
@@ -398,12 +408,9 @@ arm_dispatch_intr(u_int hw_irq, struct trapframe *tf)
 {
 	struct arm64_intr_entry *intr;
 
-	SLIST_FOREACH(intr, &irq_slist_head, entries) {
-		if (intr->i_hw_irq == hw_irq) {
-			break;
-		}
-	}
-
+	mtx_lock_spin(&intr_list_lock);
+	intr = intr_lookup_locked(hw_irq);
+	mtx_unlock_spin(&intr_list_lock);
 	if (intr == NULL)
 		goto stray;