On Fri, 25 Jul 2025 at 18:02, Alex Richardson <alexrichard...@google.com> 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 on the 32-bit one to avoid generating an invalid GDB XML.
>
> See 
> https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en
>
> Change v2->v3:
> - Moved ARM_CP_NO_GDB to the 32-bit register if Armv8 is supported
>
> Changes v1->v2:
> - Moved to new file
> - Updated commit message
> - Added ARM_CP_NO_GDB
>
> Signed-off-by: Alex Richardson <alexrichard...@google.com>

Thanks for this patch; I added the following comment tweaks
and have applied it to target-arm.next for 10.1 (and marked
for potential backports to stable branches):

--- a/target/arm/cpregs-pmu.c
+++ b/target/arm/cpregs-pmu.c
@@ -1206,7 +1206,11 @@ void define_pm_cpregs(ARMCPU *cpu)
         define_one_arm_cp_reg(cpu, &pmcr);
         define_one_arm_cp_reg(cpu, &pmcr64);
         define_arm_cp_regs(cpu, v7_pm_reginfo);
-        /* When Armv8 is supported, PMCCNTR aliases the new 64-bit version */
+        /*
+         * 32-bit AArch32 PMCCNTR. We don't expose this to GDB if the
+         * new-in-v8 PMUv3 64-bit AArch32 PMCCNTR register is implemented
+         * (as that will provide the GDB user's view of "PMCCNTR").
+         */
         ARMCPRegInfo pmccntr = {
             .name = "PMCCNTR",
             .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
@@ -1284,6 +1288,7 @@ void define_pm_cpregs(ARMCPU *cpu)
               .access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST,
               .fgt = FGT_PMCEIDN_EL0,
               .resetvalue = cpu->pmceid1 },
+            /* AArch32 64-bit PMCCNTR view: added in PMUv3 with Armv8 */
             { .name = "PMCCNTR", .state = ARM_CP_STATE_AA32,
               .cp = 15, .crm = 9, .opc1 = 0,
               .access = PL0_RW, .accessfn = pmreg_access_ccntr,
.resetvalue = 0,

thanks
-- PMM

Reply via email to