On 4/15/20 1:45 PM, Wainer dos Santos Moschetta wrote:
Add the test_overlap_memory_regions() test case in
set_memory_region_test. This should check that overlapping
memory regions on the guest physical address cannot be added.
I think the commit header and the body need some improvement. For example,
Header: Test that overlapping guest memory regions can not be added
Body: Enhance the existing tests in set_memory_region_test.c
so that it tests overlapping guest
memory regions. The new test verifies that adding
overlapping guest memory regions fails.
Signed-off-by: Wainer dos Santos Moschetta <wainersm@xxxxxxxxxx>
---
.../selftests/kvm/set_memory_region_test.c | 75 ++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index 260e638826dc..74a987002273 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -331,6 +331,8 @@ static void test_add_max_memory_regions(void)
uint64_t mem_reg_npages;
void *mem;
+ pr_info("Testing KVM_CAP_NR_MEMSLOTS memory regions can be added\n");
+
max_mem_slots = kvm_check_cap(KVM_CAP_NR_MEMSLOTS);
TEST_ASSERT(max_mem_slots > 0,
"KVM_CAP_NR_MEMSLOTS should be greater than 0");
@@ -338,7 +340,8 @@ static void test_add_max_memory_regions(void)
vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR);
- mem_reg_npages = vm_calc_num_guest_pages(VM_MODE_DEFAULT, MEM_REGION_SIZE);
+ mem_reg_npages = vm_calc_num_guest_pages(VM_MODE_DEFAULT,
+ MEM_REGION_SIZE);
/* Check it can be added memory slots up to the maximum allowed */
pr_info("Adding slots 0..%i, each memory region with %dK size\n",
@@ -365,6 +368,75 @@ static void test_add_max_memory_regions(void)
kvm_vm_free(vm);
}
+/*
+ * Test it cannot add memory slots with overlapped regions.
"Test that we can not add memory slots with overlapping regions."
+ *
+ * The following cases are covered:
+ *
+ * 0x100000 0x300000
+ * 0x0 0x200000 0x400000
+ * slot0 | |---2MB--| (SUCCESS)
+ * slot1 |---2MB--| (FAIL)
+ * slot2 |---2MB--| (SUCCESS)
+ * slot3 |---2MB--| (FAIL)
+ * slot4 |---2MB--| (FAIL)
+ * slot5 |---2MB--| (SUCCESS)
+ */
+void test_overlap_memory_regions(void)
+{
+ int i;
+ int ret;
+ int vm_fd;
+ struct kvm_userspace_memory_region kvm_region;
+ struct kvm_vm *vm;
+ struct slot_t {
+ uint64_t guest_addr;
+ int exp_ret; /* Expected ioctl return value */
+ };
+ struct slot_t slots[] = {{0x200000, 0}, {0x100000, -1}, {0x000000, 0},
+ {0x200000, -1}, {0x300000, -1}, {0x400000, 0}
+ };
+ uint64_t mem_reg_npages;
+ void *mem;
+
+ pr_info("Testing KVM_SET_USER_MEMORY_REGION with overlapped memory regions\n");
+
+ vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR);
+ vm_fd = vm_get_fd(vm);
+
+ pr_info("Working with memory region of %iMB\n", MEM_REGION_SIZE >> 20);
+ mem_reg_npages = vm_calc_num_guest_pages(VM_MODE_DEFAULT,
+ MEM_REGION_SIZE);
+
+ mem = mmap(NULL, MEM_REGION_SIZE, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() host");
I think a better message would be: "mmap() failure in host".
+
+ kvm_region.flags = 0;
+ kvm_region.memory_size = MEM_REGION_SIZE;
+ kvm_region.userspace_addr = (uint64_t) mem;
+
+ for (i = 0; i < sizeof(slots)/sizeof(struct slot_t); i++) {
+ pr_info("Add slot %i, guest address 0x%06lx, expect rc=%i\n",
+ i, slots[i].guest_addr, slots[i].exp_ret);
+ if (slots[i].exp_ret == 0) {
+ vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+ slots[i].guest_addr, i,
+ mem_reg_npages, 0);
+ } else {
+ kvm_region.slot = i;
+ kvm_region.guest_phys_addr = slots[i].guest_addr;
+ ret = ioctl(vm_fd, KVM_SET_USER_MEMORY_REGION,
+ &kvm_region);
+ TEST_ASSERT(ret == -1 && errno == EEXIST,
+ "Adding overlapped memory region should fail with EEXIT");
+ }
+ }
+
+ munmap(mem, MEM_REGION_SIZE);
+ kvm_vm_free(vm);
+}
+
int main(int argc, char *argv[])
{
#ifdef __x86_64__
@@ -383,6 +455,7 @@ int main(int argc, char *argv[])
#endif
test_add_max_memory_regions();
+ test_overlap_memory_regions();
#ifdef __x86_64__
if (argc > 1)
Other than the comments above,
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
