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@xxxxxxxxxx>
Signed-off-by: Thomas Huth <thuth@xxxxxxxxxx>
---
.../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
