1. Patchew
  2. QEMU
  3. View series

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

Max Filippov posted 1 patch 4 years, 8 months ago
Test docker-clang@ubuntu passed
Test FreeBSD passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190825221704.1574-1-jcmvbkbc@gmail.com
Maintainers: Max Filippov <jcmvbkbc@gmail.com>, Riku Voipio <riku.voipio@iki.fi>, Laurent Vivier <laurent@vivier.eu>
There is a newer version of this series
linux-user/xtensa/signal.c | 25 +++++++++++++++++--------
target/xtensa/cpu.c        | 22 ++++++++++++++++++----
2 files changed, 35 insertions(+), 12 deletions(-)
[Qemu-devel] [PATCH] target/xtensa: linux-user: add call0 ABI support
Posted by Max Filippov 4 years, 8 months ago 
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. Provide an environment variable QEMU_XTENSA_ABI_CALL0 and
use it to initialize PS.WOE in xtensa_cpu_reset. Check this flag in
setup_rt_frame to determine how a signal should be delivered.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 linux-user/xtensa/signal.c | 25 +++++++++++++++++--------
 target/xtensa/cpu.c        | 22 ++++++++++++++++++----
 2 files changed, 35 insertions(+), 12 deletions(-)

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..791c061880e7 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -53,6 +53,18 @@ static bool xtensa_cpu_has_work(CPUState *cs)
 #endif
 }
 
+#ifdef CONFIG_USER_ONLY
+static int xtensa_abi_call0(void)
+{
+    static int abi_call0 = -1;
+
+    if (abi_call0 == -1) {
+        abi_call0 = getenv("QEMU_XTENSA_ABI_CALL0") != NULL;
+    }
+    return abi_call0;
+}
+#endif
+
 /* CPUClass::reset() */
 static void xtensa_cpu_reset(CPUState *s)
 {
@@ -70,10 +82,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;
-- 
2.11.0
Re: [Qemu-devel] [PATCH] target/xtensa: linux-user: add call0 ABI support
Posted by Laurent Vivier 4 years, 8 months ago 
Le 26/08/2019 à 00:17, 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. Provide an environment variable QEMU_XTENSA_ABI_CALL0 and
> use it to initialize PS.WOE in xtensa_cpu_reset. Check this flag in
> setup_rt_frame to determine how a signal should be delivered.
> 
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  linux-user/xtensa/signal.c | 25 +++++++++++++++++--------
>  target/xtensa/cpu.c        | 22 ++++++++++++++++++----
>  2 files changed, 35 insertions(+), 12 deletions(-)
> 
> 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;

Is this safe to rely on content of sregs[PS]?
Why don't you use xtensa_abi_call0()?

> +    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..791c061880e7 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -53,6 +53,18 @@ static bool xtensa_cpu_has_work(CPUState *cs)
>  #endif
>  }
>  
> +#ifdef CONFIG_USER_ONLY
> +static int xtensa_abi_call0(void)
> +{
> +    static int abi_call0 = -1;
> +
> +    if (abi_call0 == -1) {
> +        abi_call0 = getenv("QEMU_XTENSA_ABI_CALL0") != NULL;

Wouldn't it be cleaner to add this in the arg_table[] in
linux-user/main.c and then use directly the variable?

Thanks,
Laurent
Re: [Qemu-devel] [PATCH] target/xtensa: linux-user: add call0 ABI support
Posted by Max Filippov 4 years, 8 months ago 
On Mon, Aug 26, 2019 at 3:27 AM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 26/08/2019 à 00:17, 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. Provide an environment variable QEMU_XTENSA_ABI_CALL0 and
> > use it to initialize PS.WOE in xtensa_cpu_reset. Check this flag in
> > setup_rt_frame to determine how a signal should be delivered.
> >
> > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> > ---
> >  linux-user/xtensa/signal.c | 25 +++++++++++++++++--------
> >  target/xtensa/cpu.c        | 22 ++++++++++++++++++----
> >  2 files changed, 35 insertions(+), 12 deletions(-)
> >
> > 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;
>
> Is this safe to rely on content of sregs[PS]?

It should be: PS is a privileged register, PS.WOE can only
be changed explicitly and windowed registers are not supposed to work
when PS.WOE is cleared.

> Why don't you use xtensa_abi_call0()?

I actually used it in early version of this change, but then
decided that PS.WOE might be a better indicator for the call0 ABI.

> > +    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..791c061880e7 100644
> > --- a/target/xtensa/cpu.c
> > +++ b/target/xtensa/cpu.c
> > @@ -53,6 +53,18 @@ static bool xtensa_cpu_has_work(CPUState *cs)
> >  #endif
> >  }
> >
> > +#ifdef CONFIG_USER_ONLY
> > +static int xtensa_abi_call0(void)
> > +{
> > +    static int abi_call0 = -1;
> > +
> > +    if (abi_call0 == -1) {
> > +        abi_call0 = getenv("QEMU_XTENSA_ABI_CALL0") != NULL;
>
> Wouldn't it be cleaner to add this in the arg_table[] in
> linux-user/main.c and then use directly the variable?

Probably. Will do so in v2.

-- 
Thanks.
-- Max

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.