[PATCH 0/3] KVM: arm64: vgic-v3: Make LPI disabling robust

[PATCH 0/3] KVM: arm64: vgic-v3: Make LPI disabling robust

[Marc Zyngier]

Yuchao Zhang reported that disabling LPIs on one CPU from another
could result in UAFs and other horrors.

There are two reasons for this:

- the last_lr_irq pointer does not contribute to LPI refcount, and
  that LPI being removed results in a dangling pointer

- LPIs can be deleted from a remote vcpu by disabling them while that
  vcpu is actually running, and has LPIs in its LRs.

Address the two issues in one go, by actively taking a refcount on all
IRQs referenced by last_lr_irq, and making sure that disabling LPIs
force all vcpus to be paused, making it safe.

Marc Zyngier (3):
  KVM: arm64: vgic: Allow last_lr_irq to be NULL when LRs are not
    overflowing
  KVM: arm64: vgic: Take a refcount on IRQs referenced by last_lr_irq
  KVM: arm64: vgic: Stop the VM when disabling LPIs

 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 10 ++++++++++
 arch/arm64/kvm/vgic/vgic-v2.c      |  6 ++++--
 arch/arm64/kvm/vgic/vgic-v3.c      |  6 ++++--
 arch/arm64/kvm/vgic/vgic.c         | 21 ++++++++++++++++-----
 4 files changed, 34 insertions(+), 9 deletions(-)

-- 
2.47.3

[PATCH 1/3] KVM: arm64: vgic: Allow last_lr_irq to be NULL when LRs are not overflowing

[Marc Zyngier]

last_lr_irq is always populated when there is any interrupt populated in
the AP list. Not only this is not necessary (it is only useful when we
completely fill the LRs), but this is in the way of further fixes.

Make sure last_lr_irq is kept to NULL when we LRs are not completely
full.

Fixes: 6da5e537f5afe ("KVM: arm64: vgic: Pick EOIcount deactivations from AP-list tail")
Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/vgic/vgic-v2.c | 6 ++++--
 arch/arm64/kvm/vgic/vgic-v3.c | 6 ++++--
 arch/arm64/kvm/vgic/vgic.c    | 8 +++-----
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index 7182f63fc9382..70cc53ba810ad 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -122,6 +122,10 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 	for (int lr = 0; lr < vgic_cpu->vgic_v2.used_lrs; lr++)
 		vgic_v2_fold_lr(vcpu, cpuif->vgic_lr[lr]);
 
+	cpuif->used_lrs = 0;
+	if (!irq)
+		return;
+
 	/* See the GICv3 equivalent for the EOIcount handling rationale */
 	list_for_each_entry_continue(irq, &vgic_cpu->ap_list_head, ap_list) {
 		u32 lr;
@@ -144,8 +148,6 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 		vgic_v2_fold_lr(vcpu, lr);
 		eoicount--;
 	}
-
-	cpuif->used_lrs = 0;
 }
 
 void vgic_v2_deactivate(struct kvm_vcpu *vcpu, u32 val)
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 726e20a1da6e7..346bacb3198f2 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -155,6 +155,10 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 	for (int lr = 0; lr < cpuif->used_lrs; lr++)
 		vgic_v3_fold_lr(vcpu, cpuif->vgic_lr[lr]);
 
+	cpuif->used_lrs = 0;
+	if (!irq)
+		return;
+
 	/*
 	 * EOIMode=0: use EOIcount to emulate deactivation. We are
 	 * guaranteed to deactivate in reverse order of the activation, so
@@ -188,8 +192,6 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 		vgic_v3_fold_lr(vcpu, lr);
 		eoicount--;
 	}
-
-	cpuif->used_lrs = 0;
 }
 
 void vgic_v3_deactivate(struct kvm_vcpu *vcpu, u64 val)
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index b25303d9919fd..425503e0c5825 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -866,9 +866,6 @@ static void vgic_fold_state(struct kvm_vcpu *vcpu)
 		return;
 	}
 
-	if (!*host_data_ptr(last_lr_irq))
-		return;
-
 	if (kvm_vgic_global_state.type == VGIC_V2)
 		vgic_v2_fold_lr_state(vcpu);
 	else
@@ -1021,11 +1018,12 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 		scoped_guard(raw_spinlock,  &irq->irq_lock) {
 			if (likely(vgic_target_oracle(irq) == vcpu)) {
 				vgic_populate_lr(vcpu, irq, count++);
-				*host_data_ptr(last_lr_irq) = irq;
+				if (count == kvm_vgic_global_state.nr_lr)
+					*host_data_ptr(last_lr_irq) = irq;
 			}
 		}
 
-		if (count == kvm_vgic_global_state.nr_lr)
+		if (*host_data_ptr(last_lr_irq))
 			break;
 	}
 
-- 
2.47.3

[PATCH 2/3] KVM: arm64: vgic: Take a refcount on IRQs referenced by last_lr_irq

[Marc Zyngier]

Referencing the last interrupt inserted in an LR is rather fragile, as
this interrupt can vanish if a concurrently unmapped LPI.

Solve this by bumping up the refcount on the interrupt when populating
last_lr_irq, and drop it at vgic_prune_ap_list() time, when LPIs are
being reclaimed.

Fixes: 6da5e537f5afe ("KVM: arm64: vgic: Pick EOIcount deactivations from AP-list tail")
Reported-by: Yuchao Zhang <ndaugoing@gmail.com>
Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/vgic/vgic.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 425503e0c5825..5cf5a1ef86cdd 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -853,6 +853,17 @@ static void vgic_prune_ap_list(struct kvm_vcpu *vcpu)
 		goto retry;
 	}
 
+	/*
+	 * Fix the last_lr_irq refcount which was obtained while
+	 * populating the LRs. This can also result in the LPI being
+	 * deleted.
+	 */
+	irq = *host_data_ptr(last_lr_irq);
+	if (irq) {
+		deleted_lpis |= vgic_put_irq_norelease(vcpu->kvm, irq);
+		*host_data_ptr(last_lr_irq) = NULL;
+	}
+
 	raw_spin_unlock(&vgic_cpu->ap_list_lock);
 
 	if (unlikely(deleted_lpis))
@@ -1018,8 +1029,10 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 		scoped_guard(raw_spinlock,  &irq->irq_lock) {
 			if (likely(vgic_target_oracle(irq) == vcpu)) {
 				vgic_populate_lr(vcpu, irq, count++);
-				if (count == kvm_vgic_global_state.nr_lr)
+				if (count == kvm_vgic_global_state.nr_lr) {
+					vgic_get_irq_ref(irq);
 					*host_data_ptr(last_lr_irq) = irq;
+				}
 			}
 		}
 
-- 
2.47.3

[PATCH 3/3] KVM: arm64: vgic: Stop the VM when disabling LPIs

[Marc Zyngier]

Disabling LPIs is pretty nasty, as it directly messes with the AP list
of the affected CPU, which could be running... This has the potential to
lead to really bad behaviours, and we should not be doing that.

Take example on the way the Active state is handled and simply pause all
the vcpus so that we are sure they are all in a quiescent state, and the
state be safely manipulated.

Nobody has any expectation of performance for this anyway.

Fixes: 96085b949672d ("KVM: arm/arm64: vgic-v3: Retire pending interrupts on disabling LPIs")
Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 5913a20d83019..adefc42964732 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -303,9 +303,19 @@ static void vgic_mmio_write_v3r_ctlr(struct kvm_vcpu *vcpu,
 		if (ctlr != GICR_CTLR_ENABLE_LPIS)
 			return;
 
+		/*
+		 * Yes, disabling LPIs is painful, since it can be done from
+		 * a *remote* vcpu! So let's not take any chance, and make
+		 * sure that everybody has written their LRs back to the irq
+		 * structures, and release any reference they would have.
+		 *
+		 * If it hurts, don't do it.
+		 */
+		kvm_arm_halt_guest(vcpu->kvm);
 		vgic_flush_pending_lpis(vcpu);
 		vgic_its_invalidate_all_caches(vcpu->kvm);
 		atomic_set_release(&vgic_cpu->ctlr, 0);
+		kvm_arm_resume_guest(vcpu->kvm);
 	} else {
 		ctlr = atomic_cmpxchg_acquire(&vgic_cpu->ctlr, 0,
 					      GICR_CTLR_ENABLE_LPIS);
-- 
2.47.3

Re: [PATCH 3/3] KVM: arm64: vgic: Stop the VM when disabling LPIs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: arm64: vgic: race in vcpu->arch.pause unpause due to missing VM lock
- [High] kvm: arm64: vgic: stale LR state during vgic_flush_pending_lpis()
--

commit 81f0f5ee375a1e011fabe86a70b81c8175c5fd88
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: vgic: Stop the VM when disabling LPIs

Disabling LPIs modifies the AP list of the affected CPU, which could be
running. This commit aims to safely manipulate the state by pausing all
vCPUs so they are in a quiescent state when disabling LPIs.

> diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> index 5913a20d83019..adefc42964732 100644
> --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> @@ -303,9 +303,19 @@ static void vgic_mmio_write_v3r_ctlr(struct kvm_vcpu *vcpu,
>  		if (ctlr != GICR_CTLR_ENABLE_LPIS)
>  			return;
>  
> +		/*
> +		 * Yes, disabling LPIs is painful, since it can be done from
> +		 * a *remote* vcpu! So let's not take any chance, and make
> +		 * sure that everybody has written their LRs back to the irq
> +		 * structures, and release any reference they would have.
> +		 *
> +		 * If it hurts, don't do it.
> +		 */
> +		kvm_arm_halt_guest(vcpu->kvm);
>  		vgic_flush_pending_lpis(vcpu);

[Severity: High]
Does the asynchronous halt ensure the target vCPU has fully synchronized its
hardware state back to software state before proceeding to flush?

kvm_arm_halt_guest() uses kvm_make_all_cpus_request(kvm, KVM_REQ_SLEEP).
If the target vCPU has exited guest mode and set vcpu->mode = OUTSIDE_GUEST_MODE
but has not yet executed kvm_vgic_sync_hwstate() to fold its LRs,
kvm_make_all_cpus_request() will skip waiting for it:

virt/kvm/kvm_main.c:kvm_request_needs_ipi() {
    ...
	if (req & KVM_REQUEST_WAIT)
		return mode != OUTSIDE_GUEST_MODE;
    ...
}

If the wait is skipped, could the flushing vCPU clear the ap_list in
vgic_flush_pending_lpis() just before the target vCPU folds its LRs? 
Would this incorrectly resurrect the deleted LPI state and cause ghost
interrupts?

>  		vgic_its_invalidate_all_caches(vcpu->kvm);
>  		atomic_set_release(&vgic_cpu->ctlr, 0);
> +		kvm_arm_resume_guest(vcpu->kvm);

[Severity: High]
Does this block need to hold a VM-wide lock?

Since vgic_mmio_write_v3r_ctlr() only synchronizes locally using an atomic
cmpxchg on the target vgic_cpu->ctlr, multiple vCPUs could concurrently
write to different VGIC redistributors to disable their LPIs.

If this happens, multiple vCPUs will concurrently call kvm_arm_halt_guest()
and race to set vcpu->arch.pause = true:

arch/arm64/kvm/arm.c:kvm_arm_halt_guest() {
    ...
	kvm_for_each_vcpu(i, vcpu, kvm)
		vcpu->arch.pause = true;
    ...
}

The first vCPU to finish flushing will call kvm_arm_resume_guest() which
unconditionally clears the pause boolean:

arch/arm64/kvm/arm.c:kvm_arm_resume_guest() {
    ...
	kvm_for_each_vcpu(i, vcpu, kvm) {
		vcpu->arch.pause = false;
		__kvm_vcpu_wake_up(vcpu);
	}
    ...
}

Could this unpause the VM prematurely while another vCPU is still tearing
down VGIC state in vgic_flush_pending_lpis(), allowing guests to run and
modify their VGIC state concurrently?

>  	} else {
>  		ctlr = atomic_cmpxchg_acquire(&vgic_cpu->ctlr, 0,
>  					      GICR_CTLR_ENABLE_LPIS);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922214212.3327146-1-maz@kernel.org?part=3

Re: [PATCH 3/3] KVM: arm64: vgic: Stop the VM when disabling LPIs

[Marc Zyngier]

On Tue, 22 Sep 2026 23:01:25 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] kvm: arm64: vgic: race in vcpu->arch.pause unpause due to missing VM lock
> - [High] kvm: arm64: vgic: stale LR state during vgic_flush_pending_lpis()
> --
> 
> commit 81f0f5ee375a1e011fabe86a70b81c8175c5fd88
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: vgic: Stop the VM when disabling LPIs
> 
> Disabling LPIs modifies the AP list of the affected CPU, which could be
> running. This commit aims to safely manipulate the state by pausing all
> vCPUs so they are in a quiescent state when disabling LPIs.
> 
> > diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > index 5913a20d83019..adefc42964732 100644
> > --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > @@ -303,9 +303,19 @@ static void vgic_mmio_write_v3r_ctlr(struct kvm_vcpu *vcpu,
> >  		if (ctlr != GICR_CTLR_ENABLE_LPIS)
> >  			return;
> >  
> > +		/*
> > +		 * Yes, disabling LPIs is painful, since it can be done from
> > +		 * a *remote* vcpu! So let's not take any chance, and make
> > +		 * sure that everybody has written their LRs back to the irq
> > +		 * structures, and release any reference they would have.
> > +		 *
> > +		 * If it hurts, don't do it.
> > +		 */
> > +		kvm_arm_halt_guest(vcpu->kvm);
> >  		vgic_flush_pending_lpis(vcpu);
> 
> [Severity: High]
> Does the asynchronous halt ensure the target vCPU has fully synchronized its
> hardware state back to software state before proceeding to flush?
> 
> kvm_arm_halt_guest() uses kvm_make_all_cpus_request(kvm, KVM_REQ_SLEEP).
> If the target vCPU has exited guest mode and set vcpu->mode = OUTSIDE_GUEST_MODE
> but has not yet executed kvm_vgic_sync_hwstate() to fold its LRs,
> kvm_make_all_cpus_request() will skip waiting for it:
> 
> virt/kvm/kvm_main.c:kvm_request_needs_ipi() {
>     ...
> 	if (req & KVM_REQUEST_WAIT)
> 		return mode != OUTSIDE_GUEST_MODE;
>     ...
> }
> 
> If the wait is skipped, could the flushing vCPU clear the ap_list in
> vgic_flush_pending_lpis() just before the target vCPU folds its LRs? 
> Would this incorrectly resurrect the deleted LPI state and cause ghost
> interrupts?

I think there is a kernel of truth here, and I reckon this is a *very*
old bug. Something like the below should help. Oliver what do you
think?

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8b080804bc90b..5f5dd8bead4b9 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1352,7 +1352,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
@@ -1386,6 +1385,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


> 
> >  		vgic_its_invalidate_all_caches(vcpu->kvm);
> >  		atomic_set_release(&vgic_cpu->ctlr, 0);
> > +		kvm_arm_resume_guest(vcpu->kvm);
> 
> [Severity: High]
> Does this block need to hold a VM-wide lock?
> 
> Since vgic_mmio_write_v3r_ctlr() only synchronizes locally using an atomic
> cmpxchg on the target vgic_cpu->ctlr, multiple vCPUs could concurrently
> write to different VGIC redistributors to disable their LPIs.
> 
> If this happens, multiple vCPUs will concurrently call kvm_arm_halt_guest()
> and race to set vcpu->arch.pause = true:
> 
> arch/arm64/kvm/arm.c:kvm_arm_halt_guest() {
>     ...
> 	kvm_for_each_vcpu(i, vcpu, kvm)
> 		vcpu->arch.pause = true;
>     ...
> }
> 
> The first vCPU to finish flushing will call kvm_arm_resume_guest() which
> unconditionally clears the pause boolean:
> 
> arch/arm64/kvm/arm.c:kvm_arm_resume_guest() {
>     ...
> 	kvm_for_each_vcpu(i, vcpu, kvm) {
> 		vcpu->arch.pause = false;
> 		__kvm_vcpu_wake_up(vcpu);
> 	}
>     ...
> }
> 
> Could this unpause the VM prematurely while another vCPU is still tearing
> down VGIC state in vgic_flush_pending_lpis(), allowing guests to run and
> modify their VGIC state concurrently?

Just as interesting. It appears that this doesn't nest correctly,
while we really rely on it to work. An obvious way to do this is to
actually turn pause into a refcount. Something like the (untested)
hack below.

	M.

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 27fe0cd5b2d7a..08dc8f816824c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -889,11 +889,10 @@ struct kvm_vcpu_arch {
 	/*
 	 * Don't run the guest (internal implementation need).
 	 *
-	 * Contrary to the flags above, this is set/cleared outside of
-	 * a vcpu context, and thus cannot be mixed with the flags
-	 * themselves (or the flag accesses need to be made atomic).
+	 * Contrary to the flags above, this is updated outside of
+	 * a vcpu context, and thus cannot be mixed with the flags.
 	 */
-	bool pause;
+	refcount_t pause;
 
 	/*
 	 * We maintain more than a single set of debug registers to support
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8b080804bc90b..62b683107ccae 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -561,6 +561,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 	kvm_arm_pvtime_vcpu_init(&vcpu->arch);
 
 	vcpu->arch.hw_mmu = &vcpu->kvm->arch.mmu;
+	refcount_set(&vcpu->arch.pause, 0);
 
 	/*
 	 * This vCPU may have been created after mpidr_data was initialized.
@@ -835,6 +836,11 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 	return ret;
 }
 
+static bool vcpu_can_run(struct kvm_vcpu *vcpu)
+{
+	return !kvm_arm_vcpu_stopped(vcpu) && !refcount_read(&vcpu->arch.pause);
+}
+
 /**
  * kvm_arch_vcpu_runnable - determine if the vcpu can be scheduled
  * @v:		The VCPU pointer
@@ -850,8 +856,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
 		      (kvm_timer_should_notify_user(v) ||
 		       kvm_pmu_should_notify_user(v)));
 
-	return ((irq_lines || kvm_vgic_vcpu_pending_irq(v))
-		&& !kvm_arm_vcpu_stopped(v) && !v->arch.pause);
+	return ((irq_lines || kvm_vgic_vcpu_pending_irq(v)) && !vcpu_can_run(v));
 }
 
 bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
@@ -1014,7 +1019,7 @@ void kvm_arm_halt_guest(struct kvm *kvm)
 	struct kvm_vcpu *vcpu;
 
 	kvm_for_each_vcpu(i, vcpu, kvm)
-		vcpu->arch.pause = true;
+		refcount_inc(&vcpu->arch.pause);
 	kvm_make_all_cpus_request(kvm, KVM_REQ_SLEEP);
 }
 
@@ -1024,8 +1029,8 @@ void kvm_arm_resume_guest(struct kvm *kvm)
 	struct kvm_vcpu *vcpu;
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
-		vcpu->arch.pause = false;
-		__kvm_vcpu_wake_up(vcpu);
+		if (!refcount_dec_and_test(&vcpu->arch.pause) == 0)
+			__kvm_vcpu_wake_up(vcpu);
 	}
 }
 
@@ -1034,10 +1039,10 @@ static void kvm_vcpu_sleep(struct kvm_vcpu *vcpu)
 	struct rcuwait *wait = kvm_arch_vcpu_get_wait(vcpu);
 
 	rcuwait_wait_event(wait,
-			   (!kvm_arm_vcpu_stopped(vcpu)) && (!vcpu->arch.pause),
+			   vcpu_can_run(vcpu),
 			   TASK_INTERRUPTIBLE);
 
-	if (kvm_arm_vcpu_stopped(vcpu) || vcpu->arch.pause) {
+	if (!vcpu_can_run(vcpu)) {
 		/* Awaken to handle a signal, request we sleep again later. */
 		kvm_make_request(KVM_REQ_SLEEP, vcpu);
 	}

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

Re: [PATCH 3/3] KVM: arm64: vgic: Stop the VM when disabling LPIs

[Oliver Upton]

Hey,

On Wed, Sep 23, 2026 at 12:21:15AM +0100, Marc Zyngier wrote:
> I think there is a kernel of truth here, and I reckon this is a *very*
> old bug. Something like the below should help. Oliver what do you
> think?
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90b..5f5dd8bead4b9 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1352,7 +1352,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
> @@ -1386,6 +1385,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
> 

Yeah, this looks reasonable to me.

> > 
> > >  		vgic_its_invalidate_all_caches(vcpu->kvm);
> > >  		atomic_set_release(&vgic_cpu->ctlr, 0);
> > > +		kvm_arm_resume_guest(vcpu->kvm);
> > 
> > [Severity: High]
> > Does this block need to hold a VM-wide lock?
> > 
> > Since vgic_mmio_write_v3r_ctlr() only synchronizes locally using an atomic
> > cmpxchg on the target vgic_cpu->ctlr, multiple vCPUs could concurrently
> > write to different VGIC redistributors to disable their LPIs.
> > 
> > If this happens, multiple vCPUs will concurrently call kvm_arm_halt_guest()
> > and race to set vcpu->arch.pause = true:
> > 
> > arch/arm64/kvm/arm.c:kvm_arm_halt_guest() {
> >     ...
> > 	kvm_for_each_vcpu(i, vcpu, kvm)
> > 		vcpu->arch.pause = true;
> >     ...
> > }
> > 
> > The first vCPU to finish flushing will call kvm_arm_resume_guest() which
> > unconditionally clears the pause boolean:
> > 
> > arch/arm64/kvm/arm.c:kvm_arm_resume_guest() {
> >     ...
> > 	kvm_for_each_vcpu(i, vcpu, kvm) {
> > 		vcpu->arch.pause = false;
> > 		__kvm_vcpu_wake_up(vcpu);
> > 	}
> >     ...
> > }
> > 
> > Could this unpause the VM prematurely while another vCPU is still tearing
> > down VGIC state in vgic_flush_pending_lpis(), allowing guests to run and
> > modify their VGIC state concurrently?
> 
> Just as interesting. It appears that this doesn't nest correctly,
> while we really rely on it to work. An obvious way to do this is to
> actually turn pause into a refcount. Something like the (untested)
> hack below.
> 
> 	M.
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27fe0cd5b2d7a..08dc8f816824c 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -889,11 +889,10 @@ struct kvm_vcpu_arch {
>  	/*
>  	 * Don't run the guest (internal implementation need).
>  	 *
> -	 * Contrary to the flags above, this is set/cleared outside of
> -	 * a vcpu context, and thus cannot be mixed with the flags
> -	 * themselves (or the flag accesses need to be made atomic).
> +	 * Contrary to the flags above, this is updated outside of
> +	 * a vcpu context, and thus cannot be mixed with the flags.
>  	 */
> -	bool pause;
> +	refcount_t pause;
>  
>  	/*
>  	 * We maintain more than a single set of debug registers to support
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90b..62b683107ccae 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -561,6 +561,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>  	kvm_arm_pvtime_vcpu_init(&vcpu->arch);
>  
>  	vcpu->arch.hw_mmu = &vcpu->kvm->arch.mmu;
> +	refcount_set(&vcpu->arch.pause, 0);
>  
>  	/*
>  	 * This vCPU may have been created after mpidr_data was initialized.
> @@ -835,6 +836,11 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
>  	return ret;
>  }
>  
> +static bool vcpu_can_run(struct kvm_vcpu *vcpu)
> +{
> +	return !kvm_arm_vcpu_stopped(vcpu) && !refcount_read(&vcpu->arch.pause);
> +}
> +
>  /**
>   * kvm_arch_vcpu_runnable - determine if the vcpu can be scheduled
>   * @v:		The VCPU pointer
> @@ -850,8 +856,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
>  		      (kvm_timer_should_notify_user(v) ||
>  		       kvm_pmu_should_notify_user(v)));
>  
> -	return ((irq_lines || kvm_vgic_vcpu_pending_irq(v))
> -		&& !kvm_arm_vcpu_stopped(v) && !v->arch.pause);
> +	return ((irq_lines || kvm_vgic_vcpu_pending_irq(v)) && !vcpu_can_run(v));
>  }
>  
>  bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
> @@ -1014,7 +1019,7 @@ void kvm_arm_halt_guest(struct kvm *kvm)
>  	struct kvm_vcpu *vcpu;
>  
>  	kvm_for_each_vcpu(i, vcpu, kvm)
> -		vcpu->arch.pause = true;
> +		refcount_inc(&vcpu->arch.pause);
>  	kvm_make_all_cpus_request(kvm, KVM_REQ_SLEEP);
>  }

No issues with this approach but it is worth noting that all other
callers of kvm_arm_halt_guest() do so behind the config_lock today. You
could probably do the same with the LPI disable path.

Thanks,
Oliver

Re: [PATCH 3/3] KVM: arm64: vgic: Stop the VM when disabling LPIs

[Marc Zyngier]

On Wed, 23 Sep 2026 17:41:53 +0100,
Oliver Upton <oupton@kernel.org> wrote:
> 
> No issues with this approach but it is worth noting that all other
> callers of kvm_arm_halt_guest() do so behind the config_lock today. You
> could probably do the same with the LPI disable path.

Not quite. The IRQ bypass stuff doesn't know about the lock... But
maybe we can fix it the same way. I'll investigate.

Thanks,

	M.

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

Re: [PATCH 3/3] KVM: arm64: vgic: Stop the VM when disabling LPIs

[Oliver Upton]

On Wed, Sep 23, 2026 at 06:56:03PM +0100, Marc Zyngier wrote:
> On Wed, 23 Sep 2026 17:41:53 +0100,
> Oliver Upton <oupton@kernel.org> wrote:
> > 
> > No issues with this approach but it is worth noting that all other
> > callers of kvm_arm_halt_guest() do so behind the config_lock today. You
> > could probably do the same with the LPI disable path.
> 
> Not quite. The IRQ bypass stuff doesn't know about the lock... But
> maybe we can fix it the same way. I'll investigate.
> 

Ah, right, good point.

On second thought, that'd require we hold the config_lock between
->stop() and ->start() on the irqbypass side of things and I'd prefer we
not externalize config_lock and how it ties into the locking hierarchy.

So ignore my feedback, I'm happy with the refcounting implementation.

Thanks,
Oliver