Rework/comment on the userspace interrupt handling

5 files changed, 58 insertions, 52 deletions

diff --git a/device/ds_routines.c b/device/ds_routines.c
index 9d399ee..7a5dda5 100644
--- a/device/ds_routines.c
+++ b/device/ds_routines.c
@@ -325,9 +325,7 @@ experimental_device_intr_register (ipc_port_t master_port, int line,
 #ifdef MACH_XEN
   return D_INVALID_OPERATION;
 #else	/* MACH_XEN */
-  extern int install_user_intr_handler (unsigned int line,
-					unsigned long flags,
-					ipc_port_t dest);
+  struct intr_entry *entry;
   io_return_t ret;
 
   /* Open must be called on the master device port.  */
@@ -338,13 +336,13 @@ experimental_device_intr_register (ipc_port_t master_port, int line,
   if (line < 0 || line >= 16)
     return D_INVALID_OPERATION;
 
-  ret = insert_intr_entry (line, receive_port);
+  ret = insert_intr_entry (line, receive_port, &entry);
   if (ret)
     return ret;
   // TODO The original port should be replaced
   // when the same device driver calls it again, 
   // in order to handle the case that the device driver crashes and restarts.
-  ret = install_user_intr_handler (line, flags, receive_port);
+  ret = install_user_intr_handler (line, flags, entry);
 
   /* If the port is installed successfully, increase its reference by 1.
    * Thus, the port won't be destroyed after its task is terminated. */
diff --git a/device/interrupt.h b/device/interrupt.h
new file mode 100644
index 0000000..0de43c2
--- /dev/null
+++ b/device/interrupt.h
@@ -0,0 +1,14 @@
+#ifndef DEVICE_INTERRUPT_H
+#define DEVICE_INTERRUPT_H
+
+struct intr_entry;
+boolean_t queue_intr (struct intr_entry *e);
+int insert_intr_entry (int line, ipc_port_t dest, struct intr_entry **entry);
+
+int install_user_intr_handler (unsigned int line,
+			       unsigned long flags,
+			       struct intr_entry *entry);
+
+void intr_thread (void);
+
+#endif	/* DEVICE_INTERRUPT_H */
diff --git a/device/intr.c b/device/intr.c
index 02e0bab..986f0ee 100644
--- a/device/intr.c
+++ b/device/intr.c
@@ -3,6 +3,8 @@
 #include <kern/queue.h>
 #include <kern/printf.h>
 
+#include "interrupt.h"
+
 #ifndef MACH_XEN
 // TODO this is only for x86 system
 #define sti() __asm__ __volatile__ ("sti": : :"memory")
@@ -35,29 +37,35 @@ search_intr (int line, ipc_port_t dest)
   return NULL;
 }
 
-void intr_thread (void);
-
 /* This function can only be used in the interrupt handler. */
-void
-queue_intr (int line, ipc_port_t dest)
+boolean_t
+queue_intr (struct intr_entry *e)
 {
-  struct intr_entry *e;
-  
+  /* The reference of the port was increased when the port was
+   * installed.  If the reference is 1, it means the port should have
+   * been destroyed and I destroy it now. */
+  if (e->dest && e->dest->ip_references == 1)
+    {
+      ipc_port_release (e->dest);
+      e->dest = NULL;
+      printk ("irq handler %d: release an dead delivery port\n", e->line);
+      return FALSE;
+    }
+
   cli ();
-  e = search_intr (line, dest);
-  assert (e);
   e->interrupts++;
   tot_num_intr++;
   sti ();
 
   thread_wakeup ((event_t) &intr_thread);
+  return TRUE;
 }
 
 /* insert an interrupt entry in the queue.
  * This entry exists in the queue until
  * the corresponding interrupt port is removed.*/
 int
-insert_intr_entry (int line, ipc_port_t dest)
+insert_intr_entry (int line, ipc_port_t dest, struct intr_entry **entry)
 {
   int err = 0;
   struct intr_entry *e, *new;
@@ -86,19 +94,10 @@ out:
   sti ();
   if (free)
     kfree ((vm_offset_t) new, sizeof (*new));
+  *entry = new;
   return err;
 }
 
-/* this function should be called when line is disabled. */
-void mark_intr_removed (int line, ipc_port_t dest)
-{
-  struct intr_entry *e;
-
-  e = search_intr (line, dest);
-  if (e)
-    e->dest = NULL;
-}
-
 void
 intr_thread ()
 {
diff --git a/kern/startup.c b/kern/startup.c
index 05de5e6..0a571ff 100644
--- a/kern/startup.c
+++ b/kern/startup.c
@@ -62,6 +62,7 @@
 #include <machine/model_dep.h>
 #include <mach/version.h>
 #include <device/device_init.h>
+#include <device/interrupt.h>
 
 #if MACH_KDB
 #include <device/cons.h>
@@ -79,7 +80,6 @@ boolean_t reboot_on_panic = TRUE;
 /* XX */
 extern vm_offset_t phys_first_addr, phys_last_addr;
 extern char *kernel_cmdline;
-extern void	intr_thread();
 
 /*
  *	Running in virtual memory, on the interrupt stack.
diff --git a/linux/dev/arch/i386/kernel/irq.c b/linux/dev/arch/i386/kernel/irq.c
index b7dfa1a..15f95c3 100644
--- a/linux/dev/arch/i386/kernel/irq.c
+++ b/linux/dev/arch/i386/kernel/irq.c
@@ -49,6 +49,7 @@
 
 #include <linux/dev/glue/glue.h>
 #include <machine/machspl.h>
+#include <device/interrupt.h>
 
 #if 0
 /* XXX: This is the way it's done in linux 2.2. GNU Mach currently uses intr_count. It should be made using local_{bh/irq}_count instead (through hardirq_enter/exit) for SMP support. */
@@ -83,7 +84,7 @@ struct linux_action
   void *dev_id;
   struct linux_action *next;
   unsigned long flags;
-  volatile ipc_port_t delivery_port;
+  struct intr_entry *userspace_handler;
 };
 
 static struct linux_action *irq_action[16] =
@@ -117,31 +118,19 @@ linux_intr (int irq)
     {
       // TODO I might need to check whether the interrupt belongs to
       // the current device. But I don't do it for now.
-      if (action->delivery_port)
+      if (action->userspace_handler)
 	{
-	  /* The reference of the port was increased
-	   * when the port was installed.
-	   * If the reference is 1, it means the port should
-	   * have been destroyed and I destroy it now. */
-	  if (action->delivery_port
-	      && action->delivery_port->ip_references == 1)
-	    {
-	      mark_intr_removed (irq, action->delivery_port);
-	      ipc_port_release (action->delivery_port);
-	      *prev = action->next;
-	      printk ("irq handler %d: release an dead delivery port\n", irq);
-	      linux_kfree(action);
-	      action = *prev;
-	      continue;
-	    }
-	  else
-	    {
 	      /* We disable the irq here and it will be enabled
 	       * after the interrupt is handled by the user space driver. */
 	      disable_irq (irq);
-	      queue_intr (irq, action->delivery_port);
-	    }
-
+	      if (! queue_intr (action->userspace_handler))
+		{
+		  *prev = action->next;
+		  linux_kfree(action);
+		  action = *prev;
+		  enable_irq (irq);
+		  continue;
+		}
 	}
       else if (action->handler)
 	action->handler (irq, action->dev_id, &regs);
@@ -285,26 +274,32 @@ setup_x86_irq (int irq, struct linux_action *new)
 
 int
 install_user_intr_handler (unsigned int irq, unsigned long flags,
-			  ipc_port_t dest)
+			   struct intr_entry *entry)
 {
   struct linux_action *action;
+#if 0
   struct linux_action *old;
+#endif
   int retval;
 
   assert (irq < 16);
 
-  /* Test whether the irq handler has been set */
+#if 0
+  /* Test whether the irq handler has already been set.  */
+  /* JW: Actually, that cannot happen.  We test for that in
+     device_intr_register before calling this function.  */
   // TODO I need to protect the array when iterating it.
   old = irq_action[irq];
   while (old)
     {
-      if (old->delivery_port == dest)
+      if (old->delivery_port == entry->dest)
 	{
 	  printk ("The interrupt handler has been installed on line %d", irq);
 	  return linux_to_mach_error (-EAGAIN);
 	}
       old = old->next;
     }
+#endif
 
   /*
    * Hmm... Should I use `kalloc()' ?
@@ -319,7 +314,7 @@ install_user_intr_handler (unsigned int irq, unsigned long flags,
   action->next = NULL;
   action->dev_id = NULL;
   action->flags = flags;
-  action->delivery_port = dest;
+  action->userspace_handler = entry;
   
   retval = setup_x86_irq (irq, action);
   if (retval)
@@ -356,7 +351,7 @@ request_irq (unsigned int irq, void (*handler) (int, void *, struct pt_regs *),
   action->next = NULL;
   action->dev_id = dev_id;
   action->flags = flags;
-  action->delivery_port = NULL;
+  action->userspace_handler = NULL;
   
   retval = setup_x86_irq (irq, action);
   if (retval)