| Message ID | 20190826195806.15998-1-jcmvbkbc@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] target/xtensa: linux-user: add call0 ABI support | expand |
Hi Laurent, On Mon, Aug 26, 2019 at 12:58 PM Max Filippov <jcmvbkbc@gmail.com> wrote: > > 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. > > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > Changes v2->v3: > > - revert to checking PS.WOE in the setup_rt_frame > > Changes v1->v2: > > - move handling of QEMU_XTENSA_ABI_CALL0 to linux-user/main.c > - check xtensa_abi_call0 instead of PS.WOE in the setup_rt_frame > > linux-user/main.c | 17 +++++++++++++++++ > linux-user/xtensa/signal.c | 25 +++++++++++++++++-------- > target/xtensa/cpu.c | 24 ++++++++++++++++++++---- > target/xtensa/cpu.h | 3 +++ > 4 files changed, 57 insertions(+), 12 deletions(-) Could you please take a look at the most recent (v3) version of this change? The corresponding linux change is queued here: https://github.com/jcmvbkbc/linux-xtensa/commit/09f8a6db20e6ed8eab1b2b23d09d2458f6e15062
Le 26/08/2019 à 21:58, 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. > > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > Changes v2->v3: > > - revert to checking PS.WOE in the setup_rt_frame > > Changes v1->v2: > > - move handling of QEMU_XTENSA_ABI_CALL0 to linux-user/main.c > - check xtensa_abi_call0 instead of PS.WOE in the setup_rt_frame > > linux-user/main.c | 17 +++++++++++++++++ > linux-user/xtensa/signal.c | 25 +++++++++++++++++-------- > target/xtensa/cpu.c | 24 ++++++++++++++++++++---- > target/xtensa/cpu.h | 3 +++ > 4 files changed, 57 insertions(+), 12 deletions(-) > > diff --git a/linux-user/main.c b/linux-user/main.c > index 47917bbb20fc..9e50b2d2a92f 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} > }; > > @@ -710,6 +721,12 @@ int main(int argc, char **argv, char **envp) > } > } > > +#if defined(TARGET_XTENSA) > + if (getenv("QEMU_XTENSA_ABI_CALL0")) { > + xtensa_set_abi_call0(); > + } Not needed, this is done by parse_args() that checks getenv(arginfo->env) and calls arginfo->handle_opt() (handle_arg_abi_call0()). Except that, it looks good. Reviewed-by: Laurent Vivier <laurent@vivier.eu>
On Fri, Sep 6, 2019 at 2:33 AM Laurent Vivier <laurent@vivier.eu> wrote: > Le 26/08/2019 à 21:58, Max Filippov a écrit : > > +#if defined(TARGET_XTENSA) > > + if (getenv("QEMU_XTENSA_ABI_CALL0")) { > > + xtensa_set_abi_call0(); > > + } > > Not needed, this is done by parse_args() that checks > getenv(arginfo->env) and calls arginfo->handle_opt() > (handle_arg_abi_call0()). Thank you, I'll fix that. I got confused by the similar handling of QEMU_STRACE, I'll post a cleanup that removes that as well.
Le 06/09/2019 à 18:55, Max Filippov a écrit : > On Fri, Sep 6, 2019 at 2:33 AM Laurent Vivier <laurent@vivier.eu> wrote: >> Le 26/08/2019 à 21:58, Max Filippov a écrit : >>> +#if defined(TARGET_XTENSA) >>> + if (getenv("QEMU_XTENSA_ABI_CALL0")) { >>> + xtensa_set_abi_call0(); >>> + } >> >> Not needed, this is done by parse_args() that checks >> getenv(arginfo->env) and calls arginfo->handle_opt() >> (handle_arg_abi_call0()). > > Thank you, I'll fix that. I got confused by the similar handling of > QEMU_STRACE, I'll post a cleanup that removes that as well. Good point. It seems getenv("QEMU_STRACE") and getenv("QEMU_RAND_SEED") could be removed. Thanks, Laurent
diff --git a/linux-user/main.c b/linux-user/main.c index 47917bbb20fc..9e50b2d2a92f 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} }; @@ -710,6 +721,12 @@ int main(int argc, char **argv, char **envp) } } +#if defined(TARGET_XTENSA) + if (getenv("QEMU_XTENSA_ABI_CALL0")) { + xtensa_set_abi_call0(); + } +#endif + target_environ = envlist_to_environ(envlist, NULL); envlist_free(envlist); 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)
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. Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> --- Changes v2->v3: - revert to checking PS.WOE in the setup_rt_frame Changes v1->v2: - move handling of QEMU_XTENSA_ABI_CALL0 to linux-user/main.c - check xtensa_abi_call0 instead of PS.WOE in the setup_rt_frame linux-user/main.c | 17 +++++++++++++++++ linux-user/xtensa/signal.c | 25 +++++++++++++++++-------- target/xtensa/cpu.c | 24 ++++++++++++++++++++---- target/xtensa/cpu.h | 3 +++ 4 files changed, 57 insertions(+), 12 deletions(-)