| Message ID | 20190516111253.4494-3-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM selftests for s390x | expand |
On 16.05.19 13:12, Thomas Huth wrote: > On s390x, there is a constraint that memory regions have to be aligned > to 1M (or running the VM will fail). Introduce a new "alignment" variable > in the vm_userspace_mem_region_add() function which now can be used for > both, huge page and s390x alignment requirements. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tools/testing/selftests/kvm/lib/kvm_util.c | 21 +++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > index 8d63ccb93e10..64a0da6efe3d 100644 > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > @@ -559,6 +559,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, > unsigned long pmem_size = 0; > struct userspace_mem_region *region; > size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size; > + size_t alignment; > > TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical " > "address not on a page boundary.\n" > @@ -608,9 +609,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, > TEST_ASSERT(region != NULL, "Insufficient Memory"); > region->mmap_size = npages * vm->page_size; > > - /* Enough memory to align up to a huge page. */ > +#ifdef __s390x__ > + /* On s390x, the host address must be aligned to 1M (due to PGSTEs) */ > + alignment = 0x100000; This corresponds to huge_page_size, maybe you can exploit this fact here. Something like alignment = 1; /* On s390x, the host address must always be aligned to the THP size */ #ifndef __s390x__ if (src_type == VM_MEM_SRC_ANONYMOUS_THP) #endif alignment = huge_page_size; Maybe in a nicer fashion. Not sure. > +#else > + alignment = 1; > +#endif > + > if (src_type == VM_MEM_SRC_ANONYMOUS_THP) > - region->mmap_size += huge_page_size; > + alignment = huge_page_size; > + > + /* Add enough memory to align up if necessary */ > + if (alignment > 1) > + region->mmap_size += alignment; > + > region->mmap_start = mmap(NULL, region->mmap_size, > PROT_READ | PROT_WRITE, > MAP_PRIVATE | MAP_ANONYMOUS > @@ -620,9 +632,8 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, > "test_malloc failed, mmap_start: %p errno: %i", > region->mmap_start, errno); > > - /* Align THP allocation up to start of a huge page. */ > - region->host_mem = align(region->mmap_start, > - src_type == VM_MEM_SRC_ANONYMOUS_THP ? huge_page_size : 1); > + /* Align host address */ > + region->host_mem = align(region->mmap_start, alignment); > > /* As needed perform madvise */ > if (src_type == VM_MEM_SRC_ANONYMOUS || src_type == VM_MEM_SRC_ANONYMOUS_THP) { >
On 16/05/2019 13.30, David Hildenbrand wrote: > On 16.05.19 13:12, Thomas Huth wrote: >> On s390x, there is a constraint that memory regions have to be aligned >> to 1M (or running the VM will fail). Introduce a new "alignment" variable >> in the vm_userspace_mem_region_add() function which now can be used for >> both, huge page and s390x alignment requirements. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> tools/testing/selftests/kvm/lib/kvm_util.c | 21 +++++++++++++++++----- >> 1 file changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c >> index 8d63ccb93e10..64a0da6efe3d 100644 >> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >> @@ -559,6 +559,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >> unsigned long pmem_size = 0; >> struct userspace_mem_region *region; >> size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size; >> + size_t alignment; >> >> TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical " >> "address not on a page boundary.\n" >> @@ -608,9 +609,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >> TEST_ASSERT(region != NULL, "Insufficient Memory"); >> region->mmap_size = npages * vm->page_size; >> >> - /* Enough memory to align up to a huge page. */ >> +#ifdef __s390x__ >> + /* On s390x, the host address must be aligned to 1M (due to PGSTEs) */ >> + alignment = 0x100000; > > This corresponds to huge_page_size, maybe you can exploit this fact here. > > Something like > > alignment = 1; > > /* On s390x, the host address must always be aligned to the THP size */ > #ifndef __s390x__ > if (src_type == VM_MEM_SRC_ANONYMOUS_THP) > #endif > alignment = huge_page_size; > > Maybe in a nicer fashion. Not sure. Hmm, but if I've got your explanation on IRC right, it's rather a coincidence that the huge page size matches the alignment requirements for KVM memslots, isn't it? So I think the code would look rather confusing if I'd try to shorten it this way...? Thomas
On 16.05.19 13:59, Thomas Huth wrote: > On 16/05/2019 13.30, David Hildenbrand wrote: >> On 16.05.19 13:12, Thomas Huth wrote: >>> On s390x, there is a constraint that memory regions have to be aligned >>> to 1M (or running the VM will fail). Introduce a new "alignment" variable >>> in the vm_userspace_mem_region_add() function which now can be used for >>> both, huge page and s390x alignment requirements. >>> >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> tools/testing/selftests/kvm/lib/kvm_util.c | 21 +++++++++++++++++----- >>> 1 file changed, 16 insertions(+), 5 deletions(-) >>> >>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c >>> index 8d63ccb93e10..64a0da6efe3d 100644 >>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >>> @@ -559,6 +559,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >>> unsigned long pmem_size = 0; >>> struct userspace_mem_region *region; >>> size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size; >>> + size_t alignment; >>> >>> TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical " >>> "address not on a page boundary.\n" >>> @@ -608,9 +609,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >>> TEST_ASSERT(region != NULL, "Insufficient Memory"); >>> region->mmap_size = npages * vm->page_size; >>> >>> - /* Enough memory to align up to a huge page. */ >>> +#ifdef __s390x__ >>> + /* On s390x, the host address must be aligned to 1M (due to PGSTEs) */ >>> + alignment = 0x100000; >> >> This corresponds to huge_page_size, maybe you can exploit this fact here. >> >> Something like >> >> alignment = 1; >> >> /* On s390x, the host address must always be aligned to the THP size */ >> #ifndef __s390x__ >> if (src_type == VM_MEM_SRC_ANONYMOUS_THP) >> #endif >> alignment = huge_page_size; >> >> Maybe in a nicer fashion. Not sure. > > Hmm, but if I've got your explanation on IRC right, it's rather a > coincidence that the huge page size matches the alignment requirements > for KVM memslots, isn't it? So I think the code would look rather > confusing if I'd try to shorten it this way...? Well, it's not really a coincidence. We have to share page tables between the gmap and the user space process. One huge page corresponds to the pages covered by a page table. So the page table "size" dictates the alignment of both things. But this is just nit picking here, do it the way you prefer, just wanted to point it out :) > > Thomas >
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index 8d63ccb93e10..64a0da6efe3d 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -559,6 +559,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, unsigned long pmem_size = 0; struct userspace_mem_region *region; size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size; + size_t alignment; TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical " "address not on a page boundary.\n" @@ -608,9 +609,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, TEST_ASSERT(region != NULL, "Insufficient Memory"); region->mmap_size = npages * vm->page_size; - /* Enough memory to align up to a huge page. */ +#ifdef __s390x__ + /* On s390x, the host address must be aligned to 1M (due to PGSTEs) */ + alignment = 0x100000; +#else + alignment = 1; +#endif + if (src_type == VM_MEM_SRC_ANONYMOUS_THP) - region->mmap_size += huge_page_size; + alignment = huge_page_size; + + /* Add enough memory to align up if necessary */ + if (alignment > 1) + region->mmap_size += alignment; + region->mmap_start = mmap(NULL, region->mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS @@ -620,9 +632,8 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, "test_malloc failed, mmap_start: %p errno: %i", region->mmap_start, errno); - /* Align THP allocation up to start of a huge page. */ - region->host_mem = align(region->mmap_start, - src_type == VM_MEM_SRC_ANONYMOUS_THP ? huge_page_size : 1); + /* Align host address */ + region->host_mem = align(region->mmap_start, alignment); /* As needed perform madvise */ if (src_type == VM_MEM_SRC_ANONYMOUS || src_type == VM_MEM_SRC_ANONYMOUS_THP) {
On s390x, there is a constraint that memory regions have to be aligned to 1M (or running the VM will fail). Introduce a new "alignment" variable in the vm_userspace_mem_region_add() function which now can be used for both, huge page and s390x alignment requirements. Signed-off-by: Thomas Huth <thuth@redhat.com> --- tools/testing/selftests/kvm/lib/kvm_util.c | 21 +++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)