[PATCH 07/19] target/ppc/pmu_book3s_helper.c: icount fine tuning

[Daniel Henrique Barboza <danielhb413@gmail.com>]

In the helper logic we're making 2 icount_get_raw() calls (via
get_insns()) in different places: one in update_PMCs() and another
in the helper_store_mmcr0() when the new base_icount is set.

We don't need two distinct calls in two different places. In fact,
calling them in the same point brings more consistency when turning the
PMU on/off during heavy load. We're also want to retrieve the current
icount value as soon as possible to avoid counting extra instructions.

To do that, we're introducing the concept of 'icount_delta', which is
the icount difference between the time PMU started and the time where
we are updating the counters.

Also, to behave more like the real hardware, discount the mtspr() calls
that turns the PMU on/off when we're about to set the PMCs values.

With these changes, running a pseries TCG with an icount shift of zero,
in an Intel i7-8650U laptop running Fedora 34, the kernel PMU
'count_instructions' test (kernel tree,
tools/testing/selftests/powerpc/pmu) gives a 99.9% average accurracy
when sampling 10M instructions:

[root@localhost powerpc]# ./pmu/count_instructions
test: count_instructions
tags: git_version:v5.13-5357-gdbe69e433722
Binding to cpu 0
main test running as pid 652
Overhead of null loop: 2315 instructions
instructions: result 1002315 running/enabled 1582058
cycles: result 4005276 running/enabled 1343324
Looped for 1000000 instructions, overhead 2315
Expected 1002315
Actual   1002315
Delta    0, 0.000000%
instructions: result 10010235 running/enabled 11598016
cycles: result 40036956 running/enabled 11356940
Looped for 10000000 instructions, overhead 2315
Expected 10002315
Actual   10010235
Delta    7920, 0.079119%

This accuracy is good enough to validate the EBB (Event-Based Branch)
support that we're going to implement shortly.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 target/ppc/pmu_book3s_helper.c | 46 ++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
index 99e62bd37b..6292b96db9 100644
--- a/target/ppc/pmu_book3s_helper.c
+++ b/target/ppc/pmu_book3s_helper.c
@@ -17,32 +17,27 @@
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
 
-static uint64_t get_insns(void)
-{
-    return (uint64_t)icount_get_raw();
-}
 
-static uint64_t get_cycles(uint64_t insns)
+static uint64_t get_cycles(uint64_t icount_delta)
 {
     /* Placeholder value */
-    return insns * 4;
+    return icount_delta * 4;
 }
 
 static void update_PMC_PM_INST_CMPL(CPUPPCState *env, int sprn,
-                                    uint64_t curr_icount)
+                                    uint64_t icount_delta)
 {
-    env->spr[sprn] += curr_icount - env->pmu_base_icount;
+    env->spr[sprn] += icount_delta;
 }
 
 static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
-                              uint64_t curr_icount)
+                              uint64_t icount_delta)
 {
-    uint64_t insns = curr_icount - env->pmu_base_icount;
-    env->spr[sprn] += get_cycles(insns);
+    env->spr[sprn] += get_cycles(icount_delta);
 }
 
 static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
-                                        uint64_t curr_icount)
+                                        uint64_t icount_delta)
 {
     int event;
 
@@ -68,10 +63,10 @@ static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
 
     switch (event) {
     case 0x2:
-        update_PMC_PM_INST_CMPL(env, sprn, curr_icount);
+        update_PMC_PM_INST_CMPL(env, sprn, icount_delta);
         break;
     case 0x1E:
-        update_PMC_PM_CYC(env, sprn, curr_icount);
+        update_PMC_PM_CYC(env, sprn, icount_delta);
         break;
     default:
         return;
@@ -84,21 +79,21 @@ static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
  * There is no need to update the base icount of each PMC since
  * the PMU is not running.
  */
-static void update_PMCs_on_freeze(CPUPPCState *env)
+static void update_PMCs(CPUPPCState *env, uint64_t icount_delta)
 {
-    uint64_t curr_icount = get_insns();
     int sprn;
 
     for (sprn = SPR_POWER_PMC1; sprn < SPR_POWER_PMC5; sprn++) {
-        update_programmable_PMC_reg(env, sprn, curr_icount);
+        update_programmable_PMC_reg(env, sprn, icount_delta);
     }
 
-    update_PMC_PM_INST_CMPL(env, SPR_POWER_PMC5, curr_icount);
-    update_PMC_PM_CYC(env, SPR_POWER_PMC6, curr_icount);
+    update_PMC_PM_INST_CMPL(env, SPR_POWER_PMC5, icount_delta);
+    update_PMC_PM_CYC(env, SPR_POWER_PMC6, icount_delta);
 }
 
 void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
 {
+    uint64_t curr_icount = (uint64_t)icount_get_raw();
     bool curr_FC = env->spr[SPR_POWER_MMCR0] & MMCR0_FC;
     bool new_FC = value & MMCR0_FC;
 
@@ -115,9 +110,18 @@ void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
      */
     if (curr_FC != new_FC) {
         if (!curr_FC) {
-            update_PMCs_on_freeze(env);
+            uint64_t icount_delta = (curr_icount - env->pmu_base_icount);
+
+            /* Exclude both mtsprs() that opened and closed the timer */
+            icount_delta -= 2;
+
+            /*
+             * Update the counter with the instructions run
+             * until the freeze.
+             */
+            update_PMCs(env, icount_delta);
         } else {
-            env->pmu_base_icount = get_insns();
+            env->pmu_base_icount = curr_icount;
         }
     }
 
-- 
2.31.1