diff mbox series

[15/20] target/arm/hvf: Really set Generic Timer counter frequency

Message ID 20250619131319.47301-16-philmd@linaro.org
State New
Headers show
Series arm: Fixes and preparatory cleanups for split-accel | expand

Commit Message

Philippe Mathieu-Daudé June 19, 2025, 1:13 p.m. UTC 
Setting ARMCPU::gt_cntfrq_hz in hvf_arch_init_vcpu() is
not correct because the timers have already be initialized
with the default frequency.

Set it earlier in the AccelOpsClass::cpu_target_realize()
handler instead, and assert the value is correct when
reaching hvf_arch_init_vcpu().

Fixes: a1477da3dde ("hvf: Add Apple Silicon support")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/hvf/hvf.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Richard Henderson June 19, 2025, 9:21 p.m. UTC | #1 
On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Setting ARMCPU::gt_cntfrq_hz in hvf_arch_init_vcpu() is
> not correct because the timers have already be initialized
> with the default frequency.
> 
> Set it earlier in the AccelOpsClass::cpu_target_realize()
> handler instead, and assert the value is correct when
> reaching hvf_arch_init_vcpu().
> 
> Fixes: a1477da3dde ("hvf: Add Apple Silicon support")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/hvf/hvf.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 632751d1eab..fce02d22ef9 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -1008,12 +1008,16 @@ int hvf_arch_init_vcpu(CPUState *cpu)
>       CPUARMState *env = &arm_cpu->env;
>       uint32_t sregs_match_len = ARRAY_SIZE(hvf_sreg_match);
>       uint32_t sregs_cnt = 0;
> +    uint64_t freq_hz = 0;
>       uint64_t pfr;
>       hv_return_t ret;
>       int i;
>   
>       env->aarch64 = true;
> -    asm volatile("mrs %0, cntfrq_el0" : "=r"(arm_cpu->gt_cntfrq_hz));
> +
> +    /* system count frequency sanity check */
> +    asm volatile("mrs %0, cntfrq_el0" : "=r"(freq_hz));
> +    assert(arm_cpu->gt_cntfrq_hz == freq_hz);
>   
>       /* Allocate enough space for our sysreg sync */
>       arm_cpu->cpreg_indexes = g_renew(uint64_t, arm_cpu->cpreg_indexes,
> @@ -1080,6 +1084,10 @@ int hvf_arch_init_vcpu(CPUState *cpu)
>   
>   bool hvf_arch_cpu_realize(CPUState *cs, Error **errp)
>   {
> +    ARMCPU *cpu = ARM_CPU(cs);
> +
> +    asm volatile("mrs %0, cntfrq_el0" : "=r"(cpu->gt_cntfrq_hz));

Worth turning the inline asm into an inline function to avoid replication?

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> +
>       return true;
>   }
>
diff mbox series

Patch

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 632751d1eab..fce02d22ef9 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1008,12 +1008,16 @@  int hvf_arch_init_vcpu(CPUState *cpu)
     CPUARMState *env = &arm_cpu->env;
     uint32_t sregs_match_len = ARRAY_SIZE(hvf_sreg_match);
     uint32_t sregs_cnt = 0;
+    uint64_t freq_hz = 0;
     uint64_t pfr;
     hv_return_t ret;
     int i;
 
     env->aarch64 = true;
-    asm volatile("mrs %0, cntfrq_el0" : "=r"(arm_cpu->gt_cntfrq_hz));
+
+    /* system count frequency sanity check */
+    asm volatile("mrs %0, cntfrq_el0" : "=r"(freq_hz));
+    assert(arm_cpu->gt_cntfrq_hz == freq_hz);
 
     /* Allocate enough space for our sysreg sync */
     arm_cpu->cpreg_indexes = g_renew(uint64_t, arm_cpu->cpreg_indexes,
@@ -1080,6 +1084,10 @@  int hvf_arch_init_vcpu(CPUState *cpu)
 
 bool hvf_arch_cpu_realize(CPUState *cs, Error **errp)
 {
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    asm volatile("mrs %0, cntfrq_el0" : "=r"(cpu->gt_cntfrq_hz));
+
     return true;
 }