Re: [PATCH kvmtool v2 2/2] cpu: vmexit: Handle KVM_EXIT_MEMORY_FAULT in KVM_RUN ioctl return

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Aneesh,

On Mon, Feb 24, 2025 at 02:40:00PM +0530, Aneesh Kumar K.V (Arm) wrote:
> Linux kernel documentation states:
> 
> "Note! KVM_EXIT_MEMORY_FAULT is unique among all KVM exit reasons in
> that it accompanies a return code of '-1', not '0'! errno will always be
> set to EFAULT or EHWPOISON when KVM exits with KVM_EXIT_MEMORY_FAULT,
> userspace should assume kvm_run.exit_reason is stale/undefined for all
> other error numbers." "
> 
> Update KVM_RUN ioctl error handling to correctly handle
> KVM_EXIT_MEMORY_FAULT. This enables the memory fault exit handlers in
> the kernel to return -EFAULT as the return value. VMM support is

I think this is too much information. Are all architectures that kvmtool
supports going to/have implemented forwarding memory faults to userspace
this same way? I would think not.

The first paragraph should be enough - kvmtool implements the KVM ABI, it
shouldn't care about how that is implemented by KVM.

Other than that, looks correct to me, and more readable (but that might
just be my personal bias):

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex

> still required to handle these memory fault exits, but that is not
> included in this change
> 
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
>  kvm-cpu.c | 15 +++++++++++++--
>  kvm.c     |  1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/kvm-cpu.c b/kvm-cpu.c
> index 7c62bfc56679..c0b10b1534ab 100644
> --- a/kvm-cpu.c
> +++ b/kvm-cpu.c
> @@ -41,8 +41,19 @@ void kvm_cpu__run(struct kvm_cpu *vcpu)
>  		return;
>  
>  	err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0);
> -	if (err < 0 && (errno != EINTR && errno != EAGAIN))
> -		die_perror("KVM_RUN failed");
> +	if (err < 0) {
> +		switch (errno) {
> +		case EINTR:
> +		case EAGAIN:
> +			return;
> +		case EFAULT:
> +			if (vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT)
> +				return;
> +			/* faullthrough */
> +		default:
> +			die_perror("KVM_RUN failed");
> +		}
> +	}
>  }
>  
>  static void kvm_cpu_signal_handler(int signum)
> diff --git a/kvm.c b/kvm.c
> index 42b881217df6..172d951bfe4e 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -55,6 +55,7 @@ const char *kvm_exit_reasons[] = {
>  #ifdef CONFIG_PPC64
>  	DEFINE_KVM_EXIT_REASON(KVM_EXIT_PAPR_HCALL),
>  #endif
> +	DEFINE_KVM_EXIT_REASON(KVM_EXIT_MEMORY_FAULT),
>  };
>  
>  static int pause_event;
> -- 
> 2.43.0
>