* [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes
@ 2026-08-06 9:10 Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 1/8] KVM: arm64: Remove VM-wide VNCR mapping counter Marc Zyngier
` (7 more replies)
0 siblings, 8 replies; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs
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, it 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.
Additionally, Sashiko kindly provided some extra horrors to look at:
- A missing sign extension for range invalidation results in the wrong
VA range being considered.
- TLBI doesn't participate in the general MMU invalidation retry
machinery, meaning that VNCR faults and TLBIs race in an
uncontrolled way.
- TLBI and vcpu_put() can race badly, leading to a TOCTOU pattern
which results in either a BUG_ON() or a call to vncr_fixmap(-1).
I have fixes for each of these issues, all stable candidates.
On top of that, I have a patch 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 now too late for 7.2, so let's try to make this 7.3 material
(hence the early repost).
* From v1 [1]
- Added sign extension to range invalidation (Sashiko)
- Use mmu_invalidate_seq to resolve fault vs TLBI races (Sashiko)
- Speculatively bump the TLB refcount on fault to make sure the TLBI
takes the slow path when we are going for a S1 walk
- Clarify the check for the TLB matching for the S1 MMU state
- Extra sanitisation added on the S2 invalidation path
- Applied RBs from Yao Yuan to the patches that didn't change, with
thanks.
[0] https://lore.kernel.org/r/ameGoxbn2wzBq2kL@v4bel
[1] https://lore.kernel.org/r/20260801124818.366274-1-maz@kernel.org
Marc Zyngier (8):
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: Handle VNCR TLB invalidation race with vcpu_put() VNCR
unmapping
KVM: arm64: Sign-extend VA for range-based TLBI invalidation
KVM: arm64: Make VNCR invalidation participate in MMU invalidation
retry
KVM: arm64: Add VNCR TLB tracking again
arch/arm64/include/asm/kvm_host.h | 4 +-
arch/arm64/include/asm/kvm_nested.h | 14 +++
arch/arm64/kvm/at.c | 2 -
arch/arm64/kvm/hyp/vhe/switch.c | 10 +-
arch/arm64/kvm/nested.c | 173 +++++++++++++++++++---------
arch/arm64/kvm/sys_regs.c | 11 ++
6 files changed, 155 insertions(+), 59 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/8] KVM: arm64: Remove VM-wide VNCR mapping counter
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
@ 2026-08-06 9:10 ` Marc Zyngier
2026-08-07 16:45 ` Lorenzo Stoakes (ARM)
2026-08-06 9:10 ` [PATCH v2 2/8] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation Marc Zyngier
` (6 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs, stable
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")
Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/8] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 1/8] KVM: arm64: Remove VM-wide VNCR mapping counter Marc Zyngier
@ 2026-08-06 9:10 ` Marc Zyngier
2026-08-07 17:12 ` Lorenzo Stoakes (ARM)
2026-08-06 9:10 ` [PATCH v2 3/8] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page Marc Zyngier
` (5 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs, stable
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/8] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 1/8] KVM: arm64: Remove VM-wide VNCR mapping counter Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 2/8] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation Marc Zyngier
@ 2026-08-06 9:10 ` Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 4/8] KVM: arm64: Correctly handle end of VA space TLBI invalidation Marc Zyngier
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs, stable
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 if it doesn't match the current
MMU state, leave the VNCR unmapped.
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 | 4 ++++
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 012d711034d17..bfed664d823bd 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_walk_translated(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..27bc7ee4b3382 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1578,6 +1578,10 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
if (!vt->valid)
return;
+ /* We cache the MMU state in the TLB. Check that it matches. */
+ if (!!(vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_M) != s1_walk_translated(&vt->wr))
+ return;
+
if (read_vncr_el2(vcpu) != vt->gva)
return;
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/8] KVM: arm64: Correctly handle end of VA space TLBI invalidation
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
` (2 preceding siblings ...)
2026-08-06 9:10 ` [PATCH v2 3/8] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page Marc Zyngier
@ 2026-08-06 9:10 ` Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 5/8] KVM: arm64: Handle VNCR TLB invalidation race with vcpu_put() VNCR unmapping Marc Zyngier
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs, stable
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")
Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>
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 27bc7ee4b3382..8a602d074dbb4 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 5/8] KVM: arm64: Handle VNCR TLB invalidation race with vcpu_put() VNCR unmapping
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
` (3 preceding siblings ...)
2026-08-06 9:10 ` [PATCH v2 4/8] KVM: arm64: Correctly handle end of VA space TLBI invalidation Marc Zyngier
@ 2026-08-06 9:10 ` Marc Zyngier
2026-08-07 6:03 ` Yao Yuan
2026-08-06 9:10 ` [PATCH v2 6/8] KVM: arm64: Sign-extend VA for range-based TLBI invalidation Marc Zyngier
` (2 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs, sashiko-bot,
stable
While VNCR TLB invalidation always occurs under the MMU lock,
vcpu_put() doesn't, while it unmaps the VNCR page.
The problem is that the invalidation evaluates vncr_tlb::cpu to
decide whether an unmapping needs to take place (cpu != -1) before
performing it. On the other hand, this_cpu_reset_vncr_fixmap()
unconditionally unmaps if L1_VNCR_MAPPED is set.
These two obviously can race, with a TOCTOU pattern on the TLBI
path, and a BUG_ON() on the vcpu_put() path. And the two can end-up
calling vncr_fixmap(-1), with extra lethal effects.
Move the reset of vncr_tlb::cpu to -1 to a common function, and make
this update atomic so that only a single thread can reset the field
and perform the corresponding unmap. The vcpu_put() still need to
unconditionally unmap the current VNCR to close another ugly race.
Finally, the assignment of vncr_tlb::cpu is moved to be kept in sync
with the actual mapping, similar to L1_VNCR_MAPPED being set.
Fixes: 7270cc9157f47 ("KVM: arm64: nv: Handle VNCR_EL2 invalidation from MMU notifiers")
Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/r/20260801130237.0FD8F1F00ACA@smtp.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
arch/arm64/kvm/nested.c | 42 +++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 8a602d074dbb4..cf0d45059edbd 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
@@ -894,16 +894,40 @@ void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
}
}
+/*
+ * Unmapping an L1 VNCR can happen concurrently without the mmu lock being
+ * effective (vcpu_put() vs TLBI handling). The atomic_xchg below ensures
+ * that only one CPU sets it to -1 while getting a valid CPU number back.
+ */
+static int unmap_l1_vncr(struct vncr_tlb *vt)
+{
+ int cpu = atomic_xchg_relaxed(&vt->cpu, -1);
+
+ if (cpu != -1)
+ clear_fixmap(vncr_fixmap(cpu));
+
+ return cpu;
+}
+
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));
- clear_fixmap(vncr_fixmap(vcpu->arch.vncr_tlb->cpu));
- vcpu->arch.vncr_tlb->cpu = -1;
+ /*
+ * Unconditionally unmap the local VNCR if we have lost the race
+ * against a concurrent TLBI. Otherwise we could end-up running
+ * another vcpu with VNCR still mapped if the TLBI thread is
+ * preempted between the exchange and the clear_fixmap().
+ *
+ * Note that we do not care about the TLBI nuking the fixmap behind
+ * the back of an running vcpu. This will only generate a fault and
+ * possibly a retranslation.
+ */
+ if (unmap_l1_vncr(vcpu->arch.vncr_tlb) == -1)
+ clear_fixmap(vncr_fixmap(smp_processor_id()));
host_data_clear_flag(L1_VNCR_MAPPED);
}
@@ -995,8 +1019,7 @@ u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime)
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,
@@ -1452,7 +1475,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);
kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
@@ -1583,8 +1606,6 @@ 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();
-
if (vt->hpa_writable && vt->wr.pw && vt->wr.pr)
prot = PAGE_KERNEL;
else if (vt->wr.pr)
@@ -1599,7 +1620,8 @@ 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);
+ atomic_set(&vt->cpu, smp_processor_id());
+ __set_fixmap(vncr_fixmap(atomic_read(&vt->cpu)), vt->hpa, prot);
host_data_set_flag(L1_VNCR_MAPPED);
}
}
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 6/8] KVM: arm64: Sign-extend VA for range-based TLBI invalidation
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
` (4 preceding siblings ...)
2026-08-06 9:10 ` [PATCH v2 5/8] KVM: arm64: Handle VNCR TLB invalidation race with vcpu_put() VNCR unmapping Marc Zyngier
@ 2026-08-06 9:10 ` Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 7/8] KVM: arm64: Make VNCR invalidation participate in MMU invalidation retry Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 8/8] KVM: arm64: Add VNCR TLB tracking again Marc Zyngier
7 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs, sashiko-bot,
stable
When the decode_range_tlbi() helper was moved to be used for S1 TLBIs,
the required sign extension was omitted. Add it.
As a result, special care must be taken to not overflow PA bits when
this is used for S2 invalidation.
Fixes: 85bba00425ae0 ("KVM: arm64: nv: Move TLBI range decoding to a helper")
Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/r/20260801130337.EB2BA1F00AC4@smtp.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
arch/arm64/include/asm/kvm_nested.h | 7 +++++++
arch/arm64/kvm/sys_regs.c | 11 +++++++++++
2 files changed, 18 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index bfed664d823bd..c83be6d0e79ac 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -291,6 +291,13 @@ static inline u64 decode_range_tlbi(u64 val, u64 *range, u16 *asid)
base = (val & GENMASK(36, 0)) << shift;
+ /*
+ * We only deal with at most 48bit VA/IPA, so 48 is where we
+ * sign-extend from. Should we support FEAT_L{VP}A* at some point,
+ * this will need to be revisited.
+ */
+ base = (u64)sign_extend64(base, 48);
+
if (asid)
*asid = FIELD_GET(TLBIR_ASID_MASK, val);
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 5d5c579d45790..797e888bf9394 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -4057,6 +4057,7 @@ static bool handle_ripas2e1is(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
u32 sys_encoding = sys_insn(p->Op0, p->Op1, p->CRn, p->CRm, p->Op2);
u64 vttbr = vcpu_read_sys_reg(vcpu, VTTBR_EL2);
u64 base, range;
+ int pa_bits;
if (!kvm_supported_tlbi_ipas2_op(vcpu, sys_encoding))
return undef_access(vcpu, p, r);
@@ -4068,6 +4069,16 @@ static bool handle_ripas2e1is(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
*/
base = decode_range_tlbi(p->regval, &range, NULL);
+ /*
+ * Ignore TLBIs that start out of PA_bits range, and cap the
+ * invalidation to the [base:bit(PA_bits)] interval.
+ */
+ pa_bits = kvm_get_pa_bits(vcpu->kvm);
+ if (fls64(base) > pa_bits)
+ return true;
+
+ range = min(range, BIT_ULL(pa_bits) - base);
+
kvm_s2_mmu_iterate_by_vmid(vcpu->kvm, get_vmid(vttbr),
&(union tlbi_info) {
.range = {
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 7/8] KVM: arm64: Make VNCR invalidation participate in MMU invalidation retry
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
` (5 preceding siblings ...)
2026-08-06 9:10 ` [PATCH v2 6/8] KVM: arm64: Sign-extend VA for range-based TLBI invalidation Marc Zyngier
@ 2026-08-06 9:10 ` Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 8/8] KVM: arm64: Add VNCR TLB tracking again Marc Zyngier
7 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs, sashiko-bot,
stable
A VNCR TLB invalidation can occur on one vcpu while another vcpu is
faulting in this same page. Without correctly handling this, we can
end up with the following scenario:
- vcpu A walks the PTs to translate VNCR
- before vcpu A is able to grab the MMU lock to insert the TLB,
vcpu B updates the S1 PTs with an invalid entry, and issues
a TLBI S1E2 for this VA
- vcpu A inserts the TLB for something that is now invalid
This isn't a new problem, and we manage S2 by having the MMU notifier
to bump up mmu_invalidate_seq on invalidation so that the fault can be
replayed.
We can perform something similar here, and extend invalidate_vncr_va() to
update the same counter, clearly indicating that the context has
changed under our feet. This is safe as the invalidation always happen
while holding the MMU lock for write, and that we sample the sequence
number before walking S1.
Fixes: 4ffa72ad8f37e ("KVM: arm64: nv: Add S1 TLB invalidation primitive for VNCR_EL2")
Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/r/20260801130454.5D9F11F00AC4@smtp.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
arch/arm64/kvm/nested.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index cf0d45059edbd..550c9bd3dbe7d 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1058,6 +1058,12 @@ 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;
+ /*
+ * Note that invalidating the VNCR on the back of an MMU notifier
+ * doesn't require messing with the invalidation counter for a
+ * parallel walk. The notifier itself will have bumped the counter,
+ * making sure we rewalk.
+ */
kvm_for_each_vncr_tlb(i, vcpu, vt, kvm)
if (vncr_tlb_intersects(vt, vt->wr.pa, start, end - start))
invalidate_vncr(vt);
@@ -1085,6 +1091,15 @@ static void invalidate_vncr_va(struct kvm *kvm,
lockdep_assert_held_write(&kvm->mmu_lock);
+ /*
+ * We might be performing a parallel S1 walk, so bump up the
+ * invalidation counter even in the absence of an actual VNCR TLB
+ * invalidation, as this could indicate that the guest has gone
+ * through a BBM sequence.
+ */
+ kvm->mmu_invalidate_seq++;
+ smp_wmb();
+
kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
switch (scope->type) {
case TLBI_ALL:
@@ -1419,15 +1434,15 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
va = read_vncr_el2(vcpu);
+ mmu_seq = vcpu->kvm->mmu_invalidate_seq;
+ smp_rmb();
+
ret = __kvm_translate_va(vcpu, &vt->wi, &vt->wr, va);
if (ret)
return ret;
write_fault = kvm_is_write_fault(vcpu);
- mmu_seq = vcpu->kvm->mmu_invalidate_seq;
- smp_rmb();
-
gfn = vt->wr.pa >> PAGE_SHIFT;
memslot = gfn_to_memslot(vcpu->kvm, gfn);
if (!memslot) {
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 8/8] KVM: arm64: Add VNCR TLB tracking again
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
` (6 preceding siblings ...)
2026-08-06 9:10 ` [PATCH v2 7/8] KVM: arm64: Make VNCR invalidation participate in MMU invalidation retry Marc Zyngier
@ 2026-08-06 9:10 ` Marc Zyngier
7 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2026-08-06 9:10 UTC (permalink / raw)
To: kvmarm, kvm, linux-arm-kernel
Cc: Steffen Eiden, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Fuad Tabba, Hyunwoo Kim, Yao Yuan, ljs
Having established that our VNCR TLB tracking was flawed and having
dropped it from KVM, it is time to replace it with something that
actually works in order to get some performance back, as the TLBI
overhead is otherwise pretty high.
The goal of such 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. Note that
the bumping up is done "speculatively" in order to make sure that a
concurrent invalidation will be taken on the slow path.
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 | 11 +++++++---
arch/arm64/kvm/nested.c | 36 +++++++++++++++++++++++++------
3 files changed, 41 insertions(+), 9 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..fa776b18a32ca 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -424,10 +424,15 @@ 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,
- * go back to the slow path for further processing.
+ * If we have to check for any VNCR TLB being invalidated, go back
+ * to the slow path for further processing.
+ *
+ * The synchronisation betweem TLBI and walk is provided by the
+ * speculative increment of the TLB counter on walk, and the
+ * invalidation counter. Yes, this is fiddly.
*/
- 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 550c9bd3dbe7d..20af94197a8a7 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)
@@ -1016,10 +1017,12 @@ 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;
unmap_l1_vncr(vt);
+ atomic_dec(&kvm->arch.vncr_tlb_count);
}
static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,
@@ -1066,7 +1069,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 {
@@ -1123,7 +1126,7 @@ static void invalidate_vncr_va(struct kvm *kvm,
break;
}
- invalidate_vncr(vt);
+ invalidate_vncr(kvm, vt);
}
}
@@ -1359,13 +1362,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)
@@ -1420,7 +1430,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,
@@ -1545,7 +1556,20 @@ 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);
+ smp_mb__after_atomic();
+
ret = kvm_translate_vncr(vcpu, &is_gmem);
+ if (ret) {
+ smp_mb__before_atomic();
+ atomic_dec(&vcpu->kvm->arch.vncr_tlb_count);
+ }
+
switch (ret) {
case -EAGAIN:
/* Let's try again... */
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 5/8] KVM: arm64: Handle VNCR TLB invalidation race with vcpu_put() VNCR unmapping
2026-08-06 9:10 ` [PATCH v2 5/8] KVM: arm64: Handle VNCR TLB invalidation race with vcpu_put() VNCR unmapping Marc Zyngier
@ 2026-08-07 6:03 ` Yao Yuan
0 siblings, 0 replies; 12+ messages in thread
From: Yao Yuan @ 2026-08-07 6:03 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, kvm, linux-arm-kernel, Steffen Eiden, Joey Gouly,
Suzuki K Poulose, Oliver Upton, Zenghui Yu, Fuad Tabba,
Hyunwoo Kim, ljs, sashiko-bot, stable
On Thu, Aug 06, 2026 at 10:10:23AM +0800, Marc Zyngier wrote:
> While VNCR TLB invalidation always occurs under the MMU lock,
> vcpu_put() doesn't, while it unmaps the VNCR page.
>
> The problem is that the invalidation evaluates vncr_tlb::cpu to
> decide whether an unmapping needs to take place (cpu != -1) before
> performing it. On the other hand, this_cpu_reset_vncr_fixmap()
> unconditionally unmaps if L1_VNCR_MAPPED is set.
>
> These two obviously can race, with a TOCTOU pattern on the TLBI
> path, and a BUG_ON() on the vcpu_put() path. And the two can end-up
> calling vncr_fixmap(-1), with extra lethal effects.
...
> 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));
>
> - clear_fixmap(vncr_fixmap(vcpu->arch.vncr_tlb->cpu));
> - vcpu->arch.vncr_tlb->cpu = -1;
> + /*
> + * Unconditionally unmap the local VNCR if we have lost the race
> + * against a concurrent TLBI. Otherwise we could end-up running
> + * another vcpu with VNCR still mapped if the TLBI thread is
> + * preempted between the exchange and the clear_fixmap().
NICE CATCH.
I thought about the remain changed code path in this patch
and they have MMU lock w/ preemption disabled. This is one
trick racing.
Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>
> + *
> + * Note that we do not care about the TLBI nuking the fixmap behind
> + * the back of an running vcpu. This will only generate a fault and
> + * possibly a retranslation.
> + */
> + if (unmap_l1_vncr(vcpu->arch.vncr_tlb) == -1)
> + clear_fixmap(vncr_fixmap(smp_processor_id()));
> host_data_clear_flag(L1_VNCR_MAPPED);
> }
>
> @@ -995,8 +1019,7 @@ u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime)
> 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,
> @@ -1452,7 +1475,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);
>
> kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
> kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
> @@ -1583,8 +1606,6 @@ 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();
> -
> if (vt->hpa_writable && vt->wr.pw && vt->wr.pr)
> prot = PAGE_KERNEL;
> else if (vt->wr.pr)
> @@ -1599,7 +1620,8 @@ 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);
> + atomic_set(&vt->cpu, smp_processor_id());
> + __set_fixmap(vncr_fixmap(atomic_read(&vt->cpu)), vt->hpa, prot);
> host_data_set_flag(L1_VNCR_MAPPED);
> }
> }
> --
> 2.47.3
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/8] KVM: arm64: Remove VM-wide VNCR mapping counter
2026-08-06 9:10 ` [PATCH v2 1/8] KVM: arm64: Remove VM-wide VNCR mapping counter Marc Zyngier
@ 2026-08-07 16:45 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 12+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-07 16:45 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, kvm, linux-arm-kernel, Steffen Eiden, Joey Gouly,
Suzuki K Poulose, Oliver Upton, Zenghui Yu, Fuad Tabba,
Hyunwoo Kim, Yao Yuan, stable
Bear with me being verbose here, as this is both nascent review + learning
:)
On Thu, Aug 06, 2026 at 10:10:19AM +0100, Marc Zyngier wrote:
> 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.
Is it largely the self-invalidation mentioned below or are there other cases?
>
> - 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.
Ahh yes this is pretty compelling then!
>
> 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")
> Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
The change LGTM, it neatly removes the described mechanism which is well
evidenced.
Comments below that are largely me talking out loud as I learn things :)
Acked-by: Lorenzo Stoakes (ARM) <ljs@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;
So this seems to be the crux of it - seems to be 'is there any possibility that
we will need to check for VNCR mappings being invalidated?'
Checks:
* vcpu_el2_e2h_is_set() - is the guest host kernel (?)'s hcr_el2.e2h
enabled? From what I gather hcr_el2.e2h is what allows sysreg_EL1 ->
sysreg_EL2 for the host kernel to allow unmodified kernels to run in EL2.
IOW - is the guest host kernel VHE?
* vcpu_el2_tge_is_set() - Similarly tests for the hcr_el2.tge bit - and this
seems to be is 'EL1 -> EL2 redirection on?' - IOW - is this a kernel running
in EL2?
Actually I see in is_hyp_ctxt():
* We are in a hypervisor context if the vcpu mode is EL2 or
* E2H and TGE bits are set. The latter means we are in the user space
* of the VHE kernel. ARMv8.1 ARM describes this as 'InHost'
So I _think_ the combination of the two is checking to see if you're the L0
kernel that _could_ send TLBi's that need to be handled?
Previously it seemed the logic was 'if there are no VNCR mappings present then
we can optimise by short-circuiting the rest of the processing in
kvm_hyp_handle_sysreg_vhe()'.
It seems that the hardware TLBi has been processed by now so it's actually more
like - there's still work to be done maintaining the software TLB and that's
done elsewhere.
As:
static bool kvm_hyp_handle_sysreg_vhe(struct kvm_vcpu *vcpu, u64 *exit_code)
{
if (kvm_hyp_handle_tlbi_el2(vcpu, exit_code)) <- return ->true
return true;
if (kvm_hyp_handle_timer(vcpu, exit_code)) <- this was a TLBi ->false
return true;
if (kvm_hyp_handle_cpacr_el1(vcpu, exit_code)) <- this was a TLBi ->false
return true;
if (kvm_hyp_handle_zcr_el2(vcpu, exit_code)) <- this was a TLBi ->false
return true;
return kvm_hyp_handle_sysreg(vcpu, exit_code); <- this was a TLBi ->false
}
And
static const exit_handler_fn hyp_exit_handlers[] = {
...
[ESR_ELx_EC_SYS64] = kvm_hyp_handle_sysreg_vhe,
...
};
And:
static inline bool kvm_hyp_handle_exit(struct kvm_vcpu *vcpu, u64 *exit_code,
const exit_handler_fn *handlers)
{
exit_handler_fn fn = handlers[kvm_vcpu_trap_get_class(vcpu)]; <- kvm_hyp_handle_sysreg_vhe()
if (fn)
return fn(vcpu, exit_code);
return false;
}
Annnd:
/*
* Return true when we were able to fixup the guest exit and should return to
* the guest, false when we should restore the host state and return to the
* main run loop.
*/
static inline bool __fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code,
const exit_handler_fn *handlers)
{
...
/* Check if there's an exit handler and allow it to handle the exit. */
if (kvm_hyp_handle_exit(vcpu, exit_code, handlers))
goto guest;
exit:
/* Return to the host kernel and handle the exit */
return false;
...
}
Finally in __kvm_vcpu_run_vhe():
do {
/* Jump in the fire! */
(Good track ;)
exit_code = __guest_enter(vcpu);
/* And we're baaack! */
} while (fixup_guest_exit(vcpu, &exit_code));
(With fixup_guest_exit() ultimately calling __fixup_guest_exit().)
And fixup_guest_exit() will return false, meaning the guest isn't
re-entered and instead you go back to the full fat slow path:
int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
{
...
ret = kvm_arm_vcpu_enter_exit(vcpu); <-- does all the above just returned.
...
ret = handle_exit(vcpu, ret);
}
Then there's some more stuff in this fuller fat handle_exit() path:
int handle_exit(struct kvm_vcpu *vcpu, int exception_index)
{
...
switch (exception_index) {
...
case ARM_EXCEPTION_TRAP:
return handle_trap_exceptions(vcpu);
...
}
}
Which then calls kvm_get_exit_handler() which ultimately gets
handle_tlbi_el2() and calls kvm_handle_s1e2_tlbi() in turn and then
invalidate_vncr_va():
static void invalidate_vncr_va(struct kvm *kvm,
struct s1e2_tlbi_scope *scope)
{
...
kvm_for_each_vncr_tlb(i, vcpu, vt, kvm) {
...
invalidate_vncr(vt);
}
}
And:
static void invalidate_vncr(struct vncr_tlb *vt)
{
vt->valid = false;
if (vt->cpu != -1)
clear_fixmap(vncr_fixmap(vt->cpu));
}
Where you are ultimately clearing the fixmap and setting the
vncr_tlb->valid to false.
I think this is all vaguely sane :)
>
> __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);
> }
> }
And it looks like you got all the places that manipulated this :)
>
> --
> 2.47.3
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/8] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation
2026-08-06 9:10 ` [PATCH v2 2/8] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation Marc Zyngier
@ 2026-08-07 17:12 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 12+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-07 17:12 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, kvm, linux-arm-kernel, Steffen Eiden, Joey Gouly,
Suzuki K Poulose, Oliver Upton, Zenghui Yu, Fuad Tabba,
Hyunwoo Kim, Yao Yuan, stable
On Thu, Aug 06, 2026 at 10:10:20AM +0100, Marc Zyngier wrote:
> 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.
So by two's complement -127 is ~0b01111111 + 1 = 0b10000001 = 129
And:
static u8 pgshift_level_to_ttl(u16 shift, u8 level)
{
u8 ttl;
... shift stuff ...
ttl <<= 2;
ttl |= level & 3;
return tll;
}
So ttl |= 1 because of the mask and in ttl_to_size():
static unsigned int ttl_to_size(u8 ttl)
{
int level = ttl & 3;
int gran = (ttl >> 2) & 3;
unsigned int max_size = 0;
switch (gran) {
case TLBI_TTL_TG_4K:
switch (level) {
...
case 1:
max_size = SZ_1G;
break;
...
case TLBI_TTL_TG_16K:
switch (level) {
...
case 1:
break;
...
case TLBI_TTL_TG_64K:
switch (level) {
...
case 1:
/* No 52bit IPA support */
break;
...
}
return max_size;
}
So actually if granularity is TLBI_TTL_TG_4K this will return SZ_1G and 0 in the
other cases unless I'm getting something wrong here?
This is really more a 'maybe worth mentioning in the commit log to be pedantic'
kind of thing :)
IOW you could luck out before with SZ_1G for TLBI_TTL_TG_4K.
>
> 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.
I guess SZ_1G is a reasonable default here?
>
> Note that the check for a negative value not explicitely checking for
NIT: explicitely -> explicitly
> 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;
> +}
Might be worth a comment about the default?
> +
> +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.
> + */
Being a bit pedantic here but I wonder if simply just to say 'Adapt to the max
mapping size allowed at S2' as the fallback is inferred?
> + 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;
Nice that you can eliminate this and the one above!
> scope->va = tlbi_va_s1_to_va(val) & ~(scope->size - 1);
> break;
> case OP_TLBI_RVAE2:
> --
> 2.47.3
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-08-07 17:12 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 9:10 [PATCH v2 0/8] KVM: arm64: VNCR TLB invalidation fixes Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 1/8] KVM: arm64: Remove VM-wide VNCR mapping counter Marc Zyngier
2026-08-07 16:45 ` Lorenzo Stoakes (ARM)
2026-08-06 9:10 ` [PATCH v2 2/8] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation Marc Zyngier
2026-08-07 17:12 ` Lorenzo Stoakes (ARM)
2026-08-06 9:10 ` [PATCH v2 3/8] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 4/8] KVM: arm64: Correctly handle end of VA space TLBI invalidation Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 5/8] KVM: arm64: Handle VNCR TLB invalidation race with vcpu_put() VNCR unmapping Marc Zyngier
2026-08-07 6:03 ` Yao Yuan
2026-08-06 9:10 ` [PATCH v2 6/8] KVM: arm64: Sign-extend VA for range-based TLBI invalidation Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 7/8] KVM: arm64: Make VNCR invalidation participate in MMU invalidation retry Marc Zyngier
2026-08-06 9:10 ` [PATCH v2 8/8] KVM: arm64: Add VNCR TLB tracking again Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox