Re: [GIT PULL v1 15/23] KVM: s390: ucontrol: Fix sca_clear_ext_call()
[email protected] Mon, 03 Aug 2026 17:09:38 +0000
| Newsgroups | org.kernel.vger.kvm,org.kernel.vger.linux-s390 |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: - [Medium] UCONTROL VM external call injection fails or succeeds inconsiste= ntly depending on the host's hardware ESCA support. -- --- Patch [15]: [GIT PULL v1 15/23] KVM: s390: ucontrol: Fix sca_clear_ext_= call() --- commit 4cf53d54c2a9bad47bf30203ae51ce4109c4612e Author: Claudio Imbrenda <[email protected]> KVM: s390: ucontrol: Fix sca_clear_ext_call() This commit addresses an issue where cleaning up a UCONTROL VM would touch memory outside of the allocated ESCA block. It fixes this by avoiding ESCA access for UCONTROL VMs and fencing the KVM_S390_INTERRUPT ioctl for them. > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c > index 2acdff130fa6c..6b3f97a7513b3 100644 > --- a/arch/s390/kvm/interrupt.c > +++ b/arch/s390/kvm/interrupt.c > @@ -45,13 +45,16 @@ static struct kvm_s390_gib *gib; > static int sca_ext_call_pending(struct kvm_vcpu *vcpu, int *src_id) > { > struct esca_block *sca =3D vcpu->kvm->arch.sca; > - union esca_sigp_ctrl sigp_ctrl =3D sca->cpu[vcpu->vcpu_id].sigp_ctrl; > + union esca_sigp_ctrl sigp_ctrl; > =20 > if (!kvm_s390_test_cpuflags(vcpu, CPUSTAT_ECALL_PEND)) > return 0; > + if (kvm_is_ucontrol(vcpu->kvm)) > + return 0; [Severity: Medium] Will this cause pending software external calls to be ignored on hosts with ESCA? If the host supports ESCA and kvm_s390_use_sca_entries() is true, the system will rely on sca_ext_call_pending(). By returning 0 here for UCONTROL VMs, won't any pending software external calls be ignored? [ ... ] > @@ -60,13 +63,16 @@ static int sca_ext_call_pending(struct kvm_vcpu *vcpu= , int *src_id) > =20 > static int sca_inject_ext_call(struct kvm_vcpu *vcpu, int src_id) > { > - struct esca_block *sca =3D vcpu->kvm->arch.sca; > - union esca_sigp_ctrl *sigp_ctrl =3D &sca->cpu[vcpu->vcpu_id].sigp_ctrl; > union esca_sigp_ctrl old_val, new_val =3D {.scn =3D src_id, .c =3D 1}; > + struct esca_block *sca =3D vcpu->kvm->arch.sca; > + union esca_sigp_ctrl *sigp_ctrl; > int expect, rc; > =20 > BUG_ON(!kvm_s390_use_sca_entries()); > + if (kvm_is_ucontrol(vcpu->kvm)) > + return -EINVAL; [Severity: Medium] Does this create an ABI inconsistency where UCONTROL VMs behave differently based on the host's hardware capabilities? When userspace calls the KVM_S390_IRQ ioctl on a UCONTROL VM to inject KVM_S390_INT_EXTERNAL_CALL, __inject_extcall() checks if the host supports ESCA: arch/s390/kvm/interrupt.c:__inject_extcall() { ... if (kvm_s390_use_sca_entries() && !kvm_s390_pv_cpu_get_handle(vcpu)) return sca_inject_ext_call(vcpu, src_id); ... } If the host doesn't support ESCA, it bypasses sca_inject_ext_call() and successfully queues the interrupt via the software fallback.=20 But if the host does support ESCA, it calls sca_inject_ext_call() which will now hit this check and return -EINVAL instead of queuing it. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803160924.2368= [email protected]?part=3D15