* [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall
@ 2018-07-18 20:06 Richard Henderson
2018-07-18 22:56 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Richard Henderson @ 2018-07-18 20:06 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent, david, qemu-ppc
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>
---
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:
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall
2018-07-18 20:06 [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall Richard Henderson
@ 2018-07-18 22:56 ` Philippe Mathieu-Daudé
2018-07-19 11:13 ` Richard Henderson
2018-07-19 13:05 ` Alex Bennée
2018-07-22 19:07 ` Laurent Vivier
2 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-18 22:56 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, alex.bennee, laurent, david
Hi Richard,
On 07/18/2018 05:06 PM, Richard Henderson wrote:
> 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>
> ---
> 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. */
This comment confuses me because do_setcontext() is available at line 625.
> +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;
Shouldn't this be -TARGET_ENOMEM?
swapcontext(3):
ERRORS
ENOMEM Insufficient stack space left.
> + }
> +
> + 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:
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall
2018-07-18 22:56 ` Philippe Mathieu-Daudé
@ 2018-07-19 11:13 ` Richard Henderson
2018-07-20 1:40 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2018-07-19 11:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-ppc, alex.bennee, laurent, david
On 07/18/2018 03:56 PM, Philippe Mathieu-Daudé wrote:
>> +
>> +/* This syscall implements {get,set,swap}context for userland. */
>
> This comment confuses me because do_setcontext() is available at line 625.
But that's not wired up as a syscall.
>> + /* 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;
>
> Shouldn't this be -TARGET_ENOMEM?
>
> swapcontext(3):
> ERRORS
> ENOMEM Insufficient stack space left.
No. Please compare against the syscall, not the libc interface.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/signal_32.c#n1045
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall
2018-07-18 20:06 [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall Richard Henderson
2018-07-18 22:56 ` Philippe Mathieu-Daudé
@ 2018-07-19 13:05 ` Alex Bennée
2018-07-19 15:24 ` Richard Henderson
2018-07-22 19:07 ` Laurent Vivier
2 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2018-07-19 13:05 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent, david, qemu-ppc
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall
2018-07-19 13:05 ` Alex Bennée
@ 2018-07-19 15:24 ` Richard Henderson
2018-07-19 15:48 ` Peter Maydell
0 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2018-07-19 15:24 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel, laurent, david, qemu-ppc
On 07/19/2018 06:05 AM, Alex Bennée wrote:
> 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?
I have no idea why those have that name. There's no lock,
just a validation of the underlying pages and a transform to
the corresponding host address.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall
2018-07-19 15:24 ` Richard Henderson
@ 2018-07-19 15:48 ` Peter Maydell
0 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2018-07-19 15:48 UTC (permalink / raw)
To: Richard Henderson
Cc: Alex Bennée, David Gibson, qemu-ppc, QEMU Developers,
Laurent Vivier
On 19 July 2018 at 16:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 07/19/2018 06:05 AM, Alex Bennée wrote:
>> 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?
>
> I have no idea why those have that name. There's no lock,
> just a validation of the underlying pages and a transform to
> the corresponding host address.
Mmm, it's unclear. It came in via 53a5960aadd542dd, and the
impression I get is that the idea was to try to decouple from
knowing that the guest memory layout is flat, as a preparation
for possible support for softmmu-in-usermode...
thanks
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall
2018-07-19 11:13 ` Richard Henderson
@ 2018-07-20 1:40 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-20 1:40 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, alex.bennee, laurent, david
On 07/19/2018 08:13 AM, Richard Henderson wrote:
> On 07/18/2018 03:56 PM, Philippe Mathieu-Daudé wrote:
>>> +
>>> +/* This syscall implements {get,set,swap}context for userland. */
>>
>> This comment confuses me because do_setcontext() is available at line 625.
>
> But that's not wired up as a syscall.
>
>>> + /* 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;
>>
>> Shouldn't this be -TARGET_ENOMEM?
>>
>> swapcontext(3):
>> ERRORS
>> ENOMEM Insufficient stack space left.
>
> No. Please compare against the syscall, not the libc interface.
Thanks, I felt I was missing something with the 3rd syscall argument.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/signal_32.c#n1045
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall
2018-07-18 20:06 [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall Richard Henderson
2018-07-18 22:56 ` Philippe Mathieu-Daudé
2018-07-19 13:05 ` Alex Bennée
@ 2018-07-22 19:07 ` Laurent Vivier
2 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2018-07-22 19:07 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee, david, qemu-ppc
Le 18/07/2018 à 22:06, Richard Henderson a écrit :
> 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>
> ---
> linux-user/qemu.h | 2 ++
> linux-user/ppc/signal.c | 56 +++++++++++++++++++++++++++++++++++++++++
> linux-user/syscall.c | 6 +++++
> 3 files changed, 64 insertions(+)
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-22 19:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-18 20:06 [Qemu-devel] [PATCH] linux-user/ppc: Implement swapcontext syscall Richard Henderson
2018-07-18 22:56 ` Philippe Mathieu-Daudé
2018-07-19 11:13 ` Richard Henderson
2018-07-20 1:40 ` Philippe Mathieu-Daudé
2018-07-19 13:05 ` Alex Bennée
2018-07-19 15:24 ` Richard Henderson
2018-07-19 15:48 ` Peter Maydell
2018-07-22 19:07 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.