Re: [PATCH 6/7] KVM: arm64: Set IL for nested 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] 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?

[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?

>  }
>

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