On Thu, 30 Jan 2025 at 11:09, Jerome Forissier
<jerome.foriss...@linaro.org> wrote:
>
> Hi Ilias,
>
> On 1/30/25 08:20, Ilias Apalodimas wrote:
> > Upcoming patches are switching the memory mappings to RW, RO, RW^X page
> > permissions after the U-Boot binary and its data are relocated. Add
> > annotations in the linker scripts to and mark text, data, rodata etc
> > sections and align them to a page boundary
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> > ---
> > Makefile | 15 +++++++++------
> > arch/arm/cpu/armv8/u-boot.lds | 32 +++++++++++++++++++-------------
> > include/asm-generic/sections.h | 2 ++
> > lib/efi_loader/efi_runtime.c | 2 ++
> > 4 files changed, 32 insertions(+), 19 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 33bb86343c5b..8d7c062ec830 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2176,13 +2176,16 @@ System.map: u-boot
> > # ARM relocations should all be R_ARM_RELATIVE (32-bit) or
> > # R_AARCH64_RELATIVE (64-bit).
> > checkarmreloc: u-boot
> > - @RELOC="`$(CROSS_COMPILE)readelf -r -W $< | cut -d ' ' -f 4 | \
> > + @RELOCS="`$(CROSS_COMPILE)readelf -r -W $< | cut -d ' ' -f 4 | \
> > grep R_A | sort -u`"; \
> > - if test "$$RELOC" != "R_ARM_RELATIVE" -a \
> > - "$$RELOC" != "R_AARCH64_RELATIVE"; then \
> > - echo "$< contains unexpected relocations: $$RELOC"; \
> > - false; \
> > - fi
> > + for reloc in $$RELOCS; do \
> > + if [ "$$reloc" != "R_ARM_RELATIVE" -a \
> > + "$$reloc" != "R_AARCH64_RELATIVE" -a \
> > + "$$reloc" != "R_AARCH64_NONE" ]; then \
> > + echo "$< contains unexpected relocations: $$reloc"; \
> > + false; \
>
> s/false/exit 1/ otherwise the loop won't break and make won't get a
> failure status unless the error is on the last reloc.
>
> > + fi; \
> > + done
>
> This Makefile change seems unrelated to the current patch. Should it
> go into a separate fixup patch?
>
> > tools/version.h: include/version.h
> > $(Q)mkdir -p $(dir $@)
> > diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
> > index 857f44412e07..18e168e27135 100644
> > --- a/arch/arm/cpu/armv8/u-boot.lds
> > +++ b/arch/arm/cpu/armv8/u-boot.lds
> > @@ -22,7 +22,7 @@ SECTIONS
> >
> > . = ALIGN(8);
> > __image_copy_start = ADDR(.text);
> > - .text :
> > + .text ALIGN(4096):
> > {
> > CPUDIR/start.o (.text*)
> > }
> > @@ -36,9 +36,12 @@ SECTIONS
> > __efi_runtime_stop = .;
> > }
> >
> > - .text_rest :
> > + .text_rest ALIGN(4096) :
> > {
> > + __text_start = .;
> > *(.text*)
> > + . = ALIGN(4096);
> > + __text_end = .;
> > }
> >
> > #ifdef CONFIG_ARMV8_PSCI
> > @@ -98,27 +101,30 @@ SECTIONS
> > }
> > #endif
> >
> > - . = ALIGN(8);
> > - .rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
> > + .rodata ALIGN(4096): {
> > + __start_rodata = .;
> > + *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*)))
> > + . = ALIGN(4096);
> > + __end_rodata = .;
> > + }
> >
> > - . = ALIGN(8);
> > - .data : {
> > + .data ALIGN(4096) : {
> > + __start_data = .;
> > *(.data*)
> > + . = ALIGN(4096);
> > + __end_data = .;
> > }
> >
> > - . = ALIGN(8);
> > -
> > - . = .;
> > -
> > . = ALIGN(8);
> > __u_boot_list : {
> > KEEP(*(SORT(__u_boot_list*)));
> > }
> >
> > - .efi_runtime_rel : {
> > + .efi_runtime_rel ALIGN(4096) : {
> > __efi_runtime_rel_start = .;
> > *(.rel*.efi_runtime)
> > *(.rel*.efi_runtime.*)
> > + . = ALIGN(4096);
> > __efi_runtime_rel_stop = .;
> > }
> >
> > @@ -136,10 +142,10 @@ SECTIONS
> > /*
> > * arch/arm/lib/crt0_64.S assumes __bss_start - __bss_end % 8 == 0
> > */
> > - .bss ALIGN(8) : {
> > + .bss ALIGN(4096) : {
> > __bss_start = .;
> > *(.bss*)
> > - . = ALIGN(8);
> > + . = ALIGN(4096);
> > __bss_end = .;
> > }
> >
> > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> > index b6bca53db10d..5626df796f33 100644
> > --- a/include/asm-generic/sections.h
> > +++ b/include/asm-generic/sections.h
> > @@ -23,6 +23,7 @@ extern char __kprobes_text_start[], __kprobes_text_end[];
> > extern char __entry_text_start[], __entry_text_end[];
> > extern char __initdata_begin[], __initdata_end[];
> > extern char __start_rodata[], __end_rodata[];
> > +extern char __start_data[], __end_data[];
> > extern char __efi_helloworld_begin[];
> > extern char __efi_helloworld_end[];
> > extern char __efi_var_file_begin[];
> > @@ -63,6 +64,7 @@ static inline int arch_is_kernel_data(unsigned long addr)
> >
> > /* Start of U-Boot text region */
> > extern char __text_start[];
> > +extern char __text_end[];
> >
> > /* This marks the text region which must be relocated */
> > extern char __image_copy_start[], __image_copy_end[];
> > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> > index 35eb6a777665..c10a79301a92 100644
> > --- a/lib/efi_loader/efi_runtime.c
> > +++ b/lib/efi_loader/efi_runtime.c
> > @@ -742,6 +742,8 @@ void efi_runtime_relocate(ulong offset, struct
> > efi_mem_desc *map)
> > rel->info, *p, rel->offset);
> >
> > switch (rel->info & R_MASK) {
> > + case 0:
> > + break;
>
> It looks like this may need to go into the same fixup patch as the
> Makefile changes above.
>
> > case R_RELATIVE:
> > #ifdef IS_RELA
> > newaddr = rel->addend + offset - CONFIG_TEXT_BASE;
>
> For the linker script changes:
All of these are needed in a single patch. The alignment changes
trigger the emitted R_AARCH64_NONE relocs.
Cheers
/Ilias
>
> Acked-by: Jerome Forissier <jerome.foriss...@linaro.org>
