Re: [PATCH v3 24/28] KVM: arm64: Engage NV3 ERET trap elision

[Marc Zyngier <maz@kernel.org>]

On Wed, 22 Jul 2026 09:50:34 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] NV3 ERET trap elision (HCRX_EL2_NVTGE) is evaluated statically at vCPU initialization, failing to reflect dynamic changes to HCR_EL2.E2H by the guest, resulting in a severe performance regression.
> - [Low] Typographical error in the newly added comment for NV3.
> --
> 
> --- Patch [24]: [PATCH v3 24/28] KVM: arm64: Engage NV3 ERET trap elision ---
> commit c8d6c5654353c481c8b8e9e0a6741ae11cc31c69
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: Engage NV3 ERET trap elision
> 
> This patch enables ERET trap elision for L1 contexts on NV3 hardware by
> setting HCRX_EL2.NVTGE, and adjusts the EL2 fast-path to bounce back to EL1
> when NV3 and E2H are active, assuming the hardware handles the fast path.
> 
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> > index c562d8171d5e1..b32870a5e1236 100644
> > --- a/arch/arm64/include/asm/kvm_emulate.h
> > +++ b/arch/arm64/include/asm/kvm_emulate.h
> [ ... ]
> > @@ -706,6 +706,16 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
> >  
> >  		if (kvm_has_feat(kvm, ID_AA64ISAR1_EL1, LS64, LS64_V))
> >  			vcpu->arch.hcrx_el2 |= HCRX_EL2_EnASR;
> > +
> > +		/*
> > +		 * NV3 is a host-specific extension, and we always use it
> > +		 * when present and that the guest uses NV. It may be be
> 
> [Severity: Low]
> This isn't a bug, but there is a typo (be be) in this comment.

Fixed.

> 
> > +		 * hidden from the guest though.
> > +		 */
> > +		if (cpus_have_final_cap(ARM64_HAS_NV3) &&
> > +		    vcpu_has_nv(vcpu) && vcpu_el2_e2h_is_set(vcpu)) {
> > +			vcpu->arch.hcrx_el2 |= HCRX_EL2_NVTGE;
> > +		}
> 
> [Severity: High]
> Does evaluating the dynamic guest state vcpu_el2_e2h_is_set() here work correctly
> given this populates the static vcpu->arch.hcrx_el2 cache?
>
> If the guest hypervisor (L1) initializes and dynamically enables HCR_EL2.E2H
> after KVM has already initialized the vCPU via kvm_calculate_traps(), this
> HCRX_EL2_NVTGE bit won't be set because vcpu_set_hcrx() is not refreshed
> dynamically.

vcpu_el2_e2h_is_set() is a fixed value. Always. It cannot change.

> 
> This causes the hardware to fail to engage trap elision, forcing the ERET
> instruction to trap to EL2 where it encounters the new check below.
> 
> > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> > index cbda9e087db39..4ed745bdde418 100644
> > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > @@ -345,6 +345,10 @@ static bool kvm_hyp_handle_eret(struct kvm_vcpu *vcpu, u64 *exit_code)
> >  	u64 esr = kvm_vcpu_get_esr(vcpu);
> >  	u64 spsr, elr, mode;
> >  
> > +	/* With NV3, the fast path is handled in HW */
> > +	if (cpus_have_final_cap(ARM64_HAS_NV3) && vcpu_el2_e2h_is_set(vcpu))
> > +		return false;
> > +
> 
> [Severity: High]
> Will this unconditionally abort the fast path and force a severe performance
> regression if the hardware trap elision wasn't engaged?

As the comment says: we never get there if we're not *already* on the
slow path. That's the spec. Read the spec.

> 
> Because the HCRX_EL2_NVTGE bit was evaluated statically in vcpu_set_hcrx()
> and missed the guest's dynamic update, the ERET traps into this function.
> 
> kvm_hyp_handle_eret() evaluates the dynamic E2H state, correctly observes
> that it is now set, and returns false under the incorrect assumption that
> the hardware handled the fast path. This bounces the trap to the slow host
> EL1 path, forcing a full, expensive context switch to emulate every L1 ERET.

Yet another Artificial Idiot nonsense.

	M.

-- 
Without deviation from the norm, progress is not possible.