From 8c67562090e792aa9a1852d45ddc494c894281bf Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Wed, 4 Oct 2023 12:19:16 +0300 Subject: [PATCH 01/10] [REVERTME] riscv_addrenc.c: Add more heap, if TLS_ALIGNED is set The TLS alignment requires more room in the stack, which means more _initial_ heap is required to accomodate the stack. Why 2x TLS_MAXSTACK ? No idea. This is a temporary fix, like the +1 page extra above. --- arch/risc-v/src/common/riscv_addrenv.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/risc-v/src/common/riscv_addrenv.c b/arch/risc-v/src/common/riscv_addrenv.c index cb49c1a51daf0..dfdcfb6aea2fb 100644 --- a/arch/risc-v/src/common/riscv_addrenv.c +++ b/arch/risc-v/src/common/riscv_addrenv.c @@ -65,6 +65,7 @@ #include #include #include +#include #include @@ -421,6 +422,12 @@ int up_addrenv_create(size_t textsize, size_t datasize, size_t heapsize, heapsize = heapsize + MM_PGALIGNUP(CONFIG_DEFAULT_TASK_STACKSIZE); +#ifdef CONFIG_TLS_ALIGNED + /* Need more stack for TLS alignment */ + + heapsize += MM_PGALIGNUP(2 * TLS_MAXSTACK); +#endif + /* Map the reserved area */ ret = create_region(addrenv, resvbase, resvsize, MMU_UDATA_FLAGS); From e6080b2ac4bcb628df00f455e754f9f4f6d310e2 Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Wed, 4 Oct 2023 15:54:49 +0300 Subject: [PATCH 02/10] riscv_addrenv_utils.c: Determine page table flags by type of vaddr Use kernel page table flags if the mapped virtual address is in kernel space. --- arch/risc-v/src/common/riscv_addrenv_utils.c | 24 ++++++++++++++++++-- arch/risc-v/src/common/riscv_mmu.h | 4 ++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/arch/risc-v/src/common/riscv_addrenv_utils.c b/arch/risc-v/src/common/riscv_addrenv_utils.c index 810563aedd47a..e13fb89ff9e64 100644 --- a/arch/risc-v/src/common/riscv_addrenv_utils.c +++ b/arch/risc-v/src/common/riscv_addrenv_utils.c @@ -63,13 +63,22 @@ uintptr_t riscv_get_pgtable(arch_addrenv_t *addrenv, uintptr_t vaddr) uintptr_t paddr; uintptr_t ptprev; uint32_t ptlevel; + uint32_t flags; /* Get the current level MAX_LEVELS-1 entry corresponding to this vaddr */ ptlevel = ARCH_SPGTS; ptprev = riscv_pgvaddr(addrenv->spgtables[ARCH_SPGTS - 1]); - paddr = mmu_pte_to_paddr(mmu_ln_getentry(ptlevel, ptprev, vaddr)); + if (!ptprev) + { + /* Something is very wrong */ + + return 0; + } + + /* Find the physical address of the final level page table */ + paddr = mmu_pte_to_paddr(mmu_ln_getentry(ptlevel, ptprev, vaddr)); if (!paddr) { /* No page table has been allocated... allocate one now */ @@ -77,10 +86,21 @@ uintptr_t riscv_get_pgtable(arch_addrenv_t *addrenv, uintptr_t vaddr) paddr = mm_pgalloc(1); if (paddr) { + /* Determine page table flags */ + + if (riscv_uservaddr(vaddr)) + { + flags = MMU_UPGT_FLAGS; + } + else + { + flags = MMU_KPGT_FLAGS; + } + /* Wipe the page and assign it */ riscv_pgwipe(paddr); - mmu_ln_setentry(ptlevel, ptprev, paddr, vaddr, MMU_UPGT_FLAGS); + mmu_ln_setentry(ptlevel, ptprev, paddr, vaddr, flags); } } diff --git a/arch/risc-v/src/common/riscv_mmu.h b/arch/risc-v/src/common/riscv_mmu.h index 0c0c6b5373749..6c9d1baebb997 100644 --- a/arch/risc-v/src/common/riscv_mmu.h +++ b/arch/risc-v/src/common/riscv_mmu.h @@ -59,6 +59,10 @@ #define MMU_IO_FLAGS (PTE_R | PTE_W | PTE_G) +/* Flags for kernel page tables */ + +#define MMU_KPGT_FLAGS (PTE_G) + /* Kernel FLASH and RAM are mapped globally */ #define MMU_KTEXT_FLAGS (PTE_R | PTE_X | PTE_G) From 78781f96fd8e8e4436aa0ae3dd0b2e5ad24912cb Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Wed, 4 Oct 2023 15:57:29 +0300 Subject: [PATCH 03/10] riscv-v/pgalloc.h: Return kernel vaddr for kernel RAM paddr All kernel memory is mapped paddr=vaddr, so it is trivial to give mapping for kernel memory. Only interesting region should be kernel RAM, so omit kernel ROM and don't allow re-mapping it. --- arch/risc-v/src/common/pgalloc.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/risc-v/src/common/pgalloc.h b/arch/risc-v/src/common/pgalloc.h index 40da29805660b..be39da0d3a0d5 100644 --- a/arch/risc-v/src/common/pgalloc.h +++ b/arch/risc-v/src/common/pgalloc.h @@ -75,6 +75,10 @@ static inline uintptr_t riscv_pgvaddr(uintptr_t paddr) { return paddr - CONFIG_ARCH_PGPOOL_PBASE + CONFIG_ARCH_PGPOOL_VBASE; } + else if (paddr >= CONFIG_RAM_START && paddr < CONFIG_RAM_END) + { + return paddr - CONFIG_RAM_START + CONFIG_RAM_VSTART; + } return 0; } From 9015dcb7e12e3a5384fcb5c68c8a9fa046ac443e Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Mon, 2 Oct 2023 17:36:18 +0300 Subject: [PATCH 04/10] sched/assert.c: Print process name in assert dump In addition to printing out the thread name (task name in flat mode), print the parent process's name as well. It is quite useful to know which process is the parent of a faulting thread, although this information can be read from the assert dump, in some cases the dump might be incomplete (due to e.g. stack corruption, which causes another exception and PANIC().) --- sched/misc/assert.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/sched/misc/assert.c b/sched/misc/assert.c index 1cbe45aef76b5..95ec7ce5a428a 100644 --- a/sched/misc/assert.c +++ b/sched/misc/assert.c @@ -553,11 +553,21 @@ void _assert(FAR const char *filename, int linenum, FAR const char *msg, FAR void *regs) { FAR struct tcb_s *rtcb = running_task(); +#if CONFIG_TASK_NAME_SIZE > 0 + FAR struct tcb_s *ptcb = NULL; +#endif struct panic_notifier_s notifier_data; struct utsname name; bool fatal = true; int flags; +#if CONFIG_TASK_NAME_SIZE > 0 + if (rtcb->group && !(rtcb->flags & TCB_FLAG_TTYPE_KERNEL)) + { + ptcb = nxsched_get_tcb(rtcb->group->tg_pid); + } +#endif + flags = enter_critical_section(); sched_lock(); @@ -602,6 +612,7 @@ void _assert(FAR const char *filename, int linenum, ": " #if CONFIG_TASK_NAME_SIZE > 0 "%s " + "process: %s " #endif "%p\n", msg ? msg : "", @@ -611,6 +622,7 @@ void _assert(FAR const char *filename, int linenum, #endif #if CONFIG_TASK_NAME_SIZE > 0 rtcb->name, + ptcb ? ptcb->name : "Kernel", #endif rtcb->entry.main); From 15aaaaa7703396712d75539723e167f1b23d5e36 Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Tue, 3 Oct 2023 12:51:36 +0300 Subject: [PATCH 05/10] kmm_map.c: Fix call to gran_alloc Provide the handle to gran_alloc, not pointer to handle. --- mm/kmap/kmm_map.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/kmap/kmm_map.c b/mm/kmap/kmm_map.c index 1312e6af0e31c..d6d1c6c9fa656 100644 --- a/mm/kmap/kmm_map.c +++ b/mm/kmap/kmm_map.c @@ -125,7 +125,7 @@ static FAR void *map_pages(FAR void **pages, size_t npages, int prot) /* Find a virtual memory area that fits */ - vaddr = gran_alloc(&g_kmm_map_vpages, size); + vaddr = gran_alloc(g_kmm_map_vpages, size); if (!vaddr) { return NULL; From f41e79226b7389e0f347a6a42279464e7f16a85b Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Tue, 3 Oct 2023 12:53:32 +0300 Subject: [PATCH 06/10] kmm_map.c: Fix user page mapping User pages are mapped from the currently active address environment. If the process is running on a borrowed address environment, then the mapping should be created from there. This happens during (new) process creation only. --- mm/kmap/kmm_map.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/kmap/kmm_map.c b/mm/kmap/kmm_map.c index d6d1c6c9fa656..4358ae57a6296 100644 --- a/mm/kmap/kmm_map.c +++ b/mm/kmap/kmm_map.c @@ -82,7 +82,7 @@ static int get_user_pages(FAR void **pages, size_t npages, uintptr_t vaddr) for (i = 0; i < npages; i++, vaddr += MM_PGSIZE) { - page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr); + page = up_addrenv_find_page(&tcb->addrenv_curr->addrenv, vaddr); if (!page) { /* Something went wrong, get out */ @@ -182,7 +182,7 @@ static FAR void *map_single_user_page(uintptr_t vaddr) /* Find the page associated with this virtual address */ - page = up_addrenv_find_page(&tcb->addrenv_own->addrenv, vaddr); + page = up_addrenv_find_page(&tcb->addrenv_curr->addrenv, vaddr); if (!page) { return NULL; From 5c2c4435f363b8b54ef1efe8958e69d79bf630ce Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Tue, 3 Oct 2023 12:55:24 +0300 Subject: [PATCH 07/10] kmm_map: Fix incorrect function name field --- arch/risc-v/src/common/riscv_addrenv_pgmap.c | 2 +- include/nuttx/arch.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/risc-v/src/common/riscv_addrenv_pgmap.c b/arch/risc-v/src/common/riscv_addrenv_pgmap.c index 9dbac8592fecc..edf9687649105 100644 --- a/arch/risc-v/src/common/riscv_addrenv_pgmap.c +++ b/arch/risc-v/src/common/riscv_addrenv_pgmap.c @@ -202,7 +202,7 @@ int up_addrenv_kmap_pages(void **pages, unsigned int npages, uintptr_t vaddr, } /**************************************************************************** - * Name: riscv_unmap_pages + * Name: up_addrenv_kunmap_pages * * Description: * Unmap a previously mapped virtual memory region. diff --git a/include/nuttx/arch.h b/include/nuttx/arch.h index 4a647afedb2af..9f4bdf78221ad 100644 --- a/include/nuttx/arch.h +++ b/include/nuttx/arch.h @@ -1352,7 +1352,7 @@ int up_addrenv_kmap_pages(FAR void **pages, unsigned int npages, #endif /**************************************************************************** - * Name: riscv_unmap_pages + * Name: up_addrenv_kunmap_pages * * Description: * Unmap a previously mapped virtual memory region. From 44dbd3a6d5ee56333790d7391c3d82f0408555e9 Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Tue, 3 Oct 2023 13:33:41 +0300 Subject: [PATCH 08/10] kmm_map.c: Add way to test if addr is within kmap area or not is_kmap_vaddr is added and used to test that a given (v)addr is actually inside the kernel map area. This gives a speed optimization for kmm_unmap, as it is no longer necessary to take the mm_map_lock to check if such a mapping exists; obviously if the address is not within the kmap area, it won't be in the list either. --- mm/kmap/kmm_map.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/mm/kmap/kmm_map.c b/mm/kmap/kmm_map.c index 4358ae57a6296..1deb0f25cef1b 100644 --- a/mm/kmap/kmm_map.c +++ b/mm/kmap/kmm_map.c @@ -192,6 +192,26 @@ static FAR void *map_single_user_page(uintptr_t vaddr) return (FAR void *)vaddr; } +/**************************************************************************** + * Name: is_kmap_vaddr + * + * Description: + * Return true if the virtual address, vaddr, lies in the kmap address + * space. + * + * Input Parameters: + * vaddr - The kernel virtual address where the mapping begins. + * + * Returned Value: + * True if vaddr is in the kmap address space; false otherwise. + * + ****************************************************************************/ + +static bool is_kmap_vaddr(uintptr_t vaddr) +{ + return (vaddr >= CONFIG_ARCH_KMAP_VBASE && vaddr < ARCH_KMAP_VEND); +} + /**************************************************************************** * Name: kmm_map_lock * @@ -301,6 +321,15 @@ void kmm_unmap(FAR void *kaddr) unsigned int npages; int ret; + /* Speed optimization: check that addr is within kmap area */ + + if (!is_kmap_vaddr((uintptr_t)kaddr)) + { + /* Nope: get out */ + + return; + } + /* Lock the mapping list when we fiddle around with it */ ret = kmm_map_lock(); From 9aaebb388a10c0ed9d082cca5cc5d68022034797 Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Wed, 4 Oct 2023 15:58:15 +0300 Subject: [PATCH 09/10] kmm_map.c: Remember to free the temporary 'pages' variable The temp variable was freed only in error cases, but it needs to be freed in the happy case as well. --- mm/kmap/kmm_map.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mm/kmap/kmm_map.c b/mm/kmap/kmm_map.c index 1deb0f25cef1b..ca94fe3bf05d2 100644 --- a/mm/kmap/kmm_map.c +++ b/mm/kmap/kmm_map.c @@ -377,7 +377,7 @@ void kmm_unmap(FAR void *kaddr) FAR void *kmm_map_user(FAR void *uaddr, size_t size) { - FAR void *pages; + FAR void **pages; uintptr_t vaddr; uintptr_t offset; size_t npages; @@ -414,7 +414,7 @@ FAR void *kmm_map_user(FAR void *uaddr, size_t size) /* No, the area must be mapped into kernel virtual address space */ - pages = kmm_zalloc(npages * sizeof(FAR void *)); + pages = (FAR void **)kmm_zalloc(npages * sizeof(FAR void *)); if (!pages) { return NULL; @@ -422,7 +422,7 @@ FAR void *kmm_map_user(FAR void *uaddr, size_t size) /* Fetch the physical pages for the user virtual address range */ - ret = get_user_pages(&pages, npages, vaddr); + ret = get_user_pages(pages, npages, vaddr); if (ret < 0) { goto errout_with_pages; @@ -430,7 +430,7 @@ FAR void *kmm_map_user(FAR void *uaddr, size_t size) /* Map the physical pages to kernel memory */ - vaddr = (uintptr_t)map_pages(&pages, npages, PROT_READ | PROT_WRITE); + vaddr = (uintptr_t)map_pages(pages, npages, PROT_READ | PROT_WRITE); if (!vaddr) { goto errout_with_pages; @@ -438,6 +438,7 @@ FAR void *kmm_map_user(FAR void *uaddr, size_t size) /* Ok, we have a virtual memory area, add the offset back */ + kmm_free(pages); return (FAR void *)(vaddr + offset); errout_with_pages: From a3a72ea26f0fbfa53c9378b0c0f7361d3906dd4c Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Thu, 5 Oct 2023 15:33:03 +0300 Subject: [PATCH 10/10] kmm_map: Add function to map a single page of kernel memory Mapping a physical page to a kernel virtual page is very simple and does not need the kernel vma list, just get the kernel addressable virtual address for the page. --- mm/kmap/kmm_map.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/mm/kmap/kmm_map.c b/mm/kmap/kmm_map.c index ca94fe3bf05d2..cb270706f137b 100644 --- a/mm/kmap/kmm_map.c +++ b/mm/kmap/kmm_map.c @@ -192,6 +192,27 @@ static FAR void *map_single_user_page(uintptr_t vaddr) return (FAR void *)vaddr; } +/**************************************************************************** + * Name: map_single_page + * + * Description: + * Map (find) a single page from the kernel addressable virtual memory + * pool. + * + * Input Parameters: + * page - The physical page. + * + * Returned Value: + * The kernel virtual address for the page, or NULL if page is not kernel + * addressable. + * + ****************************************************************************/ + +static FAR void *map_single_page(uintptr_t page) +{ + return (FAR void *)up_addrenv_page_vaddr(page); +} + /**************************************************************************** * Name: is_kmap_vaddr * @@ -290,6 +311,13 @@ FAR void *kmm_map(FAR void **pages, size_t npages, int prot) return NULL; } + /* A single page can be addressed directly, if it is a kernel page */ + + if (npages == 1) + { + return map_single_page((uintptr_t)pages[0]); + } + /* Attempt to map the pages */ vaddr = (uintptr_t)map_pages(pages, npages, prot);