From cd41ed1758f4842d7b78870385e16b579cc89c38 Mon Sep 17 00:00:00 2001 From: Kimplul Date: Sun, 25 Jan 2026 17:14:53 +0200 Subject: implement dmem semi-properly + Add tests and fix virtual mapping, this is enough to start writing proper userspace drivers --- include/kmi/dmem.h | 23 ++++++++ include/kmi/mem.h | 3 + include/kmi/syscalls.h | 4 +- lib/fdt.c | 6 ++ src/dmem.c | 77 +++++++++++++++++++------- src/fdt.c | 15 ----- src/uapi/dispatch.c | 2 +- src/uapi/mem.c | 4 +- src/vmem.c | 9 +++ tests/common/sys.h | 4 +- tests/dmem/init.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/dmem/source.mk | 1 + tests/scripts/makefile | 20 +++++-- 13 files changed, 269 insertions(+), 45 deletions(-) delete mode 100644 src/fdt.c create mode 100644 tests/dmem/init.c create mode 100644 tests/dmem/source.mk diff --git a/include/kmi/dmem.h b/include/kmi/dmem.h index 697e55a..ec3e330 100644 --- a/include/kmi/dmem.h +++ b/include/kmi/dmem.h @@ -41,10 +41,33 @@ vm_t alloc_devmem(struct tcb *p, pm_t dev_start, size_t bytes, vmflags_t flags); /** * Free direct device mapping. * + * Frees both the uvmem and the underlying physical + * memory, can be called from syscall handler for example. + * * @param p Process to free mapping from. * @param dev_start Start of allocation. * @return \ref OK when succesful, \c ERR_NF when mapping not found. */ stat_t free_devmem(struct tcb *p, vm_t dev_start); +/** + * Free direct device mapping when the memory region is known. + * + * Arguably this means that the responsibilities of uvmem/vmem/dmem are not as + * nicely defined as I would like and they all kind of step on eachother's toes + * at the moment but good enough for right now, allows starting to write drivers in + * userspace. + * + * Note that at least currently this function has some strong assumptions about + * when it can be called, namely that the process must own the region and that + * there can be no external references to it. In other words, should only be + * called when the underlying physical memory should be freed. + * Does NOT free the uvmem of the region, only the underlying physical memory. + * + * @param p Process to free mapping from. + * @param m Memory region previously known to be a device memory mapping. + * @return \ref OK when succesful, \c ERR_INVAL otherwise. + */ +stat_t free_known_devmem(struct tcb *p, struct mem_region *m); + #endif /* KMI_DEV_H */ diff --git a/include/kmi/mem.h b/include/kmi/mem.h index 8c66eb5..129ae5e 100644 --- a/include/kmi/mem.h +++ b/include/kmi/mem.h @@ -188,6 +188,9 @@ size_t page_shift(); /** Memory region is private but not backed by memory. */ #define MR_NONBACKED (1 << (ARCH_VP_FLAGS + 3)) +/** Memory region in device memory region */ +#define MR_DEV (1 << (ARCH_VP_FLAGS + 4)) + /** @} */ /** diff --git a/include/kmi/syscalls.h b/include/kmi/syscalls.h index 30e46be..8921343 100644 --- a/include/kmi/syscalls.h +++ b/include/kmi/syscalls.h @@ -47,8 +47,8 @@ enum sys_code { /** Request memory from anywhere. */ SYS_REQ_MEM, - /** Request memory with physical address. */ - SYS_REQ_PMEM, + /** Request device memory, i.e. some physical address outside RAM. */ + SYS_REQ_DMEM, /** Request memory at fixed virtual address. */ SYS_REQ_FIXMEM, diff --git a/lib/fdt.c b/lib/fdt.c index cc39527..259e8dd 100644 --- a/lib/fdt.c +++ b/lib/fdt.c @@ -8,3 +8,9 @@ #include "libfdt_env.h" #include "../dtc/libfdt/fdt.c" + +struct cell_info get_cellinfo(const void *fdt, const int offset) +{ + return (struct cell_info){ fdt_size_cells(fdt, offset), + fdt_address_cells(fdt, offset) }; +} diff --git a/src/dmem.c b/src/dmem.c index d21a9c5..6b98003 100644 --- a/src/dmem.c +++ b/src/dmem.c @@ -19,13 +19,8 @@ static struct mem_region_root post_ram = { 0 }; stat_t init_devmem(pm_t ram_base, pm_t ram_top) { - size_t pre_pages = __pages(ram_base); - size_t ram_pages = __pages(ram_top - ram_base); - /* -1 being the effective highest address possible */ - size_t post_pages = __pages(-1) - ram_pages - pre_pages; - - init_region(&pre_ram, 0, pre_pages, 0); - init_region(&post_ram, ram_top, post_pages, 0); + init_region(&pre_ram, 0, ram_base, 0); + init_region(&post_ram, ram_top, -1, 0); return OK; } @@ -54,39 +49,83 @@ vm_t alloc_devmem(struct tcb *t, pm_t start, size_t bytes, vmflags_t flags) if (!region) return NULL; - vm_t v = alloc_region(region, bytes, &bytes, flags); - if (!v) + /* carve out physical region (vm_t is misleading, this region is + * physical memory) */ + vm_t p = alloc_fixed_region(region, start, bytes, &bytes, flags); + if (!p) + return NULL; + + /* carve out virtual region */ + vm_t v = alloc_region(&t->uvmem.region, bytes, &bytes, flags | MR_DEV); + if (!v) { + free_region(region, p); return NULL; + } - if (map_fixed_region(t->proc.vmem, v, start, bytes, flags)) { + /* convert from literal physical address to direct mapping address, + * which map_* expects (kind of annoying to deal with, admittedly, is + * there maybe a better way to handle these things?) */ + p = (pm_t)__va(p); + if (map_fixed_region(t->proc.vmem, v, p, bytes, flags)) { unmap_region(t->proc.vmem, v, bytes); - free_region(region, v); + free_region(&t->uvmem.region, v); + free_region(region, p); return NULL; } return v; } -stat_t free_devmem(struct tcb *t, vm_t start) +stat_t free_known_devmem(struct tcb *t, struct mem_region *m) { assert(t && is_proc(t)); + + /* ensure region is a device memory region and is currently not shared */ + assert(is_set(m->flags, MR_DEV)); + assert(m->pid != 0 || m->refcount == 1); + pm_t addr = 0; - stat_vpage(t->proc.vmem, start, &addr, NULL, NULL); + stat_vpage(t->proc.vmem, __addr(m->start), &addr, NULL, NULL); - struct mem_region_root *region = __select_region((pm_t)__pa(addr)); + /* free physical device memory */ + pm_t p = (pm_t)__pa(addr); + struct mem_region_root *region = __select_region(p); if (!region) return ERR_INVAL; + free_region(region, p); - struct mem_region *m = find_used_region(region, start); - if (!m) - return ERR_NF; - + /* free virtual allocation associated with device mem */ vm_t base = __addr(m->start); vm_t end = __addr(m->end); + + /* free virtual memory associated with physical mapping */ size_t size = end - base; unmap_fixed_region(t->proc.vmem, base, size); - free_region(region, base); + free_known_region(&t->uvmem.region, m); + return OK; +} + +stat_t free_devmem(struct tcb *t, vm_t start) +{ + assert(t && is_proc(t)); + struct mem_region *m = find_used_region(&t->uvmem.region, start); + if (!m) + return ERR_NF; + + /* not a devmem region */ + if (!is_set(m->flags, MR_DEV)) + return ERR_INVAL; + + /* mapping is shared, can't free right now */ + if (m->pid == 0 && m->refcount > 1) + return ERR_INVAL; + + stat_t ret = free_known_devmem(t, m); + if (ret) + return ret; + + free_known_region(&t->uvmem.region, m); return OK; } diff --git a/src/fdt.c b/src/fdt.c deleted file mode 100644 index 677be7b..0000000 --- a/src/fdt.c +++ /dev/null @@ -1,15 +0,0 @@ -/* SPDX-License-Identifier: copyleft-next-0.3.1 */ -/* Copyright 2021 - 2022, Kim Kuparinen < kimi.h.kuparinen@gmail.com > */ - -/** - * @file fdt.c - * Helper functions for handling the global FDT. - */ - -#include - -struct cell_info get_cellinfo(const void *fdt, const int offset) -{ - return (struct cell_info){ fdt_size_cells(fdt, offset), - fdt_address_cells(fdt, offset) }; -} diff --git a/src/uapi/dispatch.c b/src/uapi/dispatch.c index 2eef5bc..4d10bbe 100644 --- a/src/uapi/dispatch.c +++ b/src/uapi/dispatch.c @@ -49,7 +49,7 @@ void handle_syscall(sys_arg_t syscall, sys_arg_t a, sys_arg_t b, case SYS_NOOP: sys_noop(t, a, b, c, d, e); break; case SYS_PUTCH: sys_putch(t, a, b, c, d, e); break; case SYS_REQ_MEM: sys_req_mem(t, a, b, c, d, e); break; - case SYS_REQ_PMEM: sys_req_pmem(t, a, b, c, d, e); break; + case SYS_REQ_DMEM: sys_req_dmem(t, a, b, c, d, e); break; case SYS_REQ_PAGE: sys_req_page(t, a, b, c, d, e); break; case SYS_REQ_FIXMEM: sys_req_fixmem(t, a, b, c, d, e); break; case SYS_REQ_SHAREDMEM: sys_req_sharedmem(t, a, b, c, d, e); break; diff --git a/src/uapi/mem.c b/src/uapi/mem.c index 7a233a5..20abc96 100644 --- a/src/uapi/mem.c +++ b/src/uapi/mem.c @@ -82,7 +82,7 @@ SYSCALL_DEFINE1(free_mem)(struct tcb *t, sys_arg_t start) } /** - * Request physical memory syscall handler. + * Request device memory syscall handler. * * @param t Current tcb. * @param paddr Physical address to map. @@ -91,7 +91,7 @@ SYSCALL_DEFINE1(free_mem)(struct tcb *t, sys_arg_t start) * @return \ref OK and start of allocation when succesful, * otherwise an error code. */ -SYSCALL_DEFINE3(req_pmem)(struct tcb *t, sys_arg_t paddr, sys_arg_t size, +SYSCALL_DEFINE3(req_dmem)(struct tcb *t, sys_arg_t paddr, sys_arg_t size, sys_arg_t flags) { /* this will require some pondering, but essentially this syscall should diff --git a/src/vmem.c b/src/vmem.c index 76e0ece..afdac95 100644 --- a/src/vmem.c +++ b/src/vmem.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -197,6 +198,14 @@ static void __free_mapping(struct tcb *t, struct mem_region *m) if (is_set(m->flags, MR_NONBACKED)) return; + if (is_set(m->flags, MR_DEV)) { + stat_t ret = free_known_devmem(t, m); + MAYBE_UNUSED(ret); + + assert(ret == OK); + return; + } + pm_t start = __addr(m->start); pm_t end = __addr(m->end); size_t size = end - start; diff --git a/tests/common/sys.h b/tests/common/sys.h index 0bee8bd..60de7b0 100644 --- a/tests/common/sys.h +++ b/tests/common/sys.h @@ -44,9 +44,9 @@ static inline void *sys_req_fixmem(uintptr_t fixed, size_t size, vmflags_t flags return (void *)r.a0; } -static inline void *sys_req_pmem(uintptr_t addr, size_t size, vmflags_t flags) +static inline void *sys_req_dmem(uintptr_t addr, size_t size, vmflags_t flags) { - struct sys_ret r = syscall3(SYS_REQ_PMEM, addr, size, flags); + struct sys_ret r = syscall3(SYS_REQ_DMEM, addr, size, flags); if (r.s) return NULL; diff --git a/tests/dmem/init.c b/tests/dmem/init.c new file mode 100644 index 0000000..c5d13f4 --- /dev/null +++ b/tests/dmem/init.c @@ -0,0 +1,146 @@ +#include +#include +#include + +/* important bits taken from src/debug.c */ +#define UART_8250_DATA 0 +#define UART_8250_LSR 5 +#define LSR_THRE (1 << 5) + +static bool __tx_empty(char *ptr, size_t shift) +{ + volatile char *lsr = ptr + (UART_8250_LSR << shift); + return (*lsr) & LSR_THRE; +} + +static void __print_char(char *ptr, size_t shift, char c) +{ + while (__tx_empty(ptr, shift) == 0) + ; + + volatile char *data = ptr + (UART_8250_DATA << shift); + *data = c; +} + +static void __print_ok(char *ptr, size_t shift) +{ + /* theoretically the kernel might jump in and print something itself, + * but for now let's ignore that possibility. In the future, we might + * want to move printing completely to userspace to avoid that issue? */ + __print_char(ptr, shift, 'O'); + __print_char(ptr, shift, 'K'); + __print_char(ptr, shift, '\n'); +} + +static bool __supported(const char *dev_name) +{ + /* qemu, for example */ + if (strncmp("ns16550", dev_name, 7) == 0) + return true; + + /* mentioned in dtc documentation as an example */ + if (strncmp("ns8250", dev_name, 7) == 0) + return true; + + /* starfive visionfive 2, for example (hopefully works) */ + if (strncmp("snps,dw-apb-uart", dev_name, 16) == 0) + return true; + + return false; +} + +START(pid, tid, d0, d1, d2, d3) +{ + UNUSED(d2); + UNUSED(d3); + + /* this starting bit is more or less directly shared with the fdt test, + * but that can be seen as a requirement for the dmem setup to work I + * guess? */ + check(pid == 0, "wrong src process ID"); + check(tid == 1, "wrong root thread ID"); + check(d0 == SYS_USER_SPAWNED, "wrong op"); + printf("fdt at %lx\n", d1); + + void *fdt = (void *)d1; + + int chosen_offset = fdt_path_offset(fdt, "/chosen"); + check(chosen_offset > 0, "couldn't find /chosen"); + + const char *stdout = + fdt_getprop(fdt, chosen_offset, "stdout-path", NULL); + + check(stdout != NULL, "couldn't find stdout-path"); + printf("stdout at %s\n", stdout); + + /* discard options */ + size_t baselen = strlen(stdout); + const char *options = strchr(stdout, ':'); + if (options) + baselen = options - stdout; + + int stdout_offset = fdt_path_offset_namelen(fdt, stdout, baselen); + check(stdout_offset > 0, "couldn't find %s offset", stdout); + + /* get serial device type */ + const char *dev_name = (const char *)fdt_getprop(fdt, stdout_offset, + "compatible", NULL); + check(dev_name != NULL, "couldn't get %s devicename", stdout); + printf("devname %s\n", dev_name); + + check(__supported(dev_name), "unsupported device: %s", dev_name); + + /* get serial device address */ + const void *reg_ptr = fdt_getprop(fdt, stdout_offset, "reg", NULL); + check(reg_ptr != NULL, "couldn't get 'reg' property"); + + struct cell_info ci = get_cellinfo(fdt, stdout_offset); + pm_t dbg_ptr = (pm_t)fdt_load_reg_addr(ci, reg_ptr, 0); + printf("uart reg at %lx\n", dbg_ptr); + + /* get serial device offset if present */ + size_t shift = 0; + const void *shift_ptr = fdt_getprop(fdt, stdout_offset, "reg-shift", + NULL); + + if (shift_ptr) + shift = (size_t)fdt_load_int32_ptr(shift_ptr); + + printf("shift: %zu\n", shift); + + /* get size of zeroth page order, as all physical memory mappings are + * aligned to them and we must take care to align things ourselves */ + size_t page_size = sys_conf_get(CONF_PAGE_SIZE, 0); + printf("page size: %zu\n", page_size); + + size_t offset = dbg_ptr - align_down(dbg_ptr, page_size); + printf("offset: %zu\n", offset); + + /* map to userspace, size is not really accurate here but definitely + * large enough to handle the 8250 */ + void *ptr = sys_req_dmem(dbg_ptr, 1024, VM_R | VM_W); + check(ptr != NULL, "failed mapping device memory"); + + printf("user mapping at %p + %zu\n", ptr, offset); + + /* check that we can't allocate the same region twice */ + void *nptr = sys_req_dmem(dbg_ptr, 1024, VM_R | VM_W); + check(nptr == NULL, "allocated same devmem twice"); + + /* check that we can free the device mem */ + enum sys_status ok = sys_free_mem((uintptr_t)ptr); + check(ok == OK, "failed freeing devmem"); + + /* try to map the memory again now that it's free */ + ptr = sys_req_dmem(dbg_ptr, 1024, VM_R | VM_W); + check(ptr != NULL, "failed reallocating devmem"); + + /* try printing */ + __print_ok((char *)ptr + offset, shift); + + /** @todo check that a fork/thread creation handled the devmem + * references correctly */ + + /* don't run off the end of the _start */ + sys_poweroff(SYS_SHUTDOWN); +} diff --git a/tests/dmem/source.mk b/tests/dmem/source.mk new file mode 100644 index 0000000..dbada55 --- /dev/null +++ b/tests/dmem/source.mk @@ -0,0 +1 @@ +TESTS += dmem diff --git a/tests/scripts/makefile b/tests/scripts/makefile index 1ba7615..06e85da 100644 --- a/tests/scripts/makefile +++ b/tests/scripts/makefile @@ -32,10 +32,13 @@ KMI := ../kmi.bin # common tools available for tests, printf/fdt/initrd handling and common memory # ops like memset and memcpy. Note that printf uses in-kernel printing at least # for now, so testsuite must be run on kernels compiled with -DDEBUG=1 -COMMON := build/printf.o \ - build/string.o \ - build/fdt.o \ - build/fdt_ro.o \ +COMMON := build/printf.o \ + build/string.o \ + build/fdt.o \ + build/fdt_ro.o \ + build/fdt_rw.o \ + build/fdt_wip.o \ + build/fdt_addresses.o \ build/bits.o # reuse some common parts from source tree. Could maybe split these into a @@ -50,6 +53,15 @@ build/fdt.o: ../lib/fdt.c $(KMI) build/fdt_ro.o: ../lib/fdt_ro.c $(KMI) $(COMPILE_TEST) -c ../lib/fdt_ro.c -o build/fdt_ro.o +build/fdt_rw.o: ../lib/fdt_rw.c $(KMI) + $(COMPILE_TEST) -c ../lib/fdt_rw.c -o build/fdt_rw.o + +build/fdt_wip.o: ../lib/fdt_wip.c $(KMI) + $(COMPILE_TEST) -c ../lib/fdt_wip.c -o build/fdt_wip.o + +build/fdt_addresses.o: ../lib/fdt_addresses.c $(KMI) + $(COMPILE_TEST) -c ../lib/fdt_addresses.c -o build/fdt_addresses.o + build/printf.o: common/printf.c $(KMI) $(COMPILE_TEST) -c common/printf.c -o build/printf.o -- cgit v1.3