On 03/19/2018 04:31 PM, Peter Maydell wrote:
> 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".
Ok, good to know. Thanks both for your explanation :)
Phil.
