[PATCH] target/arm: Make number of counters in PMCR follow the CPU

[PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Peter Maydell]

Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
means that we don't provide the 6 counters that are required by the
Arm BSA (Base System Architecture) specification if the CPU supports
the Virtualization extensions.

Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
specify the PMCR reset value (obtained from the appropriate TRM), and
use the 'N' field of that value to define the number of counters
provided.

This means that we now supply 6 counters for Cortex-A53, A57, A72,
A15 and A9 as well as '-cpu max'; Cortex-A7 and A8 stay at 4; and
Cortex-R5 goes down to 3.

Note that because we now use the PMCR reset value of the specific
implementation, we no longer set the LC bit out of reset.  This has
an UNKNOWN value out of reset for all cores with any AArch32 support,
so guest software should be setting it anyway if it wants it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is pretty much untested (I just checked Linux still boots;
haven't tried it with KVM either). It's an alternative to
just bumping PMCR_NUM_COUNTERS to 6.
---
 target/arm/cpu.h     |  1 +
 target/arm/cpu64.c   |  3 +++
 target/arm/cpu_tcg.c |  5 +++++
 target/arm/helper.c  | 29 +++++++++++++++++------------
 target/arm/kvm64.c   |  2 ++
 5 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 193a49ec7fa..fe68f464b3a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -942,6 +942,7 @@ struct ARMCPU {
         uint64_t id_aa64mmfr2;
         uint64_t id_aa64dfr0;
         uint64_t id_aa64dfr1;
+        uint64_t reset_pmcr_el0;
     } isar;
     uint64_t midr;
     uint32_t revidr;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index f0a9e968c9c..5d9d56a33c3 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -141,6 +141,7 @@ static void aarch64_a57_initfn(Object *obj)
     cpu->gic_num_lrs = 4;
     cpu->gic_vpribits = 5;
     cpu->gic_vprebits = 5;
+    cpu->isar.reset_pmcr_el0 = 0x41013000;
     define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
 }
 
@@ -194,6 +195,7 @@ static void aarch64_a53_initfn(Object *obj)
     cpu->gic_num_lrs = 4;
     cpu->gic_vpribits = 5;
     cpu->gic_vprebits = 5;
+    cpu->isar.reset_pmcr_el0 = 0x41033000;
     define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
 }
 
@@ -245,6 +247,7 @@ static void aarch64_a72_initfn(Object *obj)
     cpu->gic_num_lrs = 4;
     cpu->gic_vpribits = 5;
     cpu->gic_vprebits = 5;
+    cpu->isar.reset_pmcr_el0 = 0x41023000;
     define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
 }
 
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 046e476f65f..8252fd29f90 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -301,6 +301,7 @@ static void cortex_a8_initfn(Object *obj)
     cpu->ccsidr[1] = 0x2007e01a; /* 16k L1 icache. */
     cpu->ccsidr[2] = 0xf0000000; /* No L2 icache. */
     cpu->reset_auxcr = 2;
+    cpu->isar.reset_pmcr_el0 = 0x41002000;
     define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
 }
 
@@ -373,6 +374,7 @@ static void cortex_a9_initfn(Object *obj)
     cpu->clidr = (1 << 27) | (1 << 24) | 3;
     cpu->ccsidr[0] = 0xe00fe019; /* 16k L1 dcache. */
     cpu->ccsidr[1] = 0x200fe019; /* 16k L1 icache. */
+    cpu->isar.reset_pmcr_el0 = 0x41093000;
     define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
 }
 
@@ -443,6 +445,7 @@ static void cortex_a7_initfn(Object *obj)
     cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
     cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
     cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
+    cpu->isar.reset_pmcr_el0 = 0x41072000;
     define_arm_cp_regs(cpu, cortexa15_cp_reginfo); /* Same as A15 */
 }
 
@@ -485,6 +488,7 @@ static void cortex_a15_initfn(Object *obj)
     cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
     cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
     cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
+    cpu->isar.reset_pmcr_el0 = 0x410F3000;
     define_arm_cp_regs(cpu, cortexa15_cp_reginfo);
 }
 
@@ -717,6 +721,7 @@ static void cortex_r5_initfn(Object *obj)
     cpu->isar.id_isar6 = 0x0;
     cpu->mp_is_up = true;
     cpu->pmsav7_dregion = 16;
+    cpu->isar.reset_pmcr_el0 = 0x41151800;
     define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
 }
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 904b0927cd2..2f3867cad79 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -38,7 +38,6 @@
 #endif
 
 #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
-#define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */
 
 #ifndef CONFIG_USER_ONLY
 
@@ -1149,7 +1148,9 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
 
 static inline uint32_t pmu_num_counters(CPUARMState *env)
 {
-  return (env->cp15.c9_pmcr & PMCRN_MASK) >> PMCRN_SHIFT;
+    ARMCPU *cpu = env_archcpu(env);
+
+    return (cpu->isar.reset_pmcr_el0 & PMCRN_MASK) >> PMCRN_SHIFT;
 }
 
 /* Bits allowed to be set/cleared for PMCNTEN* and PMINTEN* */
@@ -5753,13 +5754,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .resetvalue = 0,
       .writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
 #endif
-    /* The only field of MDCR_EL2 that has a defined architectural reset value
-     * is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
-     */
-    { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
-      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
-      .access = PL2_RW, .resetvalue = PMCR_NUM_COUNTERS,
-      .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2), },
     { .name = "HPFAR", .state = ARM_CP_STATE_AA32,
       .cp = 15, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
       .access = PL2_RW, .accessfn = access_el3_aa32ns,
@@ -6689,7 +6683,7 @@ static void define_pmu_regs(ARMCPU *cpu)
      * field as main ID register, and we implement four counters in
      * addition to the cycle count register.
      */
-    unsigned int i, pmcrn = PMCR_NUM_COUNTERS;
+    unsigned int i, pmcrn = pmu_num_counters(&cpu->env);
     ARMCPRegInfo pmcr = {
         .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
         .access = PL0_RW,
@@ -6704,10 +6698,10 @@ static void define_pmu_regs(ARMCPU *cpu)
         .access = PL0_RW, .accessfn = pmreg_access,
         .type = ARM_CP_IO,
         .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
-        .resetvalue = (cpu->midr & 0xff000000) | (pmcrn << PMCRN_SHIFT) |
-                      PMCRLC,
+        .resetvalue = cpu->isar.reset_pmcr_el0,
         .writefn = pmcr_write, .raw_writefn = raw_write,
     };
+
     define_one_arm_cp_reg(cpu, &pmcr);
     define_one_arm_cp_reg(cpu, &pmcr64);
     for (i = 0; i < pmcrn; i++) {
@@ -7825,6 +7819,17 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
             REGINFO_SENTINEL
         };
+        /*
+         * The only field of MDCR_EL2 that has a defined architectural reset
+         * value is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
+         */
+        ARMCPRegInfo mdcr_el2 = {
+            .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
+            .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
+            .access = PL2_RW, .resetvalue = pmu_num_counters(env),
+            .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2),
+        };
+        define_one_arm_cp_reg(cpu, &mdcr_el2);
         define_arm_cp_regs(cpu, vpidr_regs);
         define_arm_cp_regs(cpu, el2_cp_reginfo);
         if (arm_feature(env, ARM_FEATURE_V8)) {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index dff85f6db94..581335e49d3 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -566,6 +566,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
                               ARM64_SYS_REG(3, 0, 0, 7, 1));
         err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr2,
                               ARM64_SYS_REG(3, 0, 0, 7, 2));
+        err |= read_sys_reg64(fdarray[2], &ahcf->isar.reset_pmcr_el0,
+                              ARM64_SYS_REG(3, 3, 9, 12, 0));
 
         /*
          * Note that if AArch32 support is not present in the host,
-- 
2.20.1

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Peter Maydell]

Ping for review, testing, opinions on whether this should go into 6.0 ?
I think I would overall prefer it to the just-bump-PMCR_NUM_COUNTERS
patch...

thanks
-- PMM

On Thu, 11 Mar 2021 at 16:59, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
> means that we don't provide the 6 counters that are required by the
> Arm BSA (Base System Architecture) specification if the CPU supports
> the Virtualization extensions.
>
> Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
> specify the PMCR reset value (obtained from the appropriate TRM), and
> use the 'N' field of that value to define the number of counters
> provided.
>
> This means that we now supply 6 counters for Cortex-A53, A57, A72,
> A15 and A9 as well as '-cpu max'; Cortex-A7 and A8 stay at 4; and
> Cortex-R5 goes down to 3.
>
> Note that because we now use the PMCR reset value of the specific
> implementation, we no longer set the LC bit out of reset.  This has
> an UNKNOWN value out of reset for all cores with any AArch32 support,
> so guest software should be setting it anyway if it wants it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is pretty much untested (I just checked Linux still boots;
> haven't tried it with KVM either). It's an alternative to
> just bumping PMCR_NUM_COUNTERS to 6.
> ---
>  target/arm/cpu.h     |  1 +
>  target/arm/cpu64.c   |  3 +++
>  target/arm/cpu_tcg.c |  5 +++++
>  target/arm/helper.c  | 29 +++++++++++++++++------------
>  target/arm/kvm64.c   |  2 ++
>  5 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 193a49ec7fa..fe68f464b3a 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -942,6 +942,7 @@ struct ARMCPU {
>          uint64_t id_aa64mmfr2;
>          uint64_t id_aa64dfr0;
>          uint64_t id_aa64dfr1;
> +        uint64_t reset_pmcr_el0;
>      } isar;
>      uint64_t midr;
>      uint32_t revidr;
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index f0a9e968c9c..5d9d56a33c3 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -141,6 +141,7 @@ static void aarch64_a57_initfn(Object *obj)
>      cpu->gic_num_lrs = 4;
>      cpu->gic_vpribits = 5;
>      cpu->gic_vprebits = 5;
> +    cpu->isar.reset_pmcr_el0 = 0x41013000;
>      define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>  }
>
> @@ -194,6 +195,7 @@ static void aarch64_a53_initfn(Object *obj)
>      cpu->gic_num_lrs = 4;
>      cpu->gic_vpribits = 5;
>      cpu->gic_vprebits = 5;
> +    cpu->isar.reset_pmcr_el0 = 0x41033000;
>      define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>  }
>
> @@ -245,6 +247,7 @@ static void aarch64_a72_initfn(Object *obj)
>      cpu->gic_num_lrs = 4;
>      cpu->gic_vpribits = 5;
>      cpu->gic_vprebits = 5;
> +    cpu->isar.reset_pmcr_el0 = 0x41023000;
>      define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>  }
>
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index 046e476f65f..8252fd29f90 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -301,6 +301,7 @@ static void cortex_a8_initfn(Object *obj)
>      cpu->ccsidr[1] = 0x2007e01a; /* 16k L1 icache. */
>      cpu->ccsidr[2] = 0xf0000000; /* No L2 icache. */
>      cpu->reset_auxcr = 2;
> +    cpu->isar.reset_pmcr_el0 = 0x41002000;
>      define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
>  }
>
> @@ -373,6 +374,7 @@ static void cortex_a9_initfn(Object *obj)
>      cpu->clidr = (1 << 27) | (1 << 24) | 3;
>      cpu->ccsidr[0] = 0xe00fe019; /* 16k L1 dcache. */
>      cpu->ccsidr[1] = 0x200fe019; /* 16k L1 icache. */
> +    cpu->isar.reset_pmcr_el0 = 0x41093000;
>      define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
>  }
>
> @@ -443,6 +445,7 @@ static void cortex_a7_initfn(Object *obj)
>      cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>      cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>      cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
> +    cpu->isar.reset_pmcr_el0 = 0x41072000;
>      define_arm_cp_regs(cpu, cortexa15_cp_reginfo); /* Same as A15 */
>  }
>
> @@ -485,6 +488,7 @@ static void cortex_a15_initfn(Object *obj)
>      cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>      cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>      cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
> +    cpu->isar.reset_pmcr_el0 = 0x410F3000;
>      define_arm_cp_regs(cpu, cortexa15_cp_reginfo);
>  }
>
> @@ -717,6 +721,7 @@ static void cortex_r5_initfn(Object *obj)
>      cpu->isar.id_isar6 = 0x0;
>      cpu->mp_is_up = true;
>      cpu->pmsav7_dregion = 16;
> +    cpu->isar.reset_pmcr_el0 = 0x41151800;
>      define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
>  }
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 904b0927cd2..2f3867cad79 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -38,7 +38,6 @@
>  #endif
>
>  #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
> -#define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */
>
>  #ifndef CONFIG_USER_ONLY
>
> @@ -1149,7 +1148,9 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
>
>  static inline uint32_t pmu_num_counters(CPUARMState *env)
>  {
> -  return (env->cp15.c9_pmcr & PMCRN_MASK) >> PMCRN_SHIFT;
> +    ARMCPU *cpu = env_archcpu(env);
> +
> +    return (cpu->isar.reset_pmcr_el0 & PMCRN_MASK) >> PMCRN_SHIFT;
>  }
>
>  /* Bits allowed to be set/cleared for PMCNTEN* and PMINTEN* */
> @@ -5753,13 +5754,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .resetvalue = 0,
>        .writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
>  #endif
> -    /* The only field of MDCR_EL2 that has a defined architectural reset value
> -     * is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
> -     */
> -    { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
> -      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> -      .access = PL2_RW, .resetvalue = PMCR_NUM_COUNTERS,
> -      .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2), },
>      { .name = "HPFAR", .state = ARM_CP_STATE_AA32,
>        .cp = 15, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
>        .access = PL2_RW, .accessfn = access_el3_aa32ns,
> @@ -6689,7 +6683,7 @@ static void define_pmu_regs(ARMCPU *cpu)
>       * field as main ID register, and we implement four counters in
>       * addition to the cycle count register.
>       */
> -    unsigned int i, pmcrn = PMCR_NUM_COUNTERS;
> +    unsigned int i, pmcrn = pmu_num_counters(&cpu->env);
>      ARMCPRegInfo pmcr = {
>          .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
>          .access = PL0_RW,
> @@ -6704,10 +6698,10 @@ static void define_pmu_regs(ARMCPU *cpu)
>          .access = PL0_RW, .accessfn = pmreg_access,
>          .type = ARM_CP_IO,
>          .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
> -        .resetvalue = (cpu->midr & 0xff000000) | (pmcrn << PMCRN_SHIFT) |
> -                      PMCRLC,
> +        .resetvalue = cpu->isar.reset_pmcr_el0,
>          .writefn = pmcr_write, .raw_writefn = raw_write,
>      };
> +
>      define_one_arm_cp_reg(cpu, &pmcr);
>      define_one_arm_cp_reg(cpu, &pmcr64);
>      for (i = 0; i < pmcrn; i++) {
> @@ -7825,6 +7819,17 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
>              REGINFO_SENTINEL
>          };
> +        /*
> +         * The only field of MDCR_EL2 that has a defined architectural reset
> +         * value is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
> +         */
> +        ARMCPRegInfo mdcr_el2 = {
> +            .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
> +            .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> +            .access = PL2_RW, .resetvalue = pmu_num_counters(env),
> +            .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2),
> +        };
> +        define_one_arm_cp_reg(cpu, &mdcr_el2);
>          define_arm_cp_regs(cpu, vpidr_regs);
>          define_arm_cp_regs(cpu, el2_cp_reginfo);
>          if (arm_feature(env, ARM_FEATURE_V8)) {
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index dff85f6db94..581335e49d3 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -566,6 +566,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>                                ARM64_SYS_REG(3, 0, 0, 7, 1));
>          err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr2,
>                                ARM64_SYS_REG(3, 0, 0, 7, 2));
> +        err |= read_sys_reg64(fdarray[2], &ahcf->isar.reset_pmcr_el0,
> +                              ARM64_SYS_REG(3, 3, 9, 12, 0));
>
>          /*
>           * Note that if AArch32 support is not present in the host,
> --
> 2.20.1

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Marcin Juszkiewicz]

W dniu 11.03.2021 o 17:59, Peter Maydell pisze:
> Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
> means that we don't provide the 6 counters that are required by the
> Arm BSA (Base System Architecture) specification if the CPU supports
> the Virtualization extensions.
> 
> Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
> specify the PMCR reset value (obtained from the appropriate TRM), and
> use the 'N' field of that value to define the number of counters
> provided.
> 
> This means that we now supply 6 counters for Cortex-A53, A57, A72,
> A15 and A9 as well as '-cpu max'; Cortex-A7 and A8 stay at 4; and
> Cortex-R5 goes down to 3.
> 
> Note that because we now use the PMCR reset value of the specific
> implementation, we no longer set the LC bit out of reset.  This has
> an UNKNOWN value out of reset for all cores with any AArch32 support,
> so guest software should be setting it anyway if it wants it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

I checked on A57/A72/max with sbsa-ref machine. Each of them got 7 PMU 
counters reported by both SBSA ACS and Linux 5.3 kernel.

Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

> ---
> This is pretty much untested (I just checked Linux still boots;
> haven't tried it with KVM either). It's an alternative to
> just bumping PMCR_NUM_COUNTERS to 6.
> ---
>   target/arm/cpu.h     |  1 +
>   target/arm/cpu64.c   |  3 +++
>   target/arm/cpu_tcg.c |  5 +++++
>   target/arm/helper.c  | 29 +++++++++++++++++------------
>   target/arm/kvm64.c   |  2 ++
>   5 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 193a49ec7fa..fe68f464b3a 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -942,6 +942,7 @@ struct ARMCPU {
>           uint64_t id_aa64mmfr2;
>           uint64_t id_aa64dfr0;
>           uint64_t id_aa64dfr1;
> +        uint64_t reset_pmcr_el0;
>       } isar;
>       uint64_t midr;
>       uint32_t revidr;
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index f0a9e968c9c..5d9d56a33c3 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -141,6 +141,7 @@ static void aarch64_a57_initfn(Object *obj)
>       cpu->gic_num_lrs = 4;
>       cpu->gic_vpribits = 5;
>       cpu->gic_vprebits = 5;
> +    cpu->isar.reset_pmcr_el0 = 0x41013000;
>       define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>   }
>   
> @@ -194,6 +195,7 @@ static void aarch64_a53_initfn(Object *obj)
>       cpu->gic_num_lrs = 4;
>       cpu->gic_vpribits = 5;
>       cpu->gic_vprebits = 5;
> +    cpu->isar.reset_pmcr_el0 = 0x41033000;
>       define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>   }
>   
> @@ -245,6 +247,7 @@ static void aarch64_a72_initfn(Object *obj)
>       cpu->gic_num_lrs = 4;
>       cpu->gic_vpribits = 5;
>       cpu->gic_vprebits = 5;
> +    cpu->isar.reset_pmcr_el0 = 0x41023000;
>       define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>   }
>   
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index 046e476f65f..8252fd29f90 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -301,6 +301,7 @@ static void cortex_a8_initfn(Object *obj)
>       cpu->ccsidr[1] = 0x2007e01a; /* 16k L1 icache. */
>       cpu->ccsidr[2] = 0xf0000000; /* No L2 icache. */
>       cpu->reset_auxcr = 2;
> +    cpu->isar.reset_pmcr_el0 = 0x41002000;
>       define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
>   }
>   
> @@ -373,6 +374,7 @@ static void cortex_a9_initfn(Object *obj)
>       cpu->clidr = (1 << 27) | (1 << 24) | 3;
>       cpu->ccsidr[0] = 0xe00fe019; /* 16k L1 dcache. */
>       cpu->ccsidr[1] = 0x200fe019; /* 16k L1 icache. */
> +    cpu->isar.reset_pmcr_el0 = 0x41093000;
>       define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
>   }
>   
> @@ -443,6 +445,7 @@ static void cortex_a7_initfn(Object *obj)
>       cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>       cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>       cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
> +    cpu->isar.reset_pmcr_el0 = 0x41072000;
>       define_arm_cp_regs(cpu, cortexa15_cp_reginfo); /* Same as A15 */
>   }
>   
> @@ -485,6 +488,7 @@ static void cortex_a15_initfn(Object *obj)
>       cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>       cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>       cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
> +    cpu->isar.reset_pmcr_el0 = 0x410F3000;
>       define_arm_cp_regs(cpu, cortexa15_cp_reginfo);
>   }
>   
> @@ -717,6 +721,7 @@ static void cortex_r5_initfn(Object *obj)
>       cpu->isar.id_isar6 = 0x0;
>       cpu->mp_is_up = true;
>       cpu->pmsav7_dregion = 16;
> +    cpu->isar.reset_pmcr_el0 = 0x41151800;
>       define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
>   }
>   
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 904b0927cd2..2f3867cad79 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -38,7 +38,6 @@
>   #endif
>   
>   #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
> -#define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */
>   
>   #ifndef CONFIG_USER_ONLY
>   
> @@ -1149,7 +1148,9 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
>   
>   static inline uint32_t pmu_num_counters(CPUARMState *env)
>   {
> -  return (env->cp15.c9_pmcr & PMCRN_MASK) >> PMCRN_SHIFT;
> +    ARMCPU *cpu = env_archcpu(env);
> +
> +    return (cpu->isar.reset_pmcr_el0 & PMCRN_MASK) >> PMCRN_SHIFT;
>   }
>   
>   /* Bits allowed to be set/cleared for PMCNTEN* and PMINTEN* */
> @@ -5753,13 +5754,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>         .resetvalue = 0,
>         .writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
>   #endif
> -    /* The only field of MDCR_EL2 that has a defined architectural reset value
> -     * is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
> -     */
> -    { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
> -      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> -      .access = PL2_RW, .resetvalue = PMCR_NUM_COUNTERS,
> -      .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2), },
>       { .name = "HPFAR", .state = ARM_CP_STATE_AA32,
>         .cp = 15, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
>         .access = PL2_RW, .accessfn = access_el3_aa32ns,
> @@ -6689,7 +6683,7 @@ static void define_pmu_regs(ARMCPU *cpu)
>        * field as main ID register, and we implement four counters in
>        * addition to the cycle count register.
>        */
> -    unsigned int i, pmcrn = PMCR_NUM_COUNTERS;
> +    unsigned int i, pmcrn = pmu_num_counters(&cpu->env);
>       ARMCPRegInfo pmcr = {
>           .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
>           .access = PL0_RW,
> @@ -6704,10 +6698,10 @@ static void define_pmu_regs(ARMCPU *cpu)
>           .access = PL0_RW, .accessfn = pmreg_access,
>           .type = ARM_CP_IO,
>           .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
> -        .resetvalue = (cpu->midr & 0xff000000) | (pmcrn << PMCRN_SHIFT) |
> -                      PMCRLC,
> +        .resetvalue = cpu->isar.reset_pmcr_el0,
>           .writefn = pmcr_write, .raw_writefn = raw_write,
>       };
> +
>       define_one_arm_cp_reg(cpu, &pmcr);
>       define_one_arm_cp_reg(cpu, &pmcr64);
>       for (i = 0; i < pmcrn; i++) {
> @@ -7825,6 +7819,17 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                 .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
>               REGINFO_SENTINEL
>           };
> +        /*
> +         * The only field of MDCR_EL2 that has a defined architectural reset
> +         * value is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
> +         */
> +        ARMCPRegInfo mdcr_el2 = {
> +            .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
> +            .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> +            .access = PL2_RW, .resetvalue = pmu_num_counters(env),
> +            .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2),
> +        };
> +        define_one_arm_cp_reg(cpu, &mdcr_el2);
>           define_arm_cp_regs(cpu, vpidr_regs);
>           define_arm_cp_regs(cpu, el2_cp_reginfo);
>           if (arm_feature(env, ARM_FEATURE_V8)) {
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index dff85f6db94..581335e49d3 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -566,6 +566,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>                                 ARM64_SYS_REG(3, 0, 0, 7, 1));
>           err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr2,
>                                 ARM64_SYS_REG(3, 0, 0, 7, 2));
> +        err |= read_sys_reg64(fdarray[2], &ahcf->isar.reset_pmcr_el0,
> +                              ARM64_SYS_REG(3, 3, 9, 12, 0));
>   
>           /*
>            * Note that if AArch32 support is not present in the host,
> 

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Richard Henderson]

On 3/11/21 10:59 AM, Peter Maydell wrote:
> Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
> means that we don't provide the 6 counters that are required by the
> Arm BSA (Base System Architecture) specification if the CPU supports
> the Virtualization extensions.
> 
> Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
> specify the PMCR reset value (obtained from the appropriate TRM), and
> use the 'N' field of that value to define the number of counters
> provided.
> 
> This means that we now supply 6 counters for Cortex-A53, A57, A72,
> A15 and A9 as well as '-cpu max'; Cortex-A7 and A8 stay at 4; and
> Cortex-R5 goes down to 3.
> 
> Note that because we now use the PMCR reset value of the specific
> implementation, we no longer set the LC bit out of reset.  This has
> an UNKNOWN value out of reset for all cores with any AArch32 support,
> so guest software should be setting it anyway if it wants it.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> This is pretty much untested (I just checked Linux still boots;
> haven't tried it with KVM either). It's an alternative to
> just bumping PMCR_NUM_COUNTERS to 6.
> ---
>   target/arm/cpu.h     |  1 +
>   target/arm/cpu64.c   |  3 +++
>   target/arm/cpu_tcg.c |  5 +++++
>   target/arm/helper.c  | 29 +++++++++++++++++------------
>   target/arm/kvm64.c   |  2 ++
>   5 files changed, 28 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Zenghui Yu]

[+kvmarm, Marc]

On 2021/3/12 0:59, Peter Maydell wrote:
> Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
> means that we don't provide the 6 counters that are required by the
> Arm BSA (Base System Architecture) specification if the CPU supports
> the Virtualization extensions.
> 
> Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
> specify the PMCR reset value (obtained from the appropriate TRM), and
> use the 'N' field of that value to define the number of counters
> provided.
> 
> This means that we now supply 6 counters for Cortex-A53, A57, A72,
> A15 and A9 as well as '-cpu max'; Cortex-A7 and A8 stay at 4; and
> Cortex-R5 goes down to 3.
> 
> Note that because we now use the PMCR reset value of the specific
> implementation, we no longer set the LC bit out of reset.  This has
> an UNKNOWN value out of reset for all cores with any AArch32 support,
> so guest software should be setting it anyway if it wants it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is pretty much untested (I just checked Linux still boots;
> haven't tried it with KVM either). It's an alternative to
> just bumping PMCR_NUM_COUNTERS to 6.

So I've tested it with kvm and I get the following error before
VM startup:

   "qemu-system-aarch64: Failed to retrieve host CPU features"

> ---
>   target/arm/cpu.h     |  1 +
>   target/arm/cpu64.c   |  3 +++
>   target/arm/cpu_tcg.c |  5 +++++
>   target/arm/helper.c  | 29 +++++++++++++++++------------
>   target/arm/kvm64.c   |  2 ++
>   5 files changed, 28 insertions(+), 12 deletions(-)

[...]

> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index dff85f6db94..581335e49d3 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -566,6 +566,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>                                 ARM64_SYS_REG(3, 0, 0, 7, 1));
>           err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr2,
>                                 ARM64_SYS_REG(3, 0, 0, 7, 2));
> +        err |= read_sys_reg64(fdarray[2], &ahcf->isar.reset_pmcr_el0,
> +                              ARM64_SYS_REG(3, 3, 9, 12, 0));

Looks like we tried to access PMCR_EL0 *before* telling kvm that
KVM_ARM_VCPU_PMU_V3 feature should be supported, which is now
refused by kvm [*].

[*] https://git.kernel.org/torvalds/c/11663111cd49

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Peter Maydell]

On Wed, 31 Mar 2021 at 09:59, Zenghui Yu <yuzenghui@huawei.com> wrote:
>
> [+kvmarm, Marc]
>
> On 2021/3/12 0:59, Peter Maydell wrote:
> > Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
> > means that we don't provide the 6 counters that are required by the
> > Arm BSA (Base System Architecture) specification if the CPU supports
> > the Virtualization extensions.

> So I've tested it with kvm and I get the following error before
> VM startup:
>
>    "qemu-system-aarch64: Failed to retrieve host CPU features"
>
> > ---
> >   target/arm/cpu.h     |  1 +
> >   target/arm/cpu64.c   |  3 +++
> >   target/arm/cpu_tcg.c |  5 +++++
> >   target/arm/helper.c  | 29 +++++++++++++++++------------
> >   target/arm/kvm64.c   |  2 ++
> >   5 files changed, 28 insertions(+), 12 deletions(-)
>
> [...]
>
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index dff85f6db94..581335e49d3 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -566,6 +566,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> >                                 ARM64_SYS_REG(3, 0, 0, 7, 1));
> >           err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr2,
> >                                 ARM64_SYS_REG(3, 0, 0, 7, 2));
> > +        err |= read_sys_reg64(fdarray[2], &ahcf->isar.reset_pmcr_el0,
> > +                              ARM64_SYS_REG(3, 3, 9, 12, 0));
>
> Looks like we tried to access PMCR_EL0 *before* telling kvm that
> KVM_ARM_VCPU_PMU_V3 feature should be supported, which is now
> refused by kvm [*].
>
> [*] https://git.kernel.org/torvalds/c/11663111cd49

Oops, that's embarrassing. I should have tested with KVM, and
I forgot the PMU needs special enablement :-(

I'm on holiday right now so I probably won't have time to
look at a fix for rc2. I might just revert this commit.

thanks
-- PMM

RE: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[ishii.shuuichir]

> Thanks for looking up the a64fx register value. You don't need to
> do anything more -- I'll fix up the TODO comment and put the right
> value into this patch, either when I post a v2 of it or else when
> I apply it to target-arm.next.

I understand.
Thank you in advance.

Shuuichirou.

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Wednesday, May 18, 2022 7:31 PM
> To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@fujitsu.com>
> Cc: Alex Bennée <alex.bennee@linaro.org>; Itaru Kitayama
> <itaru.kitayama@gmail.com>; qemu-arm@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH] target/arm: Make number of counters in PMCR follow the
> CPU
> 
> On Wed, 18 May 2022 at 00:24, ishii.shuuichir@fujitsu.com
> <ishii.shuuichir@fujitsu.com> wrote:
> >
> > Hi, Peter.
> >
> > > Shuuichirou, Itaru: this is another patch where we need to know
> > > an A64FX register value...
> >
> > Sorry for the late reply.
> >
> > The initial value of the pmcr_el0 register in A64FX is 0x46014040.
> >
> > After applying this Peter's patch, should we submit a new patch as a64fx patch
> from us?
> > or do you want to fix your own modifications to the patch that peter has posted?
> > Which is the best procedure?
> 
> Thanks for looking up the a64fx register value. You don't need to
> do anything more -- I'll fix up the TODO comment and put the right
> value into this patch, either when I post a v2 of it or else when
> I apply it to target-arm.next.
> 
> -- PMM

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Peter Maydell]

On Wed, 18 May 2022 at 00:24, ishii.shuuichir@fujitsu.com
<ishii.shuuichir@fujitsu.com> wrote:
>
> Hi, Peter.
>
> > Shuuichirou, Itaru: this is another patch where we need to know
> > an A64FX register value...
>
> Sorry for the late reply.
>
> The initial value of the pmcr_el0 register in A64FX is 0x46014040.
>
> After applying this Peter's patch, should we submit a new patch as a64fx patch from us?
> or do you want to fix your own modifications to the patch that peter has posted?
> Which is the best procedure?

Thanks for looking up the a64fx register value. You don't need to
do anything more -- I'll fix up the TODO comment and put the right
value into this patch, either when I post a v2 of it or else when
I apply it to target-arm.next.

-- PMM

RE: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[ishii.shuuichir]

Hi, Peter.

> Shuuichirou, Itaru: this is another patch where we need to know
> an A64FX register value...

Sorry for the late reply.

The initial value of the pmcr_el0 register in A64FX is 0x46014040.

After applying this Peter's patch, should we submit a new patch as a64fx patch from us?
or do you want to fix your own modifications to the patch that peter has posted?
Which is the best procedure?

Best regards,
Shuuichirou.

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Friday, May 13, 2022 9:29 PM
> To: qemu-arm@nongnu.org; qemu-devel@nongnu.org
> Cc: Alex Bennée <alex.bennee@linaro.org>; Ishii, Shuuichirou/石井 周一郎
> <ishii.shuuichir@fujitsu.com>; Itaru Kitayama <itaru.kitayama@gmail.com>
> Subject: [PATCH] target/arm: Make number of counters in PMCR follow the CPU
> 
> Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
> means that we don't provide the 6 counters that are required by the
> Arm BSA (Base System Architecture) specification if the CPU supports
> the Virtualization extensions.
> 
> Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
> specify the PMCR reset value (obtained from the appropriate TRM), and
> use the 'N' field of that value to define the number of counters
> provided.
> 
> This means that we now supply 6 counters instead of 4 for:
>  Cortex-A9, Cortex-A15, Cortex-A53, Cortex-A57, Cortex-A72,
>  Cortex-A76, Neoverse-N1, '-cpu max'
> These CPUs remain with 4 counters:
>  Cortex-A7, Cortex-A8
> This CPU goes down from 4 to 3 counters:
>  Cortex-R5
> 
> TODO: A64FX -- I don't know the correct PMCR_EL0 reset value.
> 
> Note that because we now use the PMCR reset value of the specific
> implementation, we no longer set the LC bit out of reset.  This has
> an UNKNOWN value out of reset for all cores with any AArch32 support,
> so guest software should be setting it anyway if it wants it.
> 
> This change was originally landed in commit f7fb73b8cdd3f7 (during
> the 6.0 release cycle) but was then reverted by commit
> 21c2dd77a6aa517 before that release because it did not work with KVM.
> This version fixes that by creating the scratch vCPU in
> kvm_arm_get_host_cpu_features() with the KVM_ARM_VCPU_PMU_V3 feature
> if KVM supports it, and then only asking KVM for the PMCR_EL0 value
> if the vCPU has a PMU.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I'd forgotten that we'd dropped this fix in the 6.0 timeframe
> and never picked it back up again until Alex reminded me of it...
> 
> Changes since original attempt:
>  -- rebased
>  -- fix the code in kvm_arm_get_host_cpu_features() that reads PMCR_EL0
>  -- set PMCR value for new CPUs cortex-a76, neoverse-n1
>  -- set PMCR value for now-separated-out aarch32 -cpu max
>  -- TODO comment for a64fx
> 
> Shuuichirou, Itaru: this is another patch where we need to know
> an A64FX register value...
> ---
>  target/arm/cpu.h       |  1 +
>  target/arm/internals.h |  4 +++-
>  target/arm/cpu64.c     | 10 ++++++++++
>  target/arm/cpu_tcg.c   |  6 ++++++
>  target/arm/helper.c    | 25 ++++++++++++++-----------
>  target/arm/kvm64.c     | 12 ++++++++++++
>  6 files changed, 46 insertions(+), 12 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 18ca61e8e25..0551be62e88 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -965,6 +965,7 @@ struct ArchCPU {
>          uint64_t id_aa64dfr0;
>          uint64_t id_aa64dfr1;
>          uint64_t id_aa64zfr0;
> +        uint64_t reset_pmcr_el0;
>      } isar;
>      uint64_t midr;
>      uint32_t revidr;
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 6ca0e957468..b3b7737048b 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1299,7 +1299,9 @@ enum MVEECIState {
> 
>  static inline uint32_t pmu_num_counters(CPUARMState *env)
>  {
> -  return (env->cp15.c9_pmcr & PMCRN_MASK) >> PMCRN_SHIFT;
> +    ARMCPU *cpu = env_archcpu(env);
> +
> +    return (cpu->isar.reset_pmcr_el0 & PMCRN_MASK) >> PMCRN_SHIFT;
>  }
> 
>  /* Bits allowed to be set/cleared for PMCNTEN* and PMINTEN* */
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 04427e073f1..6008efcbbf0 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -79,6 +79,7 @@ static void aarch64_a57_initfn(Object *obj)
>      cpu->isar.id_aa64isar0 = 0x00011120;
>      cpu->isar.id_aa64mmfr0 = 0x00001124;
>      cpu->isar.dbgdidr = 0x3516d000;
> +    cpu->isar.reset_pmcr_el0 = 0x41013000;
>      cpu->clidr = 0x0a200023;
>      cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
>      cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
> @@ -132,6 +133,7 @@ static void aarch64_a53_initfn(Object *obj)
>      cpu->isar.id_aa64isar0 = 0x00011120;
>      cpu->isar.id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
>      cpu->isar.dbgdidr = 0x3516d000;
> +    cpu->isar.reset_pmcr_el0 = 0x41033000;
>      cpu->clidr = 0x0a200023;
>      cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */
>      cpu->ccsidr[1] = 0x201fe00a; /* 32KB L1 icache */
> @@ -183,6 +185,7 @@ static void aarch64_a72_initfn(Object *obj)
>      cpu->isar.id_aa64isar0 = 0x00011120;
>      cpu->isar.id_aa64mmfr0 = 0x00001124;
>      cpu->isar.dbgdidr = 0x3516d000;
> +    cpu->isar.reset_pmcr_el0 = 0x41023000;
>      cpu->clidr = 0x0a200023;
>      cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
>      cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
> @@ -257,6 +260,9 @@ static void aarch64_a76_initfn(Object *obj)
>      cpu->isar.mvfr0 = 0x10110222;
>      cpu->isar.mvfr1 = 0x13211111;
>      cpu->isar.mvfr2 = 0x00000043;
> +
> +    /* From D5.1 AArch64 PMU register summary */
> +    cpu->isar.reset_pmcr_el0 = 0x410b3000;
>  }
> 
>  static void aarch64_neoverse_n1_initfn(Object *obj)
> @@ -322,6 +328,9 @@ static void aarch64_neoverse_n1_initfn(Object *obj)
>      cpu->isar.mvfr0 = 0x10110222;
>      cpu->isar.mvfr1 = 0x13211111;
>      cpu->isar.mvfr2 = 0x00000043;
> +
> +    /* From D5.1 AArch64 PMU register summary */
> +    cpu->isar.reset_pmcr_el0 = 0x410c3000;
>  }
> 
>  void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
> @@ -1005,6 +1014,7 @@ static void aarch64_a64fx_initfn(Object *obj)
>      set_bit(3, cpu->sve_vq_supported); /* 512bit */
> 
>      /* TODO:  Add A64FX specific HPC extension registers */
> +// FIXME reset_pmcr_el0
>  }
> 
>  static const ARMCPUInfo aarch64_cpus[] = {
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index ea4eccddc35..b751a19c8a7 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -425,6 +425,7 @@ static void cortex_a8_initfn(Object *obj)
>      cpu->ccsidr[1] = 0x2007e01a; /* 16k L1 icache. */
>      cpu->ccsidr[2] = 0xf0000000; /* No L2 icache. */
>      cpu->reset_auxcr = 2;
> +    cpu->isar.reset_pmcr_el0 = 0x41002000;
>      define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
>  }
> 
> @@ -496,6 +497,7 @@ static void cortex_a9_initfn(Object *obj)
>      cpu->clidr = (1 << 27) | (1 << 24) | 3;
>      cpu->ccsidr[0] = 0xe00fe019; /* 16k L1 dcache. */
>      cpu->ccsidr[1] = 0x200fe019; /* 16k L1 icache. */
> +    cpu->isar.reset_pmcr_el0 = 0x41093000;
>      define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
>  }
> 
> @@ -565,6 +567,7 @@ static void cortex_a7_initfn(Object *obj)
>      cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>      cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>      cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
> +    cpu->isar.reset_pmcr_el0 = 0x41072000;
>      define_arm_cp_regs(cpu, cortexa15_cp_reginfo); /* Same as A15 */
>  }
> 
> @@ -607,6 +610,7 @@ static void cortex_a15_initfn(Object *obj)
>      cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>      cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>      cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
> +    cpu->isar.reset_pmcr_el0 = 0x410F3000;
>      define_arm_cp_regs(cpu, cortexa15_cp_reginfo);
>  }
> 
> @@ -835,6 +839,7 @@ static void cortex_r5_initfn(Object *obj)
>      cpu->isar.id_isar6 = 0x0;
>      cpu->mp_is_up = true;
>      cpu->pmsav7_dregion = 16;
> +    cpu->isar.reset_pmcr_el0 = 0x41151800;
>      define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
>  }
> 
> @@ -1093,6 +1098,7 @@ static void arm_max_initfn(Object *obj)
>      cpu->isar.id_isar5 = 0x00011121;
>      cpu->isar.id_isar6 = 0;
>      cpu->isar.dbgdidr = 0x3516d000;
> +    cpu->isar.reset_pmcr_el0 = 0x41013000;
>      cpu->clidr = 0x0a200023;
>      cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
>      cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 432bd819195..439220e4574 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -39,7 +39,6 @@
>  #include "cpregs.h"
> 
>  #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable
> */
> -#define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */
> 
>  #ifndef CONFIG_USER_ONLY
> 
> @@ -5533,13 +5532,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .resetvalue = 0,
>        .writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
>  #endif
> -    /* The only field of MDCR_EL2 that has a defined architectural reset value
> -     * is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
> -     */
> -    { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
> -      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> -      .access = PL2_RW, .resetvalue = PMCR_NUM_COUNTERS,
> -      .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2), },
>      { .name = "HPFAR", .state = ARM_CP_STATE_AA32,
>        .cp = 15, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
>        .access = PL2_RW, .accessfn = access_el3_aa32ns,
> @@ -6586,7 +6578,7 @@ static void define_pmu_regs(ARMCPU *cpu)
>       * field as main ID register, and we implement four counters in
>       * addition to the cycle count register.
>       */
> -    unsigned int i, pmcrn = PMCR_NUM_COUNTERS;
> +    unsigned int i, pmcrn = pmu_num_counters(&cpu->env);
>      ARMCPRegInfo pmcr = {
>          .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
>          .access = PL0_RW,
> @@ -6601,10 +6593,10 @@ static void define_pmu_regs(ARMCPU *cpu)
>          .access = PL0_RW, .accessfn = pmreg_access,
>          .type = ARM_CP_IO,
>          .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
> -        .resetvalue = (cpu->midr & 0xff000000) | (pmcrn << PMCRN_SHIFT) |
> -                      PMCRLC,
> +        .resetvalue = cpu->isar.reset_pmcr_el0,
>          .writefn = pmcr_write, .raw_writefn = raw_write,
>      };
> +
>      define_one_arm_cp_reg(cpu, &pmcr);
>      define_one_arm_cp_reg(cpu, &pmcr64);
>      for (i = 0; i < pmcrn; i++) {
> @@ -7961,6 +7953,17 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                .type = ARM_CP_EL3_NO_EL2_C_NZ,
>                .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
>          };
> +        /*
> +         * The only field of MDCR_EL2 that has a defined architectural reset
> +         * value is MDCR_EL2.HPMN which should reset to the value of
> PMCR_EL0.N.
> +         */
> +        ARMCPRegInfo mdcr_el2 = {
> +            .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
> +            .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> +            .access = PL2_RW, .resetvalue = pmu_num_counters(env),
> +            .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2),
> +        };
> +        define_one_arm_cp_reg(cpu, &mdcr_el2);
>          define_arm_cp_regs(cpu, vpidr_regs);
>          define_arm_cp_regs(cpu, el2_cp_reginfo);
>          if (arm_feature(env, ARM_FEATURE_V8)) {
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index b8cfaf5782a..363032da903 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -505,6 +505,7 @@ bool
> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>       */
>      int fdarray[3];
>      bool sve_supported;
> +    bool pmu_supported = false;
>      uint64_t features = 0;
>      uint64_t t;
>      int err;
> @@ -537,6 +538,11 @@ bool
> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>                               1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
>      }
> 
> +    if (kvm_arm_pmu_supported()) {
> +        init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
> +        pmu_supported = true;
> +    }
> +
>      if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
>          return false;
>      }
> @@ -659,6 +665,12 @@ bool
> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>              dbgdidr |= (1 << 15); /* RES1 bit */
>              ahcf->isar.dbgdidr = dbgdidr;
>          }
> +
> +        if (pmu_supported) {
> +            /* PMCR_EL0 is only accessible if the vCPU has feature PMU_V3
> */
> +            err |= read_sys_reg64(fdarray[2], &ahcf->isar.reset_pmcr_el0,
> +                                  ARM64_SYS_REG(3, 3, 9, 12, 0));
> +        }
>      }
> 
>      sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION,
> KVM_CAP_ARM_SVE) > 0;
> --
> 2.25.1

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Richard Henderson]

On 5/13/22 05:28, Peter Maydell wrote:
> Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
> means that we don't provide the 6 counters that are required by the
> Arm BSA (Base System Architecture) specification if the CPU supports
> the Virtualization extensions.
> 
> Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
> specify the PMCR reset value (obtained from the appropriate TRM), and
> use the 'N' field of that value to define the number of counters
> provided.
> 
> This means that we now supply 6 counters instead of 4 for:
>   Cortex-A9, Cortex-A15, Cortex-A53, Cortex-A57, Cortex-A72,
>   Cortex-A76, Neoverse-N1, '-cpu max'
> These CPUs remain with 4 counters:
>   Cortex-A7, Cortex-A8
> This CPU goes down from 4 to 3 counters:
>   Cortex-R5
> 
> TODO: A64FX -- I don't know the correct PMCR_EL0 reset value.
> 
> Note that because we now use the PMCR reset value of the specific
> implementation, we no longer set the LC bit out of reset.  This has
> an UNKNOWN value out of reset for all cores with any AArch32 support,
> so guest software should be setting it anyway if it wants it.
> 
> This change was originally landed in commit f7fb73b8cdd3f7 (during
> the 6.0 release cycle) but was then reverted by commit
> 21c2dd77a6aa517 before that release because it did not work with KVM.
> This version fixes that by creating the scratch vCPU in
> kvm_arm_get_host_cpu_features() with the KVM_ARM_VCPU_PMU_V3 feature
> if KVM supports it, and then only asking KVM for the PMCR_EL0 value
> if the vCPU has a PMU.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> I'd forgotten that we'd dropped this fix in the 6.0 timeframe
> and never picked it back up again until Alex reminded me of it...
> 
> Changes since original attempt:
>   -- rebased
>   -- fix the code in kvm_arm_get_host_cpu_features() that reads PMCR_EL0
>   -- set PMCR value for new CPUs cortex-a76, neoverse-n1
>   -- set PMCR value for now-separated-out aarch32 -cpu max
>   -- TODO comment for a64fx
> 
> Shuuichirou, Itaru: this is another patch where we need to know
> an A64FX register value...
> ---

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Peter Maydell]

Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
means that we don't provide the 6 counters that are required by the
Arm BSA (Base System Architecture) specification if the CPU supports
the Virtualization extensions.

Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
specify the PMCR reset value (obtained from the appropriate TRM), and
use the 'N' field of that value to define the number of counters
provided.

This means that we now supply 6 counters instead of 4 for:
 Cortex-A9, Cortex-A15, Cortex-A53, Cortex-A57, Cortex-A72,
 Cortex-A76, Neoverse-N1, '-cpu max'
These CPUs remain with 4 counters:
 Cortex-A7, Cortex-A8
This CPU goes down from 4 to 3 counters:
 Cortex-R5

TODO: A64FX -- I don't know the correct PMCR_EL0 reset value.

Note that because we now use the PMCR reset value of the specific
implementation, we no longer set the LC bit out of reset.  This has
an UNKNOWN value out of reset for all cores with any AArch32 support,
so guest software should be setting it anyway if it wants it.

This change was originally landed in commit f7fb73b8cdd3f7 (during
the 6.0 release cycle) but was then reverted by commit
21c2dd77a6aa517 before that release because it did not work with KVM.
This version fixes that by creating the scratch vCPU in
kvm_arm_get_host_cpu_features() with the KVM_ARM_VCPU_PMU_V3 feature
if KVM supports it, and then only asking KVM for the PMCR_EL0 value
if the vCPU has a PMU.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I'd forgotten that we'd dropped this fix in the 6.0 timeframe
and never picked it back up again until Alex reminded me of it...

Changes since original attempt:
 -- rebased
 -- fix the code in kvm_arm_get_host_cpu_features() that reads PMCR_EL0
 -- set PMCR value for new CPUs cortex-a76, neoverse-n1
 -- set PMCR value for now-separated-out aarch32 -cpu max
 -- TODO comment for a64fx

Shuuichirou, Itaru: this is another patch where we need to know
an A64FX register value...
---
 target/arm/cpu.h       |  1 +
 target/arm/internals.h |  4 +++-
 target/arm/cpu64.c     | 10 ++++++++++
 target/arm/cpu_tcg.c   |  6 ++++++
 target/arm/helper.c    | 25 ++++++++++++++-----------
 target/arm/kvm64.c     | 12 ++++++++++++
 6 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 18ca61e8e25..0551be62e88 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -965,6 +965,7 @@ struct ArchCPU {
         uint64_t id_aa64dfr0;
         uint64_t id_aa64dfr1;
         uint64_t id_aa64zfr0;
+        uint64_t reset_pmcr_el0;
     } isar;
     uint64_t midr;
     uint32_t revidr;
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 6ca0e957468..b3b7737048b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1299,7 +1299,9 @@ enum MVEECIState {
 
 static inline uint32_t pmu_num_counters(CPUARMState *env)
 {
-  return (env->cp15.c9_pmcr & PMCRN_MASK) >> PMCRN_SHIFT;
+    ARMCPU *cpu = env_archcpu(env);
+
+    return (cpu->isar.reset_pmcr_el0 & PMCRN_MASK) >> PMCRN_SHIFT;
 }
 
 /* Bits allowed to be set/cleared for PMCNTEN* and PMINTEN* */
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 04427e073f1..6008efcbbf0 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -79,6 +79,7 @@ static void aarch64_a57_initfn(Object *obj)
     cpu->isar.id_aa64isar0 = 0x00011120;
     cpu->isar.id_aa64mmfr0 = 0x00001124;
     cpu->isar.dbgdidr = 0x3516d000;
+    cpu->isar.reset_pmcr_el0 = 0x41013000;
     cpu->clidr = 0x0a200023;
     cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
     cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
@@ -132,6 +133,7 @@ static void aarch64_a53_initfn(Object *obj)
     cpu->isar.id_aa64isar0 = 0x00011120;
     cpu->isar.id_aa64mmfr0 = 0x00001122; /* 40 bit physical addr */
     cpu->isar.dbgdidr = 0x3516d000;
+    cpu->isar.reset_pmcr_el0 = 0x41033000;
     cpu->clidr = 0x0a200023;
     cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */
     cpu->ccsidr[1] = 0x201fe00a; /* 32KB L1 icache */
@@ -183,6 +185,7 @@ static void aarch64_a72_initfn(Object *obj)
     cpu->isar.id_aa64isar0 = 0x00011120;
     cpu->isar.id_aa64mmfr0 = 0x00001124;
     cpu->isar.dbgdidr = 0x3516d000;
+    cpu->isar.reset_pmcr_el0 = 0x41023000;
     cpu->clidr = 0x0a200023;
     cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
     cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
@@ -257,6 +260,9 @@ static void aarch64_a76_initfn(Object *obj)
     cpu->isar.mvfr0 = 0x10110222;
     cpu->isar.mvfr1 = 0x13211111;
     cpu->isar.mvfr2 = 0x00000043;
+
+    /* From D5.1 AArch64 PMU register summary */
+    cpu->isar.reset_pmcr_el0 = 0x410b3000;
 }
 
 static void aarch64_neoverse_n1_initfn(Object *obj)
@@ -322,6 +328,9 @@ static void aarch64_neoverse_n1_initfn(Object *obj)
     cpu->isar.mvfr0 = 0x10110222;
     cpu->isar.mvfr1 = 0x13211111;
     cpu->isar.mvfr2 = 0x00000043;
+
+    /* From D5.1 AArch64 PMU register summary */
+    cpu->isar.reset_pmcr_el0 = 0x410c3000;
 }
 
 void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
@@ -1005,6 +1014,7 @@ static void aarch64_a64fx_initfn(Object *obj)
     set_bit(3, cpu->sve_vq_supported); /* 512bit */
 
     /* TODO:  Add A64FX specific HPC extension registers */
+// FIXME reset_pmcr_el0
 }
 
 static const ARMCPUInfo aarch64_cpus[] = {
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index ea4eccddc35..b751a19c8a7 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -425,6 +425,7 @@ static void cortex_a8_initfn(Object *obj)
     cpu->ccsidr[1] = 0x2007e01a; /* 16k L1 icache. */
     cpu->ccsidr[2] = 0xf0000000; /* No L2 icache. */
     cpu->reset_auxcr = 2;
+    cpu->isar.reset_pmcr_el0 = 0x41002000;
     define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
 }
 
@@ -496,6 +497,7 @@ static void cortex_a9_initfn(Object *obj)
     cpu->clidr = (1 << 27) | (1 << 24) | 3;
     cpu->ccsidr[0] = 0xe00fe019; /* 16k L1 dcache. */
     cpu->ccsidr[1] = 0x200fe019; /* 16k L1 icache. */
+    cpu->isar.reset_pmcr_el0 = 0x41093000;
     define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
 }
 
@@ -565,6 +567,7 @@ static void cortex_a7_initfn(Object *obj)
     cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
     cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
     cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
+    cpu->isar.reset_pmcr_el0 = 0x41072000;
     define_arm_cp_regs(cpu, cortexa15_cp_reginfo); /* Same as A15 */
 }
 
@@ -607,6 +610,7 @@ static void cortex_a15_initfn(Object *obj)
     cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
     cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
     cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
+    cpu->isar.reset_pmcr_el0 = 0x410F3000;
     define_arm_cp_regs(cpu, cortexa15_cp_reginfo);
 }
 
@@ -835,6 +839,7 @@ static void cortex_r5_initfn(Object *obj)
     cpu->isar.id_isar6 = 0x0;
     cpu->mp_is_up = true;
     cpu->pmsav7_dregion = 16;
+    cpu->isar.reset_pmcr_el0 = 0x41151800;
     define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
 }
 
@@ -1093,6 +1098,7 @@ static void arm_max_initfn(Object *obj)
     cpu->isar.id_isar5 = 0x00011121;
     cpu->isar.id_isar6 = 0;
     cpu->isar.dbgdidr = 0x3516d000;
+    cpu->isar.reset_pmcr_el0 = 0x41013000;
     cpu->clidr = 0x0a200023;
     cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
     cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 432bd819195..439220e4574 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -39,7 +39,6 @@
 #include "cpregs.h"
 
 #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
-#define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */
 
 #ifndef CONFIG_USER_ONLY
 
@@ -5533,13 +5532,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .resetvalue = 0,
       .writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
 #endif
-    /* The only field of MDCR_EL2 that has a defined architectural reset value
-     * is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
-     */
-    { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
-      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
-      .access = PL2_RW, .resetvalue = PMCR_NUM_COUNTERS,
-      .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2), },
     { .name = "HPFAR", .state = ARM_CP_STATE_AA32,
       .cp = 15, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
       .access = PL2_RW, .accessfn = access_el3_aa32ns,
@@ -6586,7 +6578,7 @@ static void define_pmu_regs(ARMCPU *cpu)
      * field as main ID register, and we implement four counters in
      * addition to the cycle count register.
      */
-    unsigned int i, pmcrn = PMCR_NUM_COUNTERS;
+    unsigned int i, pmcrn = pmu_num_counters(&cpu->env);
     ARMCPRegInfo pmcr = {
         .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
         .access = PL0_RW,
@@ -6601,10 +6593,10 @@ static void define_pmu_regs(ARMCPU *cpu)
         .access = PL0_RW, .accessfn = pmreg_access,
         .type = ARM_CP_IO,
         .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
-        .resetvalue = (cpu->midr & 0xff000000) | (pmcrn << PMCRN_SHIFT) |
-                      PMCRLC,
+        .resetvalue = cpu->isar.reset_pmcr_el0,
         .writefn = pmcr_write, .raw_writefn = raw_write,
     };
+
     define_one_arm_cp_reg(cpu, &pmcr);
     define_one_arm_cp_reg(cpu, &pmcr64);
     for (i = 0; i < pmcrn; i++) {
@@ -7961,6 +7953,17 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .type = ARM_CP_EL3_NO_EL2_C_NZ,
               .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
         };
+        /*
+         * The only field of MDCR_EL2 that has a defined architectural reset
+         * value is MDCR_EL2.HPMN which should reset to the value of PMCR_EL0.N.
+         */
+        ARMCPRegInfo mdcr_el2 = {
+            .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
+            .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
+            .access = PL2_RW, .resetvalue = pmu_num_counters(env),
+            .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2),
+        };
+        define_one_arm_cp_reg(cpu, &mdcr_el2);
         define_arm_cp_regs(cpu, vpidr_regs);
         define_arm_cp_regs(cpu, el2_cp_reginfo);
         if (arm_feature(env, ARM_FEATURE_V8)) {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index b8cfaf5782a..363032da903 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -505,6 +505,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
      */
     int fdarray[3];
     bool sve_supported;
+    bool pmu_supported = false;
     uint64_t features = 0;
     uint64_t t;
     int err;
@@ -537,6 +538,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
                              1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
     }
 
+    if (kvm_arm_pmu_supported()) {
+        init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
+        pmu_supported = true;
+    }
+
     if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
         return false;
     }
@@ -659,6 +665,12 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
             dbgdidr |= (1 << 15); /* RES1 bit */
             ahcf->isar.dbgdidr = dbgdidr;
         }
+
+        if (pmu_supported) {
+            /* PMCR_EL0 is only accessible if the vCPU has feature PMU_V3 */
+            err |= read_sys_reg64(fdarray[2], &ahcf->isar.reset_pmcr_el0,
+                                  ARM64_SYS_REG(3, 3, 9, 12, 0));
+        }
     }
 
     sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0;
-- 
2.25.1