Re: [PATCH v2 8/8] KVM: s390: Fix KVM_S390_VCPU_FAULT ioctl

[Steffen Eiden]

On Fri, Mar 20, 2026 at 05:15:42PM +0100, Claudio Imbrenda wrote:
> A previous commit changed the behaviour of the KVM_S390_VCPU_FAULT
> ioctl. The current (wrong) implementation will trigger a guest
> addressing exception if the requested address lies outside of a
> memslot, unless the VM is UCONTROL.
> 
> Restore the previous behaviour by open coding the fault-in logic.
> 
> Fixes: 3762e905ec2e ("KVM: s390: use __kvm_faultin_pfn()")
> Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>

Reviewed-by: Steffen Eiden <seiden@xxxxxxxxxxxxx>

But I have a comment on a changed logic. And a nit

> ---
>  arch/s390/kvm/kvm-s390.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index ebcb0ef8835e..62f04931b54d 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -5520,9 +5520,21 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  	}
>  #endif
>  	case KVM_S390_VCPU_FAULT: {
> -		idx = srcu_read_lock(&vcpu->kvm->srcu);
> -		r = vcpu_dat_fault_handler(vcpu, arg, 0);
	in here every vcpu_ucontrol_translate error (incl ENONEMs from
	kvm_s390_mmu_cache_topup) is converted into EREMOTE ...
> -		srcu_read_unlock(&vcpu->kvm->srcu, idx);
> +		gpa_t gaddr = arg;
> +
> +		scoped_guard(srcu, &vcpu->kvm->srcu) {
> +			r = vcpu_ucontrol_translate(vcpu, &gaddr);
> +			if (r)
> +				break;
	... which is not longer the case here. As you explicitly convert
	ENOMENS in gmap_ucas_translate before the topup call tnot converting
	might be an overlook (in the topup function?).
> +
> +			r = kvm_s390_faultin_gfn_simple(vcpu, NULL, gpa_to_gfn(gaddr), false);
> +			if (r == PGM_ADDRESSING)
> +				r = -EFAULT;
> +			if (r <= 0)
> +				break;
	nit: in vcpu_dat_fault_handler the ifs are in the inverse order.
	They are independent, so this does not make any difference, but this
	itches me a little. :) 

> +			r = -EIO;
> +			KVM_BUG_ON(r, vcpu->kvm);
> +		}
>  		break;
>  	}
>  	case KVM_ENABLE_CAP:
> -- 
> 2.53.0
> 
	Steffen