On 24/05/2019 10.29, Christian Borntraeger wrote: > > > On 23.05.19 19:40, Andrew Jones wrote: >> On Thu, May 23, 2019 at 06:43:05PM +0200, 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@xxxxxxxxxx> >>> --- >>> 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 08edb8436c47..656df9d5cd4d 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; >> >> I guess s390x won't ever support VM_MEM_SRC_ANONYMOUS_THP? If it does, >> then we need 'alignment = max(huge_page_size, alignment)'. Actually >> that might be a nice way to write this anyway for future-proofing. > > I can do > - alignment = huge_page_size; > + alignment = max(huge_page_size, alignment); > > when applying. Yes, please, that's certainly cleaner this way. Thanks, Thomas