[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: |
Aaron Lindsay |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH v3 14/22] target/arm: Make PMOVSCLR 64 bits wide |
Date: |
Mon, 19 Mar 2018 11:24:27 -0400 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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). See
an earlier email from Peter from v2 of this patchset:
https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03983.html
Does this still look wrong to you? If so, I'll take a more thorough look
into how these accesses work.
> > uint32_t c9_pmuserenr; /* perf monitor user enable */
Whatever we decide should likely be done to PMUSERENR too - I think I
overlooked this one before.
> > uint64_t c9_pmselr; /* perf monitor counter selection register */
> > uint64_t c9_pminten; /* perf monitor interrupt enables */
> >
>
> Regards,
>
> Phil.
-Aaron
--
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
[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