Re: [PATCH v4 22/29] KVM: arm64: Add routing for NVHCR_EL2 trap

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The evaluation of HCRX_EL2.NVTGE ignores its architectural dependency on HCR_EL2.E2H, leading to incorrect trap routing for non-VHE guest hypervisors.
--

--- Patch [22]: [PATCH v4 22/29] KVM: arm64: Add routing for NVHCR_EL2 trap ---
commit 007c598e0e2ba89316901f92f901362aec3f3a5b
Author: Marc Zyngier <[email protected]>

KVM: arm64: Add routing for NVHCR_EL2 trap
    
This commit describes the exception routing tables for NVHCR_EL2 accesses
from EL1, taking them to EL2 when HCRX_EL2.NVTGE==0 and HCR_EL2.NV==1.

> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index a6d1a265841f2..ffe307f222e66 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c

[ ... ]

> @@ -588,6 +590,15 @@ static enum trap_behaviour check_mdcr_hpmn(struct kvm_vcpu *vcpu)
>  	return BEHAVE_HANDLE_LOCALLY;
>  }
>  
> +static enum trap_behaviour check_hcr_nv_hcrx_nnvtge(struct kvm_vcpu *vcpu)
> +{
> +	if ((__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_EL2_NV) &&
> +	    !(__vcpu_sys_reg(vcpu, HCRX_EL2) & HCRX_EL2_NVTGE))

[Severity: High]
According to the ARM Architecture Reference Manual, the HCRX_EL2.NVTGE bit
behaves as 0 for all purposes other than reading the value of the bit when
HCR_EL2.E2H is 0.

Does this raw check of HCRX_EL2_NVTGE need to account for the HCR_EL2.E2H
dependency?

If a non-VHE guest hypervisor sets HCR_EL2.E2H = 0, HCR_EL2.NV = 1, and
HCRX_EL2.NVTGE = 1 (which is architecturally valid), it appears KVM might
falsely conclude the trap shouldn't be forwarded. It would then handle it
locally in L0, which injects an UNDEF exception into the L2 guest.

There appears to be a similar pattern in is_nested_nv3_ctxt() in 
arch/arm64/include/asm/kvm_emulate.h where the raw NVTGE bit is evaluated
without factoring in the E2H dependency.

> +		return BEHAVE_FORWARD_RW;
> +
> +	return BEHAVE_HANDLE_LOCALLY;
> +}
> +

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