Re: [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE

[Peter Maydell]

On 16 December 2013 23:39, Christoffer Dall <address@hidden> 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].

> 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.

> 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.

>>       */
>>      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.

-- PMM