On Tue, Jun 23, 2015 at 11:54 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 23 June 2015 at 19:43, Peter Crosthwaite > <peter.crosthwa...@xilinx.com> wrote: >> On Tue, Jun 23, 2015 at 1:04 AM, Peter Maydell <peter.mayd...@linaro.org> >> wrote: >>> The Linux userland ABI says: >>> (1) the ELF file defines whether an executable is BE8 or not >>> (2) this setting affects: >>> (a) whether we start at the process entry point in BE or LE >>> (b) whether we run signal handlers in BE or LE >>> (c) whether newly cloned threads start in BE or LE >>> >>> signal_cpsr_e is how this patch implements that -- we set it >>> based on the ELF file flags, then set CPSR.E based on it: >>> * in main, for the initial thread >>> * in cpu_clone_regs, for subsequent threads > > Aside: this is a bug in the patch which I noted first time > round with code review -- new threads don't get CPSR.E reset > like this, they inherit the CPSR.E of the thread they're > cloned from. > >>> * in signal.c, for signal handlers > > This is what the flag is really for. > >>> For AArch64 BE we will need something similar. I don't know if >>> there's somewhere more appropriate to store this "what's the >>> ELF file endianness" state, but we do need to keep it somewhere... >>> >> >> So my current thinking is the new state captured in TB flags, >> disas-context and this thing is just a bool for endianess. No sense of >> CPSR.E or SCTLR.xx in the newly added state across the series. The TB >> flag is then based on SCTLR.EE, SCTLR.E0E or CPSR.E depending on >> processor mode. We already have arm_cpu_is_big_endian() to calculate >> this. > > 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 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. > "endianness for data accesses", This can work. "endianness for code accesses", > "BE8 vs BE32", This is probably more complete, as could be an enum for LE, BE8 and BE32. Regards, Peter "setting of TARGET_WORDS_BIGENDIAN", "endianness > to use for signal handlers", "exception endianness" and so on > are all different concepts which can't necessarily be collapsed > into a single "endianness bool". > > thanks > -- PMM >
