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  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

Powered by blists - more mailing lists