linux-kernel - Re: [RFC PATCH 2/4] KVM: selftests: Align memory region addresses to 1M on s390x

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 16 May 2019 14:08:03 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Thomas Huth <thuth@...hat.com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Janosch Frank <frankja@...ux.ibm.com>, kvm@...r.kernel.org
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Shuah Khan <shuah@...nel.org>,
        Cornelia Huck <cohuck@...hat.com>,
        Andrew Jones <drjones@...hat.com>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-s390@...r.kernel.org
Subject: Re: [RFC PATCH 2/4] KVM: selftests: Align memory region addresses to
 1M on s390x

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@...hat.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
> 


-- 

Thanks,

David / dhildenb