Re: [PATCH v2 5/7] KVM: arm64: Set IL for emulated SError injection

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] kvm_inject_nested_sea() passes an Instruction/Data Abort EC instead of an SError EC when FEAT_EASE converts a Synchronous External Abort into an SError.
- [High] The constructed ESR for a nested SError is discarded during injection because kvm_inject_el2_exception() never writes it to the guest's ESR_EL2.
--

commit da891cfcd9b452feacd8b631f8c20dae39b009fc
Author: Fuad Tabba <tabba@google.com>

KVM: arm64: Set IL for emulated SError injection

kvm_inject_serror_esr() constructs an SError syndrome without IL. The
architecture mandates IL=1 for SError unconditionally.

> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index 444d219b02174..d6c4fc16f8795 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -381,7 +381,7 @@ int kvm_inject_serror_esr(struct kvm_vcpu *vcpu, u64 esr)
>  	 */
>  	if (!serror_is_masked(vcpu)) {
>  		pend_serror_exception(vcpu);
> -		esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR);
> +		esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_SERROR) | ESR_ELx_IL;
>  		vcpu_write_sys_reg(vcpu, esr, exception_esr_elx(vcpu));
>  		return 1;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this SError
injection logic, I noticed an issue in how ESR is constructed for nested
SErrors. When SCTLR2_ELx.EASE is set and a Synchronous External Abort is
converted to an SError, does kvm_inject_nested_sea() pass an incorrect
Exception Class?

In arch/arm64/kvm/emulate-nested.c:kvm_inject_nested_sea():

	if (__vcpu_sys_reg(vcpu, SCTLR2_EL2) & SCTLR2_EL1_EASE)
		return kvm_inject_nested(vcpu, esr, except_type_serror);

The esr variable retains the previously constructed IABT/DABT Exception
Class instead of being reconstructed with ESR_ELx_EC_SERROR. Does this
present an invalid syndrome to the nested guest hypervisor, since the
architecture mandates that ESR_ELx.EC must be 0x2F (SError) when an
External abort is treated as an SError? 

Also, does this unconditionally update FAR_EL2, which is architecturally
incorrect for an SError?

[Severity: High]
This is also a pre-existing issue, but there appears to be a problem where
the constructed ESR for a nested SError is discarded during injection. Does
kvm_inject_el2_exception() fail to write the esr_el2 argument to ESR_EL2?

In arch/arm64/kvm/emulate-nested.c:kvm_inject_el2_exception():

	case except_type_serror:
		kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SERR);
		break;

Unlike the synchronous exception case, the esr_el2 argument isn't written
to the guest's ESR_EL2. Since the downstream exception entry path
enter_exception64() does not write ESR_ELx, does this result in the nested
guest hypervisor receiving an SError with a stale, unmodified ESR_EL2?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618121643.4105064-1-tabba@google.com?part=5