[PATCH] KVM: arm64: nv: Don't skip VNCR invalidation when the TLB size is unknown

[PATCH] KVM: arm64: nv: Don't skip VNCR invalidation when the TLB size is unknown

[Hyunwoo Kim]

ttl_to_size() returns 0 when no size is defined for the (granule, level)
pair. That is the case for level 0 with 4kB pages, and for levels 0 and 1
with 16kB and 64kB pages. For a VNCR pseudo-TLB this happens when the
guest runs with SCTLR_EL2.M cleared, because __kvm_translate_va() returns
success with wr->level left at S1_MMU_DISABLED, and that s8 value becomes
level 1 as it goes through the u8 parameter of pgshift_level_to_ttl(). The
size is then 0 if the guest has TCR_EL2.TG0 set to 16kB or 64kB.

kvm_invalidate_vncr_ipa() and invalidate_vncr_va() feed that 0 straight
into the range computation. When the size is 0 the alignment mask is 0 as
well, both ends of the range collapse to 0, and the first comparison, the
one that decides there is no overlap, is always true. As a result the
pseudo-TLB is skipped whatever range the caller asks for, including the
full IPA range passed by kvm_nested_s2_wp() and kvm_nested_s2_unmap().

kvm_nested_s2_unmap() is called from kvm_unmap_gfn_range(). A guest that
meets the above conditions therefore keeps its VNCR fixmap mapping across
an MMU notifier that reclaims the page, and still holds a writable EL2
alias of that page once it has been freed and reallocated for something
else.

compute_tlb_inval_range() and compute_s1_tlbi_range() already treat a size
of 0 as "no size information" and fall back to a conservative value. Apply
the same principle here, folding the range computation into a helper that
reports an overlap when the size is unknown. This also removes the
computation that was duplicated between the two functions. The result of
the test is unchanged for a non-zero size.

Fixes: ebb2d8fd81b8 ("KVM: arm64: nv: Fix incorrect VNCR invalidation range calculation")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 arch/arm64/kvm/nested.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index dfb96edbdc43c6..3ac8789b458f39 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -985,6 +985,21 @@ static void invalidate_vncr(struct vncr_tlb *vt)
 		clear_fixmap(vncr_fixmap(vt->cpu));
 }
 
+static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,
+				u64 start, u64 end)
+{
+	u64 size, tlb_start, tlb_end;
+
+	size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift, vt->wr.level));
+	if (!size)
+		return true;
+
+	tlb_start = addr & ~(size - 1);
+	tlb_end = tlb_start + size;
+
+	return !(tlb_end <= start || tlb_start >= end);
+}
+
 /*
  * VNCR TLB invalidation occurs from MMU notifiers or TLBI instructions, and
  * either can race against a vcpu not being onlined yet (no pseudo-TLB
@@ -1008,14 +1023,7 @@ static void kvm_invalidate_vncr_ipa(struct kvm *kvm, u64 start, u64 end)
 		return;
 
 	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
-		u64 ipa_start, ipa_end, ipa_size;
-
-		ipa_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift,
-							    vt->wr.level));
-		ipa_start = vt->wr.pa & ~(ipa_size - 1);
-		ipa_end = ipa_start + ipa_size;
-
-		if (ipa_end <= start || ipa_start >= end)
+		if (!vncr_tlb_intersects(vt, vt->wr.pa, start, end))
 			continue;
 
 		invalidate_vncr(vt);
@@ -1045,28 +1053,21 @@ static void invalidate_vncr_va(struct kvm *kvm,
 	lockdep_assert_held_write(&kvm->mmu_lock);
 
 	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
-		u64 va_start, va_end, va_size;
-
-		va_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift,
-							   vt->wr.level));
-		va_start = vt->gva & ~(va_size - 1);
-		va_end = va_start + va_size;
-
 		switch (scope->type) {
 		case TLBI_ALL:
 			break;
 
 		case TLBI_VA:
-			if (va_end <= scope->va ||
-			    va_start >= (scope->va + scope->size))
+			if (!vncr_tlb_intersects(vt, vt->gva, scope->va,
+						 scope->va + scope->size))
 				continue;
 			if (vt->wr.nG && vt->wr.asid != scope->asid)
 				continue;
 			break;
 
 		case TLBI_VAA:
-			if (va_end <= scope->va ||
-			    va_start >= (scope->va + scope->size))
+			if (!vncr_tlb_intersects(vt, vt->gva, scope->va,
+						 scope->va + scope->size))
 				continue;
 			break;
 
-- 
2.43.0

Re: [PATCH] KVM: arm64: nv: Don't skip VNCR invalidation when the TLB size is unknown

[Fuad Tabba]

Hi Hyunwoo,

On Mon, 27 Jul 2026 at 17:26, Hyunwoo Kim <imv4bel@gmail.com> wrote:
>
> ttl_to_size() returns 0 when no size is defined for the (granule, level)
> pair. That is the case for level 0 with 4kB pages, and for levels 0 and 1
> with 16kB and 64kB pages. For a VNCR pseudo-TLB this happens when the
> guest runs with SCTLR_EL2.M cleared, because __kvm_translate_va() returns
> success with wr->level left at S1_MMU_DISABLED, and that s8 value becomes
> level 1 as it goes through the u8 parameter of pgshift_level_to_ttl(). The
> size is then 0 if the guest has TCR_EL2.TG0 set to 16kB or 64kB.
>
> kvm_invalidate_vncr_ipa() and invalidate_vncr_va() feed that 0 straight
> into the range computation. When the size is 0 the alignment mask is 0 as
> well, both ends of the range collapse to 0, and the first comparison, the
> one that decides there is no overlap, is always true. As a result the
> pseudo-TLB is skipped whatever range the caller asks for, including the
> full IPA range passed by kvm_nested_s2_wp() and kvm_nested_s2_unmap().
>
> kvm_nested_s2_unmap() is called from kvm_unmap_gfn_range(). A guest that
> meets the above conditions therefore keeps its VNCR fixmap mapping across
> an MMU notifier that reclaims the page, and still holds a writable EL2
> alias of that page once it has been freed and reallocated for something
> else.
>
> compute_tlb_inval_range() and compute_s1_tlbi_range() already treat a size
> of 0 as "no size information" and fall back to a conservative value. Apply
> the same principle here, folding the range computation into a helper that
> reports an overlap when the size is unknown. This also removes the
> computation that was duplicated between the two functions. The result of
> the test is unchanged for a non-zero size.
>
> Fixes: ebb2d8fd81b8 ("KVM: arm64: nv: Fix incorrect VNCR invalidation range calculation")

The tag looks right to me. Before that commit the mask was "pa & (size
- 1)", so with a size of 0 both ends collapsed onto pa itself rather
than onto 0, and the full-range unmap still overlapped. So the missed
invalidation only starts at v6.17.


> Cc: stable@vger.kernel.org
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  arch/arm64/kvm/nested.c | 39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index dfb96edbdc43c6..3ac8789b458f39 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -985,6 +985,21 @@ static void invalidate_vncr(struct vncr_tlb *vt)
>                 clear_fixmap(vncr_fixmap(vt->cpu));
>  }
>
> +static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,
> +                               u64 start, u64 end)
> +{
> +       u64 size, tlb_start, tlb_end;
> +
> +       size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift, vt->wr.level));
> +       if (!size)
> +               return true;

I was wondering if this deserves a short comment. It is the whole
point of the patch, and on its own an early return from a function
called vncr_tlb_intersects() reads as if an unknown size meant "no
overlap" rather than "assume the worst".

With that and with the caveat that I am not very familiar with this
code, but traced this while reviewing (and testing):

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

Cheers,
/fuad




> +
> +       tlb_start = addr & ~(size - 1);
> +       tlb_end = tlb_start + size;
> +
> +       return !(tlb_end <= start || tlb_start >= end);
> +}
> +
>  /*
>   * VNCR TLB invalidation occurs from MMU notifiers or TLBI instructions, and
>   * either can race against a vcpu not being onlined yet (no pseudo-TLB
> @@ -1008,14 +1023,7 @@ static void kvm_invalidate_vncr_ipa(struct kvm *kvm, u64 start, u64 end)
>                 return;
>
>         kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
> -               u64 ipa_start, ipa_end, ipa_size;
> -
> -               ipa_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift,
> -                                                           vt->wr.level));
> -               ipa_start = vt->wr.pa & ~(ipa_size - 1);
> -               ipa_end = ipa_start + ipa_size;
> -
> -               if (ipa_end <= start || ipa_start >= end)
> +               if (!vncr_tlb_intersects(vt, vt->wr.pa, start, end))
>                         continue;
>
>                 invalidate_vncr(vt);
> @@ -1045,28 +1053,21 @@ static void invalidate_vncr_va(struct kvm *kvm,
>         lockdep_assert_held_write(&kvm->mmu_lock);
>
>         kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
> -               u64 va_start, va_end, va_size;
> -
> -               va_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift,
> -                                                          vt->wr.level));
> -               va_start = vt->gva & ~(va_size - 1);
> -               va_end = va_start + va_size;
> -
>                 switch (scope->type) {
>                 case TLBI_ALL:
>                         break;
>
>                 case TLBI_VA:
> -                       if (va_end <= scope->va ||
> -                           va_start >= (scope->va + scope->size))
> +                       if (!vncr_tlb_intersects(vt, vt->gva, scope->va,
> +                                                scope->va + scope->size))
>                                 continue;
>                         if (vt->wr.nG && vt->wr.asid != scope->asid)
>                                 continue;
>                         break;
>
>                 case TLBI_VAA:
> -                       if (va_end <= scope->va ||
> -                           va_start >= (scope->va + scope->size))
> +                       if (!vncr_tlb_intersects(vt, vt->gva, scope->va,
> +                                                scope->va + scope->size))
>                                 continue;
>                         break;
>
> --
> 2.43.0
>