Re: [Qemu-devel] [PATCH v3 04/11] linux-user: arm: set CPSR.E correctly for BE8 mode

[Peter Maydell <peter.maydell@linaro.org>]

On 23 June 2015 at 21:30, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Tue, Jun 23, 2015 at 11:54 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 23 June 2015 at 19:43, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>> I'm confused. arm_cpu_is_big_endian() tells you whether the CPU
>> is *currently* big-endian or not. That doesn't help you with
>> answering the question "I'm about to run a signal handler; what
>> should I set the CPSR.E bit to?" in linux-user mode. That's
>> what signal_cpsr_e does.
>>
>
> arm_cpu_is_bigendian is the consumer of this information. We still
> need some state for signal_cpsr_e, just the question is what state
> does that set. If we reuse arm_cpu_is_big_endian, then signal_cpsr_e
> (or its rename) needs to drive CPSR.E as well as SCTLR.E0E.

That doesn't make sense to me. arm_cpu_is_bigendian() is asking
a runtime question about the current state of the CPU.
signal_cpsr_e is a set-once-at-startup bit of information about
the environment we're trying to run the program in. The point
at which we want to ask the question "what should we set CPSR.E
to for signal delivery" is when we try to deliver the signal;
it doesn't depend upon the current state of the CPU at all.
And (except at that exact point when we've just set CPSR.E but
haven't run any guest code) the current state of the CPU doesn't
depend on signal_cpsr_e.

>>> That means that this logic would change signal_cpsr_e to a generic
>>> endianess bool that will set both SCTLR_EL1.E0E and CPSR.E at all the
>>> points Paolo is patching. SCTLR.EEs shouldn't need patching as
>>> usermode shouldn't be affected (maybe add an assert in
>>> arm_cpu_big_endian for usermode).
>>
>> I'm not entirely sure what you're suggesting here, but
>> a "generic endianness bool" sounds more confusing than something
>> that's specific about exactly what it's trying to control.
>
> Agree, what I am trying to get away from though is using "CPSR" or any
> other regs in the naming scheme.

Certainly we can fix the naming to be less AArch32-ish. But
in general I think the logic in Paolo's patchset for AArch32
was exactly correct, and we should be trying to extend it
to cover the (actually much simpler) AArch64 cases, not
rewriting it.

thanks
-- PMM