Hi,
This issue remains on the latest qemu, right?
Let me add some comments for the patch.
On Tue, Nov 27, 2018 at 01:07:16PM -0800, Bijan Mottahedeh wrote:
> Initialize the generic timer scale factor based on the counter frequency
> register cntfrq_el0, and default to the current static value if necessary.
> Always use the default value for TCG.
>
> Signed-off-by: Bijan Mottahedeh <bijan.mottahedeh@xxxxxxxxxx>
> ---
> hw/arm/virt.c | 17 +++++++++++++++++
> target/arm/helper.c | 19 ++++++++++++++++---
> target/arm/internals.h | 8 ++++++--
> target/arm/kvm64.c | 1 +
> 4 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 281ddcd..792d223 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -59,6 +59,7 @@
> #include "qapi/visitor.h"
> #include "standard-headers/linux/input.h"
> #include "hw/arm/smmuv3.h"
> +#include "target/arm/internals.h"
>
> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -1710,6 +1711,21 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
> return NULL;
> }
>
> +static void set_system_clock_scale(void)
> +{
> + unsigned long cntfrq_el0 = 0;
> +
> +#ifdef CONFIG_KVM
> + asm volatile("mrs %0, cntfrq_el0" : "=r"(cntfrq_el0));
> +#endif
> +
> + if (cntfrq_el0 == 0) {
> + cntfrq_el0 = GTIMER_SCALE_DEF;
> + }
> +
> + system_clock_scale = NANOSECONDS_PER_SECOND / (int)cntfrq_el0;
> +}
> +
> static void virt_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1736,6 +1752,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> assert(!mc->get_hotplug_handler);
> mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
> hc->plug = virt_machine_device_plug_cb;
> + set_system_clock_scale();
> }
>
> static const TypeInfo virt_machine_info = {
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 66afb08..6330586 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -18,6 +18,7 @@
> #include "sysemu/kvm.h"
> #include "fpu/softfloat.h"
> #include "qemu/range.h"
> +#include "hw/arm/arm.h"
arm.h is renamed to boot.h, so:
#include "hw/arm/boot.h"
>
> #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
>
> @@ -1614,6 +1615,18 @@ static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri,
> return CP_ACCESS_OK;
> }
>
> +static void gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> + assert(GTIMER_SCALE);
> + assert(ri->fieldoffset);
> +
> + if (cpreg_field_is_64bit(ri)) {
> + CPREG_FIELD64(env, ri) = NANOSECONDS_PER_SECOND / GTIMER_SCALE;
> + } else {
> + CPREG_FIELD32(env, ri) = NANOSECONDS_PER_SECOND / GTIMER_SCALE;
> + }
> +}
> +
> static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx,
> bool isread)
> {
> @@ -1709,7 +1722,7 @@ static CPAccessResult gt_stimer_access(CPUARMState *env,
> }
> }
>
> -static uint64_t gt_get_countervalue(CPUARMState *env)
> +uint64_t gt_get_countervalue(CPUARMState *env)
> {
> return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / GTIMER_SCALE;
> }
> @@ -1996,7 +2009,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 = gt_cntfrq_reset,
Build error is here on the target aarch64-linux-user, like as:
/home/foo/qemu/target/arm/helper.c:2901:18: error: 'gt_cntfrq_reset' undeclared here (not in a function); did you mean 'g_timer_reset'?
.resetfn = gt_cntfrq_reset,
^~~~~~~~~~~~~~~
The issue doesn't affect to aarch64-linux-user, right?
If so:
#ifdef CONFIG_LINUX_USER
.resetvalue = (1000 * 1000 * 1000) / GTIMER_SCALE,
#else
.resetfn = gt_cntfrq_reset,
#endif
> },
> /* overall control: mostly access permissions */
> { .name = "CNTKCTL", .state = ARM_CP_STATE_BOTH,
> @@ -2187,7 +2200,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
> .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 0,
> .type = ARM_CP_CONST, .access = PL0_R /* no PL1_RW in linux-user */,
> .fieldoffset = offsetof(CPUARMState, cp15.c14_cntfrq),
> - .resetvalue = NANOSECONDS_PER_SECOND / GTIMER_SCALE,
> + .resetfn = gt_cntfrq_reset,
Same as above.
> },
> { .name = "CNTVCT_EL0", .state = ARM_CP_STATE_AA64,
> .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 2,
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index dc93577..b66a1fa 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -52,9 +52,13 @@ static inline bool excp_is_internal(int excp)
> }
>
> /* Scale factor for generic timers, ie number of ns per tick.
> - * This gives a 62.5MHz timer.
> + * Calculated dynamically based on CNTFRQ with a default value
> + * that gives a 62.5MHZ timer.
> */
> -#define GTIMER_SCALE 16
> +#define GTIMER_SCALE system_clock_scale
> +#define GTIMER_SCALE_DEF 16
I think extern for system_clock_scale should be added here,
otherwise the build error happens.
If the issue doesn't affect to aarch64-linux-user, how about the following?
#ifndef CONFIG_LINUX_USER
extern int system_clock_scale;
#define GTIMER_SCALE system_clock_scale
#define GTIMER_SCALE_DEF 16
#else
#define GTIMER_SCALE 16
#endif
> +
> +uint64_t gt_get_countervalue(CPUARMState *);
>
> /* Bit definitions for the v7M CONTROL register */
> FIELD(V7M_CONTROL, NPRIV, 0, 1)
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index e0b8246..5d1c394 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -485,6 +485,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> set_feature(&features, ARM_FEATURE_NEON);
> set_feature(&features, ARM_FEATURE_AARCH64);
> set_feature(&features, ARM_FEATURE_PMU);
> + set_feature(&features, ARM_FEATURE_GENERIC_TIMER);
>
> ahcf->features = features;
Thanks,
Masa
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
