Re: [PATCH 2/2] KVM: selftests: Extend x86's sync_regs_test to check for races

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/3/23 18:41, Sean Christopherson wrote:
> KVM doesn't expost the bugged/dead information, though I suppose userspace could
> probe that information by doing an ioctl() that is guaranteed to succeeed and
> looking for -EIO, e.g. KVM_CHECK_EXTENSION on the VM.
> 
> I was going to say that it's not worth trying to detect a bugged/dead VM in
> selftests, because it requires having the pointer to the VM, and that's not
> typically available when an assert fails, but the obviously solution is to tap
> into the VM and vCPU ioctl() helpers.  That's also good motivation to add helpers
> and consolidate asserts for ioctls() that return fds, i.e. for which a positive
> return is considered success.
> 
> With the below (partial conversion), the failing testcase yields this.  Using a
> heuristic isn't ideal, but practically speaking I can't see a way for the -EIO
> check to go awry, and anything to make debugging errors easier is definitely worth
> doing IMO.
> 
> ==== 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 kernel log for clues

Yes, such automatic reporting of dead VMs is a really nice feature.

> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 07732a157ccd..e48ac57be13a 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -258,17 +258,42 @@ 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 (obviously), 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.
> + */

Do you think it's worth mentioning the ioctl() always returning -EIO in case of
kvm->mm != current->mm? I suppose that's something purely hypothetical in this
context.

thanks,
Michal




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux