Re: [PATCH v4 5/6] KVM: selftests: Refactor invalid nVMX state test to prepare for RSM testcase

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The assertion failure message in `vcpu_run_to_io` hardcodes 'L2' as the expected guest level, ignoring the `want_l2` parameter.
--

--- Patch [5]: [PATCH v4 5/6] KVM: selftests: Refactor invalid nVMX state test to prepare for RSM testcase ---
commit 7bfb34d63ffa2d4c98580f574a703358bca449c2
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Refactor invalid nVMX state test to prepare for RSM testcase

This commit extracts the VM creation and initial vCPU execution logic into
helper functions to allow reuse for a future RSM testcase. It also eliminates
the global vm variable and adds an assertion to ensure L2 is never resumed
after an I/O exit to L1.

> diff --git a/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c b/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
> --- a/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
> +++ b/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c

[ ... ]

> +static void vcpu_run_to_io(struct kvm_vcpu *vcpu, bool want_l2)
>  {
> -	gva_t vmx_pages_gva;
> -	struct kvm_sregs sregs;
> -	struct kvm_vcpu *vcpu;
> -	struct kvm_run *run;
> -	struct ucall uc;
> +	struct kvm_run *run = vcpu->run;
>  
> -	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_VMX));
> +	vcpu_run(vcpu);
>  
> -	vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code);
> +	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
>  
> -	/* Allocate VMX pages and shared descriptors (vmx_pages). */
> -	vcpu_alloc_vmx(vm, &vmx_pages_gva);
> -	vcpu_args_set(vcpu, 1, vmx_pages_gva);
> +	TEST_ASSERT(run->io.port == ARBITRARY_IO_PORT &&
> +		    (!!(run->flags & KVM_RUN_X86_GUEST_MODE) == want_l2  ||
> +		     !kvm_has_cap(KVM_CAP_X86_GUEST_MODE)),
> +		    "Expected IN from port %d from L2, got port %d from L%u",

[Severity: Low]
Is the hardcoded "from L2" in the expected message string intentional?

Since vcpu_run_to_io() is a helper that takes a want_l2 parameter, the error
output could be misleading if a test calls it expecting an exit from L1.
Would it make sense to dynamically print L1 or L2 in the failure message
based on the want_l2 parameter?

> +		    ARBITRARY_IO_PORT, run->io.port,
> +		    1 + !!(run->flags & KVM_RUN_X86_GUEST_MODE));
> +}

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