Re: [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> This allows the tests generated by debian-powerpc-user-cross
> to function properly, especially tests/test-coroutine.
>
> Technically this syscall is available to both ppc32 and ppc64,
> but only ppc32 glibc actually uses it.  Thus the ppc64 path is
> untested.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Tested-by: Alex Bennée <alex.bennee@linaro.org>

and a caveat-ed:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

I'm confused by the lock_user_struct/unlock_user_struct which AFAICT are
basically access checks. Is there an implied locking I'm missing?

> ---
>  linux-user/qemu.h       |  2 ++
>  linux-user/ppc/signal.c | 56 +++++++++++++++++++++++++++++++++++++++++
>  linux-user/syscall.c    |  6 +++++
>  3 files changed, 64 insertions(+)
>
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index bb85c81aa4..e0963676c7 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -395,6 +395,8 @@ long do_sigreturn(CPUArchState *env);
>  long do_rt_sigreturn(CPUArchState *env);
>  abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp);
>  int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset);
> +abi_long do_swapcontext(CPUArchState *env, abi_ulong uold_ctx,
> +                        abi_ulong unew_ctx, abi_long ctx_size);
>  /**
>   * block_signals: block all signals while handling this guest syscall
>   *
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index ef4c518f11..2ae120a2bc 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -675,3 +675,59 @@ sigsegv:
>      force_sig(TARGET_SIGSEGV);
>      return -TARGET_QEMU_ESIGRETURN;
>  }
> +
> +/* This syscall implements {get,set,swap}context for userland.  */
> +abi_long do_swapcontext(CPUArchState *env, abi_ulong uold_ctx,
> +                        abi_ulong unew_ctx, abi_long ctx_size)
> +{
> +    struct target_ucontext *uctx;
> +    struct target_mcontext *mctx;
> +
> +    /* For ppc32, ctx_size is "reserved for future use".
> +     * For ppc64, we do not yet support the VSX extension.
> +     */
> +    if (ctx_size < sizeof(struct target_ucontext)) {
> +        return -TARGET_EINVAL;
> +    }
> +
> +    if (uold_ctx) {
> +        TaskState *ts = (TaskState *)thread_cpu->opaque;
> +
> +        if (!lock_user_struct(VERIFY_WRITE, uctx, uold_ctx, 1)) {
> +            return -TARGET_EFAULT;
> +        }
> +
> +#ifdef TARGET_PPC64
> +        mctx = &uctx->tuc_sigcontext.mcontext;
> +#else
> +        /* ??? The kernel aligns the pointer down here into padding, but
> +         * in setup_rt_frame we don't.  Be self-compatible for now.
> +         */
> +        mctx = &uctx->tuc_mcontext;
> +        __put_user(h2g(mctx), &uctx->tuc_regs);
> +#endif
> +
> +        save_user_regs(env, mctx);
> +        host_to_target_sigset(&uctx->tuc_sigmask, &ts->signal_mask);
> +
> +        unlock_user_struct(uctx, uold_ctx, 1);
> +    }
> +
> +    if (unew_ctx) {
> +        int err;
> +
> +        if (!lock_user_struct(VERIFY_READ, uctx, unew_ctx, 1)) {
> +            return -TARGET_EFAULT;
> +        }
> +        err = do_setcontext(uctx, env, 0);
> +        unlock_user_struct(uctx, unew_ctx, 1);
> +
> +        if (err) {
> +            /* We cannot return to a partially updated context.  */
> +            force_sig(TARGET_SIGSEGV);
> +        }
> +        return -TARGET_QEMU_ESIGRETURN;
> +    }
> +
> +    return 0;
> +}
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 3df3bdffb2..dfc851cc35 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -12790,6 +12790,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          ret = get_errno(kcmp(arg1, arg2, arg3, arg4, arg5));
>          break;
>  #endif
> +#ifdef TARGET_NR_swapcontext
> +    case TARGET_NR_swapcontext:
> +        /* PowerPC specific.  */
> +        ret = do_swapcontext(cpu_env, arg1, arg2, arg3);
> +        break;
> +#endif
>
>      default:
>      unimplemented:


--
Alex Bennée