| Message ID | 20240801220328.941866-1-alexrichardson@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: add support for 64-bit PMCCNTR in AArch32 mode | expand |
On Fri, 2 Aug 2024 at 02:00, Alex Richardson <alexrichardson@google.com> wrote: > > See https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en > > Signed-off-by: Alex Richardson <alexrichardson@google.com> > --- > target/arm/helper.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 8fb4b474e8..94900667c3 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -5952,6 +5952,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = { > .access = PL1_RW, .accessfn = access_trap_aa32s_el1, > .writefn = sdcr_write, > .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) }, > + { .name = "PMCCNTR", .state = ARM_CP_STATE_AA32, > + .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT, > + .cp = 15, .crm = 9, .opc1 = 0, > + .access = PL0_RW, .resetvalue = 0, .fgt = FGT_PMCCNTR_EL0, > + .readfn = pmccntr_read, .writefn = pmccntr_write, > + .accessfn = pmreg_access_ccntr }, > }; > > /* These are present only when EL1 supports AArch32 */ > -- > 2.46.0.rc2.264.g509ed76dc8-goog Coincidentally I'd also noticed recently that we don't implement the 64-bit accessor for PMCCNTR, but I hadn't got round to writing a patch, so thanks for doing this. The 64-bit AArch32 PMCCNTR was added in v8 with the PMUv3, and presumably most guests which use the PMU in AArch32 code want to retain the ability to work with PMUv1 or v2 (or were simply written for PMUv1/v2 and never updated), so they stick to the 32-bit sysreg, which is why we haven't noticed this bug before. There is an argument that we should gate this on ARM_FEATURE_PMU being set (or on an ID register test for the PMU version field being at least 3), but we don't currently do that for the existing PMU registers, which we just add regardless for v7 CPUs. So I think we should consider that a separate cleanup, and this is OK. I've applied this to target-arm.next (with an expansion of the commit message to note some of the above). thanks -- PMM
On Thu, 8 Aug 2024 at 14:03, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 2 Aug 2024 at 02:00, Alex Richardson <alexrichardson@google.com> wrote: > > > > See https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en > > > > Signed-off-by: Alex Richardson <alexrichardson@google.com> > > --- > > target/arm/helper.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/target/arm/helper.c b/target/arm/helper.c > > index 8fb4b474e8..94900667c3 100644 > > --- a/target/arm/helper.c > > +++ b/target/arm/helper.c > > @@ -5952,6 +5952,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = { > > .access = PL1_RW, .accessfn = access_trap_aa32s_el1, > > .writefn = sdcr_write, > > .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) }, > > + { .name = "PMCCNTR", .state = ARM_CP_STATE_AA32, > > + .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT, > > + .cp = 15, .crm = 9, .opc1 = 0, > > + .access = PL0_RW, .resetvalue = 0, .fgt = FGT_PMCCNTR_EL0, > > + .readfn = pmccntr_read, .writefn = pmccntr_write, > > + .accessfn = pmreg_access_ccntr }, > > }; > > > > /* These are present only when EL1 supports AArch32 */ > I've applied this to target-arm.next (with an expansion > of the commit message to note some of the above). It turns out that this breaks 'make check-tcg', because when we generate the XML for the system registers for the gdbstub we want to not have any duplicate register names. (See thread at: https://lore.kernel.org/qemu-devel/20240809180835.1243269-1-peter.maydell@linaro.org/T/#ma8051fd02da69ed0cac44e898ac0a61c131ff561 ) So I'm dropping this for the moment. thanks -- PMM
diff --git a/target/arm/helper.c b/target/arm/helper.c index 8fb4b474e8..94900667c3 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -5952,6 +5952,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .access = PL1_RW, .accessfn = access_trap_aa32s_el1, .writefn = sdcr_write, .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) }, + { .name = "PMCCNTR", .state = ARM_CP_STATE_AA32, + .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT, + .cp = 15, .crm = 9, .opc1 = 0, + .access = PL0_RW, .resetvalue = 0, .fgt = FGT_PMCCNTR_EL0, + .readfn = pmccntr_read, .writefn = pmccntr_write, + .accessfn = pmreg_access_ccntr }, }; /* These are present only when EL1 supports AArch32 */
See https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en Signed-off-by: Alex Richardson <alexrichardson@google.com> --- target/arm/helper.c | 6 ++++++ 1 file changed, 6 insertions(+)