From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48689) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7UqN-0004P5-0u for qemu-devel@nongnu.org; Tue, 23 Jun 2015 16:30:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7UqI-0003bJ-3Y for qemu-devel@nongnu.org; Tue, 23 Jun 2015 16:30:54 -0400 Received: from mail-yh0-f41.google.com ([209.85.213.41]:34310) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7UqG-0003aJ-Po for qemu-devel@nongnu.org; Tue, 23 Jun 2015 16:30:49 -0400 Received: by yhnv31 with SMTP id v31so8678347yhn.1 for ; Tue, 23 Jun 2015 13:30:47 -0700 (PDT) MIME-Version: 1.0 Sender: peter.crosthwaite@petalogix.com In-Reply-To: References: <1403355502-12288-1-git-send-email-pbonzini@redhat.com> <1403355502-12288-5-git-send-email-pbonzini@redhat.com> <53AC2B9B.40801@redhat.com> Date: Tue, 23 Jun 2015 13:30:47 -0700 Message-ID: From: Peter Crosthwaite Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v3 04/11] linux-user: arm: set CPSR.E correctly for BE8 mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Paolo Bonzini , QEMU Developers On Tue, Jun 23, 2015 at 11:54 AM, Peter Maydell wrote: > On 23 June 2015 at 19:43, Peter Crosthwaite > wrote: >> On Tue, Jun 23, 2015 at 1:04 AM, Peter Maydell 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 >