diff mbox

[v3,20/22] target/arm: PMU: Add instruction and cycle events

Message ID 1521232280-13089-21-git-send-email-alindsay@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Aaron Lindsay March 16, 2018, 8:31 p.m. UTC 
The instruction event is only enabled when icount is used, cycles are
always supported. Always defining get_cycle_count (but altering its
behavior depending on CONFIG_USER_ONLY) allows us to remove some
CONFIG_USER_ONLY #defines throughout the rest of the code.

Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
---
 target/arm/helper.c | 99 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 52 insertions(+), 47 deletions(-)

Comments

Philippe Mathieu-Daudé March 18, 2018, 10:43 p.m. UTC | #1 
On 03/16/2018 09:31 PM, Aaron Lindsay wrote:
> The instruction event is only enabled when icount is used, cycles are
> always supported. Always defining get_cycle_count (but altering its
> behavior depending on CONFIG_USER_ONLY) allows us to remove some
> CONFIG_USER_ONLY #defines throughout the rest of the code.
> 
> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/arm/helper.c | 99 ++++++++++++++++++++++++++++-------------------------
>  1 file changed, 52 insertions(+), 47 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 2fa8308..679897a 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -15,6 +15,7 @@
>  #include "arm_ldst.h"
>  #include <zlib.h> /* For crc32 */
>  #include "exec/semihost.h"
> +#include "sysemu/cpus.h"
>  #include "sysemu/kvm.h"
>  #include "fpu/softfloat.h"
>  
> @@ -935,8 +936,54 @@ typedef struct pm_event {
>      uint64_t (*get_count)(CPUARMState *, uint64_t cycles);
>  } pm_event;
>  
> +/*
> + * Return the underlying cycle count for the PMU cycle counters. If we're in
> + * usermode, simply return 0.
> + */
> +static uint64_t get_cycle_count(CPUARMState *env)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> +                   ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
> +#else
> +    return 0;
> +#endif
> +}
> +
> +static bool event_always_supported(CPUARMState *env)
> +{
> +    return true;
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +static uint64_t cycles_get_count(CPUARMState *env, uint64_t cycles)
> +{
> +    return cycles;
> +}
> +
> +static bool instructions_supported(CPUARMState *env)
> +{
> +    return use_icount == 1 /* Precise instruction counting */;
> +}
> +
> +static uint64_t instructions_get_count(CPUARMState *env, uint64_t cycles)
> +{
> +    return (uint64_t)cpu_get_icount_raw();
> +}
> +#endif
> +
>  #define SUPPORTED_EVENT_SENTINEL UINT16_MAX
>  static const pm_event pm_events[] = {
> +#ifndef CONFIG_USER_ONLY
> +    { .number = 0x008, /* INST_RETIRED */
> +      .supported = instructions_supported,
> +      .get_count = instructions_get_count
> +    },
> +    { .number = 0x011, /* CPU_CYCLES */
> +      .supported = event_always_supported,
> +      .get_count = cycles_get_count
> +    },
> +#endif
>      { .number = SUPPORTED_EVENT_SENTINEL }
>  };
>  static uint16_t supported_event_map[0x3f];
> @@ -1016,8 +1063,6 @@ static CPAccessResult pmreg_access_swinc(CPUARMState *env,
>      return pmreg_access(env, ri, isread);
>  }
>  
> -#ifndef CONFIG_USER_ONLY
> -
>  static CPAccessResult pmreg_access_selr(CPUARMState *env,
>                                          const ARMCPRegInfo *ri,
>                                          bool isread)
> @@ -1126,11 +1171,7 @@ static inline bool pmu_counter_filtered(CPUARMState *env, uint64_t pmxevtyper)
>   */
>  uint64_t pmccntr_op_start(CPUARMState *env)
>  {
> -    uint64_t cycles = 0;
> -#ifndef CONFIG_USER_ONLY
> -    cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
> -#endif
> +    uint64_t cycles = get_cycle_count(env);
>  
>      if (arm_ccnt_enabled(env) &&
>            !pmu_counter_filtered(env, env->cp15.pmccfiltr_el0)) {
> @@ -1268,26 +1309,6 @@ static void pmccntr_write32(CPUARMState *env, const ARMCPRegInfo *ri,
>      pmccntr_write(env, ri, deposit64(cur_val, 0, 32, value));
>  }
>  
> -#else /* CONFIG_USER_ONLY */
> -
> -uint64_t pmccntr_op_start(CPUARMState *env)
> -{
> -}
> -
> -void pmccntr_op_finish(CPUARMState *env, uint64_t prev_cycles)
> -{
> -}
> -
> -uint64_t pmu_op_start(CPUARMState *env)
> -{
> -}
> -
> -void pmu_op_finish(CPUARMState *env, uint64_t prev_cycles)
> -{
> -}
> -
> -#endif
> -
>  static void pmccfiltr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                              uint64_t value)
>  {
> @@ -1346,11 +1367,7 @@ static void pmevtyper_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      if (counter == 0x1f) {
>          pmccfiltr_write(env, ri, value);
>      } else if (counter < PMU_NUM_COUNTERS(env)) {
> -        uint64_t cycles = 0;
> -#ifndef CONFIG_USER_ONLY
> -        cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
> -#endif
> +        uint64_t cycles = get_cycle_count(env);
>          pmu_sync_counter(env, counter, cycles);
>          env->cp15.c14_pmevtyper[counter] = value & 0xfe0003ff;
>          pmu_sync_counter(env, counter, cycles);
> @@ -1404,11 +1421,7 @@ static void pmevcntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                               uint64_t value, uint8_t counter)
>  {
>      if (counter < PMU_NUM_COUNTERS(env)) {
> -        uint64_t cycles = 0;
> -#ifndef CONFIG_USER_ONLY
> -        cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
> -#endif
> +        uint64_t cycles = get_cycle_count(env);
>          env->cp15.c14_pmevcntr[counter] = value;
>          pmu_sync_counter(env, counter, cycles);
>      }
> @@ -1420,12 +1433,8 @@ static uint64_t pmevcntr_read(CPUARMState *env, const ARMCPRegInfo *ri,
>                                uint8_t counter)
>  {
>      if (counter < PMU_NUM_COUNTERS(env)) {
> -        uint64_t ret;
> -        uint64_t cycles = 0;
> -#ifndef CONFIG_USER_ONLY
> -        cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
> -#endif
> +        uint64_t ret, cycles;
> +        cycles = get_cycle_count(env);
>          pmu_sync_counter(env, counter, cycles);
>          ret = env->cp15.c14_pmevcntr[counter];
>          pmu_sync_counter(env, counter, cycles);
> @@ -1613,7 +1622,6 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>      /* Unimplemented so WI. */
>      { .name = "PMSWINC", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 4,
>        .access = PL0_W, .accessfn = pmreg_access_swinc, .type = ARM_CP_NOP },
> -#ifndef CONFIG_USER_ONLY
>      { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
>        .access = PL0_RW, .type = ARM_CP_ALIAS,
>        .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmselr),
> @@ -1633,7 +1641,6 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>        .access = PL0_RW, .accessfn = pmreg_access_ccntr,
>        .type = ARM_CP_IO,
>        .readfn = pmccntr_read, .writefn = pmccntr_write, },
> -#endif
>      { .name = "PMCCFILTR", .cp = 15, .opc1 = 0, .crn = 14, .crm = 15, .opc2 = 7,
>        .writefn = pmccfiltr_write_a32, .readfn = pmccfiltr_read_a32,
>        .access = PL0_RW, .accessfn = pmreg_access,
> @@ -5171,7 +5178,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>           * field as main ID register, and we implement only the cycle
>           * count register.
>           */
> -#ifndef CONFIG_USER_ONLY
>          ARMCPRegInfo pmcr = {
>              .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
>              .access = PL0_RW,
> @@ -5225,7 +5231,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>              g_free(pmevtyper_name);
>              g_free(pmevtyper_el0_name);
>          }
> -#endif
>          ARMCPRegInfo clidr = {
>              .name = "CLIDR", .state = ARM_CP_STATE_BOTH,
>              .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1,
>
Philippe Mathieu-Daudé March 18, 2018, 10:48 p.m. UTC | #2 
On 03/16/2018 09:31 PM, Aaron Lindsay wrote:
> The instruction event is only enabled when icount is used, cycles are
> always supported. Always defining get_cycle_count (but altering its
> behavior depending on CONFIG_USER_ONLY) allows us to remove some
> CONFIG_USER_ONLY #defines throughout the rest of the code.
> 
> Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
> ---
[...]>  #define SUPPORTED_EVENT_SENTINEL UINT16_MAX
>  static const pm_event pm_events[] = {
> +#ifndef CONFIG_USER_ONLY
> +    { .number = 0x008, /* INST_RETIRED */

"Instruction architecturally executed" seems more explicit to me.

> +      .supported = instructions_supported,
> +      .get_count = instructions_get_count
> +    },
> +    { .number = 0x011, /* CPU_CYCLES */
> +      .supported = event_always_supported,
> +      .get_count = cycles_get_count
> +    },
> +#endif
[...]
Aaron Lindsay March 19, 2018, 5:36 p.m. UTC | #3 
On Mar 18 23:48, Philippe Mathieu-Daudé wrote:
> On 03/16/2018 09:31 PM, Aaron Lindsay wrote:
> > The instruction event is only enabled when icount is used, cycles are
> > always supported. Always defining get_cycle_count (but altering its
> > behavior depending on CONFIG_USER_ONLY) allows us to remove some
> > CONFIG_USER_ONLY #defines throughout the rest of the code.
> > 
> > Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
> > ---
> [...]>  #define SUPPORTED_EVENT_SENTINEL UINT16_MAX
> >  static const pm_event pm_events[] = {
> > +#ifndef CONFIG_USER_ONLY
> > +    { .number = 0x008, /* INST_RETIRED */
> 
> "Instruction architecturally executed" seems more explicit to me.

I've updated v4 to include this wording as well.

> > +      .supported = instructions_supported,
> > +      .get_count = instructions_get_count
> > +    },
> > +    { .number = 0x011, /* CPU_CYCLES */
> > +      .supported = event_always_supported,
> > +      .get_count = cycles_get_count
> > +    },
> > +#endif
> [...]

-Aaron
diff mbox

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2fa8308..679897a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -15,6 +15,7 @@ 
 #include "arm_ldst.h"
 #include <zlib.h> /* For crc32 */
 #include "exec/semihost.h"
+#include "sysemu/cpus.h"
 #include "sysemu/kvm.h"
 #include "fpu/softfloat.h"
 
@@ -935,8 +936,54 @@  typedef struct pm_event {
     uint64_t (*get_count)(CPUARMState *, uint64_t cycles);
 } pm_event;
 
+/*
+ * Return the underlying cycle count for the PMU cycle counters. If we're in
+ * usermode, simply return 0.
+ */
+static uint64_t get_cycle_count(CPUARMState *env)
+{
+#ifndef CONFIG_USER_ONLY
+    return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+                   ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
+#else
+    return 0;
+#endif
+}
+
+static bool event_always_supported(CPUARMState *env)
+{
+    return true;
+}
+
+#ifndef CONFIG_USER_ONLY
+static uint64_t cycles_get_count(CPUARMState *env, uint64_t cycles)
+{
+    return cycles;
+}
+
+static bool instructions_supported(CPUARMState *env)
+{
+    return use_icount == 1 /* Precise instruction counting */;
+}
+
+static uint64_t instructions_get_count(CPUARMState *env, uint64_t cycles)
+{
+    return (uint64_t)cpu_get_icount_raw();
+}
+#endif
+
 #define SUPPORTED_EVENT_SENTINEL UINT16_MAX
 static const pm_event pm_events[] = {
+#ifndef CONFIG_USER_ONLY
+    { .number = 0x008, /* INST_RETIRED */
+      .supported = instructions_supported,
+      .get_count = instructions_get_count
+    },
+    { .number = 0x011, /* CPU_CYCLES */
+      .supported = event_always_supported,
+      .get_count = cycles_get_count
+    },
+#endif
     { .number = SUPPORTED_EVENT_SENTINEL }
 };
 static uint16_t supported_event_map[0x3f];
@@ -1016,8 +1063,6 @@  static CPAccessResult pmreg_access_swinc(CPUARMState *env,
     return pmreg_access(env, ri, isread);
 }
 
-#ifndef CONFIG_USER_ONLY
-
 static CPAccessResult pmreg_access_selr(CPUARMState *env,
                                         const ARMCPRegInfo *ri,
                                         bool isread)
@@ -1126,11 +1171,7 @@  static inline bool pmu_counter_filtered(CPUARMState *env, uint64_t pmxevtyper)
  */
 uint64_t pmccntr_op_start(CPUARMState *env)
 {
-    uint64_t cycles = 0;
-#ifndef CONFIG_USER_ONLY
-    cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
-#endif
+    uint64_t cycles = get_cycle_count(env);
 
     if (arm_ccnt_enabled(env) &&
           !pmu_counter_filtered(env, env->cp15.pmccfiltr_el0)) {
@@ -1268,26 +1309,6 @@  static void pmccntr_write32(CPUARMState *env, const ARMCPRegInfo *ri,
     pmccntr_write(env, ri, deposit64(cur_val, 0, 32, value));
 }
 
-#else /* CONFIG_USER_ONLY */
-
-uint64_t pmccntr_op_start(CPUARMState *env)
-{
-}
-
-void pmccntr_op_finish(CPUARMState *env, uint64_t prev_cycles)
-{
-}
-
-uint64_t pmu_op_start(CPUARMState *env)
-{
-}
-
-void pmu_op_finish(CPUARMState *env, uint64_t prev_cycles)
-{
-}
-
-#endif
-
 static void pmccfiltr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                             uint64_t value)
 {
@@ -1346,11 +1367,7 @@  static void pmevtyper_write(CPUARMState *env, const ARMCPRegInfo *ri,
     if (counter == 0x1f) {
         pmccfiltr_write(env, ri, value);
     } else if (counter < PMU_NUM_COUNTERS(env)) {
-        uint64_t cycles = 0;
-#ifndef CONFIG_USER_ONLY
-        cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
-#endif
+        uint64_t cycles = get_cycle_count(env);
         pmu_sync_counter(env, counter, cycles);
         env->cp15.c14_pmevtyper[counter] = value & 0xfe0003ff;
         pmu_sync_counter(env, counter, cycles);
@@ -1404,11 +1421,7 @@  static void pmevcntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                              uint64_t value, uint8_t counter)
 {
     if (counter < PMU_NUM_COUNTERS(env)) {
-        uint64_t cycles = 0;
-#ifndef CONFIG_USER_ONLY
-        cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
-#endif
+        uint64_t cycles = get_cycle_count(env);
         env->cp15.c14_pmevcntr[counter] = value;
         pmu_sync_counter(env, counter, cycles);
     }
@@ -1420,12 +1433,8 @@  static uint64_t pmevcntr_read(CPUARMState *env, const ARMCPRegInfo *ri,
                               uint8_t counter)
 {
     if (counter < PMU_NUM_COUNTERS(env)) {
-        uint64_t ret;
-        uint64_t cycles = 0;
-#ifndef CONFIG_USER_ONLY
-        cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
-#endif
+        uint64_t ret, cycles;
+        cycles = get_cycle_count(env);
         pmu_sync_counter(env, counter, cycles);
         ret = env->cp15.c14_pmevcntr[counter];
         pmu_sync_counter(env, counter, cycles);
@@ -1613,7 +1622,6 @@  static const ARMCPRegInfo v7_cp_reginfo[] = {
     /* Unimplemented so WI. */
     { .name = "PMSWINC", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 4,
       .access = PL0_W, .accessfn = pmreg_access_swinc, .type = ARM_CP_NOP },
-#ifndef CONFIG_USER_ONLY
     { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
       .access = PL0_RW, .type = ARM_CP_ALIAS,
       .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmselr),
@@ -1633,7 +1641,6 @@  static const ARMCPRegInfo v7_cp_reginfo[] = {
       .access = PL0_RW, .accessfn = pmreg_access_ccntr,
       .type = ARM_CP_IO,
       .readfn = pmccntr_read, .writefn = pmccntr_write, },
-#endif
     { .name = "PMCCFILTR", .cp = 15, .opc1 = 0, .crn = 14, .crm = 15, .opc2 = 7,
       .writefn = pmccfiltr_write_a32, .readfn = pmccfiltr_read_a32,
       .access = PL0_RW, .accessfn = pmreg_access,
@@ -5171,7 +5178,6 @@  void register_cp_regs_for_features(ARMCPU *cpu)
          * field as main ID register, and we implement only the cycle
          * count register.
          */
-#ifndef CONFIG_USER_ONLY
         ARMCPRegInfo pmcr = {
             .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0,
             .access = PL0_RW,
@@ -5225,7 +5231,6 @@  void register_cp_regs_for_features(ARMCPU *cpu)
             g_free(pmevtyper_name);
             g_free(pmevtyper_el0_name);
         }
-#endif
         ARMCPRegInfo clidr = {
             .name = "CLIDR", .state = ARM_CP_STATE_BOTH,
             .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1,