Sean Christopherson <[email protected]> writes: > > [...snip...] > >> >> Since this test program is meant to test set_memory_region, should we be >> retaining the original test? The original test is wrong in that it >> doesn't test guest_memfd's binding, but it does test that >> set_memory_region returns -EEXIST on overlapping GPAs. >> >> Perhaps to just test overlapping GPAs we can use anonymous memory >> instead of guest_memfd. > > Eh, I see no harm in having both. E.g. if we do this, then we don't have to > explain why we're not testing the other case :-) >
Makes sense to have both :) > diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c > b/tools/testing/selftests/kvm/set_memory_region_test.c > index 9b919a231c93..283392bcc3a0 100644 > --- a/tools/testing/selftests/kvm/set_memory_region_test.c > +++ b/tools/testing/selftests/kvm/set_memory_region_test.c > @@ -510,7 +510,7 @@ static void > test_add_overlapping_private_memory_regions(void) > > vm = vm_create_barebones_type(KVM_X86_SW_PROTECTED_VM); > > - memfd = vm_create_guest_memfd(vm, MEM_REGION_SIZE * 4, 0); > + memfd = vm_create_guest_memfd(vm, MEM_REGION_SIZE * 5, 0); > > vm_set_user_memory_region2(vm, MEM_REGION_SLOT, KVM_MEM_GUEST_MEMFD, > MEM_REGION_GPA, MEM_REGION_SIZE * 2, 0, > memfd, 0); > @@ -542,6 +542,26 @@ static void > test_add_overlapping_private_memory_regions(void) > TEST_ASSERT(r == -1 && errno == EEXIST, "%s", > "Overlapping guest_memfd() bindings should fail with > EEXIST"); > > + /* > + * Repeat the overlap tests, but so that there is overlap in the > + * guest_memfd bindings (i.e. in guest_memfd file offsets), but _not_ > + * in the GPA space. Regardless of where there's overlap, KVM should > + * return -EEXIST. > + */ > + r = __vm_set_user_memory_region2(vm, MEM_REGION_SLOT, > KVM_MEM_GUEST_MEMFD, > + MEM_REGION_GPA, > + MEM_REGION_SIZE * 2, > + 0, memfd, MEM_REGION_SIZE); > + TEST_ASSERT(r == -1 && errno == EEXIST, "%s", > + "Overlapping guest_memfd() bindings should fail with > EEXIST"); > + > + /* And now the back half of the other slot's guest_memfd binding. */ > + r = __vm_set_user_memory_region2(vm, MEM_REGION_SLOT, > KVM_MEM_GUEST_MEMFD, > + MEM_REGION_GPA, > + MEM_REGION_SIZE * 2, > + 0, memfd, MEM_REGION_SIZE * 3); > + TEST_ASSERT(r == -1 && errno == EEXIST, "%s", > + "Overlapping guest_memfd() bindings should fail with > EEXIST"); I just noticed this is kind of odd, what is the purpose of "%s" and then filling the string in with a hardcoded string? > close(memfd); > kvm_vm_free(vm); > }
