Re: [RFC PATCH 5/5] KVM: selftests: nSVM: Add nested PML coverage to nested_dirty_log_test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] selftests: kvm: l2_pml_guest_code executes VMX-specific vmcall() on SVM
--

commit cb64ef5e62e8854d8adff372ccbc0426babe6f16
Author: Nikunj A Dadhania <nikunj@amd.com>

KVM: selftests: nSVM: Add nested PML coverage to nested_dirty_log_test

This commit extends the existing nested PML selftest to also cover AMD's
nested Page Modification Logging (nSVM PML) by introducing an SVM-specific
L1 guest code path.

> diff --git a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> index b1008724f46dc..cc49fa0c42937 100644
> --- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> @@ -250,6 +250,45 @@ static void l1_pml_vmx_code(struct vmx_pages *vmx)
>  	GUEST_DONE();
>  }
>  
> +static void l1_pml_svm_code(struct svm_test_data *svm)
> +{
> +	struct vmcb_control_area *ctrl = &svm->vmcb->control;
> +	int nr_pml_full = 0;
> +
> +	/* PML requires NPT. */
> +	GUEST_ASSERT(svm->ncr3_gpa);
> +
> +	generic_svm_setup(svm, l2_pml_guest_code);

[Severity: High]
Could this setup lead to an Invalid Opcode (#UD) exception on AMD systems?

Looking at l2_pml_guest_code(), it appears to unconditionally execute the
vmcall() instruction:

tools/testing/selftests/kvm/x86/nested_dirty_log_test.c:l2_pml_guest_code() {
    ...
    vmcall();
    ...
}

If l2_pml_guest_code() emits the Intel-specific VMCALL instruction, won't
executing VMCALL on AMD systems raise an Invalid Opcode (#UD) exception
instead of causing a VM-exit to L1?

Since generic_svm_setup() does not intercept #UD, does this exception
get delivered to the L2 guest, triggering the host's selftest abort
handler and crashing the nested AMD guest?

> +
> +	ctrl->pml_addr = svm->pml_gpa;
> +	ctrl->pml_index = PML_LOG_NR_ENTRIES;
> +	ctrl->misc_ctl |= SVM_MISC_ENABLE_PML;
> +
> +	run_guest(svm->vmcb, svm->vmcb_gpa);
> +
> +	for (;;) {
> +		if (ctrl->exit_code == SVM_EXIT_VMMCALL)
> +			break;

[Severity: High]
Will this loop exit condition ever be met?

If l2_pml_guest_code() crashes with a #UD exception before completing the
hypercall, L1 won't receive a clean SVM_EXIT_VMMCALL, causing the test
to fail on AMD.

Would it be better to use a generic wrapper (like vmmcall()) in L2 that
handles both VMX and SVM properly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015103933.2088307-1-nikunj@amd.com?part=5