Re: [Qemu-devel] [PATCH v4] target/xtensa: linux-user: add call0 ABI support

[Laurent Vivier]

Le 06/09/2019 à 18:57, Max Filippov a écrit :
> Xtensa binaries built for call0 ABI don't rotate register window on
> function calls and returns. Invocation of signal handlers from the
> kernel is therefore different in windowed and call0 ABIs.
> There's currently no way to determine xtensa ELF binary ABI from the
> binary itself. Add handler for the -xtensa-abi-call0 command line
> parameter/QEMU_XTENSA_ABI_CALL0 envitonment variable to the qemu-user
> and record ABI choice. Use it to initialize PS.WOE in xtensa_cpu_reset.
> Check PS.WOE in setup_rt_frame to determine how a signal should be
> delivered.
> 
> Reviewed-by: Laurent Vivier <address@hidden>
> Signed-off-by: Max Filippov <address@hidden>
> ---
>  linux-user/main.c          | 11 +++++++++++
>  linux-user/xtensa/signal.c | 25 +++++++++++++++++--------
>  target/xtensa/cpu.c        | 24 ++++++++++++++++++++----
>  target/xtensa/cpu.h        |  3 +++
>  4 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 47917bbb20fc..c9d97d2b1fc6 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -393,6 +393,13 @@ static void handle_arg_trace(const char *arg)
>      trace_file = trace_opt_parse(arg);
>  }
>  
> +#if defined(TARGET_XTENSA)
> +static void handle_arg_abi_call0(const char *arg)
> +{
> +    xtensa_set_abi_call0();
> +}
> +#endif
> +
>  struct qemu_argument {
>      const char *argv;
>      const char *env;
> @@ -446,6 +453,10 @@ static const struct qemu_argument arg_table[] = {
>       "",           "[[enable=]<pattern>][,events=<file>][,file=<file>]"},
>      {"version",    "QEMU_VERSION",     false, handle_arg_version,
>       "",           "display version information and exit"},
> +#if defined(TARGET_XTENSA)
> +    {"xtensa-abi-call0", "QEMU_XTENSA_ABI_CALL0", false, 
> handle_arg_abi_call0,
> +     "",           "assume CALL0 Xtensa ABI"},
> +#endif
>      {NULL, NULL, false, NULL, NULL, NULL}
>  };
>  
> diff --git a/linux-user/xtensa/signal.c b/linux-user/xtensa/signal.c
> index 8d54ef3ae34b..590f0313ffe9 100644
> --- a/linux-user/xtensa/signal.c
> +++ b/linux-user/xtensa/signal.c
> @@ -134,6 +134,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>      abi_ulong frame_addr;
>      struct target_rt_sigframe *frame;
>      uint32_t ra;
> +    bool abi_call0;
> +    unsigned base;
>      int i;
>  
>      frame_addr = get_sigframe(ka, env, sizeof(*frame));
> @@ -182,20 +184,27 @@ void setup_rt_frame(int sig, struct target_sigaction 
> *ka,
>          __put_user(0x00, &frame->retcode[5]);
>  #endif
>      }
> -    env->sregs[PS] = PS_UM | (3 << PS_RING_SHIFT);
> -    if (xtensa_option_enabled(env->config, XTENSA_OPTION_WINDOWED_REGISTER)) 
> {
> -        env->sregs[PS] |= PS_WOE | (1 << PS_CALLINC_SHIFT);
> -    }
>      memset(env->regs, 0, sizeof(env->regs));
>      env->pc = ka->_sa_handler;
>      env->regs[1] = frame_addr;
>      env->sregs[WINDOW_BASE] = 0;
>      env->sregs[WINDOW_START] = 1;
>  
> -    env->regs[4] = (ra & 0x3fffffff) | 0x40000000;
> -    env->regs[6] = sig;
> -    env->regs[7] = frame_addr + offsetof(struct target_rt_sigframe, info);
> -    env->regs[8] = frame_addr + offsetof(struct target_rt_sigframe, uc);
> +    abi_call0 = (env->sregs[PS] & PS_WOE) == 0;
> +    env->sregs[PS] = PS_UM | (3 << PS_RING_SHIFT);
> +
> +    if (abi_call0) {
> +        base = 0;
> +        env->regs[base] = ra;
> +    } else {
> +        env->sregs[PS] |= PS_WOE | (1 << PS_CALLINC_SHIFT);
> +        base = 4;
> +        env->regs[base] = (ra & 0x3fffffff) | 0x40000000;
> +    }
> +    env->regs[base + 2] = sig;
> +    env->regs[base + 3] = frame_addr + offsetof(struct target_rt_sigframe,
> +                                                info);
> +    env->regs[base + 4] = frame_addr + offsetof(struct target_rt_sigframe, 
> uc);
>      unlock_user_struct(frame, frame_addr, 1);
>      return;
>  
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index 76db1741a796..c65dcf9dd782 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -53,6 +53,20 @@ static bool xtensa_cpu_has_work(CPUState *cs)
>  #endif
>  }
>  
> +#ifdef CONFIG_USER_ONLY
> +static bool abi_call0;
> +
> +void xtensa_set_abi_call0(void)
> +{
> +    abi_call0 = true;
> +}
> +
> +bool xtensa_abi_call0(void)
> +{
> +    return abi_call0;
> +}
> +#endif
> +
>  /* CPUClass::reset() */
>  static void xtensa_cpu_reset(CPUState *s)
>  {
> @@ -70,10 +84,12 @@ static void xtensa_cpu_reset(CPUState *s)
>              XTENSA_OPTION_INTERRUPT) ? 0x1f : 0x10;
>      env->pending_irq_level = 0;
>  #else
> -    env->sregs[PS] =
> -        (xtensa_option_enabled(env->config,
> -                               XTENSA_OPTION_WINDOWED_REGISTER) ? PS_WOE : 
> 0) |
> -        PS_UM | (3 << PS_RING_SHIFT);
> +    env->sregs[PS] = PS_UM | (3 << PS_RING_SHIFT);
> +    if (xtensa_option_enabled(env->config,
> +                              XTENSA_OPTION_WINDOWED_REGISTER) &&
> +        !xtensa_abi_call0()) {
> +        env->sregs[PS] |= PS_WOE;
> +    }
>  #endif
>      env->sregs[VECBASE] = env->config->vecbase;
>      env->sregs[IBREAKENABLE] = 0;
> diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
> index 0459243e6bb1..b363ffcf1066 100644
> --- a/target/xtensa/cpu.h
> +++ b/target/xtensa/cpu.h
> @@ -673,6 +673,9 @@ static inline MemoryRegion 
> *xtensa_get_er_region(CPUXtensaState *env)
>  {
>      return env->system_er;
>  }
> +#else
> +void xtensa_set_abi_call0(void);
> +bool xtensa_abi_call0(void);
>  #endif
>  
>  static inline uint32_t xtensa_replicate_windowstart(CPUXtensaState *env)
> 

Applied to my linux-user branch.

Thanks,
Laurent