Re: [Qemu-devel] [PATCH v3 14/22] target/arm: Make PMOVSCLR 64 bits wide

[Peter Maydell]

On 19 March 2018 at 15:24, Aaron Lindsay <address@hidden> wrote:
> Phil,
>
> On Mar 19 00:14, Philippe Mathieu-Daudé wrote:
>> Hi Aaron,
>>
>> On 03/16/2018 09:31 PM, Aaron Lindsay wrote:
>> > This is a bug fix to ensure 64-bit reads of this register don't read
>> > adjacent data.
>> >
>> > Signed-off-by: Aaron Lindsay <address@hidden>
>> > ---
>> >  target/arm/cpu.h | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> > index 9c3b5ef..fb2f983 100644
>> > --- a/target/arm/cpu.h
>> > +++ b/target/arm/cpu.h
>> > @@ -367,7 +367,7 @@ typedef struct CPUARMState {
>> >          uint32_t c9_data;
>> >          uint64_t c9_pmcr; /* performance monitor control register */
>> >          uint64_t c9_pmcnten; /* perf monitor counter enables */
>> > -        uint32_t c9_pmovsr; /* perf monitor overflow status */
>> > +        uint64_t c9_pmovsr; /* perf monitor overflow status */
>>
>> This doesn't look correct, since this reg is 32b.
>>
>> I *think* the correct fix is in ARMCPRegInfo v7_cp_reginfo[]:
>>
>>     { .name = "PMOVSR", ...
>> -     ..., .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr),
>> +     ..., .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmovsr),
>>       .accessfn = pmreg_access,
>>       .writefn = pmovsr_write,
>>       .raw_writefn = raw_write },
>
> Nearly all of these PMU registers are 32 bits wide, but most of them are
> implemented as 64-bit registers (PMCR, PMCNTEN*, PMSELR, PMINTEN* are a
> few examples I see in this patch's context). My understanding is that
> AArch64 register accesses are handled as 64 bits, even if the register
> itself isn't that wide (though I haven't personally verified this).

Correct. Technically there's no such thing as a 32-bit wide AArch64
system register -- that is just a shorthand in the Arm ARM for
"64-bit wide with the top 32-bits being RES0".

thanks
-- PMM