[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 4/8] linux-user: arm: handle CPSR.E correctly

From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH v2 4/8] linux-user: arm: handle CPSR.E correctly in strex emulation
Date: Fri, 30 May 2014 08:52:13 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0

Il 29/05/2014 22:21, Peter Maydell ha scritto:
On 29 May 2014 20:46, Paolo Bonzini <address@hidden> wrote:
Now that CPSR.E is set correctly, prepare for when setend will be able
to change it; bswap data in and out of strex manually by comparing
bswap_code to CPSR.E (we do not have the luxury of using TCGMemOps).

Signed-off-by: Paolo Bonzini <address@hidden>
 linux-user/main.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 8eb910a..b129a2b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -449,6 +449,38 @@ void cpu_loop(CPUX86State *env)
         __r;                                            \

+#define get_user_data_u32(x, gaddr, env)                \
+    ({ abi_long __r = get_user_u32((x), (gaddr));       \
+        if (!__r && (env)->bswap_code != !!((env)->uncached_cpsr & CPSR_E)) { \
+            (x) = bswap32(x);                           \
+        }                                               \
+        __r;                                            \
+    })

This looks bogus. bswap_code doesn't have anything to do
with whether data should be byteswapped. Consider the
ARMv5 big-endian code, which qemu-armeb also supports:
there both code and data are big-endian, and bswap_code
is false. bswap_code should only be consulted for iside

This is correct. It is not very intuitive, but a XOR does exactly what we want.

For v3 I'll wrap it into an inline function like this:

/* get_user and put_user respectivaly return and expect data according
 * to TARGET_WORDS_BIGENDIAN, but ldrex/strex emulation needs to take
 * into account CPSR.E too.  It turns out that a XOR gives exactly the
 * required result:
 *             TARGET_WORDS_BIGENDIAN  bswap_code  CPSR.E    need swap?
 *   LE/LE               no                0        0          no
 *   LE/BE               no                0        1          yes
 *   BE8/LE              yes               1        0          yes
 *   BE8/BE              yes               1        1          no
 *   BE32/BE             yes               0        0          no
static inline bool swap_get_put_user(CPUARMState *env)
    return env->bswap_code ^ !!(env->uncached_cpsr & CPSR_E);

The reason is that bswap_code is about more than just code endianness, it affects get_user as well. It more generally means "is the endianness given by TARGET_WORDS_BIGENDIAN the opposite of what we get at CPSR.E=0?", and it affects the setting of CPSR.E=1 in the signal handlers, as you pointed out in the review of patch 2. I'll prepend a patch that renames bswap_code to be8_user, since in the end BE8 binaries on qemu-armeb are the only case where it is true.

Instead, BE32 is handled incorrectly for TCG-generated accesses. I'll fix it for v3.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]