Skip to content

Commit 1b8705a

Browse files
author
Marc Zyngier
committed
KVM: arm64: timer: Correctly handle EL1 timer emulation when !FEAT_ECV
Both Wei-Lin Chang and Volodymyr Babchuk report that the way we handle the emulation of EL1 timers with NV is completely wrong, specially in the case of HCR_EL2.E2H==0. There are three problems in about as many lines of code: - With E2H==0, the EL1 timers are overwritten with the EL1 state, while they should actually contain the EL2 state (as per the timer map) - With E2H==1, we run the full EL1 timer emulation even when ECV is present, hiding a bug in timer_emulate() (see previous patch) - The comments are actively misleading, and say all the wrong things. This is only attributable to the code having been initially written for FEAT_NV, hacked up to handle FEAT_NV2 *in parallel*, and vaguely hacked again to be FEAT_NV2 only. Oh, and yours truly being a gold plated idiot. The fix is obvious: just delete most of the E2H==0 code, have a unified handling of the timers (because they really are E2H agnostic), and make sure we don't execute any of that when FEAT_ECV is present. Fixes: 4bad306 ("KVM: arm64: nv: Sync nested timer state with FEAT_NV2") Reported-by: Wei-Lin Chang <r09922117@csie.ntu.edu.tw> Reported-by: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> Link: https://lore.kernel.org/r/fqiqfjzwpgbzdtouu2pwqlu7llhnf5lmy4hzv5vo6ph4v3vyls@jdcfy3fjjc5k Link: https://lore.kernel.org/r/87frl51tse.fsf@epam.com Tested-by: Dmytro Terletskyi <dmytro_terletskyi@epam.com> Reviewed-by: Oliver Upton <oliver.upton@linux.dev> Link: https://lore.kernel.org/r/20250204110050.150560-3-maz@kernel.org Signed-off-by: Marc Zyngier <maz@kernel.org>
1 parent b450dcc commit 1b8705a

File tree

1 file changed

+10
-20
lines changed

1 file changed

+10
-20
lines changed

arch/arm64/kvm/arch_timer.c

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -974,31 +974,21 @@ void kvm_timer_sync_nested(struct kvm_vcpu *vcpu)
974974
* which allows trapping of the timer registers even with NV2.
975975
* Still, this is still worse than FEAT_NV on its own. Meh.
976976
*/
977-
if (!vcpu_el2_e2h_is_set(vcpu)) {
978-
if (cpus_have_final_cap(ARM64_HAS_ECV))
979-
return;
980-
981-
/*
982-
* A non-VHE guest hypervisor doesn't have any direct access
983-
* to its timers: the EL2 registers trap (and the HW is
984-
* fully emulated), while the EL0 registers access memory
985-
* despite the access being notionally direct. Boo.
986-
*
987-
* We update the hardware timer registers with the
988-
* latest value written by the guest to the VNCR page
989-
* and let the hardware take care of the rest.
990-
*/
991-
write_sysreg_el0(__vcpu_sys_reg(vcpu, CNTV_CTL_EL0), SYS_CNTV_CTL);
992-
write_sysreg_el0(__vcpu_sys_reg(vcpu, CNTV_CVAL_EL0), SYS_CNTV_CVAL);
993-
write_sysreg_el0(__vcpu_sys_reg(vcpu, CNTP_CTL_EL0), SYS_CNTP_CTL);
994-
write_sysreg_el0(__vcpu_sys_reg(vcpu, CNTP_CVAL_EL0), SYS_CNTP_CVAL);
995-
} else {
977+
if (!cpus_have_final_cap(ARM64_HAS_ECV)) {
996978
/*
997979
* For a VHE guest hypervisor, the EL2 state is directly
998-
* stored in the host EL1 timers, while the emulated EL0
980+
* stored in the host EL1 timers, while the emulated EL1
999981
* state is stored in the VNCR page. The latter could have
1000982
* been updated behind our back, and we must reset the
1001983
* emulation of the timers.
984+
*
985+
* A non-VHE guest hypervisor doesn't have any direct access
986+
* to its timers: the EL2 registers trap despite being
987+
* notionally direct (we use the EL1 HW, as for VHE), while
988+
* the EL1 registers access memory.
989+
*
990+
* In both cases, process the emulated timers on each guest
991+
* exit. Boo.
1002992
*/
1003993
struct timer_map map;
1004994
get_timer_map(vcpu, &map);

0 commit comments

Comments
 (0)