Re: [PATCH 1/1] efi_loader: fix building aarch64 EFI binaries

2023-01-04 Thread Ilias Apalodimas
Hi Heinrich 

On Sat, Dec 31, 2022 at 12:03:45PM +0100, Heinrich Schuchardt wrote:
> While our EFI binaries execute without problems on EDK II they crash on
> a Lenovo X13s. Let our binaries look more like what EDK II produces:
> 
> * move all writable data to a .data section
> * align sections to 4 KiB boundaries (matching EFI page size)
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  arch/arm/lib/crt0_aarch64_efi.S  | 35 ++--
>  arch/arm/lib/elf_aarch64_efi.lds |  6 --
>  2 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/lib/crt0_aarch64_efi.S b/arch/arm/lib/crt0_aarch64_efi.S
> index b4fc263adf..52b2482efe 100644
> --- a/arch/arm/lib/crt0_aarch64_efi.S
> +++ b/arch/arm/lib/crt0_aarch64_efi.S
> @@ -25,7 +25,7 @@ pe_header:
>   .long   IMAGE_NT_SIGNATURE  /* 'PE' */
>  coff_header:
>   .short  IMAGE_FILE_MACHINE_ARM64/* AArch64 */
> - .short  2   /* nr_sections */
> + .short  3   /* nr_sections */
>   .long   0   /* TimeDateStamp */
>   .long   0   /* PointerToSymbolTable */
>   .long   0   /* NumberOfSymbols */
> @@ -40,7 +40,7 @@ optional_header:
>   .short  IMAGE_NT_OPTIONAL_HDR64_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 - ImageBase  /* AddressOfEntryPoint */
> @@ -48,7 +48,7 @@ optional_header:
>  
>  extra_header_fields:
>   .quad   0   /* ImageBase */
> - .long   0x200   /* SectionAlignment */
> + .long   0x1000  /* SectionAlignment */
>   .long   0x200   /* FileAlignment */
>   .short  0   /* MajorOperatingSystemVersion 
> */
>   .short  0   /* MinorOperatingSystemVersion 
> */
> @@ -107,18 +107,31 @@ section_table:
>   .byte   0
>   .byte   0
>   .byte   0   /* end of 0 padding of section name */
> - .long   _edata - _start /* VirtualSize */
> + .long   _etext - _start /* VirtualSize */
>   .long   _start - ImageBase  /* VirtualAddress */
> - .long   _edata - _start /* SizeOfRawData */
> + .long   _etext - _start /* SizeOfRawData */
>   .long   _start - ImageBase  /* PointerToRawData */
> + .long   0   /* PointerToRelocations */
> + .long   0   /* PointerToLineNumbers */
> + .short  0   /* NumberOfRelocations */
> + .short  0   /* NumberOfLineNumbers */
> + .long   0x6020  /* Characteristics (section flags) */

Can we replace the 0x6020 magic here with defines from
include/asm-generic/pe.h?

>  
> - .long   0   /* PointerToRelocations (0 for executables) */
> - .long   0   /* PointerToLineNumbers (0 for executables) */
> - .short  0   /* NumberOfRelocations  (0 for executables) */
> - .short  0   /* NumberOfLineNumbers  (0 for executables) */
> - .long   0xe0500020  /* Characteristics (section flags) */
> + .ascii  ".data"
> + .byte   0
> + .byte   0
> + .byte   0   /* end of 0 padding of section name */
> + .long   _data_size  /* VirtualSize */
> + .long   _data - ImageBase   /* VirtualAddress */
> + .long   _data_size  /* SizeOfRawData */
> + .long   _data - ImageBase   /* PointerToRawData */
> + .long   0   /* PointerToRelocations */
> + .long   0   /* PointerToLineNumbers */
> + .short  0   /* NumberOfRelocations */
> + .short  0   /* NumberOfLineNumbers */
> + .long   0xc040  /* Characteristics (section flags) */

Same here

>  
> - .align  9
> + .align  12
>  _start:
>   stp x29, x30, [sp, #-32]!
>   mov x29, sp
> diff --git a/arch/arm/lib/elf_aarch64_efi.lds 
> b/arch/arm/lib/elf_aarch64_efi.lds
> index c0604dad46..ffc6f6e604 100644
> --- a/arch/arm/lib/elf_aarch64_efi.lds
> +++ b/arch/arm/lib/elf_aarch64_efi.lds
> @@ -18,11 +18,13 @@ SECTIONS
>   *(.gnu.linkonce.t.*)
>   *(.srodata)
>   *(.rodata*)
> + . = ALIGN(16);
> + 

Re: [PATCH 1/1] efi_loader: fix building aarch64 EFI binaries

2023-01-03 Thread Simon Glass
On Sat, 31 Dec 2022 at 05:04, Heinrich Schuchardt
 wrote:
>
> While our EFI binaries execute without problems on EDK II they crash on
> a Lenovo X13s. Let our binaries look more like what EDK II produces:
>
> * move all writable data to a .data section
> * align sections to 4 KiB boundaries (matching EFI page size)
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  arch/arm/lib/crt0_aarch64_efi.S  | 35 ++--
>  arch/arm/lib/elf_aarch64_efi.lds |  6 --
>  2 files changed, 28 insertions(+), 13 deletions(-)

Reviewed-by: Simon Glass 


[PATCH 1/1] efi_loader: fix building aarch64 EFI binaries

2022-12-31 Thread Heinrich Schuchardt
While our EFI binaries execute without problems on EDK II they crash on
a Lenovo X13s. Let our binaries look more like what EDK II produces:

* move all writable data to a .data section
* align sections to 4 KiB boundaries (matching EFI page size)

Signed-off-by: Heinrich Schuchardt 
---
 arch/arm/lib/crt0_aarch64_efi.S  | 35 ++--
 arch/arm/lib/elf_aarch64_efi.lds |  6 --
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/arch/arm/lib/crt0_aarch64_efi.S b/arch/arm/lib/crt0_aarch64_efi.S
index b4fc263adf..52b2482efe 100644
--- a/arch/arm/lib/crt0_aarch64_efi.S
+++ b/arch/arm/lib/crt0_aarch64_efi.S
@@ -25,7 +25,7 @@ pe_header:
.long   IMAGE_NT_SIGNATURE  /* 'PE' */
 coff_header:
.short  IMAGE_FILE_MACHINE_ARM64/* AArch64 */
-   .short  2   /* nr_sections */
+   .short  3   /* nr_sections */
.long   0   /* TimeDateStamp */
.long   0   /* PointerToSymbolTable */
.long   0   /* NumberOfSymbols */
@@ -40,7 +40,7 @@ optional_header:
.short  IMAGE_NT_OPTIONAL_HDR64_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 - ImageBase  /* AddressOfEntryPoint */
@@ -48,7 +48,7 @@ optional_header:
 
 extra_header_fields:
.quad   0   /* ImageBase */
-   .long   0x200   /* SectionAlignment */
+   .long   0x1000  /* SectionAlignment */
.long   0x200   /* FileAlignment */
.short  0   /* MajorOperatingSystemVersion 
*/
.short  0   /* MinorOperatingSystemVersion 
*/
@@ -107,18 +107,31 @@ section_table:
.byte   0
.byte   0
.byte   0   /* end of 0 padding of section name */
-   .long   _edata - _start /* VirtualSize */
+   .long   _etext - _start /* VirtualSize */
.long   _start - ImageBase  /* VirtualAddress */
-   .long   _edata - _start /* SizeOfRawData */
+   .long   _etext - _start /* SizeOfRawData */
.long   _start - ImageBase  /* PointerToRawData */
+   .long   0   /* PointerToRelocations */
+   .long   0   /* PointerToLineNumbers */
+   .short  0   /* NumberOfRelocations */
+   .short  0   /* NumberOfLineNumbers */
+   .long   0x6020  /* Characteristics (section flags) */
 
-   .long   0   /* PointerToRelocations (0 for executables) */
-   .long   0   /* PointerToLineNumbers (0 for executables) */
-   .short  0   /* NumberOfRelocations  (0 for executables) */
-   .short  0   /* NumberOfLineNumbers  (0 for executables) */
-   .long   0xe0500020  /* Characteristics (section flags) */
+   .ascii  ".data"
+   .byte   0
+   .byte   0
+   .byte   0   /* end of 0 padding of section name */
+   .long   _data_size  /* VirtualSize */
+   .long   _data - ImageBase   /* VirtualAddress */
+   .long   _data_size  /* SizeOfRawData */
+   .long   _data - ImageBase   /* PointerToRawData */
+   .long   0   /* PointerToRelocations */
+   .long   0   /* PointerToLineNumbers */
+   .short  0   /* NumberOfRelocations */
+   .short  0   /* NumberOfLineNumbers */
+   .long   0xc040  /* Characteristics (section flags) */
 
-   .align  9
+   .align  12
 _start:
stp x29, x30, [sp, #-32]!
mov x29, sp
diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds
index c0604dad46..ffc6f6e604 100644
--- a/arch/arm/lib/elf_aarch64_efi.lds
+++ b/arch/arm/lib/elf_aarch64_efi.lds
@@ -18,11 +18,13 @@ SECTIONS
*(.gnu.linkonce.t.*)
*(.srodata)
*(.rodata*)
+   . = ALIGN(16);
+   *(.dynamic);
. = ALIGN(512);
}
_etext = .;
_text_size = . - _text;
-   .dynamic  : { *(.dynamic) }
+   . = ALIGN(4096);
.data : {
_data = .;
*(.sdata)
@@ -48,11 +50,11 @@