From 1b65f850ee47ec1bef16b41dc951191351a0dad3 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Wed, 27 Mar 2024 15:06:24 +0200 Subject: [PATCH] Kernel: Rename thread stacks to more appropriate names --- kernel/include/kernel/Thread.h | 21 ++++++++++---------- kernel/kernel/IDT.cpp | 17 ++++++++-------- kernel/kernel/Process.cpp | 6 +++--- kernel/kernel/Scheduler.cpp | 2 +- kernel/kernel/Thread.cpp | 36 +++++++++++++++++----------------- 5 files changed, 40 insertions(+), 42 deletions(-) diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index 5e66069b20..dd2e276360 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -67,13 +67,13 @@ namespace Kernel void set_started() { ASSERT(m_state == State::NotStarted); m_state = State::Executing; } State state() const { return m_state; } - vaddr_t stack_base() const { return m_stack->vaddr(); } - size_t stack_size() const { return m_stack->size(); } - VirtualRange& stack() { return *m_stack; } - VirtualRange& interrupt_stack() { return *m_interrupt_stack; } + vaddr_t kernel_stack_bottom() const { return m_kernel_stack->vaddr(); } + vaddr_t kernel_stack_top() const { return m_kernel_stack->vaddr() + m_kernel_stack->size(); } + VirtualRange& kernel_stack() { return *m_kernel_stack; } - vaddr_t interrupt_stack_base() const { return m_interrupt_stack ? m_interrupt_stack->vaddr() : 0; } - size_t interrupt_stack_size() const { return m_interrupt_stack ? m_interrupt_stack->size() : 0; } + vaddr_t userspace_stack_bottom() const { return is_userspace() ? m_userspace_stack->vaddr() : 0; } + vaddr_t userspace_stack_top() const { return is_userspace() ? m_userspace_stack->vaddr() + m_userspace_stack->size() : 0; } + VirtualRange& userspace_stack() { ASSERT(is_userspace()); return *m_userspace_stack; } static Thread& current(); static pid_t current_tid(); @@ -84,7 +84,7 @@ namespace Kernel bool is_userspace() const { return m_is_userspace; } - size_t virtual_page_count() const { return m_stack->size() / PAGE_SIZE; } + size_t virtual_page_count() const { return (m_kernel_stack->size() / PAGE_SIZE) + (m_userspace_stack->size() / PAGE_SIZE); } size_t physical_page_count() const { return virtual_page_count(); } #if __enable_sse @@ -101,10 +101,9 @@ namespace Kernel private: static constexpr size_t m_kernel_stack_size = PAGE_SIZE * 4; - static constexpr size_t m_userspace_stack_size = PAGE_SIZE * 2; - static constexpr size_t m_interrupt_stack_size = PAGE_SIZE * 2; - BAN::UniqPtr m_interrupt_stack; - BAN::UniqPtr m_stack; + static constexpr size_t m_userspace_stack_size = PAGE_SIZE * 4; + BAN::UniqPtr m_kernel_stack; + BAN::UniqPtr m_userspace_stack; uintptr_t m_ip { 0 }; uintptr_t m_sp { 0 }; const pid_t m_tid { 0 }; diff --git a/kernel/kernel/IDT.cpp b/kernel/kernel/IDT.cpp index 1b8ea33d93..2d52b2a0e3 100644 --- a/kernel/kernel/IDT.cpp +++ b/kernel/kernel/IDT.cpp @@ -179,20 +179,19 @@ namespace Kernel if (isr == ISR::PageFault) { // Check if stack is OOB - auto& stack = Thread::current().stack(); - auto& istack = Thread::current().interrupt_stack(); - if (stack.vaddr() < interrupt_stack->sp && interrupt_stack->sp <= stack.vaddr() + stack.size()) - ; // using normal stack - else if (istack.vaddr() < interrupt_stack->sp && interrupt_stack->sp <= istack.vaddr() + istack.size()) - ; // using interrupt stack + auto& thread = Thread::current(); + if (thread.userspace_stack_bottom() < interrupt_stack->sp && interrupt_stack->sp <= thread.userspace_stack_top()) + ; // using userspace stack + else if (thread.kernel_stack_bottom() < interrupt_stack->sp && interrupt_stack->sp <= thread.kernel_stack_top()) + ; // using kernel stack else { derrorln("Stack pointer out of bounds!"); derrorln("rip {H}", interrupt_stack->ip); - derrorln("rsp {H}, stack {H}->{H}, istack {H}->{H}", + derrorln("rsp {H}, userspace stack {H}->{H}, kernel stack {H}->{H}", interrupt_stack->sp, - stack.vaddr(), stack.vaddr() + stack.size(), - istack.vaddr(), istack.vaddr() + istack.size() + thread.userspace_stack_bottom(), thread.userspace_stack_top(), + thread.kernel_stack_bottom(), thread.kernel_stack_top() ); Thread::current().handle_signal(SIGKILL); goto done; diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index 2d757a10c8..927ac3fa95 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -676,9 +676,9 @@ namespace Kernel LockGuard _(m_process_lock); - if (Thread::current().stack().contains(address)) + if (Thread::current().userspace_stack().contains(address)) { - TRY(Thread::current().stack().allocate_page_for_demand_paging(address)); + TRY(Thread::current().userspace_stack().allocate_page_for_demand_paging(address)); return true; } @@ -1879,7 +1879,7 @@ namespace Kernel if (vaddr == 0) return {}; - if (vaddr >= thread.stack_base() && vaddr + size <= thread.stack_base() + thread.stack_size()) + if (vaddr >= thread.userspace_stack_bottom() && vaddr + size <= thread.userspace_stack_top()) return {}; // FIXME: should we allow cross mapping access? diff --git a/kernel/kernel/Scheduler.cpp b/kernel/kernel/Scheduler.cpp index a2440e963d..cf59a8a304 100644 --- a/kernel/kernel/Scheduler.cpp +++ b/kernel/kernel/Scheduler.cpp @@ -251,7 +251,7 @@ namespace Kernel if (current->has_process()) { current->process().page_table().load(); - Processor::gdt().set_tss_stack(current->interrupt_stack_base() + current->interrupt_stack_size()); + Processor::gdt().set_tss_stack(current->kernel_stack_top()); } else PageTable::kernel().load(); diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index 413335adb6..7a38469dfa 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -38,7 +38,7 @@ namespace Kernel BAN::ScopeGuard thread_deleter([thread] { delete thread; }); // Initialize stack and registers - thread->m_stack = TRY(VirtualRange::create_to_vaddr_range( + thread->m_kernel_stack = TRY(VirtualRange::create_to_vaddr_range( PageTable::kernel(), KERNEL_OFFSET, ~(uintptr_t)0, @@ -46,7 +46,7 @@ namespace Kernel PageTable::Flags::ReadWrite | PageTable::Flags::Present, true )); - thread->m_sp = thread->stack_base() + thread->stack_size(); + thread->m_sp = thread->kernel_stack_top(); thread->m_ip = (uintptr_t)entry; // Initialize stack for returning @@ -72,7 +72,7 @@ namespace Kernel thread->m_is_userspace = true; - thread->m_stack = TRY(VirtualRange::create_to_vaddr_range( + thread->m_userspace_stack = TRY(VirtualRange::create_to_vaddr_range( process->page_table(), 0x300000, KERNEL_OFFSET, m_userspace_stack_size, @@ -80,10 +80,10 @@ namespace Kernel true )); - thread->m_interrupt_stack = TRY(VirtualRange::create_to_vaddr_range( + thread->m_kernel_stack = TRY(VirtualRange::create_to_vaddr_range( process->page_table(), 0x300000, KERNEL_OFFSET, - m_interrupt_stack_size, + m_kernel_stack_size, PageTable::Flags::ReadWrite | PageTable::Flags::Present, true )); @@ -162,8 +162,8 @@ namespace Kernel thread->m_is_userspace = true; - thread->m_interrupt_stack = TRY(m_interrupt_stack->clone(new_process->page_table())); - thread->m_stack = TRY(m_stack->clone(new_process->page_table())); + thread->m_kernel_stack = TRY(m_kernel_stack->clone(new_process->page_table())); + thread->m_userspace_stack = TRY(m_userspace_stack->clone(new_process->page_table())); thread->m_state = State::Executing; @@ -183,11 +183,11 @@ namespace Kernel [](void*) { const auto& info = Process::current().userspace_info(); - thread_userspace_trampoline(Thread::current().sp(), info.entry, info.argc, info.argv, info.envp); + thread_userspace_trampoline(Thread::current().userspace_stack_top(), info.entry, info.argc, info.argv, info.envp); ASSERT_NOT_REACHED(); } ); - m_sp = stack_base() + stack_size(); + m_sp = kernel_stack_top(); m_ip = (uintptr_t)entry_trampoline; // Signal mask is inherited @@ -216,7 +216,7 @@ namespace Kernel ASSERT_NOT_REACHED(); } ); - m_sp = stack_base() + stack_size(); + m_sp = kernel_stack_top(); m_ip = (uintptr_t)entry; m_signal_pending_mask = 0; @@ -244,7 +244,7 @@ namespace Kernel { if (!is_userspace() || m_state != State::Executing) return false; - auto& interrupt_stack = *reinterpret_cast(interrupt_stack_base() + interrupt_stack_size() - sizeof(InterruptStack)); + auto& interrupt_stack = *reinterpret_cast(kernel_stack_top() - sizeof(InterruptStack)); if (!GDT::is_user_segment(interrupt_stack.cs)) return false; uint64_t full_pending_mask = m_signal_pending_mask | process().signal_pending_mask();; @@ -255,7 +255,7 @@ namespace Kernel { if (!is_userspace() || m_state != State::Executing) return false; - auto& interrupt_stack = *reinterpret_cast(interrupt_stack_base() + interrupt_stack_size() - sizeof(InterruptStack)); + auto& interrupt_stack = *reinterpret_cast(kernel_stack_top() - sizeof(InterruptStack)); return interrupt_stack.ip == (uintptr_t)signal_trampoline; } @@ -266,7 +266,7 @@ namespace Kernel SpinLockGuard _(m_signal_lock); - auto& interrupt_stack = *reinterpret_cast(interrupt_stack_base() + interrupt_stack_size() - sizeof(InterruptStack)); + auto& interrupt_stack = *reinterpret_cast(kernel_stack_top() - sizeof(InterruptStack)); ASSERT(GDT::is_user_segment(interrupt_stack.cs)); if (signal == 0) @@ -398,13 +398,13 @@ namespace Kernel void Thread::validate_stack() const { - if (stack_base() <= m_sp && m_sp <= stack_base() + stack_size()) + if (kernel_stack_bottom() <= m_sp && m_sp <= kernel_stack_top()) return; - if (interrupt_stack_base() <= m_sp && m_sp <= interrupt_stack_base() + interrupt_stack_size()) + if (userspace_stack_bottom() <= m_sp && m_sp <= userspace_stack_top()) return; - Kernel::panic("sp {8H}, stack {8H}->{8H}, interrupt_stack {8H}->{8H}", m_sp, - stack_base(), stack_base() + stack_size(), - interrupt_stack_base(), interrupt_stack_base() + interrupt_stack_size() + Kernel::panic("sp {8H}, kernel stack {8H}->{8H}, userspace stack {8H}->{8H}", m_sp, + kernel_stack_bottom(), kernel_stack_top(), + userspace_stack_bottom(), userspace_stack_top() ); }