From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 75F9CC04AAF for ; Thu, 16 May 2019 12:08:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 432A720833 for ; Thu, 16 May 2019 12:08:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727429AbfEPMIJ (ORCPT ); Thu, 16 May 2019 08:08:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53258 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726878AbfEPMII (ORCPT ); Thu, 16 May 2019 08:08:08 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2BEF73B718; Thu, 16 May 2019 12:08:08 +0000 (UTC) Received: from [10.36.117.217] (ovpn-117-217.ams2.redhat.com [10.36.117.217]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8F82D5D6A9; Thu, 16 May 2019 12:08:04 +0000 (UTC) Subject: Re: [RFC PATCH 2/4] KVM: selftests: Align memory region addresses to 1M on s390x To: Thomas Huth , Christian Borntraeger , Janosch Frank , kvm@vger.kernel.org Cc: Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Shuah Khan , Cornelia Huck , Andrew Jones , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-s390@vger.kernel.org References: <20190516111253.4494-1-thuth@redhat.com> <20190516111253.4494-3-thuth@redhat.com> <395e1b02-09b7-9420-33e1-a3abb36282f0@redhat.com> From: David Hildenbrand Openpgp: preference=signencrypt Autocrypt: addr=david@redhat.com; prefer-encrypt=mutual; keydata= xsFNBFXLn5EBEAC+zYvAFJxCBY9Tr1xZgcESmxVNI/0ffzE/ZQOiHJl6mGkmA1R7/uUpiCjJ dBrn+lhhOYjjNefFQou6478faXE6o2AhmebqT4KiQoUQFV4R7y1KMEKoSyy8hQaK1umALTdL QZLQMzNE74ap+GDK0wnacPQFpcG1AE9RMq3aeErY5tujekBS32jfC/7AnH7I0v1v1TbbK3Gp XNeiN4QroO+5qaSr0ID2sz5jtBLRb15RMre27E1ImpaIv2Jw8NJgW0k/D1RyKCwaTsgRdwuK Kx/Y91XuSBdz0uOyU/S8kM1+ag0wvsGlpBVxRR/xw/E8M7TEwuCZQArqqTCmkG6HGcXFT0V9 PXFNNgV5jXMQRwU0O/ztJIQqsE5LsUomE//bLwzj9IVsaQpKDqW6TAPjcdBDPLHvriq7kGjt WhVhdl0qEYB8lkBEU7V2Yb+SYhmhpDrti9Fq1EsmhiHSkxJcGREoMK/63r9WLZYI3+4W2rAc UucZa4OT27U5ZISjNg3Ev0rxU5UH2/pT4wJCfxwocmqaRr6UYmrtZmND89X0KigoFD/XSeVv jwBRNjPAubK9/k5NoRrYqztM9W6sJqrH8+UWZ1Idd/DdmogJh0gNC0+N42Za9yBRURfIdKSb B3JfpUqcWwE7vUaYrHG1nw54pLUoPG6sAA7Mehl3nd4pZUALHwARAQABzSREYXZpZCBIaWxk ZW5icmFuZCA8ZGF2aWRAcmVkaGF0LmNvbT7CwX4EEwECACgFAljj9eoCGwMFCQlmAYAGCwkI BwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJEE3eEPcA/4Na5IIP/3T/FIQMxIfNzZshIq687qgG 8UbspuE/YSUDdv7r5szYTK6KPTlqN8NAcSfheywbuYD9A4ZeSBWD3/NAVUdrCaRP2IvFyELj xoMvfJccbq45BxzgEspg/bVahNbyuBpLBVjVWwRtFCUEXkyazksSv8pdTMAs9IucChvFmmq3 jJ2vlaz9lYt/lxN246fIVceckPMiUveimngvXZw21VOAhfQ+/sofXF8JCFv2mFcBDoa7eYob s0FLpmqFaeNRHAlzMWgSsP80qx5nWWEvRLdKWi533N2vC/EyunN3HcBwVrXH4hxRBMco3jvM m8VKLKao9wKj82qSivUnkPIwsAGNPdFoPbgghCQiBjBe6A75Z2xHFrzo7t1jg7nQfIyNC7ez MZBJ59sqA9EDMEJPlLNIeJmqslXPjmMFnE7Mby/+335WJYDulsRybN+W5rLT5aMvhC6x6POK z55fMNKrMASCzBJum2Fwjf/VnuGRYkhKCqqZ8gJ3OvmR50tInDV2jZ1DQgc3i550T5JDpToh dPBxZocIhzg+MBSRDXcJmHOx/7nQm3iQ6iLuwmXsRC6f5FbFefk9EjuTKcLMvBsEx+2DEx0E UnmJ4hVg7u1PQ+2Oy+Lh/opK/BDiqlQ8Pz2jiXv5xkECvr/3Sv59hlOCZMOaiLTTjtOIU7Tq 7ut6OL64oAq+zsFNBFXLn5EBEADn1959INH2cwYJv0tsxf5MUCghCj/CA/lc/LMthqQ773ga uB9mN+F1rE9cyyXb6jyOGn+GUjMbnq1o121Vm0+neKHUCBtHyseBfDXHA6m4B3mUTWo13nid 0e4AM71r0DS8+KYh6zvweLX/LL5kQS9GQeT+QNroXcC1NzWbitts6TZ+IrPOwT1hfB4WNC+X 2n4AzDqp3+ILiVST2DT4VBc11Gz6jijpC/KI5Al8ZDhRwG47LUiuQmt3yqrmN63V9wzaPhC+ xbwIsNZlLUvuRnmBPkTJwwrFRZvwu5GPHNndBjVpAfaSTOfppyKBTccu2AXJXWAE1Xjh6GOC 8mlFjZwLxWFqdPHR1n2aPVgoiTLk34LR/bXO+e0GpzFXT7enwyvFFFyAS0Nk1q/7EChPcbRb hJqEBpRNZemxmg55zC3GLvgLKd5A09MOM2BrMea+l0FUR+PuTenh2YmnmLRTro6eZ/qYwWkC u8FFIw4pT0OUDMyLgi+GI1aMpVogTZJ70FgV0pUAlpmrzk/bLbRkF3TwgucpyPtcpmQtTkWS gDS50QG9DR/1As3LLLcNkwJBZzBG6PWbvcOyrwMQUF1nl4SSPV0LLH63+BrrHasfJzxKXzqg rW28CTAE2x8qi7e/6M/+XXhrsMYG+uaViM7n2je3qKe7ofum3s4vq7oFCPsOgwARAQABwsFl BBgBAgAPBQJVy5+RAhsMBQkJZgGAAAoJEE3eEPcA/4NagOsP/jPoIBb/iXVbM+fmSHOjEshl KMwEl/m5iLj3iHnHPVLBUWrXPdS7iQijJA/VLxjnFknhaS60hkUNWexDMxVVP/6lbOrs4bDZ NEWDMktAeqJaFtxackPszlcpRVkAs6Msn9tu8hlvB517pyUgvuD7ZS9gGOMmYwFQDyytpepo YApVV00P0u3AaE0Cj/o71STqGJKZxcVhPaZ+LR+UCBZOyKfEyq+ZN311VpOJZ1IvTExf+S/5 lqnciDtbO3I4Wq0ArLX1gs1q1XlXLaVaA3yVqeC8E7kOchDNinD3hJS4OX0e1gdsx/e6COvy qNg5aL5n0Kl4fcVqM0LdIhsubVs4eiNCa5XMSYpXmVi3HAuFyg9dN+x8thSwI836FoMASwOl C7tHsTjnSGufB+D7F7ZBT61BffNBBIm1KdMxcxqLUVXpBQHHlGkbwI+3Ye+nE6HmZH7IwLwV W+Ajl7oYF+jeKaH4DZFtgLYGLtZ1LDwKPjX7VAsa4Yx7S5+EBAaZGxK510MjIx6SGrZWBrrV TEvdV00F2MnQoeXKzD7O4WFbL55hhyGgfWTHwZ457iN9SgYi1JLPqWkZB0JRXIEtjd4JEQcx +8Umfre0Xt4713VxMygW0PnQt5aSQdMD58jHFxTk092mU+yIHj5LeYgvwSgZN4airXk5yRXl SE+xAvmumFBY Organization: Red Hat GmbH Message-ID: Date: Thu, 16 May 2019 14:08:03 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <395e1b02-09b7-9420-33e1-a3abb36282f0@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 16 May 2019 12:08:08 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>> --- >>> 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 > -- Thanks, David / dhildenb From mboxrd@z Thu Jan 1 00:00:00 1970 From: david at redhat.com (David Hildenbrand) Date: Thu, 16 May 2019 14:08:03 +0200 Subject: [RFC PATCH 2/4] KVM: selftests: Align memory region addresses to 1M on s390x In-Reply-To: <395e1b02-09b7-9420-33e1-a3abb36282f0@redhat.com> References: <20190516111253.4494-1-thuth@redhat.com> <20190516111253.4494-3-thuth@redhat.com> <395e1b02-09b7-9420-33e1-a3abb36282f0@redhat.com> Message-ID: 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 >>> --- >>> 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 > -- Thanks, David / dhildenb From mboxrd@z Thu Jan 1 00:00:00 1970 From: david@redhat.com (David Hildenbrand) Date: Thu, 16 May 2019 14:08:03 +0200 Subject: [RFC PATCH 2/4] KVM: selftests: Align memory region addresses to 1M on s390x In-Reply-To: <395e1b02-09b7-9420-33e1-a3abb36282f0@redhat.com> References: <20190516111253.4494-1-thuth@redhat.com> <20190516111253.4494-3-thuth@redhat.com> <395e1b02-09b7-9420-33e1-a3abb36282f0@redhat.com> Message-ID: Content-Type: text/plain; charset="UTF-8" Message-ID: <20190516120803.6G9ya2eGEWTBtVvWyfDZKAK9Bb23KYBXzFzveOABd7A@z> 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 >>> --- >>> 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 > -- Thanks, David / dhildenb