Re: [PATCH] target/arm: Configure number of pmu counters

[Peter Maydell]

On Mon, 31 Aug 2020 at 12:44, Sai Pavan Boddu
<sai.pavan.boddu@xilinx.com> wrote:
>
> Default the pmu counters to 4 and configure it a 6 for a53 cores.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
>  target/arm/cpu.c    | 3 +++
>  target/arm/cpu.h    | 3 +++
>  target/arm/cpu64.c  | 1 +
>  target/arm/helper.c | 2 +-
>  4 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 6b382fc..805a692 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1051,6 +1051,9 @@ static void arm_cpu_initfn(Object *obj)
>      cpu->psci_version = 1; /* By default assume PSCI v0.1 */
>      cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE;
>
> +    /* set number of pmu counters to 4 */
> +    cpu->pmcrn = 4;
> +
>      if (tcg_enabled()) {
>          cpu->psci_version = 2; /* TCG implements PSCI 0.2 */
>      }
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index ac857bd..3b47ba8 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -879,6 +879,9 @@ struct ARMCPU {
>       */
>      int32_t core_count;
>
> +    /* Number of pmu counters */
> +    uint8_t pmcrn;
> +
>      /* The instance init functions for implementation-specific subclasses
>       * set these fields to specify the implementation-dependent values of
>       * various constant registers and reset values of non-constant

Rather than doing this, I think the better approach would be to
switch to treating PMCR_EL0 as an ID register in the same way
we do for other varies-per-CPU ID registers:

 * new field uint64_t pmcr_el0 in the ARMISARegisters sub-struct of ARMCPU
 * each CPU's initfn needs to set this (sorry, this is going to mean
   a fair amount of digging through TRMs to find the right values)
 * define_pmu_regs() just uses the isar.pmcr_el0 field as the resetvalue
   for PMCR_EL0
 * define_pmu_regs() needs to use pmu_num_counters() to get the number
   of counters for its loops

Side note: looking at the code I see we don't implement the effect
that MDCR_EL2.HPMN has on this register... (and probably we don't
in general get the EL2 handling of the PMU right in other places too).

thanks
-- PMM