Re: [PATCH 6/7] KVM: arm64: Set IL for nested SError injection

[Oliver Upton <oupton@kernel.org>]

On Sun, Jun 14, 2026 at 04:44:16PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The constructed SError syndrome (ESR) is silently dropped during nested EL2 exception injection because `ESR_EL2` is never updated.
> - [High] When SCTLR2_EL1_EASE routes a Synchronous External abort to an SError, the ESR injected has the wrong EC (IABT/DABT instead of SERROR) and an incorrect ISS format.
> --
> 
> commit 09fd06d56ab3236a0e7af03e0d026ed6725ad6d6
> Author: Fuad Tabba <tabba@google.com>
> 
> KVM: arm64: Set IL for nested SError injection
> 
> This patch modifies the emulated SError injection to unconditionally
> set the IL (Instruction Length) bit in the generated ESR, as mandated
> by the architecture.
> 
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > index 4b39363cf8911..4262d4c17a877 100644
> > --- a/arch/arm64/kvm/emulate-nested.c
> > +++ b/arch/arm64/kvm/emulate-nested.c
> > @@ -2938,6 +2938,6 @@ int kvm_inject_nested_serror(struct kvm_vcpu *vcpu, u64 esr)
> >  	 * vSError injection. Manually populate EC for an emulated SError
> >  	 * exception.
> >  	 */
> > -	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;
> >  	return kvm_inject_nested(vcpu, esr, except_type_serror);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does the carefully constructed
> esr value actually get written to the guest's ESR_EL2?
> 
> Looking at kvm_inject_nested(), it passes the esr_el2 argument down:
> 
> kvm_inject_nested(vcpu, esr_el2, type) {
>     ...
>     kvm_inject_el2_exception(vcpu, esr_el2, type);
>     ...
> }
> 
> However, in kvm_inject_el2_exception():
> 
> static void kvm_inject_el2_exception(...) {
>     ...
>     switch (type) {
>     ...
>     case except_type_serror:
>         kvm_pend_exception(vcpu, EXCEPT_AA64_EL2_SERR);
>         break;
>     ...
> }
> 
> Since except_type_serror only pends the exception and doesn't explicitly
> write the esr_el2 argument into ESR_EL2 (unlike except_type_sync), does the
> nested hypervisor observe a stale or uninitialized syndrome when it takes
> the SError?

Oops, this definitely needs fixing :) OTOH...

> [Severity: High]
> This is a pre-existing issue, but since this code deals with fixing SError
> syndromes, what happens when SCTLR2_ELx.EASE is enabled and a Synchronous
> External abort is routed to an SError?
> 
> Looking at kvm_inject_nested_sea() in the same file:
> 
> int kvm_inject_nested_sea(struct kvm_vcpu *vcpu) {
>     ...
>     u64 esr = FIELD_PREP(ESR_ELx_EC_MASK,
>                          iabt ? ESR_ELx_EC_IABT_LOW : ESR_ELx_EC_DABT_LOW);
>     ...
>     if (__vcpu_sys_reg(vcpu, SCTLR2_EL2) & SCTLR2_EL1_EASE)
>         return kvm_inject_nested(vcpu, esr, except_type_serror);
>     ...
> }
> 
> Does this result in an SError being injected with a Data/Instruction Abort
> EC? According to the ARM Architecture Reference Manual (D8.2.5), shouldn't
> an SError always have its ESR_ELx.EC field set to 0b101111 (SError)?
> 
> A similar pattern appears in inject_abt64() for EL1, where a DABT/IABT
> syndrome is constructed and actively written to exception_esr_elx(vcpu)
> even though an SError exception was pended. Would this incorrect EC format
> confuse the guest's exception handlers?

This is confidently wrong, down to citations of the Arm ARM.

While I couldn't find an explicit rule to cite in my cursory search of
the manual, the definition of the bit describes this as "External Abort
to SError Vector". On top of that, AArch64.DataAbort() just changes the
vector offset (but not the EC) based on EASE.

My understanding is that the exception syndrome of an external abort is
preserved despite taking the SError vector.

Thanks,
Oliver