]> xenbits.xensource.com Git - people/pauldu/linux.git/commitdiff
KVM: selftests: Add logic to detect if ioctl() failed because VM was killed
authorSean Christopherson <seanjc@google.com>
Wed, 8 Nov 2023 01:09:53 +0000 (17:09 -0800)
committerSean Christopherson <seanjc@google.com>
Wed, 29 Nov 2023 22:51:20 +0000 (14:51 -0800)
Add yet another macro to the VM/vCPU ioctl() framework to detect when an
ioctl() failed because KVM killed/bugged the VM, i.e. when there was
nothing wrong with the ioctl() itself.  If KVM kills a VM, e.g. by way of
a failed KVM_BUG_ON(), all subsequent VM and vCPU ioctl()s will fail with
-EIO, which can be quite misleading and ultimately waste user/developer
time.

Use KVM_CHECK_EXTENSION on KVM_CAP_USER_MEMORY to detect if the VM is
dead and/or bug, as KVM doesn't provide a dedicated ioctl().  Using a
heuristic is obviously less than ideal, but practically speaking the logic
is bulletproof barring a KVM change, and any such change would arguably
break userspace, e.g. if KVM returns something other than -EIO.

Without the detection, tearing down a bugged VM yields a cryptic failure
when deleting memslots:

  ==== Test Assertion Failure ====
  lib/kvm_util.c:689: !ret
  pid=45131 tid=45131 errno=5 - Input/output error
     1 0x00000000004036c3: __vm_mem_region_delete at kvm_util.c:689
     2 0x00000000004042f0: kvm_vm_free at kvm_util.c:724 (discriminator 12)
     3 0x0000000000402929: race_sync_regs at sync_regs_test.c:193
     4 0x0000000000401cab: main at sync_regs_test.c:334 (discriminator 6)
     5 0x0000000000416f13: __libc_start_call_main at libc-start.o:?
     6 0x000000000041855f: __libc_start_main_impl at ??:?
     7 0x0000000000401d40: _start at ??:?
  KVM_SET_USER_MEMORY_REGION failed, rc: -1 errno: 5 (Input/output error)

Which morphs into a more pointed error message with the detection:

  ==== Test Assertion Failure ====
  lib/kvm_util.c:689: false
  pid=80347 tid=80347 errno=5 - Input/output error
     1 0x00000000004039ab: __vm_mem_region_delete at kvm_util.c:689 (discriminator 5)
     2 0x0000000000404660: kvm_vm_free at kvm_util.c:724 (discriminator 12)
     3 0x0000000000402ac9: race_sync_regs at sync_regs_test.c:193
     4 0x0000000000401cb7: main at sync_regs_test.c:334 (discriminator 6)
     5 0x0000000000418263: __libc_start_call_main at libc-start.o:?
     6 0x00000000004198af: __libc_start_main_impl at ??:?
     7 0x0000000000401d90: _start at ??:?
  KVM killed/bugged the VM, check the kernel log for clues

Suggested-by: Michal Luczaj <mhal@rbox.co>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Colton Lewis <coltonlewis@google.com>
Link: https://lore.kernel.org/r/20231108010953.560824-3-seanjc@google.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
tools/testing/selftests/kvm/include/kvm_util_base.h
tools/testing/selftests/kvm/lib/kvm_util.c

index 190f9c0ef5840662528a79572fe3354c9a4afd09..e0da036a13aefd43b3c911dbcfef8ef789a3c9f2 100644 (file)
@@ -301,11 +301,40 @@ static __always_inline void static_assert_is_vm(struct kvm_vm *vm) { }
        kvm_do_ioctl((vm)->fd, cmd, arg);                       \
 })
 
+/*
+ * Assert that a VM or vCPU ioctl() succeeded, with extra magic to detect if
+ * the ioctl() failed because KVM killed/bugged the VM.  To detect a dead VM,
+ * probe KVM_CAP_USER_MEMORY, which (a) has been supported by KVM since before
+ * selftests existed and (b) should never outright fail, i.e. is supposed to
+ * return 0 or 1.  If KVM kills a VM, KVM returns -EIO for all ioctl()s for the
+ * VM and its vCPUs, including KVM_CHECK_EXTENSION.
+ */
+#define __TEST_ASSERT_VM_VCPU_IOCTL(cond, name, ret, vm)                               \
+do {                                                                                   \
+       int __errno = errno;                                                            \
+                                                                                       \
+       static_assert_is_vm(vm);                                                        \
+                                                                                       \
+       if (cond)                                                                       \
+               break;                                                                  \
+                                                                                       \
+       if (errno == EIO &&                                                             \
+           __vm_ioctl(vm, KVM_CHECK_EXTENSION, (void *)KVM_CAP_USER_MEMORY) < 0) {     \
+               TEST_ASSERT(errno == EIO, "KVM killed the VM, should return -EIO");     \
+               TEST_FAIL("KVM killed/bugged the VM, check the kernel log for clues");  \
+       }                                                                               \
+       errno = __errno;                                                                \
+       TEST_ASSERT(cond, __KVM_IOCTL_ERROR(name, ret));                                \
+} while (0)
+
+#define TEST_ASSERT_VM_VCPU_IOCTL(cond, cmd, ret, vm)          \
+       __TEST_ASSERT_VM_VCPU_IOCTL(cond, #cmd, ret, vm)
+
 #define vm_ioctl(vm, cmd, arg)                                 \
 ({                                                             \
        int ret = __vm_ioctl(vm, cmd, arg);                     \
                                                                \
-       TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(#cmd, ret));        \
+       __TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, vm);               \
 })
 
 static __always_inline void static_assert_is_vcpu(struct kvm_vcpu *vcpu) { }
@@ -320,7 +349,7 @@ static __always_inline void static_assert_is_vcpu(struct kvm_vcpu *vcpu) { }
 ({                                                             \
        int ret = __vcpu_ioctl(vcpu, cmd, arg);                 \
                                                                \
-       TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(#cmd, ret));        \
+       __TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, (vcpu)->vm);       \
 })
 
 /*
@@ -331,7 +360,7 @@ static inline int vm_check_cap(struct kvm_vm *vm, long cap)
 {
        int ret =  __vm_ioctl(vm, KVM_CHECK_EXTENSION, (void *)cap);
 
-       TEST_ASSERT(ret >= 0, KVM_IOCTL_ERROR(KVM_CHECK_EXTENSION, ret));
+       TEST_ASSERT_VM_VCPU_IOCTL(ret >= 0, KVM_CHECK_EXTENSION, ret, vm);
        return ret;
 }
 
@@ -438,7 +467,7 @@ static inline int vm_get_stats_fd(struct kvm_vm *vm)
 {
        int fd = __vm_ioctl(vm, KVM_GET_STATS_FD, NULL);
 
-       TEST_ASSERT(fd >= 0, KVM_IOCTL_ERROR(KVM_GET_STATS_FD, fd));
+       TEST_ASSERT_VM_VCPU_IOCTL(fd >= 0, KVM_GET_STATS_FD, fd, vm);
        return fd;
 }
 
@@ -680,7 +709,7 @@ static inline int vcpu_get_stats_fd(struct kvm_vcpu *vcpu)
 {
        int fd = __vcpu_ioctl(vcpu, KVM_GET_STATS_FD, NULL);
 
-       TEST_ASSERT(fd >= 0, KVM_IOCTL_ERROR(KVM_GET_STATS_FD, fd));
+       TEST_ASSERT_VM_VCPU_IOCTL(fd >= 0, KVM_CHECK_EXTENSION, fd, vcpu->vm);
        return fd;
 }
 
index 9b29cbf49476d3134367a5d475f9b937383d731e..17a978b8a2c4b99c1b01c64524a3eb9153c99d61 100644 (file)
@@ -1271,7 +1271,7 @@ struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
        vcpu->vm = vm;
        vcpu->id = vcpu_id;
        vcpu->fd = __vm_ioctl(vm, KVM_CREATE_VCPU, (void *)(unsigned long)vcpu_id);
-       TEST_ASSERT(vcpu->fd >= 0, KVM_IOCTL_ERROR(KVM_CREATE_VCPU, vcpu->fd));
+       TEST_ASSERT_VM_VCPU_IOCTL(vcpu->fd >= 0, KVM_CREATE_VCPU, vcpu->fd, vm);
 
        TEST_ASSERT(vcpu_mmap_sz() >= sizeof(*vcpu->run), "vcpu mmap size "
                "smaller than expected, vcpu_mmap_sz: %i expected_min: %zi",