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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8634iwubgw.wl-maz@kernel.org>
Date: Mon, 09 Dec 2024 15:23:27 +0000
From: Marc Zyngier <maz@...nel.org>
To: Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
Cc: kvmarm <kvmarm@...ts.linux.dev>,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	oliver.upton@...ux.dev,
	christoffer.dall@....com,
	suzuki.poulose@....com,
	will@...nel.org,
	catalin.marinas@....com,
	coltonlewis@...gle.com,
	joey.gouly@....com,
	yuzenghui@...wei.com,
	darren@...amperecomputing.com,
	vishnu@...amperecomputing.com
Subject: Re: [PATCH] KVM: arm64: nv: Set ISTATUS for emulated timers, If timer expired

On Mon, 09 Dec 2024 13:20:48 +0000,
Marc Zyngier <maz@...nel.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.