[PATCH 0/2] KVM: selftests: Add test case for readonly memslots on x86

[PATCH 0/2] KVM: selftests: Add test case for readonly memslots on x86

[Yohei Kojima]

Currently, there are no x86 test cases to verify that read and execute
on RO-memslot succeeds, and write is blocked as MMIO fault. This
behavior might break if changes are made to the MMIO path, because
x86's RO-memslot relies on the fault propagation mechanism for MMIO.

The first patch adds a function vm_userspace_mem_region_add_map(), which
is essentially the memslot setup code extracted from spawn_vm(). The
second patch adds an x86 test case for read/write/exec on RO-memslot.

This series is developed based on kvm/master, although this series adds
a test case for x86. This is because the first patch changes the
spawn_vm() function, which is used in the tests for other architectures.

base-commit: 0de4a0eec25b9171f2a2abb1a820e125e6797770

Yohei Kojima (2):
  KVM: selftests: Extract memslot setup code from spawn_vm()
  KVM: selftests: Add test case for readonly memslots on x86

 .../selftests/kvm/set_memory_region_test.c    | 147 +++++++++++++++---
 1 file changed, 129 insertions(+), 18 deletions(-)

-- 
2.43.0

[PATCH 1/2] KVM: selftests: Extract memslot setup code from spawn_vm()

[Yohei Kojima]

Add vm_userspace_mem_region_add_map() function to set up memslot, and
update spawn_vm() to use it. This helps the test cases to create
additional memslots easily.

Signed-off-by: Yohei Kojima <yohei.kojima@sony.com>
---
 .../selftests/kvm/set_memory_region_test.c    | 47 ++++++++++++-------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index 7fe427ff9b38..8d4fd713347c 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -107,31 +107,42 @@ static void wait_for_vcpu(void)
 	usleep(100000);
 }
 
+static void vm_userspace_mem_region_add_map(struct kvm_vm *vm,
+		uint64_t addr, uint32_t slot, size_t size, uint32_t flags)
+{
+	uint64_t *hva;
+	uint64_t gpa;
+
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS_THP,
+				    addr, slot,
+				    size / getpagesize(), flags);
+
+	/*
+	 * Allocate and map two pages so that the GPA accessed by guest_code()
+	 * stays valid across the memslot move.
+	 */
+	gpa = vm_phy_pages_alloc(vm, 2, addr, slot);
+	TEST_ASSERT(gpa == addr, "Failed vm_phy_pages_alloc\n");
+
+	virt_map(vm, addr, addr, 2);
+
+	/* Ditto for the host mapping so that both pages can be zeroed. */
+	hva = addr_gpa2hva(vm, MEM_REGION_GPA);
+	memset(hva, 0, 2 * 4096);
+}
+
 static struct kvm_vm *spawn_vm(struct kvm_vcpu **vcpu, pthread_t *vcpu_thread,
 			       void *guest_code)
 {
 	struct kvm_vm *vm;
-	uint64_t *hva;
-	uint64_t gpa;
 
 	vm = vm_create_with_one_vcpu(vcpu, guest_code);
 
-	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS_THP,
-				    MEM_REGION_GPA, MEM_REGION_SLOT,
-				    MEM_REGION_SIZE / getpagesize(), 0);
-
-	/*
-	 * Allocate and map two pages so that the GPA accessed by guest_code()
-	 * stays valid across the memslot move.
-	 */
-	gpa = vm_phy_pages_alloc(vm, 2, MEM_REGION_GPA, MEM_REGION_SLOT);
-	TEST_ASSERT(gpa == MEM_REGION_GPA, "Failed vm_phy_pages_alloc\n");
-
-	virt_map(vm, MEM_REGION_GPA, MEM_REGION_GPA, 2);
-
-	/* Ditto for the host mapping so that both pages can be zeroed. */
-	hva = addr_gpa2hva(vm, MEM_REGION_GPA);
-	memset(hva, 0, 2 * 4096);
+	vm_userspace_mem_region_add_map(vm,
+					MEM_REGION_GPA,
+					MEM_REGION_SLOT,
+					MEM_REGION_SIZE,
+					0);
 
 	pthread_create(vcpu_thread, NULL, vcpu_worker, *vcpu);
 
-- 
2.43.0

[PATCH 2/2] KVM: selftests: Add test case for readonly memslots on x86

[Yohei Kojima]

Extend set_memory_region_test to verify the following properties:

* read on RO-memslot succeeds,
* execute on RO-memslot backed by executable memory succeeds, and
* write on RO-memslot fails with mmio fault.

Signed-off-by: Yohei Kojima <yohei.kojima@sony.com>
---
 .../selftests/kvm/set_memory_region_test.c    | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index 8d4fd713347c..2f21bfcbc821 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -28,7 +28,10 @@
  * Somewhat arbitrary location and slot, intended to not overlap anything.
  */
 #define MEM_REGION_GPA		0xc0000000
+#define MEM_REGION_RO_GPA	0xd0000000
+
 #define MEM_REGION_SLOT		10
+#define MEM_REGION_RO_SLOT	11
 
 static const uint64_t MMIO_VAL = 0xbeefull;
 
@@ -49,6 +52,8 @@ static inline uint64_t guest_spin_on_val(uint64_t spin_val)
 	return val;
 }
 
+static int allow_mmio_fault;
+
 static void *vcpu_worker(void *data)
 {
 	struct kvm_vcpu *vcpu = data;
@@ -76,6 +81,13 @@ static void *vcpu_worker(void *data)
 		if (run->exit_reason != KVM_EXIT_MMIO)
 			break;
 
+		if (allow_mmio_fault && run->mmio.is_write)
+			/*
+			 * in this case, skip checking mmio-related assertions
+			 * and exit status
+			 */
+			return NULL;
+
 		TEST_ASSERT(!run->mmio.is_write, "Unexpected exit mmio write");
 		TEST_ASSERT(run->mmio.len == 8,
 			    "Unexpected exit mmio size = %u", run->mmio.len);
@@ -336,6 +348,92 @@ static void test_delete_memory_region(bool disable_slot_zap_quirk)
 	kvm_vm_free(vm);
 }
 
+static void guest_code_ro_memory_region(void)
+{
+	uint64_t val;
+	unsigned char c;
+	void *instruction_addr;
+
+	GUEST_SYNC(0);
+
+	val = guest_spin_on_val(0);
+	__GUEST_ASSERT(val == 1, "Expected '1', got '%lx'", val);
+
+	/* RO memory read; should succeed if the backing memory is readable */
+	c = *(unsigned char *)MEM_REGION_RO_GPA;
+	__GUEST_ASSERT(c == 0xab, "Expected '0xab', got '0x%x'", c);
+
+	/* RO memory exec; should succeed if the backing memory is executable */
+	instruction_addr = ((unsigned char *)MEM_REGION_RO_GPA) + 8;
+	val = ((uint32_t (*)(void))instruction_addr)();
+	__GUEST_ASSERT(val == 0xbeef,
+			"Expected 0xbeef, but got '%lx'", val);
+
+	/* Spin until the mmio fault is allowed for RO-memslot write */
+	val = guest_spin_on_val(1);
+	__GUEST_ASSERT(val == 2, "Expected '2', got '%lx'", val);
+
+	/* RO memory write; should fail */
+	WRITE_ONCE(*((uint64_t *)MEM_REGION_RO_GPA), 0x12);
+	__GUEST_ASSERT(0, "RO memory write is expected to fail, but it didn't");
+}
+
+/*
+ * On x86 environment, write access to the readonly memslots are trapped as
+ * a special MMIO fault. This test verifies that write access on the readonly
+ * memslot is blocked, and read/exec access isn't.
+ */
+static void test_ro_memory_region(void)
+{
+	pthread_t vcpu_thread;
+	uint64_t *hva, *hva_ro;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+
+	/*
+	 * Equivalent C function (assuming SysV ABI):
+	 * uint32_t some_function(void) {
+	 * 	return 0xbeef;
+	 * }
+	 */
+	unsigned char inst_bytes[] = {
+		0x48, 0xc7, 0xc0, 0xef, 0xbe, 0x00, 0x00,	// mov %eax, $0xbeef
+		0xc3,						// ret
+	};
+
+	pr_info("Testing write on RO memslot\n");
+
+	vm = spawn_vm(&vcpu, &vcpu_thread, guest_code_ro_memory_region);
+
+	vm_userspace_mem_region_add_map(vm,
+					MEM_REGION_RO_GPA,
+					MEM_REGION_RO_SLOT,
+					MEM_REGION_SIZE,
+					KVM_MEM_READONLY);
+
+	hva = addr_gpa2hva(vm, MEM_REGION_GPA);
+	hva_ro = addr_gpa2hva(vm, MEM_REGION_RO_GPA);
+
+	memset(hva_ro, 0xcccccccc, 0x2000);
+	WRITE_ONCE(*hva_ro, 0xab);
+	memcpy(((unsigned char *)hva_ro) + 8, inst_bytes, sizeof(inst_bytes));
+
+	WRITE_ONCE(*hva, 1);
+
+	/* Wait the vcpu thread to complete read/exec on ro memory */
+	usleep(100000);
+
+	allow_mmio_fault = true;
+	WRITE_ONCE(*hva, 2);
+
+	wait_for_vcpu();
+
+	pthread_join(vcpu_thread, NULL);
+
+	kvm_vm_free(vm);
+	allow_mmio_fault = false;
+}
+
 static void test_zero_memory_regions(void)
 {
 	struct kvm_vcpu *vcpu;
@@ -629,6 +727,8 @@ int main(int argc, char *argv[])
 	 */
 	test_zero_memory_regions();
 	test_mmio_during_vectoring();
+
+	test_ro_memory_region();
 #endif
 
 	test_invalid_memory_region_flags();
-- 
2.43.0