[Qemu-devel] [PATCH v3] target-arm: Make the counter tick relative to cntfrq

Andrew Jeffery posted 1 patch 4 years, 7 months ago
Test docker-clang@ubuntu failed
Test FreeBSD passed
Test checkpatch passed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190911034302.29108-1-andrew@aj.id.au
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
target/arm/cpu.c    | 36 ++++++++++++++++++++++++++++--------
target/arm/helper.c | 20 +++++++++++++++++---
2 files changed, 45 insertions(+), 11 deletions(-)
[Qemu-devel] [PATCH v3] target-arm: Make the counter tick relative to cntfrq
Posted by Andrew Jeffery 4 years, 7 months ago
Allow machines to configure CNTFRQ via a property if the ARM core
supports the generic timer. This is necessary on e.g. the ASPEED AST2600
SoC where the generic timer clock is run at 800MHz or above.

CNTFRQ is a read-as-written co-processor register; the property sets the
register's initial value which is used during realize() to configure the
QEMUTimers that back the generic timers. Beyond that the firmware can to
do whatever it sees fit with the CNTFRQ register though changes to the
value will not be reflected in the timers' rate.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
Peter - I think this addresses most of your feedback. I still reach into
the QEMUTimer to fetch out scale when adjusting the nexttick
calculation, but we no longer need to update the scale member and force
a recalculation of the period.
---
 target/arm/cpu.c    | 36 ++++++++++++++++++++++++++++--------
 target/arm/helper.c | 20 +++++++++++++++++---
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2399c144718d..7ac422729608 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -40,6 +40,8 @@
 #include "disas/capstone.h"
 #include "fpu/softfloat.h"
 
+#include <inttypes.h>
+
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -976,6 +978,10 @@ static void arm_cpu_initfn(Object *obj)
     }
 }
 
+static Property arm_cpu_cp_cntfrq_property =
+            DEFINE_PROP_UINT64("cntfrq", ARMCPU, env.cp15.c14_cntfrq,
+                               (1000 * 1000 * 1000) / GTIMER_SCALE);
+
 static Property arm_cpu_reset_cbar_property =
             DEFINE_PROP_UINT64("reset-cbar", ARMCPU, reset_cbar, 0);
 
@@ -1172,6 +1178,11 @@ void arm_cpu_post_init(Object *obj)
 
     qdev_property_add_static(DEVICE(obj), &arm_cpu_cfgend_property,
                              &error_abort);
+
+    if (arm_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER)) {
+        qdev_property_add_static(DEVICE(cpu), &arm_cpu_cp_cntfrq_property,
+                                 &error_abort);
+    }
 }
 
 static void arm_cpu_finalizefn(Object *obj)
@@ -1238,14 +1249,23 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
 
-    cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-                                           arm_gt_ptimer_cb, cpu);
-    cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-                                           arm_gt_vtimer_cb, cpu);
-    cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-                                          arm_gt_htimer_cb, cpu);
-    cpu->gt_timer[GTIMER_SEC] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-                                          arm_gt_stimer_cb, cpu);
+    if (!env->cp15.c14_cntfrq) {
+        error_setg(errp, "Invalid CNTFRQ: %"PRIx64"Hz", env->cp15.c14_cntfrq);
+        return;
+    }
+
+    {
+        uint64_t scale = MAX(1, NANOSECONDS_PER_SECOND / env->cp15.c14_cntfrq);
+
+        cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
+                                               arm_gt_ptimer_cb, cpu);
+        cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
+                                               arm_gt_vtimer_cb, cpu);
+        cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
+                                              arm_gt_htimer_cb, cpu);
+        cpu->gt_timer[GTIMER_SEC] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
+                                              arm_gt_stimer_cb, cpu);
+    }
 #endif
 
     cpu_exec_realizefn(cs, &local_err);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 507026c9154b..8783f92c718c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2409,7 +2409,21 @@ static CPAccessResult gt_stimer_access(CPUARMState *env,
 
 static uint64_t gt_get_countervalue(CPUARMState *env)
 {
-    return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / GTIMER_SCALE;
+    uint64_t effective;
+
+    /*
+     * Deal with quantized clock scaling by calculating the effective frequency
+     * in terms of the timer scale.
+     */
+    if (env->cp15.c14_cntfrq <= NANOSECONDS_PER_SECOND) {
+        uint32_t scale = NANOSECONDS_PER_SECOND / env->cp15.c14_cntfrq;
+        effective = NANOSECONDS_PER_SECOND / scale;
+    } else {
+        effective = NANOSECONDS_PER_SECOND;
+    }
+
+    return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), effective,
+                    NANOSECONDS_PER_SECOND);
 }
 
 static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
@@ -2445,8 +2459,8 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
          * set the timer for as far in the future as possible. When the
          * timer expires we will reset the timer for any remaining period.
          */
-        if (nexttick > INT64_MAX / GTIMER_SCALE) {
-            nexttick = INT64_MAX / GTIMER_SCALE;
+        if (nexttick > INT64_MAX / cpu->gt_timer[timeridx]->scale) {
+            nexttick = INT64_MAX / cpu->gt_timer[timeridx]->scale;
         }
         timer_mod(cpu->gt_timer[timeridx], nexttick);
         trace_arm_gt_recalc(timeridx, irqstate, nexttick);
-- 
2.20.1


Re: [Qemu-devel] [PATCH v3] target-arm: Make the counter tick relative to cntfrq
Posted by no-reply@patchew.org 4 years, 7 months ago
Patchew URL: https://patchew.org/QEMU/20190911034302.29108-1-andrew@aj.id.au/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

libudev           no
default devices   yes

warning: Python 2 support is deprecated
warning: Python 3 will be required for building future versions of QEMU

NOTE: cross-compilers enabled:  'cc'
  GEN     x86_64-softmmu/config-devices.mak.tmp
---
qemu-system-aarch64: Invalid CNTFRQ: 0Hz
Broken pipe
/tmp/qemu-test/src/tests/libqtest.c:135: kill_qemu() tried to terminate QEMU process but encountered exit status 1
ERROR - too few tests run (expected 61, got 0)
make: *** [check-qtest-aarch64] Error 1
make: *** Waiting for unfinished jobs....
Could not access KVM kernel module: No such file or directory


The full log is available at
http://patchew.org/logs/20190911034302.29108-1-andrew@aj.id.au/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel][PATCH v3] target-arm: Make the counter tick relative to cntfrq
Posted by Andrew Jeffery 4 years, 7 months ago

On Wed, 11 Sep 2019, at 15:40, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190911034302.29108-1-andrew@aj.id.au/
> 
> 
> 
> Hi,
> 
> This series failed the docker-quick@centos7 build test. Please find the 
> testing commands and
> their output below. If you have Docker installed, you can probably 
> reproduce it
> locally.

Disregard v3, I got distracted by some updates to the way ASPEED's u-boot configures
CNTFRQ and didn't test some relevant cases.

Sorry for the noise.

Andrew