Messages in this thread | | | Date | Thu, 9 Nov 2023 09:22:46 +0100 | Subject | Re: [PATCH] selftests: sud_test: return correct emulated syscall value on RISC-V | From | Clément Léger <> |
| |
On 09/11/2023 04:26, Palmer Dabbelt wrote: > On Wed, 13 Sep 2023 07:07:11 PDT (-0700), cleger@rivosinc.com wrote: >> Currently, the sud_test expects the emulated syscall to return the >> emulated syscall number. This assumption only works on architectures >> were the syscall calling convention use the same register for syscall >> number/syscall return value. This is not the case for RISC-V and thus >> the return value must be also emulated using the provided ucontext. >> >> Signed-off-by: Clément Léger <cleger@rivosinc.com> >> --- >> tools/testing/selftests/syscall_user_dispatch/sud_test.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/tools/testing/selftests/syscall_user_dispatch/sud_test.c >> b/tools/testing/selftests/syscall_user_dispatch/sud_test.c >> index b5d592d4099e..1b5553c19700 100644 >> --- a/tools/testing/selftests/syscall_user_dispatch/sud_test.c >> +++ b/tools/testing/selftests/syscall_user_dispatch/sud_test.c >> @@ -158,6 +158,14 @@ static void handle_sigsys(int sig, siginfo_t >> *info, void *ucontext) >> >> /* In preparation for sigreturn. */ >> SYSCALL_DISPATCH_OFF(glob_sel); >> + >> + /* >> + * Modify interrupted context returned value according to syscall >> + * calling convention >> + */ >> +#if defined(__riscv) >> + ((ucontext_t*)ucontext)->uc_mcontext.__gregs[REG_A0] = >> MAGIC_SYSCALL_1; >> +#endif >> } >> >> TEST(dispatch_and_return) > > I'm not sure if I'm just tired, but it took me a while to figure out why > this was necessary. I think this is a better explanation:
I think it's because this mechanism does not behave like other syscalls at all and the classic calling convention does not really apply...
> > diff --git a/tools/testing/selftests/syscall_user_dispatch/sud_test.c > b/tools/testing/selftests/syscall_user_dispatch/sud_test.c > index b5d592d4099e..a913fd90cfa3 100644 > --- a/tools/testing/selftests/syscall_user_dispatch/sud_test.c > +++ b/tools/testing/selftests/syscall_user_dispatch/sud_test.c > @@ -158,6 +158,16 @@ static void handle_sigsys(int sig, siginfo_t > *info, void *ucontext) > /* In preparation for sigreturn. */ > SYSCALL_DISPATCH_OFF(glob_sel); > + /* > + * The tests for argument handling assume that `syscall(x) == > x`. This > + * is a NOP on x86 because the syscall number is passed in %rax, > which > + * happens to also be the function ABI return register. Other > + * architectures may need to swizzle the arguments around. > + */
Indeed, that is more clear. Should I send a v2 ?
> +#if defined(__riscv) > + (ucontext_t*)ucontext)->uc_mcontext.__gregs[REG_A0] = > + (ucontext_t*)ucontext)->uc_mcontext.__gregs[REG_A7]; > +#endif > } > TEST(dispatch_and_return) > > but also > > Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > as I agree this is correct. > > also: wouldn't arm64 also need to move x8 into x0 here, for essentially > the same reason as we do?
Yes, as well as for a bunch of other architectures. I suspect this has only been tested on x86. AFAIK, this feature is mainly for wine usage which then makes sense for x86 and games.
Thanks,
Clément
| |