[PATCH] KVM: selftests: Test the PV_UNHALT feature, not just its CPUID bit

[PATCH] KVM: selftests: Test the PV_UNHALT feature, not just its CPUID bit

[Hemanth Selam]

test_pv_unhalt() only checks that KVM clears KVM_FEATURE_PV_UNHALT from
guest CPUID when HLT-exiting is disabled.  The feature itself has never
been exercised, hence the FIXME.

Add a two vCPU test for it.  The first vCPU halts with interrupts
disabled, so nothing except the KVM_HC_KICK_CPU issued by the second can
resume it: KVM delivers the kick as APIC_DM_REMRD, which sets pv_unhalted
and makes the vCPU runnable without injecting an interrupt.  Reaching the
instruction after HLT is therefore proof that the kick arrived.

Wait for the halter's halt_exits to tick before kicking so that the kick
lands on a vCPU that has actually halted, and bound the wait so a vCPU
that never halts fails the test instead of hanging it.

The kicking vCPU runs with KVM_CAP_ENFORCE_PV_FEATURE_CPUID enabled and
PV_UNHALT advertised, so that KVM services the hypercall because the
feature is exposed rather than because enforcement is off.

Assisted-by: Cursor:claude-opus-5
Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
---
Built and run on x86_64 (AMD).  Untested on Intel, though the kick is
handled in common code and delivered through the generic LAPIC path.

 - On kvm-x86/next, built warning-free and passed 10 of 10 runs, and 20 of
   20 runs on a mainline build.

 - Also run inside a VM booted on a kernel built from kvm-x86/next, so
   against the KVM this targets rather than the host's: 6 of 6 runs passed,
   which also exercises the halt/kick ordering under much slower timing.

 - Whole x86 selftest suite with this applied: 61 passed, 24 skipped, and
   set_sregs_test failed with "KVM allowed invalid efer bit (0x100)".  That
   one fails identically with this patch reverted, i.e. it is the host
   kernel, not this change.

The test was checked against three deliberate breakages, to make sure it
can only pass when the kick really works:

 - drop the KVM_HC_KICK_CPU call from the kicking vCPU: the halted vCPU is
   never resumed and the test times out, i.e. nothing else wakes it;

 - clear PV_UNHALT from the kicking vCPU's CPUID while enforcement is on:
   the hypercall returns -KVM_ENOSYS and the test fails with

     0xfffffffffffffc18 != 0x0 (kvm_hypercall(KVM_HC_KICK_CPU, ...) != 0)

 - remove the halt from the halting vCPU: the bounded wait trips and the
   test fails with "vCPU never halted" rather than hanging.

The explicit vcpu_set_cpuid_feature() is redundant today, since KVM
advertises PV_UNHALT by default while HLT-exiting is enabled - verified by
asserting on it - but it keeps the test honest if that ever changes.

 tools/testing/selftests/kvm/x86/kvm_pv_test.c | 93 ++++++++++++++++++-
 1 file changed, 92 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/x86/kvm_pv_test.c b/tools/testing/selftests/kvm/x86/kvm_pv_test.c
index 8ed5fa635021..d14d55d3a9ba 100644
--- a/tools/testing/selftests/kvm/x86/kvm_pv_test.c
+++ b/tools/testing/selftests/kvm/x86/kvm_pv_test.c
@@ -6,8 +6,10 @@
  */
 #include <asm/kvm_para.h>
 #include <linux/kvm_para.h>
+#include <pthread.h>
 #include <stdint.h>
 
+#include "apic.h"
 #include "test_util.h"
 #include "kvm_util.h"
 #include "processor.h"
@@ -193,7 +195,95 @@ static void test_pv_unhalt(void)
 	TEST_ASSERT(!vcpu_cpuid_has(vcpu, X86_FEATURE_KVM_PV_UNHALT),
 		    "PV_UNHALT set in guest CPUID when HLT-exiting is disabled");
 
-	/* FIXME: actually test KVM_FEATURE_PV_UNHALT feature */
+	kvm_vm_free(vm);
+}
+
+static void pv_unhalt_halter_guest_code(void)
+{
+	/*
+	 * Interrupts are disabled, so nothing except the KVM_HC_KICK_CPU from
+	 * the other vCPU can end the halt, i.e. reaching GUEST_DONE() proves
+	 * the kick was delivered.
+	 */
+	asm volatile("cli; hlt");
+
+	GUEST_DONE();
+}
+
+static void pv_unhalt_kicker_guest_code(u32 halter_apic_id)
+{
+	GUEST_ASSERT_EQ(kvm_hypercall(KVM_HC_KICK_CPU, halter_apic_id, 0, 0, 0), 0);
+	GUEST_DONE();
+}
+
+static void run_guest_to_done(struct kvm_vcpu *vcpu)
+{
+	struct ucall uc;
+	u64 cmd;
+
+	vcpu_run(vcpu);
+	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
+
+	cmd = get_ucall(vcpu, &uc);
+	if (cmd == UCALL_ABORT)
+		REPORT_GUEST_ASSERT(uc);
+	TEST_ASSERT_EQ(cmd, UCALL_DONE);
+}
+
+static void *pv_unhalt_halter_thread(void *vcpu)
+{
+	run_guest_to_done(vcpu);
+	return NULL;
+}
+
+static u32 vcpu_get_apic_id(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic_state lapic;
+
+	vcpu_ioctl(vcpu, KVM_GET_LAPIC, &lapic);
+	return GET_APIC_ID_FIELD(*(u32 *)&lapic.regs[APIC_ID]);
+}
+
+static void test_pv_unhalt_kick(void)
+{
+	struct kvm_vcpu *halter, *kicker;
+	struct timespec start;
+	struct kvm_vm *vm;
+	pthread_t thread;
+	int r;
+
+	pr_info("testing KVM_HC_KICK_CPU\n");
+
+	vm = vm_create_with_one_vcpu(&halter, pv_unhalt_halter_guest_code);
+	kicker = vm_vcpu_add(vm, 1, pv_unhalt_kicker_guest_code);
+
+	/*
+	 * Enforce the PV CPUID so that KVM services the hypercall because
+	 * PV_UNHALT is advertised to the kicker, and not because enforcement
+	 * is off.  KVM advertises PV_UNHALT by default while HLT-exiting is
+	 * enabled; set it explicitly so that the test keeps testing the
+	 * feature if that ever changes.
+	 */
+	vcpu_enable_cap(kicker, KVM_CAP_ENFORCE_PV_FEATURE_CPUID, 1);
+	vcpu_set_cpuid_feature(kicker, X86_FEATURE_KVM_PV_UNHALT);
+	vcpu_args_set(kicker, 1, vcpu_get_apic_id(halter));
+
+	r = pthread_create(&thread, NULL, pv_unhalt_halter_thread, halter);
+	TEST_ASSERT(!r, "pthread_create halter failed, errno=%d", errno);
+
+	/* Kick only once the halter has taken its HLT exit. */
+	clock_gettime(CLOCK_MONOTONIC, &start);
+	while (!vcpu_get_stat(halter, halt_exits)) {
+		TEST_ASSERT(timespec_elapsed(start).tv_sec < 10,
+			    "vCPU never halted");
+		usleep(100);
+	}
+
+	run_guest_to_done(kicker);
+
+	/* Nothing except the kick can get the halter to GUEST_DONE(). */
+	r = pthread_join(thread, NULL);
+	TEST_ASSERT(!r, "pthread_join halter failed, errno=%d", errno);
 
 	kvm_vm_free(vm);
 }
@@ -215,4 +305,5 @@ int main(void)
 	kvm_vm_free(vm);
 
 	test_pv_unhalt();
+	test_pv_unhalt_kick();
 }
-- 
2.43.7

Re: [PATCH] KVM: selftests: Test the PV_UNHALT feature, not just its CPUID bit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The arguments to `KVM_HC_KICK_CPU` are swapped in the selftest's `kvm_hypercall()` invocation, causing the test to falsely pass only because the target vCPU's APIC ID happens to be 0.
- [Low] Incorrect error reporting for `pthread_create` and `pthread_join`, as they do not set `errno`.
--

commit 6ba52c6d2b00ca080ff07b971fc3f8bbdf50de71
Author: Hemanth Selam <hemanth.selam@gmail.com>

KVM: selftests: Test the PV_UNHALT feature, not just its CPUID bit

This commit implements a two-vCPU test for the KVM PV_UNHALT feature to
actually exercise it instead of just checking its CPUID bit. It verifies
that one vCPU can successfully kick and wake up a halted vCPU using
KVM_HC_KICK_CPU.

> diff --git a/tools/testing/selftests/kvm/x86/kvm_pv_test.c b/tools/testing/selftests/kvm/x86/kvm_pv_test.c
> index 8ed5fa6350211..d14d55d3a9ba7 100644
> --- a/tools/testing/selftests/kvm/x86/kvm_pv_test.c
> +++ b/tools/testing/selftests/kvm/x86/kvm_pv_test.c
[ ... ]
> +static void pv_unhalt_kicker_guest_code(u32 halter_apic_id)
> +{
> +	GUEST_ASSERT_EQ(kvm_hypercall(KVM_HC_KICK_CPU, halter_apic_id, 0, 0, 0), 0);

[Severity: Medium]
Are the arguments to kvm_hypercall() in the correct order here?

Passing halter_apic_id as the first argument places it into a0 instead of a1.
Since KVM's handler uses a1 for the APIC ID, it will always kick APIC ID 0
instead of the dynamically determined halter_apic_id.

Does this regression falsely allow the test to pass only because the target
vCPU is created first and its APIC ID happens to be 0?

> +	GUEST_DONE();
> +}
[ ... ]
> +	r = pthread_create(&thread, NULL, pv_unhalt_halter_thread, halter);
> +	TEST_ASSERT(!r, "pthread_create halter failed, errno=%d", errno);

[Severity: Low]
Does pthread_create() set errno?

It returns the error code directly in r. Should this print r instead of
errno to avoid showing a stale or zero value when it fails?

[ ... ]
> +	/* Nothing except the kick can get the halter to GUEST_DONE(). */
> +	r = pthread_join(thread, NULL);
> +	TEST_ASSERT(!r, "pthread_join halter failed, errno=%d", errno);

[Severity: Low]
Similarly here, pthread_join() returns the error code directly. Should this
log r instead of errno?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826101627.2873660-1-hemanth.selam@gmail.com?part=1

Re: [PATCH] KVM: selftests: Test the PV_UNHALT feature, not just its CPUID bit

[Hemanth Selam]

Please ignore this version, the report about the hypercall arguments is
correct.  KVM takes the APIC ID to kick in a1, as kvm_kick_cpu() does, so
this asked for APIC ID 0 to be kicked and only passed because the halting
vCPU was vCPU 0.

v2 halts on a vCPU with a non-zero APIC ID so that a kick sent to the wrong
vCPU can no longer pass by accident, and I confirmed that reintroducing the
v1 argument order makes it fail:

  https://lore.kernel.org/all/20260826115900.2882139-1-hemanth.selam@gmail.com

Sorry for the noise.

Hemanth