From 1dd61e93b6932ef59a26d9c65bc74bac6fb88f8a Mon Sep 17 00:00:00 2001 From: Bananymous Date: Thu, 2 Mar 2023 01:56:09 +0200 Subject: [PATCH] Kernel: Threads cannot take arguments anymore --- kernel/arch/i386/Thread.S | 19 +++++++------------ kernel/arch/x86_64/Thread.S | 6 +++--- kernel/include/kernel/Scheduler.h | 14 ++------------ kernel/include/kernel/Thread.h | 21 +++++---------------- kernel/kernel/Scheduler.cpp | 22 +++++++++++++++------- kernel/kernel/Shell.cpp | 15 ++++++--------- kernel/kernel/Thread.cpp | 27 ++++++++++----------------- 7 files changed, 48 insertions(+), 76 deletions(-) diff --git a/kernel/arch/i386/Thread.S b/kernel/arch/i386/Thread.S index 934d41c3..54e48a65 100644 --- a/kernel/arch/i386/Thread.S +++ b/kernel/arch/i386/Thread.S @@ -5,23 +5,18 @@ read_rip: jmp *%eax exit_thread_trampoline: - addl $16, %esp - popl %eax - pushl $0x696969 - pushl %eax + addl $4, %esp + pushl (%esp) ret -# void start_thread(uint32_t arg0, uint32_t arg1, uint32_t arg2, uint32_t arg3, uint32_t rsp, uint32_t rip) +# void start_thread(uint32_t function, uint32_t esp, uint32_t eip) .global start_thread start_thread: - movl %esp, %eax - movl 24(%eax), %ecx - movl 20(%eax), %esp + movl 4(%esp), %eax + movl 12(%esp), %ecx + movl 8(%esp), %esp - pushl 16(%eax) - pushl 12(%eax) - pushl 8(%eax) - pushl 4(%eax) + pushl %eax pushl $exit_thread_trampoline movl $0, %ebp diff --git a/kernel/arch/x86_64/Thread.S b/kernel/arch/x86_64/Thread.S index 51bdb0ef..8d316926 100644 --- a/kernel/arch/x86_64/Thread.S +++ b/kernel/arch/x86_64/Thread.S @@ -8,14 +8,14 @@ exit_thread_trampoline: movq 8(%rsp), %rdi ret -# void start_thread(uint64_t arg0, uint64_t arg1, uint64_t arg2, uint64_t arg3, uint64_t rsp, uint64_t rip) +# void start_thread(uint64_t function, uint64_t rsp, uint64_t rip) .global start_thread start_thread: - movq %r8, %rsp + movq %rsi, %rsp movq $0, %rbp pushq $exit_thread_trampoline sti - jmp *%r9 + jmp *%rdx # void continue_thread(uint64_t rsp, uint64_t rip) .global continue_thread diff --git a/kernel/include/kernel/Scheduler.h b/kernel/include/kernel/Scheduler.h index 9e3af85c..f624b0e4 100644 --- a/kernel/include/kernel/Scheduler.h +++ b/kernel/include/kernel/Scheduler.h @@ -17,18 +17,8 @@ namespace Kernel static Scheduler& get(); const Thread& current_thread() const; - - template - BAN::ErrorOr add_thread(const BAN::Function& func, Args... args) - { - uintptr_t flags; - asm volatile("pushf; pop %0" : "=r"(flags)); - asm volatile("cli"); - TRY(m_threads.emplace_back(func, BAN::forward(args)...)); - if (flags & (1 << 9)) - asm volatile("sti"); - return {}; - } + + BAN::ErrorOr add_thread(const BAN::Function& function); void reschedule(); void set_current_thread_sleeping(); diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index 3518033a..ac162cbb 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -22,19 +22,10 @@ namespace Kernel }; public: -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wpmf-conversions" - template - Thread(const BAN::Function& func, Args... args) - : Thread((uintptr_t)(void*)&BAN::Function::operator(), (uintptr_t)&func, ((uintptr_t)args)...) - { - static_assert(((BAN::is_integral_v || BAN::is_pointer_v) && ...)); - } -#pragma GCC diagnostic pop - + Thread(const BAN::Function&); ~Thread(); - uint32_t id() const { return m_id; } + uint32_t tid() const { return m_tid; } void set_rsp(uintptr_t rsp) { m_rsp = rsp; } void set_rip(uintptr_t rip) { m_rip = rip; } @@ -43,21 +34,19 @@ namespace Kernel uintptr_t rip() const { return m_rip; } State state() const { return m_state; } - const uintptr_t* args() const { return m_args; } + const BAN::Function* function() const { return &m_function; } private: - Thread(uintptr_t rip, uintptr_t func, uintptr_t arg1 = 0, uintptr_t arg2 = 0, uintptr_t arg3 = 0); void on_exit(); private: void* m_stack_base = nullptr; State m_state = State::NotStarted; - uintptr_t m_args[4] = {}; uintptr_t m_rip = 0; uintptr_t m_rsp = 0; - const uint32_t m_id = 0; + const uint32_t m_tid = 0; - alignas(max_align_t) uint8_t m_function[BAN::Function::size()] { 0 }; + BAN::Function m_function; }; } \ No newline at end of file diff --git a/kernel/kernel/Scheduler.cpp b/kernel/kernel/Scheduler.cpp index f10dd211..077d39c9 100644 --- a/kernel/kernel/Scheduler.cpp +++ b/kernel/kernel/Scheduler.cpp @@ -7,7 +7,7 @@ namespace Kernel static Scheduler* s_instance = nullptr; - extern "C" void start_thread(uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3, uintptr_t rsp, uintptr_t rip); + extern "C" void start_thread(const BAN::Function* function, uintptr_t rsp, uintptr_t rip); extern "C" void continue_thread(uintptr_t rsp, uintptr_t rip); extern "C" uintptr_t read_rip(); @@ -30,6 +30,18 @@ namespace Kernel return *m_current_iterator; } + + BAN::ErrorOr Scheduler::add_thread(const BAN::Function& function) + { + uintptr_t flags; + asm volatile("pushf; pop %0" : "=r"(flags)); + asm volatile("cli"); + TRY(m_threads.emplace_back(function)); + if (flags & (1 << 9)) + asm volatile("sti"); + return {}; + } + void Scheduler::reschedule() { ASSERT(InterruptController::get().is_in_service(PIT_IRQ)); @@ -62,9 +74,6 @@ namespace Kernel Thread& current = *m_current_iterator; - //if (m_threads.size() == 2 && current.id() != 0 && current.state() == Thread::State::Running) - // return; - if (current.state() == Thread::State::Done) { m_threads.remove(m_current_iterator); @@ -91,7 +100,7 @@ namespace Kernel { case Thread::State::NotStarted: next.set_state(Thread::State::Running); - start_thread(next.args()[0], next.args()[1], next.args()[2], next.args()[3], next.rsp(), next.rip()); + start_thread(next.function(), next.rsp(), next.rip()); break; case Thread::State::Paused: next.set_state(Thread::State::Running); @@ -123,8 +132,7 @@ namespace Kernel ASSERT(current.state() == Thread::State::NotStarted); current.set_state(Thread::State::Running); - const uintptr_t* args = current.args(); - start_thread(args[0], args[1], args[2], args[3], current.rsp(), current.rip()); + start_thread(current.function(), current.rsp(), current.rip()); ASSERT(false); } diff --git a/kernel/kernel/Shell.cpp b/kernel/kernel/Shell.cpp index 800285dd..1c847cc4 100644 --- a/kernel/kernel/Shell.cpp +++ b/kernel/kernel/Shell.cpp @@ -180,21 +180,18 @@ argument_done: { static SpinLock s_thread_spinlock; - // NOTE: This is a workaround to pass values as copies to threads. - // I have only implemented passing integer and pointers. - // We don't continue execution until the thread has unlocked - // the spinlock. s_thread_spinlock.lock(); - MUST(Scheduler::get().add_thread(Function*)>( - [this] (const Vector* args_ptr) + + MUST(Scheduler::get().add_thread(Function( + [this, &arguments] { - auto args = *args_ptr; - s_thread_spinlock.unlock(); + auto args = arguments; args.remove(0); + s_thread_spinlock.unlock(); PIT::sleep(5000); process_command(args); } - ), &arguments)); + ))); while (s_thread_spinlock.is_locked()); } diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index b7fa686d..8599c6ff 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -10,7 +10,7 @@ namespace Kernel { - static uint32_t s_next_id = 0; + static uint32_t s_next_tid = 0; static constexpr size_t thread_stack_size = 16384; @@ -21,26 +21,19 @@ namespace Kernel memcpy((void*)rsp, (void*)&value, size); } - Thread::Thread(uintptr_t rip, uintptr_t func, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3) - : m_id(s_next_id++) + Thread::Thread(const BAN::Function& function) + : m_tid(s_next_tid++) + , m_function(function) { m_stack_base = kmalloc(thread_stack_size, PAGE_SIZE); ASSERT(m_stack_base); - - m_rsp = (uintptr_t)m_stack_base + thread_stack_size; - m_rip = rip; - m_args[1] = arg1; - m_args[2] = arg2; - m_args[3] = arg3; - // NOTE: in System V ABI arg0 is the pointer to 'this' - // we copy the function object to Thread object - // so we can ensure the lifetime of it. We store - // it as raw bytes so that Thread can be non-templated. - // This requires BAN::Function to be trivially copyable - // but for now it should be. - memcpy(m_function, (void*)func, sizeof(m_function)); - m_args[0] = (uintptr_t)m_function; + m_rsp = (uintptr_t)m_stack_base + thread_stack_size; + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpmf-conversions" + m_rip = (uintptr_t)(void*)&BAN::Function::operator(); +#pragma GCC diagnostic pop write_to_stack(m_rsp, this); write_to_stack(m_rsp, &Thread::on_exit);