[PATCH 0/6] KVM: arm64: VNCR TLB invalidation fixes

[PATCH 0/6] KVM: arm64: VNCR TLB invalidation fixes

[Marc Zyngier]

Prompted by a patch [0] from Hyunwoo Kim which was addressing a pretty
annoying corner case, I spent some time looking at what was wrong in
our VNCR TLBI code. And there was a few things to say about it...

- The TLB tracking to get into the slow path is wrong. It tracks what
  is mapped, but not the established TLBs. Kind of annoying.

- Handling IPA invalidation when the TLB was established with the S1
  MMU disabled is failing for a number of reasons. This is the bug
  that Hyunwoo Kim found, but I decided to address it at its root
  rather than just fixing the symptoms.

- Deciding to run with a VNCR TLB doesn't take the state of
  SCTLR_EL2.M into account. Yes, this is a special sort of sport to
  run a hypervisor with its MMU disabled...

- TLB invalidation by VA targeting the last page/block of TTBR1_EL2 is
  dropped on the floor, because I cannot count. This was amusing. Not.

I have fixes for each of these issues, all stable candidates. On top
of that, I have a couple of patches reintroducing the tracking that
the first patch removes, this time in a way that is actually
functional. Or at least I think it is...

I'd like to thank Hyunwoo Kim for their initial patch and for
providing a reproducer that helped me finding these issues by running
it at multiple levels of nesting. It is most probably too late for
7.2, so let's try to make this 7.3 material.

[0] https://lore.kernel.org/r/ameGoxbn2wzBq2kL@v4bel

Marc Zyngier (6):
  KVM: arm64: Remove VM-wide VNCR mapping counter
  KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation
  KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
  KVM: arm64: Correctly handle end of VA space TLBI invalidation
  KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation
  KVM: arm64: Add VNCR TLB tracking again

 arch/arm64/include/asm/kvm_host.h   |   4 +-
 arch/arm64/include/asm/kvm_nested.h |   7 ++
 arch/arm64/kvm/at.c                 |   2 -
 arch/arm64/kvm/hyp/vhe/switch.c     |   4 +-
 arch/arm64/kvm/nested.c             | 109 +++++++++++++++++-----------
 5 files changed, 78 insertions(+), 48 deletions(-)

-- 
2.47.3

[PATCH 1/6] KVM: arm64: Remove VM-wide VNCR mapping counter

[Marc Zyngier]

The global VNCR mapping counter is used to decide whether an L1
provided VNCR page is mapped in L0 on any CPU at the point of
dealing with a TLB invalidation. It is incremented when a mapping
is made in the fixmap, and decremented when unmapped.

As it turns out, this tracking has several flaws:

- we are trying to invalidate TLBs, and the mapping is only an
  opportunistic consequence of the TLB. Checking this counter to
  decide whether a TLB needs to be invalidated may result in missed
  invalidations.

- an L1 vcpu invalidating its own TLB (a very likely case) will not
  succeed in invalidating the VNCR pseudo TLB because that page is
  not mapped in L0 at this stage.

Given that this tracking fails at delivering the minimum guarantees
that are required and is only a performance optimisation, remove it
completely.

Fixes: 4ffa72ad8f37e ("KVM: arm64: nv: Add S1 TLB invalidation primitive for VNCR_EL2")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/include/asm/kvm_host.h | 3 ---
 arch/arm64/kvm/hyp/vhe/switch.c   | 3 +--
 arch/arm64/kvm/nested.c           | 3 ---
 3 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index bae2c4f92ef5c..ac16f96c878d6 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -411,9 +411,6 @@ struct kvm_arch {
 	/* Masks for VNCR-backed and general EL2 sysregs */
 	struct kvm_sysreg_masks	*sysreg_masks;
 
-	/* Count the number of VNCR_EL2 currently mapped */
-	atomic_t vncr_map_count;
-
 	/*
 	 * For an untrusted host VM, 'pkvm.handle' is used to lookup
 	 * the associated pKVM instance in the hypervisor.
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index bbe9cebd3d9d5..c09b1d411c584 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -427,8 +427,7 @@ static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code)
 	 * If we have to check for any VNCR mapping being invalidated,
 	 * go back to the slow path for further processing.
 	 */
-	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu) &&
-	    atomic_read(&vcpu->kvm->arch.vncr_map_count))
+	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))
 		return false;
 
 	__kvm_skip_instr(vcpu);
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index dfb96edbdc43c..f3c75954cf36c 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -48,7 +48,6 @@ void kvm_init_nested(struct kvm *kvm)
 {
 	kvm->arch.nested_mmus = NULL;
 	kvm->arch.nested_mmus_size = 0;
-	atomic_set(&kvm->arch.vncr_map_count, 0);
 }
 
 static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
@@ -890,7 +889,6 @@ static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
 	clear_fixmap(vncr_fixmap(vcpu->arch.vncr_tlb->cpu));
 	vcpu->arch.vncr_tlb->cpu = -1;
 	host_data_clear_flag(L1_VNCR_MAPPED);
-	atomic_dec(&vcpu->kvm->arch.vncr_map_count);
 }
 
 void kvm_vcpu_put_hw_mmu(struct kvm_vcpu *vcpu)
@@ -1592,7 +1590,6 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
 	if (pgprot_val(prot) != pgprot_val(PAGE_NONE)) {
 		__set_fixmap(vncr_fixmap(vt->cpu), vt->hpa, prot);
 		host_data_set_flag(L1_VNCR_MAPPED);
-		atomic_inc(&vcpu->kvm->arch.vncr_map_count);
 	}
 }
 
-- 
2.47.3

Re: [PATCH 1/6] KVM: arm64: Remove VM-wide VNCR mapping counter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The fast-path TLBI handler (`kvm_hyp_handle_tlbi_el2`) fails to invalidate the `vncr_tlb` for EL2 TLBI instructions when the guest hypervisor has `E2H=0` or `TGE=0`.
--

--- Patch [1]: [PATCH 1/6] KVM: arm64: Remove VM-wide VNCR mapping counter ---
commit 47ea057c11b9c9218fbfb24b57715b00bd1a72d7
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Remove VM-wide VNCR mapping counter

This commit removes the global VNCR mapping counter used to decide if a VNCR
page is mapped in L0 during TLB invalidations. The tracking was deemed flawed
and only served as a performance optimization, so it is completely removed.

[ ... ]

> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index bbe9cebd3d9d5..c09b1d411c584 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -427,8 +427,7 @@ static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code)
>  	 * If we have to check for any VNCR mapping being invalidated,
>  	 * go back to the slow path for further processing.
>  	 */
> -	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu) &&
> -	    atomic_read(&vcpu->kvm->arch.vncr_map_count))
> +	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))

[Severity: High]
This is a pre-existing issue, but does this fast-path logic inadvertently
skip invalidating the vncr_tlb for EL2 TLBI instructions when E2H=0 or
TGE=0?

Since kvm_hyp_handle_tlbi_el2() only falls back to the slow path
(returning false) when vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)
is true, it seems that for a non-VHE guest hypervisor or a VHE guest hypervisor
preparing an L2 environment (TGE=0), this returns true and bypasses the slow
path.

The slow path handle_tlbi_el2() in arch/arm64/kvm/sys_regs.c normally calls
kvm_handle_s1e2_tlbi() to handle vncr_tlb invalidation. By bypassing this,
could the vncr_tlb retain stale translations, causing KVM to map the VNCR
fixmap to the wrong guest physical page and corrupting the nested guest's
memory?

>  		return false;
>  
>  	__kvm_skip_instr(vcpu);
> 
>  	return true;

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

Re: [PATCH 1/6] KVM: arm64: Remove VM-wide VNCR mapping counter

[Marc Zyngier]

On Sat, 01 Aug 2026 14:08:05 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The fast-path TLBI handler (`kvm_hyp_handle_tlbi_el2`)
> fails to invalidate the `vncr_tlb` for EL2 TLBI instructions when
> the guest hypervisor has `E2H=0` or `TGE=0`.

- There is no guest provided VNCR when E2H=0

- A TLBI that targets the EL2&0 translation regime must have TGE==1

	M.

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

[PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation

[Marc Zyngier]

Computing the effects of a TLB invalidation involves looking at
the size of the mapping cached by the TLB. For S1 mappings such as
VNCR, this is deducted from the combination of the base granule size
and the mapping level.

However, this implies that the S1 MMU is *on*. When the MMU is off,
we indicate this with the level being set to a "creative" value of
-127 (S1_MMU_DISABLED).

This ends-up being misinterpreted by pgshift_level_to_ttl() as it
doesn't handle negative levels at all (the level is immediately cast
to a u8 and only the bottom two bits considered), leading to an
invalidation size of 0. Not helpful.

Tidy-up pgshift_level_to_ttl() to handle these negative levels, and
ttl_to_size() to always return SZ_1G when no valid TTL is present.
This allows the removal of open-coded checks for similar situations.

Note that the check for a negative value not explicitely checking for
S1_MMU_DISABLED is deliberate, so that actual negative levels introduced
with LVA2 and D128 can take the same path if we ever support them.

Fixes: 7270cc9157f47 ("KVM: arm64: nv: Handle VNCR_EL2 invalidation from MMU notifiers")
Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Link: https://lore.kernel.org/r/ameGoxbn2wzBq2kL@v4bel
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/nested.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index f3c75954cf36c..035cda256e2a5 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -505,7 +505,7 @@ int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
 	return ret;
 }
 
-static unsigned int ttl_to_size(u8 ttl)
+static unsigned int __ttl_to_size(u8 ttl)
 {
 	int level = ttl & 3;
 	int gran = (ttl >> 2) & 3;
@@ -561,10 +561,22 @@ static unsigned int ttl_to_size(u8 ttl)
 	return max_size;
 }
 
-static u8 pgshift_level_to_ttl(u16 shift, u8 level)
+static unsigned int ttl_to_size(u8 ttl)
+{
+	return __ttl_to_size(ttl) ?: SZ_1G;
+}
+
+static u8 pgshift_level_to_ttl(u16 shift, s8 level)
 {
 	u8 ttl;
 
+	/*
+	 * If we don't have a proper level, fallback to the maximum
+	 * size.
+	 */
+	if (level < 0)
+		return 0;
+
 	switch(shift) {
 	case 12:
 		ttl = TLBI_TTL_TG_4K;
@@ -675,7 +687,11 @@ unsigned long compute_tlb_inval_range(struct kvm_s2_mmu *mmu, u64 val)
 		ttl = get_guest_mapping_ttl(mmu, addr);
 	}
 
-	max_size = ttl_to_size(ttl);
+	/*
+	 * Don't use the default 1GB fallback, as we can adapt to the
+	 * max mapping size we allow at S2.
+	 */
+	max_size = __ttl_to_size(ttl);
 
 	if (!max_size) {
 		/* Compute the maximum extent of the invalidation */
@@ -1124,8 +1140,6 @@ static void compute_s1_tlbi_range(struct kvm_vcpu *vcpu, u32 inst, u64 val,
 	case OP_TLBI_VALE1OSNXS:
 		scope->type = TLBI_VA;
 		scope->size = ttl_to_size(FIELD_GET(TLBI_TTL_MASK, val));
-		if (!scope->size)
-			scope->size = SZ_1G;
 		scope->va = tlbi_va_s1_to_va(val) & ~(scope->size - 1);
 		scope->asid = FIELD_GET(TLBIR_ASID_MASK, val);
 		break;
@@ -1152,8 +1166,6 @@ static void compute_s1_tlbi_range(struct kvm_vcpu *vcpu, u32 inst, u64 val,
 	case OP_TLBI_VAALE1OSNXS:
 		scope->type = TLBI_VAA;
 		scope->size = ttl_to_size(FIELD_GET(TLBI_TTL_MASK, val));
-		if (!scope->size)
-			scope->size = SZ_1G;
 		scope->va = tlbi_va_s1_to_va(val) & ~(scope->size - 1);
 		break;
 	case OP_TLBI_RVAE2:
-- 
2.47.3

Re: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Stage-1 huge block mappings (e.g., 4TB, 64GB) are incorrectly truncated to 1GB due to a 32-bit return type and a hardcoded fallback, leading to under-invalidation of the VNCR software cache.
--

--- Patch [2]: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation ---
commit 55a3817e6985fb11462e68c9b396c2d3ee056461
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation

This patch fixes a bug where negative Stage-1 mapping levels (used to indicate
S1_MMU_DISABLED) were mishandled by pgshift_level_to_ttl(), leading to an
invalidation size of 0. It introduces __ttl_to_size() and ensures ttl_to_size()
always falls back to SZ_1G when no valid TTL size is found.

Link: https://lore.kernel.org/r/ameGoxbn2wzBq2kL@v4bel

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 983ba0adaf4e6..b026d160f253d 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c

[ ... ]

> @@ -506,7 +506,7 @@ int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
>  	return ret;
>  }
>  
> -static unsigned int ttl_to_size(u8 ttl)
> +static unsigned int __ttl_to_size(u8 ttl)

[Severity: High]
This is a pre-existing issue, but does this unsigned int return type cause
huge block mapping sizes to be incorrectly truncated?

Since __ttl_to_size() and ttl_to_size() return a 32-bit unsigned int, they
cannot represent sizes 4GB or larger (like 4TB for 64K granule Level 1, or
64GB for 16K granule Level 1).

> @@ -562,10 +562,22 @@ static unsigned int ttl_to_size(u8 ttl)
>  	return max_size;
>  }
>  
> -static u8 pgshift_level_to_ttl(u16 shift, u8 level)
> +static unsigned int ttl_to_size(u8 ttl)
> +{
> +	return __ttl_to_size(ttl) ?: SZ_1G;
> +}

If a nested L1 guest maps its VNCR page in such a huge block using 52-bit
VAs and later issues a TLBI with a VA far from the VNCR page VA, KVM will
calculate a severely undersized invalidation window. Because the true size
overflows unsigned int (or evaluates to 0), the fallback restricts it to 1GB.

This could leave KVM using a stale IPA, potentially corrupting nested guest
state by writing into unmapped or reassigned guest physical memory.

Should these size calculation functions be returning an unsigned long or u64
instead?

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

Re: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation

[Marc Zyngier]

On Sat, 01 Aug 2026 14:02:35 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Stage-1 huge block mappings (e.g., 4TB, 64GB) are incorrectly truncated to 1GB due to a 32-bit return type and a hardcoded fallback, leading to under-invalidation of the VNCR software cache.
> --
> 
> --- Patch [2]: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation ---
> commit 55a3817e6985fb11462e68c9b396c2d3ee056461
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation
> 
> This patch fixes a bug where negative Stage-1 mapping levels (used to indicate
> S1_MMU_DISABLED) were mishandled by pgshift_level_to_ttl(), leading to an
> invalidation size of 0. It introduces __ttl_to_size() and ensures ttl_to_size()
> always falls back to SZ_1G when no valid TTL size is found.
> 
> Link: https://lore.kernel.org/r/ameGoxbn2wzBq2kL@v4bel
> 
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index 983ba0adaf4e6..b026d160f253d 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> 
> [ ... ]
> 
> > @@ -506,7 +506,7 @@ int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
> >  	return ret;
> >  }
> >  
> > -static unsigned int ttl_to_size(u8 ttl)
> > +static unsigned int __ttl_to_size(u8 ttl)
> 
> [Severity: High]
> This is a pre-existing issue, but does this unsigned int return type cause
> huge block mapping sizes to be incorrectly truncated?
> 
> Since __ttl_to_size() and ttl_to_size() return a 32-bit unsigned int, they
> cannot represent sizes 4GB or larger (like 4TB for 64K granule Level 1, or
> 64GB for 16K granule Level 1).

Which is adequate since we don't support these sizes.

> 
> > @@ -562,10 +562,22 @@ static unsigned int ttl_to_size(u8 ttl)
> >  	return max_size;
> >  }
> >  
> > -static u8 pgshift_level_to_ttl(u16 shift, u8 level)
> > +static unsigned int ttl_to_size(u8 ttl)
> > +{
> > +	return __ttl_to_size(ttl) ?: SZ_1G;
> > +}
> 
> If a nested L1 guest maps its VNCR page in such a huge block using 52-bit
> VAs and later issues a TLBI with a VA far from the VNCR page VA, KVM will
> calculate a severely undersized invalidation window. Because the true size
> overflows unsigned int (or evaluates to 0), the fallback restricts it to 1GB.
> 
> This could leave KVM using a stale IPA, potentially corrupting nested guest
> state by writing into unmapped or reassigned guest physical memory.
> 
> Should these size calculation functions be returning an unsigned long or u64
> instead?

No, we don't support LVA/LVA2 for nested guests.

	M.

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

[PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page

[Marc Zyngier]

We record a VNCR TLB even when SCTLR_EL2.M is 0 in order to make
our life easier. But this is not something that the architecture
anticipate.

As a consequence, a hypervisor is free to set VNCR_EL2 to
some PA when SCTLR_EL2.M==0, use it to run a guest which indirectly
accesses the VNCR page, then eventually set SCTLR_EL2.M==1 with
the same VA. Yes, this is odd, but apparently legal.

A common trick in HW is to invalidate the TLBs on SCTLR_ELx.M being
flipped. But doing this is a not a good idea for us (we'd need to
trap SCTLR accesses), and wouldn't scale as we nest deeper.

Instead, use the fact that the S1 MMU being off at the point of
translation is cached in our TLB, and simply ignore it if it
doesn't match the current MMU state.

Fixes: 2a359e072596f ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/include/asm/kvm_nested.h | 7 +++++++
 arch/arm64/kvm/at.c                 | 2 --
 arch/arm64/kvm/nested.c             | 3 +++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 012d711034d17..cc48817a5bf7a 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -388,6 +388,8 @@ struct s1_walk_result {
 	bool	failed;
 };
 
+#define S1_MMU_DISABLED		(-127)
+
 static inline void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool s1ptw)
 {
 	wr->fst		= fst;
@@ -396,6 +398,11 @@ static inline void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool s1ptw)
 	wr->failed	= true;
 }
 
+static inline bool s1_mmu_disabled(struct s1_walk_result *wr)
+{
+	return wr->level == S1_MMU_DISABLED;
+}
+
 int __kvm_translate_va(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
 		       struct s1_walk_result *wr, u64 va);
 int __kvm_find_s1_desc_level(struct kvm_vcpu *vcpu, u64 va, u64 ipa,
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
index 640f2dc00a8ba..0926426b87989 100644
--- a/arch/arm64/kvm/at.c
+++ b/arch/arm64/kvm/at.c
@@ -11,8 +11,6 @@
 #include <asm/kvm_mmu.h>
 #include <asm/lsui.h>
 
-#define S1_MMU_DISABLED		(-127)
-
 static int get_ia_size(struct s1_walk_info *wi)
 {
 	return 64 - wi->txsz;
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 035cda256e2a5..d7dba02dc84fe 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1578,6 +1578,9 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
 	if (!vt->valid)
 		return;
 
+	if (!(vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_M) != s1_mmu_disabled(&vt->wr))
+		return;
+
 	if (read_vncr_el2(vcpu) != vt->gva)
 		return;
 
-- 
2.47.3

Re: [PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Acquiring `mmu_lock` (an `rwlock_t`) while preemption is disabled causes a sleep-in-atomic bug on PREEMPT_RT kernels.
--

--- Patch [3]: [PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page ---
commit 35765bd8ff67c3b2b00cff5d84e75d6902846a0f
Author: Marc Zyngier <maz@kernel.org>
Subject: KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page

This commit ensures the S1 MMU state is verified against the cached state
in the TLB when mapping the L1 VNCR page. If the cached state does not match
the current MMU state, the mapping is ignored to prevent inconsistent
translations.

[ ... ]

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index b026d160f253d..711c0d0ff8339 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1579,6 +1579,9 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
>  	if (!vt->valid)
>  		return;

[Severity: High]
This isn't a regression introduced by this patch, but looking at the earlier
lines in kvm_map_l1_vncr():

arch/arm64/kvm/nested.c:kvm_map_l1_vncr() {
    ...
	guard(preempt)();
	guard(read_lock)(&vcpu->kvm->mmu_lock);
    ...
}

I noticed the order of these guards causes preemption to be disabled prior to
acquiring mmu_lock (which is an rwlock_t).

On Preempt-RT kernels, rwlock_t is mapped to an rt_rw_lock which is a
sleepable lock. Does acquiring it while preemption is explicitly disabled
trigger a sleep-in-atomic splat here?

Since this path is reachable by a nested guest's normal operations, could this
allow an untrusted guest to panic the host kernel on a Preempt-RT system?

> +	if (!(vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_M) != s1_mmu_disabled(&vt->wr))
> +		return;
> +
>  	if (read_vncr_el2(vcpu) != vt->gva)
>  		return;

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

[PATCH 4/6] KVM: arm64: Correctly handle end of VA space TLBI invalidation

[Marc Zyngier]

Our TLB invalidation by VA code is based on comparing two ranges,
one defined by the TLB, and one defined by the TLBI instruction.

Each range is defined by a start and a size. However, the way the
comparison is done doesn't account for address rollover, as it
compares an address with (base + size). This works nicely until
this expression represent the last page/block in the TTBR1 VA space,
as the result is a big fat 0. And a failed TLB invalidation.

Rewrite the comparison in a way that is immune to the address
rollover (making the end address inclusive instead of exclusive),
and move this into a common helper that is used by both VA and IPA
invalidations, as suggested by Hyunwoo Kim (although the IPA version
didn't suffer from this particular problem, obviously).

Fixes: 4ffa72ad8f37e ("KVM: arm64: nv: Add S1 TLB invalidation primitive for VNCR_EL2")
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/nested.c | 43 ++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index d7dba02dc84fe..47d61d3cf053c 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -999,6 +999,20 @@ 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 scope_start, u64 scope_size)
+{
+	u64 tlb_size, tlb_start, tlb_end, scope_end;
+
+	tlb_size = ttl_to_size(pgshift_level_to_ttl(vt->wi.pgshift, vt->wr.level));
+
+	tlb_start = addr & ~(tlb_size - 1);
+	tlb_end = tlb_start + tlb_size - 1;
+	scope_end = scope_start + scope_size - 1;
+
+	return !(tlb_end < scope_start || tlb_start > scope_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
@@ -1021,19 +1035,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_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)
-			continue;
-
-		invalidate_vncr(vt);
-	}
+	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm)
+		if (vncr_tlb_intersects(vt, vt->wr.pa, start, end - start))
+			invalidate_vncr(vt);
 }
 
 struct s1e2_tlbi_scope {
@@ -1059,28 +1063,19 @@ 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->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->size))
 				continue;
 			break;
 
-- 
2.47.3

Re: [PATCH 4/6] KVM: arm64: Correctly handle end of VA space TLBI invalidation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing sign-extension in `decode_range_tlbi()` breaks Stage-1 range TLB invalidations (e.g., `TLBI RVAE1`) for TTBR1 addresses.
--

--- Patch [4]: [PATCH 4/6] KVM: arm64: Correctly handle end of VA space TLBI invalidation ---
commit 33057eee0c17792f671722b92173b4f67ff0fde6
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Correctly handle end of VA space TLBI invalidation

This patch corrects the overlap check for TLB invalidations by modifying the
comparison logic to be inclusive rather than exclusive. This fixes an issue
where the invalidation would fail if the range ended exactly at the boundary of
the TTBR1 VA space due to address rollover.

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 711c0d0ff8339..69a4652f39336 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c

[ ... ]

> @@ -1060,28 +1064,19 @@ static void invalidate_vncr_va(struct kvm *kvm,
>  	lockdep_assert_held_write(&kvm->mmu_lock);
>  
>  	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
[ ... ]
>  		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->size))

[Severity: High]
This is a pre-existing issue, but does scope->va fail to match TTBR1 addresses
because it is not properly sign-extended?

Looking at compute_s1_tlbi_range() which populates scope->va using
decode_range_tlbi(val, &scope->size, &scope->asid), the base address is
extracted in arch/arm64/include/asm/kvm_nested.h:

	base = (val & GENMASK(36, 0)) << shift;

Because this shifts the bits without sign-extending them to 64 bits, TTBR1
addresses are zero-extended. When vncr_tlb_intersects() compares this positive
scope_start against a correctly sign-extended, negative vt->gva (mapped to
tlb_start), the unsigned comparison (tlb_start > scope_end) evaluates to true.

Could this cause Stage-1 range TLB invalidations (like TLBI RVAE1) to silently
fail for kernel space addresses, leaving stale translations in the nested
guest's TLB?

>  				continue;
>  			if (vt->wr.nG && vt->wr.asid != scope->asid)
>  				continue;
>  			break;

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

Re: [PATCH 4/6] KVM: arm64: Correctly handle end of VA space TLBI invalidation

[Marc Zyngier]

On Sat, 01 Aug 2026 14:03:37 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing sign-extension in `decode_range_tlbi()` breaks
> - Stage-1 range TLB invalidations (e.g., `TLBI RVAE1`) for TTBR1
> - addresses.

Yup, that's yet another bug. I'll queue this:

diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index cc48817a5bf7a..f1ce0d9fd50ef 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -290,6 +290,7 @@ static inline u64 decode_range_tlbi(u64 val, u64 *range, u16 *asid)
 	}
 
 	base	= (val & GENMASK(36, 0)) << shift;
+	base	= (u64)sign_extend64(base, 48);
 
 	if (asid)
 		*asid = FIELD_GET(TLBIR_ASID_MASK, val);

Thanks,

	M.

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

[PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation

[Marc Zyngier]

A mapped VNCR translation must indicate which CPU it is resident.
Unmapping such a translation must make this CPU number -1.

Couple the unmap and the setting to -1 in a helper, so that we
are always sure they are done together.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/nested.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 47d61d3cf053c..84915e2cff604 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -894,6 +894,12 @@ void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
 	}
 }
 
+static void unmap_l1_vncr(struct vncr_tlb *vt)
+{
+	clear_fixmap(vncr_fixmap(vt->cpu));
+	vt->cpu = -1;
+}
+
 static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
 {
 	if (!host_data_test_flag(L1_VNCR_MAPPED))
@@ -902,8 +908,7 @@ static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
 	BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id());
 	BUG_ON(is_hyp_ctxt(vcpu));
 
-	clear_fixmap(vncr_fixmap(vcpu->arch.vncr_tlb->cpu));
-	vcpu->arch.vncr_tlb->cpu = -1;
+	unmap_l1_vncr(vcpu->arch.vncr_tlb);
 	host_data_clear_flag(L1_VNCR_MAPPED);
 }
 
@@ -996,7 +1001,7 @@ static void invalidate_vncr(struct vncr_tlb *vt)
 {
 	vt->valid = false;
 	if (vt->cpu != -1)
-		clear_fixmap(vncr_fixmap(vt->cpu));
+		unmap_l1_vncr(vt);
 }
 
 static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,
-- 
2.47.3

Re: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Critical] By modifying `invalidate_vncr()` to call the new `unmap_l1_vncr()` helper, this patch asynchronously sets `vt->cpu = -1` when invalidating a remote vCPU's VNCR TLB. However, the host CPU's `L1_VNCR_MAPPED` flag remains set on the remote CPU. When the remote vCPU is eventually scheduled out and executes the lockless `this_cpu_reset_vncr_fixmap()`, it observes `L1_VNCR_MAPPED` is true but finds `vt->cpu` has been changed to `-1`. This triggers a deterministic host kernel panic via `BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id())`. A guest can trivially exploit this by issuing a TLBI from one vCPU while another is running.

Pre-existing issues:
- [Critical] In `kvm_map_l1_vncr()`, `vt->cpu` is unconditionally set to `smp_processor_id()`. If the mapping has no read permissions (`prot == PAGE_NONE`), the function returns without mapping the fixmap or setting the `L1_VNCR_MAPPED` flag. When the vCPU is scheduled out, `this_cpu_reset_vncr_fixmap()` checks `L1_VNCR_MAPPED` and returns early, failing to reset `vt->cpu` to `-1`. This leaves a stale CPU ID in `vt->cpu`. If the host later invalidates this vCPU's VNCR TLB, `invalidate_vncr()` reads the stale CPU ID and maliciously clears the fixmap for that CPU. If another vCPU is running on that CPU and using the fixmap slot, its VNCR mapping is unexpectedly destroyed, leading to a host Data Abort kernel panic when accessed. This problem wasn't introduced by this patch, but remains a critical vulnerability.
- [Critical] A Time-of-Check to Time-of-Use (TOCTOU) race exists on `vt->cpu` between `invalidate_vncr()` and the lockless `this_cpu_reset_vncr_fixmap()`. `invalidate_vncr()` executes `if (vt->cpu != -1)` and then calls `unmap_l1_vncr()`, which evaluates `vncr_fixmap(vt->cpu)`. Because `this_cpu_reset_vncr_fixmap()` is called from `kvm_arch_vcpu_put` without holding `kvm->mmu_lock`, it can run concurrently on the target vCPU's host CPU and set `vt->cpu = -1` exactly between the check and the use. `unmap_l1_vncr()` then evaluates `vncr_fixmap(-1)`, triggering `BUG_ON(__c >= NR_CPUS)` and panicking the host. This problem wasn't introduced by this patch, but remains a critical vulnerability.
--

--- Patch [5]: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation ---
commit 9f716e0407bd9a5d530b0e1d1e94c5434eaeedb6
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation

A mapped VNCR translation must indicate which CPU it is resident.
Unmapping such a translation must make this CPU number -1. Couple the unmap
and the setting to -1 in a helper, so that we are always sure they are done
together.

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 69a4652f39336..8d5b0208ad728 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c

[ ... ]

> @@ -895,6 +895,12 @@ void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
>  	}
>  }
>  
> +static void unmap_l1_vncr(struct vncr_tlb *vt)
> +{
> +	clear_fixmap(vncr_fixmap(vt->cpu));
> +	vt->cpu = -1;
> +}
> +
>  static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
>  {
>  	if (!host_data_test_flag(L1_VNCR_MAPPED))
>  		return;

[Severity: Critical]
This is a pre-existing issue, but can a stale CPU ID be left in vt->cpu if 
we return early here?

In kvm_map_l1_vncr(), vt->cpu is unconditionally set to smp_processor_id().
If the mapping has no read permissions (prot == PAGE_NONE), the function
returns without mapping the fixmap or setting the L1_VNCR_MAPPED flag.

When the vCPU is later scheduled out, this_cpu_reset_vncr_fixmap() returns
early at this check because L1_VNCR_MAPPED is unset, failing to reset
vt->cpu to -1.

If the host later invalidates this vCPU's VNCR TLB, invalidate_vncr() reads
the stale CPU ID and maliciously clears the fixmap for that CPU. If another
vCPU is running on that CPU and using the fixmap slot, its VNCR mapping is
unexpectedly destroyed, leading to a host Data Abort when accessed.

>  	BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id());
>  	BUG_ON(is_hyp_ctxt(vcpu));
>  
> -	clear_fixmap(vncr_fixmap(vcpu->arch.vncr_tlb->cpu));
> -	vcpu->arch.vncr_tlb->cpu = -1;
> +	unmap_l1_vncr(vcpu->arch.vncr_tlb);
>  	host_data_clear_flag(L1_VNCR_MAPPED);
>  }
>  
> @@ -997,7 +1002,7 @@ static void invalidate_vncr(struct vncr_tlb *vt)
>  {
>  	vt->valid = false;
>  	if (vt->cpu != -1)

[Severity: Critical]
This is a pre-existing issue, but does this check have a time-of-check to
time-of-use race?

In invalidate_vncr(), we check if vt->cpu != -1 and then call
unmap_l1_vncr(vt), which evaluates vncr_fixmap(vt->cpu) internally.

Because this_cpu_reset_vncr_fixmap() is called from kvm_arch_vcpu_put()
without holding kvm->mmu_lock, it can run concurrently on the target vCPU's
host CPU and set vt->cpu = -1 exactly between the check and the use.

If that happens, unmap_l1_vncr() evaluates vncr_fixmap(-1), which triggers
BUG_ON(__c >= NR_CPUS) and panics the host.

> -		clear_fixmap(vncr_fixmap(vt->cpu));
> +		unmap_l1_vncr(vt);

[Severity: Critical]
Can this trigger a BUG_ON() when a remote vCPU is scheduled out?

By modifying invalidate_vncr() to call unmap_l1_vncr(), this path now
asynchronously sets vt->cpu = -1 when invalidating a remote vCPU's
VNCR TLB. However, the host CPU's L1_VNCR_MAPPED flag remains set on
the remote CPU.

When the remote vCPU is eventually scheduled out, it will execute the
lockless this_cpu_reset_vncr_fixmap():

    if (!host_data_test_flag(L1_VNCR_MAPPED))
        return;

    BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id());

Since L1_VNCR_MAPPED is still true, the code reaches the BUG_ON(), but
vt->cpu has been changed to -1, which deterministicly panics the host
kernel. A guest could trivially trigger this by issuing a TLBI from one
vCPU while another is running.

>  }
>  
>  static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,

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

Re: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation

[Marc Zyngier]

On Sat, 01 Aug 2026 14:02:36 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [Critical] By modifying `invalidate_vncr()` to call the new `unmap_l1_vncr()` helper, this patch asynchronously sets `vt->cpu = -1` when invalidating a remote vCPU's VNCR TLB. However, the host CPU's `L1_VNCR_MAPPED` flag remains set on the remote CPU. When the remote vCPU is eventually scheduled out and executes the lockless `this_cpu_reset_vncr_fixmap()`, it observes `L1_VNCR_MAPPED` is true but finds `vt->cpu` has been changed to `-1`. This triggers a deterministic host kernel panic via `BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id())`. A guest can trivially exploit this by issuing a TLBI from one vCPU while another is running.
> 
> Pre-existing issues:
> - [Critical] In `kvm_map_l1_vncr()`, `vt->cpu` is unconditionally set to `smp_processor_id()`. If the mapping has no read permissions (`prot == PAGE_NONE`), the function returns without mapping the fixmap or setting the `L1_VNCR_MAPPED` flag. When the vCPU is scheduled out, `this_cpu_reset_vncr_fixmap()` checks `L1_VNCR_MAPPED` and returns early, failing to reset `vt->cpu` to `-1`. This leaves a stale CPU ID in `vt->cpu`. If the host later invalidates this vCPU's VNCR TLB, `invalidate_vncr()` reads the stale CPU ID and maliciously clears the fixmap for that CPU. If another vCPU is running on that CPU and using the fixmap slot, its VNCR mapping is unexpectedly destroyed, leading to a host Data Abort kernel panic when accessed. This problem wasn't introduced by this patch, but remains a critical vulnerability.
> - [Critical] A Time-of-Check to Time-of-Use (TOCTOU) race exists on `vt->cpu` between `invalidate_vncr()` and the lockless `this_cpu_reset_vncr_fixmap()`. `invalidate_vncr()` executes `if (vt->cpu != -1)` and then calls `unmap_l1_vncr()`, which evaluates `vncr_fixmap(vt->cpu)`. Because `this_cpu_reset_vncr_fixmap()` is called from `kvm_arch_vcpu_put` without holding `kvm->mmu_lock`, it can run concurrently on the target vCPU's host CPU and set `vt->cpu = -1` exactly between the check and the use. `unmap_l1_vncr()` then evaluates `vncr_fixmap(-1)`, triggering `BUG_ON(__c >= NR_CPUS)` and panicking the host. This problem wasn't introduced by this patch, but remains a critical vulnerability.
> --
> 
> --- Patch [5]: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation ---
> commit 9f716e0407bd9a5d530b0e1d1e94c5434eaeedb6
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation
> 
> A mapped VNCR translation must indicate which CPU it is resident.
> Unmapping such a translation must make this CPU number -1. Couple the unmap
> and the setting to -1 in a helper, so that we are always sure they are done
> together.
> 
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index 69a4652f39336..8d5b0208ad728 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> 
> [ ... ]
> 
> > @@ -895,6 +895,12 @@ void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
> >  	}
> >  }
> >  
> > +static void unmap_l1_vncr(struct vncr_tlb *vt)
> > +{
> > +	clear_fixmap(vncr_fixmap(vt->cpu));
> > +	vt->cpu = -1;
> > +}
> > +
> >  static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
> >  {
> >  	if (!host_data_test_flag(L1_VNCR_MAPPED))
> >  		return;
> 
> [Severity: Critical]
> This is a pre-existing issue, but can a stale CPU ID be left in vt->cpu if 
> we return early here?
> 
> In kvm_map_l1_vncr(), vt->cpu is unconditionally set to smp_processor_id().
> If the mapping has no read permissions (prot == PAGE_NONE), the function
> returns without mapping the fixmap or setting the L1_VNCR_MAPPED flag.
> 
> When the vCPU is later scheduled out, this_cpu_reset_vncr_fixmap() returns
> early at this check because L1_VNCR_MAPPED is unset, failing to reset
> vt->cpu to -1.
> 
> If the host later invalidates this vCPU's VNCR TLB, invalidate_vncr() reads
> the stale CPU ID and maliciously clears the fixmap for that CPU. If another

Maliciously?

> vCPU is running on that CPU and using the fixmap slot, its VNCR mapping is
> unexpectedly destroyed, leading to a host Data Abort when accessed.

It is always possible to unmap an VNCR at any point and to handle the
corresponding abort. That's what a TLBI from another CPU does.

So no, not a bug. The *real* bug is to not correctly handle WO/NONE
permissions, which will have to wait until we have S1POE support in
the kernel.

> 
> >  	BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id());
> >  	BUG_ON(is_hyp_ctxt(vcpu));
> >  
> > -	clear_fixmap(vncr_fixmap(vcpu->arch.vncr_tlb->cpu));
> > -	vcpu->arch.vncr_tlb->cpu = -1;
> > +	unmap_l1_vncr(vcpu->arch.vncr_tlb);
> >  	host_data_clear_flag(L1_VNCR_MAPPED);
> >  }
> >  
> > @@ -997,7 +1002,7 @@ static void invalidate_vncr(struct vncr_tlb *vt)
> >  {
> >  	vt->valid = false;
> >  	if (vt->cpu != -1)
> 
> [Severity: Critical]
> This is a pre-existing issue, but does this check have a time-of-check to
> time-of-use race?
> 
> In invalidate_vncr(), we check if vt->cpu != -1 and then call
> unmap_l1_vncr(vt), which evaluates vncr_fixmap(vt->cpu) internally.
> 
> Because this_cpu_reset_vncr_fixmap() is called from kvm_arch_vcpu_put()
> without holding kvm->mmu_lock, it can run concurrently on the target vCPU's
> host CPU and set vt->cpu = -1 exactly between the check and the use.

This looks indeed like a disaster waiting to happen. I think upgrading
this to an atomic variable should do it, as below.

> 
> If that happens, unmap_l1_vncr() evaluates vncr_fixmap(-1), which triggers
> BUG_ON(__c >= NR_CPUS) and panics the host.
> 
> > -		clear_fixmap(vncr_fixmap(vt->cpu));
> > +		unmap_l1_vncr(vt);
> 
> [Severity: Critical]
> Can this trigger a BUG_ON() when a remote vCPU is scheduled out?
> 
> By modifying invalidate_vncr() to call unmap_l1_vncr(), this path now
> asynchronously sets vt->cpu = -1 when invalidating a remote vCPU's
> VNCR TLB. However, the host CPU's L1_VNCR_MAPPED flag remains set on
> the remote CPU.
> 
> When the remote vCPU is eventually scheduled out, it will execute the
> lockless this_cpu_reset_vncr_fixmap():
> 
>     if (!host_data_test_flag(L1_VNCR_MAPPED))
>         return;
> 
>     BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id());
> 
> Since L1_VNCR_MAPPED is still true, the code reaches the BUG_ON(), but
> vt->cpu has been changed to -1, which deterministicly panics the host
> kernel. A guest could trivially trigger this by issuing a TLBI from one
> vCPU while another is running.

Yup, that BUG_ON() needs to go for the same reason as above. I'm
adding the stuff below (untested).

	M.

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 1e3fd98f6589b..be9d233dffac6 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -27,7 +27,7 @@ struct vncr_tlb {
 	bool			hpa_writable;
 
 	/* -1 when not mapped on a CPU */
-	int			cpu;
+	atomic_t		cpu;
 
 	/*
 	 * true if the TLB is valid. Can only be changed with the
@@ -895,10 +895,17 @@ void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
 	}
 }
 
+/*
+ * Unmapping an L1 VNCR can happen concurrently without holding the mmu lock
+ * (vcpu_put() vs TLBI handling). the atomic_xchg below ensures that only
+ * CPU set -1 while getting a valid CPU number back.
+ */
 static void unmap_l1_vncr(struct vncr_tlb *vt)
 {
-	clear_fixmap(vncr_fixmap(vt->cpu));
-	vt->cpu = -1;
+	int cpu = atomic_xchg_relaxed(&vt->cpu, -1);
+
+	if (cpu != -1)
+		clear_fixmap(vncr_fixmap(cpu));
 }
 
 static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
@@ -906,7 +913,6 @@ static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
 	if (!host_data_test_flag(L1_VNCR_MAPPED))
 		return;
 
-	BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id());
 	BUG_ON(is_hyp_ctxt(vcpu));
 
 	unmap_l1_vncr(vcpu->arch.vncr_tlb);
@@ -1003,8 +1009,7 @@ static void invalidate_vncr(struct kvm *kvm, struct vncr_tlb *vt)
 	BUG_ON(!vt->valid);
 	vt->valid = false;
 	atomic_dec(&kvm->arch.vncr_tlb_count);
-	if (vt->cpu != -1)
-		unmap_l1_vncr(vt);
+	unmap_l1_vncr(vt);
 }
 
 static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,
@@ -1468,7 +1473,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 		vt->hpa = pfn << PAGE_SHIFT;
 		vt->hpa_writable = writable;
 		vt->valid = true;
-		vt->cpu = -1;
+		atomic_set(&vt->cpu, -1);
 
 		atomic_inc(&vcpu->kvm->arch.vncr_tlb_count);
 
@@ -1600,7 +1605,7 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
 	if (vt->wr.nG && get_asid_by_regime(vcpu, TR_EL20) != vt->wr.asid)
 		return;
 
-	vt->cpu = smp_processor_id();
+	atomic_set(&vt->cpu, smp_processor_id());
 
 	if (vt->hpa_writable && vt->wr.pw && vt->wr.pr)
 		prot = PAGE_KERNEL;
@@ -1616,7 +1621,7 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
 	 * FIXME: WO doesn't work at all, need POE support in the kernel.
 	 */
 	if (pgprot_val(prot) != pgprot_val(PAGE_NONE)) {
-		__set_fixmap(vncr_fixmap(vt->cpu), vt->hpa, prot);
+		__set_fixmap(vncr_fixmap(atomic_read(&vt->cpu)), vt->hpa, prot);
 		host_data_set_flag(L1_VNCR_MAPPED);
 	}
 }

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

[PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again

[Marc Zyngier]

Having established that our VNCR TLB tracking was flawed and dropped
it from KVM, it is time to replace it with something that works.

The goal of that tracking is to hit the TLBI slow path if there
are any VNCR TLBs in the guest, irrespective of their mapping state.

For this purpose, we introduce an VM wide counter (vncr_tlb_count)
that tracks how many valid VNCR TLB are present. This means that
creating such TLB must increment the counter, and invalidation
decrement it, and both these operations must be done with the MMU
lock held for write.

On TLBI handling affecting EL2 S1, a non-zero counter forces the
handling to take the slow path to consider the VNCR TLBs.

Not exactly rocket science. Hopefully I got it right this time.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/kvm_host.h |  3 +++
 arch/arm64/kvm/hyp/vhe/switch.c   |  5 +++--
 arch/arm64/kvm/nested.c           | 25 +++++++++++++++++++------
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index ac16f96c878d6..108966a9db12b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -411,6 +411,9 @@ struct kvm_arch {
 	/* Masks for VNCR-backed and general EL2 sysregs */
 	struct kvm_sysreg_masks	*sysreg_masks;
 
+	/* Count the number of VNCR_EL2 TLBs */
+	atomic_t vncr_tlb_count;
+
 	/*
 	 * For an untrusted host VM, 'pkvm.handle' is used to lookup
 	 * the associated pKVM instance in the hypervisor.
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index c09b1d411c584..eb59549ec2172 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -424,10 +424,11 @@ static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code)
 		return false;
 
 	/*
-	 * If we have to check for any VNCR mapping being invalidated,
+	 * If we have to check for any VNCR TLB being invalidated,
 	 * go back to the slow path for further processing.
 	 */
-	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))
+	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu) &&
+	    atomic_read(&vcpu->kvm->arch.vncr_tlb_count))
 		return false;
 
 	__kvm_skip_instr(vcpu);
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 84915e2cff604..1e3fd98f6589b 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -48,6 +48,7 @@ void kvm_init_nested(struct kvm *kvm)
 {
 	kvm->arch.nested_mmus = NULL;
 	kvm->arch.nested_mmus_size = 0;
+	atomic_set(&kvm->arch.vncr_tlb_count, 0);
 }
 
 static int init_nested_s2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
@@ -997,9 +998,11 @@ u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime)
 	return asid;
 }
 
-static void invalidate_vncr(struct vncr_tlb *vt)
+static void invalidate_vncr(struct kvm *kvm, struct vncr_tlb *vt)
 {
+	BUG_ON(!vt->valid);
 	vt->valid = false;
+	atomic_dec(&kvm->arch.vncr_tlb_count);
 	if (vt->cpu != -1)
 		unmap_l1_vncr(vt);
 }
@@ -1042,7 +1045,7 @@ static void kvm_invalidate_vncr_ipa(struct kvm *kvm, u64 start, u64 end)
 
 	kvm_for_each_vncr_tlb(i, vcpu, vt, kvm)
 		if (vncr_tlb_intersects(vt, vt->wr.pa, start, end - start))
-			invalidate_vncr(vt);
+			invalidate_vncr(kvm, vt);
 }
 
 struct s1e2_tlbi_scope {
@@ -1090,7 +1093,7 @@ static void invalidate_vncr_va(struct kvm *kvm,
 			break;
 		}
 
-		invalidate_vncr(vt);
+		invalidate_vncr(kvm, vt);
 	}
 }
 
@@ -1326,13 +1329,20 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
  *   intersects with the TLBI request, invalidate it, and unmap the page
  *   from the fixmap. Because we need to look at all the vcpu-private TLBs,
  *   this requires some wide-ranging locking to ensure that nothing races
- *   against it. This may require some refcounting to avoid the search when
- *   no such TLB is present.
+ *   against it. This requires some refcounting to avoid the search when
+ *   no such TLB is present (see below).
  *
  * - On MMU notifiers, we must invalidate our TLB in a similar way, but
  *   looking at the IPA instead. The funny part is that there may not be a
  *   stage-2 mapping for this page if L1 hasn't accessed it using LD/ST
  *   instructions.
+ *
+ * - vncr_tlb_count tracks the number of valid VNCR TLBs VM-wide. This isn't
+ *   the number of *mapped* L1 VNCR pages, which is likely be a subset (and
+ *   by definition, a TLBI handled from L1 runs with the canonical VNCR
+ *   page, not the L1's). The innermost trap handling code checks this to
+ *   find out whether to return to the guest ASAP (no L1 TLBs) or to visit
+ *   this part of the world for some extra invalidation work.
  */
 
 int kvm_vcpu_allocate_vncr_tlb(struct kvm_vcpu *vcpu)
@@ -1387,7 +1397,8 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 	 */
 	scoped_guard(write_lock, &vcpu->kvm->mmu_lock) {
 		this_cpu_reset_vncr_fixmap(vcpu);
-		vt->valid = false;
+		if (vt->valid)
+			invalidate_vncr(vcpu->kvm, vt);
 
 		vt->wi = (struct s1_walk_info) {
 			.regime	= TR_EL20,
@@ -1459,6 +1470,8 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 		vt->valid = true;
 		vt->cpu = -1;
 
+		atomic_inc(&vcpu->kvm->arch.vncr_tlb_count);
+
 		kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
 		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
 	}
-- 
2.47.3

Re: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Cross-CPU invalidation of VNCR TLBs (e.g., via MMU notifiers) clears the fixmap and resets `vt->cpu` but fails to clear the per-CPU `L1_VNCR_MAPPED` flag on the CPU where the vCPU is running. When the vCPU is subsequently scheduled out, `this_cpu_reset_vncr_fixmap()` checks the local flag, finds it set, and hits a `BUG_ON` because `vt->cpu` is `-1`. This allows unprivileged host userspace to panic the host kernel.
- [High] The fast-path check in `kvm_hyp_handle_tlbi_el2()` skips VNCR TLB invalidation for EL2 TLBI instructions when `E2H` or `TGE` is 0, leaving stale translations in the software VNCR TLB.
- [High] A TOCTOU race exists between lockless guest page table walks and guest TLBI intercepts, allowing stale VNCR translations to survive invalidation.
--

--- Patch [6]: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again ---
commit bde680578e8485472440ba4858d93cb92173897d
Author: Marc Zyngier <maz@kernel.org>
Date:   Sat Aug 1 12:48:18 2026 +0000

    KVM: arm64: Add VNCR TLB tracking again
    
    This patch re-introduces VNCR TLB tracking to force EL2 S1 TLBI handling
    onto the slow path when valid VNCR TLBs exist. It uses a VM-wide atomic
    counter to track the presence of valid VNCR TLBs, updating it under the
    MMU lock.

> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index c09b1d411c584..eb59549ec2172 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -424,10 +424,11 @@ static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code)
>  		return false;
>  
>  	/*
> -	 * If we have to check for any VNCR mapping being invalidated,
> +	 * If we have to check for any VNCR TLB being invalidated,
>  	 * go back to the slow path for further processing.
>  	 */
> -	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))
> +	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu) &&
> +	    atomic_read(&vcpu->kvm->arch.vncr_tlb_count))
>  		return false;

[Severity: High]
This isn't a bug introduced by this patch, but does this check skip VNCR TLB
invalidation for EL2 TLBI instructions when the guest is in nVHE mode?

If a guest running with E2H=0 changes its EL2 page tables for the VNCR_EL2
mapping and issues an EL2 TLBI, this condition evaluates to false.

The fast-path will return true, skipping the slow path that would normally
handle the invalidation. Could this leave stale translations in the software
VNCR TLB?

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 8d5b0208ad728..2a7dd9f9fb597 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
[ ... ]
> @@ -998,9 +999,11 @@ u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime)
>  	return asid;
>  }
>  
> -static void invalidate_vncr(struct vncr_tlb *vt)
> +static void invalidate_vncr(struct kvm *kvm, struct vncr_tlb *vt)
>  {
> +	BUG_ON(!vt->valid);
>  	vt->valid = false;
> +	atomic_dec(&kvm->arch.vncr_tlb_count);
>  	if (vt->cpu != -1)
>  		unmap_l1_vncr(vt);
>  }

[Severity: Critical]
This is a pre-existing issue, but does this cross-CPU invalidation path leave
the per-CPU L1_VNCR_MAPPED flag set locally?

If an MMU notifier on CPU B calls kvm_invalidate_vncr_ipa() which reaches
invalidate_vncr(), unmap_l1_vncr() will clear the fixmap and set vt->cpu = -1.

When CPU A (which was running the vCPU) exits the guest and calls
this_cpu_reset_vncr_fixmap(), won't it see its local L1_VNCR_MAPPED flag still
set and trigger BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id()) because
vt->cpu is now -1?

[ ... ]
> @@ -1460,6 +1471,8 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>  		vt->valid = true;
>  		vt->cpu = -1;
>  
> +		atomic_inc(&vcpu->kvm->arch.vncr_tlb_count);
> +
>  		kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
>  		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
>  	}

[Severity: High]
This is a pre-existing issue, but is there a time-of-check to time-of-use race
between the lockless guest page table walk and guest TLBI intercepts here?

CPU1 locklessly resolves the guest virtual address via __kvm_translate_va()
earlier in kvm_translate_vncr().

Before CPU1 acquires the mmu_lock and increments vncr_tlb_count, CPU2 could
modify the guest page table and issue a TLBI.

The TLBI on CPU2 would see vncr_tlb_count == 0 (or vt->valid == false) and
complete. CPU1 would then acquire mmu_lock, pass the mmu_invalidate_retry()
check (which only tracks host MMU notifiers), and install the stale
translation.

Could this result in caching a stale VNCR mapping?

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

Re: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again

[Marc Zyngier]

On Sat, 01 Aug 2026 14:04:53 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Cross-CPU invalidation of VNCR TLBs (e.g., via MMU notifiers) clears the fixmap and resets `vt->cpu` but fails to clear the per-CPU `L1_VNCR_MAPPED` flag on the CPU where the vCPU is running. When the vCPU is subsequently scheduled out, `this_cpu_reset_vncr_fixmap()` checks the local flag, finds it set, and hits a `BUG_ON` because `vt->cpu` is `-1`. This allows unprivileged host userspace to panic the host kernel.
> - [High] The fast-path check in `kvm_hyp_handle_tlbi_el2()` skips VNCR TLB invalidation for EL2 TLBI instructions when `E2H` or `TGE` is 0, leaving stale translations in the software VNCR TLB.
> - [High] A TOCTOU race exists between lockless guest page table walks and guest TLBI intercepts, allowing stale VNCR translations to survive invalidation.
> --
> 
> --- Patch [6]: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again ---
> commit bde680578e8485472440ba4858d93cb92173897d
> Author: Marc Zyngier <maz@kernel.org>
> Date:   Sat Aug 1 12:48:18 2026 +0000
> 
>     KVM: arm64: Add VNCR TLB tracking again
>     
>     This patch re-introduces VNCR TLB tracking to force EL2 S1 TLBI handling
>     onto the slow path when valid VNCR TLBs exist. It uses a VM-wide atomic
>     counter to track the presence of valid VNCR TLBs, updating it under the
>     MMU lock.
> 
> > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> > index c09b1d411c584..eb59549ec2172 100644
> > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > @@ -424,10 +424,11 @@ static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code)
> >  		return false;
> >  
> >  	/*
> > -	 * If we have to check for any VNCR mapping being invalidated,
> > +	 * If we have to check for any VNCR TLB being invalidated,
> >  	 * go back to the slow path for further processing.
> >  	 */
> > -	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))
> > +	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu) &&
> > +	    atomic_read(&vcpu->kvm->arch.vncr_tlb_count))
> >  		return false;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does this check skip VNCR TLB
> invalidation for EL2 TLBI instructions when the guest is in nVHE
> mode?

nVHE and NV are mutually exclusive in KVM.

[...]

> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index 8d5b0208ad728..2a7dd9f9fb597 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> [ ... ]
> > @@ -998,9 +999,11 @@ u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime)
> >  	return asid;
> >  }
> >  
> > -static void invalidate_vncr(struct vncr_tlb *vt)
> > +static void invalidate_vncr(struct kvm *kvm, struct vncr_tlb *vt)
> >  {
> > +	BUG_ON(!vt->valid);
> >  	vt->valid = false;
> > +	atomic_dec(&kvm->arch.vncr_tlb_count);
> >  	if (vt->cpu != -1)
> >  		unmap_l1_vncr(vt);
> >  }
> 
> [Severity: Critical]
> This is a pre-existing issue, but does this cross-CPU invalidation path leave
> the per-CPU L1_VNCR_MAPPED flag set locally?
> 
> If an MMU notifier on CPU B calls kvm_invalidate_vncr_ipa() which reaches
> invalidate_vncr(), unmap_l1_vncr() will clear the fixmap and set vt->cpu = -1.
> 
> When CPU A (which was running the vCPU) exits the guest and calls
> this_cpu_reset_vncr_fixmap(), won't it see its local L1_VNCR_MAPPED flag still
> set and trigger BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id()) because
> vt->cpu is now -1?

Addressed in my reply to the comment on the previous patch.

> 
> [ ... ]
> > @@ -1460,6 +1471,8 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
> >  		vt->valid = true;
> >  		vt->cpu = -1;
> >  
> > +		atomic_inc(&vcpu->kvm->arch.vncr_tlb_count);
> > +
> >  		kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
> >  		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
> >  	}
> 
> [Severity: High]
> This is a pre-existing issue, but is there a time-of-check to time-of-use race
> between the lockless guest page table walk and guest TLBI intercepts here?
> 
> CPU1 locklessly resolves the guest virtual address via __kvm_translate_va()
> earlier in kvm_translate_vncr().
> 
> Before CPU1 acquires the mmu_lock and increments vncr_tlb_count, CPU2 could
> modify the guest page table and issue a TLBI.
> 
> The TLBI on CPU2 would see vncr_tlb_count == 0 (or vt->valid == false) and
> complete. CPU1 would then acquire mmu_lock, pass the mmu_invalidate_retry()
> check (which only tracks host MMU notifiers), and install the stale
> translation.
> 
> Could this result in caching a stale VNCR mapping?

Yup. That's definitely another problem.

I think we need something that like the mmu notifier retry
mechanism. We can hoist the increment of vcpu_tlb_count to the
beginning of the translation process, sample a counter, and recheck
this counter once we've acquired the mmu lock.

I wonder if we can actually use the exact same counter outside of the
MMU notifiers, bumping it up from the TLBI VA code... Something like
the untested hack below.

	M.

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index dfdd1b4abb839..73c7bd4223f13 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1010,6 +1010,8 @@ static void invalidate_vncr(struct kvm *kvm, struct vncr_tlb *vt)
 	vt->valid = false;
 	atomic_dec(&kvm->arch.vncr_tlb_count);
 	unmap_l1_vncr(vt);
+	kvm->mmu_invalidate_seq++;
+	smp_wmb();
 }
 
 static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,
@@ -1475,8 +1477,6 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 		vt->valid = true;
 		atomic_set(&vt->cpu, -1);
 
-		atomic_inc(&vcpu->kvm->arch.vncr_tlb_count);
-
 		kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
 		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
 	}
@@ -1530,7 +1530,17 @@ int kvm_handle_vncr_abort(struct kvm_vcpu *vcpu)
 		return -EIO;
 	}
 
+	/*
+	 * Speculatively increment the TLB count to make sure concurrent
+	 * TLBIs will take the slow path, and will interact with the retry
+	 * mechanism. Drop it again on error.
+	 */
+	atomic_inc(&vcpu->kvm->arch.vncr_tlb_count);
+
 	ret = kvm_translate_vncr(vcpu, &is_gmem);
+	if (ret)
+		atomic_dec(&vcpu->kvm->arch.vncr_tlb_count);
+
 	switch (ret) {
 	case -EAGAIN:
 		/* Let's try again... */

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