Re: [PATCH 3/7] linux-user/nios2: Adjust error return

[Peter Maydell]

On Sun, 20 Mar 2022 at 16:06, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Follow syscall_set_return_value rather than the kernel assembly
> in setting the syscall return values.  Only negate ret on error.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/nios2/cpu_loop.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
> index ac71f4ee47..2ae94f4a95 100644
> --- a/linux-user/nios2/cpu_loop.c
> +++ b/linux-user/nios2/cpu_loop.c
> @@ -48,9 +48,18 @@ void cpu_loop(CPUNios2State *env)
>                                   env->regs[7], env->regs[8], env->regs[9],
>                                   0, 0);
>
> -                env->regs[2] = abs(ret);
> -                /* Return value is 0..4096 */
> -                env->regs[7] = ret > 0xfffff000u;
> +                /*
> +                 * See syscall_set_return_value.
> +                 * Use the QEMU traditional -515 error indication in
> +                 * preference to the < 0 indication used in entry.S.
> +                 */

Well, it is traditional, in that we've used it for sparc for
instance right back to commit 060366c5ad18b3e in 2004, and
even earlier for ppc since commit 678673089d1b.
probably for about as long for ppc. But *why* do we use this?
Well, 516 is ERESTART_RESTARTBLOCK, and that's what the
arch/sparc/kernel/entry.S code is comparing against (it does a
greater-than-or-equal check, I think, hence 516, not 515).

For powerpc, however, the kernel handles setting the CCR
bit in syscall_exit_prepare(), and there it checks against
-MAX_ERRNO.

nios2, as you note in the commit message, does a < 0 check.

So I think:
 * 515 is correct for SPARC, but we really ought not to use
   a hardcoded constant there
 * we are checking against the wrong value for PPC and should
   be checking MAX_ERRNO
 * this patch does the wrong thing for nios2

If we do the same < 0 check that the kernel does, does anything
break ?

thanks
-- PMM