Re: [PATCH v2 16/20] KVM: s390: arm64: Implement exception injection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Synthesized ESR_ELx for injected exceptions (Aborts and Unknown exceptions) conditionally sets the IL bit, violating architectural requirements.
--

commit 0f2bf8b10fedf69578d14536aed0e0dc39a725b4
Author: Steffen Eiden <seiden@linux.ibm.com>

KVM: s390: arm64: Implement exception injection

This commit adds core exception entry and fault injection mechanisms for the
s390 KVM arm64 architecture, enabling guest exception handling.

> diff --git a/arch/s390/kvm/arm64/inject_fault.c b/arch/s390/kvm/arm64/inject_fault.c
> index 425dbeaa421c6..7ecf91f86d901 100644
> --- a/arch/s390/kvm/arm64/inject_fault.c
> +++ b/arch/s390/kvm/arm64/inject_fault.c
[ ... ]
> +static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long addr)
> +{
> +	unsigned long cpsr = *vcpu_cpsr(vcpu);
> +	bool is_aarch32 = vcpu_mode_is_32bit(vcpu);
> +	u64 esr = 0;
> +
> +	if (kvm_vcpu_abt_iss1tw(vcpu))
> +		esr |= ESR_ELx_FSC_SEA_TTW(kvm_vcpu_abt_gltl(vcpu));
> +	else
> +		esr |= ESR_ELx_FSC_EXTABT;
> +
> +	pend_sync_exception(vcpu);
> +
> +	if (kvm_vcpu_trap_il_is32bit(vcpu))
> +		esr |= ESR_ELx_IL;

[Severity: High]
Does this code conditionally omit the IL bit for injected aborts when the 
trapped instruction is 16-bit? 

The ARM architecture requires the IL bit in ESR_ELx to be unconditionally set
to 1 for injected Instruction and Data Aborts, regardless of the trapping 
instruction's length. 

Could this cause the guest kernel to receive an architecturally incorrect 
ESR_ELx value (IL=0 instead of IL=1) and potentially break guest exception 
handlers if it traps on a 16-bit instruction?

> +
> +	if (is_aarch32 || (cpsr & PSR_MODE_MASK) == PSR_MODE_EL0t)
> +		esr |= (ESR_ELx_EC_IABT_LOW << ESR_ELx_EC_SHIFT);
> +	else
> +		esr |= (ESR_ELx_EC_IABT_CUR << ESR_ELx_EC_SHIFT);
[ ... ]
>  void kvm_inject_undefined(struct kvm_vcpu *vcpu)
>  {
> -	/* Stub until s390 supports arm64 sysregs TODO sysregs*/
> +	u64 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT);
> +
> +	/*
> +	 * Build an unknown exception, depending on the instruction
> +	 * set.
> +	 */
> +	if (kvm_vcpu_trap_il_is32bit(vcpu))
> +		esr |= ESR_ELx_IL;

[Severity: High]
Similar to the abort injection above, does this code conditionally omit the IL
bit for an injected Unknown exception? 

The architecture also requires the IL bit to be unconditionally set to 1 for 
Unknown exceptions.

> +
> +	kvm_inject_sync(vcpu, esr);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831145536.913567-1-seiden@linux.ibm.com?part=16