Fix the interrupt handling...

we now differntiate between an I/O interrupt (ie one that is likely
to be triggered by a device on the system using an IRQ between
0 and 15) and one that we're creating to allow us to have functions
called for events, ie a software interrupt. Behaviour is all
commented in the file int.c, but you need to make sure that you call the
correct version.

install_io_interrupt_handler now needs an IRQ value between 0 and 15 and
will treat the irq as one it needs to enable/disable

install_interrupt_handler is for software interrupts and is essentially
the function we had.

For mroe read the comments in the file.


git-svn-id: file:///srv/svn/repos/haiku/trunk/current@348 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/kernel/core/arch/x86/arch_int.c

@@ -78,9 +78,10 @@ static void _set_gate(desc_table *gate_addr, unsigned int addr, int type, int dp
 
 void arch_int_enable_io_interrupt(int irq)
 {
-	if(irq < 0x20 || irq >= 0x30) return;
+	if (irq < 0 || irq >= 0x10)
-	irq -= 0x20;
+		return;
-	// if this is a external interrupt via 8239, enable it here
+dprintf("arch_int_enable_io_interrupt: irq %d\n", irq);
+	/* if this is a external interrupt via 8239, enable it here */
 	if (irq < 8)
 		out8(in8(0x21) & ~(1 << irq), 0x21);
 	else
@@ -89,9 +90,9 @@ void arch_int_enable_io_interrupt(int irq)
 
 void arch_int_disable_io_interrupt(int irq)
 {
-	if(irq < 0x20 || irq >= 0x30) return;
+	if (irq < 0 || irq >= 0x10)
-	irq -= 0x20;
+		return;
-	// if this is a external interrupt via 8239, disable it here
+	/* if this is a external interrupt via 8239, disable it here */
 	if (irq < 8)
 		out8(in8(0x21) | (1 << irq), 0x21);
 	else

src/kernel/core/arch/x86/arch_smp.c

@@ -92,10 +92,10 @@ int arch_smp_init(kernel_args *ka)
 		vm_create_anonymous_region(vm_get_kernel_aspace_id(), "ioapic", (void *)&ioapic,
 			REGION_ADDR_EXACT_ADDRESS, PAGE_SIZE, REGION_WIRING_WIRED_ALREADY, LOCK_RW|LOCK_KERNEL);
 
-		install_io_interrupt_handler(0xfb, &i386_timer_interrupt, NULL, 0);
+		install_interrupt_handler(0xfb, &i386_timer_interrupt, NULL);
-		install_io_interrupt_handler(0xfd, &i386_ici_interrupt, NULL, 0);
+		install_interrupt_handler(0xfd, &i386_ici_interrupt, NULL);
-		install_io_interrupt_handler(0xfe, &i386_smp_error_interrupt, NULL, 0);
+		install_interrupt_handler(0xfe, &i386_smp_error_interrupt, NULL);
-		install_io_interrupt_handler(0xff, &i386_spurious_interrupt, NULL, 0);
+		install_interrupt_handler(0xff, &i386_spurious_interrupt, NULL);
 	} else {
 		num_cpus = 1;
 	}

src/kernel/core/arch/x86/arch_timer.c

@@ -72,7 +72,7 @@ int arch_init_timer(kernel_args *ka)
 {
 	dprintf("arch_init_timer: entry\n");
 	
-	install_io_interrupt_handler(0x20, &isa_timer_interrupt, NULL, 0);
+	install_io_interrupt_handler(0, &isa_timer_interrupt, NULL, 0);
 	// apic timer interrupt set up by smp code
 
 	return 0;

src/kernel/core/int.c

@@ -51,17 +51,20 @@ int_init2(kernel_args *ka)
 	return arch_int_init2(ka); 
 } 
 
-
+/* install_interrupt_handler
-/* install_io_interrupt_handler
+ * This function is used internally to install a handler on the given vector.
- * install a handler to be called when an interrupt is triggered
+ * NB this does NOT take an IRQ, but a system interrupt value.
- * for the given irq with data as the argument
+ * As this is intended for system use this function doe NOT call
- */
+ * arch_int_enable_io_interrupt() as it only works for IRQ values
-long install_io_interrupt_handler(long irq, interrupt_handler handler,
+ */ 
-                                 void* data, ulong flags) 
+long install_interrupt_handler(long vector, interrupt_handler handler,
-{ 
+                               void* data)
+{
 	struct io_handler *io = NULL; 
-	int state; 
+	int state;
 
+	dprintf("install_interrupt_handler: vector %ld\n", vector);
+	
 	/* find the chain of handlers for this irq.
 	 * NB there can be multiple handlers for the same IRQ, especially for
 	 * PCI drivers. Where we have multiple handlers we will call each in turn
@@ -74,19 +77,37 @@ long install_io_interrupt_handler(long irq, interrupt_handler handler,
 	io->data = data; 
 
 	/* Make sure our list is init'd or bad things will happen */
-	if (io_vectors[irq].handler_list.next == NULL) {
+	if (io_vectors[vector].handler_list.next == NULL) {
-		io_vectors[irq].handler_list.next = &io_vectors[irq].handler_list;
+		io_vectors[vector].handler_list.next = &io_vectors[vector].handler_list;
-		io_vectors[irq].handler_list.prev = &io_vectors[irq].handler_list;
+		io_vectors[vector].handler_list.prev = &io_vectors[vector].handler_list;
 	}
 	
 	/* Disable the interrupts, get the spinlock for this irq only
 	 * and then insert the handler */
 	state = int_disable_interrupts();
-	acquire_spinlock(&io_vectors[irq].vector_lock); 
+	acquire_spinlock(&io_vectors[vector].vector_lock); 
-	insque(io, &io_vectors[irq].handler_list);
+	insque(io, &io_vectors[vector].handler_list);
-	release_spinlock(&io_vectors[irq].vector_lock); 
+	release_spinlock(&io_vectors[vector].vector_lock); 
 	int_restore_interrupts(state); 
 
+	return 0;
+}
+
+/* install_io_interrupt_handler
+ * install a handler to be called when an interrupt is triggered
+ * for the given irq with data as the argument
+ */
+long install_io_interrupt_handler(long irq, interrupt_handler handler,
+                                 void* data, ulong flags) 
+{ 
+	long vector = irq + 0x20;
+	long rv;
+	dprintf("install_io_interrupt_handler: irq %ld\n", irq);
+	rv = install_interrupt_handler(vector, handler, data);
+	
+	if (rv != 0)
+		return rv;
+
 	/* If we were passed the bit-flag B_NO_ENABLE_COUNTER then 
 	 * we're being asked to not alter whether the interrupt is set
 	 * regardless of setting.
@@ -97,50 +118,60 @@ long install_io_interrupt_handler(long irq, interrupt_handler handler,
 	return 0; 
 }
 
+/* remove_interrupt_handler
+ * read notes for install_interrupt_handler
+ */
+long remove_interrupt_handler(long vector, interrupt_handler handler, 
+                              void* data) 
+{ 
+	struct io_handler *io = NULL; 
+
+	/* lock the structures down so it is not modified while we search */
+	int state = int_disable_interrupts(); 
+	acquire_spinlock(&io_vectors[vector].vector_lock); 
+
+	/* loop through the available handlers and try to find a match.
+	 * We go forward through the list but this means we start with the 
+	 * most recently added handlers.
+	 */
+	io = io_vectors[vector].handler_list.next; 
+	for (io = io_vectors[vector].handler_list.next;
+	     io != &io_vectors[vector].handler_list;
+	     io = io->next) { 
+		/* we have to match both function and data */ 
+		if (io->func == handler && io->data == data) 
+			break; 
+	} 
+
+	if (io) {
+		remque(io);
+		kfree(io); 
+	}
+	
+	// release our lock as we're done with the vector 
+	release_spinlock(&io_vectors[vector].vector_lock); 
+	int_restore_interrupts(state); 
+
+	return (io != NULL) ? 0 : EINVAL; 
+} 
+
 /* remove_io_interrupt_handler
  * remove an interrupt handler previously inserted
  */
 long remove_io_interrupt_handler(long irq, interrupt_handler handler, 
                                      void* data) 
 { 
-	struct io_handler *io = NULL; 
+	long vector = irq + 0x20;
-	int state; 
+	long rv = remove_interrupt_handler(vector, handler, data);
-
-	// lock the structures down so it is not modified while we search 
-	state = int_disable_interrupts(); 
-	acquire_spinlock(&io_vectors[irq].vector_lock); 
-
-	/* loop through the available handlers and try to find a match.
-	 * We go forward through the list but this means we start with the 
-	 * most recently added handlers.
-	 */
-	io = io_vectors[irq].handler_list.next; 
-	while (io != &io_vectors[irq].handler_list) { 
-		/* we have to match both function and data */ 
-		if (io->func == handler && io->data == data) 
-			break; 
-		io = io->next; 
-	} 
-
-	if (io) 
-		remque(io);
 	
-	// release our lock as we're done with the vector 
+	if (rv < 0)
-	release_spinlock(&io_vectors[irq].vector_lock); 
+		return rv;
-	int_restore_interrupts(state); 
 
-	// and disable the IRQ if nothing left 
+	/* Check if we need to disable interrupts... */
-	if (io != NULL) { 
+	if (io_vectors[vector].handler_list.next != &io_vectors[vector].handler_list) 
-		/* we still have handlers left if the next handler doesn't point back
+		arch_int_disable_io_interrupt(irq); 
-		 * to the head of the list.
-		 */
-		if (io_vectors[irq].handler_list.next != &io_vectors[irq].handler_list) 
-			arch_int_disable_io_interrupt(irq); 
-	
-		kfree(io); 
-	} 
 
-	return (io != NULL) ? 0 : EINVAL; 
+	return 0; 
 } 
 
 /* int_io_interrupt_handler
@@ -165,6 +196,18 @@ int int_io_interrupt_handler(int vector)
 		 *                        attention is required
 		 * - B_INVOKE_SCHEDULER, the interrupt has been handled, but the function wants
 		 *                       the scheduler to be invoked
+		 *
+		 * XXX - this is a change of behaviour from newos where every handler registered
+		 *       be called, even if the interrupt had been "handled" by a previous
+		 *       function.
+		 *       The logic now is that if there are no handlers then we return
+		 *       B_UNHANDLED_INTERRUPT and let the system do as it will.
+		 *       When we have the first function that claims to have "handled" the
+		 *       interrupt, by returning B_HANDLED_... or B_INVOKE_SCHEDULER we simply
+		 *       stop calling further handlers and return the value from that
+		 *       handler.
+		 *       This may not be correct but appears to be what BeOS did and seems
+		 *       right.
 		 */
 		for (io = io_vectors[vector].handler_list.next; 
 		     io != &io_vectors[vector].handler_list;