[Qemu-devel] [PATCH target-arm v7 1/1] target-arm: Implements the ARM PMCCNTR register

[Qemu-devel] [PATCH target-arm v7 1/1] target-arm: Implements the ARM PMCCNTR register

[Alistair Francis]

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(-)

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,
-- 
1.7.1

Re: [Qemu-devel] [PATCH target-arm v7 1/1] target-arm: Implements the ARM PMCCNTR register

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH target-arm v7 1/1] target-arm: Implements the ARM PMCCNTR register

[Alistair Francis]

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?

Re: [Qemu-devel] [PATCH target-arm v7 1/1] target-arm: Implements the ARM PMCCNTR register

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH target-arm v7 1/1] target-arm: Implements the ARM PMCCNTR register

[Alistair Francis]

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