From 9c9d589ea310f1290e4d6d2019fc8b51d9a86e42 Mon Sep 17 00:00:00 2001 From: Kimplul Date: Mon, 9 Oct 2023 14:57:18 +0300 Subject: move rpc stack handling to arch-specific code + Fairly considerable speedup, as we don't have to look up the rpc pte every time separately, instead cacheing them. Adds an architecture specific limitation to total rpc stack size, though. --- arch/riscv64/asm/asm-offsets.c | 2 +- arch/riscv64/config.h | 3 -- arch/riscv64/include/tcb.h | 3 ++ arch/riscv64/kernel/entry.S | 4 +-- arch/riscv64/kernel/smp.c | 3 ++ arch/riscv64/kernel/vmem.c | 81 +++++++++++++++++++++++++++++++++++++++++- common/tcb.c | 38 +------------------- common/uapi/conf.c | 16 +-------- common/uapi/ipc.c | 69 ++++++----------------------------- include/arch/tcb.h | 7 ++++ include/kmi/tcb.h | 2 +- 11 files changed, 109 insertions(+), 119 deletions(-) diff --git a/arch/riscv64/asm/asm-offsets.c b/arch/riscv64/asm/asm-offsets.c index a053996..80a48f2 100644 --- a/arch/riscv64/asm/asm-offsets.c +++ b/arch/riscv64/asm/asm-offsets.c @@ -98,7 +98,7 @@ void asm_offsets() /* At the moment tcbd is just a single register slot, so this works, but * if it's expanded in the future I'll need to figure out a way to * target specific substructure members. */ - OFFSETOF(tcbd, struct tcb); + OFFSETOF(arch, struct tcb); SIZEOF(tcb, struct tcb); ENUM(SYS_IPC_RESP); diff --git a/arch/riscv64/config.h b/arch/riscv64/config.h index a2694d5..765aa6b 100644 --- a/arch/riscv64/config.h +++ b/arch/riscv64/config.h @@ -73,9 +73,6 @@ /** User virtual memory space end. */ #define UVMEM_END (SZ_256G - SZ_8G) -/** RPC stack top. */ -#define RPC_STACK_TOP (UVMEM_END + SZ_1G) - /** RPC stack base. */ #define RPC_STACK_BASE (UVMEM_END) diff --git a/arch/riscv64/include/tcb.h b/arch/riscv64/include/tcb.h index 33e8fad..a804175 100644 --- a/arch/riscv64/include/tcb.h +++ b/arch/riscv64/include/tcb.h @@ -15,6 +15,9 @@ struct arch_tcbd { /** Extra scratch register. */ long scratch; + + struct vmem *rpc_leaf; + int rpc_idx; }; #endif /* ARCH_RISCV_TCB_H */ diff --git a/arch/riscv64/kernel/entry.S b/arch/riscv64/kernel/entry.S index 2959e2f..815694b 100644 --- a/arch/riscv64/kernel/entry.S +++ b/arch/riscv64/kernel/entry.S @@ -67,7 +67,7 @@ handle_trap: call kernel_panic continue_trap: - sr sp, offsetof_tcbd(tp) + sr sp, offsetof_arch(tp) lr sp, offsetof_regs(tp) addi sp, sp, -sizeof_registers csrrw tp, CSR_SSCRATCH, tp @@ -78,7 +78,7 @@ continue_trap: /* get current tcb into tp and set scratch to 0 so we can figure out if * exception occured in kernel or userspace */ csrrw tp, CSR_SSCRATCH, x0 - lr t5, offsetof_tcbd(tp) + lr t5, offsetof_arch(tp) sr t5, offsetof_sp(sp) /* load supervisor cause */ diff --git a/arch/riscv64/kernel/smp.c b/arch/riscv64/kernel/smp.c index cacb3b7..8d04491 100644 --- a/arch/riscv64/kernel/smp.c +++ b/arch/riscv64/kernel/smp.c @@ -114,6 +114,9 @@ void core_bringup(long hartid) { /* assume smp_bringup assigned our cpuid correctly */ id_t cpuid = hartid_to_cpuid(hartid); + + /* output is somewhat messed up due to no synchronisation but I guess + * that's fine for now */ info("core %ld online\n", (long)cpuid); /* realistically stuff after this point could probably be placed diff --git a/arch/riscv64/kernel/vmem.c b/arch/riscv64/kernel/vmem.c index e2d0fa3..7d1d3cc 100644 --- a/arch/riscv64/kernel/vmem.c +++ b/arch/riscv64/kernel/vmem.c @@ -6,6 +6,7 @@ * riscv64 implementation of arch-specific virtual memory handling. */ +#include #include #include #include @@ -375,7 +376,7 @@ void flush_tlb_full() void flush_tlb_all() { - /** @todo this only works on a single core atm. */ + /** @todo this only works on a single core atm. needs to do an IPI */ __asm__ volatile ("sfence.vma\n" ::: "memory"); } @@ -490,3 +491,81 @@ void clone_uvmem(struct vmem * restrict r, struct vmem * restrict b) b->leaf[i] = 0; } } + +size_t max_rpc_size() +{ + return SZ_512K; +} + +void setup_rpc_stack(struct tcb *t) +{ + /* by default rpc stack is marked inaccessible to generate segfaults on + * access so as to ease stack usage tracking */ + vmflags_t flags = VM_V | VM_R | VM_W | VM_U; + + size_t pages = order_size(MM_O1) / BASE_PAGE_SIZE; + for (size_t i = 0; i < pages; ++i) { + pm_t page = alloc_page(BASE_PAGE); + map_vpage(t->rpc.vmem, page, + RPC_STACK_BASE + BASE_PAGE_SIZE * i, + flags, BASE_PAGE); + + map_vpage(t->proc.vmem, page, + RPC_STACK_BASE + BASE_PAGE_SIZE * i, + flags, BASE_PAGE); + } + + /* we allocated a second order page for rpc stack usage */ + t->rpc_stack = RPC_STACK_BASE + order_size(MM_O1); + /* slightly hacky maybe but we know the first pte is at RPC_STACK_BASE, + * which means that it must also be the leaf */ + t->arch.rpc_leaf = (struct vmem *)__find_vmem(t->rpc.vmem, + RPC_STACK_BASE, BASE_PAGE); + /* 'reserve' top page of stack for kernel use */ + t->arch.rpc_idx = 511; +} + +vm_t rpc_position(struct tcb *t) +{ + /** @todo we assume rpc_idx is updated on every segfault of the rpc stack */ + /** @todo hmm, technically speaking we always know that on riscv the base + * page size if 4096, would it be a good idea to replace BASE_PAGE_SIZE in + * riscv-specific code with a RISCV_BASE_PAGE_SIZE or something? */ + return RPC_STACK_BASE + (BASE_PAGE_SIZE * t->arch.rpc_idx); +} + +void mark_rpc_invalid(struct tcb *t, vm_t top) +{ + struct vmem *b = t->arch.rpc_leaf; + int top_idx = t->arch.rpc_idx; + int bottom_idx = (top - RPC_STACK_BASE) / BASE_PAGE_SIZE; + catastrophic_assert(bottom_idx < top_idx); + + while (top_idx != bottom_idx) { + pm_t *pte = (pm_t *)&b->leaf[top_idx]; + /* make page not accessible from userspace */ + clear_bits(*pte, vp_flags(VM_U)); + top_idx--; + } + + t->arch.rpc_idx = top_idx; +} + +void mark_rpc_valid(struct tcb *t, vm_t bottom) +{ + struct vmem *b = t->arch.rpc_leaf; + int bottom_idx = t->arch.rpc_idx; + int top_idx = (bottom - RPC_STACK_BASE) / BASE_PAGE_SIZE; + catastrophic_assert(bottom_idx < top_idx); + + while (top_idx != bottom_idx) { + pm_t *pte = (pm_t *)&b->leaf[bottom_idx]; + /* make page accessible from userspace */ + set_bits(*pte, vp_flags(VM_U)); + /* clear used bits */ + clear_bits(*pte, vp_flags(VM_A | VM_D)); + bottom_idx++; + } + + t->arch.rpc_idx = bottom_idx; +} diff --git a/common/tcb.c b/common/tcb.c index d6194e8..e69f2f0 100644 --- a/common/tcb.c +++ b/common/tcb.c @@ -83,42 +83,6 @@ static id_t __alloc_tid(struct tcb *t) return ERR_NF; } -/** - * Setup RPC stack. - * - * RPC stack is local to each thread, and should not be visible to other threads - * in the same process. Currently maps the RPC stack in BASE_PAGE increments, to - * hopefully allow us to later quickly disallow access to programs lower down in - * the RPC call chain by turning off all stack pages lower than the current - * stack pointer. We shall see if this actually works or not. - * - * @param t Thread to setup RPC stack for. - * @param bytes Minimum size of RPC stack. - * @return Base of allocated RPC stack. - * - * \todo add error checking */ -static vm_t __setup_rpc_stack(struct tcb *t, size_t bytes) -{ - pm_t offset = 0; - size_t pages = __pages(bytes); - vmflags_t flags = VM_V | VM_R | VM_W | VM_U; - for (size_t i = 1; i <= pages; ++i) { - offset = alloc_page(BASE_PAGE); - map_vpage(t->rpc.vmem, offset, - RPC_STACK_TOP - BASE_PAGE_SIZE * i, - flags, BASE_PAGE); - - /* map stack into both process and rpc vmem since we want to - * optimistically write data into it during initialization of an - * rpc. */ - map_vpage(t->proc.vmem, offset, - RPC_STACK_TOP - BASE_PAGE_SIZE * i, - flags, BASE_PAGE); - } - t->rpc_stack = RPC_STACK_TOP; - return RPC_STACK_TOP - BASE_PAGE_SIZE * pages; -} - /** * Setup thread stack. * @@ -179,7 +143,7 @@ struct tcb *create_thread(struct tcb *p) t->eid = t->pid; t->rid = p->rid; t->rpc.vmem = create_vmem(); - __setup_rpc_stack(t, __call_stack_size); + setup_rpc_stack(t); t->regs = (vm_t)t; diff --git a/common/uapi/conf.c b/common/uapi/conf.c index 13be3ee..894b98a 100644 --- a/common/uapi/conf.c +++ b/common/uapi/conf.c @@ -18,14 +18,12 @@ /** \todo stack size should really be set on a per-thread basis, and are the * conf*-syscalls even necessary? */ size_t __thread_stack_size = SZ_2M; -size_t __call_stack_size = SZ_2M; size_t __rpc_stack_size = SZ_512K; /** IDs for configuration parameters. */ /** @todo should probably be moved somewhere so it can be shared with userspace */ enum conf_param { CONF_THREAD_STACK = 0, - CONF_CALL_STACK, CONF_RPC_STACK, }; @@ -49,10 +47,6 @@ SYSCALL_DEFINE1(conf_get)(struct tcb *t, sys_arg_t param) val = __thread_stack_size; break; - case CONF_CALL_STACK: - val = __call_stack_size; - break; - case CONF_RPC_STACK: val = __rpc_stack_size; break; @@ -85,17 +79,9 @@ SYSCALL_DEFINE2(conf_set)(struct tcb *t, sys_arg_t param, sys_arg_t val) __thread_stack_size = align_up(val, BASE_PAGE_SIZE); break; - case CONF_CALL_STACK: - size = align_up(val, BASE_PAGE_SIZE); - if (size < __rpc_stack_size) - return_args(t, SYS_RET1(ERR_MISC)); - - __call_stack_size = size; - break; - case CONF_RPC_STACK: size = align_up(val, BASE_PAGE_SIZE); - if (size > __call_stack_size) + if (size > max_rpc_size()) return_args1(t, ERR_MISC); __rpc_stack_size = size; diff --git a/common/uapi/ipc.c b/common/uapi/ipc.c index f9b2817..13682c7 100644 --- a/common/uapi/ipc.c +++ b/common/uapi/ipc.c @@ -11,43 +11,6 @@ #include #include -/** - * Mark rpc stack between \p start and \p end inaccessible. - * - * @param t Thread whose rpc stack to modify. - * @param start Start address of rpc stack to mark inaccessible. - * @param end End address of rpc stack to mark inaccessible. - */ -static void mark_rpc_inaccessible(struct tcb *t, vm_t start, vm_t end) -{ - size_t page_size = BASE_PAGE_SIZE; - size_t size = end - start; - size_t pages = size / page_size; - while (pages--) - clear_vpage_flags(t->rpc.vmem, start + pages * page_size, VM_U); -} - -/** - * Mark rpc stack between \p start and \p end accessible. - * - * @param t Thread whose rpc stack to modify. - * @param start Start address of rpc stack to mark accessible. - * @param end End address of rpc stack to mark accessible. - */ -static void mark_rpc_accessible(struct tcb *t, vm_t start, vm_t end) -{ - /** @todo this could still be optimized with arch-specific stuff I would - * imagine, as set_vpage_flags() has to traverse the whole tree for each - * page to mark. Instead it should be possible to mark the pages - * continuously once they've been traversed once. Or maybe even keep - * around a pointer to where the last stack left off? */ - size_t page_size = BASE_PAGE_SIZE; - size_t size = end - start; - size_t pages = size / page_size; - while (pages--) - set_vpage_flags(t->rpc.vmem, start + pages * page_size, VM_U); -} - /** Structure for maintaining the required context data for an rpc call. */ struct call_ctx { /** Execution continuation point. */ @@ -93,7 +56,7 @@ enum ipc_kind { * @param r Process to migrate to. * @param sd RPC stack regions to mark inaccessible. */ -static void finalize_rpc(struct tcb *t, struct tcb *r, struct stack_diff sd) +static void finalize_rpc(struct tcb *t, struct tcb *r, vm_t s) { clone_uvmem(r->proc.vmem, t->rpc.vmem); set_return(t, r->callback); @@ -101,7 +64,7 @@ static void finalize_rpc(struct tcb *t, struct tcb *r, struct stack_diff sd) t->pid = r->rid; /* make sure updates are visible when swapping to the new virtual memory */ - mark_rpc_inaccessible(t, sd.start, sd.end); + mark_rpc_invalid(t, s); use_vmem(t->rpc.vmem); } @@ -115,18 +78,10 @@ static void finalize_rpc(struct tcb *t, struct tcb *r, struct stack_diff sd) * @param kind Kind of IPC we're doing. Essentially toggles kick boolean. * @return RPC stack difference that should be passed to finalize_rpc(). */ -static struct stack_diff enter_rpc(struct tcb *t, struct sys_ret a, +static vm_t enter_rpc(struct tcb *t, struct sys_ret a, enum ipc_kind kind) { - vm_t rpc_stack = t->rpc_stack; - if (is_rpc(t)) - /** @todo what if user uses their own stack? Or is a dick and - * sets the stack pointer to RPC_STACK_TOP or something? It'll - * likely only cause a fuckup in the process who did the dumb - * thing, so maybe just consider it user error? Except by - * causing the stack of the next rpc to run out of memory... */ - rpc_stack = align_down(get_stack(t), BASE_PAGE_SIZE); - + vm_t rpc_stack = rpc_position(t); struct call_ctx *ctx = (struct call_ctx *)(rpc_stack) - 1; ctx->regs = t->regs; @@ -148,7 +103,6 @@ static struct stack_diff enter_rpc(struct tcb *t, struct sys_ret a, * return a status? except it shouldn't happen after we've run * enough_rpc_stack(). */ vm_t new_stack = rpc_stack - BASE_PAGE_SIZE; - struct stack_diff sd = {new_stack, t->rpc_stack}; /** @todo what if each stack is only some number of pages, and if a proc * goes over the limit is is seen as programming error? Possibly user @@ -162,7 +116,7 @@ static struct stack_diff enter_rpc(struct tcb *t, struct sys_ret a, * */ t->rpc_stack = new_stack; set_stack(t, new_stack); - return sd; + return new_stack; } /** @@ -190,7 +144,7 @@ static void leave_rpc(struct tcb *t, struct sys_ret a) set_return(t, ctx->exec); /* if we're returning from a failed rpc, this should essentially be a * no-op */ - mark_rpc_accessible(t, t->rpc_stack, top); + mark_rpc_valid(t, top); t->rpc_stack = ctx->rpc_stack; t->pid = ctx->pid; t->eid = ctx->eid; @@ -211,13 +165,11 @@ static void leave_rpc(struct tcb *t, struct sys_ret a) static bool enough_rpc_stack(struct tcb *t) { /* get top of call stack */ - vm_t top = RPC_STACK_BASE + __call_stack_size; - /* get start of the next rpc stack instance */ - vm_t rpc_stack = t->rpc_stack + BASE_PAGE_SIZE; + vm_t top = rpc_position(t); /* if we can still fit an rpc stack into the call stack, we can safely * do the migration. */ - return top - rpc_stack >= __rpc_stack_size; + return (top - BASE_PAGE_SIZE) >= (t->rpc_stack - __rpc_stack_size); } /** @@ -263,8 +215,7 @@ static void do_ipc(struct tcb *t, if (unlikely(!enough_rpc_stack(t))) return_args1(t, ERR_OOMEM); - struct stack_diff sd = - enter_rpc(t, SYS_RET6(OK, t->eid, d0, d1, d2, d3), kind); + vm_t s = enter_rpc(t, SYS_RET6(OK, t->eid, d0, d1, d2, d3), kind); struct tcb *r = get_tcb(pid); if (unlikely(!r)) { @@ -282,7 +233,7 @@ static void do_ipc(struct tcb *t, if (kind != IPC_REQ) t->eid = t->pid; - finalize_rpc(t, r, sd); + finalize_rpc(t, r, s); /* I tested out passing the return values as arguments to * ret_userspace_fast, but apparently that causes enough stack shuffling * to be slower overall. */ diff --git a/include/arch/tcb.h b/include/arch/tcb.h index f4c80c6..af12e0b 100644 --- a/include/arch/tcb.h +++ b/include/arch/tcb.h @@ -25,4 +25,11 @@ */ void tcb_assign(struct tcb *t); +void setup_rpc_stack(struct tcb *t); +size_t max_rpc_size(); + +vm_t rpc_position(struct tcb *t); +void mark_rpc_valid(struct tcb *t, vm_t top); +void mark_rpc_invalid(struct tcb *t, vm_t bottom); + #endif /* KMI_ARCH_TCB_H */ diff --git a/include/kmi/tcb.h b/include/kmi/tcb.h index 4f2eb1b..ba71dd3 100644 --- a/include/kmi/tcb.h +++ b/include/kmi/tcb.h @@ -99,7 +99,7 @@ struct tcb { vm_t regs; /** Arch-specific data. */ - struct arch_tcbd tcbd; + struct arch_tcbd arch; /** Memory mapping data. */ struct mem_region_root sp_r; -- cgit v1.3