linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] kvm: selftests fixes and a new smm_test
@ 2019-04-11 16:48 Paolo Bonzini
  2019-04-11 16:48 ` [PATCH 1/3] selftests: kvm/evmcs_test: complete I/O before migrating guest state Paolo Bonzini
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-04-11 16:48 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: vkuznets, Sean Christopherson

It turns out that the remaining issue in smm_test was in the test itself.
Here is the result...

Paolo

Paolo Bonzini (2):
  selftests: kvm/evmcs_test: complete I/O before migrating guest state
  selftests: kvm: fix for compilers that do not support -no-pie

Vitaly Kuznetsov (1):
  selftests: kvm: add a selftest for SMM

 tools/testing/selftests/kvm/Makefile               |   9 +-
 .../selftests/kvm/include/x86_64/processor.h       |  27 ++++
 tools/testing/selftests/kvm/lib/kvm_util.c         |   5 +
 tools/testing/selftests/kvm/lib/x86_64/processor.c |  20 ++-
 tools/testing/selftests/kvm/x86_64/evmcs_test.c    |   5 +-
 tools/testing/selftests/kvm/x86_64/smm_test.c      | 157 +++++++++++++++++++++
 tools/testing/selftests/kvm/x86_64/state_test.c    |  15 +-
 7 files changed, 215 insertions(+), 23 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86_64/smm_test.c

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] selftests: kvm/evmcs_test: complete I/O before migrating guest state
  2019-04-11 16:48 [PATCH 0/3] kvm: selftests fixes and a new smm_test Paolo Bonzini
@ 2019-04-11 16:48 ` Paolo Bonzini
  2019-04-16 13:39   ` Andrew Jones
  2019-04-11 16:48 ` [PATCH 2/3] selftests: kvm: fix for compilers that do not support -no-pie Paolo Bonzini
  2019-04-11 16:48 ` [PATCH 3/3] selftests: kvm: add a selftest for SMM Paolo Bonzini
  2 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2019-04-11 16:48 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: vkuznets, Sean Christopherson

Starting state migration after an IO exit without first completing IO
may result in test failures.  We already have two tests that need this
(this patch in fact fixes evmcs_test, similar to what was fixed for
state_test in commit 0f73bbc851ed, "KVM: selftests: complete IO before
migrating guest state", 2019-03-13) and a third is coming.  So, move the
code to vcpu_save_state, and while at it do not access register state
until after I/O is complete.
---
 tools/testing/selftests/kvm/lib/kvm_util.c         |  5 +++++
 tools/testing/selftests/kvm/lib/x86_64/processor.c |  8 ++++++++
 tools/testing/selftests/kvm/x86_64/evmcs_test.c    |  5 +++--
 tools/testing/selftests/kvm/x86_64/state_test.c    | 15 +--------------
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index efa0aad8b3c6..4ca96b228e46 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -91,6 +91,11 @@ static void vm_open(struct kvm_vm *vm, int perm, unsigned long type)
 	if (vm->kvm_fd < 0)
 		exit(KSFT_SKIP);
 
+	if (!kvm_check_cap(KVM_CAP_IMMEDIATE_EXIT)) {
+		fprintf(stderr, "immediate_exit not available, skipping test\n");
+		exit(KSFT_SKIP);
+	}
+
 	vm->fd = ioctl(vm->kvm_fd, KVM_CREATE_VM, type);
 	TEST_ASSERT(vm->fd >= 0, "KVM_CREATE_VM ioctl failed, "
 		"rc: %i errno: %i", vm->fd, errno);
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index f28127f4a3af..b363c9611bd6 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -1030,6 +1030,14 @@ struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid)
 			    nested_size, sizeof(state->nested_));
 	}
 
+	/*
+	 * 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, vcpuid);
+
 	nmsrs = kvm_get_num_msrs(vm);
 	list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
 	list->nmsrs = nmsrs;
diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
index c49c2a28b0eb..36669684eca5 100644
--- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
@@ -123,8 +123,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],
@@ -144,6 +142,9 @@ int main(int argc, char *argv[])
 			    stage, (ulong)uc.args[1]);
 
 		state = vcpu_save_state(vm, VCPU_ID);
+		memset(&regs1, 0, sizeof(regs1));
+		vcpu_regs_get(vm, VCPU_ID, &regs1);
+
 		kvm_vm_release(vm);
 
 		/* Restore state in a new VM.  */
diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
index 30f75856cf39..e0a3c0204b7c 100644
--- a/tools/testing/selftests/kvm/x86_64/state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/state_test.c
@@ -134,11 +134,6 @@ 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());
@@ -179,18 +174,10 @@ 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);
-
+		state = vcpu_save_state(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);
 
 		/* Restore state in a new VM.  */
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] selftests: kvm: fix for compilers that do not support -no-pie
  2019-04-11 16:48 [PATCH 0/3] kvm: selftests fixes and a new smm_test Paolo Bonzini
  2019-04-11 16:48 ` [PATCH 1/3] selftests: kvm/evmcs_test: complete I/O before migrating guest state Paolo Bonzini
@ 2019-04-11 16:48 ` Paolo Bonzini
  2019-04-11 16:48 ` [PATCH 3/3] selftests: kvm: add a selftest for SMM Paolo Bonzini
  2 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-04-11 16:48 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: vkuznets, Sean Christopherson

-no-pie was added to GCC at the same time as their configuration option
--enable-default-pie.  Compilers that were built before do not have
-no-pie, but they also do not need it.  Detect the option at build
time.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tools/testing/selftests/kvm/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 7514fcea91a7..2d2e10b219d5 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -1,3 +1,5 @@
+include ../../../../scripts/Kbuild.include
+
 all:
 
 top_srcdir = ../../../..
@@ -30,7 +32,11 @@ INSTALL_HDR_PATH = $(top_srcdir)/usr
 LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/
 LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include
 CFLAGS += -O2 -g -std=gnu99 -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I..
-LDFLAGS += -pthread -no-pie
+
+no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
+        $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -no-pie -x c - -o "$$TMP", -no-pie)
+
+LDFLAGS += -pthread $(no-pie-option)
 
 # After inclusion, $(OUTPUT) is defined and
 # $(TEST_GEN_PROGS) starts with $(OUTPUT)/
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] selftests: kvm: add a selftest for SMM
  2019-04-11 16:48 [PATCH 0/3] kvm: selftests fixes and a new smm_test Paolo Bonzini
  2019-04-11 16:48 ` [PATCH 1/3] selftests: kvm/evmcs_test: complete I/O before migrating guest state Paolo Bonzini
  2019-04-11 16:48 ` [PATCH 2/3] selftests: kvm: fix for compilers that do not support -no-pie Paolo Bonzini
@ 2019-04-11 16:48 ` Paolo Bonzini
  2 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-04-11 16:48 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: vkuznets, Sean Christopherson

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Add a simple test for SMM, based on VMX.  The test implements its own
sync between the guest and the host as using our ucall library seems to
be too cumbersome: SMI handler is happening in real-address mode.

This patch also fixes KVM_SET_NESTED_STATE to happen after
KVM_SET_VCPU_EVENTS, in fact it places it last.  This is because
KVM needs to know whether the processor is in SMM or not.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tools/testing/selftests/kvm/Makefile               |   1 +
 .../selftests/kvm/include/x86_64/processor.h       |  27 ++++
 tools/testing/selftests/kvm/lib/x86_64/processor.c |  12 +-
 tools/testing/selftests/kvm/x86_64/smm_test.c      | 157 +++++++++++++++++++++
 4 files changed, 191 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86_64/smm_test.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 2d2e10b219d5..f8588cca2bef 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -19,6 +19,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/state_test
 TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
 TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
 TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test
+TEST_GEN_PROGS_x86_64 += x86_64/smm_test
 TEST_GEN_PROGS_x86_64 += dirty_log_test
 TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
 
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index e2884c2b81ff..6063d5b2f356 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -778,6 +778,33 @@ void vcpu_set_msr(struct kvm_vm *vm, uint32_t vcpuid, uint64_t msr_index,
 #define MSR_IA32_APICBASE_ENABLE	(1<<11)
 #define MSR_IA32_APICBASE_BASE		(0xfffff<<12)
 
+#define APIC_BASE_MSR	0x800
+#define X2APIC_ENABLE	(1UL << 10)
+#define	APIC_ICR	0x300
+#define		APIC_DEST_SELF		0x40000
+#define		APIC_DEST_ALLINC	0x80000
+#define		APIC_DEST_ALLBUT	0xC0000
+#define		APIC_ICR_RR_MASK	0x30000
+#define		APIC_ICR_RR_INVALID	0x00000
+#define		APIC_ICR_RR_INPROG	0x10000
+#define		APIC_ICR_RR_VALID	0x20000
+#define		APIC_INT_LEVELTRIG	0x08000
+#define		APIC_INT_ASSERT		0x04000
+#define		APIC_ICR_BUSY		0x01000
+#define		APIC_DEST_LOGICAL	0x00800
+#define		APIC_DEST_PHYSICAL	0x00000
+#define		APIC_DM_FIXED		0x00000
+#define		APIC_DM_FIXED_MASK	0x00700
+#define		APIC_DM_LOWEST		0x00100
+#define		APIC_DM_SMI		0x00200
+#define		APIC_DM_REMRD		0x00300
+#define		APIC_DM_NMI		0x00400
+#define		APIC_DM_INIT		0x00500
+#define		APIC_DM_STARTUP		0x00600
+#define		APIC_DM_EXTINT		0x00700
+#define		APIC_VECTOR_MASK	0x000FF
+#define	APIC_ICR2	0x310
+
 #define MSR_IA32_TSCDEADLINE		0x000006e0
 
 #define MSR_IA32_UCODE_WRITE		0x00000079
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index b363c9611bd6..dc7fae9fa424 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -1101,12 +1101,6 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s
 	struct vcpu *vcpu = vcpu_find(vm, vcpuid);
 	int r;
 
-	if (state->nested.size) {
-		r = ioctl(vcpu->fd, KVM_SET_NESTED_STATE, &state->nested);
-		TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_NESTED_STATE, r: %i",
-			r);
-	}
-
 	r = ioctl(vcpu->fd, KVM_SET_XSAVE, &state->xsave);
         TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_XSAVE, r: %i",
                 r);
@@ -1138,4 +1132,10 @@ void vcpu_load_state(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_x86_state *s
 	r = ioctl(vcpu->fd, KVM_SET_REGS, &state->regs);
         TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_REGS, r: %i",
                 r);
+
+	if (state->nested.size) {
+		r = ioctl(vcpu->fd, KVM_SET_NESTED_STATE, &state->nested);
+		TEST_ASSERT(r == 0, "Unexpected result from KVM_SET_NESTED_STATE, r: %i",
+			r);
+	}
 }
diff --git a/tools/testing/selftests/kvm/x86_64/smm_test.c b/tools/testing/selftests/kvm/x86_64/smm_test.c
new file mode 100644
index 000000000000..436227c2ea39
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/smm_test.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018, Red Hat, Inc.
+ *
+ * Tests for SMM.
+ */
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include "test_util.h"
+
+#include "kvm_util.h"
+
+#include "vmx.h"
+
+#define VCPU_ID	      1
+
+#define PAGE_SIZE  4096
+
+#define SMRAM_SIZE 65536
+#define SMRAM_MEMSLOT ((1 << 16) | 1)
+#define SMRAM_PAGES (SMRAM_SIZE / PAGE_SIZE)
+#define SMRAM_GPA 0x1000000
+#define SMRAM_STAGE 0xfe
+
+#define STR(x) #x
+#define XSTR(s) STR(s)
+
+#define SYNC_PORT 0xe
+#define DONE 0xff
+
+/*
+ * This is compiled as normal 64-bit code, however, SMI handler is executed
+ * in real-address mode. To stay simple we're limiting ourselves to a mode
+ * independent subset of asm here.
+ * SMI handler always report back fixed stage SMRAM_STAGE.
+ */
+uint8_t smi_handler[] = {
+	0xb0, SMRAM_STAGE,    /* mov $SMRAM_STAGE, %al */
+	0xe4, SYNC_PORT,      /* in $SYNC_PORT, %al */
+	0x0f, 0xaa,           /* rsm */
+};
+
+void sync_with_host(uint64_t phase)
+{
+	asm volatile("in $" XSTR(SYNC_PORT)", %%al \n"
+		     : : "a" (phase));
+}
+
+void self_smi(void)
+{
+	wrmsr(APIC_BASE_MSR + (APIC_ICR >> 4),
+	      APIC_DEST_SELF | APIC_INT_ASSERT | APIC_DM_SMI);
+}
+
+void guest_code(struct vmx_pages *vmx_pages)
+{
+	uint64_t apicbase = rdmsr(MSR_IA32_APICBASE);
+
+	sync_with_host(1);
+
+	wrmsr(MSR_IA32_APICBASE, apicbase | X2APIC_ENABLE);
+
+	sync_with_host(2);
+
+	self_smi();
+
+	sync_with_host(4);
+
+	if (vmx_pages) {
+		GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+
+		sync_with_host(5);
+
+		self_smi();
+
+		sync_with_host(7);
+	}
+
+	sync_with_host(DONE);
+}
+
+int main(int argc, char *argv[])
+{
+	struct vmx_pages *vmx_pages = NULL;
+	vm_vaddr_t vmx_pages_gva = 0;
+
+	struct kvm_regs regs;
+	struct kvm_vm *vm;
+	struct kvm_run *run;
+	struct kvm_x86_state *state;
+	int stage, stage_reported;
+
+	/* Create VM */
+	vm = vm_create_default(VCPU_ID, 0, guest_code);
+
+	vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
+
+	run = vcpu_state(vm, VCPU_ID);
+
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, SMRAM_GPA,
+				    SMRAM_MEMSLOT, SMRAM_PAGES, 0);
+	TEST_ASSERT(vm_phy_pages_alloc(vm, SMRAM_PAGES, SMRAM_GPA, SMRAM_MEMSLOT)
+		    == SMRAM_GPA, "could not allocate guest physical addresses?");
+
+	memset(addr_gpa2hva(vm, SMRAM_GPA), 0x0, SMRAM_SIZE);
+	memcpy(addr_gpa2hva(vm, SMRAM_GPA) + 0x8000, smi_handler,
+	       sizeof(smi_handler));
+
+	vcpu_set_msr(vm, VCPU_ID, MSR_IA32_SMBASE, SMRAM_GPA);
+
+	if (kvm_check_cap(KVM_CAP_NESTED_STATE)) {
+		vmx_pages = vcpu_alloc_vmx(vm, &vmx_pages_gva);
+		vcpu_args_set(vm, VCPU_ID, 1, vmx_pages_gva);
+	} else {
+		printf("will skip SMM test with VMX enabled\n");
+		vcpu_args_set(vm, VCPU_ID, 1, 0);
+	}
+
+	for (stage = 1;; stage++) {
+		_vcpu_run(vm, VCPU_ID);
+		TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
+			    "Stage %d: unexpected exit reason: %u (%s),\n",
+			    stage, run->exit_reason,
+			    exit_reason_str(run->exit_reason));
+
+		memset(&regs, 0, sizeof(regs));
+		vcpu_regs_get(vm, VCPU_ID, &regs);
+
+		stage_reported = regs.rax & 0xff;
+
+		if (stage_reported == DONE)
+			goto done;
+
+		TEST_ASSERT(stage_reported == stage ||
+			    stage_reported == SMRAM_STAGE,
+			    "Unexpected stage: #%x, got %x",
+			    stage, stage_reported);
+
+		state = vcpu_save_state(vm, VCPU_ID);
+		kvm_vm_release(vm);
+		kvm_vm_restart(vm, O_RDWR);
+		vm_vcpu_add(vm, VCPU_ID, 0, 0);
+		vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
+		vcpu_load_state(vm, VCPU_ID, state);
+		run = vcpu_state(vm, VCPU_ID);
+		free(state);
+	}
+
+done:
+	kvm_vm_free(vm);
+}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] selftests: kvm/evmcs_test: complete I/O before migrating guest state
  2019-04-11 16:48 ` [PATCH 1/3] selftests: kvm/evmcs_test: complete I/O before migrating guest state Paolo Bonzini
@ 2019-04-16 13:39   ` Andrew Jones
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Jones @ 2019-04-16 13:39 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, vkuznets, Sean Christopherson

On Thu, Apr 11, 2019 at 06:48:26PM +0200, Paolo Bonzini wrote:
> Starting state migration after an IO exit without first completing IO
> may result in test failures.  We already have two tests that need this
> (this patch in fact fixes evmcs_test, similar to what was fixed for
> state_test in commit 0f73bbc851ed, "KVM: selftests: complete IO before
> migrating guest state", 2019-03-13) and a third is coming.  So, move the
> code to vcpu_save_state, and while at it do not access register state
> until after I/O is complete.
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c         |  5 +++++
>  tools/testing/selftests/kvm/lib/x86_64/processor.c |  8 ++++++++
>  tools/testing/selftests/kvm/x86_64/evmcs_test.c    |  5 +++--
>  tools/testing/selftests/kvm/x86_64/state_test.c    | 15 +--------------
>  4 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index efa0aad8b3c6..4ca96b228e46 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -91,6 +91,11 @@ static void vm_open(struct kvm_vm *vm, int perm, unsigned long type)
>  	if (vm->kvm_fd < 0)
>  		exit(KSFT_SKIP);
>  
> +	if (!kvm_check_cap(KVM_CAP_IMMEDIATE_EXIT)) {
> +		fprintf(stderr, "immediate_exit not available, skipping test\n");
> +		exit(KSFT_SKIP);
> +	}
> +
>  	vm->fd = ioctl(vm->kvm_fd, KVM_CREATE_VM, type);
>  	TEST_ASSERT(vm->fd >= 0, "KVM_CREATE_VM ioctl failed, "
>  		"rc: %i errno: %i", vm->fd, errno);
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> index f28127f4a3af..b363c9611bd6 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> @@ -1030,6 +1030,14 @@ struct kvm_x86_state *vcpu_save_state(struct kvm_vm *vm, uint32_t vcpuid)
>  			    nested_size, sizeof(state->nested_));
>  	}
>  
> +	/*
> +	 * 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, vcpuid);
> +

Since the need for IO completion also affects MMIO exits and there's no
reason to believe that vcpu_save_state() will always be called after
an IO exit, then shouldn't we instead put this in get_ucall()?

diff --git a/tools/testing/selftests/kvm/lib/ucall.c b/tools/testing/selftests/kvm/lib/ucall.c
index a2ab38be2f47..e8c6f2741ce7 100644
--- a/tools/testing/selftests/kvm/lib/ucall.c
+++ b/tools/testing/selftests/kvm/lib/ucall.c
@@ -132,6 +132,7 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc)
        if (ucall_type == UCALL_PIO && run->exit_reason == KVM_EXIT_IO &&
            run->io.port == UCALL_PIO_PORT) {
                struct kvm_regs regs;
+               vcpu_run_complete_io(vm, vcpu_id);
                vcpu_regs_get(vm, vcpu_id, &regs);
                memcpy(uc, addr_gva2hva(vm, (vm_vaddr_t)regs.rdi), sizeof(*uc));
                return uc->cmd;
@@ -144,6 +145,7 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc)
                            "Unexpected ucall exit mmio address access");
                gva = *(vm_vaddr_t *)run->mmio.data;
                memcpy(uc, addr_gva2hva(vm, gva), sizeof(*uc));
+               vcpu_run_complete_io(vm, vcpu_id);
        }
 
        return uc->cmd;



>  	nmsrs = kvm_get_num_msrs(vm);
>  	list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
>  	list->nmsrs = nmsrs;
> diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> index c49c2a28b0eb..36669684eca5 100644
> --- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> @@ -123,8 +123,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],
> @@ -144,6 +142,9 @@ int main(int argc, char *argv[])
>  			    stage, (ulong)uc.args[1]);
>  
>  		state = vcpu_save_state(vm, VCPU_ID);
> +		memset(&regs1, 0, sizeof(regs1));
> +		vcpu_regs_get(vm, VCPU_ID, &regs1);
> +

We would still need this change to get the vcpu_regs_get() below the
get_ucall().

>  		kvm_vm_release(vm);
>  
>  		/* Restore state in a new VM.  */
> diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
> index 30f75856cf39..e0a3c0204b7c 100644
> --- a/tools/testing/selftests/kvm/x86_64/state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/state_test.c
> @@ -134,11 +134,6 @@ 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());
> @@ -179,18 +174,10 @@ 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);
> -
> +		state = vcpu_save_state(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);
>  
>  		/* Restore state in a new VM.  */
> -- 
> 1.8.3.1
> 
>

Thanks,
drew 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-04-16 13:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-11 16:48 [PATCH 0/3] kvm: selftests fixes and a new smm_test Paolo Bonzini
2019-04-11 16:48 ` [PATCH 1/3] selftests: kvm/evmcs_test: complete I/O before migrating guest state Paolo Bonzini
2019-04-16 13:39   ` Andrew Jones
2019-04-11 16:48 ` [PATCH 2/3] selftests: kvm: fix for compilers that do not support -no-pie Paolo Bonzini
2019-04-11 16:48 ` [PATCH 3/3] selftests: kvm: add a selftest for SMM Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).