Re: [RFC PATCH 4/5] KVM: selftests: nVMX: 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:
- [Medium] selftests: kvm: nVMX test asserts on invalid guest PML index
--

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

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

This patch adds coverage for nested PML to nested_dirty_log_test. It
verifies that L1 takes a PML_FULL exit when the buffer fills, can drain it,
and can resume L2 without losing entries.

> 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 7e5f9ae1cb00a..b1008724f46dc 100644
> --- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c

[ ... ]

> @@ -192,6 +192,149 @@ static void test_handle_ucall_sync(struct kvm_vm *vm, u64 arg,
>  	}
>  }
>  
> +#define PML_LOG_NR_ENTRIES	512
> +#define PML_HEAD_INDEX		(PML_LOG_NR_ENTRIES - 1)
> +
> +#define PML_FULL_MAX_EXITS	64

[ ... ]

> +static void l1_pml_vmx_code(struct vmx_pages *vmx)
> +{
> +	int nr_pml_full = 0;
> +
> +	GUEST_ASSERT(vmx->vmcs_gpa);
> +	prepare_for_vmx_operation(vmx);
> +	load_vmcs(vmx);
> +
> +	/* PML requires EPT. */
> +	GUEST_ASSERT(vmx->eptp_gpa);
> +
> +	prepare_vmcs(vmx, l2_pml_guest_code);
> +
> +	vmwrite(PML_ADDRESS, vmx->pml_gpa);
> +	vmwrite(GUEST_PML_INDEX, PML_LOG_NR_ENTRIES);
> +	vmwrite(SECONDARY_VM_EXEC_CONTROL,
> +		vmread(SECONDARY_VM_EXEC_CONTROL) | SECONDARY_EXEC_ENABLE_PML);
> +
> +	vmlaunch();
> +
> +	for (;;) {
> +		u64 reason = vmread(VM_EXIT_REASON);
> +
> +		if (reason == EXIT_REASON_VMCALL)
> +			break;
> +
> +		GUEST_ASSERT_EQ(reason, EXIT_REASON_PML_FULL);

[Severity: Medium]
Since the test intentionally sets GUEST_PML_INDEX to 512, shouldn't this
result in a VM-entry failure instead of a successful entry followed by a
PML_FULL exit?

According to the Intel SDM (Volume 3C, Section 26.3.1.5 "Checks on Guest
Non-Register State"), the guest PML index must be less than 512 when PML
is enabled.

Does this assertion rely on KVM improperly omitting the guest state check?
If KVM is updated in the future to correctly emulate the architecturally
mandated check and fail the VM-entry, will this assertion break the test
suite?

> +		GUEST_ASSERT(++nr_pml_full < PML_FULL_MAX_EXITS);
> +
> +		vmwrite(GUEST_PML_INDEX, PML_HEAD_INDEX);
> +		vmresume();
> +	}
> +
> +	GUEST_SYNC_ARGS(vmread(GUEST_PML_INDEX), nr_pml_full, 0, 0, 0);
> +	GUEST_DONE();
> +}

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