KVM: arm64: VNCR TLB invalidation fixes
Subsystems (4)
Marc Zyngier
maintainercover KVM: arm64: VNCR TLB invalidation fixes
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(-)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH 1/6 KVM: arm64: Remove VM-wide VNCR mapping counter
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.
--- 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(-)
arch/arm64/include/asm/kvm_host.h +0 -3
@@ -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.
arch/arm64/kvm/hyp/vhe/switch.c +1 -2
@@ -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);
arch/arm64/kvm/nested.c +0 -3
@@ -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); } }
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 1/6] KVM: arm64: Remove VM-wide VNCR mapping counter
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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.
[ ... ]
arch/arm64/kvm/hyp/vhe/switch.c static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code) · line 430
Quoted 11 lines
arch/arm64/kvm/hyp/vhe/switch.c +1
-2@@ -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?
Quoted 5 lines
arch/arm64/kvm/hyp/vhe/switch.c +0
-0return false;__kvm_skip_instr (vcpu);return true;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Marc Zyngier
maintainerRe: [PATCH 1/6] KVM: arm64: Remove VM-wide VNCR mapping counter
Quoted 7 lines · On Sat, 01 Aug 2026 14:08:05 +0100, [email protected] 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 thevncr_tlbfor EL2 TLBI instructions when the guest hypervisor hasE2H=0orTGE=0.
- There is no guest provided VNCR when E2H=0
- A TLBI that targets the EL2&0 translation regime must have TGE==1
M.
Signature · 1 line
Jazz isn't dead. It just smells funny.
Re: [PATCH 1/6] KVM: arm64: Remove VM-wide VNCR mapping counter
In reply to Marc ZyngierQuoted 7 lines · 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:
Quoted 8 lines
- 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.
Agree, the easiest way to trigger the issue is run single vcpu L1 and run single vcpu L2. I think it's more harder to hit the issue if the L1 has more vcpus and run L2 w/ more than 1 L2 vcpus.
Quoted 73 lines
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 <[email protected]>--- 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(-)arch/arm64/include/asm/kvm_host.h +0
-3@@ -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. arch/arm64/kvm/hyp/vhe/switch.c +1
-2@@ -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);arch/arm64/kvm/nested.c +0
-4@@ -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
Marc Zyngier
maintainerPATCH 2/6 KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation
In reply to Marc ZyngierComputing 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.
--- arch/arm64/kvm/nested.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
arch/arm64/kvm/nested.c +19 -7
@@ -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 :
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0
[ ... ]
arch/arm64/kvm/nested.c int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa, · line 509
Quoted 6 lines
arch/arm64/kvm/nested.c +1
-1@@ -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).
arch/arm64/kvm/nested.c static unsigned int ttl_to_size(u8 ttl) · line 568
Quoted 9 lines
arch/arm64/kvm/nested.c +4
-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?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Marc Zyngier
maintainerRe: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation
Quoted 38 lines · On Sat, 01 Aug 2026 14:02:35 +0100, [email protected] 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 <[email protected]>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 bypgshift_level_to_ttl(), leading to an invalidation size of 0. It introduces__ttl_to_size()and ensuresttl_to_size()always falls back toSZ_1Gwhen no valid TTL size is found.Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0[ ... ]
arch/arm64/kvm/nested.c int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa, · line 509
Quoted 6 lines
arch/arm64/kvm/nested.c +1
-1@@ -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?
Quoted 3 lines
Since
__ttl_to_size()andttl_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.
Quoted 16 lines
arch/arm64/kvm/nested.c static unsigned int ttl_to_size(u8 ttl) · line 568
Quoted 10 lines
arch/arm64/kvm/nested.c +4
-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.
Quoted 5 lines
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.
Signature · 1 line
Jazz isn't dead. It just smells funny.
Marc Zyngier
maintainerPATCH 3/6 KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
In reply to Marc ZyngierWe 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.
--- 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(-)
arch/arm64/include/asm/kvm_nested.h +7 -0
@@ -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,
arch/arm64/kvm/at.c +0 -2
@@ -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;
arch/arm64/kvm/nested.c +3 -0
@@ -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 ;
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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.
[ ... ]
arch/arm64/kvm/nested.c static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu) · line 1580
Quoted 7 lines
arch/arm64/kvm/nested.c +0
-0@@ -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?
Quoted 5 lines
arch/arm64/kvm/nested.c +3
-0+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 ;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Joey Gouly
reviewerRe: [PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
In reply to Marc ZyngierHi Marc,
Two small comments/suggestions.
Quoted 9 lines · 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_EL2to 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.
Quoted 7 lines
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.
Could you expand/change this a little, something like:
Instead, use the fact that the S1 MMU being off at the point of translation is cached in our VNCR TLB, and if it doesn't match the current MMU state, leave the VNCR unmapped.
Quoted 53 lines
Fixes 2a359e072596f ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")Signed-off-by Marc Zyngier <[email protected]>--- 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(-)arch/arm64/include/asm/kvm_nested.h +7
-0@@ -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,arch/arm64/kvm/at.c +0
-2@@ -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;arch/arm64/kvm/nested.c +0
-0
arch/arm64/kvm/nested.c static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu) · line 1582
Quoted 5 lines
arch/arm64/kvm/nested.c +2
-0@@ -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 ;
A comment here would also be useful for future archaeology (also current reading since the !( .. != ) code is confusing).
Or maybe:
bool mmu_disabled_sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_M) == 0; if (mmu_disabled_sctlr != s1_mmu_disabled(&vt->wr))
arch/arm64/kvm/nested.c static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu) · line 1586
Quoted 4 lines
arch/arm64/kvm/nested.c +1
-0+if (read_vncr_el2 (vcpu) != vt->gva)return ;
Thanks,
Joey
Marc Zyngier
maintainerRe: [PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
Quoted 23 lines · Joey Gouly
Hi Marc,
Two small comments/suggestions.
Quoted 9 lines · 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_EL2to 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.Quoted 7 lines
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.
Quoted 6 lines
Could you expand/change this a little, something like:
Instead, use the fact that the S1 MMU being off at the point of translation is cached in our VNCR TLB, and if it doesn't match the current MMU state, leave the VNCR unmapped.
Yup, that reads better. Thanks.
Quoted 62 lines
Quoted 53 lines
Fixes 2a359e072596f ("KVM: arm64: nv: Handle mapping of VNCR_EL2 at EL2")Signed-off-by Marc Zyngier <[email protected]>--- 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(-)arch/arm64/include/asm/kvm_nested.h +7
-0@@ -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,arch/arm64/kvm/at.c +0
-2@@ -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;arch/arm64/kvm/nested.c +0
-0arch/arm64/kvm/nested.c static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu) · line 1582
Quoted 5 lines
arch/arm64/kvm/nested.c +2
-0@@ -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 ;A comment here would also be useful for future archaeology (also current reading since the !( .. != ) code is confusing).
Quoted 4 lines
Or maybe:
bool mmu_disabled_sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL2) & SCTLR_ELx_M) == 0; if (mmu_disabled_sctlr != s1_mmu_disabled(&vt->wr))
And maybe using positive logic? I'll see what I can do.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Joey Gouly
reviewerRe: [PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
In reply to Marc ZyngierAnother thing I forgot..
Quoted 69 lines · 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_EL2to 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 <[email protected]>--- 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(-)arch/arm64/include/asm/kvm_nested.h +7
-0@@ -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,arch/arm64/kvm/at.c +0
-2@@ -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;arch/arm64/kvm/nested.c +0
-0
arch/arm64/kvm/nested.c static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu) · line 1582
Quoted 5 lines
arch/arm64/kvm/nested.c +2
-0@@ -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 ;
Another thing, should we set valid=false? (Haven't really looked into what that implies..)
Thanks,
Joey
Marc Zyngier
maintainerRe: [PATCH 3/6] KVM: arm64: Consider SCTLR_EL2.M when mapping the L1 VNCR page
Quoted 76 lines · Joey Gouly
Another thing I forgot..
Quoted 72 lines · 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_EL2to 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 <[email protected]>--- 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(-)arch/arm64/include/asm/kvm_nested.h +7
-0@@ -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,arch/arm64/kvm/at.c +0
-2@@ -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;arch/arm64/kvm/nested.c +0
-0@@ -1578,6 +1578,9 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)if (!vt->valid)return ;
Quoted 5 lines
arch/arm64/kvm/nested.c static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu) · line 1582
Quoted 2 lines
arch/arm64/kvm/nested.c +2
-0+if (!(vcpu_read_sys_reg (vcpu,SCTLR_EL2 ) & SCTLR_ELx_M) !=s1_mmu_disabled (&vt->wr)) +return ;Another thing, should we set valid=false? (Haven't really looked into what that implies..)
No. The TLB itself is still valid. We just haven't hit it. Which is why we don't map the page in the fixmap, and wait until we take the fault. When (and if) it happens, we can invalidate it, rewalk, and establish the new TLB.
At this stage, this doesn't bring much. But if one day we need to cache multiple translations, we will want this exact flow.
HTH,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH 4/6 KVM: arm64: Correctly handle end of VA space TLBI invalidation
In reply to Marc ZyngierOur 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).
--- arch/arm64/kvm/nested.c | 43 ++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 24 deletions(-)
arch/arm64/kvm/nested.c +19 -24
@@ -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 ;
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 4/6] KVM: arm64: Correctly handle end of VA space TLBI invalidation
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0
[ ... ]
Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0@@ -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) {
[ ... ]
arch/arm64/kvm/nested.c static void invalidate_vncr_va(struct kvm *kvm, · line 1071
Quoted 8 lines
arch/arm64/kvm/nested.c +1
-2switch (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?
Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0continue ;if (vt->wr.nG && vt->wr.asid != scope->asid)continue ;break ;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Marc Zyngier
maintainerRe: [PATCH 4/6] KVM: arm64: Correctly handle end of VA space TLBI invalidation
Quoted 7 lines · On Sat, 01 Aug 2026 14:03:37 +0100, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing sign-extension indecode_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:
arch/arm64/include/asm/kvm_nested.h +1 -0
@@ -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.
Signature · 1 line
Jazz isn't dead. It just smells funny.
Re: [PATCH 4/6] KVM: arm64: Correctly handle end of VA space TLBI invalidation
In reply to Marc ZyngierQuoted 32 lines · 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 <[email protected]>--- arch/arm64/kvm/nested.c | 43 ++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 24 deletions(-)arch/arm64/kvm/nested.c +2
-0@@ -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)
arch/arm64/kvm/nested.c static void invalidate_vncr(struct vncr_tlb *vt) · line 1013
Quoted 10 lines
arch/arm64/kvm/nested.c +10
-0+{ +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);
Quoted 62 lines
arch/arm64/kvm/nested.c +7
-25+} +/* * 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
Marc Zyngier
maintainerPATCH 5/6 KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation
In reply to Marc ZyngierA 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.
--- arch/arm64/kvm/nested.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
arch/arm64/kvm/nested.c +8 -3
@@ -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,
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0
[ ... ]
Quoted 13 lines
arch/arm64/kvm/nested.c +6
-0@@ -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 ))
Quoted 1 line
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.
arch/arm64/kvm/nested.c static void invalidate_vncr(struct vncr_tlb *vt) · line 1004
Quoted 13 lines
arch/arm64/kvm/nested.c +1
-2BUG_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.
arch/arm64/kvm/nested.c static void invalidate_vncr(struct vncr_tlb *vt) · line 1004
Quoted 2 lines
arch/arm64/kvm/nested.c +1
-1-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.
Quoted 3 lines
arch/arm64/kvm/nested.c +0
-0} static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
Marc Zyngier
maintainerRe: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation
Quoted 52 lines · On Sat, 01 Aug 2026 14:02:36 +0100, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Critical] By modifyinginvalidate_vncr()to call the newunmap_l1_vncr()helper, this patch asynchronously setsvt->cpu = -1when invalidating a remote vCPU's VNCR TLB. However, the host CPU'sL1_VNCR_MAPPEDflag remains set on the remote CPU. When the remote vCPU is eventually scheduled out and executes the locklessthis_cpu_reset_vncr_fixmap(), it observesL1_VNCR_MAPPEDis true but findsvt->cpuhas been changed to-1. This triggers a deterministic host kernel panic viaBUG_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] Inkvm_map_l1_vncr(),vt->cpuis unconditionally set tosmp_processor_id(). If the mapping has no read permissions (prot == PAGE_NONE), the function returns without mapping the fixmap or setting theL1_VNCR_MAPPEDflag. When the vCPU is scheduled out,this_cpu_reset_vncr_fixmap()checksL1_VNCR_MAPPEDand returns early, failing to resetvt->cputo-1. This leaves a stale CPU ID invt->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 onvt->cpubetweeninvalidate_vncr()and the locklessthis_cpu_reset_vncr_fixmap().invalidate_vncr()executesif (vt->cpu != -1)and then callsunmap_l1_vncr(), which evaluatesvncr_fixmap(vt->cpu). Becausethis_cpu_reset_vncr_fixmap()is called fromkvm_arch_vcpu_putwithout holdingkvm->mmu_lock, it can run concurrently on the target vCPU's host CPU and setvt->cpu = -1exactly between the check and the use.unmap_l1_vncr()then evaluatesvncr_fixmap(-1), triggeringBUG_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 <[email protected]>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.
Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0[ ... ]
Quoted 13 lines
arch/arm64/kvm/nested.c +6
-0@@ -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 ))Quoted 1 line
return;
[Severity: Critical]
This is a pre-existing issue, but can a stale CPU ID be left invt->cpuif we return early here?In
kvm_map_l1_vncr(),vt->cpuis unconditionally set tosmp_processor_id(). If the mapping has no read permissions (prot ==PAGE_NONE), the function returns without mapping the fixmap or setting theL1_VNCR_MAPPEDflag.
Quoted 6 lines
When the vCPU is later scheduled out,
this_cpu_reset_vncr_fixmap()returns early at this check becauseL1_VNCR_MAPPEDis unset, failing to resetvt->cputo -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?
Quoted 2 lines
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.
Quoted 19 lines
Quoted 11 lines
arch/arm64/kvm/nested.c +1
-2BUG_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 ); }arch/arm64/kvm/nested.c static void invalidate_vncr(struct vncr_tlb *vt) · line 1004
Quoted 3 lines
arch/arm64/kvm/nested.c +0
-0@@ -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?
Quoted 6 lines
In
invalidate_vncr(), we check ifvt->cpu!= -1 and then callunmap_l1_vncr(vt), which evaluatesvncr_fixmap(vt->cpu)internally.Because
this_cpu_reset_vncr_fixmap()is called fromkvm_arch_vcpu_put()without holdingkvm->mmu_lock, it can run concurrently on the target vCPU's host CPU and setvt->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.
Quoted 23 lines
If that happens,
unmap_l1_vncr()evaluatesvncr_fixmap(-1), which triggersBUG_ON(__c >= NR_CPUS)and panics the host.arch/arm64/kvm/nested.c static void invalidate_vncr(struct vncr_tlb *vt) · line 1005
Quoted 2 lines
arch/arm64/kvm/nested.c +1
-1-clear_fixmap (vncr_fixmap (vt->cpu)); +unmap_l1_vncr (vt);[Severity: Critical]
Can this trigger aBUG_ON()when a remote vCPU is scheduled out?By modifying
invalidate_vncr()to callunmap_l1_vncr(), this path now asynchronously setsvt->cpu= -1 when invalidating a remote vCPU's VNCR TLB. However, the host CPU'sL1_VNCR_MAPPEDflag 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());
Quoted 4 lines
Since
L1_VNCR_MAPPEDis still true, the code reaches theBUG_ON(), butvt->cpuhas 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.
arch/arm64/kvm/nested.c +14 -9
@@ -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 ); } }
Signature · 1 line
Jazz isn't dead. It just smells funny.
Re: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation
In reply to Marc ZyngierQuoted 39 lines · 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 <[email protected]>--- arch/arm64/kvm/nested.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)arch/arm64/kvm/nested.c +7
-2@@ -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 ); }
arch/arm64/kvm/nested.c static void invalidate_vncr(struct vncr_tlb *vt) · line 1004
Quoted 5 lines
arch/arm64/kvm/nested.c +1
-1@@ -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);
Quoted 6 lines
arch/arm64/kvm/nested.c +0
-1} static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr, --2.47.3
Marc Zyngier
maintainerPATCH 6/6 KVM: arm64: Add VNCR TLB tracking again
In reply to Marc ZyngierHaving 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.
--- 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(-)
arch/arm64/include/asm/kvm_host.h +3 -0
@@ -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.
arch/arm64/kvm/hyp/vhe/switch.c +3 -2
@@ -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);
arch/arm64/kvm/nested.c +19 -6
@@ -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); }
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
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 <[email protected]> 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.
Quoted 6 lines
arch/arm64/kvm/hyp/vhe/switch.c +0
-0@@ -424,10 +424,11 @@ static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code)return false;
arch/arm64/kvm/hyp/vhe/switch.c static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code) · line 432
Quoted 10 lines
arch/arm64/kvm/hyp/vhe/switch.c +3
-2/* -* 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?
Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0
[ ... ]
arch/arm64/kvm/nested.c u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 1009
Quoted 13 lines
arch/arm64/kvm/nested.c +3
-1@@ -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?
[ ... ]
arch/arm64/kvm/nested.c static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem) · line 1478
Quoted 9 lines
arch/arm64/kvm/nested.c +2
-0@@ -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?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
Marc Zyngier
maintainerRe: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again
Quoted 28 lines · On Sat, 01 Aug 2026 14:04:53 +0100, [email protected] 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 resetsvt->cpubut fails to clear the per-CPUL1_VNCR_MAPPEDflag 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 aBUG_ONbecausevt->cpuis-1. This allows unprivileged host userspace to panic the host kernel.
- [High] The fast-path check inkvm_hyp_handle_tlbi_el2()skips VNCR TLB invalidation for EL2 TLBI instructions whenE2HorTGEis 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
bde680578e8485472440ba4858d93cb92173897dAuthor: Marc Zyngier <[email protected]> Date: Sat Aug 1 12:48:18 2026 +0000KVM: 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.
Quoted 7 lines
arch/arm64/kvm/hyp/vhe/switch.c +0
-0@@ -424,10 +424,11 @@ static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code)return false;
Quoted 14 lines
arch/arm64/kvm/hyp/vhe/switch.c static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code) · line 432
Quoted 9 lines
arch/arm64/kvm/hyp/vhe/switch.c +3
-2/* -* 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.
[...]
Quoted 23 lines
Quoted 4 lines
arch/arm64/kvm/nested.c +0
-0[ ... ]
arch/arm64/kvm/nested.c u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 1009
Quoted 13 lines
arch/arm64/kvm/nested.c +3
-1@@ -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-CPUL1_VNCR_MAPPEDflag set locally?
Quoted 7 lines
If an MMU notifier on CPU B calls
kvm_invalidate_vncr_ipa()which reachesinvalidate_vncr(),unmap_l1_vncr()will clear the fixmap and setvt->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 localL1_VNCR_MAPPEDflag still set and triggerBUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id())becausevt->cpuis now -1?
Addressed in my reply to the comment on the previous patch.
Quoted 22 lines
[ ... ]
arch/arm64/kvm/nested.c static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem) · line 1478
Quoted 9 lines
arch/arm64/kvm/nested.c +2
-0@@ -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 inkvm_translate_vncr().Before CPU1 acquires the
mmu_lockand incrementsvncr_tlb_count, CPU2 could modify the guest page table and issue a TLBI.
Quoted 6 lines
The TLBI on CPU2 would see
vncr_tlb_count== 0 (orvt->valid== false) and complete. CPU1 would then acquiremmu_lock, pass themmu_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.
arch/arm64/kvm/nested.c +12 -2
@@ -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... */
Signature · 1 line
Jazz isn't dead. It just smells funny.
Re: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again
In reply to Marc ZyngierQuoted 12 lines · 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.
Quoted 4 lines
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.
Quoted 128 lines
Signed-off-by Marc Zyngier <[email protected]>--- 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(-)arch/arm64/include/asm/kvm_host.h +3
-0@@ -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. arch/arm64/kvm/hyp/vhe/switch.c +3
-2@@ -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);arch/arm64/kvm/nested.c +19
-7@@ -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