Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 11 Feb 2016 11:57:14 +0000 (UTC)
From:      Zbigniew Bodek <zbb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r295514 - head/sys/arm64/arm64
Message-ID:  <201602111157.u1BBvENP050235@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: zbb
Date: Thu Feb 11 11:57:13 2016
New Revision: 295514
URL: https://svnweb.freebsd.org/changeset/base/295514

Log:
  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

Modified:
  head/sys/arm64/arm64/intr_machdep.c

Modified: head/sys/arm64/arm64/intr_machdep.c
==============================================================================
--- head/sys/arm64/arm64/intr_machdep.c	Thu Feb 11 11:57:12 2016	(r295513)
+++ head/sys/arm64/arm64/intr_machdep.c	Thu Feb 11 11:57:13 2016	(r295514)
@@ -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 
 }
 
 /*
+ * 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_
 	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_
 	    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_
 
 			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_
 {
 	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 t
 {
 	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;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201602111157.u1BBvENP050235>