Re: [PATCH kvmtool v2 1/2] cpu: vmexit: Handle KVM_EXIT_UNKNOWN exit reason correctly

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Will Deacon <will@kernel.org> writes:

> On Mon, Feb 24, 2025 at 02:39:59PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> 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.
>> 
>> 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 f66dcd07220c..7c62bfc56679 100644
>> --- a/kvm-cpu.c
>> +++ b/kvm-cpu.c
>> @@ -170,7 +170,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
>
> This breaks SMP boot on my x86 machine:
>
> # ./lkvm run
> ...
> [    0.628472] smp: Bringing up secondary CPUs ...
> [    0.630401] smpboot: x86: Booting SMP configuration:
>   Error: KVM exit reason: 0 ("KVM_EXIT_UNKNOWN")
>   Error: KVM exit code: 0
>

Turns out we should handle EINTR and EAGAIN as special such that we do
an retry of KVM_RUN ioctl without checking the exit_reason.
I can send a v3 if you are ok with the change below.

@@ -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);
modified   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;
 			/* faullthrough */
 		default:
 			die_perror("KVM_RUN failed");
 		}
 	}
+	return 0;
 }
 
 static void kvm_cpu_signal_handler(int signum)
@@ -179,11 +184,13 @@ 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;
 


-aneesh