[PATCH v2 1/7] KVM: arm64: Move OUTSIDE_GUEST_MODE publication past context being saved

[PATCH v2 1/7] KVM: arm64: Move OUTSIDE_GUEST_MODE publication past context being saved

[Marc Zyngier]

OUTSIDE_GUEST_MODE indicates to the rest of KVM that the vcpu has
exited. So far, this is set as soon as we return from the notional
hypervisor (which with VHE is just a function call away).

However, code that uses OUTSIDE_GUEST_MODE as a synchronisation point
relies on the vcpu state to have been written back to the in-memory
structures. Clearly, this is not what is happening

Move the publication of OUTSIDE_GUEST_MODE to the point where the state
is actually written, and give this write release semantics to ensure the
correct ordering.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/arm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

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
-- 
2.47.3

Re: [PATCH v2 1/7] KVM: arm64: Move OUTSIDE_GUEST_MODE publication past context being saved

[Fuad Tabba]

Hi Marc,

On Tue, 29 Sep 2026 10:35:42 +0100, Marc Zyngier <maz@kernel.org> wrote:
[...]
> Move the publication of OUTSIDE_GUEST_MODE to the point where the state
> is actually written, and give this write release semantics to ensure the
> correct ordering.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org


No Fixes: tag?

[...]
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
[...]
> @@ -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);

Pardon my atomics :), but what does the release pair with? On the halt
path, the only reader I can find is the cmpxchg() in
kvm_vcpu_exiting_guest_mode(), and the LPI-disable and MOVALL halts
then take ap_list_lock or irq_lock. Would WRITE_ONCE() be enough?

Should the early exit path (the kvm_vcpu_exit_request() bail-out) get
the same treatment? I think that's what Sashiko is trying to say in
the patch 5 review [1].

nit: "let advertise" -> "publish"?

Cheers,
/fuad

[1] https://sashiko.dev/#/patchset/20260929093548.3598547-1-maz%40kernel.org?part=5

Re: [PATCH v2 1/7] KVM: arm64: Move OUTSIDE_GUEST_MODE publication past context being saved

[Marc Zyngier]

On Tue, 29 Sep 2026 13:59:23 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> Hi Marc,
> 
> On Tue, 29 Sep 2026 10:35:42 +0100, Marc Zyngier <maz@kernel.org> wrote:
> [...]
> > Move the publication of OUTSIDE_GUEST_MODE to the point where the state
> > is actually written, and give this write release semantics to ensure the
> > correct ordering.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Cc: stable@vger.kernel.org
> 
> 
> No Fixes: tag?

No. It's always been fsck'd.

> 
> [...]
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> [...]
> > @@ -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);
> 
> Pardon my atomics :)

This is not an atomic instruction. However, it composes with atomics.

> , but what does the release pair with? On the halt
> path, the only reader I can find is the cmpxchg() in
> kvm_vcpu_exiting_guest_mode()

From Documentation/atomic_t.txt:

<quote>
 - RMW operations that have a return value are fully ordered;

 - RMW operations that are conditional are unordered on FAILURE,
   otherwise the above rules apply.
</quote>

The acquire side of cmpxchg() is therefore interacting with the above
release, which gives us the required ordering.

However, there is a problem if cmpxchg() fails, as there is no
ordering in that case, and I'm not sure the smp_mb__before_atomic()
saves the bacon in that case. It feels we'd need an acquire
somewhere, a bit like this:

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 03bfc92864b6e..2efb4febcb235 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -563,9 +563,15 @@ static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
 	 * The memory barrier ensures a previous write to vcpu->requests cannot
 	 * be reordered with the read of vcpu->mode.  It pairs with the general
 	 * memory barrier following the write of vcpu->mode in VCPU RUN.
+	 *
+	 * cmpxchg() is not ordered when failing, so make sure we perform an
+	 * acquire in that case.
 	 */
 	smp_mb__before_atomic();
-	return cmpxchg(&vcpu->mode, IN_GUEST_MODE, EXITING_GUEST_MODE);
+	if (cmpxchg(&vcpu->mode, IN_GUEST_MODE, EXITING_GUEST_MODE) != IN_GUEST_MODE)
+		return smp_load_acquire(&vcpu->mode);
+
+	return IN_GUEST_MODE;
 }
 
 /*

> , and the LPI-disable and MOVALL halts
> then take ap_list_lock or irq_lock. Would WRITE_ONCE() be enough?

We need a release so that we know for sure that any state stored
before is visible by the time we can observe OUTSIDE_GUEST_MODE, and
WRITE_ONCE() doesn't provide that (it can be reordered).

I don't see what taking a lock changes to the ordering requirement.

>
> Should the early exit path (the kvm_vcpu_exit_request() bail-out) get
> the same treatment? I think that's what Sashiko is trying to say in
> the patch 5 review [1].

I don't understand what sashiko is trying to say, but this is clearly
missing from the patch, see below. Not sure how I missed that one.

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 9a4871cd796bc..1a3a15bc6f55c 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1333,13 +1333,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 		smp_store_mb(vcpu->mode, IN_GUEST_MODE);
 
 		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;

Thanks,

	M.

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

Re: [PATCH v2 1/7] KVM: arm64: Move OUTSIDE_GUEST_MODE publication past context being saved

[Fuad Tabba]

Hi Marc,

On Tue, 29 Sep 2026 15:13:28 +0100, Marc Zyngier <maz@kernel.org> wrote:
[...]
> However, there is a problem if cmpxchg() fails, as there is no
> ordering in that case, and I'm not sure the smp_mb__before_atomic()
> saves the bacon in that case. It feels we'd need an acquire
> somewhere, a bit like this:

It doesn't save the bacon. I ran it through herd7 with the LKMM, the
fold walk in vgic_v3_fold_lr_state() against the list_del() in
vgic_flush_pending_lpis(): with smp_mb__before_atomic() and a failing
cmpxchg(), the walk can see the list_del(). With your
smp_load_acquire() it can't.

[...]
> We need a release so that we know for sure that any state stored
> before is visible by the time we can observe OUTSIDE_GUEST_MODE, and
> WRITE_ONCE() doesn't provide that (it can be reordered).
>
> I don't see what taking a lock changes to the ordering requirement.

The fold walk takes no lock, so the locks on the halt side don't order
it against the list_del(). herd7 agrees: WRITE_ONCE() with your
acquire can see the list_del() again.

[...]
> I don't understand what sashiko is trying to say, but this is clearly
> missing from the patch, see below. Not sure how I missed that one.

I don't understand Sashiko half of the time either :) That said, the
diff looks right to me.

Cheers,
/fuad

Re: [PATCH v2 1/7] KVM: arm64: Move OUTSIDE_GUEST_MODE publication past context being saved

[Will Deacon]

Hi folks,

Sorry, but this is probably an incredibly unhelpful drive-by comment
but Marc was talking about vcpu->mode the other day and I couldn't
resist looking at it some more. Like a moth to a flame...

See below.

On Tue, Sep 29, 2026 at 03:13:28PM +0100, Marc Zyngier wrote:
> On Tue, 29 Sep 2026 13:59:23 +0100,
> Fuad Tabba <fuad.tabba@linux.dev> wrote:
> > On Tue, 29 Sep 2026 10:35:42 +0100, Marc Zyngier <maz@kernel.org> wrote:
> > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > [...]
> > > @@ -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);
> > 
> > Pardon my atomics :)
> 
> This is not an atomic instruction. However, it composes with atomics.
> 
> > , but what does the release pair with? On the halt
> > path, the only reader I can find is the cmpxchg() in
> > kvm_vcpu_exiting_guest_mode()
> 
> From Documentation/atomic_t.txt:
> 
> <quote>
>  - RMW operations that have a return value are fully ordered;
> 
>  - RMW operations that are conditional are unordered on FAILURE,
>    otherwise the above rules apply.
> </quote>
> 
> The acquire side of cmpxchg() is therefore interacting with the above
> release, which gives us the required ordering.
> 
> However, there is a problem if cmpxchg() fails, as there is no
> ordering in that case, and I'm not sure the smp_mb__before_atomic()
> saves the bacon in that case. It feels we'd need an acquire
> somewhere, a bit like this:

(as discussed off list, you can use smp_acquire__after_ctrl_dep() if
you're feeling really brave)

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 03bfc92864b6e..2efb4febcb235 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -563,9 +563,15 @@ static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
>  	 * The memory barrier ensures a previous write to vcpu->requests cannot
>  	 * be reordered with the read of vcpu->mode.  It pairs with the general
>  	 * memory barrier following the write of vcpu->mode in VCPU RUN.
> +	 *
> +	 * cmpxchg() is not ordered when failing, so make sure we perform an
> +	 * acquire in that case.
>  	 */
>  	smp_mb__before_atomic();
> -	return cmpxchg(&vcpu->mode, IN_GUEST_MODE, EXITING_GUEST_MODE);
> +	if (cmpxchg(&vcpu->mode, IN_GUEST_MODE, EXITING_GUEST_MODE) != IN_GUEST_MODE)
> +		return smp_load_acquire(&vcpu->mode);
> +
> +	return IN_GUEST_MODE;
>  }
>  
>  /*
> 
> > , and the LPI-disable and MOVALL halts
> > then take ap_list_lock or irq_lock. Would WRITE_ONCE() be enough?
> 
> We need a release so that we know for sure that any state stored
> before is visible by the time we can observe OUTSIDE_GUEST_MODE, and
> WRITE_ONCE() doesn't provide that (it can be reordered).
> 
> I don't see what taking a lock changes to the ordering requirement.
> 
> >
> > Should the early exit path (the kvm_vcpu_exit_request() bail-out) get
> > the same treatment? I think that's what Sashiko is trying to say in
> > the patch 5 review [1].
> 
> I don't understand what sashiko is trying to say, but this is clearly
> missing from the patch, see below. Not sure how I missed that one.
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 9a4871cd796bc..1a3a15bc6f55c 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1333,13 +1333,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  		smp_store_mb(vcpu->mode, IN_GUEST_MODE);
>  
>  		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);

I'm struggling to see why a release is sufficient here, but I'm also
struggling to understand the bigger picture so I'm probably just confused.

I can see why a release is necessary for the saved state to be visible
to another CPU that has kicked the vCPU out of the guest and then uses
vcpu->mode == OUTSIDE_GUEST_MODE as the indication that the state is
safe to consume. However, don't we also need to make sure that any
subsequent check for a pending request on _this_ vCPU is ordered after
that write to the mode? Now that we've toggled it away from IN_GUEST_MODE,
I think IPIs can be elided by the kick, so a subsequent call to
e.g. kvm_request_pending() must be observed after that toggle, otherwise
I think we could miss a request.

Can you see the tree I'm barking up here?

Will

[PATCH v2 2/7] KVM: arm64: Turn vcpu->arch.pause into a counter

[Marc Zyngier]

We have situations where we can call kvm_arm_halt_guest() without
holding config_lock, which means that halt and resume can overlap in
funny ways when called from separate contexts, and result in situations
where a vcpu is resumed while other parts of KVM assume it is halted.
Consequences are left to the imagination of the reader.

Fix this sorry situation by turning kvm_vcpu_arch::pause into an
atomic counter, which makes the races described above harmless.

Fixes: 3b92830ad41b2 ("KVM: arm/arm64: implement kvm_arm_[halt,resume]_guest")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/include/asm/kvm_host.h |  7 +++----
 arch/arm64/kvm/arm.c              | 21 ++++++++++++---------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 27fe0cd5b2d7a..66ea2372f9870 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;
+	atomic_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 5f5dd8bead4b9..9a4871cd796bc 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;
+	atomic_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) && !atomic_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;
+		atomic_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 (atomic_dec_and_test(&vcpu->arch.pause))
+			__kvm_vcpu_wake_up(vcpu);
 	}
 }
 
@@ -1033,11 +1038,9 @@ 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),
-			   TASK_INTERRUPTIBLE);
+	rcuwait_wait_event(wait, 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);
 	}
-- 
2.47.3

Re: [PATCH v2 2/7] KVM: arm64: Turn vcpu->arch.pause into a counter

[Fuad Tabba]

On Tue, 29 Sept 2026 at 10:36, Marc Zyngier <maz@kernel.org> wrote:
>
> We have situations where we can call kvm_arm_halt_guest() without
> holding config_lock, which means that halt and resume can overlap in
> funny ways when called from separate contexts, and result in situations
> where a vcpu is resumed while other parts of KVM assume it is halted.
> Consequences are left to the imagination of the reader.
>
> Fix this sorry situation by turning kvm_vcpu_arch::pause into an
> atomic counter, which makes the races described above harmless.
>
> Fixes: 3b92830ad41b2 ("KVM: arm/arm64: implement kvm_arm_[halt,resume]_guest")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org

Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba < fuad.tabba@linux.dev>

Cheers,
/fuad


> ---
>  arch/arm64/include/asm/kvm_host.h |  7 +++----
>  arch/arm64/kvm/arm.c              | 21 ++++++++++++---------
>  2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27fe0cd5b2d7a..66ea2372f9870 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;
> +       atomic_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 5f5dd8bead4b9..9a4871cd796bc 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;
> +       atomic_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) && !atomic_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;
> +               atomic_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 (atomic_dec_and_test(&vcpu->arch.pause))
> +                       __kvm_vcpu_wake_up(vcpu);
>         }
>  }
>
> @@ -1033,11 +1038,9 @@ 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),
> -                          TASK_INTERRUPTIBLE);
> +       rcuwait_wait_event(wait, 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);
>         }
> --
> 2.47.3
>

[PATCH v2 3/7] 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 v2 4/7] 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 v2 5/7] 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

[PATCH v2 6/7] KVM: arm64: vgic-its: Fix MOVALL handling of source redistributor

[Marc Zyngier]

The MOVALL command moves the pending state of all LPIs targeting a given
redistributor to another one. However, we seem to have lost the
filtering on the source RD, which means we move all LPIs to the target.

Not quite what the spec mandates.

Hack update_affinity() to take an optional source vcpu that is used as a
filter when non-NULL, restoring the filtering that was performed by
vgic_copy_lpi_list() back in the days.

Fixes: 11f4f8f3e6e06 ("KVM: arm64: vgic-its: Walk LPI xarray in vgic_its_cmd_handle_movall()")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/vgic/vgic-its.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 9e782a4fea7e5..cc02d9b62bd95 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -319,12 +319,16 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
 	return ret;
 }
 
-static int update_affinity(struct vgic_irq *irq, struct kvm_vcpu *vcpu)
+static int update_affinity(struct vgic_irq *irq,
+			   struct kvm_vcpu *from_vcpu, struct kvm_vcpu *vcpu)
 {
 	struct its_vlpi_map map;
 	int ret;
 
 	guard(raw_spinlock_irqsave)(&irq->irq_lock);
+	if (from_vcpu && irq->target_vcpu != from_vcpu)
+		return 0;
+
 	irq->target_vcpu = vcpu;
 
 	if (!irq->hw)
@@ -362,7 +366,7 @@ static void update_affinity_ite(struct kvm *kvm, struct its_ite *ite)
 		return;
 
 	vcpu = collection_to_vcpu(kvm, ite->collection);
-	update_affinity(ite->irq, vcpu);
+	update_affinity(ite->irq, NULL, vcpu);
 }
 
 /*
@@ -856,7 +860,7 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, struct vgic_its *its,
 
 	vgic_its_invalidate_cache(its);
 
-	return update_affinity(ite->irq, vcpu);
+	return update_affinity(ite->irq, NULL, vcpu);
 }
 
 static bool __is_visible_gfn_locked(struct vgic_its *its, gpa_t gpa)
@@ -1383,7 +1387,7 @@ static int vgic_its_cmd_handle_movall(struct kvm *kvm, struct vgic_its *its,
 		if (!irq)
 			continue;
 
-		update_affinity(irq, vcpu2);
+		update_affinity(irq, vcpu1, vcpu2);
 
 		vgic_put_irq(kvm, irq);
 	}
-- 
2.47.3

Re: [PATCH v2 6/7] KVM: arm64: vgic-its: Fix MOVALL handling of source redistributor

[Fuad Tabba]

On Tue, 29 Sept 2026 at 10:36, Marc Zyngier <maz@kernel.org> wrote:
>
> The MOVALL command moves the pending state of all LPIs targeting a given
> redistributor to another one. However, we seem to have lost the
> filtering on the source RD, which means we move all LPIs to the target.
>
> Not quite what the spec mandates.
>
> Hack update_affinity() to take an optional source vcpu that is used as a
> filter when non-NULL, restoring the filtering that was performed by
> vgic_copy_lpi_list() back in the days.
>
> Fixes: 11f4f8f3e6e06 ("KVM: arm64: vgic-its: Walk LPI xarray in vgic_its_cmd_handle_movall()")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org

Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba < fuad.tabba@linux.dev>

Cheers,
/fuad


> ---
>  arch/arm64/kvm/vgic/vgic-its.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index 9e782a4fea7e5..cc02d9b62bd95 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -319,12 +319,16 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
>         return ret;
>  }
>
> -static int update_affinity(struct vgic_irq *irq, struct kvm_vcpu *vcpu)
> +static int update_affinity(struct vgic_irq *irq,
> +                          struct kvm_vcpu *from_vcpu, struct kvm_vcpu *vcpu)
>  {
>         struct its_vlpi_map map;
>         int ret;
>
>         guard(raw_spinlock_irqsave)(&irq->irq_lock);
> +       if (from_vcpu && irq->target_vcpu != from_vcpu)
> +               return 0;
> +
>         irq->target_vcpu = vcpu;
>
>         if (!irq->hw)
> @@ -362,7 +366,7 @@ static void update_affinity_ite(struct kvm *kvm, struct its_ite *ite)
>                 return;
>
>         vcpu = collection_to_vcpu(kvm, ite->collection);
> -       update_affinity(ite->irq, vcpu);
> +       update_affinity(ite->irq, NULL, vcpu);
>  }
>
>  /*
> @@ -856,7 +860,7 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, struct vgic_its *its,
>
>         vgic_its_invalidate_cache(its);
>
> -       return update_affinity(ite->irq, vcpu);
> +       return update_affinity(ite->irq, NULL, vcpu);
>  }
>
>  static bool __is_visible_gfn_locked(struct vgic_its *its, gpa_t gpa)
> @@ -1383,7 +1387,7 @@ static int vgic_its_cmd_handle_movall(struct kvm *kvm, struct vgic_its *its,
>                 if (!irq)
>                         continue;
>
> -               update_affinity(irq, vcpu2);
> +               update_affinity(irq, vcpu1, vcpu2);
>
>                 vgic_put_irq(kvm, irq);
>         }
> --
> 2.47.3
>