Re: [PATCH 25/28] KVM: arm64: Engage NV3 TLBI trap elision

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Enabling FEAT_NV3 TLBI elision (NVnTTLB) breaks KVM's nested S2 MMU caching, leaving stale L2 guest S1 TLB entries across cached physical VMIDs.
- [High] TLBI trap elision for NV1 guests is nested inside a check requiring the guest to use VHE, rendering the optimization dead code.
- [Medium] NV3 TLBI trap elision is incomplete because HCRX_EL2_NVnTTLBOS is omitted from the TLBI elision mask.

Pre-existing issues:
- [High] vcpu_set_hcrx() statically evaluates the guest's dynamic HCR_EL2.E2H state at boot, permanently disabling HCRX_EL2_NVTGE even if the guest later enables VHE.
--

commit e34c9c8b4a16d0026abc090c888c49e7f1c66d8a
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Engage NV3 TLBI trap elision

This commit enables FEAT_NV3 TLBI trap elision for guests that are not
NV2-aware. It relaxes TLBI traps to only TGE=0 using HCRX_EL2 bits.

> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index b32870a5e1236..d6f432b1558f5 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -715,6 +715,15 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
>  		if (cpus_have_final_cap(ARM64_HAS_NV3) &&
>  		    vcpu_has_nv(vcpu) && vcpu_el2_e2h_is_set(vcpu)) {

[Severity: High]
This isn't a bug introduced by this patch, but because vcpu_set_hcrx is
called once during kvm_calculate_traps at the first VCPU run, the guest
has not yet executed and its virtual HCR_EL2.E2H is 0. 

Since vcpu_el2_e2h_is_set checks the dynamic HCR_EL2 register, does this
evaluate to false and permanently disable HCRX_EL2_NVTGE even if the guest
later sets HCR_EL2.E2H at runtime?

[Severity: High]
Does checking vcpu_el2_e2h_is_set here prevent the NV1 TLBI elision
optimization from ever being reached?

Since an NV1 guest does not have VHE enabled (E2H=0), this outer condition
will evaluate to false, meaning the inner !kvm_has_nv2(vcpu->kvm) block
added below will be skipped.

>  			vcpu->arch.hcrx_el2 |= HCRX_EL2_NVTGE;
> +
> +			/*
> +			 * If the guest is NV2-capable, then we need to see
> +			 * all the TLBIs, as configured in HCR_EL2.
> +			 * Otherwise, relax the TLBI traps to only TGE=0.
> +			 */
> +			if (!kvm_has_nv2(vcpu->kvm))
> +				vcpu->arch.hcrx_el2 |= (HCRX_EL2_NVnTTLB  |
> +							HCRX_EL2_NVnTTLBIS);

[Severity: Medium]
Does this mask omit the Outer Shareable S1 TLBI elision bit
(HCRX_EL2_NVnTTLBOS)?

Without it, will Outer Shareable S1 TLBI instructions continue to
trap to KVM, missing out on the intended performance optimization?

[Severity: Critical]
Does enabling hardware TLBI elision here break KVM's nested S2 MMU
caching?

KVM caches multiple S2 MMU contexts (with distinct physical VMIDs) for
the same virtual VMID, and defers physical VMID switches until guest entry.

If a guest hypervisor writes to VTTBR_EL2 and executes an S1 TLBI natively,
hardware will only invalidate the currently active physical VMID.

Could this cause the hardware to invalidate the wrong context (the
hypervisor's own S1 TLB instead of the L2 guest's) and miss other cached
physical VMIDs sharing the virtual VMID, potentially leading to stale
TLB entries?

>  		}
>  	}
>  }

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