diff mbox

[target-arm,v7,1/1] target-arm: Implements the ARM PMCCNTR register

Message ID 57a391d51a6430e74fad5b88316c504b9217c306.1392853953.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis Feb. 19, 2014, 11:57 p.m. UTC
This patch implements the ARM PMCCNTR register including
the disable and reset components of the PMCR register.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
V7: Fixed a bug that caused the cycle count scaling to be determined
by the PMCRDP register instead of PMCRD. Also stopped PMCRDP from
disabling the counter. Thanks to Peter Maydell
V6: Rebase to include Peter Maydell's 'Convert performance monitor
reginfo to accesfn' patch. Remove the raw_fn's as the read/write
functions already do what is required.
V5: Implement the actual write function to make sure that
migration works correctly. Also includes the raw_read/write as
the normal read/write functions depend on the pmcr register. So
they don't allow for the pmccntr register to be written first.
V4: Some bug fixes pointed out by Peter Crosthwaite. Including
increasing the accuracy of the timer.
V3: Fixed up incorrect reset, disable and enable handling that
was submitted in V2. The patch should now also handle changing
of the clock scaling.
V2: Incorporated the comments that Peter Maydell and Peter
Crosthwaite had. Now the implementation only requires one
CPU state

 target-arm/cpu.h    |    4 ++
 target-arm/helper.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 2 deletions(-)

Comments

Peter Maydell Feb. 20, 2014, 3:44 p.m. UTC | #1
On 19 February 2014 23:57, Alistair Francis <alistair.francis@xilinx.com> wrote:
> This patch implements the ARM PMCCNTR register including
> the disable and reset components of the PMCR register.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>

This breaks compilation of the arm-linux-user target
(the qemu_clock_get_* functions are only available in
system mode). You probably want to do something similar
to how we handle the generic timers, where we #ifdef
everything out. (I don't think Linux gives user
space access to the perf monitor registers.)

Other than that I think it looks good now.

thanks
-- PMM
Alistair Francis Feb. 20, 2014, 11:58 p.m. UTC | #2
On Fri, Feb 21, 2014 at 1:44 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 19 February 2014 23:57, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> This patch implements the ARM PMCCNTR register including
>> the disable and reset components of the PMCR register.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>
> This breaks compilation of the arm-linux-user target
> (the qemu_clock_get_* functions are only available in
> system mode). You probably want to do something similar
> to how we handle the generic timers, where we #ifdef
> everything out. (I don't think Linux gives user
> space access to the perf monitor registers.)
>
> Other than that I think it looks good now.
>
> thanks
> -- PMM
>

What do you think the functions should do when in user space? Should
they just return 0?
Peter Maydell Feb. 21, 2014, 12:07 a.m. UTC | #3
On 20 February 2014 23:58, Alistair Francis <alistair.francis@xilinx.com> wrote:
> On Fri, Feb 21, 2014 at 1:44 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This breaks compilation of the arm-linux-user target
>> (the qemu_clock_get_* functions are only available in
>> system mode). You probably want to do something similar
>> to how we handle the generic timers, where we #ifdef
>> everything out. (I don't think Linux gives user
>> space access to the perf monitor registers.)

> What do you think the functions should do when in user space? Should
> they just return 0?

Just don't define reginfo entries for them at all -- then we will
UNDEF because the lookup will return no result. This is what
the generic timer code does.

thanks
-- PMM
Alistair Francis Feb. 21, 2014, 12:52 a.m. UTC | #4
On Fri, Feb 21, 2014 at 10:07 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 20 February 2014 23:58, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> On Fri, Feb 21, 2014 at 1:44 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> This breaks compilation of the arm-linux-user target
>>> (the qemu_clock_get_* functions are only available in
>>> system mode). You probably want to do something similar
>>> to how we handle the generic timers, where we #ifdef
>>> everything out. (I don't think Linux gives user
>>> space access to the perf monitor registers.)
>
>> What do you think the functions should do when in user space? Should
>> they just return 0?
>
> Just don't define reginfo entries for them at all -- then we will
> UNDEF because the lookup will return no result. This is what
> the generic timer code does.
>
> thanks
> -- PMM
>

Ok, so if it is user mode, it just treats the pmccntr register exactly
as it does before the patch. I've implemented that and it seems to
work. I'll submit the patch now
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 3c8a2db..14fd1ae 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -215,6 +215,10 @@  typedef struct CPUARMState {
         uint32_t c15_diagnostic; /* diagnostic register */
         uint32_t c15_power_diagnostic;
         uint32_t c15_power_control; /* power control */
+        /* If the counter is enabled, this stores the last time the counter
+         * was reset. Otherwise it stores the counter value
+         */
+        uint32_t c15_ccnt;
     } cp15;
 
     struct {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index b547f04..a5f4770 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -13,6 +13,11 @@  static inline int get_phys_addr(CPUARMState *env, uint32_t address,
                                 target_ulong *page_size);
 #endif
 
+/* Definitions for the PMCCNTR and PMCR registers */
+#define PMCRD   0x8
+#define PMCRC   0x4
+#define PMCRE   0x1
+
 static int vfp_gdb_get_reg(CPUARMState *env, uint8_t *buf, int reg)
 {
     int nregs;
@@ -478,9 +483,37 @@  static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri)
 static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                        uint64_t value)
 {
+    uint32_t temp_ticks;
+
+    temp_ticks = 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;
+        }
+    }
+
+    if (value & PMCRC) {
+        /* The counter has been reset */
+        env->cp15.c15_ccnt = 0;
+    }
+
     /* only the DP, X, D and E bits are writable */
     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;
+    }
 }
 
 static void pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -536,6 +569,46 @@  static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
     env->cp15.c12_vbar = value & ~0x1Ful;
 }
 
+static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint32_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;
+
+    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 pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                        uint64_t value)
+{
+    uint32_t total_ticks;
+
+    if (!(env->cp15.c9_pmcr & PMCRE)) {
+        /* Counter is disabled, set the absolute value */
+        env->cp15.c15_ccnt = value;
+        return;
+    }
+
+    total_ticks = 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 */
+        total_ticks /= 64;
+    }
+    env->cp15.c15_ccnt = total_ticks - value;
+}
+
 static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
@@ -595,9 +668,9 @@  static const ARMCPRegInfo v7_cp_reginfo[] = {
     { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
       .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
       .accessfn = pmreg_access },
-    /* Unimplemented, RAZ/WI. */
     { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
-      .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0,
+      .access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
+      .readfn = pmccntr_read, .writefn = pmccntr_write,
       .accessfn = pmreg_access },
     { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1,
       .access = PL0_RW,