Re: [PATCH v3 4/5] KVM: selftests: Add module param API to check if nested virtualization is enabled

[Sean Christopherson <seanjc@google.com>]

On Wed, Sep 30, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] kvm: selftests: kvm_is_nested_virtualization_enabled() aborts on Intel
> - [Low] kvm: selftests: get_common_kvm_vendor_param_bool() returns int instead of bool
> --
> 
> commit 680296c2620e4e5c37fadd1bdce211406dcc250c
> Author: Sean Christopherson <seanjc@google.com>
> 
> KVM: selftests: Add module param API to check if nested virtualization is enabled
>     
> Add an API to check if the nested virtualization module param is enabled,
> e.g. so that tests can verify KVM's own enumeration of the corresponding
> CPU feature. Provide a big warning above the API to make it as obvious as
> possible clear that tests should use "kvm_cpu_has(X86_FEATURE_VMX) ||
> kvm_cpu_has(X86_FEATURE_SVM)" when checking for nested virtualization.
> 
> Link: https://lore.kernel.org/all/20260910115414.3015260-2-ewanhai-oc@zhaoxin.com
> 
> > diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> > index 6e6f70035508a..1608ddb91b6e6 100644
> > --- a/tools/testing/selftests/kvm/include/x86/processor.h
> > +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> > @@ -1429,6 +1429,14 @@ static inline bool get_kvm_amd_param_bool(const char *param)
> >  	return kvm_get_module_param_bool("kvm_amd", param);
> >  }
> >  
> > +static inline int get_common_kvm_vendor_param_bool(const char *param)
> 
> [Severity: Low]
> Does this function have a type mismatch? It evaluates vendor-specific boolean
> parameter helpers, but returns an int instead of a bool.

That part is intentional.

> > +{
> > +	if (host_cpu_is_intel)
> > +		return get_kvm_intel_param_bool(param);
> > +
> > +	return get_kvm_amd_param_bool(param);
> > +}
> 
> [ ... ]
> 
> > @@ -1472,6 +1485,16 @@ static inline bool kvm_is_lbrv_enabled(void)
> >  	return !!get_kvm_amd_param_integer("lbrv");
> >  }
> >  
> > +/*
> > + * Do NOT use this to check for nVMX or nSVM support.  Querying kvm_cpu_has()
> > + * for either of X86_FEATURE_{VMX,SVM} is the idiomatic way to check for nested
> > + * virtualization support.  Use this *only* to validate KVM's own enumeration.
> > + */
> > +static inline bool kvm_is_nested_virtualization_enabled(void)
> > +{
> > +	return get_common_kvm_vendor_param_integer("nested");
> 
> [Severity: Medium]
> Will this unconditionally abort the test suite on Intel hosts?

Ugh, yes.  I had a brain fart and mentally inverted the X86_FEATURE_SVM result,
i.e. thought I was testing this helper.

> The VMX "nested" module parameter is a boolean exposed via sysfs as "Y\n" or
> "N\n". If this helper uses the integer reader on Intel, it will read "Y\n"
> and pass "Y" directly to atoi_paranoid(), which throws a TEST_ASSERT because
> strtol() fails to parse "Y" as an integer:
> 
> tools/testing/selftests/kvm/lib/test_util.c:atoi_paranoid() {
>     ...
>     TEST_ASSERT(num_str != end_ptr,
>                 "strtol(\"%s\") didn't find a valid integer.", num_str);
>     ...

OMG, I hate KVM.  I'll just do the obvious thing, and then make a mental note to
call out the semantic "conflict" in the pull request.  *sigh*

/*
 * Do NOT use this to check for nVMX or nSVM support.  Querying kvm_cpu_has()
 * for either of X86_FEATURE_{VMX,SVM} is the idiomatic way to check for nested
 * virtualization support.  Use this *only* to validate KVM's own enumeration.
 */
static inline bool kvm_is_nested_virtualization_enabled(void)
{
	if (host_cpu_is_intel)
		return get_kvm_intel_param_bool("nested");

	return get_kvm_amd_param_integer("nested");
}