Re: [PATCH v3 20/23] KVM: s390: arm64: Finalize page fault handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] KVM: s390: arm64: preemptible smp_processor_id() in fault
  injection
--

commit 2432a7ede0d5feb054800e41b623b3e845ffe9c3
Author: Steffen Eiden <seiden@linux.ibm.com>

KVM: s390: arm64: Finalize page fault handling

This commit replaces temporary -ENOEXEC returns in the arm64-on-s390 page
fault handler with address size fault and synchronous external abort
injection into the guest, and bounds the guest address limit and
ID_AA64MMFR0_EL1.PARANGE against the supported physical address range.

> diff --git a/arch/s390/kvm/arm64/mmu.c b/arch/s390/kvm/arm64/mmu.c
> index 31e0d036896f1..7a6f6ba326c13 100644
> --- a/arch/s390/kvm/arm64/mmu.c
> +++ b/arch/s390/kvm/arm64/mmu.c
[ ... ]
> @@ -32,12 +33,14 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, gpa_t fault_ipa,
>  	ret = kvm_s390_faultin_gfn(vcpu, NULL, &f);
>  	if (ret <= 0)
>  		return ret;
> -	if (ret == PGM_ADDRESSING)
> +	if (ret == PGM_ADDRESSING) {
>  		/*
> -		 * Without the relevant sysregs we cannot do anything for now.
> -		 * Go back to userspace with an error. TODO sysreg handling
> +		 * There is no page with the requested address. Inject size fault
> +		 * which is the closest arm match to PGM-addressing
>  		 */
> -		return -ENOEXEC;
> +		kvm_inject_size_fault(vcpu);
> +		return 1;
> +	}
>  	KVM_BUG_ON(ret, vcpu->kvm);
>  	return -EINVAL;
>  }
[ ... ]
> @@ -98,16 +130,17 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu)
>  	}
>  
>  	if (translation) {
> -		/*
> -		 * For both cases:
> -		 * Without the relevant sysregs we cannot do anything for now.
> -		 * Go back to userspace with an error. TODO sysreg handling
> -		 */
> -		if (fault_ipa >= BIT_ULL(get_kvm_ipa_limit()))
> -			return -ENOEXEC;
> +		/* Beyond the PA range programmed for the guest */
> +		if (fault_ipa >= kvm_get_pa_address(vcpu->kvm)) {
> +			kvm_inject_size_fault(vcpu);
> +			return 1;
> +		}
>  
> -		if (fault_ipa >= vcpu->kvm->arch.guest_phys_size)
> -			return -ENOEXEC;
> +		/* Falls between the IPA range and the PA range? */
> +		if (fault_ipa >= vcpu->kvm->arch.guest_phys_size) {
> +			fault_ipa |= FAR_TO_FIPA_OFFSET(kvm_vcpu_get_hfar(vcpu));
> +			return kvm_inject_sea(vcpu, is_iabt, fault_ipa);
> +		}
>  	}

[Severity: Medium]
Can calling kvm_inject_size_fault() and kvm_inject_sea() (as well as
kvm_inject_sea_iabt() and kvm_inject_sea_dabt() below) from
kvm_handle_guest_abort() invoke smp_processor_id() in preemptible context?

kvm_arch_vcpu_ioctl_run() calls handle_exit() -> kvm_handle_guest_abort()
with both interrupts and preemption enabled while the vCPU is loaded
(vcpu->cpu != -1).  The fault injection helpers access FAR_EL1 and ESR_EL1
through vcpu_read_sys_reg() and vcpu_write_sys_reg(), which expand to
_vcpu_read_sys_reg() and _vcpu_write_sys_reg():

arch/s390/include/asm/kvm_host_arm64.h:_vcpu_write_sys_reg() {
	({	BUILD_BUG_ON(!__builtin_constant_p((R))); \
		KVM_BUG_ON(vcpu_is_loaded(C) && smp_processor_id() != (C)->cpu, (C)->kvm); \
		(vcpu_is_loaded(C)) \
			? __vcpu_write_sr((C), (V), (R), 0) \
			: __vcpu_write_sr((C), (V), (R), SASR_FLAG_INITIALIZED); })
}

When a guest faults on an address above its IPA or PA range, this triggers
a debug_smp_processor_id() warning under CONFIG_DEBUG_PREEMPT=y, and if
the vCPU thread is preempted and migrated to another CPU between reading
vcpu->cpu and smp_processor_id() (where kvm_sched_out() / kvm_sched_in()
update vcpu->cpu), KVM_BUG_ON() fires and marks the VM bugged.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-sysreg-v3-0-75e95980fb10@linux.ibm.com?part=20