On 19 March 2018 at 15:24, Aaron Lindsay <alind...@codeaurora.org> 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 <alind...@codeaurora.org>
>> > ---
>> > 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
