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 26c94cd5757e437b8c60fac68ac1fd2fe6ff86a4 Author: Marco Casaroli <[email protected]> AuthorDate: Sun Aug 9 15:37:57 2026 +0200 arch/armv7-a: 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 .text, .data and heap regions of the source are walked page by page and copied into fresh pages hung off the child's own L1 and L2 tables. Two things go with it. up_initial_state() now puts a user process's register save area on its kernel stack rather than at the top of its user stack, which is what risc-v and arm64 already do. The area must not be user-writable -- it holds the CPSR the thread is resumed with -- and, more to the point here, a fork() child inherits the parent's stack address, so the top of "its" stack is occupied by the parent's live frames, including the exception frame the child is built from. Zeroing XCPTCONTEXT_SIZE bytes there would destroy both. arm_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. With the save area on the kernel stack there is nothing left for arm_fork_syscall() to re-point. ARMv7-A selects ARCH_HAVE_FORK in a kernel build with an address environment. Verified on qemu-armv7a:knsh under qemu-system-arm: ostest's fork_test reports "Parent and child had independent memory", and vfork_test passes. qemu-armv7a:nsh 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 | 8 +- arch/arm/Kconfig | 1 + arch/arm/src/armv7-a/addrenv.h | 20 ++++ arch/arm/src/armv7-a/arm_addrenv.c | 119 +++++++++++++++++++ arch/arm/src/armv7-a/arm_addrenv_utils.c | 126 +++++++++++++++++++++ arch/arm/src/armv7-a/arm_initialstate.c | 37 +++++- arch/arm/src/common/arm_fork.c | 13 +-- 7 files changed, 307 insertions(+), 17 deletions(-) diff --git a/Documentation/guides/concurrency/fork_vfork_migration.rst b/Documentation/guides/concurrency/fork_vfork_migration.rst index f6461b4a556..cc15ba24494 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, -x86_64 or Xtensa (ESP32-S3), so every caller of ``fork()`` has to be -looked at. +armv7-a, 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,8 +225,8 @@ 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 and Xtensa (the ESP32-S3 -kernel build) select it today. +with no further generic work. x86_64, arm64, armv7-a and Xtensa (the +ESP32-S3 kernel build) select it today. Note also on ``waitpid()`` after ``vfork()`` ============================================ diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b81cb175120..41b9eaf69cf 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1164,6 +1164,7 @@ config ARCH_CORTEXM7 config ARCH_ARMV7A bool default n + select ARCH_HAVE_FORK if BUILD_KERNEL && ARCH_ADDRENV select ARCH_HAVE_CPUINFO select ARCH_HAVE_DEBUG select ARCH_HAVE_PERF_EVENTS diff --git a/arch/arm/src/armv7-a/addrenv.h b/arch/arm/src/armv7-a/addrenv.h index a5453afd97f..afa7b2f99fc 100644 --- a/arch/arm/src/armv7-a/addrenv.h +++ b/arch/arm/src/armv7-a/addrenv.h @@ -85,6 +85,26 @@ int arm_addrenv_create_region(uintptr_t *l1table, unsigned int listlen, uintptr_t vaddr, size_t regionsize, uint32_t mmuflags); +/**************************************************************************** + * Name: arm_addrenv_fork_region + * + * Description: + * Duplicate one memory region of an address environment for fork(). The + * destination gets its own page tables and, unless `share' is requested, + * its own copy of the source's pages, mapped at the same virtual + * addresses. + * + * Returned Value: + * Zero (OK) on success; a negated errno value on failure. + * + ****************************************************************************/ + +#ifdef CONFIG_ARCH_HAVE_FORK +int arm_addrenv_fork_region(uintptr_t *srcl1, uintptr_t *destl1, + unsigned int listlen, uintptr_t vaddr, + uint32_t mmuflags, bool share); +#endif + /**************************************************************************** * Name: arm_addrenv_destroy_region * diff --git a/arch/arm/src/armv7-a/arm_addrenv.c b/arch/arm/src/armv7-a/arm_addrenv.c index ce82d7e0721..fa31d50e7e3 100644 --- a/arch/arm/src/armv7-a/arm_addrenv.c +++ b/arch/arm/src/armv7-a/arm_addrenv.c @@ -255,6 +255,125 @@ errout: return ret; } +#ifdef CONFIG_ARCH_HAVE_FORK +/**************************************************************************** + * Name: up_addrenv_fork + * + * Description: + * Duplicate an address environment for POSIX fork(). The destination is + * backed by fresh pages holding a copy of the source's contents, mapped at + * the same virtual addresses. + * + * Only the pages the source actually has mapped are duplicated, so the + * cost is the size of the process, not the size of its address space. + * There is no copy-on-write, so this needs as much free page memory as + * the parent occupies and returns -ENOMEM when that is not available. + * + * 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(FAR const arch_addrenv_t *src, FAR arch_addrenv_t *dest) +{ + size_t npage = MM_NPAGES(PGTABLE_SIZE); + int ret; + + binfo("src=%p dest=%p\n", src, dest); + + DEBUGASSERT(src && dest && src->l1table); + + memset(dest, 0, sizeof(arch_addrenv_t)); + + /* Give the child its own L1 page table, seeded from the kernel's, exactly + * as up_addrenv_create() does. + */ + + dest->l1table = (uintptr_t *)mm_pgalloc_align(npage, npage); + if (dest->l1table == NULL) + { + ret = -ENOMEM; + goto errout; + } + + memcpy(dest->l1table, (void *)PGTABLE_BASE_VADDR, PGTABLE_SIZE); + + /* The duplicate lives at the same virtual addresses as the original -- + * that is the whole point -- so the bases are simply carried over. + */ + + dest->textvbase = src->textvbase; + dest->datavbase = src->datavbase; + dest->heapvbase = src->heapvbase; + dest->heapsize = src->heapsize; +#ifdef CONFIG_ARCH_VMA_MAPPING + dest->shmvbase = src->shmvbase; +#endif + + ret = arm_addrenv_fork_region(src->l1table, dest->l1table, + ARCH_TEXT_NSECTS, CONFIG_ARCH_TEXT_VBASE, + MMU_L2_UTEXTFLAGS, false); + if (ret < 0) + { + berr("ERROR: Failed to duplicate .text region: %d\n", ret); + goto errout; + } + + ret = arm_addrenv_fork_region(src->l1table, dest->l1table, + ARCH_DATA_NSECTS, CONFIG_ARCH_DATA_VBASE, + MMU_L2_UDATAFLAGS, false); + if (ret < 0) + { + berr("ERROR: Failed to duplicate .bss/.data region: %d\n", ret); + goto errout; + } + +#ifdef CONFIG_BUILD_KERNEL + ret = arm_addrenv_fork_region(src->l1table, dest->l1table, + ARCH_HEAP_NSECTS, CONFIG_ARCH_HEAP_VBASE, + MMU_L2_UDATAFLAGS, false); + if (ret < 0) + { + berr("ERROR: Failed to duplicate heap region: %d\n", ret); + goto errout; + } + + /* Note that the stack region is deliberately not duplicated here. The + * child's stack is allocated separately by nxtask_setup_fork() and filled + * in by arm_fork(), and up_addrenv_destroy() does not own the stack region + * either -- up_addrenv_ustackfree() does. Duplicating it here would leak + * it. CONFIG_ARCH_HAVE_FORK therefore depends on + * !CONFIG_ARCH_STACK_DYNAMIC. + */ + +#ifdef CONFIG_ARCH_VMA_MAPPING + /* Shared memory stays shared across fork(), so the child is given the + * parent's own pages rather than copies of them. + */ + + ret = arm_addrenv_fork_region(src->l1table, dest->l1table, + ARCH_SHM_NSECTS, CONFIG_ARCH_SHM_VBASE, + MMU_L2_UDATAFLAGS, true); + if (ret < 0) + { + berr("ERROR: Failed to share SHM region: %d\n", ret); + goto errout; + } +#endif +#endif /* CONFIG_BUILD_KERNEL */ + + return OK; + +errout: + up_addrenv_destroy(dest); + return ret; +} +#endif /* CONFIG_ARCH_HAVE_FORK */ + /**************************************************************************** * Name: up_addrenv_destroy * diff --git a/arch/arm/src/armv7-a/arm_addrenv_utils.c b/arch/arm/src/armv7-a/arm_addrenv_utils.c index 6a9f1ea0dd4..17e5125b58b 100644 --- a/arch/arm/src/armv7-a/arm_addrenv_utils.c +++ b/arch/arm/src/armv7-a/arm_addrenv_utils.c @@ -149,6 +149,132 @@ int arm_addrenv_create_region(uintptr_t *l1table, unsigned int listlen, return npages; } +#ifdef CONFIG_ARCH_HAVE_FORK +/**************************************************************************** + * Name: arm_addrenv_fork_region + * + * Description: + * Duplicate one memory region of an address environment for fork(). + * + * The destination gets its own L2 page tables and, unless `share' is + * requested, its own physical pages holding a copy of the source's + * contents. Both are mapped at the same virtual addresses as the source, + * which is what makes the duplicate an exact copy: a pointer the parent + * held into its own memory means the same thing in the child. + * + * Only the sections the source actually has mapped are duplicated, so the + * copy is the size of the parent, not the size of the region. + * + * `share' maps the source's own pages into the destination rather than + * copying them. It is used for the shared memory region, which POSIX says + * remains shared across fork(). + * + * Input Parameters: + * srcl1 - The L1 page table of the address environment to duplicate + * destl1 - The L1 page table receiving the duplicate + * listlen - Number of sections in the region + * vaddr - Virtual base address of the region + * mmuflags - L2 flags for the duplicated pages + * share - Map the source's pages rather than copying them + * + * Returned Value: + * Zero (OK) on success; a negated errno value on failure. On failure the + * caller is expected to tear the destination down with + * up_addrenv_destroy(), which copes with a partially built environment. + * + ****************************************************************************/ + +int arm_addrenv_fork_region(uintptr_t *srcl1, uintptr_t *destl1, + unsigned int listlen, uintptr_t vaddr, + uint32_t mmuflags, bool share) +{ + uintptr_t l1entry; + uintptr_t *srcl2; + uintptr_t *destl2; + uintptr_t paddr; + unsigned int i; + unsigned int j; + + binfo("listlen=%d vaddr=%08lx share=%d\n", listlen, + (unsigned long)vaddr, share); + + for (i = 0; i < listlen; i++, vaddr += SECTION_SIZE) + { + /* Skip sections the source never populated */ + + l1entry = mmu_l1table_getentry(srcl1, vaddr); + if (l1entry == 0) + { + continue; + } + + srcl2 = (uintptr_t *)arm_pgvaddr(l1entry & PTE_SMALL_PADDR_MASK); + + /* Allocate one physical page for the destination L2 page table */ + + paddr = mm_pgalloc(1); + if (!paddr) + { + return -ENOMEM; + } + + DEBUGASSERT(MM_ISALIGNED(paddr)); + + mmu_l1table_setentry(destl1, paddr, vaddr, MMU_L1_PGTABFLAGS); + + destl2 = (uintptr_t *)arm_pgvaddr(paddr); + memset(destl2, 0, ENTRIES_PER_L2TABLE * sizeof(uintptr_t)); + + for (j = 0; j < ENTRIES_PER_L2TABLE; j++) + { + uintptr_t srcpage = srcl2[j]; + uintptr_t destpage; + + if (srcpage == 0) + { + continue; + } + + if (share) + { + /* Map the very same page. Note that up_addrenv_destroy() + * tears a shared region down with keep=true, so this does not + * hand the same page to the page allocator twice. + */ + + destl2[j] = srcpage; + continue; + } + + srcpage &= PTE_SMALL_PADDR_MASK; + + destpage = mm_pgalloc(1); + if (!destpage) + { + return -ENOMEM; + } + + memcpy((void *)arm_pgvaddr(destpage), + (const void *)arm_pgvaddr(srcpage), MM_PGSIZE); + + up_flush_dcache(arm_pgvaddr(destpage), + arm_pgvaddr(destpage) + MM_PGSIZE); + + set_l2_entry(destl2, destpage, vaddr + (j << MM_PGSHIFT), + mmuflags); + } + + /* Make sure the initialized L2 table is visible to the MMU */ + + up_flush_dcache((uintptr_t)destl2, + (uintptr_t)destl2 + + ENTRIES_PER_L2TABLE * sizeof(uintptr_t)); + } + + return OK; +} +#endif /* CONFIG_ARCH_HAVE_FORK */ + /**************************************************************************** * Name: arm_addrenv_destroy_region * diff --git a/arch/arm/src/armv7-a/arm_initialstate.c b/arch/arm/src/armv7-a/arm_initialstate.c index c0f9dd469e4..bb074e94914 100644 --- a/arch/arm/src/armv7-a/arm_initialstate.c +++ b/arch/arm/src/armv7-a/arm_initialstate.c @@ -55,6 +55,7 @@ void up_initial_state(struct tcb_s *tcb) { struct xcptcontext *xcp = &tcb->xcp; + uintptr_t topstack; uint32_t cpsr; #ifdef CONFIG_ARCH_KERNEL_STACK uint32_t *kstack = xcp->kstack; @@ -85,21 +86,47 @@ void up_initial_state(struct tcb_s *tcb) return; } + topstack = (uintptr_t)tcb->stack_base_ptr + tcb->adj_stack_size; + #ifdef CONFIG_ARCH_KERNEL_STACK xcp->kstack = kstack; + + /* A user process in a kernel build keeps its register save area on its own + * kernel stack, not at the top of its user stack. Two reasons: + * + * - The area must not be user-writable. It holds the CPSR the thread is + * resumed with, so a task that could scribble on it could resume + * itself in a privileged mode. + * - The kernel must be able to write it without regard to which address + * environment is current, and without regard to what the user stack + * already contains. A fork() child inherits the parent's stack + * address, so the top of "its" stack is occupied by the parent's live + * frames -- including the exception frame the child is built from, + * which sits just below the caller's stack pointer. Zeroing + * XCPTCONTEXT_SIZE bytes there would destroy both. + * + * This is what risc-v and arm64 already do; see riscv_initialstate.c. + */ + + if (kstack != NULL) + { + topstack = (uintptr_t)kstack + ARCH_KERNEL_STACKSIZE; + } #endif - /* Initialize the context registers to stack top */ + /* Initialize the context registers to the top of whichever stack holds the + * save area + */ - xcp->regs = (void *)((uint32_t)tcb->stack_base_ptr + - tcb->adj_stack_size - - XCPTCONTEXT_SIZE); + xcp->regs = (void *)(topstack - XCPTCONTEXT_SIZE); /* Initialize the xcp registers */ memset(xcp->regs, 0, XCPTCONTEXT_SIZE); - /* Save the initial stack pointer */ + /* Save the initial stack pointer. The thread itself always starts on its + * user stack, whichever stack the save area came off. + */ xcp->regs[REG_SP] = (uint32_t)tcb->stack_base_ptr + tcb->adj_stack_size; diff --git a/arch/arm/src/common/arm_fork.c b/arch/arm/src/common/arm_fork.c index a33c55a1eb9..947b6904954 100644 --- a/arch/arm/src/common/arm_fork.c +++ b/arch/arm/src/common/arm_fork.c @@ -279,6 +279,11 @@ static pid_t arm_fork_direct(bool vfork, struct tcb_s *parent, * in a system call at all, so it inherits none of the parent's nesting * state. * + * A process in a kernel build keeps its register save area on its kernel + * stack, so building the child's context writes nothing to any user stack + * -- see up_initial_state(). That is what lets a child which shares the + * parent's stack addresses be left exactly as it is. + * * Input Parameters: * vfork - true for vfork(), false for fork() * parent - The calling task's TCB @@ -347,14 +352,6 @@ static pid_t arm_fork_syscall(bool vfork, struct tcb_s *parent) child->adj_stack_size; newsp = newtop - stackutil; - /* Put the child's register save area where the parent's is: just - * below the stack the caller was using. It cannot be left at the top - * of the child's stack, which is where up_initial_state() put it, - * because the copy of the parent's stack below is about to land there. - */ - - child->xcp.regs = (uint32_t *)(newsp - XCPTCONTEXT_SIZE); - memcpy((void *)newsp, (const void *)oldsp, stackutil); /* Was there a frame pointer in place before? */
