KVM: selftests: Mostly fix broken Hyper-V Features test

Explicitly do all setup at every stage of the Hyper-V Features test, e.g.
set the MSR/hypercall, enable capabilities, etc...  Now that the VM is
recreated for every stage, values that are written into the VM's address
space, i.e. shared with the guest, are reset between sub-tests, as are
any capabilities, etc...

Fix the hypercall params as well, which were broken in the same rework.
The "hcall" struct/pointer needs to point at the hcall_params object, not
the set of hypercall pages.

The goofs were hidden by the test's dubious behavior of using '0' to
signal "done", i.e. the MSR test ran exactly one sub-test, and the
hypercall test was a gigantic nop.

Fixes: 276ff22ed440 ("KVM: selftests: Avoid KVM_SET_CPUID2 after KVM_RUN in hyperv_features test")
Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-Id: <20220608224516.3788274-4-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index d5f37495ade8c4d08d183a312b6235e59dc5e4c7..1a2a06c3027aa6a411a4f455f236648bc634bd33 100644 (file)
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -101,52 +101,44 @@ struct hcall_data {
 
 static void guest_msr(struct msr_data *msr)
 {
-       int i = 0;
+       GUEST_ASSERT(msr->idx);
 
-       while (msr->idx) {
-               WRITE_ONCE(nr_gp, 0);
-               if (!msr->write)
-                       do_rdmsr(msr->idx);
-               else
-                       do_wrmsr(msr->idx, msr->write_val);
-
-               if (msr->available)
-                       GUEST_ASSERT(READ_ONCE(nr_gp) == 0);
-               else
-                       GUEST_ASSERT(READ_ONCE(nr_gp) == 1);
-
-               GUEST_SYNC(i++);
-       }
+       WRITE_ONCE(nr_gp, 0);
+       if (!msr->write)
+               do_rdmsr(msr->idx);
+       else
+               do_wrmsr(msr->idx, msr->write_val);
 
+       if (msr->available)
+               GUEST_ASSERT(READ_ONCE(nr_gp) == 0);
+       else
+               GUEST_ASSERT(READ_ONCE(nr_gp) == 1);
        GUEST_DONE();
 }
 
 static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
 {
-       int i = 0;
        u64 res, input, output;
 
+       GUEST_ASSERT(hcall->control);
+
        wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
        wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
 
-       while (hcall->control) {
-               nr_ud = 0;
-               if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
-                       input = pgs_gpa;
-                       output = pgs_gpa + 4096;
-               } else {
-                       input = output = 0;
-               }
-
-               res = hypercall(hcall->control, input, output);
-               if (hcall->ud_expected)
-                       GUEST_ASSERT(nr_ud == 1);
-               else
-                       GUEST_ASSERT(res == hcall->expect);
-
-               GUEST_SYNC(i++);
+       nr_ud = 0;
+       if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
+               input = pgs_gpa;
+               output = pgs_gpa + 4096;
+       } else {
+               input = output = 0;
        }
 
+       res = hypercall(hcall->control, input, output);
+       if (hcall->ud_expected)
+               GUEST_ASSERT(nr_ud == 1);
+       else
+               GUEST_ASSERT(res == hcall->expect);
+
        GUEST_DONE();
 }
 
@@ -204,6 +196,10 @@ static void guest_test_msrs_access(void)
 
                run = vcpu->run;
 
+               /* TODO: Make this entire test easier to maintain. */
+               if (stage >= 21)
+                       vcpu_enable_cap(vcpu, KVM_CAP_HYPERV_SYNIC2, 0);
+
                switch (stage) {
                case 0:
                        /*
@@ -247,11 +243,13 @@ static void guest_test_msrs_access(void)
                        break;
                case 6:
                        feat.eax |= HV_MSR_VP_RUNTIME_AVAILABLE;
+                       msr->idx = HV_X64_MSR_VP_RUNTIME;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 7:
                        /* Read only */
+                       msr->idx = HV_X64_MSR_VP_RUNTIME;
                        msr->write = 1;
                        msr->write_val = 1;
                        msr->available = 0;
@@ -264,11 +262,13 @@ static void guest_test_msrs_access(void)
                        break;
                case 9:
                        feat.eax |= HV_MSR_TIME_REF_COUNT_AVAILABLE;
+                       msr->idx = HV_X64_MSR_TIME_REF_COUNT;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 10:
                        /* Read only */
+                       msr->idx = HV_X64_MSR_TIME_REF_COUNT;
                        msr->write = 1;
                        msr->write_val = 1;
                        msr->available = 0;
@@ -281,11 +281,13 @@ static void guest_test_msrs_access(void)
                        break;
                case 12:
                        feat.eax |= HV_MSR_VP_INDEX_AVAILABLE;
+                       msr->idx = HV_X64_MSR_VP_INDEX;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 13:
                        /* Read only */
+                       msr->idx = HV_X64_MSR_VP_INDEX;
                        msr->write = 1;
                        msr->write_val = 1;
                        msr->available = 0;
@@ -298,10 +300,12 @@ static void guest_test_msrs_access(void)
                        break;
                case 15:
                        feat.eax |= HV_MSR_RESET_AVAILABLE;
+                       msr->idx = HV_X64_MSR_RESET;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 16:
+                       msr->idx = HV_X64_MSR_RESET;
                        msr->write = 1;
                        msr->write_val = 0;
                        msr->available = 1;
@@ -314,10 +318,12 @@ static void guest_test_msrs_access(void)
                        break;
                case 18:
                        feat.eax |= HV_MSR_REFERENCE_TSC_AVAILABLE;
+                       msr->idx = HV_X64_MSR_REFERENCE_TSC;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 19:
+                       msr->idx = HV_X64_MSR_REFERENCE_TSC;
                        msr->write = 1;
                        msr->write_val = 0;
                        msr->available = 1;
@@ -333,14 +339,18 @@ static void guest_test_msrs_access(void)
                         * Remains unavailable even with KVM_CAP_HYPERV_SYNIC2
                         * capability enabled and guest visible CPUID bit unset.
                         */
-                       vcpu_enable_cap(vcpu, KVM_CAP_HYPERV_SYNIC2, 0);
+                       msr->idx = HV_X64_MSR_EOM;
+                       msr->write = 0;
+                       msr->available = 0;
                        break;
                case 22:
                        feat.eax |= HV_MSR_SYNIC_AVAILABLE;
+                       msr->idx = HV_X64_MSR_EOM;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 23:
+                       msr->idx = HV_X64_MSR_EOM;
                        msr->write = 1;
                        msr->write_val = 0;
                        msr->available = 1;
@@ -353,22 +363,28 @@ static void guest_test_msrs_access(void)
                        break;
                case 25:
                        feat.eax |= HV_MSR_SYNTIMER_AVAILABLE;
+                       msr->idx = HV_X64_MSR_STIMER0_CONFIG;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 26:
+                       msr->idx = HV_X64_MSR_STIMER0_CONFIG;
                        msr->write = 1;
                        msr->write_val = 0;
                        msr->available = 1;
                        break;
                case 27:
                        /* Direct mode test */
+                       msr->idx = HV_X64_MSR_STIMER0_CONFIG;
                        msr->write = 1;
                        msr->write_val = 1 << 12;
                        msr->available = 0;
                        break;
                case 28:
                        feat.edx |= HV_STIMER_DIRECT_MODE_AVAILABLE;
+                       msr->idx = HV_X64_MSR_STIMER0_CONFIG;
+                       msr->write = 1;
+                       msr->write_val = 1 << 12;
                        msr->available = 1;
                        break;
 
@@ -379,6 +395,7 @@ static void guest_test_msrs_access(void)
                        break;
                case 30:
                        feat.eax |= HV_MSR_APIC_ACCESS_AVAILABLE;
+                       msr->idx = HV_X64_MSR_EOI;
                        msr->write = 1;
                        msr->write_val = 1;
                        msr->available = 1;
@@ -391,11 +408,13 @@ static void guest_test_msrs_access(void)
                        break;
                case 32:
                        feat.eax |= HV_ACCESS_FREQUENCY_MSRS;
+                       msr->idx = HV_X64_MSR_TSC_FREQUENCY;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 33:
                        /* Read only */
+                       msr->idx = HV_X64_MSR_TSC_FREQUENCY;
                        msr->write = 1;
                        msr->write_val = 1;
                        msr->available = 0;
@@ -408,10 +427,12 @@ static void guest_test_msrs_access(void)
                        break;
                case 35:
                        feat.eax |= HV_ACCESS_REENLIGHTENMENT;
+                       msr->idx = HV_X64_MSR_REENLIGHTENMENT_CONTROL;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 36:
+                       msr->idx = HV_X64_MSR_REENLIGHTENMENT_CONTROL;
                        msr->write = 1;
                        msr->write_val = 1;
                        msr->available = 1;
@@ -431,10 +452,12 @@ static void guest_test_msrs_access(void)
                        break;
                case 39:
                        feat.edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
+                       msr->idx = HV_X64_MSR_CRASH_P0;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 40:
+                       msr->idx = HV_X64_MSR_CRASH_P0;
                        msr->write = 1;
                        msr->write_val = 1;
                        msr->available = 1;
@@ -448,28 +471,26 @@ static void guest_test_msrs_access(void)
                case 42:
                        feat.edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
                        dbg.eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+                       msr->idx = HV_X64_MSR_SYNDBG_STATUS;
                        msr->write = 0;
                        msr->available = 1;
                        break;
                case 43:
+                       msr->idx = HV_X64_MSR_SYNDBG_STATUS;
                        msr->write = 1;
                        msr->write_val = 0;
                        msr->available = 1;
                        break;
 
                case 44:
-                       /* END */
-                       msr->idx = 0;
-                       break;
+                       kvm_vm_free(vm);
+                       return;
                }
 
                hv_set_cpuid(vcpu, best, &feat, &recomm, &dbg);
 
-               if (msr->idx)
-                       pr_debug("Stage %d: testing msr: 0x%x for %s\n", stage,
-                                msr->idx, msr->write ? "write" : "read");
-               else
-                       pr_debug("Stage %d: finish\n", stage);
+               pr_debug("Stage %d: testing msr: 0x%x for %s\n", stage,
+                        msr->idx, msr->write ? "write" : "read");
 
                vcpu_run(vcpu);
                TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
@@ -477,16 +498,14 @@ static void guest_test_msrs_access(void)
                            run->exit_reason, exit_reason_str(run->exit_reason));
 
                switch (get_ucall(vcpu, &uc)) {
-               case UCALL_SYNC:
-                       TEST_ASSERT(uc.args[1] == 0,
-                                   "Unexpected stage: %ld (0 expected)\n",
-                                   uc.args[1]);
-                       break;
                case UCALL_ABORT:
                        TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
                                  __FILE__, uc.args[1]);
                        return;
                case UCALL_DONE:
+                       break;
+               default:
+                       TEST_FAIL("Unhandled ucall: %ld", uc.cmd);
                        return;
                }
 
@@ -525,11 +544,11 @@ static void guest_test_hcalls_access(void)
 
                /* Hypercall input/output */
                hcall_page = vm_vaddr_alloc_pages(vm, 2);
-               hcall = addr_gva2hva(vm, hcall_page);
                memset(addr_gva2hva(vm, hcall_page), 0x0, 2 * getpagesize());
 
                hcall_params = vm_vaddr_alloc_page(vm);
                memset(addr_gva2hva(vm, hcall_params), 0x0, getpagesize());
+               hcall = addr_gva2hva(vm, hcall_params);
 
                vcpu_args_set(vcpu, 2, addr_gva2gpa(vm, hcall_page), hcall_params);
                vcpu_enable_cap(vcpu, KVM_CAP_HYPERV_ENFORCE_CPUID, 1);
@@ -552,6 +571,7 @@ static void guest_test_hcalls_access(void)
                        break;
                case 2:
                        feat.ebx |= HV_POST_MESSAGES;
+                       hcall->control = HVCALL_POST_MESSAGE;
                        hcall->expect = HV_STATUS_INVALID_HYPERCALL_INPUT;
                        break;
 
@@ -561,6 +581,7 @@ static void guest_test_hcalls_access(void)
                        break;
                case 4:
                        feat.ebx |= HV_SIGNAL_EVENTS;
+                       hcall->control = HVCALL_SIGNAL_EVENT;
                        hcall->expect = HV_STATUS_INVALID_HYPERCALL_INPUT;
                        break;
 
@@ -570,10 +591,12 @@ static void guest_test_hcalls_access(void)
                        break;
                case 6:
                        dbg.eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+                       hcall->control = HVCALL_RESET_DEBUG_SESSION;
                        hcall->expect = HV_STATUS_ACCESS_DENIED;
                        break;
                case 7:
                        feat.ebx |= HV_DEBUGGING;
+                       hcall->control = HVCALL_RESET_DEBUG_SESSION;
                        hcall->expect = HV_STATUS_OPERATION_DENIED;
                        break;
 
@@ -583,6 +606,7 @@ static void guest_test_hcalls_access(void)
                        break;
                case 9:
                        recomm.eax |= HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED;
+                       hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE;
                        hcall->expect = HV_STATUS_SUCCESS;
                        break;
                case 10:
@@ -591,6 +615,7 @@ static void guest_test_hcalls_access(void)
                        break;
                case 11:
                        recomm.eax |= HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED;
+                       hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX;
                        hcall->expect = HV_STATUS_SUCCESS;
                        break;
 
@@ -600,6 +625,7 @@ static void guest_test_hcalls_access(void)
                        break;
                case 13:
                        recomm.eax |= HV_X64_CLUSTER_IPI_RECOMMENDED;
+                       hcall->control = HVCALL_SEND_IPI;
                        hcall->expect = HV_STATUS_INVALID_HYPERCALL_INPUT;
                        break;
                case 14:
@@ -614,6 +640,7 @@ static void guest_test_hcalls_access(void)
                        break;
                case 16:
                        recomm.ebx = 0xfff;
+                       hcall->control = HVCALL_NOTIFY_LONG_SPIN_WAIT;
                        hcall->expect = HV_STATUS_SUCCESS;
                        break;
                case 17:
@@ -623,23 +650,18 @@ static void guest_test_hcalls_access(void)
                        break;
                case 18:
                        feat.edx |= HV_X64_HYPERCALL_XMM_INPUT_AVAILABLE;
+                       hcall->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE | HV_HYPERCALL_FAST_BIT;
                        hcall->ud_expected = false;
                        hcall->expect = HV_STATUS_SUCCESS;
                        break;
-
                case 19:
-                       /* END */
-                       hcall->control = 0;
-                       break;
+                       kvm_vm_free(vm);
+                       return;
                }
 
                hv_set_cpuid(vcpu, best, &feat, &recomm, &dbg);
 
-               if (hcall->control)
-                       pr_debug("Stage %d: testing hcall: 0x%lx\n", stage,
-                                hcall->control);
-               else
-                       pr_debug("Stage %d: finish\n", stage);
+               pr_debug("Stage %d: testing hcall: 0x%lx\n", stage, hcall->control);
 
                vcpu_run(vcpu);
                TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
@@ -647,16 +669,14 @@ static void guest_test_hcalls_access(void)
                            run->exit_reason, exit_reason_str(run->exit_reason));
 
                switch (get_ucall(vcpu, &uc)) {
-               case UCALL_SYNC:
-                       TEST_ASSERT(uc.args[1] == 0,
-                                   "Unexpected stage: %ld (0 expected)\n",
-                                   uc.args[1]);
-                       break;
                case UCALL_ABORT:
                        TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
                                  __FILE__, uc.args[1]);
                        return;
                case UCALL_DONE:
+                       break;
+               default:
+                       TEST_FAIL("Unhandled ucall: %ld", uc.cmd);
                        return;
                }