Re: [PATCH -next] arm32: enable HAVE_LD_DEAD_CODE_DATA_ELIMINATION

["liuyuntao (F)" <liuyuntao12@huawei.com>]

在 2024/2/20 16:40, Arnd Bergmann 写道:
> On Tue, Feb 20, 2024, at 09:15, Yuntao Liu wrote:
>> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S
>> b/arch/arm/boot/compressed/vmlinux.lds.S
>> index 3fcb3e62dc56..da21244aa892 100644
>> --- a/arch/arm/boot/compressed/vmlinux.lds.S
>> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
>> @@ -89,7 +89,7 @@ SECTIONS
>>        * The EFI stub always executes from RAM, and runs strictly before
>> the
>>        * decompressor, so we can make an exception for its r/w data, and
>> keep it
>>        */
>> -    *(.data.efistub .bss.efistub)
>> +    *(.data.* .bss.*)
>>       __pecoff_data_end = .;
>>
>>       /*
> 
> This doesn't seem right to me, or maybe I misunderstand what
> the original version does. Have you tested with both
> CONFIG_EFI_STUB on and off, and booting with and without
> UEFI?

Yes, I have tested with CONFIG_EFI_STUB on and off, and booting with 
UEFI on a single-board computer, and it boots well.


> 
> If I read this right, you would move all .data and .bss
> into the stub here, not just the parts we actually want?

In the file "drivers/firmware/efi/libstub/Makefile", it is written:

---

#
# ARM discards the .data section because it disallows r/w data in the
# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
# which are preserved explicitly by the decompressor linker script.
#
STUBCOPY_FLAGS-$(CONFIG_ARM)	+= --rename-section .data=.data.efistub	\
				   --rename-section .bss=.bss.efistub,load,alloc

---

I think that .data.efistub represents the entire .data section, the same 
applies to .bss as well,

so i move all .data and .bss into the stub here.


> 
>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>> index bd9127c4b451..de373c6c2ae8 100644
>> --- a/arch/arm/kernel/vmlinux.lds.S
>> +++ b/arch/arm/kernel/vmlinux.lds.S
>> @@ -74,7 +74,7 @@ SECTIONS
>>   	. = ALIGN(4);
>>   	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
>>   		__start___ex_table = .;
>> -		ARM_MMU_KEEP(*(__ex_table))
>> +		ARM_MMU_KEEP(KEEP(*(__ex_table)))
>>   		__stop___ex_table = .;
>>   	}
>>
>> @@ -116,7 +116,7 @@ SECTIONS
>>   #endif
>>   	.init.pv_table : {
>>   		__pv_table_begin = .;
>> -		*(.pv_table)
>> +		KEEP(*(.pv_table))
>>   		__pv_table_end = .;
>>   	}
> 
> I guess this prevents discarding any function that has a reference
> from pv_table or ex_table, even if there are no other references,
> right?

Indeed so, if not keep ex_table,  the compilation process will result in

an error:

     no __ex_table in file: vmlinux

     Failed to sort kernel tables

and if not keep pv_table, It can be compiled successfully, but the QEMU 
boots will fail.

> 
> I don't know how to solve this other than forcing all the
> uaccess and virt_to_phys functions to be out of line
> helpers. For uaccess, there are probably very few functions
> that need this, so it should make little difference.
> 
> You might want to try changing CONFIG_ARM_PATCH_PHYS_VIRT
> into a method that just always adds an offset from C code
> instead of the boot time patching. That way the code would
> be a bit less efficient but you might be able to get
> a larger size reduction by dropping additional unused code.
> 
> Maybe test your patch both with and without
> ARM_PATCH_PHYS_VIRT to see what the best-case impact would
> be.
> 

This is a very good idea, I will give it a try.

>        Arnd