[PATCH kvmtool v3 0/3] cpu: vmexit: KVM_RUN ioctl error handling fixes

[PATCH kvmtool v3 0/3] cpu: vmexit: KVM_RUN ioctl error handling fixes

[Aneesh Kumar K.V (Arm)]

Changes from v2:
* Fix smp boot failure on x86.

Aneesh Kumar K.V (Arm) (3):
  cpu: vmexit: Handle KVM_EXIT_MEMORY_FAULT in KVM_RUN ioctl return
  cpu: vmexit: Retry KVM_RUN ioctl on EINTR and EAGAIN
  cpu: vmexit: Handle KVM_EXIT_UNKNOWN exit reason correctly

 include/kvm/kvm-cpu.h |  2 +-
 kvm-cpu.c             | 30 ++++++++++++++++++++++++------
 kvm.c                 |  1 +
 3 files changed, 26 insertions(+), 7 deletions(-)


base-commit: d410d9a16f91458ae2b912cc088015396f22dfad
-- 
2.43.0

[PATCH kvmtool v3 1/3] cpu: vmexit: Handle KVM_EXIT_MEMORY_FAULT in KVM_RUN ioctl return

[Aneesh Kumar K.V (Arm)]

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
still required to handle these memory fault exits, but that is not
included in this change

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
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 7362f2e99261..40041a22b3fe 100644
--- a/kvm-cpu.c
+++ b/kvm-cpu.c
@@ -43,8 +43,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;
+			/* fallthrough */
+		default:
+			die_perror("KVM_RUN failed");
+		}
+	}
 }
 
 static void kvm_cpu_signal_handler(int signum)
diff --git a/kvm.c b/kvm.c
index 07089cf1b332..b6375a114d11 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

Re: [PATCH kvmtool v3 1/3] cpu: vmexit: Handle KVM_EXIT_MEMORY_FAULT in KVM_RUN ioctl return

[Suzuki K Poulose]

On 28/04/2025 12:57, 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
> still required to handle these memory fault exits, but that is not
> included in this change
> 
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
> 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 7362f2e99261..40041a22b3fe 100644
> --- a/kvm-cpu.c
> +++ b/kvm-cpu.c
> @@ -43,8 +43,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:

Do we need to handle EHWPOISON too, as per the API ?


Suzuki


> +			if (vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT)
> +				return;
> +			/* fallthrough */
> +		default:
> +			die_perror("KVM_RUN failed");


> +		}
> +	}
>   }
>   
>   static void kvm_cpu_signal_handler(int signum)
> diff --git a/kvm.c b/kvm.c
> index 07089cf1b332..b6375a114d11 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;

[PATCH kvmtool v3 2/3] cpu: vmexit: Retry KVM_RUN ioctl on EINTR and EAGAIN

[Aneesh Kumar K.V (Arm)]

When KVM_RUN fails with EINTR or EAGAIN, we should retry the ioctl
without checking kvm_run->exit_reason. These errors don't indicate a
valid VM exit, hence exit_reason may contain stale or undefined values.

Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
---
 include/kvm/kvm-cpu.h |  2 +-
 kvm-cpu.c             | 17 ++++++++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/kvm/kvm-cpu.h b/include/kvm/kvm-cpu.h
index 8f76f8a1123a..72cbb86e6cef 100644
--- a/include/kvm/kvm-cpu.h
+++ b/include/kvm/kvm-cpu.h
@@ -16,7 +16,7 @@ void kvm_cpu__delete(struct kvm_cpu *vcpu);
 void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu);
 void kvm_cpu__setup_cpuid(struct kvm_cpu *vcpu);
 void kvm_cpu__enable_singlestep(struct kvm_cpu *vcpu);
-void kvm_cpu__run(struct kvm_cpu *vcpu);
+int kvm_cpu__run(struct kvm_cpu *vcpu);
 int kvm_cpu__start(struct kvm_cpu *cpu);
 bool kvm_cpu__handle_exit(struct kvm_cpu *vcpu);
 int kvm_cpu__get_endianness(struct kvm_cpu *vcpu);
diff --git a/kvm-cpu.c b/kvm-cpu.c
index 40041a22b3fe..7abbdcebf075 100644
--- a/kvm-cpu.c
+++ b/kvm-cpu.c
@@ -35,27 +35,32 @@ void kvm_cpu__enable_singlestep(struct kvm_cpu *vcpu)
 		pr_warning("KVM_SET_GUEST_DEBUG failed");
 }
 
-void kvm_cpu__run(struct kvm_cpu *vcpu)
+/*
+ * return value -1 if we need to call the kvm_cpu__run again without checking
+ * exit_reason. return value 0 results in taking action based on exit_reason.
+ */
+int kvm_cpu__run(struct kvm_cpu *vcpu)
 {
 	int err;
 
 	if (!vcpu->is_running)
-		return;
+		return -1;
 
 	err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0);
 	if (err < 0) {
 		switch (errno) {
 		case EINTR:
 		case EAGAIN:
-			return;
+			return -1;
 		case EFAULT:
 			if (vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT)
-				return;
+				return 0;
 			/* fallthrough */
 		default:
 			die_perror("KVM_RUN failed");
 		}
 	}
+	return 0;
 }
 
 static void kvm_cpu_signal_handler(int signum)
@@ -179,7 +184,9 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
 		if (cpu->task)
 			kvm_cpu__run_task(cpu);
 
-		kvm_cpu__run(cpu);
+		if (kvm_cpu__run(cpu) == -1)
+			/* retry without an exit_reason check */
+			continue;
 
 		switch (cpu->kvm_run->exit_reason) {
 		case KVM_EXIT_UNKNOWN:
-- 
2.43.0

Re: [PATCH kvmtool v3 2/3] cpu: vmexit: Retry KVM_RUN ioctl on EINTR and EAGAIN

[Suzuki K Poulose]

Hi Aneesh

On 28/04/2025 12:57, Aneesh Kumar K.V (Arm) wrote:
> When KVM_RUN fails with EINTR or EAGAIN, we should retry the ioctl
> without checking kvm_run->exit_reason. These errors don't indicate a
> valid VM exit, hence exit_reason may contain stale or undefined values.
> 
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
>   include/kvm/kvm-cpu.h |  2 +-
>   kvm-cpu.c             | 17 ++++++++++++-----
>   2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/include/kvm/kvm-cpu.h b/include/kvm/kvm-cpu.h
> index 8f76f8a1123a..72cbb86e6cef 100644
> --- a/include/kvm/kvm-cpu.h
> +++ b/include/kvm/kvm-cpu.h
> @@ -16,7 +16,7 @@ void kvm_cpu__delete(struct kvm_cpu *vcpu);
>   void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu);
>   void kvm_cpu__setup_cpuid(struct kvm_cpu *vcpu);
>   void kvm_cpu__enable_singlestep(struct kvm_cpu *vcpu);
> -void kvm_cpu__run(struct kvm_cpu *vcpu);
> +int kvm_cpu__run(struct kvm_cpu *vcpu);
>   int kvm_cpu__start(struct kvm_cpu *cpu);
>   bool kvm_cpu__handle_exit(struct kvm_cpu *vcpu);
>   int kvm_cpu__get_endianness(struct kvm_cpu *vcpu);
> diff --git a/kvm-cpu.c b/kvm-cpu.c
> index 40041a22b3fe..7abbdcebf075 100644
> --- a/kvm-cpu.c
> +++ b/kvm-cpu.c
> @@ -35,27 +35,32 @@ void kvm_cpu__enable_singlestep(struct kvm_cpu *vcpu)
>   		pr_warning("KVM_SET_GUEST_DEBUG failed");
>   }
>   
> -void kvm_cpu__run(struct kvm_cpu *vcpu)
> +/*
> + * return value -1 if we need to call the kvm_cpu__run again without checking
> + * exit_reason. return value 0 results in taking action based on exit_reason.
> + */

minor nit: Should we make the return value meaningful, say -EAGAIN 
instead of -1 ?

> +int kvm_cpu__run(struct kvm_cpu *vcpu)
>   {
>   	int err;
>   
>   	if (!vcpu->is_running)
> -		return;
> +		return -1;
>   
>   	err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0);
>   	if (err < 0) {
>   		switch (errno) {
>   		case EINTR:
>   		case EAGAIN:
> -			return;
> +			return -1;
>   		case EFAULT:
>   			if (vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT)
> -				return;
> +				return 0;
>   			/* fallthrough */
>   		default:
>   			die_perror("KVM_RUN failed");
>   		}
>   	}
> +	return 0;
>   }
>   
>   static void kvm_cpu_signal_handler(int signum)
> @@ -179,7 +184,9 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
>   		if (cpu->task)
>   			kvm_cpu__run_task(cpu);
>   
> -		kvm_cpu__run(cpu);
> +		if (kvm_cpu__run(cpu) == -1)

and this could be :
		if (kvm_cpu__run(cpu) == -EAGAIN)

> +			/* retry without an exit_reason check */
> +			continue;
>   
>   		switch (cpu->kvm_run->exit_reason) {
>   		case KVM_EXIT_UNKNOWN:


Suzuki

Re: [PATCH kvmtool v3 2/3] cpu: vmexit: Retry KVM_RUN ioctl on EINTR and EAGAIN

[Aneesh Kumar K.V]

Suzuki K Poulose <suzuki.poulose@arm.com> writes:

> Hi Aneesh
>
> On 28/04/2025 12:57, Aneesh Kumar K.V (Arm) wrote:
>> When KVM_RUN fails with EINTR or EAGAIN, we should retry the ioctl
>> without checking kvm_run->exit_reason. These errors don't indicate a
>> valid VM exit, hence exit_reason may contain stale or undefined values.
>> 
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>>   include/kvm/kvm-cpu.h |  2 +-
>>   kvm-cpu.c             | 17 ++++++++++++-----
>>   2 files changed, 13 insertions(+), 6 deletions(-)
>> 
>> diff --git a/include/kvm/kvm-cpu.h b/include/kvm/kvm-cpu.h
>> index 8f76f8a1123a..72cbb86e6cef 100644
>> --- a/include/kvm/kvm-cpu.h
>> +++ b/include/kvm/kvm-cpu.h
>> @@ -16,7 +16,7 @@ void kvm_cpu__delete(struct kvm_cpu *vcpu);
>>   void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu);
>>   void kvm_cpu__setup_cpuid(struct kvm_cpu *vcpu);
>>   void kvm_cpu__enable_singlestep(struct kvm_cpu *vcpu);
>> -void kvm_cpu__run(struct kvm_cpu *vcpu);
>> +int kvm_cpu__run(struct kvm_cpu *vcpu);
>>   int kvm_cpu__start(struct kvm_cpu *cpu);
>>   bool kvm_cpu__handle_exit(struct kvm_cpu *vcpu);
>>   int kvm_cpu__get_endianness(struct kvm_cpu *vcpu);
>> diff --git a/kvm-cpu.c b/kvm-cpu.c
>> index 40041a22b3fe..7abbdcebf075 100644
>> --- a/kvm-cpu.c
>> +++ b/kvm-cpu.c
>> @@ -35,27 +35,32 @@ void kvm_cpu__enable_singlestep(struct kvm_cpu *vcpu)
>>   		pr_warning("KVM_SET_GUEST_DEBUG failed");
>>   }
>>   
>> -void kvm_cpu__run(struct kvm_cpu *vcpu)
>> +/*
>> + * return value -1 if we need to call the kvm_cpu__run again without checking
>> + * exit_reason. return value 0 results in taking action based on exit_reason.
>> + */
>
> minor nit: Should we make the return value meaningful, say -EAGAIN 
> instead of -1 ?
>

I was not sure. Having both EINTR and EAGAIN map to just EAGAIN is
confusing IMHO. Instead having a proper return value (-1) which is
documented to imply a retry is better?

>> +int kvm_cpu__run(struct kvm_cpu *vcpu)
>>   {
>>   	int err;
>>   
>>   	if (!vcpu->is_running)
>> -		return;
>> +		return -1;
>>   
>>   	err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0);
>>   	if (err < 0) {
>>   		switch (errno) {
>>   		case EINTR:
>>   		case EAGAIN:
>> -			return;
>> +			return -1;
>>   		case EFAULT:
>>   			if (vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT)
>> -				return;
>> +				return 0;
>>   			/* fallthrough */
>>   		default:
>>   			die_perror("KVM_RUN failed");
>>   		}
>>   	}
>> +	return 0;
>>   }
>>   
>>   static void kvm_cpu_signal_handler(int signum)
>> @@ -179,7 +184,9 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
>>   		if (cpu->task)
>>   			kvm_cpu__run_task(cpu);
>>   
>> -		kvm_cpu__run(cpu);
>> +		if (kvm_cpu__run(cpu) == -1)
>
> and this could be :
> 		if (kvm_cpu__run(cpu) == -EAGAIN)
>
>> +			/* retry without an exit_reason check */
>> +			continue;
>>   
>>   		switch (cpu->kvm_run->exit_reason) {
>>   		case KVM_EXIT_UNKNOWN:
>
>
> Suzuki

Re: [PATCH kvmtool v3 2/3] cpu: vmexit: Retry KVM_RUN ioctl on EINTR and EAGAIN

[Alexandru Elisei]

Hi Aneesh,

Is this to fix Will's report that the series breaks boot on x86?

On Mon, Apr 28, 2025 at 05:27:44PM +0530, Aneesh Kumar K.V (Arm) wrote:
> When KVM_RUN fails with EINTR or EAGAIN, we should retry the ioctl
> without checking kvm_run->exit_reason. These errors don't indicate a
> valid VM exit, hence exit_reason may contain stale or undefined values.

EAGAIN is not documented in Documentation/virt/kvm/api.rst. So I'm going to
assume it's this code path that is causing the -EAGAIN return value [1].

If that's the case, how does retrying KVM_RUN solve the issue? Just trying to
get to the bottom of it, because there's not much detail in the docs.

[1] https://elixir.bootlin.com/linux/v6.15-rc3/source/arch/x86/kvm/x86.c#L11532


Thanks,
Alex

> 
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
>  include/kvm/kvm-cpu.h |  2 +-
>  kvm-cpu.c             | 17 ++++++++++++-----
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/include/kvm/kvm-cpu.h b/include/kvm/kvm-cpu.h
> index 8f76f8a1123a..72cbb86e6cef 100644
> --- a/include/kvm/kvm-cpu.h
> +++ b/include/kvm/kvm-cpu.h
> @@ -16,7 +16,7 @@ void kvm_cpu__delete(struct kvm_cpu *vcpu);
>  void kvm_cpu__reset_vcpu(struct kvm_cpu *vcpu);
>  void kvm_cpu__setup_cpuid(struct kvm_cpu *vcpu);
>  void kvm_cpu__enable_singlestep(struct kvm_cpu *vcpu);
> -void kvm_cpu__run(struct kvm_cpu *vcpu);
> +int kvm_cpu__run(struct kvm_cpu *vcpu);
>  int kvm_cpu__start(struct kvm_cpu *cpu);
>  bool kvm_cpu__handle_exit(struct kvm_cpu *vcpu);
>  int kvm_cpu__get_endianness(struct kvm_cpu *vcpu);
> diff --git a/kvm-cpu.c b/kvm-cpu.c
> index 40041a22b3fe..7abbdcebf075 100644
> --- a/kvm-cpu.c
> +++ b/kvm-cpu.c
> @@ -35,27 +35,32 @@ void kvm_cpu__enable_singlestep(struct kvm_cpu *vcpu)
>  		pr_warning("KVM_SET_GUEST_DEBUG failed");
>  }
>  
> -void kvm_cpu__run(struct kvm_cpu *vcpu)
> +/*
> + * return value -1 if we need to call the kvm_cpu__run again without checking
> + * exit_reason. return value 0 results in taking action based on exit_reason.
> + */
> +int kvm_cpu__run(struct kvm_cpu *vcpu)
>  {
>  	int err;
>  
>  	if (!vcpu->is_running)
> -		return;
> +		return -1;
>  
>  	err = ioctl(vcpu->vcpu_fd, KVM_RUN, 0);
>  	if (err < 0) {
>  		switch (errno) {
>  		case EINTR:
>  		case EAGAIN:
> -			return;
> +			return -1;
>  		case EFAULT:
>  			if (vcpu->kvm_run->exit_reason == KVM_EXIT_MEMORY_FAULT)
> -				return;
> +				return 0;
>  			/* fallthrough */
>  		default:
>  			die_perror("KVM_RUN failed");
>  		}
>  	}
> +	return 0;
>  }
>  
>  static void kvm_cpu_signal_handler(int signum)
> @@ -179,7 +184,9 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
>  		if (cpu->task)
>  			kvm_cpu__run_task(cpu);
>  
> -		kvm_cpu__run(cpu);
> +		if (kvm_cpu__run(cpu) == -1)
> +			/* retry without an exit_reason check */
> +			continue;
>  
>  		switch (cpu->kvm_run->exit_reason) {
>  		case KVM_EXIT_UNKNOWN:
> -- 
> 2.43.0
> 
> 

Re: [PATCH kvmtool v3 2/3] cpu: vmexit: Retry KVM_RUN ioctl on EINTR and EAGAIN

[Aneesh Kumar K.V]

Alexandru Elisei <alexandru.elisei@arm.com> writes:

> Hi Aneesh,
>
> Is this to fix Will's report that the series breaks boot on x86?
>

Yes.

> On Mon, Apr 28, 2025 at 05:27:44PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> When KVM_RUN fails with EINTR or EAGAIN, we should retry the ioctl
>> without checking kvm_run->exit_reason. These errors don't indicate a
>> valid VM exit, hence exit_reason may contain stale or undefined values.
>
> EAGAIN is not documented in Documentation/virt/kvm/api.rst. So I'm going to
> assume it's this code path that is causing the -EAGAIN return value [1].
>

IIUC, EAGAIN and EINTR are syscall return errno that indicates a system
call need to be retried. Documentation/virt/kvm/api.rst do mention that
exit_reason is value only with return value 0.

	__u32 exit_reason;

When KVM_RUN has returned successfully (return value 0), this informs
application code why KVM_RUN has returned.

>
> If that's the case, how does retrying KVM_RUN solve the issue? Just trying to
> get to the bottom of it, because there's not much detail in the docs.
>
> [1] https://elixir.bootlin.com/linux/v6.15-rc3/source/arch/x86/kvm/x86.c#L11532
>
>

So in that code path vcpu will be in kvm_vcpu_block(vcpu) waiting for
the IPI. On IPI kvm_apic_accept_events() will return 0 after setting the
vcpu->arch.mp_state. Hence a KVM_RUN ioctl again will find the mp_state
correctly updated. 

-aneesh

Re: [PATCH kvmtool v3 2/3] cpu: vmexit: Retry KVM_RUN ioctl on EINTR and EAGAIN

[Alexandru Elisei]

Hi Anessh,

On Tue, Apr 29, 2025 at 09:07:02AM +0530, Aneesh Kumar K.V wrote:
> Alexandru Elisei <alexandru.elisei@arm.com> writes:
> 
> > Hi Aneesh,
> >
> > Is this to fix Will's report that the series breaks boot on x86?
> >
> 
> Yes.
> 
> > On Mon, Apr 28, 2025 at 05:27:44PM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> When KVM_RUN fails with EINTR or EAGAIN, we should retry the ioctl
> >> without checking kvm_run->exit_reason. These errors don't indicate a
> >> valid VM exit, hence exit_reason may contain stale or undefined values.
> >
> > EAGAIN is not documented in Documentation/virt/kvm/api.rst. So I'm going to
> > assume it's this code path that is causing the -EAGAIN return value [1].
> >
> 
> IIUC, EAGAIN and EINTR are syscall return errno that indicates a system
> call need to be retried. Documentation/virt/kvm/api.rst do mention that
> exit_reason is value only with return value 0.
> 
> 	__u32 exit_reason;
> 
> When KVM_RUN has returned successfully (return value 0), this informs
> application code why KVM_RUN has returned.

Yeah, that makes sense. api.rst is a bit weird, because it doesn't document
all of the error codes, and they are documented in different places, like
-EFAULT which is under KVM_EXIT_MEMORY_FAULT, not under KVM_RUN.

Thanks for the explanation!

> 
> >
> > If that's the case, how does retrying KVM_RUN solve the issue? Just trying to
> > get to the bottom of it, because there's not much detail in the docs.
> >
> > [1] https://elixir.bootlin.com/linux/v6.15-rc3/source/arch/x86/kvm/x86.c#L11532
> >
> >
> 
> So in that code path vcpu will be in kvm_vcpu_block(vcpu) waiting for
> the IPI. On IPI kvm_apic_accept_events() will return 0 after setting the
> vcpu->arch.mp_state. Hence a KVM_RUN ioctl again will find the mp_state
> correctly updated. 

I think that would be useful to have in the commit message.

Thanks,
Alex

> 
> -aneesh

[PATCH kvmtool v3 3/3] cpu: vmexit: Handle KVM_EXIT_UNKNOWN exit reason correctly

[Aneesh Kumar K.V (Arm)]

The return value for kernel VM exit handlers is confusing and has led to
errors in different kernel exit handlers. A return value of 0 indicates
a return to the VMM, whereas a return value of 1 indicates resuming
execution in the guest. Some handlers mistakenly return 0 to force a
return to the guest.

This worked in kvmtool because the exit_reason defaulted to
0 (KVM_EXIT_UNKNOWN), and kvmtool did not error out on an unknown exit
reason. However, forcing a VMM exit with error on KVM_EXIT_UNKNOWN
exit_reson would help catch these bugs early.

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
---
 kvm-cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kvm-cpu.c b/kvm-cpu.c
index 7abbdcebf075..a76dfee561ec 100644
--- a/kvm-cpu.c
+++ b/kvm-cpu.c
@@ -190,7 +190,7 @@ int kvm_cpu__start(struct kvm_cpu *cpu)
 
 		switch (cpu->kvm_run->exit_reason) {
 		case KVM_EXIT_UNKNOWN:
-			break;
+			goto panic_kvm;
 		case KVM_EXIT_DEBUG:
 			kvm_cpu__show_registers(cpu);
 			kvm_cpu__show_code(cpu);
-- 
2.43.0

Re: [PATCH kvmtool v3 0/3] cpu: vmexit: KVM_RUN ioctl error handling fixes

[Aneesh Kumar K.V]

"Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> writes:

> Changes from v2:
> * Fix smp boot failure on x86.
>
> Aneesh Kumar K.V (Arm) (3):
>   cpu: vmexit: Handle KVM_EXIT_MEMORY_FAULT in KVM_RUN ioctl return
>   cpu: vmexit: Retry KVM_RUN ioctl on EINTR and EAGAIN
>   cpu: vmexit: Handle KVM_EXIT_UNKNOWN exit reason correctly
>
>  include/kvm/kvm-cpu.h |  2 +-
>  kvm-cpu.c             | 30 ++++++++++++++++++++++++------
>  kvm.c                 |  1 +
>  3 files changed, 26 insertions(+), 7 deletions(-)
>
>
> base-commit: d410d9a16f91458ae2b912cc088015396f22dfad

Any feedback on this?

-aneesh