[PATCH] KVM: selftests: hyperv_features: test write of 1 to HV_X64_MSR_RESET

[PATCH] KVM: selftests: hyperv_features: test write of 1 to HV_X64_MSR_RESET

[Piotr Zarycki]

Writing 1 to HV_X64_MSR_RESET triggers a real vCPU reset; the test
was writing 0 because the host loop was not prepared to handle the
resulting KVM_EXIT_SYSTEM_EVENT. Add the missing handling and write
1 to actually exercise the reset path.

Signed-off-by: Piotr Zarycki <piotr.zarycki@gmail.com>
---
 .../selftests/kvm/x86/hyperv_features.c       | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/hyperv_features.c b/tools/testing/selftests/kvm/x86/hyperv_features.c
index 7347f1fe5157..7c8afe43eef2 100644
--- a/tools/testing/selftests/kvm/x86/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86/hyperv_features.c
@@ -26,6 +26,7 @@ struct msr_data {
 	bool fault_expected;
 	bool write;
 	u64 write_val;
+	bool reset_expected;
 };
 
 struct hcall_data {
@@ -267,14 +268,9 @@ static void guest_test_msrs_access(void)
 		case 16:
 			msr->idx = HV_X64_MSR_RESET;
 			msr->write = true;
-			/*
-			 * TODO: the test only writes '0' to HV_X64_MSR_RESET
-			 * at the moment, writing some other value there will
-			 * trigger real vCPU reset and the code is not prepared
-			 * to handle it yet.
-			 */
-			msr->write_val = 0;
+			msr->write_val = 1;
 			msr->fault_expected = false;
+			msr->reset_expected = true;
 			break;
 
 		case 17:
@@ -497,6 +493,15 @@ static void guest_test_msrs_access(void)
 			 msr->idx, msr->write ? "write" : "read");
 
 		vcpu_run(vcpu);
+
+		if (msr->reset_expected) {
+			TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_SYSTEM_EVENT);
+			TEST_ASSERT(vcpu->run->system_event.type == KVM_SYSTEM_EVENT_RESET,
+				    "Expected reset system event, got type %u",
+				    vcpu->run->system_event.type);
+			goto next_stage;
+		}
+
 		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
 
 		switch (get_ucall(vcpu, &uc)) {
-- 
2.54.0

Re: [PATCH] KVM: selftests: hyperv_features: test write of 1 to HV_X64_MSR_RESET

[Piotr Zarycki]

Hi,

After receiving KVM_SYSTEM_EVENT_RESET the test simply advances to the
next stage without resetting the vCPU. Is verifying
KVM_SYSTEM_EVENT_RESET is delivered sufficient?

Thanks,
Piotr

Re: [PATCH] KVM: selftests: hyperv_features: test write of 1 to HV_X64_MSR_RESET

[Vitaly Kuznetsov]

Piotr Zarycki <piotr.zarycki@gmail.com> writes:

> Hi,
>
> After receiving KVM_SYSTEM_EVENT_RESET the test simply advances to the
> next stage without resetting the vCPU. Is verifying
> KVM_SYSTEM_EVENT_RESET is delivered sufficient?
>

For every stage of this test we're basically using a new VM
(vm_create_with_one_vcpu()) thus it does not really matter much what we
do after receiving KVM_SYSTEM_EVENT_RESET.

The basic idea behind the test was to verify that
KVM_CAP_HYPERV_ENFORCE_CPUID works as expected, i.e. the guest gets
access to the MSRs/hypercalls according to the guest visible CPUIDs and
not to everything KVM knows about (I guess we could've named it
'hyperv_feature_access' to actually reflect that). It, however, makes
sense to actually test side-effects of writing to MSRs/using hypercalls,
especially when it's relatively easy and I see that the change here goes
in this direction. With this, I think it is a good and sufficient
change, so

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Thanks!

-- 
Vitaly