-
Notifications
You must be signed in to change notification settings - Fork 28
Add basic U-mode support with ecall-based syscalls #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
9ec8f5c to
e2eec20
Compare
|
How can you validate U-mode support? |
That is exactly the issue I am facing right now. To validate U-mode support properly, I need to define the architectural role of Currently, This ambiguity presents two different paths for validation: Scenario A: Kernel Bootstrap (Current Behavior)
Scenario B: Pure User Process
I would appreciate your guidance on the intended design for |
e2eec20 to
7893e1e
Compare
Since this change is pretty fundamental, it would be nice to:
|
7bfa04e to
ab61d11
Compare
|
Provide test programs for newly-introduced |
jserv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update 'Documentation' as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 9 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="kernel/task.c">
<violation number="1" location="kernel/task.c:804">
mo_task_spawn still creates machine-mode tasks, making user-mode support unreachable—pass `true` here so default spawns run in U-mode.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
As an apologies, I need to convert this to draft temporarily. While writing the test code, I ran into some unexpected task management issues that I couldn't solve immediately. To ensure the feature is actually correct before wasting anyone's time, I'll fix these blockers first. I'll open this back up for review once everything is working as expected. And the proper documentation and comments as well. |
Regarding the test program, here is the plan: Basically, I designed the test to cover two phases in a single task:
However, I hit a major blocker during verification. I found a core conflict in But later on, the timer interrupt handles context switching using the I need to refactor to properly use the ISR frame for task initialization when running in preemptive mode. That's what I'm fixing right now. |
8625230 to
8786865
Compare
Based on the test output: It confirms two critical architectural requirements:
Here is a diagram summarizing the validation logic: sequenceDiagram
participant U as User Task (U-mode)
participant K as Kernel (M-mode)
Note over U, K: Requirement 1: Functional Syscall Interface
U->>K: Request Service (sys_tid) via ecall
K-->>U: Return Result (2) via mret
Note right of U: Log: "PASS: sys_tid() returned 2"
Note over U, K: Requirement 2: Privilege Isolation
U->>U: Attempt Privileged Op (read mstatus)
rect rgb(255, 230, 230)
Note right of U: Hardware blocks access
U-xK: EXCEPTION TRIGGERED
end
K->>K: Kernel Panic (Illegal Instruction)
Note right of K: Log confirms MPP=0 (User Mode)
|
8786865 to
0944bd0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 issues found across 18 files
Prompt for AI agents (all 5 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="Documentation/hal-calling-convention.md">
<violation number="1" location="Documentation/hal-calling-convention.md:112">
The frame description equates 33 words × 4 bytes to 144 bytes, but the ISR actually saves 33 words (132 bytes) and only reaches 144 with padding, so the documented math is incorrect.
(Based on your team's feedback about cross-checking ISR frame layouts.) [FEEDBACK_USED]</violation>
<violation number="2" location="Documentation/hal-calling-convention.md:119">
Padding for the ISR frame actually covers offsets 132–143 (12 bytes) to reach the 144-byte allocation, so documenting it as only 132–140 is inaccurate.
(Based on your team's feedback about cross-checking ISR frame layouts.) [FEEDBACK_USED]</violation>
<violation number="3" location="Documentation/hal-calling-convention.md:266">
The context-switch cost line equates 33 loads/stores to 144 bytes, but the ISR actually performs 33 loads/stores = 132 bytes and simply pads the frame to 144 bytes, so the documentation misrepresents the overhead.
(Based on your team's feedback about cross-checking ISR frame layouts.) [FEEDBACK_USED]</violation>
</file>
<file name="Documentation/hal-riscv-context-switch.md">
<violation number="1" location="Documentation/hal-riscv-context-switch.md:125">
The example prototype advertises a `tp_val` parameter and `bool user_mode`, but the real function only takes `int user_mode` and calculates `tp` itself, so the documentation cannot be used to call the API correctly.</violation>
<violation number="2" location="Documentation/hal-riscv-context-switch.md:128">
The snippet allocates the ISR frame at `stack_top - ISR_FRAME_SIZE`, but the real code subtracts both `INITIAL_STACK_RESERVE` and `ISR_STACK_FRAME_SIZE`; omitting that reserve (and referencing an undefined constant) makes the documentation incorrect and misleading.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
The generic syscall dispatcher coupled privilege transition mechanisms with table lookup logic, preventing architecture-specific trap implementations from reusing the dispatch table. Introduce separate dispatcher for direct table lookup that trap handlers can invoke without triggering privilege transitions. Mark user-space interface as weak symbol to enable architecture overrides. Rename wrapper functions to match generated short names.
Architecture-specific implementations require direct linkage to override weak symbols. Archives extract objects only when symbols are unresolved, skipping strong overrides when weak symbols satisfy references. Introduce trap-based syscall entry using ecall instruction and modify build system to link entry point before archive, ensuring architecture override takes precedence at link time.
0944bd0 to
b2d9887
Compare
|
I've updated the documentation to align with the actual implementation. The frame size math is corrected to 132 bytes for the 33 words, with padding filling the remaining space to reach the 144-byte alignment. For I also included |
arch/riscv/boot.c
Outdated
| "mv a2, sp\n" /* Arg 3: isr_sp (current stack frame) */ | ||
| "sw a0, 30*4(sp)\n" | ||
| "sw a1, 31*4(sp)\n" | ||
| "csrr t0, mcause\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @HeatCrab
Can you explain the reason why you use t0, t1, and t2 to store the control state registers rather than use a0, a1, and a2, directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I initially used t registers just to play it safe and avoid confusion. After checking, I see it works fine, so I've adopted your suggestion to use a0-a2 directly. Thanks!
User mode tasks require privilege escalation to invoke kernel services. Without proper trap frame preservation, context switches corrupt privilege state, preventing tasks from resuming at correct levels. Add trap handler for user mode environment calls to dispatch syscalls. Extend trap frame to preserve privilege mode across context switches. Correct frame layout to match actual register storage order in trap entry sequence.
Kernel requires distinct privilege modes for kernel services and user applications. Return from trap instruction needs previous interrupt enable bit set to preserve interrupt state across privilege transitions. Parameterize context initialization to configure privilege mode during task creation. Set previous interrupt enable bit for correct interrupt behavior after mode transitions. Provide separate interface for spawning user mode tasks alongside existing kernel task interface.
The preemptive scheduler requires interrupt frame restoration during task startup to properly transition privilege modes. However, the dispatcher was initializing tasks using cooperative mode context structures, which lack the necessary state for privilege transitions. This mismatch caused privilege mode corruption and prevented tasks from executing correctly. The dispatcher initialization now selects the appropriate context type based on the active scheduler mode. For preemptive scheduling, the system restores the full interrupt frame and uses trap return instructions to transfer control with proper privilege level switching. The initial status register configuration has been adjusted to prevent interrupts from enabling prematurely during the restoration sequence, avoiding race conditions during task startup.
User mode tasks cannot directly use the standard output functions because the logger system requires privileged operations for synchronization. When user mode code attempts these operations, the processor triggers illegal instruction exceptions that prevent normal execution. To address this limitation, a new system call interface provides safe output capabilities for user mode tasks. The implementation splits the work between user and machine modes: formatting occurs in user space using only unprivileged operations, while the actual output is performed through a system call that executes in machine mode where privileged operations are permitted. The kernel handles all synchronization and hardware access transparently, allowing user mode tasks to produce output without violating privilege boundaries.
This test application validates both the system call interface and privilege isolation mechanisms in a two-phase approach. The first phase verifies that system calls execute correctly from user mode. It invokes several read-only system calls to confirm that the trap-based calling convention functions properly and that return values propagate correctly across privilege boundaries. All output uses the safe user mode output interface to avoid triggering privilege violations during the test itself. The second phase validates security isolation by deliberately attempting to execute a privileged instruction from user mode. The test expects this to trigger an illegal instruction exception, confirming that the hardware properly enforces privilege restrictions. When the exception occurs as expected, it demonstrates that user mode code cannot bypass the privilege system to access machine mode resources. This intentional test failure is the correct outcome and proves the isolation mechanism works as designed.
The user mode validation test intentionally triggers an illegal instruction exception to verify privilege isolation, which would normally be classified as a test failure in the standard application test suite. This test has been moved to the functional test suite where its expected behavior can be properly validated. The application test suite now excludes this test to avoid false negatives. The functional test suite has been updated to recognize the expected privilege violation as a valid success criterion alongside the syscall mechanism validation. The crash detection logic now permits expected exceptions for tests that intentionally verify security boundaries.
The hardware abstraction layer now supports both cooperative and preemptive scheduling modes with distinct context management approaches. The documentation has been updated to reflect these architectural differences and their implications for task initialization and privilege management. The interrupt frame structure preserves complete trap context with 33 words for register state and control registers, plus 12 bytes of padding to maintain 16-byte alignment, totaling 144 bytes. This frame supports both interrupt handling and initial task setup for preemptive scheduling, where tasks launch through trap return rather than standard function calls. Task initialization varies between modes. Cooperative mode uses lightweight context structures containing only callee-saved registers for voluntary yielding. Preemptive mode builds complete interrupt frames with all registers initialized to zero, global and thread pointers configured, and processor state set for proper privilege transitions. The frame is positioned with a 256-byte initial stack reserve below the stack top to accommodate startup requirements. The dispatcher initialization process differs for each scheduling mode. Cooperative tasks transfer control through standard calling conventions with global interrupts enabled before execution. Preemptive tasks restore interrupt frames and execute trap return instructions, allowing hardware to transition to the configured privilege level and enable interrupts based on the saved processor state. The system call interface operates through the RISC-V trap mechanism for privilege boundary crossing. User mode tasks invoke kernel services using environment call instructions that trigger synchronous exceptions. The trap handler preserves all registers except the return value, maintaining standard calling convention semantics across the privilege boundary while the kernel validates parameters and mediates access to protected resources.
b2d9887 to
814b636
Compare
This PR implements basic U-mode (User Mode) execution for tasks, addressing Issue #19 where tasks executing in M-mode could bypass isolation by modifying privileged CSRs. This change is essential for PMP (Physical Memory Protection) to function correctly, as PMP only affects U-mode and S-mode.
Tasks now start in user mode and interact with the kernel through a syscall interface based on the ecall trap mechanism. The kernel dispatcher has been refactored to support architecture-specific implementations through weak linking, while the RISC-V backend provides an ecall wrapper following standard calling conventions. Exception handling has been extended to recognize and service user mode traps, maintaining proper privilege boundaries throughout task execution.
Related to #19
Summary by cubic
Adds user-mode task support on RISC-V with an ecall-based syscall path to enforce privilege separation and make PMP effective. Addresses the isolation issue where tasks could modify privileged CSRs in M-mode (Issue #19).
New Features
Refactors
Written for commit 814b636. Summary will update automatically on new commits.