arch/armv7-a: Implement up_addrenv_fork() and provide POSIX fork() - #19775
Draft
casaroli wants to merge 2 commits into
Draft
arch/armv7-a: Implement up_addrenv_fork() and provide POSIX fork()#19775casaroli wants to merge 2 commits into
casaroli wants to merge 2 commits into
Conversation
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. 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 <marco.casaroli@gmail.com>
|
Review of apache#19772 asked for this shape, and it applies to every architecture in the series. ARCH_HAVE_FORK described when it was available from inside its own definition, which put the per-architecture condition somewhere nobody looks. The architecture now says so itself, next to the other things ARMv7-A provides. The condition repeats the ARCH_ADDRENV dependency rather than relying on it, because a select bypasses depends on: without that repetition an architecture could offer fork() where there is no address environment to duplicate. Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fork()was withdrawn from every architecture by #19562, and each architecture restores it with the correct behaviour. This pull request restores it for armv7-a.up_addrenv_fork()duplicates an address environment into freshly allocated pages mapped at the same virtual addresses. The text, data and heap regions of the source are walked page by page and copied into fresh pages hung off the L1 and L2 tables of the child.The saved-syscall-frame half is already in master. #19562 added it, so
arm_syscall()stores the exception frame of the caller inxcp.sregsandarm_fork()builds the child from it.The change that reaches beyond fork()
up_initial_state()now puts the register save area of a user process on its kernel stack, not at the top of its user stack. RISC-V and ARM64 already do this, so armv7-a is being brought into line.Two reasons. The area holds the CPSR that the thread is resumed with, so it must not be user-writable. And a
fork()child inherits the stack address of its parent, so the top of "its" stack is occupied by the live frames of the parent, including the exception frame that the child is built from. ZeroingXCPTCONTEXT_SIZEbytes there would destroy both.arm_fork()then lets the child run at the stack addresses of the parent. A pointer to a stack local taken beforefork()must name the same object in the child that it named in the parent, so the child adopts the stack geometry of the parent rather than a relocated copy. With the save area on the kernel stack there is nothing left forarm_fork_syscall()to re-point.This touches every task on the architecture, not only forked ones. It is the part of this patch that most needs review.
Impact
The change is specific to armv7-a. It adds capability and removes none.
fork()becomes available on armv7-a in a kernel build over an MMU.ARCH_HAVE_FORKkeepsdepends on ARCH_ADDRENV, so a configuration without address environments is unaffected. A protected build is excluded.Cortex-M and Cortex-R are untouched, because
BUILD_KERNELneedsARCH_USE_MMU.vfork()does not change. No board configuration changes.Testing
Emulation only. QEMU 11.0.3 on macOS 15 with Apple Silicon, Arm GNU
arm-none-eabi-gcc14.2.rel1.qemu-armv7a:knshvfork()andfork()pass,ostestexits 0qemu-armv7a:nshvfork()passes,fork()correctly absentThe kernel row needs
-semihosting, because a kernel build loads its applications over hostfs.The flat row is there on purpose.
up_initial_state()affects every task, so a flat build had to be checked as well, even though it has nofork().tools/checkpatch.sh -c -u -m -ggives no errors.What is not tested
No armv7-a hardware. This is the reason for the draft status. A board with an MMU, such as an i.MX6 or a SAMA5, would be the useful report.
The protected build is not tested.
fork()is excluded from it in any case.A report from an armv7-a board is welcome. Take this branch with apache/nuttx-apps#3685, build a kernel configuration and run
ostest. The fork tests are the first output.