target/arm: Reorganize PMCCNTR accesses

pmccntr_read and pmccntr_write contained duplicate code that was already
being handled by pmccntr_sync. Consolidate the duplicated code into two
functions: pmccntr_op_start and pmccntr_op_finish. Add a companion to
c15_ccnt in CPUARMState so that we can simultaneously save both the
architectural register value and the last underlying cycle count - this
ensures time isn't lost and will also allow us to access the 'old'
architectural register value in order to detect overflows in later
patches.

Backports commit 5d05b9d462666ed21b7fef61aa45dec9aaa9f0ff from qemu

qemu/aarch64.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_aarch64
 #define pmccfiltr_write pmccfiltr_write_aarch64
 #define pmccntr_read pmccntr_read_aarch64
-#define pmccntr_sync pmccntr_sync_aarch64
 #define pmccntr_write pmccntr_write_aarch64
 #define pmccntr_write32 pmccntr_write32_aarch64
 #define pmcntenclr_write pmcntenclr_write_aarch64
@@ -4281,6 +4280,10 @@
 #define mls_op mls_op_aarch64
 #define new_tmp_a64 new_tmp_a64_aarch64
 #define new_tmp_a64_zero new_tmp_a64_zero_aarch64
+#define pmccntr_op_start pmccntr_op_start_aarch64
+#define pmccntr_op_finish pmccntr_op_finish_aarch64
+#define pmu_op_start pmu_op_start_aarch64
+#define pmu_op_finish pmu_op_finish_aarch64
 #define pred_esz_masks pred_esz_masks_aarch64
 #define raise_exception raise_exception_aarch64
 #define raise_exception_ra raise_exception_ra_aarch64

qemu/aarch64eb.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_aarch64eb
 #define pmccfiltr_write pmccfiltr_write_aarch64eb
 #define pmccntr_read pmccntr_read_aarch64eb
-#define pmccntr_sync pmccntr_sync_aarch64eb
 #define pmccntr_write pmccntr_write_aarch64eb
 #define pmccntr_write32 pmccntr_write32_aarch64eb
 #define pmcntenclr_write pmcntenclr_write_aarch64eb
@@ -4281,6 +4280,10 @@
 #define mls_op mls_op_aarch64eb
 #define new_tmp_a64 new_tmp_a64_aarch64eb
 #define new_tmp_a64_zero new_tmp_a64_zero_aarch64eb
+#define pmccntr_op_start pmccntr_op_start_aarch64eb
+#define pmccntr_op_finish pmccntr_op_finish_aarch64eb
+#define pmu_op_start pmu_op_start_aarch64eb
+#define pmu_op_finish pmu_op_finish_aarch64eb
 #define pred_esz_masks pred_esz_masks_aarch64eb
 #define raise_exception raise_exception_aarch64eb
 #define raise_exception_ra raise_exception_ra_aarch64eb

qemu/arm.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_arm
 #define pmccfiltr_write pmccfiltr_write_arm
 #define pmccntr_read pmccntr_read_arm
-#define pmccntr_sync pmccntr_sync_arm
 #define pmccntr_write pmccntr_write_arm
 #define pmccntr_write32 pmccntr_write32_arm
 #define pmcntenclr_write pmcntenclr_write_arm
@@ -3289,6 +3288,10 @@
 #define gen_cmtst_i64 gen_cmtst_i64_arm
 #define mla_op mla_op_arm
 #define mls_op mls_op_arm
+#define pmccntr_op_start pmccntr_op_start_arm
+#define pmccntr_op_finish pmccntr_op_finish_arm
+#define pmu_op_start pmu_op_start_arm
+#define pmu_op_finish pmu_op_finish_arm
 #define raise_exception raise_exception_arm
 #define raise_exception_ra raise_exception_ra_arm
 #define sli_op sli_op_arm

qemu/armeb.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_armeb
 #define pmccfiltr_write pmccfiltr_write_armeb
 #define pmccntr_read pmccntr_read_armeb
-#define pmccntr_sync pmccntr_sync_armeb
 #define pmccntr_write pmccntr_write_armeb
 #define pmccntr_write32 pmccntr_write32_armeb
 #define pmcntenclr_write pmcntenclr_write_armeb
@@ -3289,6 +3288,10 @@
 #define gen_cmtst_i64 gen_cmtst_i64_armeb
 #define mla_op mla_op_armeb
 #define mls_op mls_op_armeb
+#define pmccntr_op_start pmccntr_op_start_armeb
+#define pmccntr_op_finish pmccntr_op_finish_armeb
+#define pmu_op_start pmu_op_start_armeb
+#define pmu_op_finish pmu_op_finish_armeb
 #define raise_exception raise_exception_armeb
 #define raise_exception_ra raise_exception_ra_armeb
 #define sli_op sli_op_armeb

qemu/header_gen.py

@@ -2182,7 +2182,6 @@ symbols = (
     'pickNaNMulAdd',
     'pmccfiltr_write',
     'pmccntr_read',
-    'pmccntr_sync',
     'pmccntr_write',
     'pmccntr_write32',
     'pmcntenclr_write',
@@ -3298,6 +3297,10 @@ arm_symbols = (
     'gen_cmtst_i64',
     'mla_op',
     'mls_op',
+    'pmccntr_op_start',
+    'pmccntr_op_finish',
+    'pmu_op_start',
+    'pmu_op_finish',
     'raise_exception',
     'raise_exception_ra',
     'sli_op',
@@ -4320,6 +4323,10 @@ aarch64_symbols = (
     'mls_op',
     'new_tmp_a64',
     'new_tmp_a64_zero',
+    'pmccntr_op_start',
+    'pmccntr_op_finish',
+    'pmu_op_start',
+    'pmu_op_finish',
     'pred_esz_masks',
     'raise_exception',
     'raise_exception_ra',

qemu/m68k.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_m68k
 #define pmccfiltr_write pmccfiltr_write_m68k
 #define pmccntr_read pmccntr_read_m68k
-#define pmccntr_sync pmccntr_sync_m68k
 #define pmccntr_write pmccntr_write_m68k
 #define pmccntr_write32 pmccntr_write32_m68k
 #define pmcntenclr_write pmcntenclr_write_m68k

qemu/mips.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_mips
 #define pmccfiltr_write pmccfiltr_write_mips
 #define pmccntr_read pmccntr_read_mips
-#define pmccntr_sync pmccntr_sync_mips
 #define pmccntr_write pmccntr_write_mips
 #define pmccntr_write32 pmccntr_write32_mips
 #define pmcntenclr_write pmcntenclr_write_mips

qemu/mips64.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_mips64
 #define pmccfiltr_write pmccfiltr_write_mips64
 #define pmccntr_read pmccntr_read_mips64
-#define pmccntr_sync pmccntr_sync_mips64
 #define pmccntr_write pmccntr_write_mips64
 #define pmccntr_write32 pmccntr_write32_mips64
 #define pmcntenclr_write pmcntenclr_write_mips64

qemu/mips64el.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_mips64el
 #define pmccfiltr_write pmccfiltr_write_mips64el
 #define pmccntr_read pmccntr_read_mips64el
-#define pmccntr_sync pmccntr_sync_mips64el
 #define pmccntr_write pmccntr_write_mips64el
 #define pmccntr_write32 pmccntr_write32_mips64el
 #define pmcntenclr_write pmcntenclr_write_mips64el

qemu/mipsel.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_mipsel
 #define pmccfiltr_write pmccfiltr_write_mipsel
 #define pmccntr_read pmccntr_read_mipsel
-#define pmccntr_sync pmccntr_sync_mipsel
 #define pmccntr_write pmccntr_write_mipsel
 #define pmccntr_write32 pmccntr_write32_mipsel
 #define pmcntenclr_write pmcntenclr_write_mipsel

qemu/powerpc.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_powerpc
 #define pmccfiltr_write pmccfiltr_write_powerpc
 #define pmccntr_read pmccntr_read_powerpc
-#define pmccntr_sync pmccntr_sync_powerpc
 #define pmccntr_write pmccntr_write_powerpc
 #define pmccntr_write32 pmccntr_write32_powerpc
 #define pmcntenclr_write pmcntenclr_write_powerpc

qemu/sparc.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_sparc
 #define pmccfiltr_write pmccfiltr_write_sparc
 #define pmccntr_read pmccntr_read_sparc
-#define pmccntr_sync pmccntr_sync_sparc
 #define pmccntr_write pmccntr_write_sparc
 #define pmccntr_write32 pmccntr_write32_sparc
 #define pmcntenclr_write pmcntenclr_write_sparc

qemu/sparc64.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_sparc64
 #define pmccfiltr_write pmccfiltr_write_sparc64
 #define pmccntr_read pmccntr_read_sparc64
-#define pmccntr_sync pmccntr_sync_sparc64
 #define pmccntr_write pmccntr_write_sparc64
 #define pmccntr_write32 pmccntr_write32_sparc64
 #define pmcntenclr_write pmcntenclr_write_sparc64

qemu/target/arm/cpu.h

@@ -464,10 +464,20 @@ typedef struct CPUARMState {
         uint64_t oslsr_el1; /* OS Lock Status */
         uint64_t mdcr_el2;
         uint64_t mdcr_el3;
-        /* If the counter is enabled, this stores the last time the counter
+        /* Stores the architectural value of the counter *the last time it was
-         * was reset. Otherwise it stores the counter value
+         * updated* by pmccntr_op_start. Accesses should always be surrounded
+         * by pmccntr_op_start/pmccntr_op_finish to guarantee the latest
+         * architecturally-correct value is being read/set.
          */
         uint64_t c15_ccnt;
+        /* Stores the delta between the architectural value and the underlying
+         * cycle count during normal operation. It is used to update c15_ccnt
+         * to be the correct architectural value before accesses. During
+         * accesses, c15_ccnt_delta contains the underlying count being used
+         * for the access, after which it reverts to the delta value in
+         * pmccntr_op_finish.
+         */
+        uint64_t c15_ccnt_delta;
         uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
         uint64_t vpidr_el2; /* Virtualization Processor ID Register */
         uint64_t vmpidr_el2; /* Virtualization Multiprocessor ID Register */
@@ -913,15 +923,25 @@ int cpu_arm_signal_handler(int host_signum, void *pinfo,
                            void *puc);
 
 /**
- * pmccntr_sync
+ * pmccntr_op_start/finish
  * @env: CPUARMState
  *
- * Synchronises the counter in the PMCCNTR. This must always be called twice,
+ * Convert the counter in the PMCCNTR between its delta form (the typical mode
- * once before any action that might affect the timer and again afterwards.
+ * when it's enabled) and the guest-visible value. These two calls must always
- * The function is used to swap the state of the register if required.
+ * surround any action which might affect the counter.
- * This only happens when not in user mode (!CONFIG_USER_ONLY)
  */
-void pmccntr_sync(CPUARMState *env);
+void pmccntr_op_start(CPUARMState *env);
+void pmccntr_op_finish(CPUARMState *env);
+
+/**
+ * pmu_op_start/finish
+ * @env: CPUARMState
+ * Convert all PMU counters between their delta form (the typical mode when
+ * they are enabled) and the guest-visible values. These two calls must
+ * surround any action which might affect the counters.
+ */
+void pmu_op_start(CPUARMState *env);
+void pmu_op_finish(CPUARMState *env);
 
 /* SCTLR bit meanings. Several bits have been reused in newer
  * versions of the architecture; in that case we define constants

qemu/target/arm/helper.c

@@ -950,27 +950,63 @@ static inline bool arm_ccnt_enabled(CPUARMState *env)
     return true;
 }
 
-void pmccntr_sync(CPUARMState *env)
+/*
+ * Ensure c15_ccnt is the guest-visible count so that operations such as
+ * enabling/disabling the counter or filtering, modifying the count itself,
+ * etc. can be done logically. This is essentially a no-op if the counter is
+ * not enabled at the time of the call.
+ */
+void pmccntr_op_start(CPUARMState *env)
 {
-    uint64_t temp_ticks;
+    uint64_t cycles = 0;
-
+    cycles = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-    temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
                           NANOSECONDS_PER_SECOND, 1000000);
 
-    if (env->cp15.c9_pmcr & PMCRD) {
-        /* Increment once every 64 processor clock cycles */
-        temp_ticks /= 64;
-    }
-
     if (arm_ccnt_enabled(env)) {
-        env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt;
+        uint64_t eff_cycles = cycles;
+        if (env->cp15.c9_pmcr & PMCRD) {
+            /* Increment once every 64 processor clock cycles */
+            eff_cycles /= 64;
+        }
+
+        env->cp15.c15_ccnt = eff_cycles - env->cp15.c15_ccnt_delta;
     }
+    env->cp15.c15_ccnt_delta = cycles;
+}
+
+/*
+ * If PMCCNTR is enabled, recalculate the delta between the clock and the
+ * guest-visible count. A call to pmccntr_op_finish should follow every call to
+ * pmccntr_op_start.
+ */
+void pmccntr_op_finish(CPUARMState *env)
+{
+    if (arm_ccnt_enabled(env)) {
+        uint64_t prev_cycles = env->cp15.c15_ccnt_delta;
+
+        if (env->cp15.c9_pmcr & PMCRD) {
+            /* Increment once every 64 processor clock cycles */
+            prev_cycles /= 64;
+        }
+
+        env->cp15.c15_ccnt_delta = prev_cycles - env->cp15.c15_ccnt;
+    }
+}
+
+void pmu_op_start(CPUARMState *env)
+{
+    pmccntr_op_start(env);
+}
+
+void pmu_op_finish(CPUARMState *env)
+{
+    pmccntr_op_finish(env);
 }
 
 static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                        uint64_t value)
 {
-    pmccntr_sync(env);
+    pmu_op_start(env);
 
     if (value & PMCRC) {
         /* The counter has been reset */
@@ -981,26 +1017,16 @@ static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     env->cp15.c9_pmcr &= ~0x39;
     env->cp15.c9_pmcr |= (value & 0x39);
 
-    pmccntr_sync(env);
+    pmu_op_finish(env);
 }
 
 static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-    uint64_t total_ticks;
+    uint64_t ret;
-
+    pmccntr_op_start(env);
-    if (!arm_ccnt_enabled(env)) {
+    ret = env->cp15.c15_ccnt;
-        /* Counter is disabled, do not change value */
+    pmccntr_op_finish(env);
-        return env->cp15.c15_ccnt;
+    return ret;
-    }
-
-    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
-                           NANOSECONDS_PER_SECOND, 1000000);
-
-    if (env->cp15.c9_pmcr & PMCRD) {
-        /* Increment once every 64 processor clock cycles */
-        total_ticks /= 64;
-    }
-    return total_ticks - env->cp15.c15_ccnt;
 }
 
 static void pmselr_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -1017,22 +1043,9 @@ static void pmselr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                         uint64_t value)
 {
-    uint64_t total_ticks;
+    pmccntr_op_start(env);
-
+    env->cp15.c15_ccnt = value;
-    if (!arm_ccnt_enabled(env)) {
+    pmccntr_op_finish(env);
-        /* Counter is disabled, set the absolute value */
-        env->cp15.c15_ccnt = value;
-        return;
-    }
-
-    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
-                           NANOSECONDS_PER_SECOND, 1000000);
-
-    if (env->cp15.c9_pmcr & PMCRD) {
-        /* Increment once every 64 processor clock cycles */
-        total_ticks /= 64;
-    }
-    env->cp15.c15_ccnt = total_ticks - value;
 }
 
 static void pmccntr_write32(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -1045,7 +1058,19 @@ static void pmccntr_write32(CPUARMState *env, const ARMCPRegInfo *ri,
 
 #else /* CONFIG_USER_ONLY */
 
-void pmccntr_sync(CPUARMState *env)
+void pmccntr_op_start(CPUARMState *env)
+{
+}
+
+void pmccntr_op_finish(CPUARMState *env)
+{
+}
+
+void pmu_op_start(CPUARMState *env)
+{
+}
+
+void pmu_op_finish(CPUARMState *env)
 {
 }
 
@@ -1054,9 +1079,9 @@ void pmccntr_sync(CPUARMState *env)
 static void pmccfiltr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                             uint64_t value)
 {
-    pmccntr_sync(env);
+    pmccntr_op_start(env);
     env->cp15.pmccfiltr_el0 = value & 0xfc000000;
-    pmccntr_sync(env);
+    pmccntr_op_finish(env);
 }
 
 static void pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri,

qemu/x86_64.h

@@ -2176,7 +2176,6 @@
 #define pickNaNMulAdd pickNaNMulAdd_x86_64
 #define pmccfiltr_write pmccfiltr_write_x86_64
 #define pmccntr_read pmccntr_read_x86_64
-#define pmccntr_sync pmccntr_sync_x86_64
 #define pmccntr_write pmccntr_write_x86_64
 #define pmccntr_write32 pmccntr_write32_x86_64
 #define pmcntenclr_write pmcntenclr_write_x86_64