Re: [PATCH -next] arm32: enable HAVE_LD_DEAD_CODE_DATA_ELIMINATION

[liuyuntao (F)]

在 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