From: Tulio A M Mendes Date: Fri, 13 Feb 2026 06:51:01 +0000 (-0300) Subject: fix: kconsole overhaul — 8 bugs fixed (echo, VGA, serial, dmesg) X-Git-Url: https://projects.tadryanom.me/docs/static/gitweb.js?a=commitdiff_plain;h=eb38508f3c07611bc65397fbeb2925a0ba46b093;p=AdrOS.git fix: kconsole overhaul — 8 bugs fixed (echo, VGA, serial, dmesg) Issues reported and fixed: 1. Double echo: TTY keyboard callback was echoing characters via tty_output_char() AND kconsole was echoing via console_write(). Fix: keyboard_set_callback(0) detaches TTY on kconsole entry. 2. Backspace printing garbage: VGA vga_put_char() only handled \n, treating \b as a visible glyph. Fix: full control char support in vga_put_char — \b moves cursor back, \r resets column, \t advances to next tab stop, non-printable chars filtered. 3. Enter printing garbage: same root cause as #2 — \r was not handled by VGA. Now handled properly. 4. Cursor not tracking: VGA had no hardware cursor update. Added hal_video_set_cursor() HAL function using CRTC registers 0x3D4/0x3D5 on x86, called after every character output. 5. Clear screen broken: was using ANSI escape \033[2J which VGA can't parse. Added vga_clear() function, kconsole calls it directly. 6. dmesg contamination: kconsole prompts and help text used kprintf() which appends to klog ring buffer. Introduced kc_puts() wrapper over console_write() for interactive output that should NOT appear in dmesg. 7. Scroll (Shift+PageUp/Down): deferred — requires scrollback buffer (significant feature). Documented as known limitation. 8. Serial input not working (-serial stdio): kgetc() only read from PS/2 keyboard via keyboard_read_blocking(). Added hal_uart_try_getc() to HAL (poll UART LSR data-ready bit), rewrote kgetc() to poll both keyboard and UART in a loop. 9. ring3 command: removed from kconsole (useless without initrd in emergency mode). Replaced with proper ls command using readdir to list directory contents. HAL changes (all 4 architectures): - hal_video_set_cursor(row, col) — x86 uses VGA CRTC I/O ports - hal_uart_try_getc() — non-blocking serial RX polling Build: clean, cppcheck: clean, smoke: 19/19 pass --- diff --git a/include/hal/uart.h b/include/hal/uart.h index 3fa3078..6a5761f 100644 --- a/include/hal/uart.h +++ b/include/hal/uart.h @@ -3,5 +3,6 @@ void hal_uart_init(void); void hal_uart_putc(char c); +int hal_uart_try_getc(void); #endif diff --git a/include/hal/video.h b/include/hal/video.h index 5d45446..b8ed2ea 100644 --- a/include/hal/video.h +++ b/include/hal/video.h @@ -4,5 +4,6 @@ #include uint16_t* hal_video_text_buffer(void); +void hal_video_set_cursor(int row, int col); #endif diff --git a/include/vga_console.h b/include/vga_console.h index 1277a86..544c215 100644 --- a/include/vga_console.h +++ b/include/vga_console.h @@ -7,5 +7,6 @@ void vga_init(void); void vga_put_char(char c); void vga_print(const char* str); void vga_set_color(uint8_t fg, uint8_t bg); +void vga_clear(void); #endif diff --git a/src/drivers/vga_console.c b/src/drivers/vga_console.c index 17770b4..69ae716 100644 --- a/src/drivers/vga_console.c +++ b/src/drivers/vga_console.c @@ -26,6 +26,55 @@ static void vga_scroll(void) { term_row = VGA_HEIGHT - 1; } +static void vga_update_hw_cursor(void) { + hal_video_set_cursor(term_row, term_col); +} + +static void vga_put_char_unlocked(char c) { + if (!VGA_BUFFER) return; + + switch (c) { + case '\n': + term_col = 0; + term_row++; + break; + case '\r': + term_col = 0; + break; + case '\b': + if (term_col > 0) { + term_col--; + } else if (term_row > 0) { + term_row--; + term_col = VGA_WIDTH - 1; + } + break; + case '\t': + term_col = (term_col + 8) & ~7; + if (term_col >= VGA_WIDTH) { + term_col = 0; + term_row++; + } + break; + default: + if ((unsigned char)c >= ' ') { + const int index = term_row * VGA_WIDTH + term_col; + VGA_BUFFER[index] = (uint16_t)(unsigned char)c | (uint16_t)term_color << 8; + term_col++; + } + break; + } + + if (term_col >= VGA_WIDTH) { + term_col = 0; + term_row++; + } + + if (term_row >= VGA_HEIGHT) { + vga_scroll(); + } +} + void vga_init(void) { VGA_BUFFER = (volatile uint16_t*)hal_video_text_buffer(); term_col = 0; @@ -42,6 +91,7 @@ void vga_init(void) { VGA_BUFFER[index] = (uint16_t) ' ' | (uint16_t) term_color << 8; } } + vga_update_hw_cursor(); } void vga_set_color(uint8_t fg, uint8_t bg) { @@ -52,33 +102,28 @@ void vga_set_color(uint8_t fg, uint8_t bg) { void vga_put_char(char c) { uintptr_t flags = spin_lock_irqsave(&vga_lock); + vga_put_char_unlocked(c); + vga_update_hw_cursor(); + spin_unlock_irqrestore(&vga_lock, flags); +} + +void vga_print(const char* str) { + uintptr_t flags = spin_lock_irqsave(&vga_lock); + if (!VGA_BUFFER) { spin_unlock_irqrestore(&vga_lock, flags); return; } - if (c == '\n') { - term_col = 0; - term_row++; - } else { - const int index = term_row * VGA_WIDTH + term_col; - VGA_BUFFER[index] = (uint16_t) c | (uint16_t) term_color << 8; - term_col++; - } - - if (term_col >= VGA_WIDTH) { - term_col = 0; - term_row++; - } - - if (term_row >= VGA_HEIGHT) { - vga_scroll(); + for (int i = 0; str[i] != '\0'; i++) { + vga_put_char_unlocked(str[i]); } + vga_update_hw_cursor(); spin_unlock_irqrestore(&vga_lock, flags); } -void vga_print(const char* str) { +void vga_clear(void) { uintptr_t flags = spin_lock_irqsave(&vga_lock); if (!VGA_BUFFER) { @@ -86,26 +131,14 @@ void vga_print(const char* str) { return; } - for (int i = 0; str[i] != '\0'; i++) { - char c = str[i]; - if (c == '\n') { - term_col = 0; - term_row++; - } else { - const int index = term_row * VGA_WIDTH + term_col; - VGA_BUFFER[index] = (uint16_t) c | (uint16_t) term_color << 8; - term_col++; - } - - if (term_col >= VGA_WIDTH) { - term_col = 0; - term_row++; - } - - if (term_row >= VGA_HEIGHT) { - vga_scroll(); + for (int y = 0; y < VGA_HEIGHT; y++) { + for (int x = 0; x < VGA_WIDTH; x++) { + VGA_BUFFER[y * VGA_WIDTH + x] = (uint16_t)' ' | (uint16_t)term_color << 8; } } + term_col = 0; + term_row = 0; + vga_update_hw_cursor(); spin_unlock_irqrestore(&vga_lock, flags); } diff --git a/src/hal/arm/uart.c b/src/hal/arm/uart.c index dc9e74a..066cb62 100644 --- a/src/hal/arm/uart.c +++ b/src/hal/arm/uart.c @@ -11,3 +11,11 @@ void hal_uart_putc(char c) { while (uart[6] & (1 << 5)) { } uart[0] = (uint32_t)c; } + +int hal_uart_try_getc(void) { + volatile uint32_t* uart = (volatile uint32_t*)UART_BASE; + if (!(uart[6] & (1 << 4))) { + return (int)(uart[0] & 0xFF); + } + return -1; +} diff --git a/src/hal/arm/video.c b/src/hal/arm/video.c index 474c9ff..d347331 100644 --- a/src/hal/arm/video.c +++ b/src/hal/arm/video.c @@ -3,3 +3,7 @@ uint16_t* hal_video_text_buffer(void) { return (uint16_t*)0; } + +void hal_video_set_cursor(int row, int col) { + (void)row; (void)col; +} diff --git a/src/hal/mips/uart.c b/src/hal/mips/uart.c index 4b76f62..c22c915 100644 --- a/src/hal/mips/uart.c +++ b/src/hal/mips/uart.c @@ -15,3 +15,10 @@ void hal_uart_putc(char c) { while ((mmio_read8(UART_BASE + 5) & 0x20) == 0) { } mmio_write8(UART_BASE, (uint8_t)c); } + +int hal_uart_try_getc(void) { + if (mmio_read8(UART_BASE + 5) & 0x01) { + return (int)mmio_read8(UART_BASE); + } + return -1; +} diff --git a/src/hal/mips/video.c b/src/hal/mips/video.c index 474c9ff..d347331 100644 --- a/src/hal/mips/video.c +++ b/src/hal/mips/video.c @@ -3,3 +3,7 @@ uint16_t* hal_video_text_buffer(void) { return (uint16_t*)0; } + +void hal_video_set_cursor(int row, int col) { + (void)row; (void)col; +} diff --git a/src/hal/riscv/uart.c b/src/hal/riscv/uart.c index 3da0906..8d7eb9a 100644 --- a/src/hal/riscv/uart.c +++ b/src/hal/riscv/uart.c @@ -13,3 +13,10 @@ void hal_uart_putc(char c) { while ((mmio_read8(UART_BASE + 5) & 0x20) == 0) { } mmio_write8(UART_BASE, (uint8_t)c); } + +int hal_uart_try_getc(void) { + if (mmio_read8(UART_BASE + 5) & 0x01) { + return (int)mmio_read8(UART_BASE); + } + return -1; +} diff --git a/src/hal/riscv/video.c b/src/hal/riscv/video.c index 474c9ff..d347331 100644 --- a/src/hal/riscv/video.c +++ b/src/hal/riscv/video.c @@ -3,3 +3,7 @@ uint16_t* hal_video_text_buffer(void) { return (uint16_t*)0; } + +void hal_video_set_cursor(int row, int col) { + (void)row; (void)col; +} diff --git a/src/hal/x86/uart.c b/src/hal/x86/uart.c index c256cf0..37dc176 100644 --- a/src/hal/x86/uart.c +++ b/src/hal/x86/uart.c @@ -20,3 +20,10 @@ void hal_uart_putc(char c) { while ((inb(UART_BASE + 5) & 0x20) == 0 && --timeout > 0) { } outb(UART_BASE, (uint8_t)c); } + +int hal_uart_try_getc(void) { + if (inb(UART_BASE + 5) & 0x01) { + return (int)inb(UART_BASE); + } + return -1; +} diff --git a/src/hal/x86/video.c b/src/hal/x86/video.c index 210af46..a8acfcb 100644 --- a/src/hal/x86/video.c +++ b/src/hal/x86/video.c @@ -1,16 +1,31 @@ #include "hal/video.h" +#include "io.h" #if defined(__i386__) || defined(__x86_64__) // This address must be mapped by VMM in higher half (see vmm_init mapping) #define VGA_TEXT_BUFFER_VIRT ((uint16_t*)0xC00B8000) +#define VGA_CRTC_INDEX 0x3D4 +#define VGA_CRTC_DATA 0x3D5 +#define VGA_WIDTH 80 uint16_t* hal_video_text_buffer(void) { return (uint16_t*)VGA_TEXT_BUFFER_VIRT; } +void hal_video_set_cursor(int row, int col) { + uint16_t pos = (uint16_t)(row * VGA_WIDTH + col); + outb(VGA_CRTC_INDEX, 0x0F); /* cursor low byte register */ + outb(VGA_CRTC_DATA, (uint8_t)(pos & 0xFF)); + outb(VGA_CRTC_INDEX, 0x0E); /* cursor high byte register */ + outb(VGA_CRTC_DATA, (uint8_t)((pos >> 8) & 0xFF)); +} + #else uint16_t* hal_video_text_buffer(void) { return (uint16_t*)0; } +void hal_video_set_cursor(int row, int col) { + (void)row; (void)col; +} #endif diff --git a/src/kernel/console.c b/src/kernel/console.c index 3bbf96d..283e081 100644 --- a/src/kernel/console.c +++ b/src/kernel/console.c @@ -7,6 +7,7 @@ #include "spinlock.h" #include "uart_console.h" #include "hal/uart.h" +#include "hal/cpu.h" #include "vga_console.h" #include "keyboard.h" @@ -234,10 +235,16 @@ void kprintf(const char* fmt, ...) { } int kgetc(void) { - char c = 0; - int rd = keyboard_read_blocking(&c, 1); - if (rd <= 0) return -1; - return (int)(unsigned char)c; + for (;;) { + char c = 0; + int rd = keyboard_read_nonblock(&c, 1); + if (rd > 0) return (int)(unsigned char)c; + + int sc = hal_uart_try_getc(); + if (sc >= 0) return sc; + + hal_cpu_idle(); + } } size_t klog_read(char* out, size_t out_size) { diff --git a/src/kernel/kconsole.c b/src/kernel/kconsole.c index 6eb7ec4..7cbc440 100644 --- a/src/kernel/kconsole.c +++ b/src/kernel/kconsole.c @@ -1,28 +1,60 @@ #include "kconsole.h" #include "console.h" +#include "vga_console.h" #include "utils.h" #include "fs.h" #include "heap.h" #include "pmm.h" #include "process.h" +#include "keyboard.h" #include "arch/arch_platform.h" #include "hal/system.h" #include "hal/cpu.h" #define KCMD_MAX 128 +static void kc_puts(const char* s) { + console_write(s); +} + static void kconsole_help(void) { - kprintf("kconsole commands:\n"); - kprintf(" help - Show this list\n"); - kprintf(" clear - Clear screen\n"); - kprintf(" ls - List files\n"); - kprintf(" cat - Read file content\n"); - kprintf(" mem - Show memory stats\n"); - kprintf(" dmesg - Show kernel log buffer\n"); - kprintf(" sleep - Sleep for N ticks\n"); - kprintf(" ring3 - Run usermode syscall test\n"); - kprintf(" reboot - Restart system\n"); - kprintf(" halt - Halt the CPU\n"); + kc_puts("kconsole commands:\n"); + kc_puts(" help - Show this list\n"); + kc_puts(" clear - Clear screen\n"); + kc_puts(" ls [path] - List files in directory\n"); + kc_puts(" cat - Read file content\n"); + kc_puts(" mem - Show memory stats\n"); + kc_puts(" dmesg - Show kernel log buffer\n"); + kc_puts(" reboot - Restart system\n"); + kc_puts(" halt - Halt the CPU\n"); +} + +static void kconsole_ls(const char* path) { + fs_node_t* dir = NULL; + + if (!path || path[0] == '\0') { + dir = fs_root; + } else { + dir = vfs_lookup(path); + } + + if (!dir) { + kprintf("ls: cannot access '%s': not found\n", path ? path : "/"); + return; + } + + if (!dir->readdir) { + kprintf("ls: not a directory\n"); + return; + } + + uint32_t idx = 0; + struct vfs_dirent ent; + while (1) { + int rc = dir->readdir(dir, &idx, &ent, sizeof(ent)); + if (rc != 0) break; + kprintf(" %s\n", ent.d_name); + } } static void kconsole_exec(const char* cmd) { @@ -30,45 +62,38 @@ static void kconsole_exec(const char* cmd) { kconsole_help(); } else if (strcmp(cmd, "clear") == 0) { - kprintf("\033[2J\033[1;1H"); + vga_clear(); } else if (strcmp(cmd, "ls") == 0) { - if (!fs_root) { - kprintf("No filesystem mounted.\n"); - } else { - kprintf("Filesystem Mounted (InitRD).\n"); - kprintf("Try: cat test.txt\n"); - } + kconsole_ls(NULL); + } + else if (strncmp(cmd, "ls ", 3) == 0) { + kconsole_ls(cmd + 3); } else if (strncmp(cmd, "cat ", 4) == 0) { - if (!fs_root) { - kprintf("No filesystem mounted.\n"); + const char* fname = cmd + 4; + fs_node_t* file = NULL; + if (fname[0] == '/') { + file = vfs_lookup(fname); } else { - const char* fname = cmd + 4; - fs_node_t* file = NULL; - if (fname[0] == '/') { - file = vfs_lookup(fname); - } else { - char abs[132]; - abs[0] = '/'; - abs[1] = 0; - strcpy(abs + 1, fname); - file = vfs_lookup(abs); - } - if (file) { - kprintf("Reading %s...\n", fname); - uint8_t* buf = (uint8_t*)kmalloc(file->length + 1); - if (buf) { - uint32_t sz = vfs_read(file, 0, file->length, buf); - buf[sz] = 0; - kprintf("%s\n", (char*)buf); - kfree(buf); - } else { - kprintf("OOM: File too big for heap.\n"); - } + char abs[132]; + abs[0] = '/'; + abs[1] = 0; + strcpy(abs + 1, fname); + file = vfs_lookup(abs); + } + if (file) { + uint8_t* buf = (uint8_t*)kmalloc(file->length + 1); + if (buf) { + uint32_t sz = vfs_read(file, 0, file->length, buf); + buf[sz] = 0; + kprintf("%s\n", (char*)buf); + kfree(buf); } else { - kprintf("File not found.\n"); + kprintf("cat: out of memory\n"); } + } else { + kprintf("cat: %s: not found\n", fname); } } else if (strcmp(cmd, "mem") == 0) { @@ -80,24 +105,16 @@ static void kconsole_exec(const char* cmd) { size_t n = klog_read(buf, sizeof(buf)); if (n > 0) { console_write(buf); + console_write("\n"); } else { - kprintf("(empty)\n"); + kc_puts("(empty)\n"); } } - else if (strncmp(cmd, "sleep ", 6) == 0) { - int ticks = atoi(cmd + 6); - kprintf("Sleeping for %s ticks...\n", cmd + 6); - process_sleep(ticks); - kprintf("Woke up!\n"); - } - else if (strcmp(cmd, "ring3") == 0) { - arch_platform_usermode_test_start(); - } else if (strcmp(cmd, "reboot") == 0) { hal_system_reboot(); } else if (strcmp(cmd, "halt") == 0) { - kprintf("System halted.\n"); + kc_puts("System halted.\n"); hal_cpu_disable_interrupts(); for (;;) hal_cpu_idle(); } @@ -107,14 +124,16 @@ static void kconsole_exec(const char* cmd) { } void kconsole_enter(void) { - kprintf("\n*** AdrOS Kernel Console (kconsole) ***\n"); - kprintf("Type 'help' for available commands.\n"); + keyboard_set_callback(0); + + kc_puts("\n*** AdrOS Kernel Console (kconsole) ***\n"); + kc_puts("Type 'help' for available commands.\n"); char line[KCMD_MAX]; int pos = 0; for (;;) { - kprintf("kconsole> "); + kc_puts("kconsole> "); pos = 0; while (pos < KCMD_MAX - 1) { @@ -134,6 +153,8 @@ void kconsole_enter(void) { continue; } + if (ch < ' ' || ch > '~') continue; + line[pos++] = (char)ch; char echo[2] = { (char)ch, 0 }; console_write(echo);