Re: [PATCH] selftests: sud_test: return correct emulated syscall value on RISC-V

[Palmer Dabbelt]

On Wed, 13 Sep 2023 07:07:11 PDT (-0700), cleger@xxxxxxxxxxxx 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@xxxxxxxxxxxx>
> ---
>  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:

   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.
   +	 */
   +#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@xxxxxxxxxxxx>
Acked-by: Palmer Dabbelt <palmer@xxxxxxxxxxxx>

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?