On 30 May 2014 07:52, Paolo Bonzini <pbonz...@redhat.com> wrote:
> Il 29/05/2014 22:21, Peter Maydell ha scritto:
>> 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
>> accesses.
>
> 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.
Um. This feels like we're wrongly overloading this flag for
more than one thing. "Is the user-mode binary BE8?" is
definitely not a property of the CPU, so it shouldn't be
a CPU state flag. (Conversely, "is the iside endianness the
opposite way round to TARGET_WORDS_BIGENDIAN" is a CPU
property of sorts.) It seems to me that we probably want
to fix this by more correctly modelling the actual CPU
state involved here, by having user-mode either set or
not set SCTLR.B [set only if BE32 binary], and the data
and insn fetches honour both that and CPSR.E appropriately.)
thanks
-- PMM
