Skip to contentSkip to conversation

target-arm: Extend PMCCNTR for ARMv8

PATCH Applied Peter Crosthwaite 3 participants 11 messages Aug 26, 2014 – Aug 29, 2014

lore
Share & tools
lore

Peter Crosthwaite

author
#0 lore raw

v4 cover target-arm: Extend PMCCNTR for ARMv8

This patch series continues on from Alistairs original PMCCNTR patch work. The counter is extended to 64-bit.

V4:
- Drop all EL awareness.
V3:
-Tidy up the arm_ccnt_enabled()
-Fixed an old commit message refering to the CCNT_ENABLED
macro
-Do EL change sync in pstate_write instead of in multiple
code paths.
-Addressed PMM V2 review
V2:
-Fix some typos identified by Christopher Covington -Convert the CCNT_ENABLED macro to the arm_ccnt_enabled
function

Alistair Francis (6):
target-arm: Make the ARM PMCCNTR register 64-bit
target-arm: Implement PMCCNTR_EL0 and related registers
target-arm: Add arm_ccnt_enabled function
target-arm: Implement pmccntr_sync function
target-arm: Remove old code and replace with new functions
target-arm: Implement pmccfiltr_write function

Peter Crosthwaite (1):
arm: Implement PMCCNTR 32b read-modify-write

 target-arm/cpu.h    |  18 +++++--
 target-arm/helper.c | 138 ++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 117 insertions(+), 39 deletions(-)
Signature · 1 line
2.1.0.1.g27b9230
author
#1 lore raw

PATCH v4 1/7 target-arm: Make the ARM PMCCNTR register 64-bit

From: Alistair Francis <[email protected]>

This makes the PMCCNTR register 64-bit to allow for the 64-bit ARMv8 version.

Signed-off-by Peter Crosthwaite <[email protected]>
Signed-off-by Alistair Francis <[email protected]>

---
changed since v2:
Fixed indentation of muldiv64s

 target-arm/cpu.h    |  2 +-
 target-arm/helper.c | 19 +++++++++----------
 2 files changed, 10 insertions(+), 11 deletions(-)
target-arm/cpu.h +1 -1
index 8098b8d..4d1fb19 100644
@@ -224,7 +224,7 @@ typedef struct CPUARMState {
         /* If the counter is enabled, this stores the last time the counter
          * was reset. Otherwise it stores the counter value
          */
-        uint32_t c15_ccnt;
+        uint64_t c15_ccnt;
     } cp15;
 
     struct {
target-arm/helper.c +9 -10
index 2a77c97..711ca12 100644
@@ -551,11 +551,10 @@ static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri)
 static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                        uint64_t value)
 {
-    /* Don't computer the number of ticks in user mode */
-    uint32_t temp_ticks;
+    uint64_t temp_ticks;
 
-    temp_ticks = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) *
-                  get_ticks_per_sec() / 1000000;
+    temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+                          get_ticks_per_sec(), 1000000);
 
     if (env->cp15.c9_pmcr & PMCRE) {
         /* If the counter is enabled */

@@ -587,15 +586,15 @@ static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 
 static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-    uint32_t total_ticks;
+    uint64_t total_ticks;
 
     if (!(env->cp15.c9_pmcr & PMCRE)) {
         /* Counter is disabled, do not change value */
         return env->cp15.c15_ccnt;
     }
 
-    total_ticks = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) *
-                  get_ticks_per_sec() / 1000000;
+    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+                           get_ticks_per_sec(), 1000000);
 
     if (env->cp15.c9_pmcr & PMCRD) {
         /* Increment once every 64 processor clock cycles */

@@ -607,7 +606,7 @@ static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                         uint64_t value)
 {
-    uint32_t total_ticks;
+    uint64_t total_ticks;
 
     if (!(env->cp15.c9_pmcr & PMCRE)) {
         /* Counter is disabled, set the absolute value */

@@ -615,8 +614,8 @@ static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
         return;
     }
 
-    total_ticks = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) *
-                  get_ticks_per_sec() / 1000000;
+    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+                           get_ticks_per_sec(), 1000000);
 
     if (env->cp15.c9_pmcr & PMCRD) {
         /* Increment once every 64 processor clock cycles */
Signature · 1 line
2.1.0.1.g27b9230
author
#2 lore raw

PATCH v4 2/7 arm: Implement PMCCNTR 32b read-modify-write

In reply to Peter Crosthwaite

The register is now 64bit, however a 32 bit write to the register should leave the higher bits unchanged. The open coded write handler does not implement this, so we need to read-modify-write accordingly.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 target-arm/helper.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
target-arm/helper.c +10 -1
index 711ca12..0d2ee41 100644
@@ -623,6 +623,15 @@ static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     }
     env->cp15.c15_ccnt = total_ticks - value;
 }
+
+static void pmccntr_write32(CPUARMState *env, const ARMCPRegInfo *ri,
+                            uint64_t value)
+{
+    uint64_t cur_val = pmccntr_read(env, NULL);
+
+    pmccntr_write(env, ri, deposit64(cur_val, 0, 32, value));
+}
+
 #endif
 
 static void pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri,

@@ -754,7 +763,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
 #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,
-      .readfn = pmccntr_read, .writefn = pmccntr_write,
+      .readfn = pmccntr_read, .writefn = pmccntr_write32,
       .accessfn = pmreg_access },
 #endif
     { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
Signature · 1 line
2.1.0.1.g27b9230
Reviewed-by on v4 2/7
#3 lore raw

Re: [Qemu-devel] [PATCH target-arm v4 2/7] arm: Implement PMCCNTR 32b read-modify-write

Quoted 5 lines · Peter Crosthwaite

The register is now 64bit, however a 32 bit write to the register should leave the higher bits unchanged. The open coded write handler does not implement this, so we need to read-modify-write accordingly.

Signed-off-by Peter Crosthwaite <[email protected]>
Reviewed-by Alistair Francis <[email protected]>
Quoted 14 lines

---

 target-arm/helper.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
target-arm/helper.c +1 -0
index 711ca12..0d2ee41 100644
@@ -623,6 +623,15 @@ static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     }
     env->cp15.c15_ccnt = total_ticks - value;
 }
+

target-arm/helper.c static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri, · line 636

Quoted 10 lines
target-arm/helper.c +8 -0
+static void pmccntr_write32(CPUARMState *env, const ARMCPRegInfo *ri,
+                            uint64_t value)
+{
+    uint64_t cur_val = pmccntr_read(env, NULL);
+
+    pmccntr_write(env, ri, deposit64(cur_val, 0, 32, value));
+}
+
 #endif

Looks good to me

Quoted 13 lines
target-arm/helper.c +1 -2
 static void pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri,

@@ -754,7 +763,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
 #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,
-      .readfn = pmccntr_read, .writefn = pmccntr_write,
+      .readfn = pmccntr_read, .writefn = pmccntr_write32,
       .accessfn = pmreg_access },
 #endif
     { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
--

2.1.0.1.g27b9230

author
#4 lore raw

PATCH v4 3/7 target-arm: Implement PMCCNTR_EL0 and related registers

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

This patch adds support for the ARMv8 version of the PMCCNTR and related registers. It also starts to implement the PMCCFILTR_EL0 register.

Signed-off-by Alistair Francis <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>

---
Changed since v2:
Widen cpu state fields as needed
Reorder CP def fields
Add NO_MIGRATES to CP dups.

 target-arm/cpu.h    |  5 +++--
 target-arm/helper.c | 45 +++++++++++++++++++++++++++++++++++++++------
 2 files changed, 42 insertions(+), 8 deletions(-)
target-arm/cpu.h +3 -2
index 4d1fb19..609ce17 100644
@@ -191,8 +191,8 @@ typedef struct CPUARMState {
         uint64_t par_el1;  /* Translation result. */
         uint32_t c9_insn; /* Cache lockdown registers.  */
         uint32_t c9_data;
-        uint32_t c9_pmcr; /* performance monitor control register */
-        uint32_t c9_pmcnten; /* perf monitor counter enables */
+        uint64_t c9_pmcr; /* performance monitor control register */
+        uint64_t c9_pmcnten; /* perf monitor counter enables */
         uint32_t c9_pmovsr; /* perf monitor overflow status */
         uint32_t c9_pmxevtyper; /* perf monitor event type */
         uint32_t c9_pmuserenr; /* perf monitor user enable */

@@ -225,6 +225,7 @@ typedef struct CPUARMState {
          * was reset. Otherwise it stores the counter value
          */
         uint64_t c15_ccnt;
+        uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
     } cp15;
 
     struct {
target-arm/helper.c +39 -6

75 lines not on this page · raw message

Signature · 1 line
2.1.0.1.g27b9230
author
#5 lore raw

PATCH v4 4/7 target-arm: Add arm_ccnt_enabled function

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

Include a helper function to determine if the CCNT counter is enabled.

Signed-off-by Alistair Francis <[email protected]>

[ PC changes
* Remove EL based checks
]

Signed-off-by Peter Crosthwaite <[email protected]>

---
Changed since v3
Remove checks based on EL. Changed since v2 (PMM review):
Blank line for readability
Use switch instead of cascading ifs. Use true and false.
Drop extraneous #endif #if

 target-arm/helper.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
target-arm/helper.c +12 -0
index 13507f7..e6c82ab 100644
@@ -548,6 +548,18 @@ static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri)
 }
 
 #ifndef CONFIG_USER_ONLY
+
+static inline bool arm_ccnt_enabled(CPUARMState *env)
+{
+    /* This does not support checking PMCCFILTR_EL0 register */
+
+    if (!(env->cp15.c9_pmcr & PMCRE)) {
+        return false;
+    }
+
+    return true;
+}
+
 static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                        uint64_t value)
 {
Signature · 1 line
2.1.0.1.g27b9230
author
#6 lore raw

PATCH v4 5/7 target-arm: Implement pmccntr_sync function

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

This is used to synchronise the PMCCNTR counter and swap its state between enabled and disabled if required. It must always be called twice, both before and after any logic that could change the state of the PMCCNTR counter.

Signed-off-by Alistair Francis <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>

---
Changed since v2:
Move prototype further up cpu.h

Remembering that the c15_ccnt register stores the last time the counter was reset if enabled. If disabled it stores the counter value (when it was disabled).

The three use cases are as below:
--
Starts enabled/disabled and is staying enabled/disabled --
The two calls to pmccntr_sync cancel each other out. Each call implements this logic:
env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt

Which expands to:
env->cp15.c15_ccnt = temp_ticks -

                     (temp_ticks - env->cp15.c15_ccnt)

env->cp15.c15_ccnt = env->cp15.c15_ccnt

--
Starts enabled, gets disabled
--
The logic is run during the first call while during the second call it is not. That means that c15_ccnt changes from storing the last time the counter was reset, to storing the absolute value of the counter.

--
Starts disabled, gets enabled
--
During the fist call no changes are made, while during the second call the register is changed. This changes it from storing the absolute value to storing the last time the counter was reset.

 target-arm/cpu.h    | 11 +++++++++++
 target-arm/helper.c | 23 +++++++++++++++++++++++
 2 files changed, 34 insertions(+)
target-arm/cpu.h +11 -0
index 609ce17..beb7eed 100644
@@ -353,6 +353,17 @@ int cpu_arm_signal_handler(int host_signum, void *pinfo,
 int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
                              int mmu_idx);
 
+/**
+ * pmccntr_sync
+ * @cpu: ARMCPU
+ *
+ * Syncronises the counter in the PMCCNTR. This must always be called twice,
+ * once before any action that might effect the timer and again afterwards.
+ * The function is used to swap the state of the register if required.
+ * This only happens when not in user mode (!CONFIG_USER_ONLY)
+ */
+void pmccntr_sync(CPUARMState *env);
+
 /* SCTLR bit meanings. Several bits have been reused in newer
  * versions of the architecture; in that case we define constants
  * for both old and new bit meanings. Code which tests against those
target-arm/helper.c +23 -0
index e6c82ab..fa79dfa 100644
@@ -560,6 +560,23 @@ static inline bool arm_ccnt_enabled(CPUARMState *env)
     return true;
 }
 
+void pmccntr_sync(CPUARMState *env)
+{
+    uint64_t temp_ticks;
+
+    temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+                          get_ticks_per_sec(), 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;
+    }
+}
+
 static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                        uint64_t value)
 {

@@ -644,6 +661,12 @@ static void pmccntr_write32(CPUARMState *env, const ARMCPRegInfo *ri,
     pmccntr_write(env, ri, deposit64(cur_val, 0, 32, value));
 }
 
+#else /* CONFIG_USER_ONLY */
+
+void pmccntr_sync(CPUARMState *env)
+{
+}
+
 #endif
 
 static void pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri,
Signature · 1 line
2.1.0.1.g27b9230
author
#7 lore raw

PATCH v4 6/7 target-arm: Remove old code and replace with new functions

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

Remove the old PMCCNTR code and replace it with calls to the new pmccntr_sync() and arm_ccnt_enabled() functions.

Signed-off-by Alistair Francis <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>

---
Changed since v2:
Commit msg tweaks.

 target-arm/helper.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)
target-arm/helper.c +4 -23
index fa79dfa..d213ed1 100644
@@ -580,20 +580,7 @@ void pmccntr_sync(CPUARMState *env)
 static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                        uint64_t value)
 {
-    uint64_t temp_ticks;
-
-    temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
-                          get_ticks_per_sec(), 1000000);
-
-    if (env->cp15.c9_pmcr & PMCRE) {
-        /* If the counter is enabled */
-        if (env->cp15.c9_pmcr & PMCRD) {
-            /* Increment once every 64 processor clock cycles */
-            env->cp15.c15_ccnt = (temp_ticks/64) - env->cp15.c15_ccnt;
-        } else {
-            env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt;
-        }
-    }
+    pmccntr_sync(env);
 
     if (value & PMCRC) {
         /* The counter has been reset */

@@ -604,20 +591,14 @@ static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     env->cp15.c9_pmcr &= ~0x39;
     env->cp15.c9_pmcr |= (value & 0x39);
 
-    if (env->cp15.c9_pmcr & PMCRE) {
-        if (env->cp15.c9_pmcr & PMCRD) {
-            /* Increment once every 64 processor clock cycles */
-            temp_ticks /= 64;
-        }
-        env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt;
-    }
+    pmccntr_sync(env);
 }
 
 static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     uint64_t total_ticks;
 
-    if (!(env->cp15.c9_pmcr & PMCRE)) {
+    if (!arm_ccnt_enabled(env)) {
         /* Counter is disabled, do not change value */
         return env->cp15.c15_ccnt;
     }

@@ -637,7 +618,7 @@ static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 {
     uint64_t total_ticks;
 
-    if (!(env->cp15.c9_pmcr & PMCRE)) {
+    if (!arm_ccnt_enabled(env)) {
         /* Counter is disabled, set the absolute value */
         env->cp15.c15_ccnt = value;
         return;
Signature · 1 line
2.1.0.1.g27b9230
author
#8 lore raw

PATCH v4 7/7 target-arm: Implement pmccfiltr_write function

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

This is the function that is called when writing to the PMCCFILTR_EL0 register

Signed-off-by Alistair Francis <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>

---

 target-arm/helper.c | 9 +++++++++
 1 file changed, 9 insertions(+)
target-arm/helper.c +9 -0
index d213ed1..2b95f33 100644
@@ -650,6 +650,14 @@ void pmccntr_sync(CPUARMState *env)
 
 #endif
 
+static void pmccfiltr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                            uint64_t value)
+{
+    pmccntr_sync(env);
+    env->cp15.pmccfiltr_el0 = value & 0x7E000000;
+    pmccntr_sync(env);
+}
+
 static void pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri,
                             uint64_t value)
 {

@@ -801,6 +809,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
 #endif
     { .name = "PMCCFILTR_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 15, .opc2 = 7,
+      .writefn = pmccfiltr_write,
       .access = PL0_RW, .accessfn = pmreg_access,
       .type = ARM_CP_IO,
       .fieldoffset = offsetof(CPUARMState, cp15.pmccfiltr_el0),
Signature · 1 line
2.1.0.1.g27b9230
Applied
#9 lore raw
In reply to Peter Crosthwaite
Quoted 26 lines · Peter Crosthwaite

This patch series continues on from Alistairs original PMCCNTR patch work. The counter is extended to 64-bit.

V4:
- Drop all EL awareness.
V3:
-Tidy up the arm_ccnt_enabled()
-Fixed an old commit message refering to the CCNT_ENABLED
macro
-Do EL change sync in pstate_write instead of in multiple
code paths.
-Addressed PMM V2 review
V2:
-Fix some typos identified by Christopher Covington -Convert the CCNT_ENABLED macro to the arm_ccnt_enabled
function

Alistair Francis (6):
target-arm: Make the ARM PMCCNTR register 64-bit
target-arm: Implement PMCCNTR_EL0 and related registers
target-arm: Add arm_ccnt_enabled function
target-arm: Implement pmccntr_sync function
target-arm: Remove old code and replace with new functions
target-arm: Implement pmccfiltr_write function

Quoted 2 lines

Peter Crosthwaite (1):
arm: Implement PMCCNTR 32b read-modify-write

Applied to target-arm.next, thanks. (I fixed some minor typos in the doc comment for pmccntr_sync.)

-- PMM

author
#10 lore raw
Quoted 27 lines · Peter Maydell
Quoted 26 lines · Peter Crosthwaite

This patch series continues on from Alistairs original PMCCNTR patch work. The counter is extended to 64-bit.

V4:
- Drop all EL awareness.
V3:
-Tidy up the arm_ccnt_enabled()
-Fixed an old commit message refering to the CCNT_ENABLED
macro
-Do EL change sync in pstate_write instead of in multiple
code paths.
-Addressed PMM V2 review
V2:
-Fix some typos identified by Christopher Covington -Convert the CCNT_ENABLED macro to the arm_ccnt_enabled
function

Alistair Francis (6):
target-arm: Make the ARM PMCCNTR register 64-bit
target-arm: Implement PMCCNTR_EL0 and related registers
target-arm: Add arm_ccnt_enabled function
target-arm: Implement pmccntr_sync function
target-arm: Remove old code and replace with new functions
target-arm: Implement pmccfiltr_write function

Quoted 7 lines
Quoted 2 lines

Peter Crosthwaite (1):
arm: Implement PMCCNTR 32b read-modify-write

Applied to target-arm.next, thanks. (I fixed some minor typos in the doc comment for pmccntr_sync.)

Thanks,

Regards,
Peter

Quoted 2 lines

-- PMM