* [PATCH gmem] KVM: selftests: Fix gmem conversion tests for multiple vCPUs @ 2023-10-16 18:47 Michael Roth 2023-10-18 1:10 ` Sean Christopherson 0 siblings, 1 reply; 3+ messages in thread From: Michael Roth @ 2023-10-16 18:47 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, seanjc, pbonzini, vannapurve Currently the private_mem_conversions_test crashes if invoked with the -n <num_vcpus> option without also specifying multiple memslots via -m. This is because the current implementation assumes -m is specified and always sets up the per-vCPU memory with a dedicated memslot for each vCPU. When -m is not specified, the test skips setting up memslots/memory for secondary vCPUs. The current code does seem to try to handle using a single memslot for multiple vCPUs in some places, e.g. the call-site, but test_mem_conversions() is missing the important bit of sizing the single memslot appropriately to handle all the per-vCPU memory. Implement that handling. Signed-off-by: Michael Roth <michael.roth@amd.com> --- .../kvm/x86_64/private_mem_conversions_test.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c index c04e7d61a585..5eb693fead33 100644 --- a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c +++ b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c @@ -388,10 +388,14 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t gmem_flags = 0; memfd = vm_create_guest_memfd(vm, memfd_size, gmem_flags); - for (i = 0; i < nr_memslots; i++) - vm_mem_add(vm, src_type, BASE_DATA_GPA + size * i, - BASE_DATA_SLOT + i, size / vm->page_size, - KVM_MEM_PRIVATE, memfd, size * i); + if (nr_memslots == 1) + vm_mem_add(vm, src_type, BASE_DATA_GPA, BASE_DATA_SLOT, + memfd_size / vm->page_size, KVM_MEM_PRIVATE, memfd, 0); + else + for (i = 0; i < nr_memslots; i++) + vm_mem_add(vm, src_type, BASE_DATA_GPA + size * i, + BASE_DATA_SLOT + i, size / vm->page_size, + KVM_MEM_PRIVATE, memfd, size * i); for (i = 0; i < nr_vcpus; i++) { uint64_t gpa = BASE_DATA_GPA + i * size; -- 2.25.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH gmem] KVM: selftests: Fix gmem conversion tests for multiple vCPUs 2023-10-16 18:47 [PATCH gmem] KVM: selftests: Fix gmem conversion tests for multiple vCPUs Michael Roth @ 2023-10-18 1:10 ` Sean Christopherson 2023-10-23 18:06 ` Sean Christopherson 0 siblings, 1 reply; 3+ messages in thread From: Sean Christopherson @ 2023-10-18 1:10 UTC (permalink / raw) To: Michael Roth; +Cc: kvm, linux-kernel, pbonzini, vannapurve On Mon, Oct 16, 2023, Michael Roth wrote: > Currently the private_mem_conversions_test crashes if invoked with the > -n <num_vcpus> option without also specifying multiple memslots via -m. Totally a PEBKAC, not a bug ;-) > This is because the current implementation assumes -m is specified and > always sets up the per-vCPU memory with a dedicated memslot for each > vCPU. When -m is not specified, the test skips setting up > memslots/memory for secondary vCPUs. > > The current code does seem to try to handle using a single memslot for > multiple vCPUs in some places, e.g. the call-site, but > test_mem_conversions() is missing the important bit of sizing the single > memslot appropriately to handle all the per-vCPU memory. Implement that > handling. > > Signed-off-by: Michael Roth <michael.roth@amd.com> > --- > .../kvm/x86_64/private_mem_conversions_test.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c > index c04e7d61a585..5eb693fead33 100644 > --- a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c > +++ b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c > @@ -388,10 +388,14 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t > gmem_flags = 0; > memfd = vm_create_guest_memfd(vm, memfd_size, gmem_flags); > > - for (i = 0; i < nr_memslots; i++) > - vm_mem_add(vm, src_type, BASE_DATA_GPA + size * i, > - BASE_DATA_SLOT + i, size / vm->page_size, > - KVM_MEM_PRIVATE, memfd, size * i); > + if (nr_memslots == 1) > + vm_mem_add(vm, src_type, BASE_DATA_GPA, BASE_DATA_SLOT, > + memfd_size / vm->page_size, KVM_MEM_PRIVATE, memfd, 0); > + else > + for (i = 0; i < nr_memslots; i++) The if-else needs curly braces. > + vm_mem_add(vm, src_type, BASE_DATA_GPA + size * i, > + BASE_DATA_SLOT + i, size / vm->page_size, > + KVM_MEM_PRIVATE, memfd, size * i); But I think that's a moot point, because isn't it easier to do this? diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c index c04e7d61a585..c99073098f98 100644 --- a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c +++ b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c @@ -367,6 +367,7 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t */ const size_t size = align_up(PER_CPU_DATA_SIZE, get_backing_src_pagesz(src_type)); const size_t memfd_size = size * nr_vcpus; + const size_t slot_size = memfd_size / nr_memslots; struct kvm_vcpu *vcpus[KVM_MAX_VCPUS]; pthread_t threads[KVM_MAX_VCPUS]; uint64_t gmem_flags; @@ -390,7 +391,7 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t for (i = 0; i < nr_memslots; i++) vm_mem_add(vm, src_type, BASE_DATA_GPA + size * i, - BASE_DATA_SLOT + i, size / vm->page_size, + BASE_DATA_SLOT + i, slot_size / vm->page_size, KVM_MEM_PRIVATE, memfd, size * i); for (i = 0; i < nr_vcpus; i++) { ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH gmem] KVM: selftests: Fix gmem conversion tests for multiple vCPUs 2023-10-18 1:10 ` Sean Christopherson @ 2023-10-23 18:06 ` Sean Christopherson 0 siblings, 0 replies; 3+ messages in thread From: Sean Christopherson @ 2023-10-23 18:06 UTC (permalink / raw) To: Michael Roth; +Cc: kvm, linux-kernel, pbonzini, vannapurve On Tue, Oct 17, 2023, Sean Christopherson wrote: > On Mon, Oct 16, 2023, Michael Roth wrote: > > + vm_mem_add(vm, src_type, BASE_DATA_GPA + size * i, > > + BASE_DATA_SLOT + i, size / vm->page_size, > > + KVM_MEM_PRIVATE, memfd, size * i); > > But I think that's a moot point, because isn't it easier to do this? > > diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c > index c04e7d61a585..c99073098f98 100644 > --- a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c > +++ b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c > @@ -367,6 +367,7 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t > */ > const size_t size = align_up(PER_CPU_DATA_SIZE, get_backing_src_pagesz(src_type)); > const size_t memfd_size = size * nr_vcpus; > + const size_t slot_size = memfd_size / nr_memslots; > struct kvm_vcpu *vcpus[KVM_MAX_VCPUS]; > pthread_t threads[KVM_MAX_VCPUS]; > uint64_t gmem_flags; > @@ -390,7 +391,7 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t > > for (i = 0; i < nr_memslots; i++) > vm_mem_add(vm, src_type, BASE_DATA_GPA + size * i, > - BASE_DATA_SLOT + i, size / vm->page_size, > + BASE_DATA_SLOT + i, slot_size / vm->page_size, > KVM_MEM_PRIVATE, memfd, size * i); This isn't quite right, the stride and offset needs to be per-memslot too. Argh, I created quite the mess by trying to take a shortcut for testing multiple memslots, i.e. by only allowing '1' or "nr_vcpus" memslots. Much of the code assumes that ranges can't be covered by multiple memslots, e.g. the UCALL_SYNC handler assumes the entire range is contiguous in the host virtual address space. And I think there's meaningful coverage we're not getting, e.g. as is I don't think we're testing KVM_SET_MEMORY_ATTRIBUTES across multiple memslots (thankfully we seem to have gotten the KVM side of things correct). I'll post a small series to clean up the mess and let the user specify the number of memslots (with some restrictions to keep the code relatively simple). ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-23 18:06 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-16 18:47 [PATCH gmem] KVM: selftests: Fix gmem conversion tests for multiple vCPUs Michael Roth 2023-10-18 1:10 ` Sean Christopherson 2023-10-23 18:06 ` Sean Christopherson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox