Re: [PATCH v4 14/17] KVM: selftests: Assert success in vmlaunch() and vmresume()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The vmlaunch() and vmresume() wrappers are defined as static inline functions instead of macros. Because __GUEST_ASSERT captures __FILE__ and __LINE__ at the point of its invocation, assertion failures in these wrappers will report the file and line number inside vmx.h rather than the actual caller's location in the test file.
--

commit 00cde4aa7e9af50759c8cee7b3f7fecea0caf3ed
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Assert success in vmlaunch() and vmresume()

This patch updates the outer vmlaunch and vmresume selftests APIs to assert
success directly, deduplicating error checking across tests. Tests expecting
VM-Enter failures now explicitly use the double-underscore versions.

> diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
> index d0ac8e128d985..1419043b41dd3 100644
> --- a/tools/testing/selftests/kvm/include/x86/vmx.h
> +++ b/tools/testing/selftests/kvm/include/x86/vmx.h
> @@ -362,14 +362,14 @@ static inline u64 vmptrst(void)
>  int __vmlaunch(void);
>  int __vmresume(void);
>  
> -static inline int vmlaunch(void)
> +static inline void vmlaunch(void)
>  {
> -	return __vmlaunch();
> +	__GUEST_ASSERT(!__vmlaunch(), "vmlaunch hit VM-Fail");
>  }

[Severity: Medium]
Because __GUEST_ASSERT captures the __FILE__ and __LINE__ macros at the point
of its invocation, placing it inside a static inline function will cause
assertion failures to always report the location inside vmx.h instead of the
actual caller's location in the test file (for example, inside state_test.c
or nested_emulation_test.c).

Does this cause a loss of debugging context when a VM-Fail assertion triggers
in tests with multiple vmlaunch invocations? Could vmlaunch be defined as a
macro instead to preserve the caller's location?

>  
> -static inline int vmresume(void)
> +static inline void vmresume(void)
>  {
> -	return __vmresume();
> +	__GUEST_ASSERT(!__vmresume(), "vmresume hit VM-Fail");
>  }

[Severity: Medium]
Similarly for vmresume, using __GUEST_ASSERT in an inline function will mask
the file and line number of the actual test invocation. Would a macro be more
appropriate here as well to retain the debugging context?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904002423.3154436-1-seanjc@google.com?part=14