From bd490c4c7b8679666eb085d1156635e97c65f4b2 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Wed, 6 Jan 2021 11:30:40 -0800 Subject: [PATCH 1/7] [scripts] Ignore demangle errors building sym table For some reason, cxxfilt fails to demangle some names on some systems. Instead of failing the build process, just skip those symbols. --- scripts/build_symbol_table.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/build_symbol_table.py b/scripts/build_symbol_table.py index 2095d34..516a072 100755 --- a/scripts/build_symbol_table.py +++ b/scripts/build_symbol_table.py @@ -19,15 +19,19 @@ def parse_syms(infile): representing the symbols in the text segment of the binary. Returns a list of (address, symbol_name).""" - from cxxfilt import demangle + from cxxfilt import demangle, InvalidName syms = [] for line in sys.stdin: addr, t, mangled = line.split() if t not in "tTvVwW": continue + try: + name = demangle(mangled) + except InvalidName: + continue + addr = int(addr, base=16) - name = demangle(mangled) syms.append((addr, name)) return sorted(syms) From 2c444dccd62e484e650b9783797876a214bfc174 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Wed, 6 Jan 2021 11:32:45 -0800 Subject: [PATCH 2/7] [build] Remove fake terminal.elf A fake terminal.elf (copy of nulldrv.elf) was added to test the loader. Now that there actually are multiple programs to load, remove the fake one. --- scripts/templates/build.ninja.j2 | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scripts/templates/build.ninja.j2 b/scripts/templates/build.ninja.j2 index 5ee5aca..8ab68b7 100644 --- a/scripts/templates/build.ninja.j2 +++ b/scripts/templates/build.ninja.j2 @@ -193,16 +193,12 @@ build $builddir/fatroot/nulldrv.elf : cp $builddir/user/nulldrv.elf build $builddir/fatroot/fb.elf : cp $builddir/user/fb.elf name = fb driver to FAT image -build $builddir/fatroot/terminal.elf : cp $builddir/user/nulldrv.elf - name = terminal driver to FAT image - build ${builddir}/fatroot/symbol_table.dat : makest ${builddir}/jsix.elf build $builddir/jsix.img : makefat | $ $builddir/fatroot/symbol_table.dat $ $builddir/fatroot/nulldrv.elf $ $builddir/fatroot/fb.elf $ - $builddir/fatroot/terminal.elf $ $builddir/fatroot/jsix.elf $ $builddir/fatroot/efi/boot/bootx64.efi name = jsix.img From cd30126f172f5e73178b1cde3c905f9ed9c1a341 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Wed, 6 Jan 2021 11:34:34 -0800 Subject: [PATCH 3/7] [boot] Reduce loader spam Now that the ELF loader is known to be working correctly, remove its extra print statements about section loading to keep the bootloader output to one screen. --- src/boot/loader.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/boot/loader.cpp b/src/boot/loader.cpp index 6d64972..94ef5c9 100644 --- a/src/boot/loader.cpp +++ b/src/boot/loader.cpp @@ -97,13 +97,8 @@ load_program( void *src_start = offset_ptr(data.data, pheader->offset); void *dest_start = offset_ptr(pages, pheader->vaddr - prog_base); bs->copy_mem(dest_start, src_start, pheader->file_size); - - console::print(L" section %d phys: 0x%lx\r\n", i, pages); - console::print(L" section %d virt: 0x%lx\r\n", i, pheader->vaddr); } - console::print(L" entrypoint: 0x%lx\r\n", header->entrypoint); - program.phys_addr = reinterpret_cast(pages); program.size = total_size; program.virt_addr = prog_base; From 8b3356e9d836be86890fe07427bdc32817af396c Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Wed, 6 Jan 2021 23:09:50 -0800 Subject: [PATCH 4/7] [kutil] Remove uint64_t hash_node specialization Using a hash of zero to signal an empty slot doesn't play nice with the hash_node specialization that uses the key for the hash, when 0 is a common key. I thought it would be ok, that it'd just be something to remember. But then I used 0 as a key anyway, so clearly it was a bad idea. --- src/libraries/kutil/include/kutil/map.h | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/libraries/kutil/include/kutil/map.h b/src/libraries/kutil/include/kutil/map.h index 9536e61..4ea1ddf 100644 --- a/src/libraries/kutil/include/kutil/map.h +++ b/src/libraries/kutil/include/kutil/map.h @@ -45,20 +45,6 @@ struct hash_node inline uint64_t hash() const { return h; } }; -template -struct hash_node -{ - uint64_t key; - V val; - - hash_node(hash_node &&o) : key(std::move(o.key)), val(std::move(o.val)) {} - hash_node(uint64_t h, uint64_t &&k, V &&v) : key(std::move(k)), val(std::move(v)) {} - ~hash_node() {} - - inline uint64_t & hash() { return key; } - inline uint64_t hash() const { return key; } -}; - /// Base class for hash maps template class base_map From e08e00790ffb08b8e121e8321ab2c6a7d86c6a08 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Wed, 6 Jan 2021 23:14:39 -0800 Subject: [PATCH 5/7] [kernel] Give processes and threads self handles It was not consistent how processes got handles to themselves or their threads, ending up with double entries. Now make such handles automatic and expose them with new self_handle() methods. --- src/kernel/objects/process.cpp | 6 +++++- src/kernel/objects/process.h | 3 +++ src/kernel/objects/thread.cpp | 2 ++ src/kernel/objects/thread.h | 5 +++++ src/kernel/scheduler.cpp | 4 ++-- src/kernel/syscalls/thread.cpp | 2 +- 6 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/kernel/objects/process.cpp b/src/kernel/objects/process.cpp index 7cdf6a3..3232fcc 100644 --- a/src/kernel/objects/process.cpp +++ b/src/kernel/objects/process.cpp @@ -21,13 +21,16 @@ process::process() : m_state {state::running} { s_processes.append(this); + + j6_handle_t self = add_handle(this); + kassert(self == self_handle(), "Process self-handle is not 0"); } // The "kernel process"-only constructor process::process(page_table *kpml4) : kobject {kobject::type::process}, m_space {kpml4}, - m_next_handle {0}, + m_next_handle {self_handle()+1}, m_state {state::running} { } @@ -120,6 +123,7 @@ process::thread_exited(thread *th) kassert(&th->m_parent == this, "Process got thread_exited for non-child!"); uint32_t status = th->m_return_code; m_threads.remove_swap(th); + remove_handle(th->self_handle()); delete th; if (m_threads.count() == 0) { diff --git a/src/kernel/objects/process.h b/src/kernel/objects/process.h index fb7d2eb..14c8a8f 100644 --- a/src/kernel/objects/process.h +++ b/src/kernel/objects/process.h @@ -68,6 +68,9 @@ public: /// \returns True if this thread ending has ended the process bool thread_exited(thread *th); + /// Get the handle for this process to refer to itself + inline j6_handle_t self_handle() const { return 0; } + /// Get the process object that owns kernel threads and the /// kernel address space static process & kernel_process(); diff --git a/src/kernel/objects/thread.cpp b/src/kernel/objects/thread.cpp index 6a5b1ca..3eb6a9f 100644 --- a/src/kernel/objects/thread.cpp +++ b/src/kernel/objects/thread.cpp @@ -26,6 +26,8 @@ thread::thread(process &parent, uint8_t pri, uintptr_t rsp0) : setup_kernel_stack(); else m_tcb.rsp0 = rsp0; + + m_self_handle = parent.add_handle(this); } thread::~thread() diff --git a/src/kernel/objects/thread.h b/src/kernel/objects/thread.h index 68e6257..41c0e7f 100644 --- a/src/kernel/objects/thread.h +++ b/src/kernel/objects/thread.h @@ -141,6 +141,9 @@ public: /// \arg rip The address to return to, must be user space void add_thunk_user(uintptr_t rip); + /// Get the handle representing this thread to its process + j6_handle_t self_handle() const { return m_self_handle; } + /// Create the kernel idle thread /// \arg kernel The process object that owns kernel tasks /// \arg pri The idle thread priority value @@ -175,4 +178,6 @@ private: uint64_t m_wait_data; j6_status_t m_wait_result; j6_koid_t m_wait_obj; + + j6_handle_t m_self_handle; }; diff --git a/src/kernel/scheduler.cpp b/src/kernel/scheduler.cpp index 5f89f25..b8e203f 100644 --- a/src/kernel/scheduler.cpp +++ b/src/kernel/scheduler.cpp @@ -116,11 +116,11 @@ load_process_image(uintptr_t phys, uintptr_t virt, size_t bytes, TCB *tcb) initv = push(tcb->rsp3); initv->type = j6_init_handle_process; - initv->value = static_cast(proc.add_handle(&proc)); + initv->value = static_cast(proc.self_handle()); initv = push(tcb->rsp3); initv->type = j6_init_handle_thread; - initv->value = static_cast(proc.add_handle(&th)); + initv->value = static_cast(th.self_handle()); initv = push(tcb->rsp3); initv->type = j6_init_handle_space; diff --git a/src/kernel/syscalls/thread.cpp b/src/kernel/syscalls/thread.cpp index f1a0cad..49091b2 100644 --- a/src/kernel/syscalls/thread.cpp +++ b/src/kernel/syscalls/thread.cpp @@ -15,7 +15,7 @@ thread_create(void *rip, j6_handle_t *handle) thread *child = p.create_thread(); child->add_thunk_user(reinterpret_cast(rip)); - *handle = p.add_handle(child); + *handle = p.self_handle(); child->clear_state(thread::state::loading); child->set_state(thread::state::ready); From d11dd0c3f93b57059440e4a3f07f7fe28b7f8b07 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Wed, 6 Jan 2021 23:16:16 -0800 Subject: [PATCH 6/7] [kernel] Fix memory clobbering from endpoint The endpoint receive syscalls can block and then write to userspace memory. Since the current address space may be different after blocking, make sure to only actually write to the user memory after returning to the syscall handler - pass values that are on the syscall handler stack deeper into the kernel. --- src/include/j6/types.h | 1 + src/kernel/syscalls/endpoint.cpp | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/include/j6/types.h b/src/include/j6/types.h index 15d762c..587775e 100644 --- a/src/include/j6/types.h +++ b/src/include/j6/types.h @@ -18,6 +18,7 @@ typedef uint64_t j6_signal_t; typedef uint64_t j6_tag_t; #define j6_tag_system_flag 0x8000000000000000 +#define j6_tag_invalid 0x0000000000000000 /// If all high bits except the last 16 are set, then the tag represents /// an IRQ. diff --git a/src/kernel/syscalls/endpoint.cpp b/src/kernel/syscalls/endpoint.cpp index 733bbb5..e4c9649 100644 --- a/src/kernel/syscalls/endpoint.cpp +++ b/src/kernel/syscalls/endpoint.cpp @@ -35,7 +35,12 @@ endpoint_receive(j6_handle_t handle, j6_tag_t *tag, size_t *len, void *data) endpoint *e = get_handle(handle); if (!e) return j6_err_invalid_arg; - return e->receive(tag, len, data); + j6_tag_t out_tag = j6_tag_invalid; + size_t out_len = 0; + j6_status_t s = e->receive(&out_tag, &out_len, data); + *tag = out_tag; + *len = out_len; + return s; } j6_status_t @@ -51,7 +56,12 @@ endpoint_sendrecv(j6_handle_t handle, j6_tag_t *tag, size_t *len, void *data) if (status != j6_status_ok) return status; - return e->receive(tag, len, data); + j6_tag_t out_tag = j6_tag_invalid; + size_t out_len = 0; + j6_status_t s = e->receive(&out_tag, &out_len, data); + *tag = out_tag; + *len = out_len; + return s; } } // namespace syscalls From 1be929b9d5f6e02805a473051e76da528b18e53b Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Wed, 6 Jan 2021 23:20:29 -0800 Subject: [PATCH 7/7] [fb] Simplify scrollback line counting Using a start and a count was redundant when we know how many lines are in the buffer already. --- src/drivers/fb/scrollback.cpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/drivers/fb/scrollback.cpp b/src/drivers/fb/scrollback.cpp index 170dd09..69aa017 100644 --- a/src/drivers/fb/scrollback.cpp +++ b/src/drivers/fb/scrollback.cpp @@ -8,7 +8,6 @@ scrollback::scrollback(unsigned lines, unsigned cols, unsigned margin) : m_rows {lines}, m_cols {cols}, - m_start {0}, m_count {0}, m_margin {margin} { @@ -23,24 +22,20 @@ scrollback::scrollback(unsigned lines, unsigned cols, unsigned margin) : void scrollback::add_line(const char *line, size_t len) { - unsigned i = 0; - if (m_count < m_rows) - i = m_count++; - else - i = m_start++; + unsigned i = m_count++ % m_rows; if (len > m_cols) len = m_cols; - memcpy(m_lines[i % m_rows], line, len); + memcpy(m_lines[i], line, len); if (len < m_cols) - memset(m_lines[i % m_rows]+len, ' ', m_cols - len); + memset(m_lines[i]+len, ' ', m_cols - len); } char * scrollback::get_line(unsigned i) { - return m_lines[(i+m_start)%m_rows]; + return m_lines[(i+m_count)%m_rows]; } void