Patchwork KVM: selftests: complete IO before migrating guest state

login
register
mail settings
Submitter Christopherson, Sean J
Date March 13, 2019, 11:49 p.m.
Message ID <20190313234931.19797-1-sean.j.christopherson@intel.com>
Download mbox | patch
Permalink /patch/748481/
State New
Headers show

Comments

Christopherson, Sean J - March 13, 2019, 11:49 p.m.
Documentation/virtual/kvm/api.txt states:

  NOTE: For KVM_EXIT_IO, KVM_EXIT_MMIO, KVM_EXIT_OSI, KVM_EXIT_PAPR and
        KVM_EXIT_EPR the corresponding operations are complete (and guest
        state is consistent) only after userspace has re-entered the
        kernel with KVM_RUN.  The kernel side will first finish incomplete
        operations and then check for pending signals.  Userspace can
        re-enter the guest with an unmasked signal pending to complete
        pending operations.

Because guest state may be inconsistent, starting state migration after
an IO exit without first completing IO may result in test failures, e.g.
a proposed change to KVM's handling of %rip in its fast PIO handling[1]
will cause the new VM, i.e. the post-migration VM, to have its %rip set
to the IN instruction that triggered KVM_EXIT_IO, leading to a test
assertion due to a stage mismatch.

For simplicitly, require KVM_CAP_IMMEDIATE_EXIT to complete IO and skip
the test if it's not available.  The addition of KVM_CAP_IMMEDIATE_EXIT
predates the state selftest by more than a year.

[1] https://patchwork.kernel.org/patch/10848545/

Fixes: fa3899add1056 ("kvm: selftests: add basic test for state save and restore")
Reported-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 tools/testing/selftests/kvm/include/kvm_util.h |  1 +
 tools/testing/selftests/kvm/lib/kvm_util.c     | 16 ++++++++++++++++
 .../testing/selftests/kvm/x86_64/state_test.c  | 18 ++++++++++++++++--
 3 files changed, 33 insertions(+), 2 deletions(-)
Paolo Bonzini - March 15, 2019, 6:11 p.m.
On 14/03/19 00:49, Sean Christopherson wrote:
> Documentation/virtual/kvm/api.txt states:
> 
>   NOTE: For KVM_EXIT_IO, KVM_EXIT_MMIO, KVM_EXIT_OSI, KVM_EXIT_PAPR and
>         KVM_EXIT_EPR the corresponding operations are complete (and guest
>         state is consistent) only after userspace has re-entered the
>         kernel with KVM_RUN.  The kernel side will first finish incomplete
>         operations and then check for pending signals.  Userspace can
>         re-enter the guest with an unmasked signal pending to complete
>         pending operations.
> 
> Because guest state may be inconsistent, starting state migration after
> an IO exit without first completing IO may result in test failures, e.g.
> a proposed change to KVM's handling of %rip in its fast PIO handling[1]
> will cause the new VM, i.e. the post-migration VM, to have its %rip set
> to the IN instruction that triggered KVM_EXIT_IO, leading to a test
> assertion due to a stage mismatch.
> 
> For simplicitly, require KVM_CAP_IMMEDIATE_EXIT to complete IO and skip
> the test if it's not available.  The addition of KVM_CAP_IMMEDIATE_EXIT
> predates the state selftest by more than a year.
> 
> [1] https://patchwork.kernel.org/patch/10848545/
> 
> Fixes: fa3899add1056 ("kvm: selftests: add basic test for state save and restore")
> Reported-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  tools/testing/selftests/kvm/include/kvm_util.h |  1 +
>  tools/testing/selftests/kvm/lib/kvm_util.c     | 16 ++++++++++++++++
>  .../testing/selftests/kvm/x86_64/state_test.c  | 18 ++++++++++++++++--
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index a84785b02557..07b71ad9734a 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -102,6 +102,7 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva);
>  struct kvm_run *vcpu_state(struct kvm_vm *vm, uint32_t vcpuid);
>  void vcpu_run(struct kvm_vm *vm, uint32_t vcpuid);
>  int _vcpu_run(struct kvm_vm *vm, uint32_t vcpuid);
> +void vcpu_run_complete_io(struct kvm_vm *vm, uint32_t vcpuid);
>  void vcpu_set_mp_state(struct kvm_vm *vm, uint32_t vcpuid,
>  		       struct kvm_mp_state *mp_state);
>  void vcpu_regs_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_regs *regs);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index b52cfdefecbf..efa0aad8b3c6 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1121,6 +1121,22 @@ int _vcpu_run(struct kvm_vm *vm, uint32_t vcpuid)
>  	return rc;
>  }
>  
> +void vcpu_run_complete_io(struct kvm_vm *vm, uint32_t vcpuid)
> +{
> +	struct vcpu *vcpu = vcpu_find(vm, vcpuid);
> +	int ret;
> +
> +	TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid);
> +
> +	vcpu->state->immediate_exit = 1;
> +	ret = ioctl(vcpu->fd, KVM_RUN, NULL);
> +	vcpu->state->immediate_exit = 0;
> +
> +	TEST_ASSERT(ret == -1 && errno == EINTR,
> +		    "KVM_RUN IOCTL didn't exit immediately, rc: %i, errno: %i",
> +		    ret, errno);
> +}
> +
>  /*
>   * VM VCPU Set MP State
>   *
> diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
> index 4b3f556265f1..30f75856cf39 100644
> --- a/tools/testing/selftests/kvm/x86_64/state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/state_test.c
> @@ -134,6 +134,11 @@ int main(int argc, char *argv[])
>  
>  	struct kvm_cpuid_entry2 *entry = kvm_get_supported_cpuid_entry(1);
>  
> +	if (!kvm_check_cap(KVM_CAP_IMMEDIATE_EXIT)) {
> +		fprintf(stderr, "immediate_exit not available, skipping test\n");
> +		exit(KSFT_SKIP);
> +	}
> +
>  	/* Create VM */
>  	vm = vm_create_default(VCPU_ID, 0, guest_code);
>  	vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
> @@ -156,8 +161,6 @@ int main(int argc, char *argv[])
>  			    stage, run->exit_reason,
>  			    exit_reason_str(run->exit_reason));
>  
> -		memset(&regs1, 0, sizeof(regs1));
> -		vcpu_regs_get(vm, VCPU_ID, &regs1);
>  		switch (get_ucall(vm, VCPU_ID, &uc)) {
>  		case UCALL_ABORT:
>  			TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
> @@ -176,6 +179,17 @@ int main(int argc, char *argv[])
>  			    uc.args[1] == stage, "Unexpected register values vmexit #%lx, got %lx",
>  			    stage, (ulong)uc.args[1]);
>  
> +		/*
> +		 * When KVM exits to userspace with KVM_EXIT_IO, KVM guarantees
> +		 * guest state is consistent only after userspace re-enters the
> +		 * kernel with KVM_RUN.  Complete IO prior to migrating state
> +		 * to a new VM.
> +		 */
> +		vcpu_run_complete_io(vm, VCPU_ID);
> +
> +		memset(&regs1, 0, sizeof(regs1));
> +		vcpu_regs_get(vm, VCPU_ID, &regs1);
> +
>  		state = vcpu_save_state(vm, VCPU_ID);
>  		kvm_vm_release(vm);
>  
> 

Queued, thanks.

Paolo

Patch

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index a84785b02557..07b71ad9734a 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -102,6 +102,7 @@  vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva);
 struct kvm_run *vcpu_state(struct kvm_vm *vm, uint32_t vcpuid);
 void vcpu_run(struct kvm_vm *vm, uint32_t vcpuid);
 int _vcpu_run(struct kvm_vm *vm, uint32_t vcpuid);
+void vcpu_run_complete_io(struct kvm_vm *vm, uint32_t vcpuid);
 void vcpu_set_mp_state(struct kvm_vm *vm, uint32_t vcpuid,
 		       struct kvm_mp_state *mp_state);
 void vcpu_regs_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_regs *regs);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index b52cfdefecbf..efa0aad8b3c6 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1121,6 +1121,22 @@  int _vcpu_run(struct kvm_vm *vm, uint32_t vcpuid)
 	return rc;
 }
 
+void vcpu_run_complete_io(struct kvm_vm *vm, uint32_t vcpuid)
+{
+	struct vcpu *vcpu = vcpu_find(vm, vcpuid);
+	int ret;
+
+	TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid);
+
+	vcpu->state->immediate_exit = 1;
+	ret = ioctl(vcpu->fd, KVM_RUN, NULL);
+	vcpu->state->immediate_exit = 0;
+
+	TEST_ASSERT(ret == -1 && errno == EINTR,
+		    "KVM_RUN IOCTL didn't exit immediately, rc: %i, errno: %i",
+		    ret, errno);
+}
+
 /*
  * VM VCPU Set MP State
  *
diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
index 4b3f556265f1..30f75856cf39 100644
--- a/tools/testing/selftests/kvm/x86_64/state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/state_test.c
@@ -134,6 +134,11 @@  int main(int argc, char *argv[])
 
 	struct kvm_cpuid_entry2 *entry = kvm_get_supported_cpuid_entry(1);
 
+	if (!kvm_check_cap(KVM_CAP_IMMEDIATE_EXIT)) {
+		fprintf(stderr, "immediate_exit not available, skipping test\n");
+		exit(KSFT_SKIP);
+	}
+
 	/* Create VM */
 	vm = vm_create_default(VCPU_ID, 0, guest_code);
 	vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
@@ -156,8 +161,6 @@  int main(int argc, char *argv[])
 			    stage, run->exit_reason,
 			    exit_reason_str(run->exit_reason));
 
-		memset(&regs1, 0, sizeof(regs1));
-		vcpu_regs_get(vm, VCPU_ID, &regs1);
 		switch (get_ucall(vm, VCPU_ID, &uc)) {
 		case UCALL_ABORT:
 			TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
@@ -176,6 +179,17 @@  int main(int argc, char *argv[])
 			    uc.args[1] == stage, "Unexpected register values vmexit #%lx, got %lx",
 			    stage, (ulong)uc.args[1]);
 
+		/*
+		 * When KVM exits to userspace with KVM_EXIT_IO, KVM guarantees
+		 * guest state is consistent only after userspace re-enters the
+		 * kernel with KVM_RUN.  Complete IO prior to migrating state
+		 * to a new VM.
+		 */
+		vcpu_run_complete_io(vm, VCPU_ID);
+
+		memset(&regs1, 0, sizeof(regs1));
+		vcpu_regs_get(vm, VCPU_ID, &regs1);
+
 		state = vcpu_save_state(vm, VCPU_ID);
 		kvm_vm_release(vm);