qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH v3 14/22] target/arm: Make PMOVSCLR 64 bits wide
Date: Tue, 20 Mar 2018 02:01:31 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 03/19/2018 04:31 PM, Peter Maydell wrote:
> 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".

Ok, good to know. Thanks both for your explanation :)

Phil.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]