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

[sashiko-bot@kernel.org]

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.

> +{
> +	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?

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);
    ...
}

> +}
> +
>  u64 *vm_get_pte(struct kvm_vm *vm, gva_t gva);
>  
>  u64 kvm_hypercall(u64 nr, u64 a0, u64 a1, u64 a2, u64 a3);

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