Re: [PATCH 12/17] KVM: selftests: Add nested VMX APIC cache invalidation test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: selftests: test randomly fails on KVM_EXIT_INTERNAL_ERROR
--

commit 9c12532066400c0055054c635c5df9f04407d1a2
Author: Fred Griffoul <fgriffo@xxxxxxxxxxxx>

KVM: selftests: Add nested VMX APIC cache invalidation test

Introduce selftest to verify nested VMX APIC virtualization page cache
invalidation and refresh mechanisms for pfncache implementation.

> diff --git a/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c b/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c
> new file mode 100644
> index 0000000000000..8dd15c92fa906
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c
[ ... ]
> +static void l1_guest_code(struct vmx_pages *vmx_pages)
> +{
[ ... ]
> +again:
> +	exit_reason = vmreadz(VM_EXIT_REASON);
> +	if (exit_reason == EXIT_REASON_APIC_ACCESS) {
> +		uint64_t guest_rip = vmreadz(GUEST_RIP);
> +		uint64_t instr_len = vmreadz(VM_EXIT_INSTRUCTION_LEN);
> +
> +		vmwrite(GUEST_RIP, guest_rip + instr_len);
> +		GUEST_ASSERT(!vmresume());

[Severity: Medium]
If the VAPIC_SLOT memslot is moved asynchronously and leaves the VAPIC_GPA
unbacked just prior to this VMRESUME call, will KVM treat the unbacked
APIC access page at VM-entry as a misconfiguration and exit to userspace
with KVM_EXIT_INTERNAL_ERROR?

> +		goto again;
> +	}
[ ... ]
> +static void *update_move_memslot(void *arg)
> +{
> +	struct update_control *ctrl = arg;
> +	uint64_t gpa = VAPIC_GPA;
> +
> +	wait_for_start_signal(ctrl);
> +
> +	while (is_running(ctrl)) {
> +		usleep(update_period_ms * 1000);
> +		gpa += 0x10000;
> +		vm_mem_region_move(ctrl->vm, VAPIC_SLOT, gpa);

[Severity: Medium]
Does this background memslot move leave the VAPIC_GPA unbacked precisely
when L1 is about to execute VMRESUME?

> +		ctrl->updates++;
> +	}
> +
> +	return NULL;
> +}
[ ... ]
> +static void run(void * (*update)(void *), const char *name)
> +{
[ ... ]
> +	while (!done) {
> +		vcpu_run(vcpu);
> +
> +		switch (vcpu->run->exit_reason) {
[ ... ]
> +		default:
> +			TEST_FAIL("%s: Unexpected exit reason: %d (flags 0x%x)",
> +				  progname,
> +				  vcpu->run->exit_reason, vcpu->run->flags);

[Severity: Medium]
Does this unconditionally fail the test if it encounters the resulting
KVM_EXIT_INTERNAL_ERROR?

If update_move_memslot() occasionally leaves the VAPIC_GPA unbacked
precisely while L1 handles an EXIT_REASON_APIC_ACCESS, the resulting
KVM_EXIT_INTERNAL_ERROR will cause this test to fail randomly in
CI environments.

> +		}
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@xxxxxxxxxxxxx?part=12