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.

>
> 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.

>  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

Reply via email to