On Fri, 25 Jul 2025 at 02:19, Peter Maydell <[email protected]> wrote: > > On Fri, 25 Jul 2025 at 01:10, Alex Richardson <[email protected]> > wrote: > > > > In the PMUv3, a new AArch32 64-bit (MCRR/MRRC) accessor for the > > PMCCNTR was added. In QEMU we forgot to implement this, so only > > provide the 32-bit accessor. Since we have a 64-bit PMCCNTR > > sysreg for AArch64, adding the 64-bit AArch32 version is easy. > > > > We add the PMCCNTR to the v8_cp_reginfo because PMUv3 was added > > in the ARMv8 architecture. This is consistent with how we > > handle the existing PMCCNTR support, where we always implement > > it for all v7 CPUs. This is arguably something we should > > clean up so it is gated on ARM_FEATURE_PMU and/or an ID > > register check for the relevant PMU version, but we should > > do that as its own tidyup rather than being inconsistent between > > this PMCCNTR accessor and the others. > > > > Since the register name is the same as the 32-bit PMCCNTR, we set > > ARM_CP_NO_GDB to avoid generating an invalid GDB XML. > > This is the wrong way around, I think. We should prefer to expose > to GDB the 64-bit view of it, not the 32-bit view, because > it's more comprehensive. Compare handling of the "PAR" definition > in target/arm/helper.c. > Thanks for the reference to the existing code, will send out v3 shortly.
> > > > See > > https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en > > > > Signed-off-by: Alex Richardson <[email protected]> > > --- > > It would have been helpful to mention here what the changes > from v1 were -- I had to go and look up the list archives to > remind myself of why we had to drop v1. Apologies, I usually contribute via code review systems that track changes and am new to contributing to email-based workflows. > > > target/arm/cpregs-pmu.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c > > index 0f295b1376..ef176e4045 100644 > > --- a/target/arm/cpregs-pmu.c > > +++ b/target/arm/cpregs-pmu.c > > @@ -1276,6 +1276,12 @@ void define_pm_cpregs(ARMCPU *cpu) > > .access = PL0_R, .accessfn = pmreg_access, .type = > > ARM_CP_CONST, > > .fgt = FGT_PMCEIDN_EL0, > > .resetvalue = cpu->pmceid1 }, > > + { .name = "PMCCNTR", .state = ARM_CP_STATE_AA32, > > + .cp = 15, .crm = 9, .opc1 = 0, > > + .access = PL0_RW, .accessfn = pmreg_access_ccntr, > > .resetvalue = 0, > > + .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT | > > ARM_CP_NO_GDB, > > + .fgt = FGT_PMCCNTR_EL0, .readfn = pmccntr_read, > > + .writefn = pmccntr_write, }, > > }; > > define_arm_cp_regs(cpu, v8_pm_reginfo); > > } > > -- > > 2.50.1.470.g6ba607880d-goog > > thanks > -- PMM
