[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.
- Re: [Qemu-arm] [PATCH v3 08/22] target/arm: Support multiple EL change hooks, (continued)
[Qemu-arm] [PATCH v3 10/22] target/arm: Allow EL change hooks to do IO, Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 09/22] target/arm: Add pre-EL change hooks, Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 11/22] target/arm: Fix bitmask for PMCCFILTR writes, Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 14/22] target/arm: Make PMOVSCLR 64 bits wide, Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 12/22] target/arm: Filter cycle counter based on PMCCFILTR_EL0, Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 13/22] target/arm: Allow AArch32 access for PMCCFILTR, Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 15/22] target/arm: Add ARM_FEATURE_V7VE for v7 Virtualization Extensions, Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 18/22] target/arm: Add array for supported PMU events, generate PMCEID[01], Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 17/22] target/arm: Split arm_ccnt_enabled into generic pmu_counter_enabled, Aaron Lindsay, 2018/03/16
[Qemu-arm] [PATCH v3 20/22] target/arm: PMU: Add instruction and cycle events, Aaron Lindsay, 2018/03/16