From 68a2250886a71868a7d3d86d53e7977e8daf2756 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Tue, 2 Feb 2021 18:27:37 -0800 Subject: [PATCH] [kernel] Use IST for kernel stacks for NMI, #DF, #PF We started actually running up against the page boundary for kernel stacks and thus double-faulting on page faults from kernel space. So I finally added IST stacks. Note that we currently just increment/decrement the IST entry by a page when we enter the handler to avoid clobbering on re-entry, but this means: * these handlers need to be able to operate with only a page of stack * kernel stacks always have to be >1 pages * the amount of nesting possible is tied to the kernel stack size. These seem fine for now, but we should maybe find a way to use something besides g_kernel_stacks to set up the IST stacks if/when this becomes an issue. --- src/include/kernel_memory.h | 2 +- src/kernel/gdt.cpp | 53 +++++++++++++--- src/kernel/gdt.h | 33 ++++++++-- src/kernel/interrupt_isrs.inc | 107 ++++++++++++++++---------------- src/kernel/interrupts.cpp | 23 +++---- src/kernel/interrupts.h | 6 +- src/kernel/interrupts.s | 5 +- src/kernel/memory_bootstrap.cpp | 24 +++++++ 8 files changed, 166 insertions(+), 87 deletions(-) diff --git a/src/include/kernel_memory.h b/src/include/kernel_memory.h index 102d1f5..248eed2 100644 --- a/src/include/kernel_memory.h +++ b/src/include/kernel_memory.h @@ -17,7 +17,7 @@ namespace memory { constexpr uintptr_t page_offset = 0xffffc00000000000ull; /// Max number of pages for a kernel stack - constexpr unsigned kernel_stack_pages = 4; + constexpr unsigned kernel_stack_pages = 2; /// Max number of pages for a kernel buffer constexpr unsigned kernel_buffer_pages = 16; diff --git a/src/kernel/gdt.cpp b/src/kernel/gdt.cpp index 7ff443b..d629eda 100644 --- a/src/kernel/gdt.cpp +++ b/src/kernel/gdt.cpp @@ -4,6 +4,7 @@ #include "kutil/enum_bitfields.h" #include "kutil/memory.h" #include "console.h" +#include "kernel_memory.h" #include "log.h" @@ -136,19 +137,55 @@ idt_set_entry(uint8_t i, uint64_t addr, uint16_t selector, uint8_t flags) } void -tss_set_stack(int ring, uintptr_t rsp) +tss_set_stack(unsigned ring, uintptr_t rsp) { kassert(ring < 3, "Bad ring passed to tss_set_stack."); g_tss.rsp[ring] = rsp; } uintptr_t -tss_get_stack(int ring) +tss_get_stack(unsigned ring) { kassert(ring < 3, "Bad ring passed to tss_get_stack."); return g_tss.rsp[ring]; } +void +idt_set_ist(unsigned i, unsigned ist) +{ + g_idt_table[i].ist = ist; +} + +void +tss_set_ist(unsigned ist, uintptr_t rsp) +{ + kassert(ist > 0 && ist < 7, "Bad ist passed to tss_set_ist."); + g_tss.ist[ist] = rsp; +} + +void +ist_increment(unsigned i) +{ + uint8_t ist = g_idt_table[i].ist; + if (ist) + g_tss.ist[ist] += memory::frame_size; +} + +void +ist_decrement(unsigned i) +{ + uint8_t ist = g_idt_table[i].ist; + if (ist) + g_tss.ist[ist] -= memory::frame_size; +} + +uintptr_t +tss_get_ist(unsigned ist) +{ + kassert(ist > 0 && ist < 7, "Bad ist passed to tss_get_ist."); + return g_tss.ist[ist]; +} + void gdt_init() { @@ -184,14 +221,14 @@ gdt_init() } void -gdt_dump(int index) +gdt_dump(unsigned index) { const table_ptr &table = g_gdtr; console *cons = console::get(); - int start = 0; - int count = (table.limit + 1) / sizeof(gdt_descriptor); + unsigned start = 0; + unsigned count = (table.limit + 1) / sizeof(gdt_descriptor); if (index != -1) { start = index; count = 1; @@ -240,13 +277,13 @@ gdt_dump(int index) } void -idt_dump(int index) +idt_dump(unsigned index) { const table_ptr &table = g_idtr; - int start = 0; - int count = (table.limit + 1) / sizeof(idt_descriptor); + unsigned start = 0; + unsigned count = (table.limit + 1) / sizeof(idt_descriptor); if (index != -1) { start = index; count = 1; diff --git a/src/kernel/gdt.h b/src/kernel/gdt.h index 848175e..9e8a959 100644 --- a/src/kernel/gdt.h +++ b/src/kernel/gdt.h @@ -17,17 +17,42 @@ void idt_set_entry(uint8_t i, uint64_t addr, uint16_t selector, uint8_t flags); /// Set the stack pointer for a given ring in the TSS /// \arg ring Ring to set for (0-2) /// \arg rsp Stack pointer to set -void tss_set_stack(int ring, uintptr_t rsp); +void tss_set_stack(unsigned ring, uintptr_t rsp); /// Get the stack pointer for a given ring in the TSS /// \arg ring Ring to get (0-2) /// \returns Stack pointers for that ring -uintptr_t tss_get_stack(int ring); +uintptr_t tss_get_stack(unsigned ring); + +/// Set the given IDT entry to use the given IST entry +/// \arg i Which IDT entry to set +/// \arg ist Which IST entry to set (1-7) +void idt_set_ist(unsigned i, unsigned ist); + +/// Set the stack pointer for a given IST in the TSS +/// \arg ist Which IST entry to set (1-7) +/// \arg rsp Stack pointer to set +void tss_set_ist(unsigned ist, uintptr_t rsp); + +/// Increment the stack pointer for the given vector, +/// if it's using an IST entry +/// \arg i Which IDT entry to use +void ist_increment(unsigned i); + +/// Decrement the stack pointer for the given vector, +/// if it's using an IST entry +/// \arg i Which IDT entry to use +void ist_decrement(unsigned i); + +/// Get the stack pointer for a given IST in the TSS +/// \arg ring Which IST entry to get (1-7) +/// \returns Stack pointers for that IST entry +uintptr_t tss_get_ist(unsigned ist); /// Dump information about the current GDT to the screen /// \arg index Which entry to print, or -1 for all entries -void gdt_dump(int index = -1); +void gdt_dump(unsigned index = -1); /// Dump information about the current IDT to the screen /// \arg index Which entry to print, or -1 for all entries -void idt_dump(int index = -1); +void idt_dump(unsigned index = -1); diff --git a/src/kernel/interrupt_isrs.inc b/src/kernel/interrupt_isrs.inc index cb084cf..2d55794 100644 --- a/src/kernel/interrupt_isrs.inc +++ b/src/kernel/interrupt_isrs.inc @@ -1,36 +1,36 @@ -ISR (0x00, isrDivideByZero) -ISR (0x01, isrDebug) -ISR (0x02, isrNMI) -ISR (0x03, isrBreakpoint) -ISR (0x04, isrOverflow) -ISR (0x05, isrBRE) -ISR (0x06, isrInvalidOp) -ISR (0x07, isrDNA) -EISR(0x08, isrDoubleFault) -ISR (0x09, isrCoprocessor) -EISR(0x0a, isrInvalidTSS) -EISR(0x0b, isrSegmentNP) -EISR(0x0c, isrSSFault) -EISR(0x0d, isrGPFault) -EISR(0x0e, isrPageFault) -ISR (0x0f, isr15) +ISR (0x00, 0, isrDivideByZero) +ISR (0x01, 0, isrDebug) +ISR (0x02, 1, isrNMI) +ISR (0x03, 0, isrBreakpoint) +ISR (0x04, 0, isrOverflow) +ISR (0x05, 0, isrBRE) +ISR (0x06, 0, isrInvalidOp) +ISR (0x07, 0, isrDNA) +EISR(0x08, 2, isrDoubleFault) +ISR (0x09, 0, isrCoprocessor) +EISR(0x0a, 0, isrInvalidTSS) +EISR(0x0b, 0, isrSegmentNP) +EISR(0x0c, 0, isrSSFault) +EISR(0x0d, 0, isrGPFault) +EISR(0x0e, 3, isrPageFault) +ISR (0x0f, 0, isr15) -ISR (0x10, isrX87FPE) -ISR (0x11, isrAlignmentChk) -ISR (0x12, isrMachineChk) -ISR (0x13, isrSIMDFPE) -ISR (0x14, isrVirt) -ISR (0x15, isr21) -ISR (0x16, isr22) -ISR (0x17, isr23) -ISR (0x18, isr24) -ISR (0x19, isr25) -ISR (0x1a, isr26) -ISR (0x1b, isr27) -ISR (0x1c, isr28) -ISR (0x1d, isr29) -ISR (0x1e, isrSecurity) -ISR (0x1f, isr31) +ISR (0x10, 0, isrX87FPE) +ISR (0x11, 0, isrAlignmentChk) +ISR (0x12, 0, isrMachineChk) +ISR (0x13, 0, isrSIMDFPE) +ISR (0x14, 0, isrVirt) +ISR (0x15, 0, isr21) +ISR (0x16, 0, isr22) +ISR (0x17, 0, isr23) +ISR (0x18, 0, isr24) +ISR (0x19, 0, isr25) +ISR (0x1a, 0, isr26) +ISR (0x1b, 0, isr27) +ISR (0x1c, 0, isr28) +ISR (0x1d, 0, isr29) +ISR (0x1e, 0, isrSecurity) +ISR (0x1f, 0, isr31) IRQ (0x20, 0x00, irq00) IRQ (0x21, 0x01, irq01) @@ -237,28 +237,27 @@ IRQ (0xde, 0xbe, irqBE) IRQ (0xdf, 0xbf, irqBF) -ISR (0xe0, isrTimer) -ISR (0xe1, isrLINT0) -ISR (0xe2, isrLINT1) -ISR (0xe4, isrAssert) +ISR (0xe0, 0, isrTimer) +ISR (0xe1, 0, isrLINT0) +ISR (0xe2, 0, isrLINT1) +ISR (0xe4, 0, isrAssert) -UISR(0xee, isrSyscall) -ISR (0xef, isrSpurious) +ISR (0xef, 0, isrSpurious) -ISR (0xf0, isrIgnore0) -ISR (0xf1, isrIgnore1) -ISR (0xf2, isrIgnore2) -ISR (0xf3, isrIgnore3) -ISR (0xf4, isrIgnore4) -ISR (0xf5, isrIgnore5) -ISR (0xf6, isrIgnore6) -ISR (0xf7, isrIgnore7) +ISR (0xf0, 0, isrIgnore0) +ISR (0xf1, 0, isrIgnore1) +ISR (0xf2, 0, isrIgnore2) +ISR (0xf3, 0, isrIgnore3) +ISR (0xf4, 0, isrIgnore4) +ISR (0xf5, 0, isrIgnore5) +ISR (0xf6, 0, isrIgnore6) +ISR (0xf7, 0, isrIgnore7) -ISR (0xf8, isrIgnore8) -ISR (0xf9, isrIgnore9) -ISR (0xfa, isrIgnoreA) -ISR (0xfb, isrIgnoreB) -ISR (0xfc, isrIgnoreC) -ISR (0xfd, isrIgnoreD) -ISR (0xfe, isrIgnoreE) -ISR (0xff, isrIgnoreF) +ISR (0xf8, 0, isrIgnore8) +ISR (0xf9, 0, isrIgnore9) +ISR (0xfa, 0, isrIgnoreA) +ISR (0xfb, 0, isrIgnoreB) +ISR (0xfc, 0, isrIgnoreC) +ISR (0xfd, 0, isrIgnoreD) +ISR (0xfe, 0, isrIgnoreE) +ISR (0xff, 0, isrIgnoreF) diff --git a/src/kernel/interrupts.cpp b/src/kernel/interrupts.cpp index f0cb76a..97797fd 100644 --- a/src/kernel/interrupts.cpp +++ b/src/kernel/interrupts.cpp @@ -28,13 +28,11 @@ extern "C" { void isr_handler(cpu_state*); void irq_handler(cpu_state*); -#define ISR(i, name) extern void name (); -#define EISR(i, name) extern void name (); -#define UISR(i, name) extern void name (); +#define ISR(i, s, name) extern void name (); +#define EISR(i, s, name) extern void name (); #define IRQ(i, q, name) extern void name (); #include "interrupt_isrs.inc" #undef IRQ -#undef UISR #undef EISR #undef ISR } @@ -50,13 +48,11 @@ uint8_t get_irq(unsigned vector) { switch (vector) { -#define ISR(i, name) -#define EISR(i, name) -#define UISR(i, name) +#define ISR(i, s, name) +#define EISR(i, s, name) #define IRQ(i, q, name) case i : return q; #include "interrupt_isrs.inc" #undef IRQ -#undef UISR #undef EISR #undef ISR @@ -87,13 +83,11 @@ disable_legacy_pic() void interrupts_init() { -#define ISR(i, name) idt_set_entry(i, reinterpret_cast(& name), 0x08, 0x8e); -#define EISR(i, name) idt_set_entry(i, reinterpret_cast(& name), 0x08, 0x8e); -#define UISR(i, name) idt_set_entry(i, reinterpret_cast(& name), 0x08, 0xee); +#define ISR(i, s, name) idt_set_entry(i, reinterpret_cast(& name), 0x08, 0x8e); +#define EISR(i, s, name) idt_set_entry(i, reinterpret_cast(& name), 0x08, 0x8e); #define IRQ(i, q, name) idt_set_entry(i, reinterpret_cast(& name), 0x08, 0x8e); #include "interrupt_isrs.inc" #undef IRQ -#undef UISR #undef EISR #undef ISR @@ -106,8 +100,10 @@ void isr_handler(cpu_state *regs) { console *cons = console::get(); + uint8_t vector = regs->interrupt & 0xff; + ist_decrement(vector); - switch (static_cast(regs->interrupt & 0xff)) { + switch (static_cast(vector)) { case isr::isrDebug: { cons->set_color(11); @@ -279,6 +275,7 @@ isr_handler(cpu_state *regs) print_stacktrace(2); _halt(); } + ist_increment(vector); *reinterpret_cast(apic_eoi_addr) = 0; } diff --git a/src/kernel/interrupts.h b/src/kernel/interrupts.h index 839c2ee..793288b 100644 --- a/src/kernel/interrupts.h +++ b/src/kernel/interrupts.h @@ -7,13 +7,11 @@ /// Enum of all defined ISR/IRQ vectors enum class isr : uint8_t { -#define ISR(i, name) name = i, -#define EISR(i, name) name = i, -#define UISR(i, name) name = i, +#define ISR(i, s, name) name = i, +#define EISR(i, s, name) name = i, #define IRQ(i, q, name) name = i, #include "interrupt_isrs.inc" #undef IRQ -#undef UISR #undef EISR #undef ISR diff --git a/src/kernel/interrupts.s b/src/kernel/interrupts.s index cbb9e7b..7be0de9 100644 --- a/src/kernel/interrupts.s +++ b/src/kernel/interrupts.s @@ -53,9 +53,8 @@ isr_handler_return: jmp irq_handler_prelude %endmacro -%define EISR(i, name) EMIT_EISR name, i ; ISR with error code -%define UISR(i, name) EMIT_ISR name, i ; ISR callable from user space -%define ISR(i, name) EMIT_ISR name, i +%define EISR(i, s, name) EMIT_EISR name, i ; ISR with error code +%define ISR(i, s, name) EMIT_ISR name, i %define IRQ(i, q, name) EMIT_IRQ name, i section .isrs diff --git a/src/kernel/memory_bootstrap.cpp b/src/kernel/memory_bootstrap.cpp index f20803d..78d7f6d 100644 --- a/src/kernel/memory_bootstrap.cpp +++ b/src/kernel/memory_bootstrap.cpp @@ -9,6 +9,7 @@ #include "device_manager.h" #include "frame_allocator.h" +#include "gdt.h" #include "io.h" #include "log.h" #include "msr.h" @@ -117,6 +118,29 @@ memory_initialize_post_ctors(args::header &kargs) reinterpret_cast(kargs.page_tables), kargs.table_count); + using memory::frame_size; + using memory::kernel_stack_pages; + constexpr size_t stack_size = kernel_stack_pages * frame_size; + + for (int ist = 1; ist <= 3; ++ist) { + uintptr_t bottom = g_kernel_stacks.get_section(); + log::debug(logs::boot, "Installing IST%d stack at %llx", ist, bottom); + + // Pre-realize and xerothese stacks, they're no good + // if they page fault + kutil::memset(reinterpret_cast(bottom), 0, stack_size); + + // Skip two entries to be the null frame + tss_set_ist(ist, bottom + stack_size - 2 * sizeof(uintptr_t)); + } + +#define ISR(i, s, name) if (s) { idt_set_ist(i, s); } +#define EISR(i, s, name) if (s) { idt_set_ist(i, s); } +#define IRQ(i, q, name) +#include "interrupt_isrs.inc" +#undef IRQ +#undef EISR +#undef ISR } static void