Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+69 −7
Open

feat: add support for restartable system calls on RISC-V64 #10520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 45 additions & 3 deletions components/lwp/arch/risc-v/rv64/lwp_arch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,50 @@ struct signal_ucontext
struct rt_hw_stack_frame frame;
};

void *arch_signal_ucontext_restore(rt_base_t user_sp)
void arch_syscall_restart(void *sp, void *ksp);

void arch_signal_check_erestart(void *eframe, void *ksp)
{
struct rt_hw_stack_frame *exp_frame = eframe;
long rc = exp_frame->a0;
long sys_id = exp_frame->a7;

(void)sys_id;

if (rc == -ERESTART)
{
LOG_D("%s(rc=%ld,sys_id=%ld,pid=%d)", __func__, rc, sys_id, lwp_self()->pid);
LOG_D("%s: restart rc = %ld", lwp_get_syscall_name(sys_id), rc);

/* t0 stores the copy of user's first syscall argument */
exp_frame->a0 = exp_frame->t0;
/* 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));
Copy link
Preview

Copilot AI Aug 7, 2025

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.

Suggested change
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.


arch_syscall_restart(eframe, ksp);
}

return ;
Copy link
Preview

Copilot AI Aug 7, 2025

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.

Suggested change
return ;

Copilot uses AI. Check for mistakes.

}

static void arch_signal_post_action(struct signal_ucontext *new_sp, rt_base_t kernel_sp)
{
arch_signal_check_erestart(&new_sp->frame, (void *)kernel_sp);

return ;
Copy link
Preview

Copilot AI Aug 7, 2025

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.

Suggested change
return ;

Copilot uses AI. Check for mistakes.

}

void *arch_signal_ucontext_restore(rt_base_t user_sp, rt_base_t kernel_sp)
{
struct signal_ucontext *new_sp;
new_sp = (void *)user_sp;

if (lwp_user_accessable(new_sp, sizeof(*new_sp)))
{
lwp_thread_signal_mask(rt_thread_self(), LWP_SIG_MASK_CMD_SET_MASK, &new_sp->save_sigmask, RT_NULL);
arch_signal_post_action(new_sp, kernel_sp);
}
else
{
Expand Down Expand Up @@ -320,7 +356,13 @@ void *arch_signal_ucontext_save(int signo, siginfo_t *psiginfo,

void arch_syscall_set_errno(void *eframe, int expected, int code)
{
/* NO support */
struct rt_hw_stack_frame *exp_frame = eframe;

if (exp_frame->a0 == -expected)
{
exp_frame->a0 = -code;
}

return ;
Copy link
Preview

Copilot AI Aug 7, 2025

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.

Suggested change
return ;

Copilot uses AI. Check for mistakes.

}

Expand Down Expand Up @@ -354,4 +396,4 @@ int arch_backtrace_uthread(rt_thread_t thread)
return -1;
}
return -1;
}
}
4 changes: 2 additions & 2 deletions components/lwp/arch/risc-v/rv64/lwp_arch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ rt_inline void icache_invalid_all(void)
}

struct rt_hw_stack_frame;
void *arch_signal_ucontext_restore(rt_base_t user_sp);
void *arch_signal_ucontext_restore(rt_base_t user_sp, rt_base_t kernel_sp);
void *arch_signal_ucontext_save(int signo, siginfo_t *psiginfo,
struct rt_hw_stack_frame *exp_frame, rt_base_t user_sp,
lwp_sigset_t *save_sig_mask);
Expand All @@ -79,4 +79,4 @@ void *arch_signal_ucontext_save(int signo, siginfo_t *psiginfo,

#endif

#endif /*LWP_ARCH_H__*/
#endif /*LWP_ARCH_H__*/
16 changes: 16 additions & 0 deletions components/lwp/arch/risc-v/rv64/lwp_gcc.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,28 @@ ret_to_user_exit:
// `RESTORE_ALL` also reset sp to user sp, and setup sscratch
sret

/*
* void void arch_syscall_restart(void *sp, void *ksp);
*/
.global arch_syscall_restart
arch_syscall_restart:
addi a1, a1, -CTX_REG_NR * REGBYTES

/* set stack pointer to saved user exception frame on kernel stack */
mv sp, a1

/* restore all user registers, and setup sscratch to point to ksp */
RESTORE_ALL

j trap_entry

/**
* Restore user context from exception frame stroraged in ustack
* And handle pending signals;
*/
arch_signal_quit:
LOAD a0, FRAME_OFF_SP(sp)
addi a1, sp, CTX_REG_NR * REGBYTES
call arch_signal_ucontext_restore

/* reset kernel sp to the stack */
Expand Down
8 changes: 6 additions & 2 deletions libcpu/risc-v/common64/syscall_c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,13 @@ void syscall_handler(struct rt_hw_stack_frame *regs)

LOG_I("[0x%lx] %s(0x%lx, 0x%lx, 0x%lx, 0x%lx, 0x%lx, 0x%lx, 0x%lx)", rt_thread_self(), syscall_name,
regs->a0, regs->a1, regs->a2, regs->a3, regs->a4, regs->a5, regs->a6);
/* Save original a0 (1st argument) in kernel-stored t0 context:
* - a0 serves as both system call's 1st argument (input) and return value (output)
* - t0 is caller-saved, user code must preserve it before ecall if needed
*/
regs->t0 = regs->a0;
regs->a0 = syscallfunc(regs->a0, regs->a1, regs->a2, regs->a3, regs->a4, regs->a5, regs->a6);
regs->a7 = 0;
regs->epc += 4; // skip ecall instruction
LOG_I("[0x%lx] %s ret: 0x%lx", rt_thread_self(), syscall_name, regs->a0);
}
#endif /* RT_USING_SMART */
#endif /* RT_USING_SMART */