Hardware interrupts code path cleanup

- the PIC master and slave irq handlers don't pass the irq hook pointer but just
  the irq number. It gives a little bit more information to the C handler as the
  irq number is not lost

- the irq code path is more achitecture independent. i386 hw interrupts are
  called irq and whereever the code is arch independent enough hw_intr_
  functions are called to mask/unmask interrupts

- the legacy PIC is not the only possible interrupt controller in the x86 world,
  therefore the intr_(un)mask functions were renamed to signal their
  functionality explicitly. APIC will add their own.

- masking and unmasking PIC interrupt lines is removed from assembler and all
  the functionality is rewriten in C and moved to i8259.c

- interrupt handlers have to unmask the interrupt line if all irq handlers are
  done. Assembler does not do it anymore

kernel/arch/i386/hw_intr.h

@@ -0,0 +1,15 @@
+#ifndef __HW_INTR_X86_H__
+#define __HW_INTR_X86_H__
+
+#include "../..//kernel.h"
+
+/* legacy PIC */
+
+_PROTOTYPE(int irq_8259_unmask,(int irq));
+_PROTOTYPE(int irq_8259_mask,(int irq));
+_PROTOTYPE(void irq_handle,(int irq));
+
+#define hw_intr_mask(irq)	irq_8259_mask(irq)
+#define hw_intr_unmask(irq)	irq_8259_unmask(irq)
+
+#endif /* __HW_INTR_X86_H__ */

kernel/arch/i386/i8259.c

@@ -77,3 +77,16 @@ PUBLIC int intr_disabled(void)
 		return 1;
 	return 0;
 }
+
+PUBLIC void irq_8259_unmask(int irq)
+{
+	unsigned ctl_mask = irq < 8 ? INT_CTLMASK : INT2_CTLMASK;
+
+	outb(ctl_mask, inb(ctl_mask) & ~(1 << (irq & 0x7)));
+}
+
+PUBLIC void irq_8259_mask(int irq)
+{
+	unsigned ctl_mask = irq < 8 ? INT_CTLMASK : INT2_CTLMASK;
+	outb(ctl_mask, inb(ctl_mask) | (1 << (irq & 0x7)));
+}

kernel/arch/i386/klib386.S

@@ -24,8 +24,6 @@
 .globl	phys_insb	/* likewise byte by byte */
 .globl	phys_outsw	/* transfer data from memory to (disk controller) port */
 .globl	phys_outsb	/* likewise byte by byte */
-.globl	intr_unmask	/* enable an irq at the 8259 controller */
-.globl	intr_mask	/* disable an irq */
 .globl	phys_copy	/* copy data from anywhere to anywhere in memory */
 .globl	phys_copy_fault	/* phys_copy pagefault */
 .globl	phys_memset	/* write pattern anywhere in memory */
@@ -294,90 +292,6 @@ phys_outsb:
 	ret
 
 
-/*==========================================================================*/
-/*				intr_unmask				    */
-/*==========================================================================*/
-/*
- * PUBLIC void intr_unmask(irq_hook_t *hook) 
- * Enable an interrupt request line by clearing an 8259 bit. 
- * Equivalent C code for hook->irq < 8: 
- *   if ((irq_actids[hook->irq] &= ~hook->id) == 0) 
- *	outb(INT_CTLMASK, inb(INT_CTLMASK) & ~(1 << irq)); 
- */
-
-.balign	16
-intr_unmask:
-	push	%ebp
-	mov	%esp, %ebp
-	pushf
-	cli
-	mov	8(%ebp), %eax	/* hook */
-	mov	8(%eax), %ecx	/* irq */
-	mov	12(%eax), %eax	/* id bit */
-	not	%eax
-	and	%eax, irq_actids(,%ecx)	/* clear this id bit */
-	jne	en_done	/* still masked by other handlers? */
-	movb	$~1, %ah
-	rolb	%cl, %ah	/* ah = ~(1 << (irq % 8)) */
-	mov	$INT_CTLMASK, %edx	/* enable irq < 8 at the master 8259 */
-	cmpb	$8, %cl
-	jb	0f
-	mov	$INT2_CTLMASK, %edx	/* enable irq >= 8 at the slave 8259 */
-0:
-	inb	%dx
-	andb	%ah, %al
-	outb	%dx	/* clear bit at the 8259 */
-en_done:
-	popf
-	leave
-	ret
-
-
-/*==========================================================================*/
-/*				intr_mask				    */
-/*==========================================================================*/
-/*
- * PUBLIC int intr_mask(irq_hook_t *hook) 
- * Disable an interrupt request line by setting an 8259 bit. 
- * Equivalent C code for irq < 8: 
- *   irq_actids[hook->irq] |= hook->id; 
- *   outb(INT_CTLMASK, inb(INT_CTLMASK) | (1 << irq)); 
- * Returns true iff the interrupt was not already disabled. 
- */
-
-.balign	16
-intr_mask:
-	push	%ebp
-	mov	%esp, %ebp
-	pushf
-	cli
-	mov	8(%ebp), %eax	/* hook */
-	mov	8(%eax), %ecx	/* irq */
-	mov	12(%eax), %eax	/* id bit */
-	or	%eax, irq_actids(,%ecx)	/* set this id bit */
-	movb	$1, %ah
-	rolb	%cl, %ah	/* ah = (1 << (irq % 8)) */
-	mov	$INT_CTLMASK, %edx	/* disable irq < 8 at the master 8259 */
-	cmpb	$8, %cl
-	jb	0f
-	mov	$INT2_CTLMASK, %edx	/* disable irq >= 8 at the slave 8259 */
-0:
-	inb	%dx
-	testb	%ah, %al
-	jne	dis_already	/* already disabled? */
-	orb	%ah, %al
-	outb	%dx	/* set bit at the 8259 */
-	mov	$1, %eax	/* disabled by this function */
-	popf
-	leave
-	ret
-dis_already:
-	xor	%eax, %eax	/* already disabled */
-	popf
-	leave
-	ret
-
-
 /*===========================================================================*/
 /*				phys_copy				     */
 /*===========================================================================*/

kernel/arch/i386/mpx386.S

@@ -248,15 +248,10 @@ csinit:
 /* Note this is a macro, it just looks like a subroutine. */
 #define hwint_master(irq)	\
 	call	save			/* save interrupted process state */;\
-	push	(irq_handlers+4*irq)	/* irq_handlers[irq]		  */;\
+	push	$irq							    ;\
-	call	intr_handle		/* intr_handle(irq_handlers[irq]) */;\
+	call	irq_handle		/* irq_handle(irq) 		  */;\
 	pop	%ecx							    ;\
-	cmp	$0, (irq_actids+4*irq)	/* interrupt still active?	  */;\
+	movb	$END_OF_INT, %al						    ;\
-	jz	0f							    ;\
-	inb	$INT_CTLMASK		/* get current mask */		    ;\
-	orb	$(1<<irq), %al		/* mask irq */			    ;\
-	outb	$INT_CTLMASK		/* disable the irq		  */;\
-0:	movb	$END_OF_INT, %al						    ;\
 	outb	$INT_CTL			/* reenable master 8259		  */;\
 	ret				/* restart (another) process      */
 
@@ -307,15 +302,10 @@ hwint07:
 /* Note this is a macro, it just looks like a subroutine. */
 #define hwint_slave(irq)	\
 	call	save			/* save interrupted process state */;\
-	push	(irq_handlers+4*irq)	/* irq_handlers[irq]		  */;\
+	push	$irq							    ;\
-	call	intr_handle		/* intr_handle(irq_handlers[irq]) */;\
+	call	irq_handle		/* irq_handle(irq) 		  */;\
 	pop	%ecx							    ;\
-	cmp	$0, (irq_actids+4*irq)	/* interrupt still active?	  */;\
+	movb	$END_OF_INT, %al						    ;\
-	jz	0f							    ;\
-	inb	$INT2_CTLMASK						    ;\
-	orb	$(1<<(irq-8)), %al					    ;\
-	outb	$INT2_CTLMASK		/* disable the irq		  */;\
-0:	movb	$END_OF_INT, %al						    ;\
 	outb	$INT_CTL			/* reenable master 8259		  */;\
 	outb	$INT2_CTL		/* reenable slave 8259		  */;\
 	ret				/* restart (another) process      */

kernel/interrupt.c

@@ -6,7 +6,7 @@
  *  
  *   put_irq_handler: register an interrupt handler.
  *   rm_irq_handler:  deregister an interrupt handler.
- *   intr_handle:     handle a hardware interrupt.
+ *   irq_handle:     handle a hardware interrupt.
  *                    called by the system dependent part when an
  *                    external interrupt occures.                     
  *   enable_irq:      enable hook for IRQ.
@@ -18,6 +18,8 @@
 #include <minix/com.h>
 #include <archconst.h>
 
+#include "arch/i386/hw_intr.h"
+
 /* number of lists of IRQ hooks, one list per supported line. */
 PUBLIC irq_hook_t* irq_handlers[NR_IRQ_VECTORS] = {0};
 /*===========================================================================*
@@ -29,7 +31,7 @@ PUBLIC void put_irq_handler( irq_hook_t* hook, int irq, irq_handler_t handler)
   int id;
   irq_hook_t **line;
   unsigned long bitmap;
-  
+
   if( irq < 0 || irq >= NR_IRQ_VECTORS )
 	minix_panic("invalid call to put_irq_handler", irq);
 
@@ -60,7 +62,9 @@ PUBLIC void put_irq_handler( irq_hook_t* hook, int irq, irq_handler_t handler)
    *
    * Internal this activates the line or source of the given interrupt.
    */
-  intr_unmask(hook);  
+  if((irq_actids[hook->irq] &= ~hook->id) == 0) {
+	  hw_intr_unmask(hook->irq);
+  }
 }
 
 /*===========================================================================*
@@ -76,7 +80,8 @@ PUBLIC void rm_irq_handler( irq_hook_t* hook ) {
 	minix_panic("invalid call to rm_irq_handler", irq);
 
   /* disable the irq.  */
-  intr_mask(hook);
+  irq_actids[hook->irq] |= hook->id;
+  hw_intr_mask(hook->irq);
     
   /* remove the hook.  */
   line = &irq_handlers[irq];
@@ -96,21 +101,27 @@ PUBLIC void rm_irq_handler( irq_hook_t* hook ) {
 }
 
 /*===========================================================================*
- *				intr_handle				     *
+ *				irq_handle				     *
  *===========================================================================*/
-PUBLIC void intr_handle(irq_hook_t *hook)
+/*
-{
+ * The function first disables interrupt is need be and restores the state at
-/* Call the interrupt handlers for an interrupt with the given hook list.
+ * the end. Before returning, it unmasks the IRQ if and only if all active ID
- * The assembly part of the handler has already masked the IRQ, reenabled the
+ * bits are cleared, and restart a process.
- * controller(s) and enabled interrupts.
  */
+PUBLIC void irq_handle(int irq)
+{
+  irq_hook_t * hook;
+
+  /* here we need not to get this IRQ until all the handlers had a say */
+  hw_intr_mask(irq);
+  hook = irq_handlers[irq];
 
   /* Call list of handlers for an IRQ. */
   while( hook != NULL ) {
     /* For each handler in the list, mark it active by setting its ID bit,
      * call the function, and unmark it if the function returns true.
      */
-    irq_actids[hook->irq] |= hook->id;
+    irq_actids[irq] |= hook->id;
     
     /* Call the hooked function. */
     if( (*hook->handler)(hook) )
@@ -119,10 +130,10 @@ PUBLIC void intr_handle(irq_hook_t *hook)
     /* Next hooked function. */
     hook = hook->next;
   }
-  
+
-  /* The assembly code will now disable interrupts, unmask the IRQ if and only
+  /* reenable the IRQ only if there is no active handler */
-   * if all active ID bits are cleared, and restart a process.
+  if (irq_actids[irq] == 0)
-   */
+	  hw_intr_unmask(irq);
 }
 
 /* Enable/Disable a interrupt line.  */
@@ -130,8 +141,7 @@ PUBLIC void enable_irq(hook)
 irq_hook_t* hook;
 {
   if((irq_actids[hook->irq] &= ~hook->id) == 0) {
-    intr_unmask(hook);
+    hw_intr_unmask(hook->irq);
-    return; 
   }
 }
 
@@ -142,7 +152,7 @@ irq_hook_t* hook;
   if(irq_actids[hook->irq] & hook->id)  /* already disabled */
     return 0;
   irq_actids[hook->irq] |= hook->id;
-  intr_mask(hook);   
+  hw_intr_mask(hook->irq);
   return TRUE;
 }
 

kernel/proto.h

@@ -72,7 +72,6 @@ _PROTOTYPE( int newmap, (struct proc *rp, struct mem_map *map_ptr)	);
 _PROTOTYPE( void vtimer_check, (struct proc *rp)			);
 
 /* interrupt.c */
-_PROTOTYPE( void intr_handle,     (irq_hook_t *hook)		          );
 _PROTOTYPE( void put_irq_handler, (irq_hook_t *hook, int irq,
 						 irq_handler_t handler)  );
 _PROTOTYPE( void rm_irq_handler, (irq_hook_t *hook)                      );
@@ -138,8 +137,6 @@ _PROTOTYPE( clock_t read_clock, (void)					);
 _PROTOTYPE( void clock_stop, (void)    					);
 _PROTOTYPE( int intr_init, (int)					);
 _PROTOTYPE( int intr_disabled, (void)					);
-_PROTOTYPE( int intr_unmask, (irq_hook_t* hook)                     );
-_PROTOTYPE( int intr_mask, (irq_hook_t* hook)                    );
 _PROTOTYPE( void idle_task, (void)                                     );
 _PROTOTYPE( void arch_init, (void)                                     );
 _PROTOTYPE( void ser_putc, (char)						);