-
Notifications
You must be signed in to change notification settings - Fork 5.2k
feat: add support for restartable system calls on RISC-V64 #10520
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: master
Are you sure you want to change the base?
feat: add support for restartable system calls on RISC-V64 #10520
Conversation
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
🏷️ Tag: components_lwpReviewers: xu18838022837 Changed Files (Click to expand)
🏷️ Tag: libcpu_riscvReviewers: Yaochenger Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-07-20 15:05 CST)
📝 Review Instructions
|
c3e2204
to
662b3f1
Compare
c9e2878
to
30ef0db
Compare
请给出更多说明,包括对系统调用部分代码的更改,谢谢。 |
好的,我这几天改改 |
30ef0db
to
68859d2
Compare
68859d2
to
05aa0a6
Compare
@BernardXiong 您好,我已补充了相关说明信息。如果您需要更多细节,或对现有信息有任何疑问,请随时告知,我将尽快提供进一步说明 |
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.
Pull Request Overview
This PR implements support for restartable system calls on RISC-V64 architecture, following the existing AArch64 implementation. It enables interrupted system calls to automatically restart after signal handling, improving compatibility and reliability.
Key changes:
- Added system call restart mechanism with proper register preservation
- Implemented signal handling integration for automatic restart detection
- Added assembly support for restoring execution context and jumping back to trap entry
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
libcpu/risc-v/common64/syscall_c.c | Preserves original a0 register value in t0 and removes a7 clearing for restart support |
components/lwp/arch/risc-v/rv64/lwp_gcc.S | Adds arch_syscall_restart assembly function and updates arch_signal_quit with kernel stack parameter |
components/lwp/arch/risc-v/rv64/lwp_arch.h | Updates arch_signal_ucontext_restore function signature to include kernel_sp parameter |
components/lwp/arch/risc-v/rv64/lwp_arch.c | Implements restart detection logic, signal post-action handling, and errno setting for restart markers |
arch_syscall_restart(eframe, ksp); | ||
} | ||
|
||
return ; |
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.
[nitpick] The explicit return;
statement at the end of a void function is unnecessary and should be removed for cleaner code.
return ; |
Copilot uses AI. Check for mistakes.
{ | ||
arch_signal_check_erestart(&new_sp->frame, (void *)kernel_sp); | ||
|
||
return ; |
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.
[nitpick] The explicit return;
statement at the end of a void function is unnecessary and should be removed for cleaner code.
return ; |
Copilot uses AI. Check for mistakes.
{ | ||
exp_frame->a0 = -code; | ||
} | ||
|
||
return ; |
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.
[nitpick] The explicit return;
statement at the end of a void function is unnecessary and should be removed for cleaner code.
return ; |
Copilot uses AI. Check for mistakes.
/* adjust for epc auto-increment in syscall_handler */ | ||
exp_frame->epc -= 4; | ||
/* copy exception frame from user stack to kernel stack */ | ||
lwp_memcpy(ksp - sizeof(*eframe), eframe, sizeof(*eframe)); |
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.
Using sizeof(*eframe)
where eframe
is a void*
parameter will not give the correct size. The size should be sizeof(struct rt_hw_stack_frame)
since that's the actual type being copied.
lwp_memcpy(ksp - sizeof(*eframe), eframe, sizeof(*eframe)); | |
lwp_memcpy(ksp - sizeof(struct rt_hw_stack_frame), eframe, sizeof(struct rt_hw_stack_frame)); |
Copilot uses AI. Check for mistakes.
拉取/合并请求描述:(PR description)
参考aarch64架构下的系统调用重启实现riscv64架构下的系统调用重启,关联问题#9761
[
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
可重启的系统调用如果由于接受到信号中断导致执行失败,则在信号退出处理时,根据用户栈保存的exception frame 恢复寄存器(包括epc以及存储系统调用号和系统调用参数的寄存器等),并跳转到 trap_entry,以重新启动系统调用。
系统调用功能函数执行前阶段
系统调用功能函数执行后阶段
移除对 a7(系统调用号寄存器)的清零操作,保留其原始值以供重启使用。
在系统调用执行被信号中断退出syscall_handler后,进行信号处理时如果判定需要进行系统调用重启,则通过 arch_syscall_set_errno 将 用户上下文中的a0 设为 -ERESTART,作为需重启的标记。之后将异常帧复制到用户栈,返回用户态执行中断处理函数(如果有的话)。
信号退出处理阶段(在这一步完成系统调用重启)
通过ecall再次陷入内核,执行arch_signal_quit
arch_signal_quit 调用 arch_signal_ucontext_restore,并传递两个关键参数。其一为用户栈中保存的异常帧的基地址,其二为新增参数,表示内核栈中,arch_signal_quit 用户上下文信息的基地址加上 CTX_REG_NR * REGBYTES,设为ksp。
通过调用链 arch_signal_ucontext_restore-> arch_signal_post_action-> arch_signal_check_erestart 执行到核心函数 arch_signal_check_erestart 。
该函数根据保存的异常帧信息判断是否需要重启,如果需要,将异常帧中的t0赋值给a0,即恢复系统调用第一个参数的值。随后将异常帧中的epc减去4进行恢复,因为系统调用在被信号中断后在 syscall_handler对epc进行了加4操作,现在需要将其恢复到跟第一次调用系统调用前一样。之后将异常帧拷贝到内核栈中原来存储 arch_signal_quit 用户上下文信息的位置。并调用汇编函数arch_syscall_restart。
arch_syscall_restart从内核栈中恢复刚刚拷贝进来的异常帧信息到寄存器(包括系统调用参数寄存器a0-a6,以及系统调用号寄存器a7,epc也被恢复指向用户空间的ecall指令地址)。此外,sscratch 会被设置为ksp。之后跳转 trap_entry以重新执行系统调用。
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality:
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0
代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up