[PATCH 1/4] efi_loader: Fix section alignment on EFI binaries
When creating EFI binaries, the alignment of the text section isn't correctly factored in. As a result trying to load signed EFI binaries throws an error with:
efi_image_region_add() efi_image_region_add: new region already part of another Image not authenticated
Except for the fixing each linker script individually carve out a linker script include for ARM and RISC-V.
Heinrich Schuchardt (3): scripts/Makefile.lib: add -L option to LD command for EFI binaries efi_loader: use INCLUDE in EFI linker scripts efi_loader: use include in ARM EFI linker script
Ilias Apalodimas (1): efi_loader: Fix section alignment on EFI binaries
arch/arm/lib/crt0_arm_efi.S | 37 +++++++-------- arch/arm/lib/elf_aarch64_efi.lds | 68 +-------------------------- arch/arm/lib/elf_arm_efi.lds | 71 +--------------------------- arch/riscv/lib/elf_riscv32_efi.lds | 68 +-------------------------- arch/riscv/lib/elf_riscv64_efi.lds | 68 +-------------------------- lib/efi_loader/elf_efi.ldsi | 74 ++++++++++++++++++++++++++++++ scripts/Makefile.lib | 4 +- 7 files changed, 99 insertions(+), 291 deletions(-) create mode 100644 lib/efi_loader/elf_efi.ldsi
From: Ilias Apalodimas ilias.apalodimas@linaro.org
When creating EFI binaries, the alignment of the text section isn't correctly factored in. As a result trying to load signed EFI binaries throws an error with:
efi_image_region_add() efi_image_region_add: new region already part of another Image not authenticated
Running the binary through sbverify has a similar warning sbverify ./lib/efi_loader/helloworld.efi warning: gap in section table: .text : 0x00001000 - 0x00001c00, .data : 0x00002000 - 0x00002200, gaps in the section table may result in different checksums warning: data remaining[7680 vs 12720]: gaps between PE/COFF sections? .....
If we include the alignment in the text section, the signed binary boots fine, and the relevant sbverify warning goes away sbverify ./lib/efi_loader/helloworld.efi warning: data remaining[8704 vs 12720]: gaps between PE/COFF sections? .....
We should look into the remaining warning at some point as well regarding the gaps between PE/COFF sections.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- arch/arm/lib/elf_aarch64_efi.lds | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds index 5dd98091698..e382254a6cf 100644 --- a/arch/arm/lib/elf_aarch64_efi.lds +++ b/arch/arm/lib/elf_aarch64_efi.lds @@ -32,9 +32,9 @@ SECTIONS .rela.plt : { *(.rela.plt) } .rela.got : { *(.rela.got) } .rela.data : { *(.rela.data) *(.rela.data*) } + . = ALIGN(4096); _etext = .; _text_size = . - _text; - . = ALIGN(4096); .data : { _data = .; *(.sdata)
The linker uses the path specified with -L to search for linker scripts and for linker script includes.
For out-of-tree builds specify the build directory with -L instead of the absolute path of the linker script. This allows using an INCLUDE statement.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- scripts/Makefile.lib | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 54403040f00..18993435eae 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -523,10 +523,10 @@ $(obj)/%.efi: $(obj)/%_efi.so KBUILD_EFILDFLAGS = -nostdlib -zexecstack -znocombreloc -znorelro KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments) quiet_cmd_efi_ld = LD $@ -cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \ +cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -L $(srctree) -T $(EFI_LDS_PATH) \ -shared -Bsymbolic -s $^ -o $@
-EFI_LDS_PATH = $(srctree)/arch/$(ARCH)/lib/$(EFI_LDS) +EFI_LDS_PATH = arch/$(ARCH)/lib/$(EFI_LDS)
$(obj)/efi_crt0.o: $(srctree)/arch/$(ARCH)/lib/$(EFI_CRT0:.o=.S) FORCE $(call if_changed_dep,as_o_S)
On Tue, 14 Jan 2025 at 12:30, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
The linker uses the path specified with -L to search for linker scripts and for linker script includes.
For out-of-tree builds specify the build directory with -L instead of the absolute path of the linker script. This allows using an INCLUDE statement.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
scripts/Makefile.lib | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 54403040f00..18993435eae 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -523,10 +523,10 @@ $(obj)/%.efi: $(obj)/%_efi.so KBUILD_EFILDFLAGS = -nostdlib -zexecstack -znocombreloc -znorelro KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments) quiet_cmd_efi_ld = LD $@ -cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \ +cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -L $(srctree) -T $(EFI_LDS_PATH) \ -shared -Bsymbolic -s $^ -o $@
-EFI_LDS_PATH = $(srctree)/arch/$(ARCH)/lib/$(EFI_LDS) +EFI_LDS_PATH = arch/$(ARCH)/lib/$(EFI_LDS)
$(obj)/efi_crt0.o: $(srctree)/arch/$(ARCH)/lib/$(EFI_CRT0:.o=.S) FORCE $(call if_changed_dep,as_o_S) -- 2.47.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Except for the architecture specific lines ARM and RISC-V can use the same linker script. Move the common lines to an include.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- arch/arm/lib/elf_aarch64_efi.lds | 68 +-------------------------- arch/riscv/lib/elf_riscv32_efi.lds | 68 +-------------------------- arch/riscv/lib/elf_riscv64_efi.lds | 68 +-------------------------- lib/efi_loader/elf_efi.ldsi | 74 ++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 201 deletions(-) create mode 100644 lib/efi_loader/elf_efi.ldsi
diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds index e382254a6cf..453d3511c28 100644 --- a/arch/arm/lib/elf_aarch64_efi.lds +++ b/arch/arm/lib/elf_aarch64_efi.lds @@ -8,70 +8,4 @@ OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64") OUTPUT_ARCH(aarch64)
-PHDRS -{ - data PT_LOAD FLAGS(3); /* SHF_WRITE | SHF_ALLOC */ -} - -ENTRY(_start) -SECTIONS -{ - .text 0x0 : { - _text = .; - *(.text.head) - *(.text) - *(.text.*) - *(.gnu.linkonce.t.*) - *(.srodata) - *(.rodata*) - . = ALIGN(16); - *(.dynamic); - . = ALIGN(512); - } - .rela.dyn : { *(.rela.dyn) } - .rela.plt : { *(.rela.plt) } - .rela.got : { *(.rela.got) } - .rela.data : { *(.rela.data) *(.rela.data*) } - . = ALIGN(4096); - _etext = .; - _text_size = . - _text; - .data : { - _data = .; - *(.sdata) - *(.data) - *(.data1) - *(.data.*) - *(.got.plt) - *(.got) - - /* - * The EFI loader doesn't seem to like a .bss section, so we - * stick it all into .data: - */ - . = ALIGN(16); - _bss = .; - *(.sbss) - *(.scommon) - *(.dynbss) - *(.bss) - *(.bss.*) - *(COMMON) - . = ALIGN(512); - _bss_end = .; - _edata = .; - } :data - _data_size = _edata - _data; - - . = ALIGN(4096); - .dynsym : { *(.dynsym) } - . = ALIGN(4096); - .dynstr : { *(.dynstr) } - . = ALIGN(4096); - .note.gnu.build-id : { *(.note.gnu.build-id) } - /DISCARD/ : { - *(.rel.reloc) - *(.eh_frame) - *(.note.GNU-stack) - } - .comment 0 : { *(.comment) } -} +INCLUDE lib/efi_loader/elf_efi.ldsi diff --git a/arch/riscv/lib/elf_riscv32_efi.lds b/arch/riscv/lib/elf_riscv32_efi.lds index 7b9bd7b7f15..e23521c4931 100644 --- a/arch/riscv/lib/elf_riscv32_efi.lds +++ b/arch/riscv/lib/elf_riscv32_efi.lds @@ -8,70 +8,4 @@ OUTPUT_FORMAT("elf32-littleriscv", "elf32-littleriscv", "elf32-littleriscv") OUTPUT_ARCH(riscv)
-PHDRS -{ - data PT_LOAD FLAGS(3); /* SHF_WRITE | SHF_ALLOC */ -} - -ENTRY(_start) -SECTIONS -{ - .text 0x0 : { - _text = .; - *(.text.head) - *(.text) - *(.text.*) - *(.gnu.linkonce.t.*) - *(.srodata) - *(.rodata*) - . = ALIGN(16); - *(.dynamic); - . = ALIGN(512); - } - .rela.dyn : { *(.rela.dyn) } - .rela.plt : { *(.rela.plt) } - .rela.got : { *(.rela.got) } - .rela.data : { *(.rela.data) *(.rela.data*) } - _etext = .; - _text_size = . - _text; - . = ALIGN(4096); - .data : { - _data = .; - *(.sdata) - *(.data) - *(.data1) - *(.data.*) - *(.got.plt) - *(.got) - - /* - * The EFI loader doesn't seem to like a .bss section, so we - * stick it all into .data: - */ - . = ALIGN(16); - _bss = .; - *(.sbss) - *(.scommon) - *(.dynbss) - *(.bss) - *(.bss.*) - *(COMMON) - . = ALIGN(512); - _bss_end = .; - _edata = .; - } :data - _data_size = _edata - _data; - - . = ALIGN(4096); - .dynsym : { *(.dynsym) } - . = ALIGN(4096); - .dynstr : { *(.dynstr) } - . = ALIGN(4096); - .note.gnu.build-id : { *(.note.gnu.build-id) } - /DISCARD/ : { - *(.rel.reloc) - *(.eh_frame) - *(.note.GNU-stack) - } - .comment 0 : { *(.comment) } -} +INCLUDE lib/efi_loader/elf_efi.ldsi diff --git a/arch/riscv/lib/elf_riscv64_efi.lds b/arch/riscv/lib/elf_riscv64_efi.lds index d0b4f3d1d64..8e4844c2eea 100644 --- a/arch/riscv/lib/elf_riscv64_efi.lds +++ b/arch/riscv/lib/elf_riscv64_efi.lds @@ -8,70 +8,4 @@ OUTPUT_FORMAT("elf64-littleriscv", "elf64-littleriscv", "elf64-littleriscv") OUTPUT_ARCH(riscv)
-PHDRS -{ - data PT_LOAD FLAGS(3); /* SHF_WRITE | SHF_ALLOC */ -} - -ENTRY(_start) -SECTIONS -{ - .text 0x0 : { - _text = .; - *(.text.head) - *(.text) - *(.text.*) - *(.gnu.linkonce.t.*) - *(.srodata) - *(.rodata*) - . = ALIGN(16); - *(.dynamic); - . = ALIGN(512); - } - .rela.dyn : { *(.rela.dyn) } - .rela.plt : { *(.rela.plt) } - .rela.got : { *(.rela.got) } - .rela.data : { *(.rela.data) *(.rela.data*) } - _etext = .; - _text_size = . - _text; - . = ALIGN(4096); - .data : { - _data = .; - *(.sdata) - *(.data) - *(.data1) - *(.data.*) - *(.got.plt) - *(.got) - - /* - * The EFI loader doesn't seem to like a .bss section, so we - * stick it all into .data: - */ - . = ALIGN(16); - _bss = .; - *(.sbss) - *(.scommon) - *(.dynbss) - *(.bss) - *(.bss.*) - *(COMMON) - . = ALIGN(512); - _bss_end = .; - _edata = .; - } :data - _data_size = _edata - _data; - - . = ALIGN(4096); - .dynsym : { *(.dynsym) } - . = ALIGN(4096); - .dynstr : { *(.dynstr) } - . = ALIGN(4096); - .note.gnu.build-id : { *(.note.gnu.build-id) } - /DISCARD/ : { - *(.rel.reloc) - *(.eh_frame) - *(.note.GNU-stack) - } - .comment 0 : { *(.comment) } -} +INCLUDE lib/efi_loader/elf_efi.ldsi diff --git a/lib/efi_loader/elf_efi.ldsi b/lib/efi_loader/elf_efi.ldsi new file mode 100644 index 00000000000..190a88fb69e --- /dev/null +++ b/lib/efi_loader/elf_efi.ldsi @@ -0,0 +1,74 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ +/* + * U-Boot EFI linker script include + * + * Modified from elf_aarch64_efi.lds in gnu-efi + */ + +PHDRS +{ + data PT_LOAD FLAGS(3); /* SHF_WRITE | SHF_ALLOC */ +} + +ENTRY(_start) +SECTIONS +{ + .text 0x0 : { + _text = .; + *(.text.head) + *(.text) + *(.text.*) + *(.gnu.linkonce.t.*) + *(.srodata) + *(.rodata*) + . = ALIGN(16); + *(.dynamic); + . = ALIGN(512); + } + .rela.dyn : { *(.rela.dyn) } + .rela.plt : { *(.rela.plt) } + .rela.got : { *(.rela.got) } + .rela.data : { *(.rela.data) *(.rela.data*) } + . = ALIGN(4096); + _etext = .; + _text_size = . - _text; + .data : { + _data = .; + *(.sdata) + *(.data) + *(.data1) + *(.data.*) + *(.got.plt) + *(.got) + + /* + * The EFI loader doesn't seem to like a .bss section, so we + * stick it all into .data: + */ + . = ALIGN(16); + _bss = .; + *(.sbss) + *(.scommon) + *(.dynbss) + *(.bss) + *(.bss.*) + *(COMMON) + . = ALIGN(512); + _bss_end = .; + _edata = .; + } :data + _data_size = _edata - _data; + + . = ALIGN(4096); + .dynsym : { *(.dynsym) } + . = ALIGN(4096); + .dynstr : { *(.dynstr) } + . = ALIGN(4096); + .note.gnu.build-id : { *(.note.gnu.build-id) } + /DISCARD/ : { + *(.rel.reloc) + *(.eh_frame) + *(.note.GNU-stack) + } + .comment 0 : { *(.comment) } +}
On Tue, 14 Jan 2025 at 12:30, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Except for the architecture specific lines ARM and RISC-V can use the same linker script. Move the common lines to an include.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
arch/arm/lib/elf_aarch64_efi.lds | 68 +-------------------------- arch/riscv/lib/elf_riscv32_efi.lds | 68 +-------------------------- arch/riscv/lib/elf_riscv64_efi.lds | 68 +-------------------------- lib/efi_loader/elf_efi.ldsi | 74 ++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 201 deletions(-) create mode 100644 lib/efi_loader/elf_efi.ldsi
diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds index e382254a6cf..453d3511c28 100644 --- a/arch/arm/lib/elf_aarch64_efi.lds +++ b/arch/arm/lib/elf_aarch64_efi.lds @@ -8,70 +8,4 @@ OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64") OUTPUT_ARCH(aarch64)
-PHDRS -{
data PT_LOAD FLAGS(3); /* SHF_WRITE | SHF_ALLOC */-}
-ENTRY(_start) -SECTIONS -{
.text 0x0 : {_text = .;*(.text.head)*(.text)*(.text.*)*(.gnu.linkonce.t.*)*(.srodata)*(.rodata*). = ALIGN(16);*(.dynamic);. = ALIGN(512);}.rela.dyn : { *(.rela.dyn) }.rela.plt : { *(.rela.plt) }.rela.got : { *(.rela.got) }.rela.data : { *(.rela.data) *(.rela.data*) }. = ALIGN(4096);_etext = .;_text_size = . - _text;.data : {_data = .;*(.sdata)*(.data)*(.data1)*(.data.*)*(.got.plt)*(.got)/** The EFI loader doesn't seem to like a .bss section, so we* stick it all into .data:*/. = ALIGN(16);_bss = .;*(.sbss)*(.scommon)*(.dynbss)*(.bss)*(.bss.*)*(COMMON). = ALIGN(512);_bss_end = .;_edata = .;} :data_data_size = _edata - _data;. = ALIGN(4096);.dynsym : { *(.dynsym) }. = ALIGN(4096);.dynstr : { *(.dynstr) }. = ALIGN(4096);.note.gnu.build-id : { *(.note.gnu.build-id) }/DISCARD/ : {*(.rel.reloc)*(.eh_frame)*(.note.GNU-stack)}.comment 0 : { *(.comment) }-} +INCLUDE lib/efi_loader/elf_efi.ldsi diff --git a/arch/riscv/lib/elf_riscv32_efi.lds b/arch/riscv/lib/elf_riscv32_efi.lds index 7b9bd7b7f15..e23521c4931 100644 --- a/arch/riscv/lib/elf_riscv32_efi.lds +++ b/arch/riscv/lib/elf_riscv32_efi.lds @@ -8,70 +8,4 @@ OUTPUT_FORMAT("elf32-littleriscv", "elf32-littleriscv", "elf32-littleriscv") OUTPUT_ARCH(riscv)
-PHDRS -{
data PT_LOAD FLAGS(3); /* SHF_WRITE | SHF_ALLOC */-}
-ENTRY(_start) -SECTIONS -{
.text 0x0 : {_text = .;*(.text.head)*(.text)*(.text.*)*(.gnu.linkonce.t.*)*(.srodata)*(.rodata*). = ALIGN(16);*(.dynamic);. = ALIGN(512);}.rela.dyn : { *(.rela.dyn) }.rela.plt : { *(.rela.plt) }.rela.got : { *(.rela.got) }.rela.data : { *(.rela.data) *(.rela.data*) }_etext = .;_text_size = . - _text;. = ALIGN(4096);.data : {_data = .;*(.sdata)*(.data)*(.data1)*(.data.*)*(.got.plt)*(.got)/** The EFI loader doesn't seem to like a .bss section, so we* stick it all into .data:*/. = ALIGN(16);_bss = .;*(.sbss)*(.scommon)*(.dynbss)*(.bss)*(.bss.*)*(COMMON). = ALIGN(512);_bss_end = .;_edata = .;} :data_data_size = _edata - _data;. = ALIGN(4096);.dynsym : { *(.dynsym) }. = ALIGN(4096);.dynstr : { *(.dynstr) }. = ALIGN(4096);.note.gnu.build-id : { *(.note.gnu.build-id) }/DISCARD/ : {*(.rel.reloc)*(.eh_frame)*(.note.GNU-stack)}.comment 0 : { *(.comment) }-} +INCLUDE lib/efi_loader/elf_efi.ldsi diff --git a/arch/riscv/lib/elf_riscv64_efi.lds b/arch/riscv/lib/elf_riscv64_efi.lds index d0b4f3d1d64..8e4844c2eea 100644 --- a/arch/riscv/lib/elf_riscv64_efi.lds +++ b/arch/riscv/lib/elf_riscv64_efi.lds @@ -8,70 +8,4 @@ OUTPUT_FORMAT("elf64-littleriscv", "elf64-littleriscv", "elf64-littleriscv") OUTPUT_ARCH(riscv)
-PHDRS -{
data PT_LOAD FLAGS(3); /* SHF_WRITE | SHF_ALLOC */-}
-ENTRY(_start) -SECTIONS -{
.text 0x0 : {_text = .;*(.text.head)*(.text)*(.text.*)*(.gnu.linkonce.t.*)*(.srodata)*(.rodata*). = ALIGN(16);*(.dynamic);. = ALIGN(512);}.rela.dyn : { *(.rela.dyn) }.rela.plt : { *(.rela.plt) }.rela.got : { *(.rela.got) }.rela.data : { *(.rela.data) *(.rela.data*) }_etext = .;_text_size = . - _text;. = ALIGN(4096);.data : {_data = .;*(.sdata)*(.data)*(.data1)*(.data.*)*(.got.plt)*(.got)/** The EFI loader doesn't seem to like a .bss section, so we* stick it all into .data:*/. = ALIGN(16);_bss = .;*(.sbss)*(.scommon)*(.dynbss)*(.bss)*(.bss.*)*(COMMON). = ALIGN(512);_bss_end = .;_edata = .;} :data_data_size = _edata - _data;. = ALIGN(4096);.dynsym : { *(.dynsym) }. = ALIGN(4096);.dynstr : { *(.dynstr) }. = ALIGN(4096);.note.gnu.build-id : { *(.note.gnu.build-id) }/DISCARD/ : {*(.rel.reloc)*(.eh_frame)*(.note.GNU-stack)}.comment 0 : { *(.comment) }-} +INCLUDE lib/efi_loader/elf_efi.ldsi diff --git a/lib/efi_loader/elf_efi.ldsi b/lib/efi_loader/elf_efi.ldsi new file mode 100644 index 00000000000..190a88fb69e --- /dev/null +++ b/lib/efi_loader/elf_efi.ldsi @@ -0,0 +1,74 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ +/*
- U-Boot EFI linker script include
- Modified from elf_aarch64_efi.lds in gnu-efi
- */
+PHDRS +{
data PT_LOAD FLAGS(3); /* SHF_WRITE | SHF_ALLOC */+}
+ENTRY(_start) +SECTIONS +{
.text 0x0 : {_text = .;*(.text.head)*(.text)*(.text.*)*(.gnu.linkonce.t.*)*(.srodata)*(.rodata*). = ALIGN(16);*(.dynamic);. = ALIGN(512);}.rela.dyn : { *(.rela.dyn) }.rela.plt : { *(.rela.plt) }.rela.got : { *(.rela.got) }.rela.data : { *(.rela.data) *(.rela.data*) }. = ALIGN(4096);_etext = .;_text_size = . - _text;.data : {_data = .;*(.sdata)*(.data)*(.data1)*(.data.*)*(.got.plt)*(.got)/** The EFI loader doesn't seem to like a .bss section, so we* stick it all into .data:*/. = ALIGN(16);_bss = .;*(.sbss)*(.scommon)*(.dynbss)*(.bss)*(.bss.*)*(COMMON). = ALIGN(512);_bss_end = .;_edata = .;} :data_data_size = _edata - _data;. = ALIGN(4096);.dynsym : { *(.dynsym) }. = ALIGN(4096);.dynstr : { *(.dynstr) }. = ALIGN(4096);.note.gnu.build-id : { *(.note.gnu.build-id) }/DISCARD/ : {*(.rel.reloc)*(.eh_frame)*(.note.GNU-stack)}.comment 0 : { *(.comment) }+}
2.47.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Use the same include as arm64 for the linker script.
Adjust the 32-bit ARM PE-COFF header accordingly and harmonize it with the 64-bit ARM header.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- arch/arm/lib/crt0_arm_efi.S | 37 ++++++++++--------- arch/arm/lib/elf_arm_efi.lds | 71 +----------------------------------- 2 files changed, 20 insertions(+), 88 deletions(-)
diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S index b5dfd4e3819..590fcf515da 100644 --- a/arch/arm/lib/crt0_arm_efi.S +++ b/arch/arm/lib/crt0_arm_efi.S @@ -14,11 +14,11 @@ /* * Magic "MZ" signature for PE/COFF */ - .globl image_base -image_base: + .globl ImageBase +ImageBase: .short IMAGE_DOS_SIGNATURE /* 'MZ' */ .skip 58 /* 'MZ' + pad + offset == 64 */ - .long pe_header - image_base /* Offset to the PE header */ + .long pe_header - ImageBase /* Offset to the PE header */ pe_header: .long IMAGE_NT_SIGNATURE /* 'PE' */ coff_header: @@ -38,16 +38,16 @@ optional_header: .short IMAGE_NT_OPTIONAL_HDR32_MAGIC /* PE32 format */ .byte 0x02 /* MajorLinkerVersion */ .byte 0x14 /* MinorLinkerVersion */ - .long _edata - _start /* SizeOfCode */ + .long _etext - _start /* SizeOfCode */ .long 0 /* SizeOfInitializedData */ .long 0 /* SizeOfUninitializedData */ - .long _start - image_base /* AddressOfEntryPoint */ - .long _start - image_base /* BaseOfCode */ + .long _start - ImageBase /* AddressOfEntryPoint */ + .long _start - ImageBase /* BaseOfCode */ .long 0 /* BaseOfData */
extra_header_fields: - .long 0 /* image_base */ - .long 0x200 /* SectionAlignment */ + .long 0 /* ImageBase */ + .long 0x1000 /* SectionAlignment */ .long 0x200 /* FileAlignment */ .short 0 /* MajorOperatingSystemVersion */ .short 0 /* MinorOperatingSystemVersion */ @@ -57,14 +57,14 @@ extra_header_fields: .short 0 /* MinorSubsystemVersion */ .long 0 /* Win32VersionValue */
- .long _edata - image_base /* SizeOfImage */ + .long _edata - ImageBase /* SizeOfImage */
/* * Everything before the kernel image is considered part of the header */ - .long _start - image_base /* SizeOfHeaders */ + .long _start - ImageBase /* SizeOfHeaders */ .long 0 /* CheckSum */ - .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ + .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ #if CONFIG_VENDOR_EFI .short 0 /* DllCharacteristics */ #else @@ -84,6 +84,7 @@ extra_header_fields: .quad 0 /* CertificationTable */ .quad 0 /* BaseRelocationTable */
+ /* Section table */ section_table:
/* @@ -111,10 +112,10 @@ section_table: .byte 0 .byte 0 .byte 0 /* end of 0 padding of section name */ - .long _text_size /* VirtualSize */ - .long _start - image_base /* VirtualAddress */ - .long _text_size /* SizeOfRawData */ - .long _start - image_base /* PointerToRawData */ + .long _etext - _start /* VirtualSize */ + .long _start - ImageBase /* VirtualAddress */ + .long _etext - _start /* SizeOfRawData */ + .long _start - ImageBase /* PointerToRawData */ .long 0 /* PointerToRelocations */ .long 0 /* PointerToLineNumbers */ .short 0 /* NumberOfRelocations */ @@ -129,9 +130,9 @@ section_table: .byte 0 .byte 0 /* end of 0 padding of section name */ .long _data_size /* VirtualSize */ - .long _data - image_base /* VirtualAddress */ + .long _data - ImageBase /* VirtualAddress */ .long _data_size /* SizeOfRawData */ - .long _data - image_base /* PointerToRawData */ + .long _data - ImageBase /* PointerToRawData */ .long 0 /* PointerToRelocations */ .long 0 /* PointerToLineNumbers */ .short 0 /* NumberOfRelocations */ @@ -148,7 +149,7 @@ _start: adr r1, .L_DYNAMIC ldr r0, [r1] add r1, r0, r1 - adrl r0, image_base + adrl r0, ImageBase bl _relocate teq r0, #0 bne 0f diff --git a/arch/arm/lib/elf_arm_efi.lds b/arch/arm/lib/elf_arm_efi.lds index 41440594aa6..eb16fae74cf 100644 --- a/arch/arm/lib/elf_arm_efi.lds +++ b/arch/arm/lib/elf_arm_efi.lds @@ -8,73 +8,4 @@ OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm)
-PHDRS -{ - data PT_LOAD FLAGS(3); /* PF_W | PF_X */ -} - -ENTRY(_start) -SECTIONS -{ - .text 0x0 : { - _text = .; - *(.text.head) - *(.text) - *(.text.*) - *(.gnu.linkonce.t.*) - *(.srodata) - *(.rodata*) - . = ALIGN(16); - *(.dynamic); - . = ALIGN(512); - } - _etext = .; - _text_size = . - _text; - . = ALIGN(4096); - .data : { - _data = .; - *(.sdata) - *(.data) - *(.data1) - *(.data.*) - *(.got.plt) - *(.got) - - /* - * The EFI loader doesn't seem to like a .bss section, so we - * stick it all into .data: - */ - . = ALIGN(16); - _bss = .; - *(.sbss) - *(.scommon) - *(.dynbss) - *(.bss) - *(.bss.*) - *(COMMON) - . = ALIGN(512); - _bss_end = .; - _edata = .; - } :data - _data_size = . - _data; - - /DISCARD/ : { - /* - * We don't support relocations. These would have to be - * translated from ELF to PE format and added to the .reloc - * section. - */ - *(.rel.dyn) - *(.rel.plt) - *(.rel.got) - *(.rel.data) - *(.rel.data*) - *(.rel.reloc) - *(.eh_frame) - *(.note.GNU-stack) - *(.dynsym) - *(.dynstr) - *(.note.gnu.build-id) - *(.comment) - } -} +INCLUDE lib/efi_loader/elf_efi.ldsi
Hi Heinrich
On Tue, 14 Jan 2025 at 12:30, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Use the same include as arm64 for the linker script.
Adjust the 32-bit ARM PE-COFF header accordingly and harmonize it with the 64-bit ARM header.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
arch/arm/lib/crt0_arm_efi.S | 37 ++++++++++--------- arch/arm/lib/elf_arm_efi.lds | 71 +----------------------------------- 2 files changed, 20 insertions(+), 88 deletions(-)
diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S index b5dfd4e3819..590fcf515da 100644 --- a/arch/arm/lib/crt0_arm_efi.S +++ b/arch/arm/lib/crt0_arm_efi.S @@ -14,11 +14,11 @@ /* * Magic "MZ" signature for PE/COFF */
.globl image_base-image_base:
.globl ImageBase+ImageBase: .short IMAGE_DOS_SIGNATURE /* 'MZ' */ .skip 58 /* 'MZ' + pad + offset == 64 */
.long pe_header - image_base /* Offset to the PE header */
.long pe_header - ImageBase /* Offset to the PE header */pe_header: .long IMAGE_NT_SIGNATURE /* 'PE' */ coff_header: @@ -38,16 +38,16 @@ optional_header: .short IMAGE_NT_OPTIONAL_HDR32_MAGIC /* PE32 format */ .byte 0x02 /* MajorLinkerVersion */ .byte 0x14 /* MinorLinkerVersion */
.long _edata - _start /* SizeOfCode */
.long _etext - _start /* SizeOfCode */
Was that an error all along? Or the boundaries changed by using the include file?
.long 0 /* SizeOfInitializedData */ .long 0 /* SizeOfUninitializedData */
.long _start - image_base /* AddressOfEntryPoint */.long _start - image_base /* BaseOfCode */
.long _start - ImageBase /* AddressOfEntryPoint */.long _start - ImageBase /* BaseOfCode */ .long 0 /* BaseOfData */
[...]
Other than that it looks ok
Thanks for cleaning this up! /Ilias
On 15.01.25 09:06, Ilias Apalodimas wrote:
Hi Heinrich
On Tue, 14 Jan 2025 at 12:30, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Use the same include as arm64 for the linker script.
Adjust the 32-bit ARM PE-COFF header accordingly and harmonize it with the 64-bit ARM header.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
arch/arm/lib/crt0_arm_efi.S | 37 ++++++++++--------- arch/arm/lib/elf_arm_efi.lds | 71 +----------------------------------- 2 files changed, 20 insertions(+), 88 deletions(-)
diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S index b5dfd4e3819..590fcf515da 100644 --- a/arch/arm/lib/crt0_arm_efi.S +++ b/arch/arm/lib/crt0_arm_efi.S @@ -14,11 +14,11 @@ /* * Magic "MZ" signature for PE/COFF */
.globl image_base-image_base:
.globl ImageBase+ImageBase: .short IMAGE_DOS_SIGNATURE /* 'MZ' */ .skip 58 /* 'MZ' + pad + offset == 64 */
.long pe_header - image_base /* Offset to the PE header */
.long pe_header - ImageBase /* Offset to the PE header */@@ -38,16 +38,16 @@ optional_header: .short IMAGE_NT_OPTIONAL_HDR32_MAGIC /* PE32 format */ .byte 0x02 /* MajorLinkerVersion */ .byte 0x14 /* MinorLinkerVersion */
.long _edata - _start /* SizeOfCode */
.long _etext - _start /* SizeOfCode */Was that an error all along? Or the boundaries changed by using the include file?
According to https://learn.microsoft.com/en-us/windows/win32/debug/pe-format
SizeOfCode: "The size of the code (text) section, or the sum of all code sections if there are multiple sections."
We only have one code section.
Looking at EDK2's Shell.efi SizeOfCode equals the VirtualSize of the .text section. It does not include the .data section.
Best regards
Heinrich
.long 0 /* SizeOfInitializedData */ .long 0 /* SizeOfUninitializedData */
.long _start - image_base /* AddressOfEntryPoint */.long _start - image_base /* BaseOfCode */
.long _start - ImageBase /* AddressOfEntryPoint */.long _start - ImageBase /* BaseOfCode */ .long 0 /* BaseOfData */[...]
Other than that it looks ok
Thanks for cleaning this up! /Ilias
On Wed, 15 Jan 2025 at 10:53, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 15.01.25 09:06, Ilias Apalodimas wrote:
Hi Heinrich
On Tue, 14 Jan 2025 at 12:30, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Use the same include as arm64 for the linker script.
Adjust the 32-bit ARM PE-COFF header accordingly and harmonize it with the 64-bit ARM header.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
arch/arm/lib/crt0_arm_efi.S | 37 ++++++++++--------- arch/arm/lib/elf_arm_efi.lds | 71 +----------------------------------- 2 files changed, 20 insertions(+), 88 deletions(-)
diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S index b5dfd4e3819..590fcf515da 100644 --- a/arch/arm/lib/crt0_arm_efi.S +++ b/arch/arm/lib/crt0_arm_efi.S @@ -14,11 +14,11 @@ /* * Magic "MZ" signature for PE/COFF */
.globl image_base-image_base:
.globl ImageBase+ImageBase: .short IMAGE_DOS_SIGNATURE /* 'MZ' */ .skip 58 /* 'MZ' + pad + offset == 64 */
.long pe_header - image_base /* Offset to the PE header */
.long pe_header - ImageBase /* Offset to the PE header */@@ -38,16 +38,16 @@ optional_header: .short IMAGE_NT_OPTIONAL_HDR32_MAGIC /* PE32 format */ .byte 0x02 /* MajorLinkerVersion */ .byte 0x14 /* MinorLinkerVersion */
.long _edata - _start /* SizeOfCode */
.long _etext - _start /* SizeOfCode */Was that an error all along? Or the boundaries changed by using the include file?
According to https://learn.microsoft.com/en-us/windows/win32/debug/pe-format
SizeOfCode: "The size of the code (text) section, or the sum of all code sections if there are multiple sections."
We only have one code section.
Looking at EDK2's Shell.efi SizeOfCode equals the VirtualSize of the .text section. It does not include the .data section.
Hmm that's interesting. gnu-efi includes .data AFAICT. Looking at the pe/coff spec there's SizeOfCode and SizeOfInitializedData (for .data) and SizeOfUninitializedData (for .bss). But in both gnu-efi and U-Boot we define those two as 0 though. So I think we should either keep SizeOfData as is, or define it as you propose and also define SizeOfInitializedData and SizeOfUninitializedData correctly -- and I don't know if the latter will have any implication on loaders.
Cheers /Ilias
Best regards
Heinrich
.long 0 /* SizeOfInitializedData */ .long 0 /* SizeOfUninitializedData */
.long _start - image_base /* AddressOfEntryPoint */.long _start - image_base /* BaseOfCode */
.long _start - ImageBase /* AddressOfEntryPoint */.long _start - ImageBase /* BaseOfCode */ .long 0 /* BaseOfData */[...]
Other than that it looks ok
Thanks for cleaning this up! /Ilias
On 15.01.25 10:34, Ilias Apalodimas wrote:
On Wed, 15 Jan 2025 at 10:53, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 15.01.25 09:06, Ilias Apalodimas wrote:
Hi Heinrich
On Tue, 14 Jan 2025 at 12:30, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Use the same include as arm64 for the linker script.
Adjust the 32-bit ARM PE-COFF header accordingly and harmonize it with the 64-bit ARM header.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
arch/arm/lib/crt0_arm_efi.S | 37 ++++++++++--------- arch/arm/lib/elf_arm_efi.lds | 71 +----------------------------------- 2 files changed, 20 insertions(+), 88 deletions(-)
diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S index b5dfd4e3819..590fcf515da 100644 --- a/arch/arm/lib/crt0_arm_efi.S +++ b/arch/arm/lib/crt0_arm_efi.S @@ -14,11 +14,11 @@ /* * Magic "MZ" signature for PE/COFF */
.globl image_base-image_base:
.globl ImageBase+ImageBase: .short IMAGE_DOS_SIGNATURE /* 'MZ' */ .skip 58 /* 'MZ' + pad + offset == 64 */
.long pe_header - image_base /* Offset to the PE header */
.long pe_header - ImageBase /* Offset to the PE header */@@ -38,16 +38,16 @@ optional_header: .short IMAGE_NT_OPTIONAL_HDR32_MAGIC /* PE32 format */ .byte 0x02 /* MajorLinkerVersion */ .byte 0x14 /* MinorLinkerVersion */
.long _edata - _start /* SizeOfCode */
.long _etext - _start /* SizeOfCode */Was that an error all along? Or the boundaries changed by using the include file?
According to https://learn.microsoft.com/en-us/windows/win32/debug/pe-format
SizeOfCode: "The size of the code (text) section, or the sum of all code sections if there are multiple sections."
We only have one code section.
Looking at EDK2's Shell.efi SizeOfCode equals the VirtualSize of the .text section. It does not include the .data section.
Hmm that's interesting. gnu-efi includes .data AFAICT. Looking at the pe/coff spec there's SizeOfCode and SizeOfInitializedData (for .data) and SizeOfUninitializedData (for .bss). But in both gnu-efi and U-Boot we define those two as 0 though. So I think we should either keep SizeOfData as is, or define it as you propose and also define SizeOfInitializedData and SizeOfUninitializedData correctly -- and I don't know if the latter will have any implication on loaders.
Arm64 and RISC-V are already using SizeOfCode = _etext - _start. This patch only followed up on 32-bit ARM.
We should follow the example of EDK II and fill SizeOfInitializedData.
We are putting the _bss sections into .data. So SizeOfUninitializedData should be kept at 0.
Best regards
Heinrich
Cheers /Ilias
Best regards
Heinrich
.long 0 /* SizeOfInitializedData */ .long 0 /* SizeOfUninitializedData */
.long _start - image_base /* AddressOfEntryPoint */.long _start - image_base /* BaseOfCode */
.long _start - ImageBase /* AddressOfEntryPoint */.long _start - ImageBase /* BaseOfCode */ .long 0 /* BaseOfData */[...]
Other than that it looks ok
Thanks for cleaning this up! /Ilias
participants (2)
-
Heinrich Schuchardt -
Ilias Apalodimas