diff mbox

[v2,2/5] target-arm: Add Some of the performance monitor registers

Message ID 309860ef711e07dc7eee7133c4dd228cd8c343b7.1454720020.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis Feb. 6, 2016, 12:55 a.m. UTC 
This patch adds the following registers including read and write functions:
PMSELR, PMSELR_EL0, PMXEVCNTR, PMXEVCNTR_EL0, PMXEVTYPER and PMXEVTYPER_EL0.

Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Tested-by: Nathan Rossi <nathan@nathanrossi.com>
---

 target-arm/cpu.h    |  6 ++++
 target-arm/helper.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 84 insertions(+), 11 deletions(-)

Comments

Peter Maydell Feb. 9, 2016, 5:32 p.m. UTC | #1 
On 6 February 2016 at 00:55, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> This patch adds the following registers including read and write functions:
> PMSELR, PMSELR_EL0, PMXEVCNTR, PMXEVCNTR_EL0, PMXEVTYPER and PMXEVTYPER_EL0.
>
> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Tested-by: Nathan Rossi <nathan@nathanrossi.com>

Is there any benefit to implementing all the type and counter
registers and the multiplex selection, when we don't actually
support any counters yet?

> ---
>
>  target-arm/cpu.h    |  6 ++++
>  target-arm/helper.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 84 insertions(+), 11 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index b8b3364..5c31c56 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -117,6 +117,8 @@ typedef struct ARMGenericTimer {
>  #define GTIMER_SEC  3
>  #define NUM_GTIMERS 4
>
> +#define NUM_PMU_COUNTERS 4
> +
>  typedef struct {
>      uint64_t raw_tcr;
>      uint32_t mask;
> @@ -300,6 +302,7 @@ typedef struct CPUARMState {
>          uint32_t c9_pmxevtyper; /* perf monitor event type */
>          uint32_t c9_pmuserenr; /* perf monitor user enable */
>          uint32_t c9_pminten; /* perf monitor interrupt enables */
> +        uint32_t c9_pmselr; /* perf monitor event counter selection */
>          union { /* Memory attribute redirection */
>              struct {
>  #ifdef HOST_WORDS_BIGENDIAN
> @@ -361,6 +364,9 @@ typedef struct CPUARMState {
>              uint64_t tpidruro_ns;
>              uint64_t tpidrro_el[1];
>          };
> +        uint32_t c14_pmccfiltr; /* Performance Monitor Filter Register */
> +        uint32_t c14_pmevcntr[NUM_PMU_COUNTERS];
> +        uint32_t c14_pmevtyper[NUM_PMU_COUNTERS];
>          uint64_t c14_cntfrq; /* Counter Frequency register */
>          uint64_t c14_cntkctl; /* Timer Control register */
>          uint32_t cnthctl_el2; /* Counter/Timer Hyp Control register */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 66aa406..164853f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -807,6 +807,58 @@ void pmccntr_sync(CPUARMState *env)
>
>  #endif
>
> +static void pmselr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                           uint64_t value)
> +{
> +    env->cp15.c9_pmselr = value & 31;
> +}
> +
> +static uint64_t pmevcntr_read(CPUARMState *env, const ARMCPRegInfo *ri,
> +                              const uint8_t idx)
> +{
> +    if (idx >= NUM_PMU_COUNTERS) {
> +        return arm_cp_read_zero(env, ri);
> +    }
> +    return env->cp15.c14_pmevcntr[idx];
> +}
> +
> +static uint64_t pmxevcntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    return pmevcntr_read(env, ri, env->cp15.c9_pmselr & 31);
> +}
> +
> +static void pmevcntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                             uint64_t value, const uint8_t idx)
> +{
> +    if (idx >= NUM_PMU_COUNTERS) {
> +        return arm_cp_write_ignore(env, ri, value);

arm_cp_write_ignore() does nothing, as you might expect from the name,
so why call it?

You might mention that this is CONSTRAINED UNPREDICTABLE and we
are choosing to RAZ/WI.

> +    }
> +    env->cp15.c14_pmevcntr[idx] = value;
> +}
> +
> +static void pmxevcntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                            uint64_t value)
> +{
> +    pmevcntr_write(env, ri, value, env->cp15.c9_pmselr & 31);
> +}
> +
> +static uint64_t pmevtyper_read(CPUARMState *env, const ARMCPRegInfo *ri,
> +                               const uint8_t idx)
> +{
> +    if (idx >= NUM_PMU_COUNTERS) {
> +        return arm_cp_read_zero(env, ri);

You could just return 0.

> +    }
> +    return env->cp15.c14_pmevtyper[idx];
> +}
> +
> +static uint64_t pmxevtyper_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    if (!!(env->cp15.c9_pmselr & 31)) {
> +        return env->cp15.c14_pmccfiltr;
> +    }
> +    return pmevtyper_read(env, ri, env->cp15.c9_pmselr & 31);
> +}
> +
>  static void pmccfiltr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                              uint64_t value)
>  {
> @@ -986,8 +1038,32 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>       * We choose to RAZ/WI.
>       */
>      { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
> -      .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
> -      .accessfn = pmreg_access },
> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
> +      .accessfn = pmreg_access, .writefn = pmselr_write,
> +      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmselr) },
> +    { .name = "PMSELR_EL0", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 5,
> +      .access = PL0_RW, .accessfn = pmreg_access,
> +      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmselr),
> +      .writefn = pmselr_write, .resetvalue = 0 },
> +    { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2,
> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
> +      .accessfn = pmreg_access, .writefn = pmxevcntr_write,
> +      .readfn = pmxevcntr_read },
> +    { .name = "PMXEVCNTR_EL0", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 13, .opc2 = 2,
> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
> +      .accessfn = pmreg_access, .writefn = pmxevcntr_write,
> +      .readfn = pmxevcntr_read },
> +    { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
> +      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
> +      .readfn = pmxevtyper_read },
> +    { .name = "PMXEVTYPER_EL0", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 13, .opc2 = 1,
> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
> +      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
> +      .readfn = pmxevtyper_read },

This isn't going to handle migration correctly, because there are
a whole pile of data values behind these registers.

It may be easiest to implement the PMEVTYPER<n>_EL0 and PMEVCNTR<n>_EL0
registers (which are PMUv3 only and provide un-multiplexed access to
the type and counter registers), gate them so they're present but not
guest-accessible on PMUv2 and below, and use them for migrating the
state. Then the X multiplexed registers can be simple CP_ALIASes as
you have them here.

>  #ifndef CONFIG_USER_ONLY
>      { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
>        .access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
> @@ -1006,15 +1082,6 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>        .type = ARM_CP_IO,
>        .fieldoffset = offsetof(CPUARMState, cp15.pmccfiltr_el0),
>        .resetvalue = 0, },
> -    { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
> -      .access = PL0_RW,
> -      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
> -      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
> -      .raw_writefn = raw_write },
> -    /* Unimplemented, RAZ/WI. */
> -    { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2,
> -      .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
> -      .accessfn = pmreg_access },
>      { .name = "PMUSERENR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 0,
>        .access = PL0_R | PL1_RW,
>        .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr),
> --
> 2.5.0
>

thanks
-- PMM
Alistair Francis Feb. 9, 2016, 11:25 p.m. UTC | #2 
On Tue, Feb 9, 2016 at 9:32 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 February 2016 at 00:55, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> This patch adds the following registers including read and write functions:
>> PMSELR, PMSELR_EL0, PMXEVCNTR, PMXEVCNTR_EL0, PMXEVTYPER and PMXEVTYPER_EL0.
>>
>> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> Tested-by: Nathan Rossi <nathan@nathanrossi.com>
>
> Is there any benefit to implementing all the type and counter
> registers and the multiplex selection, when we don't actually
> support any counters yet?

Thanks for reviewing these. The others (patch 1, 3 and 4) are more
important to me, so I'm sending them out so they can be applied then
I'll look at these after.

Thanks,

Alistair

>
>> ---
>>
>>  target-arm/cpu.h    |  6 ++++
>>  target-arm/helper.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++-------
>>  2 files changed, 84 insertions(+), 11 deletions(-)
>>
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index b8b3364..5c31c56 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -117,6 +117,8 @@ typedef struct ARMGenericTimer {
>>  #define GTIMER_SEC  3
>>  #define NUM_GTIMERS 4
>>
>> +#define NUM_PMU_COUNTERS 4
>> +
>>  typedef struct {
>>      uint64_t raw_tcr;
>>      uint32_t mask;
>> @@ -300,6 +302,7 @@ typedef struct CPUARMState {
>>          uint32_t c9_pmxevtyper; /* perf monitor event type */
>>          uint32_t c9_pmuserenr; /* perf monitor user enable */
>>          uint32_t c9_pminten; /* perf monitor interrupt enables */
>> +        uint32_t c9_pmselr; /* perf monitor event counter selection */
>>          union { /* Memory attribute redirection */
>>              struct {
>>  #ifdef HOST_WORDS_BIGENDIAN
>> @@ -361,6 +364,9 @@ typedef struct CPUARMState {
>>              uint64_t tpidruro_ns;
>>              uint64_t tpidrro_el[1];
>>          };
>> +        uint32_t c14_pmccfiltr; /* Performance Monitor Filter Register */
>> +        uint32_t c14_pmevcntr[NUM_PMU_COUNTERS];
>> +        uint32_t c14_pmevtyper[NUM_PMU_COUNTERS];
>>          uint64_t c14_cntfrq; /* Counter Frequency register */
>>          uint64_t c14_cntkctl; /* Timer Control register */
>>          uint32_t cnthctl_el2; /* Counter/Timer Hyp Control register */
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 66aa406..164853f 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -807,6 +807,58 @@ void pmccntr_sync(CPUARMState *env)
>>
>>  #endif
>>
>> +static void pmselr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                           uint64_t value)
>> +{
>> +    env->cp15.c9_pmselr = value & 31;
>> +}
>> +
>> +static uint64_t pmevcntr_read(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                              const uint8_t idx)
>> +{
>> +    if (idx >= NUM_PMU_COUNTERS) {
>> +        return arm_cp_read_zero(env, ri);
>> +    }
>> +    return env->cp15.c14_pmevcntr[idx];
>> +}
>> +
>> +static uint64_t pmxevcntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>> +{
>> +    return pmevcntr_read(env, ri, env->cp15.c9_pmselr & 31);
>> +}
>> +
>> +static void pmevcntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                             uint64_t value, const uint8_t idx)
>> +{
>> +    if (idx >= NUM_PMU_COUNTERS) {
>> +        return arm_cp_write_ignore(env, ri, value);
>
> arm_cp_write_ignore() does nothing, as you might expect from the name,
> so why call it?
>
> You might mention that this is CONSTRAINED UNPREDICTABLE and we
> are choosing to RAZ/WI.
>
>> +    }
>> +    env->cp15.c14_pmevcntr[idx] = value;
>> +}
>> +
>> +static void pmxevcntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                            uint64_t value)
>> +{
>> +    pmevcntr_write(env, ri, value, env->cp15.c9_pmselr & 31);
>> +}
>> +
>> +static uint64_t pmevtyper_read(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                               const uint8_t idx)
>> +{
>> +    if (idx >= NUM_PMU_COUNTERS) {
>> +        return arm_cp_read_zero(env, ri);
>
> You could just return 0.
>
>> +    }
>> +    return env->cp15.c14_pmevtyper[idx];
>> +}
>> +
>> +static uint64_t pmxevtyper_read(CPUARMState *env, const ARMCPRegInfo *ri)
>> +{
>> +    if (!!(env->cp15.c9_pmselr & 31)) {
>> +        return env->cp15.c14_pmccfiltr;
>> +    }
>> +    return pmevtyper_read(env, ri, env->cp15.c9_pmselr & 31);
>> +}
>> +
>>  static void pmccfiltr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>                              uint64_t value)
>>  {
>> @@ -986,8 +1038,32 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>>       * We choose to RAZ/WI.
>>       */
>>      { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
>> -      .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
>> -      .accessfn = pmreg_access },
>> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
>> +      .accessfn = pmreg_access, .writefn = pmselr_write,
>> +      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmselr) },
>> +    { .name = "PMSELR_EL0", .state = ARM_CP_STATE_AA64,
>> +      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 5,
>> +      .access = PL0_RW, .accessfn = pmreg_access,
>> +      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmselr),
>> +      .writefn = pmselr_write, .resetvalue = 0 },
>> +    { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2,
>> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
>> +      .accessfn = pmreg_access, .writefn = pmxevcntr_write,
>> +      .readfn = pmxevcntr_read },
>> +    { .name = "PMXEVCNTR_EL0", .state = ARM_CP_STATE_AA64,
>> +      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 13, .opc2 = 2,
>> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
>> +      .accessfn = pmreg_access, .writefn = pmxevcntr_write,
>> +      .readfn = pmxevcntr_read },
>> +    { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
>> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
>> +      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
>> +      .readfn = pmxevtyper_read },
>> +    { .name = "PMXEVTYPER_EL0", .state = ARM_CP_STATE_AA64,
>> +      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 13, .opc2 = 1,
>> +      .access = PL0_RW, .type = ARM_CP_ALIAS,
>> +      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
>> +      .readfn = pmxevtyper_read },
>
> This isn't going to handle migration correctly, because there are
> a whole pile of data values behind these registers.
>
> It may be easiest to implement the PMEVTYPER<n>_EL0 and PMEVCNTR<n>_EL0
> registers (which are PMUv3 only and provide un-multiplexed access to
> the type and counter registers), gate them so they're present but not
> guest-accessible on PMUv2 and below, and use them for migrating the
> state. Then the X multiplexed registers can be simple CP_ALIASes as
> you have them here.
>
>>  #ifndef CONFIG_USER_ONLY
>>      { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
>>        .access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
>> @@ -1006,15 +1082,6 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>>        .type = ARM_CP_IO,
>>        .fieldoffset = offsetof(CPUARMState, cp15.pmccfiltr_el0),
>>        .resetvalue = 0, },
>> -    { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
>> -      .access = PL0_RW,
>> -      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
>> -      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
>> -      .raw_writefn = raw_write },
>> -    /* Unimplemented, RAZ/WI. */
>> -    { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2,
>> -      .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
>> -      .accessfn = pmreg_access },
>>      { .name = "PMUSERENR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 0,
>>        .access = PL0_R | PL1_RW,
>>        .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr),
>> --
>> 2.5.0
>>
>
> thanks
> -- PMM
>
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index b8b3364..5c31c56 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -117,6 +117,8 @@  typedef struct ARMGenericTimer {
 #define GTIMER_SEC  3
 #define NUM_GTIMERS 4
 
+#define NUM_PMU_COUNTERS 4
+
 typedef struct {
     uint64_t raw_tcr;
     uint32_t mask;
@@ -300,6 +302,7 @@  typedef struct CPUARMState {
         uint32_t c9_pmxevtyper; /* perf monitor event type */
         uint32_t c9_pmuserenr; /* perf monitor user enable */
         uint32_t c9_pminten; /* perf monitor interrupt enables */
+        uint32_t c9_pmselr; /* perf monitor event counter selection */
         union { /* Memory attribute redirection */
             struct {
 #ifdef HOST_WORDS_BIGENDIAN
@@ -361,6 +364,9 @@  typedef struct CPUARMState {
             uint64_t tpidruro_ns;
             uint64_t tpidrro_el[1];
         };
+        uint32_t c14_pmccfiltr; /* Performance Monitor Filter Register */
+        uint32_t c14_pmevcntr[NUM_PMU_COUNTERS];
+        uint32_t c14_pmevtyper[NUM_PMU_COUNTERS];
         uint64_t c14_cntfrq; /* Counter Frequency register */
         uint64_t c14_cntkctl; /* Timer Control register */
         uint32_t cnthctl_el2; /* Counter/Timer Hyp Control register */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 66aa406..164853f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -807,6 +807,58 @@  void pmccntr_sync(CPUARMState *env)
 
 #endif
 
+static void pmselr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                           uint64_t value)
+{
+    env->cp15.c9_pmselr = value & 31;
+}
+
+static uint64_t pmevcntr_read(CPUARMState *env, const ARMCPRegInfo *ri,
+                              const uint8_t idx)
+{
+    if (idx >= NUM_PMU_COUNTERS) {
+        return arm_cp_read_zero(env, ri);
+    }
+    return env->cp15.c14_pmevcntr[idx];
+}
+
+static uint64_t pmxevcntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return pmevcntr_read(env, ri, env->cp15.c9_pmselr & 31);
+}
+
+static void pmevcntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                             uint64_t value, const uint8_t idx)
+{
+    if (idx >= NUM_PMU_COUNTERS) {
+        return arm_cp_write_ignore(env, ri, value);
+    }
+    env->cp15.c14_pmevcntr[idx] = value;
+}
+
+static void pmxevcntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                            uint64_t value)
+{
+    pmevcntr_write(env, ri, value, env->cp15.c9_pmselr & 31);
+}
+
+static uint64_t pmevtyper_read(CPUARMState *env, const ARMCPRegInfo *ri,
+                               const uint8_t idx)
+{
+    if (idx >= NUM_PMU_COUNTERS) {
+        return arm_cp_read_zero(env, ri);
+    }
+    return env->cp15.c14_pmevtyper[idx];
+}
+
+static uint64_t pmxevtyper_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    if (!!(env->cp15.c9_pmselr & 31)) {
+        return env->cp15.c14_pmccfiltr;
+    }
+    return pmevtyper_read(env, ri, env->cp15.c9_pmselr & 31);
+}
+
 static void pmccfiltr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                             uint64_t value)
 {
@@ -986,8 +1038,32 @@  static const ARMCPRegInfo v7_cp_reginfo[] = {
      * We choose to RAZ/WI.
      */
     { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
-      .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
-      .accessfn = pmreg_access },
+      .access = PL0_RW, .type = ARM_CP_ALIAS,
+      .accessfn = pmreg_access, .writefn = pmselr_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmselr) },
+    { .name = "PMSELR_EL0", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 5,
+      .access = PL0_RW, .accessfn = pmreg_access,
+      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmselr),
+      .writefn = pmselr_write, .resetvalue = 0 },
+    { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2,
+      .access = PL0_RW, .type = ARM_CP_ALIAS,
+      .accessfn = pmreg_access, .writefn = pmxevcntr_write,
+      .readfn = pmxevcntr_read },
+    { .name = "PMXEVCNTR_EL0", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 13, .opc2 = 2,
+      .access = PL0_RW, .type = ARM_CP_ALIAS,
+      .accessfn = pmreg_access, .writefn = pmxevcntr_write,
+      .readfn = pmxevcntr_read },
+    { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
+      .access = PL0_RW, .type = ARM_CP_ALIAS,
+      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
+      .readfn = pmxevtyper_read },
+    { .name = "PMXEVTYPER_EL0", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 13, .opc2 = 1,
+      .access = PL0_RW, .type = ARM_CP_ALIAS,
+      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
+      .readfn = pmxevtyper_read },
 #ifndef CONFIG_USER_ONLY
     { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
       .access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
@@ -1006,15 +1082,6 @@  static const ARMCPRegInfo v7_cp_reginfo[] = {
       .type = ARM_CP_IO,
       .fieldoffset = offsetof(CPUARMState, cp15.pmccfiltr_el0),
       .resetvalue = 0, },
-    { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
-      .access = PL0_RW,
-      .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
-      .accessfn = pmreg_access, .writefn = pmxevtyper_write,
-      .raw_writefn = raw_write },
-    /* Unimplemented, RAZ/WI. */
-    { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2,
-      .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
-      .accessfn = pmreg_access },
     { .name = "PMUSERENR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 0,
       .access = PL0_R | PL1_RW,
       .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr),