Fix reference counting; use dead-name notification

5 files changed, 141 insertions, 59 deletions

diff --git a/device/ds_routines.c b/device/ds_routines.c
index 7a5dda5..2b9869a 100644
--- a/device/ds_routines.c
+++ b/device/ds_routines.c
@@ -87,6 +87,7 @@
 #include <device/dev_hdr.h>
 #include <device/conf.h>
 #include <device/io_req.h>
+#include <device/interrupt.h>
 #include <device/ds_routines.h>
 #include <device/net_status.h>
 #include <device/device_port.h>
@@ -332,6 +333,9 @@ experimental_device_intr_register (ipc_port_t master_port, int line,
   if (master_port != master_device_port)
     return D_INVALID_OPERATION;
 
+  if (receive_port == MACH_PORT_NULL)
+    return D_INVALID_OPERATION;
+
   /* XXX: move to arch-specific */
   if (line < 0 || line >= 16)
     return D_INVALID_OPERATION;
@@ -339,17 +343,8 @@ experimental_device_intr_register (ipc_port_t master_port, int line,
   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, entry);
-
-  /* If the port is installed successfully, increase its reference by 1.
-   * Thus, the port won't be destroyed after its task is terminated. */
-  if (ret == 0)
-    ip_reference (receive_port);
 
-  return ret;
+  return install_user_intr_handler (line, flags, entry);
 #endif	/* MACH_XEN */
 }
 
diff --git a/device/interrupt.h b/device/interrupt.h
index 0de43c2..8adc02d 100644
--- a/device/interrupt.h
+++ b/device/interrupt.h
@@ -3,12 +3,14 @@
 
 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);
+kern_return_t 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);
 
+boolean_t intr_entry_notify (mach_msg_header_t *msg);
 void intr_thread (void);
 
 #endif	/* DEVICE_INTERRUPT_H */
diff --git a/device/intr.c b/device/intr.c
index 0fa43c5..7a1eccb 100644
--- a/device/intr.c
+++ b/device/intr.c
@@ -1,7 +1,9 @@
 #include <device/intr.h>
 #include <device/ds_routines.h>
+#include <ipc/ipc_space.h>
 #include <kern/queue.h>
 #include <kern/printf.h>
+#include <mach/notify.h>
 
 #include "interrupt.h"
 
@@ -15,12 +17,14 @@ static struct kmem_cache intr_entry_cache;
 
 struct intr_entry
 {
+  ipc_port_t port;	/* We receive notifications on this port.  */
   queue_chain_t chain;
   ipc_port_t dest;
   int line;
   /* The number of interrupts occur since last run of intr_thread. */
   int interrupts;
 };
+typedef struct intr_entry *intr_entry_t;
 
 static queue_head_t intr_queue;
 /* The total number of unprocessed interrupts. */
@@ -30,18 +34,6 @@ static int tot_num_intr;
 boolean_t
 queue_intr (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)
-    {
-      /* JW: I don't like running this from an interrupt handler.  */
-      ipc_port_release (e->dest);
-      e->dest = NULL;
-      printf ("irq handler %d: release an dead delivery port\n", e->line);
-      return FALSE;
-    }
-
   cli ();
   e->interrupts++;
   tot_num_intr++;
@@ -54,20 +46,58 @@ queue_intr (struct intr_entry *e)
 /* insert an interrupt entry in the queue.
  * This entry exists in the queue until
  * the corresponding interrupt port is removed.*/
-int
+kern_return_t
 insert_intr_entry (int line, ipc_port_t dest, struct intr_entry **entry)
 {
-  int err = 0;
+  kern_return_t err = 0;
   struct intr_entry *e, *new;
   int free = 0;
+  ipc_port_t dnnotify;
+  ipc_port_request_index_t dnindex;
 
   new = (struct intr_entry *) kmem_cache_alloc (&intr_entry_cache);
   if (new == NULL)
     return D_NO_MEMORY;
+
+  /* Allocate port, keeping a reference for it.  */
+  new->port = ipc_port_alloc_kernel ();
+  if (new->port == IP_NULL)
+    {
+      kmem_cache_free (&intr_entry_cache, (vm_offset_t) new);
+      return KERN_RESOURCE_SHORTAGE;
+    }
+
+  /* Associate the port with the object.  */
+  ipc_kobject_set (new->port, (ipc_kobject_t) new, IKOT_INTR_ENTRY);
+
   new->line = line;
   new->dest = dest;
   new->interrupts = 0;
 
+  /* Register a dead-name notification so that we are notified if the
+     userspace handler dies.  */
+  dnnotify = ipc_port_make_sonce (new->port);
+  ip_lock (dest);
+  /* We use a bogus port name.  We don't need it to distinguish the
+     notifications because we register just one per object.  */
+ retry:
+  err = ipc_port_dnrequest (dest, (mach_port_t) 1, dnnotify, &dnindex);
+  if (err)
+    {
+      err = ipc_port_dngrow (dest);
+      /* dest is unlocked */
+      if (err != KERN_SUCCESS)
+	{
+	  ipc_port_release_sonce (dnnotify);
+	  kmem_cache_free (&intr_entry_cache, (vm_offset_t) new);
+	  return err;
+	}
+      ip_lock (dest);
+      goto retry;
+    }
+  /* dest is locked.  dnindex is only valid until we unlock it and we
+     might decide to cancel.  */
+
   /* check whether the intr entry has been in the queue. */
   cli ();
   queue_iterate (&intr_queue, e, struct intr_entry *, chain)
@@ -84,11 +114,74 @@ insert_intr_entry (int line, ipc_port_t dest, struct intr_entry **entry)
 out:
   sti ();
   if (free)
-    kmem_cache_free (&intr_entry_cache, (vm_offset_t) new);
+    {
+      ipc_port_dncancel (new->dest, (mach_port_t) 1, dnindex);
+      ip_unlock (new->dest);
+      ipc_port_release_sonce (dnnotify);
+      ipc_port_dealloc_kernel (new->port);
+      ipc_port_release_send (new->dest);
+      kmem_cache_free (&intr_entry_cache, (vm_offset_t) new);
+    }
+  else
+    ip_unlock (new->dest);
   *entry = new;
   return err;
 }
 
+/* Lookup a intr_entry object by its port.  */
+static intr_entry_t
+intr_entry_port_lookup (ipc_port_t port)
+{
+  struct intr_entry *entry;
+
+  if (!IP_VALID(port))
+    return 0;
+
+  ip_lock (port);
+  if (ip_active (port) && (ip_kotype (port) == IKOT_INTR_ENTRY))
+    entry = (struct intr_entry *) port->ip_kobject;
+  else
+    entry = 0;
+  ip_unlock (port);
+  return entry;
+}
+
+/* Process a dead-name notification for a userspace interrupt handler
+   notification port.  */
+boolean_t
+intr_entry_notify (mach_msg_header_t *msg)
+{
+  if (msg->msgh_id == MACH_NOTIFY_DEAD_NAME)
+    {
+      struct intr_entry *entry;
+      int line;
+      mach_dead_name_notification_t *dn;
+
+      dn = (mach_dead_name_notification_t *) msg;
+      entry = intr_entry_port_lookup
+	((ipc_port_t) dn->not_header.msgh_remote_port);
+      assert (entry);
+
+      cli ();
+      line = entry->line;
+      assert (!queue_empty (&intr_queue));
+      queue_remove (&intr_queue, entry, struct intr_entry *, chain);
+      sti ();
+
+      ipc_port_dealloc_kernel (entry->port);
+      ipc_port_release_send (entry->dest);
+      kmem_cache_free (&intr_entry_cache, (vm_offset_t) entry);
+
+      printf ("irq handler %d: userspace handler died\n", line);
+      return TRUE;
+    }
+
+  printf ("intr_entry_notify: strange notification %d\n",
+	  msg->msgh_id);
+  return FALSE;
+}
+
+
 mach_intr_notification_t mach_intr_notification_template;
 
 static void
@@ -156,43 +249,26 @@ intr_thread ()
     {
       assert_wait ((event_t) &intr_thread, FALSE);
       cli ();
-      while (tot_num_intr)
-	{
-	  int del = 0;
 
-	  queue_iterate (&intr_queue, e, struct intr_entry *, chain)
+      while (tot_num_intr)
+	queue_iterate (&intr_queue, e, struct intr_entry *, chain)
+	  if (e->interrupts)
 	    {
-	      /* if an entry doesn't have dest port,
-	       * we should remove it. */
-	      if (e->dest == NULL)
-		{
-		  del = 1;
-		  break;
-		}
-
-	      if (e->interrupts)
-		{
-		  line = e->line;
-		  dest = e->dest;
-		  e->interrupts--;
-		  tot_num_intr--;
-
-		  sti ();
-		  deliver_intr (line, dest);
-		  cli ();
-		}
-	    }
+	      line = e->line;
+	      dest = e->dest;
+	      e->interrupts--;
+	      tot_num_intr--;
 
-	  /* remove the entry without dest port from the queue and free it. */
-	  if (del)
-	    {
-	      assert (!queue_empty (&intr_queue));
-	      queue_remove (&intr_queue, e, struct intr_entry *, chain);
 	      sti ();
-	      kmem_cache_free (&intr_entry_cache, (vm_offset_t) e);
+	      deliver_intr (line, dest);
 	      cli ();
+
+	      /* We cannot assume that e still exists at this point
+		 because we released the lock.  Hence we restart the
+		 iteration.  */
+	      break;
 	    }
-	}
+
       sti ();
       thread_block (thread_no_continuation);
     }
diff --git a/kern/ipc_kobject.c b/kern/ipc_kobject.c
index c65458b..97c632a 100644
--- a/kern/ipc_kobject.c
+++ b/kern/ipc_kobject.c
@@ -48,6 +48,7 @@
 #include <vm/vm_object.h>
 #include <vm/memory_object_proxy.h>
 #include <device/ds_routines.h>
+#include <device/interrupt.h>
 
 #include <kern/mach.server.h>
 #include <ipc/mach_port.server.h>
@@ -326,6 +327,10 @@ ipc_kobject_destroy(
 		vm_object_pager_wakeup(port);
 		break;
 
+	    case IKOT_INTR_ENTRY:
+		/* Do nothing.  */
+		break;
+
 	    default:
 #if	MACH_ASSERT
 		printf("ipc_kobject_destroy: port 0x%p, kobj 0x%lx, type %d\n",
@@ -368,6 +373,9 @@ ipc_kobject_notify(request_header, reply_header)
 		case IKOT_PAGER_PROXY:
 		return memory_object_proxy_notify(request_header);
 
+		case IKOT_INTR_ENTRY:
+		return intr_entry_notify(request_header);
+
 		default:
 		return FALSE;
 	}
diff --git a/kern/ipc_kobject.h b/kern/ipc_kobject.h
index 606a66a..6aefea7 100644
--- a/kern/ipc_kobject.h
+++ b/kern/ipc_kobject.h
@@ -77,9 +77,10 @@ typedef unsigned int ipc_kobject_type_t;
 #define IKOT_CLOCK		25
 #define IKOT_CLOCK_CTRL		26
 #define	IKOT_PAGER_PROXY	27
+#define	IKOT_INTR_ENTRY		28
 					/* << new entries here	*/
-#define	IKOT_UNKNOWN		28	/* magic catchall	*/
-#define	IKOT_MAX_TYPE		29	/* # of IKOT_ types	*/
+#define	IKOT_UNKNOWN		29	/* magic catchall	*/
+#define	IKOT_MAX_TYPE		30	/* # of IKOT_ types	*/
  /* Please keep ipc/ipc_object.c:ikot_print_array up to date	*/
 
 #define is_ipc_kobject(ikot)	(ikot != IKOT_NONE)