[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] linux-user: Add signal handling support for
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH v2] linux-user: Add signal handling support for x86_64 |
Date: |
Sun, 26 Feb 2017 13:29:20 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
Le 26/02/2017 à 00:04, Pranith Kumar a écrit :
> Note that x86_64 has only _rt signal handlers. This implementation
> attempts to share code with the x86_32 implementation.
>
> CC: Laurent Vivier <address@hidden>
> Signed-off-by: Allan Wirth <address@hidden>
> Reviewed-by: Peter Maydell <address@hidden>
> Signed-off-by: Pranith Kumar <address@hidden>
> ---
> v2: Update with review comments from Laurent Vivier
> ---
> linux-user/signal.c | 280
> ++++++++++++++++++++++++++++++++++++++---------
> target/i386/cpu.h | 2 +
> target/i386/fpu_helper.c | 12 ++
> 3 files changed, 243 insertions(+), 51 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 8209539555..c8eb854b29 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -254,7 +254,7 @@ int do_sigprocmask(int how, const sigset_t *set, sigset_t
> *oldset)
> }
>
> #if !defined(TARGET_OPENRISC) && !defined(TARGET_UNICORE32) && \
> - !defined(TARGET_X86_64) && !defined(TARGET_NIOS2)
> + !defined(TARGET_NIOS2)
> /* Just set the guest's signal mask to the specified value; the
> * caller is assumed to have called block_signals() already.
> */
> @@ -512,7 +512,7 @@ void signal_init(void)
> }
> }
>
> -#if !(defined(TARGET_X86_64) || defined(TARGET_UNICORE32))
> +#ifndef TARGET_UNICORE32
> /* Force a synchronously taken signal. The kernel force_sig() function
> * also forces the signal to "not blocked, not ignored", but for QEMU
> * that work is done in process_pending_signals().
> @@ -819,9 +819,8 @@ int do_sigaction(int sig, const struct target_sigaction
> *act,
> return ret;
> }
>
> -#if defined(TARGET_I386) && TARGET_ABI_BITS == 32
> -
> -/* from the Linux kernel */
> +#if defined(TARGET_I386)
> +/* from the Linux kernel - /arch/x86/include/uapi/asm/sigcontext.h */
>
> struct target_fpreg {
> uint16_t significand[4];
> @@ -835,58 +834,120 @@ struct target_fpxreg {
> };
>
> struct target_xmmreg {
> - abi_ulong element[4];
> + uint32_t element[4];
> };
>
> -struct target_fpstate {
> +struct target_fpstate_32 {
> /* Regular FPU environment */
> - abi_ulong cw;
> - abi_ulong sw;
> - abi_ulong tag;
> - abi_ulong ipoff;
> - abi_ulong cssel;
> - abi_ulong dataoff;
> - abi_ulong datasel;
> - struct target_fpreg _st[8];
> + uint32_t cw;
> + uint32_t sw;
> + uint32_t tag;
> + uint32_t ipoff;
> + uint32_t cssel;
> + uint32_t dataoff;
> + uint32_t datasel;
> + struct target_fpreg st[8];
Why don't you keep the original name "_st"?
> uint16_t status;
> uint16_t magic; /* 0xffff = regular FPU data only */
>
> /* FXSR FPU environment */
> - abi_ulong _fxsr_env[6]; /* FXSR FPU env is ignored */
> - abi_ulong mxcsr;
> - abi_ulong reserved;
> - struct target_fpxreg _fxsr_st[8]; /* FXSR FPU reg data is ignored */
> - struct target_xmmreg _xmm[8];
> - abi_ulong padding[56];
> + uint32_t _fxsr_env[6]; /* FXSR FPU env is ignored */
> + uint32_t mxcsr;
> + uint32_t reserved;
> + struct target_fpxreg fxsr_st[8]; /* FXSR FPU reg data is ignored */
"_fxsr_st"?
> @@ -1198,7 +1374,7 @@ long do_rt_sigreturn(CPUX86State *env)
> struct rt_sigframe *frame;
> sigset_t set;
>
> - frame_addr = env->regs[R_ESP] - 4;
> + frame_addr = env->regs[R_ESP] - sizeof(abi_ulong);
> trace_user_do_rt_sigreturn(env, frame_addr);
> if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
> goto badframe;
> @@ -6421,11 +6597,13 @@ static void handle_pending_signal(CPUArchState
> *cpu_env, int sig,
> || defined(TARGET_NIOS2)
> /* These targets do not have traditional signals. */
> setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
> -#else
> +#elif !defined(TARGET_X86_64)
> if (sa->sa_flags & TARGET_SA_SIGINFO)
> setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
> else
> setup_frame(sig, sa, &target_old_set, cpu_env);
> +#else
> + setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
> #endif
You should just add the "defined(TARGET_X86_64)" on the "#if".
Thanks,
Laurent