| Message ID | 20230712075910.22480-3-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use TAP in some more x86 KVM selftests | expand |
On Wed, Jul 12, 2023, Thomas Huth wrote: > The sync_regs test currently does not have any output (unless one > of the TEST_ASSERT statement fails), so it's hard to say for a user > whether a certain new sub-test has been included in the binary or > not. Let's make this a little bit more user-friendly and include > some TAP output via the kselftest_harness.h interface. > To be able to use the interface, we have to break up the huge main() > function here in more fine grained parts - then we can use the > TEST_F() macro to define the individual tests. Since these are run > with a separate VM now, we have also to make sure to create the > expected state at the beginning of each test, so some parts grow > a little bit - which should be OK considering that the individual > tests are more self-contained now. > > Suggested-by: David Matlack <dmatlack@google.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > .../selftests/kvm/x86_64/sync_regs_test.c | 113 +++++++++++++++--- FYI, there's an in-flight patch[*] to expand this test's coverage, and I plan on grabbing that in some form before this one (sorry). Let me know if there are any tweaks that can be done to Michal's patch to make it easier to convert the test to tap. I'll also try to get Michal's patch into kvm-x86/next sooner than later so that you can use that as the basic. Oh, and no need to post "KVM: selftests: Rename the ASSERT_EQ macro" in the next version, I'm planning on grabbing that one straightaway. [*] https://lore.kernel.org/all/20230728001606.2275586-3-mhal@rbox.co > 1 file changed, 98 insertions(+), 15 deletions(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c > index 2da89fdc2471a..e1359a4a07fea 100644 > --- a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c > +++ b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c > @@ -16,6 +16,7 @@ > #include <string.h> > #include <sys/ioctl.h> > > +#include "kselftest_harness.h" > #include "test_util.h" > #include "kvm_util.h" > #include "processor.h" > @@ -80,23 +81,24 @@ static void compare_vcpu_events(struct kvm_vcpu_events *left, > #define TEST_SYNC_FIELDS (KVM_SYNC_X86_REGS|KVM_SYNC_X86_SREGS|KVM_SYNC_X86_EVENTS) > #define INVALID_SYNC_FIELD 0x80000000 > > -int main(int argc, char *argv[]) > -{ > - struct kvm_vcpu *vcpu; > +FIXTURE(sync_regs_test) { > struct kvm_vm *vm; > - struct kvm_run *run; > - struct kvm_regs regs; > - struct kvm_sregs sregs; > - struct kvm_vcpu_events events; > - int rv, cap; > + struct kvm_vcpu *vcpu; > +}; > > - cap = kvm_check_cap(KVM_CAP_SYNC_REGS); > - TEST_REQUIRE((cap & TEST_SYNC_FIELDS) == TEST_SYNC_FIELDS); > - TEST_REQUIRE(!(cap & INVALID_SYNC_FIELD)); > +FIXTURE_SETUP(sync_regs_test) { > + self->vm = vm_create_with_one_vcpu(&self->vcpu, guest_code); > +} > > - vm = vm_create_with_one_vcpu(&vcpu, guest_code); > +FIXTURE_TEARDOWN(sync_regs_test) { > + kvm_vm_free(self->vm); > +} > > - run = vcpu->run; > +TEST_F(sync_regs_test, read_invalid) > +{ > + struct kvm_vcpu *vcpu = self->vcpu; > + struct kvm_run *run = vcpu->run; > + int rv; > > /* Request reading invalid register set from VCPU. */ > run->kvm_valid_regs = INVALID_SYNC_FIELD; > @@ -112,6 +114,13 @@ int main(int argc, char *argv[]) > "Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n", > rv); > run->kvm_valid_regs = 0; > +} > + > +TEST_F(sync_regs_test, set_invalid) > +{ > + struct kvm_vcpu *vcpu = self->vcpu; > + struct kvm_run *run = vcpu->run; > + int rv; > > /* Request setting invalid register set into VCPU. */ > run->kvm_dirty_regs = INVALID_SYNC_FIELD; > @@ -127,11 +136,22 @@ int main(int argc, char *argv[]) > "Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n", > rv); > run->kvm_dirty_regs = 0; > +} > + > +TEST_F(sync_regs_test, req_and_verify_all_valid) > +{ > + struct kvm_vcpu *vcpu = self->vcpu; > + struct kvm_run *run = vcpu->run; > + struct kvm_vcpu_events events; > + struct kvm_sregs sregs; > + struct kvm_regs regs; > + int rv; > > /* Request and verify all valid register sets. */ > /* TODO: BUILD TIME CHECK: TEST_ASSERT(KVM_SYNC_X86_NUM_FIELDS != 3); */ > run->kvm_valid_regs = TEST_SYNC_FIELDS; > rv = _vcpu_run(vcpu); > + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); Just use vcpu_run() instead of _vcpu_run(). And please post that as a separate patch, I think/hope it will make the conversion-to-tap patch smaller. > TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > > vcpu_regs_get(vcpu, ®s); > @@ -142,6 +162,22 @@ int main(int argc, char *argv[]) > > vcpu_events_get(vcpu, &events); > compare_vcpu_events(&events, &run->s.regs.events); > +} > + > +TEST_F(sync_regs_test, set_and_verify_various) > +{ > + struct kvm_vcpu *vcpu = self->vcpu; > + struct kvm_run *run = vcpu->run; > + struct kvm_vcpu_events events; > + struct kvm_sregs sregs; > + struct kvm_regs regs; > + int rv; > + > + /* Run once to get register set */ > + run->kvm_valid_regs = TEST_SYNC_FIELDS; > + rv = _vcpu_run(vcpu); > + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); Same comment here. > + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > > /* Set and verify various register values. */ > run->s.regs.regs.rbx = 0xBAD1DEA; > @@ -151,6 +187,7 @@ int main(int argc, char *argv[]) > run->kvm_valid_regs = TEST_SYNC_FIELDS; > run->kvm_dirty_regs = KVM_SYNC_X86_REGS | KVM_SYNC_X86_SREGS; > rv = _vcpu_run(vcpu); > + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); And here. > TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > TEST_ASSERT(run->s.regs.regs.rbx == 0xBAD1DEA + 1, > "rbx sync regs value incorrect 0x%llx.", > @@ -167,6 +204,13 @@ int main(int argc, char *argv[]) > > vcpu_events_get(vcpu, &events); > compare_vcpu_events(&events, &run->s.regs.events); > +} > + > +TEST_F(sync_regs_test, clear_kvm_dirty_regs_bits) > +{ > + struct kvm_vcpu *vcpu = self->vcpu; > + struct kvm_run *run = vcpu->run; > + int rv; > > /* Clear kvm_dirty_regs bits, verify new s.regs values are > * overwritten with existing guest values. > @@ -175,10 +219,25 @@ int main(int argc, char *argv[]) > run->kvm_dirty_regs = 0; > run->s.regs.regs.rbx = 0xDEADBEEF; > rv = _vcpu_run(vcpu); > + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); Here too. > TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > TEST_ASSERT(run->s.regs.regs.rbx != 0xDEADBEEF, > "rbx sync regs value incorrect 0x%llx.", > run->s.regs.regs.rbx); > +} > + > +TEST_F(sync_regs_test, clear_kvm_valid_and_dirty_regs) > +{ > + struct kvm_vcpu *vcpu = self->vcpu; > + struct kvm_run *run = vcpu->run; > + struct kvm_regs regs; > + int rv; > + > + /* Run once to get register set */ > + run->kvm_valid_regs = TEST_SYNC_FIELDS; > + rv = _vcpu_run(vcpu); > + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); At least you're consistent :-) > + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > > /* Clear kvm_valid_regs bits and kvm_dirty_bits. > * Verify s.regs values are not overwritten with existing guest values > @@ -187,9 +246,11 @@ int main(int argc, char *argv[]) > run->kvm_valid_regs = 0; > run->kvm_dirty_regs = 0; > run->s.regs.regs.rbx = 0xAAAA; > + vcpu_regs_get(vcpu, ®s); Can you split this change to its own patch too? I'm pretty sure that change stands on its own, and slotting it in here made me do a double-take. > regs.rbx = 0xBAC0; > vcpu_regs_set(vcpu, ®s); > rv = _vcpu_run(vcpu); > + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); > TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > TEST_ASSERT(run->s.regs.regs.rbx == 0xAAAA, > "rbx sync regs value incorrect 0x%llx.", > @@ -198,6 +259,20 @@ int main(int argc, char *argv[]) > TEST_ASSERT(regs.rbx == 0xBAC0 + 1, > "rbx guest value incorrect 0x%llx.", > regs.rbx); > +} > + > +TEST_F(sync_regs_test, clear_kvm_valid_regs_bits) > +{ > + struct kvm_vcpu *vcpu = self->vcpu; > + struct kvm_run *run = vcpu->run; > + struct kvm_regs regs; > + int rv; > + > + /* Run once to get register set */ > + run->kvm_valid_regs = TEST_SYNC_FIELDS; > + rv = _vcpu_run(vcpu); > + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); Once more, with feeling! > + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > > /* Clear kvm_valid_regs bits. Verify s.regs values are not overwritten > * with existing guest values but that guest values are overwritten > @@ -207,6 +282,7 @@ int main(int argc, char *argv[]) > run->kvm_dirty_regs = TEST_SYNC_FIELDS; > run->s.regs.regs.rbx = 0xBBBB; > rv = _vcpu_run(vcpu); > + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); Heh. > TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > TEST_ASSERT(run->s.regs.regs.rbx == 0xBBBB, > "rbx sync regs value incorrect 0x%llx.", > @@ -215,8 +291,15 @@ int main(int argc, char *argv[]) > TEST_ASSERT(regs.rbx == 0xBBBB + 1, > "rbx guest value incorrect 0x%llx.", > regs.rbx); > +} > + > +int main(int argc, char *argv[]) > +{ > + int cap; > > - kvm_vm_free(vm); > + cap = kvm_check_cap(KVM_CAP_SYNC_REGS); > + TEST_REQUIRE((cap & TEST_SYNC_FIELDS) == TEST_SYNC_FIELDS); > + TEST_REQUIRE(!(cap & INVALID_SYNC_FIELD)); > > - return 0; > + return test_harness_run(argc, argv); > } > -- > 2.39.3 >
On Wed, Aug 02, 2023, Sean Christopherson wrote: > Oh, and no need to post "KVM: selftests: Rename the ASSERT_EQ macro" in the next > version, I'm planning on grabbing that one straightaway. After paging this all back in... I would much prefer that we implement the KVM specific macros[*], e.g. KVM_ONE_VCPU_TEST(), and build on top of those. I'm definitely ok doing a "slow" conversion, i.e. starting with a few easy tests. IIRC at some point I said I strongly preferred an all-or-nothing approach, but realistically I don't think we'll make progress anytime soon if we try to boil the ocean. But I do think we should spend the time to implement the infrastructure right away. We may end up having to tweak the infrastructure down the road, e.g. to convert other tests, but I would rather do that then convert some tests twice. [*] https://lore.kernel.org/all/Y2v+B3xxYKJSM%2FfH@google.com
On 02/08/2023 21.55, Sean Christopherson wrote: > On Wed, Jul 12, 2023, Thomas Huth wrote: >> The sync_regs test currently does not have any output (unless one >> of the TEST_ASSERT statement fails), so it's hard to say for a user >> whether a certain new sub-test has been included in the binary or >> not. Let's make this a little bit more user-friendly and include >> some TAP output via the kselftest_harness.h interface. >> To be able to use the interface, we have to break up the huge main() >> function here in more fine grained parts - then we can use the >> TEST_F() macro to define the individual tests. Since these are run >> with a separate VM now, we have also to make sure to create the >> expected state at the beginning of each test, so some parts grow >> a little bit - which should be OK considering that the individual >> tests are more self-contained now. >> >> Suggested-by: David Matlack <dmatlack@google.com> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> .../selftests/kvm/x86_64/sync_regs_test.c | 113 +++++++++++++++--- > > FYI, there's an in-flight patch[*] to expand this test's coverage, and I plan on > grabbing that in some form before this one (sorry). Let me know if there are > any tweaks that can be done to Michal's patch to make it easier to convert the > test to tap. > > I'll also try to get Michal's patch into kvm-x86/next sooner than later so that > you can use that as the basic. Ok, I'll wait 'til the dust settles and then redo my patch (there is no hurry for this, and I'm only doing it in my spare minutes). Any chance that you could already take at least the other conversion patches from my series? ... >> +TEST_F(sync_regs_test, req_and_verify_all_valid) >> +{ >> + struct kvm_vcpu *vcpu = self->vcpu; >> + struct kvm_run *run = vcpu->run; >> + struct kvm_vcpu_events events; >> + struct kvm_sregs sregs; >> + struct kvm_regs regs; >> + int rv; >> >> /* Request and verify all valid register sets. */ >> /* TODO: BUILD TIME CHECK: TEST_ASSERT(KVM_SYNC_X86_NUM_FIELDS != 3); */ >> run->kvm_valid_regs = TEST_SYNC_FIELDS; >> rv = _vcpu_run(vcpu); >> + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); > > Just use vcpu_run() instead of _vcpu_run(). And please post that as a separate > patch, I think/hope it will make the conversion-to-tap patch smaller. Ok, makes sense. >> +TEST_F(sync_regs_test, clear_kvm_valid_and_dirty_regs) >> +{ >> + struct kvm_vcpu *vcpu = self->vcpu; >> + struct kvm_run *run = vcpu->run; >> + struct kvm_regs regs; >> + int rv; >> + >> + /* Run once to get register set */ >> + run->kvm_valid_regs = TEST_SYNC_FIELDS; >> + rv = _vcpu_run(vcpu); >> + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); > > At least you're consistent :-) Just copy-n-pasting the preexistent code ;-) >> + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); >> >> /* Clear kvm_valid_regs bits and kvm_dirty_bits. >> * Verify s.regs values are not overwritten with existing guest values >> @@ -187,9 +246,11 @@ int main(int argc, char *argv[]) >> run->kvm_valid_regs = 0; >> run->kvm_dirty_regs = 0; >> run->s.regs.regs.rbx = 0xAAAA; >> + vcpu_regs_get(vcpu, ®s); > > Can you split this change to its own patch too? I'm pretty sure that change > stands on its own, and slotting it in here made me do a double-take. Ok. Thomas
On 02/08/2023 23.31, Sean Christopherson wrote: > On Wed, Aug 02, 2023, Sean Christopherson wrote: >> Oh, and no need to post "KVM: selftests: Rename the ASSERT_EQ macro" in the next >> version, I'm planning on grabbing that one straightaway. > > After paging this all back in... > > I would much prefer that we implement the KVM specific macros[*], e.g. KVM_ONE_VCPU_TEST(), > and build on top of those. I'm definitely ok doing a "slow" conversion, i.e. starting > with a few easy tests. IIRC at some point I said I strongly preferred an all-or-nothing > approach, but realistically I don't think we'll make progress anytime soon if we try to > boil the ocean. At least I don't have enough spare time to do such a big conversion all at once - I'm only occasionally looking at the KVM selftests, mostly for s390x, and I also lack the knowledge how to test all those x86 tests. So don't expect such a big conversion from me, all I can provide is a small patch here or there. > But I do think we should spend the time to implement the infrastructure right away. We > may end up having to tweak the infrastructure down the road, e.g. to convert other tests, > but I would rather do that then convert some tests twice. > > [*] https://lore.kernel.org/all/Y2v+B3xxYKJSM%2FfH@google.com Sorry, I somehow completely missed that KVM_ONE_VCPU_TEST suggestion when picking up the series up again after working on other stuff for more than half a year. I'll try to incorporate this into the next version. (the other patches don't need a fixture, so I think they shouldn't be affected by this?) Thomas
diff --git a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c index 2da89fdc2471a..e1359a4a07fea 100644 --- a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c +++ b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c @@ -16,6 +16,7 @@ #include <string.h> #include <sys/ioctl.h> +#include "kselftest_harness.h" #include "test_util.h" #include "kvm_util.h" #include "processor.h" @@ -80,23 +81,24 @@ static void compare_vcpu_events(struct kvm_vcpu_events *left, #define TEST_SYNC_FIELDS (KVM_SYNC_X86_REGS|KVM_SYNC_X86_SREGS|KVM_SYNC_X86_EVENTS) #define INVALID_SYNC_FIELD 0x80000000 -int main(int argc, char *argv[]) -{ - struct kvm_vcpu *vcpu; +FIXTURE(sync_regs_test) { struct kvm_vm *vm; - struct kvm_run *run; - struct kvm_regs regs; - struct kvm_sregs sregs; - struct kvm_vcpu_events events; - int rv, cap; + struct kvm_vcpu *vcpu; +}; - cap = kvm_check_cap(KVM_CAP_SYNC_REGS); - TEST_REQUIRE((cap & TEST_SYNC_FIELDS) == TEST_SYNC_FIELDS); - TEST_REQUIRE(!(cap & INVALID_SYNC_FIELD)); +FIXTURE_SETUP(sync_regs_test) { + self->vm = vm_create_with_one_vcpu(&self->vcpu, guest_code); +} - vm = vm_create_with_one_vcpu(&vcpu, guest_code); +FIXTURE_TEARDOWN(sync_regs_test) { + kvm_vm_free(self->vm); +} - run = vcpu->run; +TEST_F(sync_regs_test, read_invalid) +{ + struct kvm_vcpu *vcpu = self->vcpu; + struct kvm_run *run = vcpu->run; + int rv; /* Request reading invalid register set from VCPU. */ run->kvm_valid_regs = INVALID_SYNC_FIELD; @@ -112,6 +114,13 @@ int main(int argc, char *argv[]) "Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n", rv); run->kvm_valid_regs = 0; +} + +TEST_F(sync_regs_test, set_invalid) +{ + struct kvm_vcpu *vcpu = self->vcpu; + struct kvm_run *run = vcpu->run; + int rv; /* Request setting invalid register set into VCPU. */ run->kvm_dirty_regs = INVALID_SYNC_FIELD; @@ -127,11 +136,22 @@ int main(int argc, char *argv[]) "Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n", rv); run->kvm_dirty_regs = 0; +} + +TEST_F(sync_regs_test, req_and_verify_all_valid) +{ + struct kvm_vcpu *vcpu = self->vcpu; + struct kvm_run *run = vcpu->run; + struct kvm_vcpu_events events; + struct kvm_sregs sregs; + struct kvm_regs regs; + int rv; /* Request and verify all valid register sets. */ /* TODO: BUILD TIME CHECK: TEST_ASSERT(KVM_SYNC_X86_NUM_FIELDS != 3); */ run->kvm_valid_regs = TEST_SYNC_FIELDS; rv = _vcpu_run(vcpu); + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); vcpu_regs_get(vcpu, ®s); @@ -142,6 +162,22 @@ int main(int argc, char *argv[]) vcpu_events_get(vcpu, &events); compare_vcpu_events(&events, &run->s.regs.events); +} + +TEST_F(sync_regs_test, set_and_verify_various) +{ + struct kvm_vcpu *vcpu = self->vcpu; + struct kvm_run *run = vcpu->run; + struct kvm_vcpu_events events; + struct kvm_sregs sregs; + struct kvm_regs regs; + int rv; + + /* Run once to get register set */ + run->kvm_valid_regs = TEST_SYNC_FIELDS; + rv = _vcpu_run(vcpu); + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); /* Set and verify various register values. */ run->s.regs.regs.rbx = 0xBAD1DEA; @@ -151,6 +187,7 @@ int main(int argc, char *argv[]) run->kvm_valid_regs = TEST_SYNC_FIELDS; run->kvm_dirty_regs = KVM_SYNC_X86_REGS | KVM_SYNC_X86_SREGS; rv = _vcpu_run(vcpu); + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); TEST_ASSERT(run->s.regs.regs.rbx == 0xBAD1DEA + 1, "rbx sync regs value incorrect 0x%llx.", @@ -167,6 +204,13 @@ int main(int argc, char *argv[]) vcpu_events_get(vcpu, &events); compare_vcpu_events(&events, &run->s.regs.events); +} + +TEST_F(sync_regs_test, clear_kvm_dirty_regs_bits) +{ + struct kvm_vcpu *vcpu = self->vcpu; + struct kvm_run *run = vcpu->run; + int rv; /* Clear kvm_dirty_regs bits, verify new s.regs values are * overwritten with existing guest values. @@ -175,10 +219,25 @@ int main(int argc, char *argv[]) run->kvm_dirty_regs = 0; run->s.regs.regs.rbx = 0xDEADBEEF; rv = _vcpu_run(vcpu); + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); TEST_ASSERT(run->s.regs.regs.rbx != 0xDEADBEEF, "rbx sync regs value incorrect 0x%llx.", run->s.regs.regs.rbx); +} + +TEST_F(sync_regs_test, clear_kvm_valid_and_dirty_regs) +{ + struct kvm_vcpu *vcpu = self->vcpu; + struct kvm_run *run = vcpu->run; + struct kvm_regs regs; + int rv; + + /* Run once to get register set */ + run->kvm_valid_regs = TEST_SYNC_FIELDS; + rv = _vcpu_run(vcpu); + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); /* Clear kvm_valid_regs bits and kvm_dirty_bits. * Verify s.regs values are not overwritten with existing guest values @@ -187,9 +246,11 @@ int main(int argc, char *argv[]) run->kvm_valid_regs = 0; run->kvm_dirty_regs = 0; run->s.regs.regs.rbx = 0xAAAA; + vcpu_regs_get(vcpu, ®s); regs.rbx = 0xBAC0; vcpu_regs_set(vcpu, ®s); rv = _vcpu_run(vcpu); + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); TEST_ASSERT(run->s.regs.regs.rbx == 0xAAAA, "rbx sync regs value incorrect 0x%llx.", @@ -198,6 +259,20 @@ int main(int argc, char *argv[]) TEST_ASSERT(regs.rbx == 0xBAC0 + 1, "rbx guest value incorrect 0x%llx.", regs.rbx); +} + +TEST_F(sync_regs_test, clear_kvm_valid_regs_bits) +{ + struct kvm_vcpu *vcpu = self->vcpu; + struct kvm_run *run = vcpu->run; + struct kvm_regs regs; + int rv; + + /* Run once to get register set */ + run->kvm_valid_regs = TEST_SYNC_FIELDS; + rv = _vcpu_run(vcpu); + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); /* Clear kvm_valid_regs bits. Verify s.regs values are not overwritten * with existing guest values but that guest values are overwritten @@ -207,6 +282,7 @@ int main(int argc, char *argv[]) run->kvm_dirty_regs = TEST_SYNC_FIELDS; run->s.regs.regs.rbx = 0xBBBB; rv = _vcpu_run(vcpu); + TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv); TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); TEST_ASSERT(run->s.regs.regs.rbx == 0xBBBB, "rbx sync regs value incorrect 0x%llx.", @@ -215,8 +291,15 @@ int main(int argc, char *argv[]) TEST_ASSERT(regs.rbx == 0xBBBB + 1, "rbx guest value incorrect 0x%llx.", regs.rbx); +} + +int main(int argc, char *argv[]) +{ + int cap; - kvm_vm_free(vm); + cap = kvm_check_cap(KVM_CAP_SYNC_REGS); + TEST_REQUIRE((cap & TEST_SYNC_FIELDS) == TEST_SYNC_FIELDS); + TEST_REQUIRE(!(cap & INVALID_SYNC_FIELD)); - return 0; + return test_harness_run(argc, argv); }
The sync_regs test currently does not have any output (unless one of the TEST_ASSERT statement fails), so it's hard to say for a user whether a certain new sub-test has been included in the binary or not. Let's make this a little bit more user-friendly and include some TAP output via the kselftest_harness.h interface. To be able to use the interface, we have to break up the huge main() function here in more fine grained parts - then we can use the TEST_F() macro to define the individual tests. Since these are run with a separate VM now, we have also to make sure to create the expected state at the beginning of each test, so some parts grow a little bit - which should be OK considering that the individual tests are more self-contained now. Suggested-by: David Matlack <dmatlack@google.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- .../selftests/kvm/x86_64/sync_regs_test.c | 113 +++++++++++++++--- 1 file changed, 98 insertions(+), 15 deletions(-)