Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: arm64: nv: Skip vCPUs without a pseudo-TLB in invalidate_vncr_va()
@ 2026-06-07  8:43 Hyunwoo Kim
  2026-06-07 13:05 ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Hyunwoo Kim @ 2026-06-07  8:43 UTC (permalink / raw)
  To: maz, oupton, joey.gouly, seiden, suzuki.poulose, yuzenghui,
	catalin.marinas, will
  Cc: linux-arm-kernel, kvmarm, imv4bel

vncr_tlb is not allocated before a vCPU runs for the first time, so
vcpu->arch.vncr_tlb is NULL for a vCPU that has been created but not yet
run. Code that iterates over every vCPU's pseudo-TLB must skip those.

invalidate_vncr_va() iterates over the vCPUs with kvm_for_each_vcpu() and
dereferences vt->valid without checking whether vncr_tlb is NULL.

While iterating, skip vCPUs whose pseudo-TLB has not been allocated.

Fixes: 4ffa72ad8f37 ("KVM: arm64: nv: Add S1 TLB invalidation primitive for VNCR_EL2")
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 arch/arm64/kvm/nested.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 6f7bc9a9992e..063e079d1d1a 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -969,6 +969,10 @@ static void invalidate_vncr_va(struct kvm *kvm,
 		struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
 		u64 va_start, va_end, va_size;
 
+		/* Skip vCPUs whose pseudo-TLB hasn't been allocated yet */
+		if (!vt)
+			continue;
+
 		if (!vt->valid)
 			continue;
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: nv: Skip vCPUs without a pseudo-TLB in invalidate_vncr_va()
  2026-06-07  8:43 [PATCH] KVM: arm64: nv: Skip vCPUs without a pseudo-TLB in invalidate_vncr_va() Hyunwoo Kim
@ 2026-06-07 13:05 ` Marc Zyngier
  2026-06-07 13:36   ` Hyunwoo Kim
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2026-06-07 13:05 UTC (permalink / raw)
  To: Hyunwoo Kim
  Cc: oupton, joey.gouly, seiden, suzuki.poulose, yuzenghui,
	catalin.marinas, will, linux-arm-kernel, kvmarm

On Sun, 07 Jun 2026 09:43:53 +0100,
Hyunwoo Kim <imv4bel@gmail.com> wrote:
> 
> vncr_tlb is not allocated before a vCPU runs for the first time, so
> vcpu->arch.vncr_tlb is NULL for a vCPU that has been created but not yet
> run. Code that iterates over every vCPU's pseudo-TLB must skip those.
> 
> invalidate_vncr_va() iterates over the vCPUs with kvm_for_each_vcpu() and
> dereferences vt->valid without checking whether vncr_tlb is NULL.
> 
> While iterating, skip vCPUs whose pseudo-TLB has not been allocated.
>
> Fixes: 4ffa72ad8f37 ("KVM: arm64: nv: Add S1 TLB invalidation primitive for VNCR_EL2")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  arch/arm64/kvm/nested.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 6f7bc9a9992e..063e079d1d1a 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -969,6 +969,10 @@ static void invalidate_vncr_va(struct kvm *kvm,
>  		struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
>  		u64 va_start, va_end, va_size;
>  
> +		/* Skip vCPUs whose pseudo-TLB hasn't been allocated yet */
> +		if (!vt)
> +			continue;
> +
>  		if (!vt->valid)
>  			continue;
>  

This looks correct and matches what we already have for
invalidate_vncr_ipa().

But I think this misses the opportunity to squash a whole class of
similar bugs, should we ever have the need for another function that
iterates over all *valid* VNCR pseudo-TLBs.

Since I'm on a train and have nothing better to do, I've written the
following hack.

Thoughts?

	M.

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 38f672e940878..f0a9f81a08302 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -897,9 +897,21 @@ static void invalidate_vncr(struct vncr_tlb *vt)
 		clear_fixmap(vncr_fixmap(vt->cpu));
 }
 
+/*
+ * VNCR TLB invalidation occurs from MMU notifiers or TLBI instructions, and
+ * either can race against a vcpu not being onlined yet (no pseudo-TLB
+ * allocated). Similarly, the TLB might be invalid.  Skip those, as they
+ * obviously don't participate in the invalidation at this stage.
+ */
+#define kvm_for_each_vncr_tlb(idx, vcpup, tlbp, kvm)	\
+	kvm_for_each_vcpu(idx, vcpu, kvm)		\
+		if (((tlbp) = vcpu->arch.vncr_tlb) &&	\
+		    (tlbp)->valid)
+
 static void kvm_invalidate_vncr_ipa(struct kvm *kvm, u64 start, u64 end)
 {
 	struct kvm_vcpu *vcpu;
+	struct vncr_tlb *vt;
 	unsigned long i;
 
 	lockdep_assert_held_write(&kvm->mmu_lock);
@@ -907,24 +919,9 @@ static void kvm_invalidate_vncr_ipa(struct kvm *kvm, u64 start, u64 end)
 	if (!kvm_has_feat(kvm, ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY))
 		return;
 
-	kvm_for_each_vcpu(i, vcpu, kvm) {
-		struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
+	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
 		u64 ipa_start, ipa_end, ipa_size;
 
-		/*
-		 * Careful here: We end-up here from an MMU notifier,
-		 * and this can race against a vcpu not being onlined
-		 * yet, without the pseudo-TLB being allocated.
-		 *
-		 * Skip those, as they obviously don't participate in
-		 * the invalidation at this stage.
-		 */
-		if (!vt)
-			continue;
-
-		if (!vt->valid)
-			continue;
-
 		ipa_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift,
 							    vt->wr.level));
 		ipa_start = vt->wr.pa & ~(ipa_size - 1);
@@ -954,17 +951,14 @@ static void invalidate_vncr_va(struct kvm *kvm,
 			       struct s1e2_tlbi_scope *scope)
 {
 	struct kvm_vcpu *vcpu;
+	struct vncr_tlb *vt;
 	unsigned long i;
 
 	lockdep_assert_held_write(&kvm->mmu_lock);
 
-	kvm_for_each_vcpu(i, vcpu, kvm) {
-		struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
+	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
 		u64 va_start, va_end, va_size;
 
-		if (!vt->valid)
-			continue;
-
 		va_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift,
 							   vt->wr.level));
 		va_start = vt->gva & ~(va_size - 1);

-- 
Jazz isn't dead. It just smells funny.


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: nv: Skip vCPUs without a pseudo-TLB in invalidate_vncr_va()
  2026-06-07 13:05 ` Marc Zyngier
@ 2026-06-07 13:36   ` Hyunwoo Kim
  2026-06-07 18:00     ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Hyunwoo Kim @ 2026-06-07 13:36 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: oupton, joey.gouly, seiden, suzuki.poulose, yuzenghui,
	catalin.marinas, will, linux-arm-kernel, kvmarm, imv4bel

On Sun, Jun 07, 2026 at 02:05:02PM +0100, Marc Zyngier wrote:
> On Sun, 07 Jun 2026 09:43:53 +0100,
> Hyunwoo Kim <imv4bel@gmail.com> wrote:
> > 
> > vncr_tlb is not allocated before a vCPU runs for the first time, so
> > vcpu->arch.vncr_tlb is NULL for a vCPU that has been created but not yet
> > run. Code that iterates over every vCPU's pseudo-TLB must skip those.
> > 
> > invalidate_vncr_va() iterates over the vCPUs with kvm_for_each_vcpu() and
> > dereferences vt->valid without checking whether vncr_tlb is NULL.
> > 
> > While iterating, skip vCPUs whose pseudo-TLB has not been allocated.
> >
> > Fixes: 4ffa72ad8f37 ("KVM: arm64: nv: Add S1 TLB invalidation primitive for VNCR_EL2")
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >  arch/arm64/kvm/nested.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index 6f7bc9a9992e..063e079d1d1a 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> > @@ -969,6 +969,10 @@ static void invalidate_vncr_va(struct kvm *kvm,
> >  		struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
> >  		u64 va_start, va_end, va_size;
> >  
> > +		/* Skip vCPUs whose pseudo-TLB hasn't been allocated yet */
> > +		if (!vt)
> > +			continue;
> > +
> >  		if (!vt->valid)
> >  			continue;
> >  
> 
> This looks correct and matches what we already have for
> invalidate_vncr_ipa().
> 
> But I think this misses the opportunity to squash a whole class of
> similar bugs, should we ever have the need for another function that
> iterates over all *valid* VNCR pseudo-TLBs.
> 
> Since I'm on a train and have nothing better to do, I've written the
> following hack.
> 
> Thoughts?

Looks like a good direction to me. I confirmed it fixes the issue (as
expected).

How about you submit this patch yourself?

> 
> 	M.
> 
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 38f672e940878..f0a9f81a08302 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -897,9 +897,21 @@ static void invalidate_vncr(struct vncr_tlb *vt)
>  		clear_fixmap(vncr_fixmap(vt->cpu));
>  }
>  
> +/*
> + * VNCR TLB invalidation occurs from MMU notifiers or TLBI instructions, and
> + * either can race against a vcpu not being onlined yet (no pseudo-TLB
> + * allocated). Similarly, the TLB might be invalid.  Skip those, as they
> + * obviously don't participate in the invalidation at this stage.
> + */
> +#define kvm_for_each_vncr_tlb(idx, vcpup, tlbp, kvm)	\
> +	kvm_for_each_vcpu(idx, vcpu, kvm)		\

Maybe vcpu -> vcpup?

> +		if (((tlbp) = vcpu->arch.vncr_tlb) &&	\
> +		    (tlbp)->valid)
> +
>  static void kvm_invalidate_vncr_ipa(struct kvm *kvm, u64 start, u64 end)
>  {
>  	struct kvm_vcpu *vcpu;
> +	struct vncr_tlb *vt;
>  	unsigned long i;
>  
>  	lockdep_assert_held_write(&kvm->mmu_lock);
> @@ -907,24 +919,9 @@ static void kvm_invalidate_vncr_ipa(struct kvm *kvm, u64 start, u64 end)
>  	if (!kvm_has_feat(kvm, ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY))
>  		return;
>  
> -	kvm_for_each_vcpu(i, vcpu, kvm) {
> -		struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
> +	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
>  		u64 ipa_start, ipa_end, ipa_size;
>  
> -		/*
> -		 * Careful here: We end-up here from an MMU notifier,
> -		 * and this can race against a vcpu not being onlined
> -		 * yet, without the pseudo-TLB being allocated.
> -		 *
> -		 * Skip those, as they obviously don't participate in
> -		 * the invalidation at this stage.
> -		 */
> -		if (!vt)
> -			continue;
> -
> -		if (!vt->valid)
> -			continue;
> -
>  		ipa_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift,
>  							    vt->wr.level));
>  		ipa_start = vt->wr.pa & ~(ipa_size - 1);
> @@ -954,17 +951,14 @@ static void invalidate_vncr_va(struct kvm *kvm,
>  			       struct s1e2_tlbi_scope *scope)
>  {
>  	struct kvm_vcpu *vcpu;
> +	struct vncr_tlb *vt;
>  	unsigned long i;
>  
>  	lockdep_assert_held_write(&kvm->mmu_lock);
>  
> -	kvm_for_each_vcpu(i, vcpu, kvm) {
> -		struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
> +	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
>  		u64 va_start, va_end, va_size;
>  
> -		if (!vt->valid)
> -			continue;
> -
>  		va_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift,
>  							   vt->wr.level));
>  		va_start = vt->gva & ~(va_size - 1);
> 
> -- 
> Jazz isn't dead. It just smells funny.


Best regards,
Hyunwoo Kim


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: nv: Skip vCPUs without a pseudo-TLB in invalidate_vncr_va()
  2026-06-07 13:36   ` Hyunwoo Kim
@ 2026-06-07 18:00     ` Marc Zyngier
  0 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2026-06-07 18:00 UTC (permalink / raw)
  To: Hyunwoo Kim
  Cc: oupton, joey.gouly, seiden, suzuki.poulose, yuzenghui,
	catalin.marinas, will, linux-arm-kernel, kvmarm

On Sun, 07 Jun 2026 14:36:44 +0100,
Hyunwoo Kim <imv4bel@gmail.com> wrote:
> 
> On Sun, Jun 07, 2026 at 02:05:02PM +0100, Marc Zyngier wrote:
> > On Sun, 07 Jun 2026 09:43:53 +0100,
> > Hyunwoo Kim <imv4bel@gmail.com> wrote:
> > > 
> > > vncr_tlb is not allocated before a vCPU runs for the first time, so
> > > vcpu->arch.vncr_tlb is NULL for a vCPU that has been created but not yet
> > > run. Code that iterates over every vCPU's pseudo-TLB must skip those.
> > > 
> > > invalidate_vncr_va() iterates over the vCPUs with kvm_for_each_vcpu() and
> > > dereferences vt->valid without checking whether vncr_tlb is NULL.
> > > 
> > > While iterating, skip vCPUs whose pseudo-TLB has not been allocated.
> > >
> > > Fixes: 4ffa72ad8f37 ("KVM: arm64: nv: Add S1 TLB invalidation primitive for VNCR_EL2")
> > > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > > ---
> > >  arch/arm64/kvm/nested.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > > index 6f7bc9a9992e..063e079d1d1a 100644
> > > --- a/arch/arm64/kvm/nested.c
> > > +++ b/arch/arm64/kvm/nested.c
> > > @@ -969,6 +969,10 @@ static void invalidate_vncr_va(struct kvm *kvm,
> > >  		struct vncr_tlb *vt = vcpu->arch.vncr_tlb;
> > >  		u64 va_start, va_end, va_size;
> > >  
> > > +		/* Skip vCPUs whose pseudo-TLB hasn't been allocated yet */
> > > +		if (!vt)
> > > +			continue;
> > > +
> > >  		if (!vt->valid)
> > >  			continue;
> > >  
> > 
> > This looks correct and matches what we already have for
> > invalidate_vncr_ipa().
> > 
> > But I think this misses the opportunity to squash a whole class of
> > similar bugs, should we ever have the need for another function that
> > iterates over all *valid* VNCR pseudo-TLBs.
> > 
> > Since I'm on a train and have nothing better to do, I've written the
> > following hack.
> > 
> > Thoughts?
> 
> Looks like a good direction to me. I confirmed it fixes the issue (as
> expected).
> 
> How about you submit this patch yourself?

Sure, can do. I'll sent that in a minute.

> > +#define kvm_for_each_vncr_tlb(idx, vcpup, tlbp, kvm)	\
> > +	kvm_for_each_vcpu(idx, vcpu, kvm)		\
> 
> Maybe vcpu -> vcpup?

Duh. Yes. Thanks.

	M.

-- 
Jazz isn't dead. It just smells funny.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-07 17:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-07  8:43 [PATCH] KVM: arm64: nv: Skip vCPUs without a pseudo-TLB in invalidate_vncr_va() Hyunwoo Kim
2026-06-07 13:05 ` Marc Zyngier
2026-06-07 13:36   ` Hyunwoo Kim
2026-06-07 18:00     ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox