[Qemu-devel] [PATCH] user-exec.c: Set is_write correctly in the ARM cpu_signal_handler()

[Qemu-devel] [PATCH] user-exec.c: Set is_write correctly in the ARM cpu_signal_handler()

[Peter Maydell]

In the ARM implementation of cpu_signal_handler(), set is_write
correctly using the FSR value which the kernel passes us in the
error_code field of uc_mcontext. Since the WnR bit of the FSR was
only introduced in ARMv6, this means that v5 cores will continue
to behave as before this patch, but they are not really supported
as hosts for linux-user mode anyway since they do not have the
modern behaviour for unaligned accesses.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Without this linux-user won't work very well. In particular after
fork() bash will segfault, with this in the QEMU_STRACE output
immediately preceding:
 sigreturn(18,4390912,1082130608,0,0,0) = -1 errno=255 (Unknown error 255)
at least for PPC and MIPSEL guests.

 user-exec.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/user-exec.c b/user-exec.c
index 71bd6c5..336ac70 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -20,6 +20,7 @@
 #include "cpu.h"
 #include "disas/disas.h"
 #include "tcg.h"
+#include "qemu/bitops.h"
 
 #undef EAX
 #undef ECX
@@ -441,8 +442,11 @@ int cpu_signal_handler(int host_signum, void *pinfo,
 #else
     pc = uc->uc_mcontext.arm_pc;
 #endif
-    /* XXX: compute is_write */
-    is_write = 0;
+
+    /* error_code is the FSR value, in which bit 11 is WnR (assuming a v6 or
+     * later processor; on v5 we will always report this as a read).
+     */
+    is_write = extract32(uc->uc_mcontext.error_code, 11, 1);
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              is_write,
                              &uc->uc_sigmask, puc);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] user-exec.c: Set is_write correctly in the ARM cpu_signal_handler()

[Peter Maydell]

Ping!

thanks
-- PMM

On 4 June 2013 14:31, Peter Maydell <peter.maydell@linaro.org> wrote:
> In the ARM implementation of cpu_signal_handler(), set is_write
> correctly using the FSR value which the kernel passes us in the
> error_code field of uc_mcontext. Since the WnR bit of the FSR was
> only introduced in ARMv6, this means that v5 cores will continue
> to behave as before this patch, but they are not really supported
> as hosts for linux-user mode anyway since they do not have the
> modern behaviour for unaligned accesses.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Without this linux-user won't work very well. In particular after
> fork() bash will segfault, with this in the QEMU_STRACE output
> immediately preceding:
>  sigreturn(18,4390912,1082130608,0,0,0) = -1 errno=255 (Unknown error 255)
> at least for PPC and MIPSEL guests.
>
>  user-exec.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/user-exec.c b/user-exec.c
> index 71bd6c5..336ac70 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -20,6 +20,7 @@
>  #include "cpu.h"
>  #include "disas/disas.h"
>  #include "tcg.h"
> +#include "qemu/bitops.h"
>
>  #undef EAX
>  #undef ECX
> @@ -441,8 +442,11 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>  #else
>      pc = uc->uc_mcontext.arm_pc;
>  #endif
> -    /* XXX: compute is_write */
> -    is_write = 0;
> +
> +    /* error_code is the FSR value, in which bit 11 is WnR (assuming a v6 or
> +     * later processor; on v5 we will always report this as a read).
> +     */
> +    is_write = extract32(uc->uc_mcontext.error_code, 11, 1);
>      return handle_cpu_signal(pc, (unsigned long)info->si_addr,
>                               is_write,
>                               &uc->uc_sigmask, puc);
> --
> 1.7.9.5
>
>

Re: [Qemu-devel] [PATCH] user-exec.c: Set is_write correctly in the ARM cpu_signal_handler()

[Andreas Färber]

Am 04.06.2013 15:31, schrieb Peter Maydell:
> In the ARM implementation of cpu_signal_handler(), set is_write
> correctly using the FSR value which the kernel passes us in the
> error_code field of uc_mcontext. Since the WnR bit of the FSR was
> only introduced in ARMv6, this means that v5 cores will continue
> to behave as before this patch, but they are not really supported
> as hosts for linux-user mode anyway since they do not have the
> modern behaviour for unaligned accesses.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Without this linux-user won't work very well. In particular after
> fork() bash will segfault, with this in the QEMU_STRACE output
> immediately preceding:
>  sigreturn(18,4390912,1082130608,0,0,0) = -1 errno=255 (Unknown error 255)
> at least for PPC and MIPSEL guests.
> 
>  user-exec.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/user-exec.c b/user-exec.c
> index 71bd6c5..336ac70 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -20,6 +20,7 @@
>  #include "cpu.h"
>  #include "disas/disas.h"
>  #include "tcg.h"
> +#include "qemu/bitops.h"
>  
>  #undef EAX
>  #undef ECX
> @@ -441,8 +442,11 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>  #else
>      pc = uc->uc_mcontext.arm_pc;
>  #endif
> -    /* XXX: compute is_write */
> -    is_write = 0;
> +
> +    /* error_code is the FSR value, in which bit 11 is WnR (assuming a v6 or
> +     * later processor; on v5 we will always report this as a read).
> +     */
> +    is_write = extract32(uc->uc_mcontext.error_code, 11, 1);

You seem to be relying on v5 and earlier reading zero here - wondering
if that is true for all implementations (OMAP, PXA, etc.)? Safer and
closer to the comment might be an explicit check for v6+ if that were
possible.

Cheers,
Andreas

>      return handle_cpu_signal(pc, (unsigned long)info->si_addr,
>                               is_write,
>                               &uc->uc_sigmask, puc);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] user-exec.c: Set is_write correctly in the ARM cpu_signal_handler()

[Peter Maydell]

On 27 June 2013 17:59, Andreas Färber <afaerber@suse.de> wrote:
> Am 04.06.2013 15:31, schrieb Peter Maydell:
>> +    /* error_code is the FSR value, in which bit 11 is WnR (assuming a v6 or
>> +     * later processor; on v5 we will always report this as a read).
>> +     */
>> +    is_write = extract32(uc->uc_mcontext.error_code, 11, 1);
>
> You seem to be relying on v5 and earlier reading zero here - wondering
> if that is true for all implementations (OMAP, PXA, etc.)? Safer and
> closer to the comment might be an explicit check for v6+ if that were
> possible.

The kernel will clear bit 11 if on a v4 or v5 core, eg:
http://lxr.linux.no/#linux+v3.9.7/arch/arm/mm/abort-ev5t.S#L24

As I say in the commit message, v5 hosts are going to be badly
broken (or possibly just hideously inefficient) because of the
unaligned access thing anyway.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] user-exec.c: Set is_write correctly in the ARM cpu_signal_handler()

[Alexander Graf]

On 04.06.2013, at 15:31, Peter Maydell wrote:

> In the ARM implementation of cpu_signal_handler(), set is_write
> correctly using the FSR value which the kernel passes us in the
> error_code field of uc_mcontext. Since the WnR bit of the FSR was
> only introduced in ARMv6, this means that v5 cores will continue
> to behave as before this patch, but they are not really supported
> as hosts for linux-user mode anyway since they do not have the
> modern behaviour for unaligned accesses.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Acked-by: Alexander Graf <agraf@suse.de>


Alex

> ---
> Without this linux-user won't work very well. In particular after
> fork() bash will segfault, with this in the QEMU_STRACE output
> immediately preceding:
> sigreturn(18,4390912,1082130608,0,0,0) = -1 errno=255 (Unknown error 255)
> at least for PPC and MIPSEL guests.
> 
> user-exec.c |    8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/user-exec.c b/user-exec.c
> index 71bd6c5..336ac70 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -20,6 +20,7 @@
> #include "cpu.h"
> #include "disas/disas.h"
> #include "tcg.h"
> +#include "qemu/bitops.h"
> 
> #undef EAX
> #undef ECX
> @@ -441,8 +442,11 @@ int cpu_signal_handler(int host_signum, void *pinfo,
> #else
>     pc = uc->uc_mcontext.arm_pc;
> #endif
> -    /* XXX: compute is_write */
> -    is_write = 0;
> +
> +    /* error_code is the FSR value, in which bit 11 is WnR (assuming a v6 or
> +     * later processor; on v5 we will always report this as a read).
> +     */
> +    is_write = extract32(uc->uc_mcontext.error_code, 11, 1);
>     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
>                              is_write,
>                              &uc->uc_sigmask, puc);
> -- 
> 1.7.9.5
> 
>