Re: [PATCH] KVM: arm64: nv: Set ISTATUS for emulated timers, If timer expired

[Marc Zyngier <maz@kernel.org>]

On Mon, 09 Dec 2024 13:20:48 +0000,
Marc Zyngier <maz@kernel.org> wrote:
> 
> So here's my current guess, since you don't give me the needed
> information. For what you describe to happen, I can only see two
> possibilities:
> 
> - either your HW doesn't have FEAT_ECV, in which case the guest
>   directly reads from memory
> 
> - or you are running with something like this patch [1], and we serve
>   the guest by reading from memory very early, without returning to
>   the bulk of the emulation code
> 
> In either case, we only publish the updated status if the current IRQ
> state is different from the computed output of the timer while
> performing the emulation.
> 
> So if you were writing back a status bit set to 0 while the interrupt
> was already pending, we'd deliver an interrupt, but not recompute the
> status. The guest would consider the interrupt as spurious, not touch
> the timer, and we'd never make forward progress. Rinse, repeat.
> 
> Assuming I got the analysis right, it would only be a matter of
> hoisting the publication of the status into timer_emulate(), so that
> it is made up to date on load.
> 
> Please give the fixup below a go.

Plus this on top for a good measure:

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index 91bda986c344b..c71193a7bb9c5 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -968,9 +968,6 @@ void kvm_timer_sync_nested(struct kvm_vcpu *vcpu)
 	 * which allows trapping of the timer registers even with NV2.
 	 * Still, this is still worse than FEAT_NV on its own. Meh.
 	 */
-	if (cpus_have_final_cap(ARM64_HAS_ECV) || !is_hyp_ctxt(vcpu))
-		return;
-
 	if (!vcpu_el2_e2h_is_set(vcpu)) {
 		/*
 		 * A non-VHE guest hypervisor doesn't have any direct access
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index ff62b8b55b46e..1b8bb30dbb2ff 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1257,7 +1257,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 		if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
 			kvm_timer_sync_user(vcpu);
 
-		if (vcpu_has_nv(vcpu))
+		if (is_hyp_ctxt(vcpu))
 			kvm_timer_sync_nested(vcpu);
 
 		kvm_arch_vcpu_ctxsync_fp(vcpu);


	M.

-- 
Without deviation from the norm, progress is not possible.