Subject: [PATCH 0/1] selftests: kvm: Overlapped memory regions test

This introduces a test case to check memory slots with overlapped
regions on the guest address cannot be added. The cases checked
are described in the block comment upon test_overlap_memory_regions()
(see the patch 01).

I didn't see the need to calcute the addresses on compile/run-time, so I
just left them hard-coded (remember: aligned 1M to work on s390x).

It works on x86_64, aarch64, and s390x.

The patch is based on queue branch.

Ah, I did some cosmetic changes on test_add_max_memory_regions() too. If
it is not OK to be in a single patch...let me know.

Wainer dos Santos Moschetta (1):
selftests: kvm: Add overlapped memory regions test

.../selftests/kvm/set_memory_region_test.c | 75 ++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)

--
2.17.2


Subject: [PATCH 1/1] selftests: kvm: Add overlapped memory regions test

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.

Signed-off-by: Wainer dos Santos Moschetta <[email protected]>
---
.../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.
+ *
+ * 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");
+
+ 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)
--
2.17.2

2020-04-16 01:15:06

by Krish Sadhukhan

[permalink] [raw]
Subject: Re: [PATCH 1/1] selftests: kvm: Add overlapped memory regions test


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 <[email protected]>
> ---
> .../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 <[email protected]>

2020-04-16 15:06:24

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 1/1] selftests: kvm: Add overlapped memory regions test

On 16/04/20 01:57, Krish Sadhukhan 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.

I like Wainer's header and your body. :)

Paolo