From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32850) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsmXc-0003Lb-RN for qemu-devel@nongnu.org; Mon, 16 Dec 2013 23:46:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VsmXV-000511-6X for qemu-devel@nongnu.org; Mon, 16 Dec 2013 23:45:56 -0500 Received: from mail-pb0-f52.google.com ([209.85.160.52]:45968) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsmXU-00050d-TR for qemu-devel@nongnu.org; Mon, 16 Dec 2013 23:45:49 -0500 Received: by mail-pb0-f52.google.com with SMTP id uo5so6448231pbc.39 for ; Mon, 16 Dec 2013 20:45:47 -0800 (PST) Date: Mon, 16 Dec 2013 20:45:46 -0800 From: Christoffer Dall Message-ID: <20131217044546.GH5711@cbox> References: <1385645602-18662-1-git-send-email-peter.maydell@linaro.org> <1385645602-18662-3-git-send-email-peter.maydell@linaro.org> <20131216233932.GB5711@cbox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Patch Tracking , QEMU Developers , "kvmarm@lists.cs.columbia.edu" On Tue, Dec 17, 2013 at 12:15:54AM +0000, Peter Maydell wrote: > On 16 December 2013 23:39, Christoffer Dall wrote: > > On Thu, Nov 28, 2013 at 01:33:17PM +0000, Peter Maydell wrote > >> --- a/target-arm/cpu.h > >> +++ b/target-arm/cpu.h > >> @@ -113,8 +113,13 @@ typedef struct CPUARMState { > >> /* Regs for A64 mode. */ > >> uint64_t xregs[32]; > >> uint64_t pc; > >> - /* TODO: pstate doesn't correspond to an architectural register; > >> - * it would be better modelled as the underlying fields. > >> + /* PSTATE isn't an architectural register for ARMv8. However, it is > >> + * convenient for us to assemble the underlying state into a 32 bit format > >> + * identical to the architectural format used for the SPSR. (This is also > >> + * what the Linux kernel's 'pstate' field in signal handlers and KVM's > >> + * 'pstate' register are.) NZCV are kept in their split fields; aarch64 > >> + * is an inverted split version of PSTATE.nRW (aka M[4]); other bits are > >> + * stored here when in AArch64. > > > > I really don't understand what you are trying to say beginning with > > aarch64 is an inverted split version... > > When PSTATE.nRW == 1, aarch64 == 0; when PSTATE.nRW == 0, > aarch64 == 1. That is, aarch64 is the nRW bit inverted. Some descriptions > of the SPSR format call the nRW bit the 4th bit of the mode field, M[4]. > ah, got it, thanks. > > Which other bits are stored > > where in AArch64? > > All the bits not listed specifically in the first half of the sentence, ie > everything except nzcv and nRW, are stored here, ie in the > "uint32_t pstate" field this comment is documenting. ok, it makes sense with the above. I think this could be written slightly more clearly for the uninitiated, but maybe I'm just not qemu-savy enough. > > > Also what is the rationale behind keeping NZCV in their split fields? > > TCG generated code is faster: there are some neat sequences for > generating the correct flags results from arithmetic and logic ops > which you can use (and which are the rationale for the rather odd > definitions of the cpu_NF/ZF/CF/VF). aarch64 we keep separate > partly for historical reasons and partly because there's a whole > load of places in the C code that want to test it. > ok, I sort of figured when I read the pstate_read/write functions, but thanks for the clarification. > >> */ > >> uint32_t pstate; > >> uint32_t aarch64; /* 1 if CPU is in aarch64 state; inverse of PSTATE.nRW */ > >> +/* Return the current PSTATE value. For the moment we don't support 32<->64 bit > >> + * interprocessing, so we don't attempt to sync with the cpsr state used by > >> + * the 32 bit decoder. > >> + */ > >> +static inline uint32_t pstate_read(CPUARMState *env) > >> +{ > >> + int ZF; > >> + > >> + ZF = (env->ZF == 0); > > > > So the comment on the ZF field means "if th ZF field is zero, then the > > pstate.Z field is set, meaning the result of an op was zero". Crystal > > clear. > > Yes. If you're generating TCG ops this means you can calculate > the correct value of cpu_ZF by just copying the result into cpu_ZF > if it's a 32 bit op. 64 bit code is not quite as nice but it's not > terrible. > ok, I think the comment on the ZF field cna be misread in a number of ways, but it has been around forever, so it was good enough for others I guess. Thanks for explaining. -Christoffer