Re: [PATCH v2 0/2] KVM: arm64: vgic-its: Fix LPI double mapping and DISCARD

[Marc Zyngier <maz@kernel.org>]

On Tue, 13 Oct 2026 13:59:22 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> Hi Marc,
> 
> On Tue, 13 Oct 2026 12:16:09 +0100, Marc Zyngier <maz@kernel.org> wrote:
> [...]
> > > A vCPU on its way into the guest does the same: it loads its LRs in
> > > kvm_vgic_flush_hwstate() while it's still OUTSIDE_GUEST_MODE, so
> > > kvm_arm_halt_guest() doesn't wait for it. With the delay after the
> > > flush instead, on v2 plus your 1/7 and 2/7 [2] and both changes from
> > > your reply to 1/7 [3], the guest takes it in 9 of 20 runs.
> >
> > Does this mean we still need OUTSIDE_GUEST_MODE to be a store-release
> > past the point where we resync the state?
> 
> Not for the 9 of 20: that run already had the release. The halt reads
> the OUTSIDE_GUEST_MODE the vCPU published at its previous exit, so it
> doesn't wait for the early exit that folds the LPI back.
> 
> We do still need it for the window after the exit: with the delay
> there and the same patches applied, the guest takes it in none of 20
> runs, against 13 of 20 on v2.

I don't understand what these probabilities represent. Either it works
or it doesn't. And given that I have no idea what you are testing
with, I can't really make head or tail of the above.

> 
> > > Once DISCARD waits for irq->on_lr to clear before clearing
> > > pending_latch, the guest takes it in none of 20 runs in either case.
> > > I'll add that to v3, unless the single-vcpu stunning already waits for
> > > it.
> >
> > Polling for on_lr feels very dodgy, and I'd like to avoid this. If the
> > OUTSIDE_GUEST release addresses it, I'd rather do that.
> 
> I'd rather avoid the polling too, but the release alone doesn't close
> this one. The vCPU would also need to store IN_GUEST_MODE and check
> its requests before kvm_vgic_flush_hwstate(), so that it never loads
> its LRs while it's OUTSIDE_GUEST_MODE. The existing check after the
> flush would still be needed, for the KVM_REQ_GUEST_HYP_IRQ_PENDING
> request kvm_vgic_flush_hwstate() makes in a nested state. With that
> and the release, the guest takes it in none of 20 runs with the delay
> after the flush, and herd7 finds no LKMM execution that leaves it
> pending.

If you have a herd litmus test, how about posting it? It would really
avoid a lot of that back and forth where the S/N ratio is pretty poor.

> The IN_GUEST_MODE change would also cover vgic_mmio_change_active(),
> which depends on the same halt (the comment above
> vgic_access_active_prepare()). Would you be OK folding it into your
> series, on top of your OUTSIDE_GUEST_MODE changes? Patch 2 then
> wouldn't need the on_lr wait.

Is the hack below what you are asking for? I can't tell. If it isn't,
please post your own version.

Thanks,

	M.

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 0576c2022ef57..f1449b08531eb 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1321,10 +1321,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 
 		local_irq_disable();
 
-		kvm_vgic_flush_hwstate(vcpu);
-
-		kvm_pmu_update_vcpu_events(vcpu);
-
 		/*
 		 * Ensure we set mode to IN_GUEST_MODE after we disable
 		 * interrupts and before the final VCPU requests check.
@@ -1333,14 +1329,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 		 */
 		smp_store_mb(vcpu->mode, IN_GUEST_MODE);
 
+		kvm_vgic_flush_hwstate(vcpu);
+
+		kvm_pmu_update_vcpu_events(vcpu);
+
 		if (ret <= 0 || kvm_vcpu_exit_request(vcpu, &ret)) {
-			vcpu->mode = OUTSIDE_GUEST_MODE;
 			isb(); /* Ensure work in x_flush_hwstate is committed */
 			if (kvm_vcpu_has_pmu(vcpu))
 				kvm_pmu_sync_hwstate(vcpu);
 			if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
 				kvm_timer_sync_user(vcpu);
 			kvm_vgic_sync_hwstate(vcpu);
+			smp_store_release(&vcpu->mode, OUTSIDE_GUEST_MODE);
 			local_irq_enable();
 			preempt_enable();
 			continue;
@@ -1356,7 +1356,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 
 		ret = kvm_arm_vcpu_enter_exit(vcpu);
 
-		vcpu->mode = OUTSIDE_GUEST_MODE;
 		vcpu->stat.exits++;
 		/*
 		 * Back from guest
@@ -1390,6 +1389,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 
 		kvm_arch_vcpu_ctxsync_fp(vcpu);
 
+		/*
+		 * All the state has been synchronised, let advertise
+		 * we're outside of the guest.
+		 */
+		smp_store_release(&vcpu->mode, OUTSIDE_GUEST_MODE);
+
 		/*
 		 * We must ensure that any pending interrupts are taken before
 		 * we exit guest timing so that timer ticks are accounted as

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