Re: [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:

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.

[ ... ]

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

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

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

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

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1