refacto(kernel: interrupts): allow generic IRQ handlers

We can now pass a generic (void *) argument to the handler when defining
it. If this parameter is omitted, we fallback to the old method of
calling the handler with the frame pointer to not break our previous
implementation.

It is now also possible to set a handler for all 255 entries of the IDT.
This means that the VTable is that much larger, as well as the number of
IRQ wrapper, but it should be good enough for now ig.

include/kernel/interrupts.h

@@ -60,10 +60,18 @@ void interrupts_init(void);
 typedef struct interrupt_frame interrupt_frame;
 
 /** Function pointer to an interrupt handler */
-typedef void (*interrupt_handler)(interrupt_frame);
+typedef u32 (*interrupt_handler)(void *);
 
-/** Dynamically set an interrupt handler */
-void interrupts_set_handler(u8, interrupt_handler);
+/** Dynamically set an interrupt handler
+ *
+ *  @param irq The IRQ number to associate the handler with
+ *  @param handler The handler function called when the interrupt occurs
+ *  @param data Data passed to the interrupt handler
+ *
+ *  @info If \c data is NULL, the kernel will pass a pointer to the interrupt
+ *        frame (\ref interrup_frame) when calling the handler instead
+ */
+void interrupts_set_handler(u8 irq, interrupt_handler, void *);
 
 /** Returns the name of an interrupt from its vector number */
 const char *interrupts_to_str(u8 nr);
@@ -76,7 +84,7 @@ const char *interrupts_to_str(u8 nr);
  * You must always use this function when defining an interrupt handler.
  */
 #define DEFINE_INTERRUPT_HANDLER(_interrupt) \
-    void INTERRUPT_HANDLER(_interrupt)(struct interrupt_frame frame)
+    u32 INTERRUPT_HANDLER(_interrupt)(void *data)
 
 /** @} */
 

kernel/arch/i686/devices/pic.c

@@ -1,5 +1,6 @@
 #include <kernel/cpu.h>
 #include <kernel/devices/timer.h>
+#include <kernel/error.h>
 #include <kernel/interrupts.h>
 #include <kernel/logger.h>
 #include <kernel/terminal.h>
@@ -56,7 +57,7 @@ void pic_reset()
     log_info("PIC", "Setting up custom IRQs handlers");
     pic_enable_irq(IRQ_KEYBOARD);
     interrupts_set_handler(PIC_MASTER_VECTOR + IRQ_KEYBOARD,
-                           INTERRUPT_HANDLER(irq_keyboard));
+                           INTERRUPT_HANDLER(irq_keyboard), NULL);
 }
 
 void pic_eoi(pic_irq irq)
@@ -86,7 +87,7 @@ void pic_disable_irq(pic_irq irq)
 
 static DEFINE_INTERRUPT_HANDLER(irq_keyboard)
 {
-    UNUSED(frame);
+    UNUSED(data);
 
     static const u8 ascii[128] = {
         0x0,  0x0,  '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '-',  '=',
@@ -104,4 +105,6 @@ static DEFINE_INTERRUPT_HANDLER(irq_keyboard)
         tty_putchar(ascii[scan_code]);
 
     pic_eoi(IRQ_KEYBOARD);
+
+    return E_SUCCESS;
 }

kernel/arch/i686/devices/timer.c

@@ -15,6 +15,7 @@
 
 #include <kernel/cpu.h>
 #include <kernel/devices/timer.h>
+#include <kernel/error.h>
 #include <kernel/interrupts.h>
 #include <kernel/logger.h>
 #include <kernel/sched.h>
@@ -62,13 +63,13 @@ void timer_start(u32 frequency)
     // Setup the timer's IRQ handler
     // It is responsible for updating our internal timer representation
     interrupts_set_handler(PIC_MASTER_VECTOR + IRQ_TIMER,
-                           INTERRUPT_HANDLER(irq_timer));
+                           INTERRUPT_HANDLER(irq_timer), NULL);
     pic_enable_irq(IRQ_TIMER);
 }
 
 static DEFINE_INTERRUPT_HANDLER(irq_timer)
 {
-    UNUSED(frame);
+    UNUSED(data);
 
     if (timer_ticks_counter == UINT64_MAX) {
         log_warn("TIMER", "The internal timer has reached its max capacity.");
@@ -89,7 +90,7 @@ static DEFINE_INTERRUPT_HANDLER(irq_timer)
     // TODO: Don't mingle IRQ and scheduling
 
     if (!scheduler_initialized)
-        return;
+        return E_INVAL;
 
     const node_t *next_wakeup = llist_head(sleeping_tasks);
     while (next_wakeup &&
@@ -102,6 +103,8 @@ static DEFINE_INTERRUPT_HANDLER(irq_timer)
 
     if (current_process->running.preempt <= timer_ticks_counter)
         schedule();
+
+    return E_SUCCESS;
 }
 
 u64 timer_gettick(void)

kernel/arch/i686/interrupts.S

@@ -45,7 +45,6 @@ interrupt_error 11, segment_not_present
 interrupt_error 12, stack_segment_fault
 interrupt_error 13, general_protection
 interrupt_error 14, page_fault
-interrupt 15, intel_reserved
 interrupt 16, fpu_error
 interrupt_error 17, alignment_check
 interrupt 18, machine_check
@@ -74,11 +73,31 @@ interrupt 47, irq_ata_secondary
     dd __%1_stub_handler
 %endmacro
 
+; Interrupt handlers for custom irqs defined through interrupt_set_handler
+
+%macro interrupt_range_custom_irq 3
+%assign i %1
+%rep %2 - %1 + 1
+%3 i
+%assign i i+1
+%endrep
+%endmacro
+
+%macro define_custom_irq 1
+    interrupt %1, irq%1
+%endmacro
+
+%macro stub_custom_irq 1
+    stub irq%1
+%endmacro
+
+interrupt_range_custom_irq 47, 255, define_custom_irq
+
 ; addressable interrupt handler stubs tables
 global interrupt_handler_stubs
-global irq_handler_stubs
 
 interrupt_handler_stubs:
+  ; x86 protected mode exceptions
   stub division_error
   stub debug
   stub nmi
@@ -94,15 +113,18 @@ interrupt_handler_stubs:
   stub stack_segment_fault
   stub general_protection
   stub page_fault
-  stub intel_reserved
+  dd $0 ; Intel reserved: do not use
   stub fpu_error
   stub alignment_check
   stub machine_check
   stub smid_fp_exception
   stub virtualization
   stub control_protection
-
-irq_handler_stubs:
+  ; 22-31: Intel reserved: do not use
+%rep 9
+  dd $0
+%endrep
+  ; IRQs
   stub irq_timer
   stub irq_keyboard
   stub irq_com2
@@ -114,3 +136,5 @@ irq_handler_stubs:
   stub irq_fpu
   stub irq_ata_primary
   stub irq_ata_secondary
+  ; Custom IRQs
+  interrupt_range_custom_irq 47, 255, stub_custom_irq

kernel/arch/i686/interrupts.c

@@ -13,16 +13,20 @@
 static volatile idt_descriptor idt[IDT_LENGTH];
 
 // Global addressable interrupt handler stub tables
-extern interrupt_handler interrupt_handler_stubs[];
-extern interrupt_handler irq_handler_stubs[PIC_IRQ_COUNT];
+extern interrupt_handler interrupt_handler_stubs[IDT_LENGTH];
+
+typedef struct {
+    interrupt_handler handler; // The interrupt handler
+    void *data;                // Data passed as an argument to the handler
+} interrupt_handler_callback;
 
 /*
  * Custom ISRs, defined at runtime and called by __stub_interrupt_handler
  * using the interrupt number (if set).
  *
  * These are set using \c interrupts_set_handler
  */
-static interrupt_handler custom_interrupt_handlers[IDT_LENGTH];
+static interrupt_handler_callback custom_interrupt_handlers[IDT_LENGTH];
 
 static const char *interrupt_names[] = {
     // Protected mode Interrupts and Exceptions (Table 6-1, Intel vol.3)
@@ -82,19 +86,20 @@ static const char *interrupt_names[] = {
 
 const char *interrupts_to_str(u8 nr)
 {
-    static const char *unknown = "Unknown Interrupt";
+    static const char *unknown = "Unnamed Interrupt";
 
     if (nr < (sizeof interrupt_names / sizeof interrupt_names[0]))
         return interrupt_names[nr];
 
     return unknown;
 }
 
-void interrupts_set_handler(u8 nr, interrupt_handler handler)
+void interrupts_set_handler(u8 nr, interrupt_handler handler, void *data)
 {
     log_info("IDT", "Setting custom handler for '%s' (" LOG_FMT_8 ")",
              interrupts_to_str(nr), nr);
-    custom_interrupt_handlers[nr] = handler;
+    custom_interrupt_handlers[nr].handler = handler;
+    custom_interrupt_handlers[nr].data = data;
 }
 
 static ALWAYS_INLINE idt_descriptor new_idt_entry(idt_gate_type type,
@@ -119,8 +124,8 @@ static ALWAYS_INLINE idt_descriptor new_idt_entry(idt_gate_type type,
     };
 }
 
-static inline void interrupts_set(u16 nr, idt_gate_type type,
-                                  interrupt_handler handler)
+static inline void interrupts_set_idt(u16 nr, idt_gate_type type,
+                                      interrupt_handler handler)
 {
     if (nr >= IDT_LENGTH) {
         log_err("IDT", "interrupts_set: invalid index: " LOG_FMT_8, nr);
@@ -142,20 +147,15 @@ void interrupts_init(void)
 
     // Empty descriptor slots in the IDT should have the present flag set to 0.
     // Fill the whole IDT with null descriptors
-    memset((void *)idt, 0, IDT_SIZE); // NOLINT
+    memset((void *)idt, 0, IDT_SIZE);
 
     // Empty the list of custom ISRs
     memset(custom_interrupt_handlers, 0, sizeof(custom_interrupt_handlers));
 
-    // Setup all known interrupts
     log_info("IDT", "Setting up interrupt handler stubs");
-    for (u8 i = 0; i <= 21; ++i)
-        interrupts_set(i, INTERRUPT_GATE_32B, interrupt_handler_stubs[i]);
-
-    log_info("IDT", "Setting up IRQ handler stubs");
-    for (pic_irq irq = IRQ_TIMER; irq <= IRQ_ATA_SECONDARY; ++irq)
-        interrupts_set(PIC_MASTER_VECTOR + irq, INTERRUPT_GATE_32B,
-                       irq_handler_stubs[irq]);
+    for (int i = 0; i <= IDT_LENGTH; ++i) {
+        interrupts_set_idt(i, INTERRUPT_GATE_32B, interrupt_handler_stubs[i]);
+    }
 
     log_dbg("IDT", "Finished setting up the IDT");
 }
@@ -181,12 +181,14 @@ void idt_log(void)
     }
 }
 
-DEFINE_INTERRUPT_HANDLER(default_interrupt)
+void default_interrupt_handler(interrupt_frame frame)
 {
+    interrupt_handler_callback *handler = &custom_interrupt_handlers[frame.nr];
+
     // Call the custom handler, defined inside custom_interrupt_handlers,
     // if it exists. Else, we consider this interrupt as 'unsupported'.
 
-    if (custom_interrupt_handlers[frame.nr] == 0) {
+    if (handler->handler == NULL) {
         log_err("interrupt", "Unsupported interrupt: %s (" LOG_FMT_32 ")",
                 interrupts_to_str(frame.nr), frame.nr);
         log_dbg("interrupt", "ERROR=" LOG_FMT_32, frame.error);
@@ -198,5 +200,7 @@ DEFINE_INTERRUPT_HANDLER(default_interrupt)
         return;
     }
 
-    custom_interrupt_handlers[frame.nr](frame);
+    // Pass the frame as argument if no data was given
+    // This is done to not have to differiente CPU exceptions from custom IRQs
+    handler->handler(handler->data ? handler->data : &frame);
 }

kernel/memory/vmm.c

@@ -165,7 +165,7 @@ bool vmm_init(vmm_t *vmm, vaddr_t start, vaddr_t end)
     // TODO: Refactor such calls (hooks, initcalls, there are better ways to do)
     //       Even more so for this one since we'll be updating the interrupt
     //       handler each time we create a new process!
-    interrupts_set_handler(PAGE_FAULT, INTERRUPT_HANDLER(page_fault));
+    interrupts_set_handler(PAGE_FAULT, INTERRUPT_HANDLER(page_fault), NULL);
 
     log_info("VMM",
              "Initialized VMM { start=" LOG_FMT_32 ", end=" LOG_FMT_32 " }",
@@ -508,11 +508,12 @@ static DEFINE_INTERRUPT_HANDLER(page_fault)
 {
     // The CR2 register holds the virtual address which caused the Page Fault
     vaddr_t faulty_address = read_cr2();
+    interrupt_frame *frame = data;
 
     vmm_t *vmm =
         IS_KERNEL_ADDRESS(faulty_address) ? &kernel_vmm : current_process->vmm;
     const vma_t *address_area = vmm_find(vmm, faulty_address);
-    page_fault_error error = *(page_fault_error *)&frame.error;
+    page_fault_error error = *(page_fault_error *)&frame->error;
 
     // Lazily allocate pageframes
     if (!error.present && address_area != NULL && address_area->allocated) {
@@ -522,7 +523,7 @@ static DEFINE_INTERRUPT_HANDLER(page_fault)
             if (address_area->flags & MAP_CLEAR)
                 memset((void *)address_area->start + off, 0, PAGE_SIZE);
         }
-        return;
+        return E_SUCCESS;
     }
 
     PANIC("PAGE FAULT at " LOG_FMT_32 ": %s access on a %s page %s",