From: Tulio A M Mendes Date: Sat, 14 Feb 2026 21:07:29 +0000 (-0300) Subject: fix: ISR GS clobber, serial IRQ stuck, ring3 page fault X-Git-Url: https://projects.tadryanom.me/docs/static/git-logo.png?a=commitdiff_plain;h=5027c8fec901a8da84d64251bbfb70392de36e07;p=AdrOS.git 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 8296c37..a55ee3e 100644 --- a/include/arch/x86/arch_types.h +++ b/include/arch/x86/arch_types.h @@ -3,11 +3,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 3319dee..670e601 100644 --- a/include/arch/x86/idt.h +++ b/include/arch/x86/idt.h @@ -20,7 +20,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 bdb7173..0efb76f 100644 --- a/include/hal/uart.h +++ b/include/hal/uart.h @@ -2,6 +2,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 2cd5430..450f5a4 100644 --- a/src/arch/x86/arch_platform.c +++ b/src/arch/x86/arch_platform.c @@ -14,6 +14,7 @@ #include "heap.h" #include "hal/cpu.h" +#include "hal/uart.h" #include "hal/usermode.h" #include "kernel/cmdline.h" @@ -136,6 +137,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 51fc18d..5c1f559 100644 --- a/src/arch/x86/interrupts.S +++ b/src/arch/x86/interrupts.S @@ -9,36 +9,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 8a4e4b6..28a1547 100644 --- a/src/arch/x86/usermode.c +++ b/src/arch/x86/usermode.c @@ -100,6 +100,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 @@ -116,18 +120,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), @@ -168,7 +172,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); @@ -248,19 +255,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 cb86767..f9ca6a0 100644 --- a/src/hal/arm/uart.c +++ b/src/hal/arm/uart.c @@ -6,6 +6,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 b170005..96b55d6 100644 --- a/src/hal/mips/uart.c +++ b/src/hal/mips/uart.c @@ -11,6 +11,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 231bf1b..4ca44ac 100644 --- a/src/hal/riscv/uart.c +++ b/src/hal/riscv/uart.c @@ -9,6 +9,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 1c5b55e..4935d5e 100644 --- a/src/hal/x86/uart.c +++ b/src/hal/x86/uart.c @@ -32,6 +32,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; }