Re: [Qemu-devel] [PATCH v2 01/35] target-arm: Fix raw read and write functions on AArch64 registers

[Peter Maydell]

On 31 January 2014 16:06, Peter Maydell <address@hidden> wrote:
> On 31 January 2014 15:56, Rob Herring <address@hidden> wrote:
>> On 31 January 2014 09:45, Peter Maydell <address@hidden> wrote:
>>> +/* Return true if this reginfo struct's field in the cpu state struct
>>> + * is 64 bits wide.
>>> + */
>>> +static inline bool cpreg_field_is_64bit(const ARMCPRegInfo *ri)
>>> +{
>>> +    return (ri->state == ARM_CP_STATE_AA64) || (ri->type & ARM_CP_64BIT);
>>
>> Won't this fail when state is ARM_CP_STATE_BOTH? That was what I found
>> in testing as TTBR writes were not causing a tlb_flush.
>
> Hmm. You're right that this won't work as it stands.
> We could either fix this condition or we could make the code
> that puts reginfo structs into the hashtable fix the state
> so that the reginfo for the AArch64 register said _AA64
> and the one for AArch32 said _AA32.
>
> (And/or we could make that code force ARM_CP_64BIT for the AArch64
> entry, but I felt it would be a bit confusing having that be
> present on none of the structs in the source code but on all
> of them at runtime.)
>
> Anybody got a preference?

On further thought, this has to be done by making the reginfo
structs in the hashtable different for the AArch32 and AArch64
views of the register. What the function is attempting to
determine is "is the view of the register corresponding to
this reginfo 32 or 64 bits wide?" (in the 32 bit case it
could well be that the field is stored in half of a uint64_t,
but still raw accesses to the field should be doing 32 bit
loads/stores, not 64 bit). So we must do something to the
copies of a STATE_BOTH reginfo as we feed them into the hash
table so that they are distinct. Changing the ri->state seems
most sensible.

thanks
-- PMM