* [PATCH v3 2/4] target/arm: Abstract the generic timer frequency
2019-12-13 5:49 [PATCH v3 0/4] Expose GT CNTFRQ as a CPU property to support AST2600 Andrew Jeffery
2019-12-13 5:49 ` [PATCH v3 1/4] target/arm: Remove redundant scaling of nexttick Andrew Jeffery
@ 2019-12-13 5:49 ` Andrew Jeffery
2019-12-13 5:49 ` [PATCH v3 3/4] target/arm: Prepare generic timer for per-platform CNTFRQ Andrew Jeffery
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Andrew Jeffery @ 2019-12-13 5:49 UTC (permalink / raw)
To: qemu-arm; +Cc: peter.maydell, Richard Henderson, qemu-devel, clg, philmd, joel
Prepare for SoCs such as the ASPEED AST2600 whose firmware configures
CNTFRQ to values significantly larger than the static 62.5MHz value
currently derived from GTIMER_SCALE. As the OS potentially derives its
timer periods from the CNTFRQ value the lack of support for running
QEMUTimers at the appropriate rate leads to sticky behaviour in the
guest.
Substitute the GTIMER_SCALE constant with use of a helper to derive the
period from gt_cntfrq_hz stored in struct ARMCPU. Initially set
gt_cntfrq_hz to the frequency associated with GTIMER_SCALE so current
behaviour is maintained.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v3:
* Uninline gt_cntfrq_period_ns()
* Rename gt_cntfrq to gt_cntfrq_hz
target/arm/cpu.c | 8 ++++++++
target/arm/cpu.h | 5 +++++
target/arm/helper.c | 10 +++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 7a4ac9339bf9..cd0dbe005d9f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -974,6 +974,8 @@ static void arm_cpu_initfn(Object *obj)
if (tcg_enabled()) {
cpu->psci_version = 2; /* TCG implements PSCI 0.2 */
}
+
+ cpu->gt_cntfrq_hz = NANOSECONDS_PER_SECOND / GTIMER_SCALE;
}
static Property arm_cpu_reset_cbar_property =
@@ -1055,6 +1057,12 @@ static void arm_set_init_svtor(Object *obj, Visitor *v, const char *name,
visit_type_uint32(v, name, &cpu->init_svtor, errp);
}
+unsigned int gt_cntfrq_period_ns(ARMCPU *cpu)
+{
+ return NANOSECONDS_PER_SECOND > cpu->gt_cntfrq_hz ?
+ NANOSECONDS_PER_SECOND / cpu->gt_cntfrq_hz : 1;
+}
+
void arm_cpu_post_init(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 83a809d4bac4..ff17ec0df545 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -932,8 +932,13 @@ struct ARMCPU {
*/
DECLARE_BITMAP(sve_vq_map, ARM_MAX_VQ);
DECLARE_BITMAP(sve_vq_init, ARM_MAX_VQ);
+
+ /* Generic timer counter frequency, in Hz */
+ uint64_t gt_cntfrq_hz;
};
+unsigned int gt_cntfrq_period_ns(ARMCPU *cpu);
+
void arm_cpu_post_init(Object *obj);
uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 65c4441a3896..2622a9a8d02f 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2409,7 +2409,9 @@ 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;
+ ARMCPU *cpu = env_archcpu(env);
+
+ return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / gt_cntfrq_period_ns(cpu);
}
static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
@@ -2445,7 +2447,7 @@ 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) {
+ if (nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
timer_mod_ns(cpu->gt_timer[timeridx], INT64_MAX);
} else {
timer_mod(cpu->gt_timer[timeridx], nexttick);
@@ -2874,11 +2876,13 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
static uint64_t gt_virt_cnt_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
+ ARMCPU *cpu = env_archcpu(env);
+
/* Currently we have no support for QEMUTimer in linux-user so we
* can't call gt_get_countervalue(env), instead we directly
* call the lower level functions.
*/
- return cpu_get_clock() / GTIMER_SCALE;
+ return cpu_get_clock() / gt_cntfrq_period_ns(cpu);
}
static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/4] target/arm: Prepare generic timer for per-platform CNTFRQ
2019-12-13 5:49 [PATCH v3 0/4] Expose GT CNTFRQ as a CPU property to support AST2600 Andrew Jeffery
2019-12-13 5:49 ` [PATCH v3 1/4] target/arm: Remove redundant scaling of nexttick Andrew Jeffery
2019-12-13 5:49 ` [PATCH v3 2/4] target/arm: Abstract the generic timer frequency Andrew Jeffery
@ 2019-12-13 5:49 ` Andrew Jeffery
2019-12-13 5:49 ` [PATCH v3 4/4] ast2600: Configure CNTFRQ at 1125MHz Andrew Jeffery
2019-12-17 15:25 ` [PATCH v3 0/4] Expose GT CNTFRQ as a CPU property to support AST2600 Peter Maydell
4 siblings, 0 replies; 7+ messages in thread
From: Andrew Jeffery @ 2019-12-13 5:49 UTC (permalink / raw)
To: qemu-arm; +Cc: peter.maydell, Richard Henderson, qemu-devel, clg, philmd, joel
The ASPEED AST2600 clocks the generic timer at the rate of HPLL. On
recent firmwares this is at 1125MHz, which is considerably quicker than
the assumed 62.5MHz of the current generic timer implementation. The
delta between the value as read from CNTFRQ and the true rate of the
underlying QEMUTimer leads to sticky behaviour in AST2600 guests.
Add a feature-gated property exposing CNTFRQ for ARM CPUs providing the
generic timer. This allows platforms to configure CNTFRQ (and the
associated QEMUTimer) to the appropriate frequency prior to starting the
guest.
As the platform can now determine the rate of CNTFRQ we're exposed to
limitations of QEMUTimer that didn't previously materialise: In the
course of emulation we need to arbitrarily and accurately convert
between guest ticks and time, but we're constrained by QEMUTimer's use
of an integer scaling factor. The effect is QEMUTimer cannot exactly
capture the period of frequencies that do not cleanly divide
NANOSECONDS_PER_SECOND for scaling ticks to time. As such, provide an
equally inaccurate scaling factor for scaling time to ticks so at least
a self-consistent inverse relationship holds.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v3:
* Relocate comment as a consequence of uninlining gt_cntfrq_period_ns() in 2/4.
Philippe - I haven't moved it to the previous patch based on my reasoning on
the list. I'm not sure whether you're satisfied by that, so I haven't added
your Reviewed-by tag.
target/arm/cpu.c | 61 ++++++++++++++++++++++++++++++++++++++--------
target/arm/helper.c | 9 ++++++-
2 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index cd0dbe005d9f..7b21eb544eae 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -974,10 +974,12 @@ static void arm_cpu_initfn(Object *obj)
if (tcg_enabled()) {
cpu->psci_version = 2; /* TCG implements PSCI 0.2 */
}
-
- cpu->gt_cntfrq_hz = NANOSECONDS_PER_SECOND / GTIMER_SCALE;
}
+static Property arm_cpu_gt_cntfrq_property =
+ DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz,
+ NANOSECONDS_PER_SECOND / GTIMER_SCALE);
+
static Property arm_cpu_reset_cbar_property =
DEFINE_PROP_UINT64("reset-cbar", ARMCPU, reset_cbar, 0);
@@ -1059,6 +1061,24 @@ static void arm_set_init_svtor(Object *obj, Visitor *v, const char *name,
unsigned int gt_cntfrq_period_ns(ARMCPU *cpu)
{
+ /*
+ * The exact approach to calculating guest ticks is:
+ *
+ * muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), cpu->gt_cntfrq_hz,
+ * NANOSECONDS_PER_SECOND);
+ *
+ * We don't do that. Rather we intentionally use integer division
+ * truncation below and in the caller for the conversion of host monotonic
+ * time to guest ticks to provide the exact inverse for the semantics of
+ * the QEMUTimer scale factor. QEMUTimer's scale facter is an integer, so
+ * it loses precision when representing frequencies where
+ * `(NANOSECONDS_PER_SECOND % cpu->gt_cntfrq) > 0` holds. Failing to
+ * provide an exact inverse leads to scheduling timers with negative
+ * periods, which in turn leads to sticky behaviour in the guest.
+ *
+ * Finally, CNTFRQ is effectively capped at 1GHz to ensure our scale factor
+ * cannot become zero.
+ */
return NANOSECONDS_PER_SECOND > cpu->gt_cntfrq_hz ?
NANOSECONDS_PER_SECOND / cpu->gt_cntfrq_hz : 1;
}
@@ -1180,6 +1200,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_gt_cntfrq_property,
+ &error_abort);
+ }
}
static void arm_cpu_finalizefn(Object *obj)
@@ -1259,14 +1284,30 @@ 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);
+
+ {
+ uint64_t scale;
+
+ if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) {
+ if (!cpu->gt_cntfrq_hz) {
+ error_setg(errp, "Invalid CNTFRQ: %"PRId64"Hz",
+ cpu->gt_cntfrq_hz);
+ return;
+ }
+ scale = gt_cntfrq_period_ns(cpu);
+ } else {
+ scale = GTIMER_SCALE;
+ }
+
+ 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 2622a9a8d02f..94a05cd978bc 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2683,6 +2683,13 @@ void arm_gt_stimer_cb(void *opaque)
gt_recalc_timer(cpu, GTIMER_SEC);
}
+static void arm_gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque)
+{
+ ARMCPU *cpu = env_archcpu(env);
+
+ cpu->env.cp15.c14_cntfrq = cpu->gt_cntfrq_hz;
+}
+
static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
/* Note that CNTFRQ is purely reads-as-written for the benefit
* of software; writing it doesn't actually change the timer frequency.
@@ -2697,7 +2704,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
.opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 0,
.access = PL1_RW | PL0_R, .accessfn = gt_cntfrq_access,
.fieldoffset = offsetof(CPUARMState, cp15.c14_cntfrq),
- .resetvalue = (1000 * 1000 * 1000) / GTIMER_SCALE,
+ .resetfn = arm_gt_cntfrq_reset,
},
/* overall control: mostly access permissions */
{ .name = "CNTKCTL", .state = ARM_CP_STATE_BOTH,
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread