This is an automated email from the ASF dual-hosted git repository. acassis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 20c58ca746f14f3c73eedfbf3b43b8a1c268f93c Author: Marco Casaroli <[email protected]> AuthorDate: Sun Aug 9 15:30:57 2026 +0200 arch/risc-v: Implement up_addrenv_fork() and provide POSIX fork(). Duplicate an address environment into freshly allocated pages mapped at the same virtual addresses, which is what POSIX fork() is built on. The walk mirrors up_addrenv_destroy(): every final level page table the source has under its static tables is visited, and every page it maps is duplicated, so the cost is the size of the process rather than the size of its address space. Shared memory stays shared -- the very same page is mapped, and up_addrenv_destroy() already knows not to free SHM pages. riscv_fork() then lets the child run at the parent's stack addresses. A pointer to a stack local taken before fork() must name the same object in the child that it named in the parent, so the child adopts the parent's stack geometry rather than being given a relocated copy; the parent's stack is already in the duplicate, at the parent's address, with its contents. RISC-V already builds the child's register context from the caller's saved syscall frame, so nothing else is needed. RISC-V selects ARCH_HAVE_FORK in a kernel build with an address environment and without dynamic stacks. Verified on rv-virt:knsh64 under qemu-system-riscv64: ostest's fork_test reports "Parent and child had independent memory", and vfork_test passes. The same on rv-virt:knsh_romfs under qemu-system-riscv32. rv-virt:nsh64 is unchanged, with vfork() passing and fork() correctly absent. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Marco Casaroli <[email protected]> --- .../guides/concurrency/fork_vfork_migration.rst | 6 +- arch/Kconfig | 21 ++- arch/risc-v/src/common/riscv_addrenv.c | 193 +++++++++++++++++++++ arch/risc-v/src/common/riscv_fork.c | 21 +-- 4 files changed, 221 insertions(+), 20 deletions(-) diff --git a/Documentation/guides/concurrency/fork_vfork_migration.rst b/Documentation/guides/concurrency/fork_vfork_migration.rst index cc15ba24494..db78a057a38 100644 --- a/Documentation/guides/concurrency/fork_vfork_migration.rst +++ b/Documentation/guides/concurrency/fork_vfork_migration.rst @@ -59,8 +59,8 @@ environment no longer builds.** ``fork()`` is not declared in ``unistd.h`` there, so you get a compile error naming the function. That is the intended outcome: a build error is strictly better than the silent wrongness it replaces. Today that is every target except a kernel build on arm64, -armv7-a, x86_64 or Xtensa (ESP32-S3), so every caller of ``fork()`` has to -be looked at. +armv7-a, RISC-V, x86_64 or Xtensa (ESP32-S3), so every caller of ``fork()`` +has to be looked at. **Code calling** ``fork()`` **on a target that does have real** ``fork()`` **changes behaviour** -- from sharing to copying. Code that (perhaps @@ -225,7 +225,7 @@ Known gaps complete -- ``addrenv_fork()``, the ``up_addrenv_fork()`` hook, the syscall, the libc wrapper and the ``ostest`` case -- so an architecture provides ``fork()`` by implementing ``up_addrenv_fork()`` and selecting ``CONFIG_ARCH_HAVE_FORK``, -with no further generic work. x86_64, arm64, armv7-a and Xtensa (the +with no further generic work. x86_64, arm64, armv7-a, RISC-V and Xtensa (the ESP32-S3 kernel build) select it today. Note also on ``waitpid()`` after ``vfork()`` diff --git a/arch/Kconfig b/arch/Kconfig index 28cbfcf1b46..d328a3030cd 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -85,6 +85,7 @@ config ARCH_RENESAS config ARCH_RISCV bool "RISC-V" + select ARCH_HAVE_FORK if BUILD_KERNEL && ARCH_ADDRENV && !ARCH_STACK_DYNAMIC select ARCH_HAVE_BACKTRACE select ARCH_HAVE_CPUINFO select ARCH_HAVE_INTERRUPTSTACK @@ -510,13 +511,19 @@ config ARCH_HAVE_FORK allocated pages holding a copy of the parent's contents, mapped at the same virtual addresses. - No architecture selects this yet. Two things are needed. First, - up_addrenv_fork() itself. Second, the architecture must build the - child's register context from the *user's* saved system call frame: - in a kernel build fork() is reached through a system call, so the - return address and stack pointer the architecture's fork entry point - can see for itself are the kernel's, not the caller's, and a child - built from those resumes at a kernel address. + It also requires the architecture to build the child's register + context from the *user's* saved system call frame: in a kernel build + fork() is reached through a system call, so the return address and + stack pointer the architecture's fork entry point can see for itself + are the kernel's, not the caller's, and a child built from those + resumes at a kernel address. + + The protected configurations are excluded, MPU and MMU alike. A + protected build has one address space protected by a fixed set of + regions, and its up_addrenv_*() are stubs; there is no per-process + mapping to duplicate at the same virtual addresses, so POSIX fork() + semantics cannot be provided. vfork(), which shares the parent's + memory, works there as everywhere else. Where this is not selected fork() is not provided at all, and code that calls it fails to build. diff --git a/arch/risc-v/src/common/riscv_addrenv.c b/arch/risc-v/src/common/riscv_addrenv.c index 7df6cd5d5db..446256c41d8 100644 --- a/arch/risc-v/src/common/riscv_addrenv.c +++ b/arch/risc-v/src/common/riscv_addrenv.c @@ -342,6 +342,33 @@ static inline bool vaddr_is_shm(uintptr_t vaddr) #endif } +#ifdef CONFIG_ARCH_HAVE_FORK +/**************************************************************************** + * Name: vaddr_is_text + * + * Description: + * Check if a vaddr is part of the .text area, which is mapped read/execute + * while everything else is mapped read/write. The two arms mirror exactly + * the two layouts up_addrenv_create() builds. + * + ****************************************************************************/ + +static inline bool vaddr_is_text(const arch_addrenv_t *addrenv, + uintptr_t vaddr) +{ +#if (CONFIG_ARCH_TEXT_VBASE != 0x0) && (CONFIG_ARCH_HEAP_VBASE != 0x0) + UNUSED(addrenv); + return vaddr >= CONFIG_ARCH_TEXT_VBASE && vaddr < ARCH_TEXT_VEND; +#else + /* Contiguous layout: the reserve sits below .text, and .data begins where + * .text ends. + */ + + return vaddr >= addrenv->textvbase && vaddr < addrenv->datavbase; +#endif +} +#endif /* CONFIG_ARCH_HAVE_FORK */ + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -497,6 +524,172 @@ errout: return ret; } +#ifdef CONFIG_ARCH_HAVE_FORK +/**************************************************************************** + * Name: up_addrenv_fork + * + * Description: + * Duplicate an address environment for POSIX fork(). The destination gets + * its own page tables and its own physical pages, holding a copy of the + * source's contents and mapped at the same virtual addresses. + * + * The walk mirrors up_addrenv_destroy(): every final level page table the + * source has under its static tables is visited, and every page it maps is + * duplicated. So the cost is the size of the process, not the size of its + * address space -- but it is an eager copy, with no copy-on-write, so + * forking needs as much free page memory as the parent occupies. + * + * Input Parameters: + * src - The address environment to be duplicated. + * dest - The location to receive the duplicate. + * + * Returned Value: + * Zero (OK) on success; a negated errno value on failure. + * + ****************************************************************************/ + +int up_addrenv_fork(const arch_addrenv_t *src, arch_addrenv_t *dest) +{ + uintptr_t *sptprev; + uintptr_t *sptlast; + uintptr_t *dptprev; + uintptr_t *dptlast; + uintptr_t paddr; + uintptr_t vaddr; + uintptr_t pgvaddr; + size_t pgsize; + int i; + int j; + int ret; + + DEBUGASSERT(src && dest); + + memset(dest, 0, sizeof(arch_addrenv_t)); + + /* Give the child its own static page tables and kernel mappings */ + + ret = create_spgtables(dest); + if (ret < 0) + { + serr("ERROR: Failed to create static page tables\n"); + goto errout; + } + + ret = copy_kernel_mappings(dest); + if (ret < 0) + { + serr("ERROR: Failed to copy kernel mappings to new environment\n"); + goto errout; + } + + /* The duplicate lives at the same virtual addresses as the original -- + * that is what makes it a copy rather than merely a similar process -- so + * the bases are carried over unchanged. + */ + + dest->textvbase = src->textvbase; + dest->datavbase = src->datavbase; + dest->heapvbase = src->heapvbase; + dest->heapsize = src->heapsize; + dest->satp = mmu_satp_reg(dest->spgtables[0], 0); + + /* Make sure the source's page tables are visible before walking them */ + + UP_ISB(); + UP_DMB(); + + vaddr = ARCH_ADDRENV_VBASE; + pgsize = mmu_get_region_size(ARCH_SPGTS); + sptprev = (uintptr_t *)riscv_pgvaddr(src->spgtables[ARCH_SPGTS - 1]); + dptprev = (uintptr_t *)riscv_pgvaddr(dest->spgtables[ARCH_SPGTS - 1]); + + if (sptprev == NULL || dptprev == NULL) + { + ret = -EINVAL; + goto errout; + } + + i = (ARCH_SPGTS < 2) ? vaddr / pgsize : 0; + for (; i < ENTRIES_PER_PGT; i++, vaddr += pgsize) + { + sptlast = (uintptr_t *)riscv_pgvaddr(mmu_pte_to_paddr(sptprev[i])); + if (sptlast == NULL) + { + continue; + } + + /* Hook the static tables up for this address, then give the child its + * own final level page table here. + */ + + map_spgtables(dest, vaddr); + + paddr = mm_pgalloc(1); + if (!paddr) + { + ret = -ENOMEM; + goto errout; + } + + riscv_pgwipe(paddr); + mmu_ln_setentry(ARCH_SPGTS, (uintptr_t)dptprev, paddr, vaddr, + MMU_UPGT_FLAGS); + dptlast = (uintptr_t *)riscv_pgvaddr(paddr); + + for (j = 0; j < ENTRIES_PER_PGT; j++) + { + uintptr_t pgvaddr_src; + uintptr_t srcpage; + uintptr_t destpage; + + srcpage = mmu_pte_to_paddr(sptlast[j]); + if (!srcpage) + { + continue; + } + + pgvaddr = vaddr + ((uintptr_t)j << MM_PGSHIFT); + + if (vaddr_is_shm(pgvaddr)) + { + /* Shared memory stays shared across fork(). Map the very same + * page; up_addrenv_destroy() knows not to free SHM pages, so + * this does not hand one page to the allocator twice. + */ + + dptlast[j] = sptlast[j]; + continue; + } + + destpage = mm_pgalloc(1); + if (!destpage) + { + ret = -ENOMEM; + goto errout; + } + + pgvaddr_src = riscv_pgvaddr(srcpage); + memcpy((void *)riscv_pgvaddr(destpage), (const void *)pgvaddr_src, + MM_PGSIZE); + + mmu_ln_setentry(ARCH_SPGTS + 1, (uintptr_t)dptlast, destpage, + pgvaddr, + vaddr_is_text(src, pgvaddr) ? MMU_UTEXT_FLAGS + : MMU_UDATA_FLAGS); + } + } + + UP_ISB(); + UP_DMB(); + + return OK; + +errout: + up_addrenv_destroy(dest); + return ret; +} +#endif /* CONFIG_ARCH_HAVE_FORK */ + /**************************************************************************** * Name: up_addrenv_destroy * diff --git a/arch/risc-v/src/common/riscv_fork.c b/arch/risc-v/src/common/riscv_fork.c index 63edbe075a7..e1a09d5cb27 100644 --- a/arch/risc-v/src/common/riscv_fork.c +++ b/arch/risc-v/src/common/riscv_fork.c @@ -121,24 +121,25 @@ pid_t riscv_fork(bool vfork, const struct fork_s *context) return (pid_t)ERROR; } - /* Copy parent user stack to child */ - - stacktop = (uintptr_t)parent->stack_base_ptr + parent->adj_stack_size; - DEBUGASSERT(stacktop > parent->xcp.sregs[REG_SP]); - stackutil = stacktop - parent->xcp.sregs[REG_SP]; - if (child->stack_base_ptr == parent->stack_base_ptr) { - /* The child is running at the parent's stack addresses, inside its - * own duplicated address environment. There is nothing to relocate: - * every stack address the child inherits is still the address it - * names. + /* A fork() child inherits the parent's stack address: its copy of the + * parent's stack is already in place, with its contents, at the same + * virtual address, so there is nothing to copy and nothing to + * relocate. The child simply resumes on the stack pointer the parent + * called with. */ newsp = parent->xcp.sregs[REG_SP]; } else { + /* Copy parent user stack to child */ + + stacktop = (uintptr_t)parent->stack_base_ptr + parent->adj_stack_size; + DEBUGASSERT(stacktop > parent->xcp.sregs[REG_SP]); + stackutil = stacktop - parent->xcp.sregs[REG_SP]; + /* Copy goes to child's user stack top */ newtop = (uintptr_t)child->stack_base_ptr + child->adj_stack_size;
