Re: [RFC PATCH v1 44/57] arm64: Align sections to PAGE_SIZE_MAX

[Ryan Roberts <ryan.roberts@arm.com>]

On 19/10/2024 15:16, Thomas Weißschuh wrote:
> On 2024-10-14 11:58:51+0100, Ryan Roberts wrote:
>> Increase alignment of sections in nvhe hyp, vdso and final vmlinux image
>> from PAGE_SIZE to PAGE_SIZE_MAX. For compile-time PAGE_SIZE,
>> PAGE_SIZE_MAX == PAGE_SIZE so there is no change. For boot-time
>> PAGE_SIZE, PAGE_SIZE_MAX is the largest selectable page size.
>>
>> For a boot-time page size build, image size is comparable to a 64K page
>> size compile-time build. In future, it may be desirable to optimize
>> run-time memory consumption by freeing unused padding pages when the
>> boot-time selected page size is less than PAGE_SIZE_MAX.
>>
>> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
>> ---
>>
>> ***NOTE***
>> Any confused maintainers may want to read the cover note here for context:
>> https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/
>>
>>  arch/arm64/include/asm/memory.h     |  4 +--
>>  arch/arm64/kernel/vdso-wrap.S       |  4 +--
>>  arch/arm64/kernel/vdso.c            |  7 +++---
>>  arch/arm64/kernel/vdso/vdso.lds.S   |  4 +--
>>  arch/arm64/kernel/vdso32-wrap.S     |  4 +--
>>  arch/arm64/kernel/vdso32/vdso.lds.S |  4 +--
>>  arch/arm64/kernel/vmlinux.lds.S     | 38 ++++++++++++++---------------
>>  arch/arm64/kvm/hyp/nvhe/hyp.lds.S   |  2 +-
>>  8 files changed, 34 insertions(+), 33 deletions(-)
> 
>> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> index 89b6e78400023..1efe98909a2e0 100644
>> --- a/arch/arm64/kernel/vdso.c
>> +++ b/arch/arm64/kernel/vdso.c
>> @@ -195,7 +195,7 @@ static int __setup_additional_pages(enum vdso_abi abi,
>>  
>>  	vdso_text_len = vdso_info[abi].vdso_pages << PAGE_SHIFT;
>>  	/* Be sure to map the data page */
>> -	vdso_mapping_len = vdso_text_len + VVAR_NR_PAGES * PAGE_SIZE;
>> +	vdso_mapping_len = vdso_text_len + VVAR_NR_PAGES * PAGE_SIZE_MAX;
>>  
>>  	vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
>>  	if (IS_ERR_VALUE(vdso_base)) {
>> @@ -203,7 +203,8 @@ static int __setup_additional_pages(enum vdso_abi abi,
>>  		goto up_fail;
>>  	}
>>  
>> -	ret = _install_special_mapping(mm, vdso_base, VVAR_NR_PAGES * PAGE_SIZE,
>> +	ret = _install_special_mapping(mm, vdso_base,
>> +				       VVAR_NR_PAGES * PAGE_SIZE_MAX,
>>  				       VM_READ|VM_MAYREAD|VM_PFNMAP,
>>  				       vdso_info[abi].dm);
>>  	if (IS_ERR(ret))
>> @@ -212,7 +213,7 @@ static int __setup_additional_pages(enum vdso_abi abi,
>>  	if (system_supports_bti_kernel())
>>  		gp_flags = VM_ARM64_BTI;
>>  
>> -	vdso_base += VVAR_NR_PAGES * PAGE_SIZE;
>> +	vdso_base += VVAR_NR_PAGES * PAGE_SIZE_MAX;
>>  	mm->context.vdso = (void *)vdso_base;
>>  	ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
>>  				       VM_READ|VM_EXEC|gp_flags|
> 
>> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
>> index 45354f2ddf706..f7d1537a689e8 100644
>> --- a/arch/arm64/kernel/vdso/vdso.lds.S
>> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
>> @@ -18,9 +18,9 @@ OUTPUT_ARCH(aarch64)
>>  
>>  SECTIONS
>>  {
>> -	PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
>> +	PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE_MAX);
>>  #ifdef CONFIG_TIME_NS
>> -	PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
>> +	PROVIDE(_timens_data = _vdso_data + PAGE_SIZE_MAX);
> 
> This looks like it also needs a change to vvar_fault() in vdso.c.
> The symbols are now always PAGE_SIZE_MAX apart, while vvar_fault() works
> in page offsets (vmf->pgoff) that are based on the runtime PAGE_SIZE and
> it expects hardcoded offsets.
> 
> As test you can use tools/testing/selftests/timens/timens.
> 
> (I can't test this right now, so it's only a suspicion)

Ahh good spot - that test does infact fail.

This fixes the problem:

---8<---
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 1efe98909a2e0..d2049ba6b19f5 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -151,10 +151,11 @@ int vdso_join_timens(struct task_struct *task, struct
time_namespace *ns)
 static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
                             struct vm_area_struct *vma, struct vm_fault *vmf)
 {
+       pgoff_t pgmaxoff = vmf->pgoff >> (PAGE_SHIFT_MAX - PAGE_SHIFT);
        struct page *timens_page = find_timens_vvar_page(vma);
        unsigned long pfn;

-       switch (vmf->pgoff) {
+       switch (pgmaxoff) {
        case VVAR_DATA_PAGE_OFFSET:
                if (timens_page)
                        pfn = page_to_pfn(timens_page);
---8<---

I'll include it in the next version.

Thanks,
Ryan

> 
>>  #endif
>>  	. = VDSO_LBASE + SIZEOF_HEADERS;
> 
>> diff --git a/arch/arm64/kernel/vdso32/vdso.lds.S b/arch/arm64/kernel/vdso32/vdso.lds.S
>> index 8d95d7d35057d..c46d18a69d1ce 100644
>> --- a/arch/arm64/kernel/vdso32/vdso.lds.S
>> +++ b/arch/arm64/kernel/vdso32/vdso.lds.S
>> @@ -18,9 +18,9 @@ OUTPUT_ARCH(arm)
>>  
>>  SECTIONS
>>  {
>> -	PROVIDE_HIDDEN(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
>> +	PROVIDE_HIDDEN(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE_MAX);
>>  #ifdef CONFIG_TIME_NS
>> -	PROVIDE_HIDDEN(_timens_data = _vdso_data + PAGE_SIZE);
>> +	PROVIDE_HIDDEN(_timens_data = _vdso_data + PAGE_SIZE_MAX);
>>  #endif
>>  	. = VDSO_LBASE + SIZEOF_HEADERS;