fix: ISR GS clobber, serial IRQ stuck, ring3 page fault

1. **ISR GS clobber (III) — FIXED**
   - interrupts.S: save/restore GS separately instead of overwriting
     with 0x10. DS/ES/FS still set to kernel data, but GS now
     preserves the per-CPU selector across interrupt entry/exit.
   - struct registers: new 'gs' field at offset 0.
   - ARCH_REGS_SIZE: 64 → 68.
   - x86_enter_usermode_regs: updated all hardcoded register offsets
     (+4 for the new GS field).

2. **Serial keyboard blocking (II) — FIXED**
   - Root cause: hal_uart_init() runs early (under PIC), enabling
     UART RX interrupts. Later, IOAPIC routes IRQ 4 as edge-triggered.
     If any character arrived between PIC-era init and IOAPIC setup,
     the UART IRQ line stays asserted — the IOAPIC never sees a
     rising edge, permanently blocking all future serial input.
   - Fix: hal_uart_drain_rx() clears pending UART FIFO + IIR + MSR
     immediately after ioapic_route_irq(4, ...) to de-assert the
     IRQ line and allow future edges.

3. **Ring3 page fault at 0xae1000 (V) — FIXED**
   - The ring3 code emitter wrote to code_phys as a virtual address,
     relying on an identity mapping that doesn't exist for all
     physical addresses. Now uses P2V (phys + 0xC0000000) to access
     physical pages via the kernel's higher-half mapping.

Tests: 20/20 smoke (8s), 16/16 battery, cppcheck clean

diff --git a/include/arch/x86/arch_types.h b/include/arch/x86/arch_types.h
index edcb6cfc88b034d959bb231e66cd1108051984a5..7a03c4fc3ab73f30ef36db2d2a4e0cdbc5049e99 100644 (file)
--- a/include/arch/x86/arch_types.h
+++ b/include/arch/x86/arch_types.h
@@ -12,11 +12,11 @@
 
 /*
  * Size in bytes of the saved user-mode register frame (struct registers).
- * x86: ds(4) + pusha(32) + int_no+err(8) + iret_frame(20) = 64
+ * x86: gs(4) + ds(4) + pusha(32) + int_no+err(8) + iret_frame(20) = 68
  *
  * Used by struct process to hold an opaque register snapshot without
  * pulling in the full x86 IDT / register definitions.
  */
-#define ARCH_REGS_SIZE 64
+#define ARCH_REGS_SIZE 68
 
 #endif /* ARCH_X86_TYPES_H */

diff --git a/include/arch/x86/idt.h b/include/arch/x86/idt.h
index 6477d17ac3d1f7980c41984cdca607f1f6a741c5..8eeae0e5939e9143cba4c48e93bdcde1fd753b50 100644 (file)
--- a/include/arch/x86/idt.h
+++ b/include/arch/x86/idt.h
@@ -29,7 +29,8 @@ struct idt_ptr {
 
 /* Registers saved by our assembly ISR stub */
 struct registers {
-    uint32_t ds;                                     // Data segment selector
+    uint32_t gs;                                     // Per-CPU GS selector (pushed second)
+    uint32_t ds;                                     // Data segment selector (pushed first)
     uint32_t edi, esi, ebp, esp, ebx, edx, ecx, eax; // Pushed by pusha
     uint32_t int_no, err_code;                       // Interrupt number and error code
     uint32_t eip, cs, eflags, useresp, ss;           // Pushed by the processor automatically

diff --git a/include/hal/uart.h b/include/hal/uart.h
index 857fe987e9e1792084ba894c914de827a996bce4..cbbff32f30e7e349989ecd98f76752eef4299bc9 100644 (file)
--- a/include/hal/uart.h
+++ b/include/hal/uart.h
@@ -11,6 +11,7 @@
 #define HAL_UART_H
 
 void hal_uart_init(void);
+void hal_uart_drain_rx(void);
 void hal_uart_putc(char c);
 int  hal_uart_try_getc(void);
 void hal_uart_set_rx_callback(void (*cb)(char));

diff --git a/src/arch/x86/arch_platform.c b/src/arch/x86/arch_platform.c
index 20af75ba98a2e32c425d67a73d8793a49df86df7..a3598a358d8a6e60cdc0b67848defd999c8d0143 100644 (file)
--- a/src/arch/x86/arch_platform.c
+++ b/src/arch/x86/arch_platform.c
@@ -23,6 +23,7 @@
 #include "heap.h"
 
 #include "hal/cpu.h"
+#include "hal/uart.h"
 #include "hal/usermode.h"
 #include "kernel/cmdline.h"
 
@@ -145,6 +146,7 @@ int arch_platform_setup(const struct boot_info* bi) {
             ioapic_route_irq(0,  32, (uint8_t)bsp_id);
             ioapic_route_irq(1,  33, (uint8_t)bsp_id);
             ioapic_route_irq(4,  36, (uint8_t)bsp_id); /* COM1 serial */
+            hal_uart_drain_rx(); /* Clear stale UART FIFO so edge-triggered IRQ4 isn't stuck high */
             ioapic_route_irq_level(11, 43, (uint8_t)bsp_id); /* E1000 NIC (PCI: level-triggered, active-low) */
             ioapic_route_irq(14, 46, (uint8_t)bsp_id); /* ATA primary */
             ioapic_route_irq(15, 47, (uint8_t)bsp_id); /* ATA secondary */

diff --git a/src/arch/x86/interrupts.S b/src/arch/x86/interrupts.S
index 51c29bb529554f946de113cf1b7ffd27ccb81b60..5dee3a26204f2e2f24a61f17222ac721347059b5 100644 (file)
--- a/src/arch/x86/interrupts.S
+++ b/src/arch/x86/interrupts.S
@@ -18,36 +18,38 @@
 isr_common_stub:
     /* Save all registers */
     pusha
-    
-    /* Save Data Segment */
+
+    /* Save segment selectors (DS and GS saved separately) */
     mov %ds, %ax
     push %eax
-    
-    /* Load Kernel Data Segment */
+    push %gs
+
+    /* Load Kernel Data Segment for DS/ES/FS.
+     * GS is NOT touched — it holds the per-CPU selector set by
+     * percpu_setup_gs() and must survive ISR entry/exit. */
     mov $0x10, %ax
     mov %ax, %ds
     mov %ax, %es
     mov %ax, %fs
-    mov %ax, %gs
-    
+
     /* Call C handler */
     push %esp   /* Pass pointer to stack structure */
     call isr_handler
     add $4, %esp
-    
-    /* Restore Data Segment */
+
+    /* Restore GS (per-CPU) then DS/ES/FS */
+    pop %gs
     pop %eax
     mov %ax, %ds
     mov %ax, %es
     mov %ax, %fs
-    mov %ax, %gs
-    
+
     /* Restore registers */
     popa
-    
+
     /* Clean up error code and ISR number */
     add $8, %esp
-    
+
     /* Return from interrupt */
     iret
 

diff --git a/src/arch/x86/usermode.c b/src/arch/x86/usermode.c
index b301e3adcdda57cd0928867dddb6897f4ef8a28f..585d53f299d0b23d5d90e42f1ff52b51b770531f 100644 (file)
--- a/src/arch/x86/usermode.c
+++ b/src/arch/x86/usermode.c
@@ -109,6 +109,10 @@ __attribute__((noreturn)) void x86_enter_usermode_regs(const struct registers* r
     }
 
     // Layout follows include/arch/x86/idt.h struct registers.
+    // struct registers { gs(0), ds(4), edi(8), esi(12), ebp(16),
+    //   esp(20), ebx(24), edx(28), ecx(32), eax(36),
+    //   int_no(40), err_code(44), eip(48), cs(52), eflags(56),
+    //   useresp(60), ss(64) };
     const uint32_t eflags = (regs->eflags | 0x200U);
 
     /* Use ESI as scratch to hold regs pointer, since we'll overwrite
@@ -125,18 +129,18 @@ __attribute__((noreturn)) void x86_enter_usermode_regs(const struct registers* r
         "mov %%ax, %%gs\n"
 
         "pushl $0x23\n"           /* ss */
-        "pushl 56(%%esi)\n"       /* useresp */
+        "pushl 60(%%esi)\n"       /* useresp */
         "pushl %[efl]\n"          /* eflags */
         "pushl $0x1B\n"           /* cs */
-        "pushl 44(%%esi)\n"       /* eip */
-
-        "mov 4(%%esi), %%edi\n"   /* edi */
-        "mov 12(%%esi), %%ebp\n"  /* ebp */
-        "mov 20(%%esi), %%ebx\n"  /* ebx */
-        "mov 24(%%esi), %%edx\n"  /* edx */
-        "mov 28(%%esi), %%ecx\n"  /* ecx */
-        "mov 32(%%esi), %%eax\n"  /* eax */
-        "mov 8(%%esi), %%esi\n"   /* esi (last — self-overwrite) */
+        "pushl 48(%%esi)\n"       /* eip */
+
+        "mov  8(%%esi), %%edi\n"  /* edi */
+        "mov 16(%%esi), %%ebp\n"  /* ebp */
+        "mov 24(%%esi), %%ebx\n"  /* ebx */
+        "mov 28(%%esi), %%edx\n"  /* edx */
+        "mov 32(%%esi), %%ecx\n"  /* ecx */
+        "mov 36(%%esi), %%eax\n"  /* eax */
+        "mov 12(%%esi), %%esi\n"  /* esi (last — self-overwrite) */
         "iret\n"
         :
         : [r] "r"(regs),
@@ -177,7 +181,10 @@ void x86_usermode_test_start(void) {
     const uint32_t t3_fail_len = 8;
     const uint32_t msg_len = 18;
 
-    struct emitter e = { .buf = (uint8_t*)(uintptr_t)code_phys, .pos = 0 };
+    /* Access the physical page via the kernel higher-half mapping (P2V)
+     * instead of relying on an identity mapping that may not exist. */
+    const uintptr_t code_kva = (uintptr_t)code_phys + 0xC0000000U;
+    struct emitter e = { .buf = (uint8_t*)code_kva, .pos = 0 };
 
     /* T1: write(valid buf) -> t1_ok_len */
     emit_mov_eax_imm(&e, SYSCALL_WRITE_NO);
@@ -257,19 +264,19 @@ void x86_usermode_test_start(void) {
     patch_rel8(e.buf, t3_fail_jne.at, t3_fail_pos);
     patch_rel8(e.buf, t3_to_exit.at, exit_pos);
 
-    memcpy((void*)((uintptr_t)code_phys + 0x200), "T1 OK\n", t1_ok_len);
-    memcpy((void*)((uintptr_t)code_phys + 0x210), "T1 FAIL\n", t1_fail_len);
-    memcpy((void*)((uintptr_t)code_phys + 0x220), "T2 OK\n", t2_ok_len);
-    memcpy((void*)((uintptr_t)code_phys + 0x230), "T2 FAIL\n", t2_fail_len);
-    memcpy((void*)((uintptr_t)code_phys + 0x240), "T3 OK\n", t3_ok_len);
-    memcpy((void*)((uintptr_t)code_phys + 0x250), "T3 FAIL\n", t3_fail_len);
-    memcpy((void*)((uintptr_t)code_phys + 0x300), "Hello from ring3!\n", msg_len);
+    memcpy((void*)(code_kva + 0x200), "T1 OK\n", t1_ok_len);
+    memcpy((void*)(code_kva + 0x210), "T1 FAIL\n", t1_fail_len);
+    memcpy((void*)(code_kva + 0x220), "T2 OK\n", t2_ok_len);
+    memcpy((void*)(code_kva + 0x230), "T2 FAIL\n", t2_fail_len);
+    memcpy((void*)(code_kva + 0x240), "T3 OK\n", t3_ok_len);
+    memcpy((void*)(code_kva + 0x250), "T3 FAIL\n", t3_fail_len);
+    memcpy((void*)(code_kva + 0x300), "Hello from ring3!\n", msg_len);
 
     /* Create a private address space so the ring3 user pages do NOT
      * pollute kernel_as (which is shared by all kernel threads).
-     * Code/data was emitted above via the identity mapping still
-     * active in kernel_as; now we switch to the new AS and map the
-     * physical pages at their user virtual addresses. */
+     * Code/data was emitted above via P2V (kernel higher-half mapping);
+     * now we switch to the new AS and map the physical pages at their
+     * user virtual addresses. */
     uintptr_t ring3_as = vmm_as_create_kernel_clone();
     if (!ring3_as) {
         kprintf("[USER] Failed to create ring3 address space.\n");

diff --git a/src/hal/arm/uart.c b/src/hal/arm/uart.c
index 2859502870f22c1f138b02810eac824fd861c24d..f16026b53bc28c6481d0fc0b9d38e8383adf9c8b 100644 (file)
--- a/src/hal/arm/uart.c
+++ b/src/hal/arm/uart.c
@@ -15,6 +15,9 @@
 void hal_uart_init(void) {
 }
 
+void hal_uart_drain_rx(void) {
+}
+
 void hal_uart_putc(char c) {
     volatile uint32_t* uart = (volatile uint32_t*)UART_BASE;
     while (uart[6] & (1 << 5)) { }

diff --git a/src/hal/mips/uart.c b/src/hal/mips/uart.c
index a83417e9adce478f9c34075f4a451153e9b88908..cda485cc6e65cb2561b87e494dfd45ace0ccdefc 100644 (file)
--- a/src/hal/mips/uart.c
+++ b/src/hal/mips/uart.c
@@ -20,6 +20,11 @@ void hal_uart_init(void) {
     /* Minimal init: assume firmware/QEMU defaults are usable */
 }
 
+void hal_uart_drain_rx(void) {
+    while (mmio_read8(UART_BASE + 5) & 0x01)
+        (void)mmio_read8(UART_BASE);
+}
+
 void hal_uart_putc(char c) {
     while ((mmio_read8(UART_BASE + 5) & 0x20) == 0) { }
     mmio_write8(UART_BASE, (uint8_t)c);

diff --git a/src/hal/riscv/uart.c b/src/hal/riscv/uart.c
index 1d1a650f9198ee140373f9126575ee8e89a58ab5..3b513263e3dde19f2790d1d5aa551d0e9a003c10 100644 (file)
--- a/src/hal/riscv/uart.c
+++ b/src/hal/riscv/uart.c
@@ -18,6 +18,11 @@ void hal_uart_init(void) {
     mmio_write8(UART_BASE + 1, 0x01);
 }
 
+void hal_uart_drain_rx(void) {
+    while (mmio_read8(UART_BASE + 5) & 0x01)
+        (void)mmio_read8(UART_BASE);
+}
+
 void hal_uart_putc(char c) {
     while ((mmio_read8(UART_BASE + 5) & 0x20) == 0) { }
     mmio_write8(UART_BASE, (uint8_t)c);

diff --git a/src/hal/x86/uart.c b/src/hal/x86/uart.c
index 5e80fcf8a005ca0d8045b47e130333a9288fffc7..e9bcd934519d024569d9a2cf6e8f6fa12c8339cd 100644 (file)
--- a/src/hal/x86/uart.c
+++ b/src/hal/x86/uart.c
@@ -41,6 +41,16 @@ void hal_uart_init(void) {
     outb(UART_BASE + 1, 0x01);
 }
 
+void hal_uart_drain_rx(void) {
+    /* Drain any pending characters from the UART FIFO.
+     * This de-asserts the IRQ line so that the next character
+     * produces a clean rising edge for the IOAPIC (edge-triggered). */
+    (void)inb(UART_BASE + 2);          /* Read IIR to ack any pending */
+    while (inb(UART_BASE + 5) & 0x01)  /* Drain RX FIFO */
+        (void)inb(UART_BASE);
+    (void)inb(UART_BASE + 6);          /* Read MSR to clear delta bits */
+}
+
 void hal_uart_set_rx_callback(void (*cb)(char)) {
     uart_rx_cb = cb;
 }