Changes in kernel/generic/src/ipc/irq.c [82cbf8c6:8a637a4] in mainline

File:

• kernel/generic/src/ipc/irq.c (modified) (12 diffs)

kernel/generic/src/ipc/irq.c

@@ -37 +37 @@
  *
  * This framework allows applications to subscribe to receive a notification
- * when an interrupt is detected. The application may provide a simple
- * 'top-half' handler as part of its registration, which can perform simple
- * operations (read/write port/memory, add information to notification IPC
- * message).
+ * when interrupt is detected. The application may provide a simple 'top-half'
+ * handler as part of its registration, which can perform simple operations
+ * (read/write port/memory, add information to notification IPC message).
  *
  * The structure of a notification message is as follows:
@@ -51 +50 @@
  * - in_phone_hash: interrupt counter (may be needed to assure correct order
  *                  in multithreaded drivers)
+ *
+ * Note on synchronization for ipc_irq_subscribe(), ipc_irq_unsubscribe(),
+ * ipc_irq_cleanup() and IRQ handlers:
+ *
+ *   By always taking all of the uspace IRQ hash table lock, IRQ structure lock
+ *   and answerbox lock, we can rule out race conditions between the
+ *   registration functions and also the cleanup function. Thus the observer can
+ *   either see the IRQ structure present in both the hash table and the
+ *   answerbox list or absent in both. Views in which the IRQ structure would be
+ *   linked in the hash table but not in the answerbox list, or vice versa, are
+ *   not possible.
+ *
+ *   By always taking the hash table lock and the IRQ structure lock, we can
+ *   rule out a scenario in which we would free up an IRQ structure, which is
+ *   still referenced by, for example, an IRQ handler. The locking scheme forces
+ *   us to lock the IRQ structure only after any progressing IRQs on that
+ *   structure are finished. Because we hold the hash table lock, we prevent new
+ *   IRQs from taking new references to the IRQ structure.
+ *
  */
 
 #include <arch.h>
-#include <assert.h>
 #include <mm/slab.h>
 #include <mm/page.h>
@@ -66 +83 @@
 #include <print.h>
 #include <macros.h>
-#include <cap/cap.h>
 
 static void ranges_unmap(irq_pio_range_t *ranges, size_t rangecount)
@@ -101 +117 @@
         }
         
-        /* Rewrite the IRQ code addresses from physical to kernel virtual. */
+        /* Rewrite the pseudocode addresses from physical to kernel virtual. */
         for (size_t i = 0; i < cmdcount; i++) {
                 uintptr_t addr;
@@ -159 +175 @@
 }
 
-/** Statically check the top-half IRQ code
- *
- * Check the top-half IRQ code for invalid or unsafe constructs.
+/** Statically check the top-half pseudocode
+ *
+ * Check the top-half pseudocode for invalid or unsafe
+ * constructs.
  *
  */
@@ -198 +215 @@
 }
 
-/** Free the top-half IRQ code.
- *
- * @param code Pointer to the top-half IRQ code.
+/** Free the top-half pseudocode.
+ *
+ * @param code Pointer to the top-half pseudocode.
  *
  */
@@ -213 +230 @@
 }
 
-/** Copy the top-half IRQ code from userspace into the kernel.
- *
- * @param ucode Userspace address of the top-half IRQ code.
- *
- * @return Kernel address of the copied IRQ code.
+/** Copy the top-half pseudocode from userspace into the kernel.
+ *
+ * @param ucode Userspace address of the top-half pseudocode.
+ *
+ * @return Kernel address of the copied pseudocode.
  *
  */
@@ -271 +288 @@
 }
 
-static void irq_destroy(void *arg)
-{
-        irq_t *irq = (irq_t *) arg;
-
-        /* Free up the IRQ code and associated structures. */
-        code_free(irq->notif_cfg.code);
-        slab_free(irq_slab, irq);
-}
-
-static kobject_ops_t irq_kobject_ops = {
-        .destroy = irq_destroy
-};
-
 /** Subscribe an answerbox as a receiving end for IRQ notifications.
  *
  * @param box     Receiving answerbox.
  * @param inr     IRQ number.
- * @param imethod Interface and method to be associated with the notification.
- * @param ucode   Uspace pointer to top-half IRQ code.
- *
- * @return  IRQ capability handle.
- * @return  Negative error code.
- *
- */
-int ipc_irq_subscribe(answerbox_t *box, inr_t inr, sysarg_t imethod,
-    irq_code_t *ucode)
-{
+ * @param devno   Device number.
+ * @param imethod Interface and method to be associated with the
+ *                notification.
+ * @param ucode   Uspace pointer to top-half pseudocode.
+ *
+ * @return EOK on success or a negative error code.
+ *
+ */
+int ipc_irq_subscribe(answerbox_t *box, inr_t inr, devno_t devno,
+    sysarg_t imethod, irq_code_t *ucode)
+{
+        sysarg_t key[] = {
+                (sysarg_t) inr,
+                (sysarg_t) devno
+        };
+        
         if ((inr < 0) || (inr > last_inr))
                 return ELIMIT;
@@ -310 +320 @@
         
         /*
-         * Allocate and populate the IRQ kernel object.
+         * Allocate and populate the IRQ structure.
          */
-        cap_handle_t handle = cap_alloc(TASK);
-        if (handle < 0)
-                return handle;
-        
-        irq_t *irq = (irq_t *) slab_alloc(irq_slab, FRAME_ATOMIC);
-        if (!irq) {
-                cap_free(TASK, handle);
-                return ENOMEM;
-        }
-
-        kobject_t *kobject = malloc(sizeof(kobject_t), FRAME_ATOMIC);
-        if (!kobject) {
-                cap_free(TASK, handle);
-                slab_free(irq_slab, irq);
-                return ENOMEM;
-        }
+        irq_t *irq = malloc(sizeof(irq_t), 0);
         
         irq_initialize(irq);
+        irq->devno = devno;
         irq->inr = inr;
         irq->claim = ipc_irq_top_half_claim;
@@ -340 +336 @@
         
         /*
-         * Insert the IRQ structure into the uspace IRQ hash table.
+         * Enlist the IRQ structure in the uspace IRQ hash table and the
+         * answerbox's list.
          */
         irq_spinlock_lock(&irq_uspace_hash_table_lock, true);
+        
+        link_t *hlp = hash_table_find(&irq_uspace_hash_table, key);
+        if (hlp) {
+                irq_t *hirq = hash_table_get_instance(hlp, irq_t, link);
+                
+                /* hirq is locked */
+                irq_spinlock_unlock(&hirq->lock, false);
+                code_free(code);
+                irq_spinlock_unlock(&irq_uspace_hash_table_lock, true);
+                
+                free(irq);
+                return EEXIST;
+        }
+        
+        /* Locking is not really necessary, but paranoid */
         irq_spinlock_lock(&irq->lock, false);
-        
-        irq->notif_cfg.hashed_in = true;
-        hash_table_insert(&irq_uspace_hash_table, &irq->link);
-        
+        irq_spinlock_lock(&box->irq_lock, false);
+        
+        hash_table_insert(&irq_uspace_hash_table, key, &irq->link);
+        list_append(&irq->notif_cfg.link, &box->irq_list);
+        
+        irq_spinlock_unlock(&box->irq_lock, false);
         irq_spinlock_unlock(&irq->lock, false);
         irq_spinlock_unlock(&irq_uspace_hash_table_lock, true);
-
-        kobject_initialize(kobject, KOBJECT_TYPE_IRQ, irq, &irq_kobject_ops);
-        cap_publish(TASK, handle, kobject);
-        
-        return handle;
+        
+        return EOK;
 }
 
 /** Unsubscribe task from IRQ notification.
  *
- * @param box     Answerbox associated with the notification.
- * @param handle  IRQ capability handle.
+ * @param box   Answerbox associated with the notification.
+ * @param inr   IRQ number.
+ * @param devno Device number.
  *
  * @return EOK on success or a negative error code.
  *
  */
-int ipc_irq_unsubscribe(answerbox_t *box, int handle)
-{
-        kobject_t *kobj = cap_unpublish(TASK, handle, KOBJECT_TYPE_IRQ);
-        if (!kobj)
+int ipc_irq_unsubscribe(answerbox_t *box, inr_t inr, devno_t devno)
+{
+        sysarg_t key[] = {
+                (sysarg_t) inr,
+                (sysarg_t) devno
+        };
+        
+        if ((inr < 0) || (inr > last_inr))
+                return ELIMIT;
+        
+        irq_spinlock_lock(&irq_uspace_hash_table_lock, true);
+        link_t *lnk = hash_table_find(&irq_uspace_hash_table, key);
+        if (!lnk) {
+                irq_spinlock_unlock(&irq_uspace_hash_table_lock, true);
                 return ENOENT;
-        
-        assert(kobj->irq->notif_cfg.answerbox == box);
-
+        }
+        
+        irq_t *irq = hash_table_get_instance(lnk, irq_t, link);
+        
+        /* irq is locked */
+        irq_spinlock_lock(&box->irq_lock, false);
+        
+        ASSERT(irq->notif_cfg.answerbox == box);
+        
+        /* Remove the IRQ from the answerbox's list. */
+        list_remove(&irq->notif_cfg.link);
+        
+        /*
+         * We need to drop the IRQ lock now because hash_table_remove() will try
+         * to reacquire it. That basically violates the natural locking order,
+         * but a deadlock in hash_table_remove() is prevented by the fact that
+         * we already held the IRQ lock and didn't drop the hash table lock in
+         * the meantime.
+         */
+        irq_spinlock_unlock(&irq->lock, false);
+        
+        /* Remove the IRQ from the uspace IRQ hash table. */
+        hash_table_remove(&irq_uspace_hash_table, key, 2);
+        
+        irq_spinlock_unlock(&box->irq_lock, false);
+        irq_spinlock_unlock(&irq_uspace_hash_table_lock, true);
+        
+        /* Free up the pseudo code and associated structures. */
+        code_free(irq->notif_cfg.code);
+        
+        /* Free up the IRQ structure. */
+        free(irq);
+        
+        return EOK;
+}
+
+/** Disconnect all IRQ notifications from an answerbox.
+ *
+ * This function is effective because the answerbox contains
+ * list of all irq_t structures that are subscribed to
+ * send notifications to it.
+ *
+ * @param box Answerbox for which we want to carry out the cleanup.
+ *
+ */
+void ipc_irq_cleanup(answerbox_t *box)
+{
+loop:
         irq_spinlock_lock(&irq_uspace_hash_table_lock, true);
-        irq_spinlock_lock(&kobj->irq->lock, false);
-        
-        if (kobj->irq->notif_cfg.hashed_in) {
-                /* Remove the IRQ from the uspace IRQ hash table. */
-                hash_table_remove_item(&irq_uspace_hash_table,
-                    &kobj->irq->link);
-                kobj->irq->notif_cfg.hashed_in = false;
-        }
-
-        irq_spinlock_unlock(&kobj->irq->lock, false);
+        irq_spinlock_lock(&box->irq_lock, false);
+        
+        while (!list_empty(&box->irq_list)) {
+                DEADLOCK_PROBE_INIT(p_irqlock);
+                
+                irq_t *irq = list_get_instance(list_first(&box->irq_list), irq_t,
+                    notif_cfg.link);
+                
+                if (!irq_spinlock_trylock(&irq->lock)) {
+                        /*
+                         * Avoid deadlock by trying again.
+                         */
+                        irq_spinlock_unlock(&box->irq_lock, false);
+                        irq_spinlock_unlock(&irq_uspace_hash_table_lock, true);
+                        DEADLOCK_PROBE(p_irqlock, DEADLOCK_THRESHOLD);
+                        goto loop;
+                }
+                
+                sysarg_t key[2];
+                key[0] = irq->inr;
+                key[1] = irq->devno;
+                
+                ASSERT(irq->notif_cfg.answerbox == box);
+                
+                /* Unlist from the answerbox. */
+                list_remove(&irq->notif_cfg.link);
+                
+                /*
+                 * We need to drop the IRQ lock now because hash_table_remove()
+                 * will try to reacquire it. That basically violates the natural
+                 * locking order, but a deadlock in hash_table_remove() is
+                 * prevented by the fact that we already held the IRQ lock and
+                 * didn't drop the hash table lock in the meantime.
+                 */
+                irq_spinlock_unlock(&irq->lock, false);
+                
+                /* Remove from the hash table. */
+                hash_table_remove(&irq_uspace_hash_table, key, 2);
+                
+                /*
+                 * Release both locks so that we can free the pseudo code.
+                 */
+                irq_spinlock_unlock(&box->irq_lock, false);
+                irq_spinlock_unlock(&irq_uspace_hash_table_lock, true);
+                
+                code_free(irq->notif_cfg.code);
+                free(irq);
+                
+                /* Reacquire both locks before taking another round. */
+                irq_spinlock_lock(&irq_uspace_hash_table_lock, true);
+                irq_spinlock_lock(&box->irq_lock, false);
+        }
+        
+        irq_spinlock_unlock(&box->irq_lock, false);
         irq_spinlock_unlock(&irq_uspace_hash_table_lock, true);
-
-        kobject_put(kobj);
-        cap_free(TASK, handle);
-        
-        return EOK;
 }
 
@@ -409 +515 @@
 }
 
-/** Apply the top-half IRQ code to find out whether to accept the IRQ or not.
+/** Apply the top-half pseudo code to find out whether to accept the IRQ or not.
  *
  * @param irq IRQ structure.
  *
- * @return IRQ_ACCEPT if the interrupt is accepted by the IRQ code.
- * @return IRQ_DECLINE if the interrupt is not accepted byt the IRQ code.
+ * @return IRQ_ACCEPT if the interrupt is accepted by the
+ *         pseudocode, IRQ_DECLINE otherwise.
  *
  */
@@ -501 +607 @@
 void ipc_irq_top_half_handler(irq_t *irq)
 {
-        assert(irq);
-        
-        assert(interrupts_disabled());
-        assert(irq_spinlock_locked(&irq->lock));
+        ASSERT(irq);
+        
+        ASSERT(interrupts_disabled());
+        ASSERT(irq_spinlock_locked(&irq->lock));
         
         if (irq->notif_cfg.answerbox) {