From 67a71026584e7eb5d0b920e19c49918209ad72cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Sun, 21 Aug 2005 23:27:51 +0000 Subject: [PATCH] Fixed a pretty fatal bug in the page fault handling: interrupts were enabled even if "kernel_startup" was "true". Page faults are now no longer allowed during kernel startup (and could be easily avoided). The only situation where we accept page faults with interrupts turned off now is during a kernel debugger session. Added a command debug_debugger_running() to test for that situation. kernel_debugger() no longer sets kernel_startup while it's running; there should be no situation when this could be helpful. Interrupts are no longer enabled when a page fault happens in the kernel debugger. This potentially fixes all sorts of problems, and not only in the kernel debugger, it could also have affected SMP (will test later). git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@14045 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/private/kernel/debug.h | 1 + src/system/kernel/arch/x86/arch_int.c | 6 ++++-- src/system/kernel/debug/debug.c | 23 +++++++++++++---------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/headers/private/kernel/debug.h b/headers/private/kernel/debug.h index 6f75cec2ff..81a1c7c326 100644 --- a/headers/private/kernel/debug.h +++ b/headers/private/kernel/debug.h @@ -32,6 +32,7 @@ extern status_t debug_init(struct kernel_args *args); extern status_t debug_init_post_vm(struct kernel_args *args); extern void debug_early_boot_message(const char *string); extern void debug_puts(const char *s); +extern bool debug_debugger_running(void); extern void _user_debug_output(const char *userString); diff --git a/src/system/kernel/arch/x86/arch_int.c b/src/system/kernel/arch/x86/arch_int.c index 7d55505fcc..11e1db6bf6 100644 --- a/src/system/kernel/arch/x86/arch_int.c +++ b/src/system/kernel/arch/x86/arch_int.c @@ -407,19 +407,21 @@ i386_handle_trap(struct iframe frame) case 14: // Page-Fault Exception (#PF) { + bool disabledInterrupts = /*kernel_startup ||*/ debug_debugger_running(); unsigned int cr2; addr_t newip; asm("movl %%cr2, %0" : "=r" (cr2)); - if ((frame.flags & 0x200) == 0 && !kernel_startup) { + if ((frame.flags & 0x200) == 0 && !disabledInterrupts) { // if the interrupts were disabled, and we are not running the kernel startup // the page fault was not allowed to happen and we must panic panic("page fault, but interrupts were disabled. Touching address %p from eip %p\n", (void *)cr2, (void *)frame.eip); } else if (thread != NULL && thread->page_faults_allowed < 1) panic("page fault not allowed at this place. Touching address %p from eip %p\n", (void *)cr2, (void *)frame.eip); - enable_interrupts(); + if (!disabledInterrupts) + enable_interrupts(); ret = vm_page_fault(cr2, frame.eip, (frame.error_code & 0x2) != 0, // write access diff --git a/src/system/kernel/debug/debug.c b/src/system/kernel/debug/debug.c index 9e7c36f9d7..0f24966b3d 100644 --- a/src/system/kernel/debug/debug.c +++ b/src/system/kernel/debug/debug.c @@ -43,7 +43,7 @@ static bool sSyslogOutputEnabled = false; static bool sBlueScreenEnabled = false; static bool sBlueScreenOutput = false; static spinlock sSpinlock = 0; -static int sDebuggerOnCPU = -1; +static int32 sDebuggerOnCPU = -1; static struct debugger_command *sCommands; @@ -272,11 +272,11 @@ parse_line(char *buf, char **argv, int *argc, int max_args) static void kernel_debugger_loop(void) { - kprintf("Welcome to Kernel Debugging Land...\n"); - kprintf("Running on CPU %d\n", smp_get_current_cpu()); - sDebuggerOnCPU = smp_get_current_cpu(); + kprintf("Welcome to Kernel Debugging Land...\n"); + kprintf("Running on CPU %ld\n", sDebuggerOnCPU); + for (;;) { struct debugger_command *cmd = NULL; int argc; @@ -310,6 +310,8 @@ kernel_debugger_loop(void) if (++sCurrentLine >= HISTORY_SIZE) sCurrentLine = 0; } + + sDebuggerOnCPU = -1; } @@ -368,6 +370,13 @@ cmd_continue(int argc, char **argv) // #pragma mark - private kernel API +bool +debug_debugger_running(void) +{ + return sDebuggerOnCPU != -1; +} + + void debug_puts(const char *string) { @@ -529,11 +538,6 @@ kernel_debugger(const char *message) state = disable_interrupts(); - // XXX by setting kernel_startup = true, we disable - // XXX the interrupt check in semaphore code etc. - // XXX should be renamed? - kernel_startup = true; - if (sDebuggerOnCPU != smp_get_current_cpu()) { // halt all of the other cpus @@ -553,7 +557,6 @@ kernel_debugger(const char *message) kernel_debugger_loop(); sBlueScreenOutput = false; - kernel_startup = false; restore_interrupts(state); // ToDo: in case we change dbg_register_file - don't we want to restore it?