efi_loader: fix building crt0 on arm
Subsystems (2)
PATCH 1/1 efi_loader: fix building crt0 on arm
Before the patch an undefined constant EFI_SUBSYSTEM was used in the crt0 code. The current version of binutils does not swallow the error.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=888403
The necessary constant IMAGE_SUBSYSTEM_EFI_APPLICATION is already defined in pe.h. So let's factor out asm-generic/pe.h for the image subsystem constants and use it in our assembler code.
IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVER does not exist in the specification let's use IMAGE_SUBSYSTEM_EFI_ROM instead.
The include pe.h is only used in code maintained by Alex so let him be the maintainer here too.
--- MAINTAINERS | 2 ++ arch/arm/lib/crt0_aarch64_efi.S | 4 +++- arch/arm/lib/crt0_arm_efi.S | 4 +++- include/asm-generic/pe.h | 21 +++++++++++++++++++++ include/pe.h | 8 ++------ lib/efi_loader/efi_image_loader.c | 2 +- 6 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 include/asm-generic/pe.h
MAINTAINERS +2 -0
@@ -291,6 +291,8 @@ S: Maintained T: git git://github.com/agraf/u-boot.git F: doc/README.iscsi F: include/efi* +F: include/pe.h +F: include/asm-generic/pe.h F: lib/efi*/ F: test/py/tests/test_efi* F: cmd/bootefi.c
arch/arm/lib/crt0_aarch64_efi.S +3 -1
@@ -8,6 +8,8 @@ * This file is taken and modified from the gnu-efi project. */ +#include <asm-generic/pe.h> + .section .text.head /*
@@ -62,7 +64,7 @@ extra_header_fields: */ .long _start - ImageBase /* SizeOfHeaders */ .long 0 /* CheckSum */ - .short EFI_SUBSYSTEM /* Subsystem */ + .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ .short 0 /* DllCharacteristics */ .quad 0 /* SizeOfStackReserve */ .quad 0 /* SizeOfStackCommit */
arch/arm/lib/crt0_arm_efi.S +3 -1
@@ -8,6 +8,8 @@ * This file is taken and modified from the gnu-efi project. */ +#include <asm-generic/pe.h> + .section .text.head /*
@@ -64,7 +66,7 @@ extra_header_fields: */ .long _start - image_base /* SizeOfHeaders */ .long 0 /* CheckSum */ - .short EFI_SUBSYSTEM /* Subsystem */ + .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ .short 0 /* DllCharacteristics */ .long 0 /* SizeOfStackReserve */ .long 0 /* SizeOfStackCommit */
include/asm-generic/pe.h new file +21 -0
@@ -0,0 +1,21 @@ +/* +* Portable Executable and Common Object Constants +* +* Copyright (c) 2018 Heinrich Schuchardt +* +* based on the "Microsoft Portable Executable and Common Object File Format +* Specification", revision 11, 2017-01-23 +* +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#ifndef _ASM_PE_H +#define _ASM_PE_H + +/* Subsystem type */ +#define IMAGE_SUBSYSTEM_EFI_APPLICATION 10 +#define IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER 11 +#define IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER 12 +#define IMAGE_SUBSYSTEM_EFI_ROM 13 + +#endif /* _ASM_PE_H */
include/pe.h +2 -6
@@ -11,6 +11,8 @@ #ifndef _PE_H #define _PE_H +#include <asm-generic/pe.h> + typedef struct _IMAGE_DOS_HEADER { uint16_t e_magic; /* 00: MZ Header signature */ uint16_t e_cblp; /* 02: Bytes on last page of file */
@@ -62,12 +64,6 @@ typedef struct _IMAGE_DATA_DIRECTORY {#define IMAGE_NUMBEROF_DIRECTORY_ENTRIES 16 -/* PE32+ Subsystem type for EFI images */ -#define IMAGE_SUBSYSTEM_EFI_APPLICATION 10 -#define IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER 11 -#define IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER 12 -#define IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVER 13 - typedef struct _IMAGE_OPTIONAL_HEADER64 { uint16_t Magic; /* 0x20b */ uint8_t MajorLinkerVersion;
lib/efi_loader/efi_image_loader.c +1 -1
@@ -94,7 +94,7 @@ static void efi_set_code_and_data_type( loaded_image_info->image_data_type =EFI_BOOT_SERVICES_DATA ;break ;case IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER : -case IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVER : +case IMAGE_SUBSYSTEM_EFI_ROM : loaded_image_info->image_code_type =EFI_RUNTIME_SERVICES_CODE ; loaded_image_info->image_data_type =EFI_RUNTIME_SERVICES_DATA ;break ;
Signature · 1 line
2.15.1
PATCH 1/1 efi_loader: fix building crt0 on arm
Quoted 19 lines · Heinrich Schuchardt
Before the patch an undefined constant
EFI_SUBSYSTEMwas used in the crt0 code. The current version of binutils does not swallow the error.https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=888403
The necessary constant
IMAGE_SUBSYSTEM_EFI_APPLICATIONis already defined inpe.h. So let's factor outasm-generic/pe.hfor the image subsystem constants and use it in our assembler code.
IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVERdoes not exist in the specification let's useIMAGE_SUBSYSTEM_EFI_ROMinstead.The include
pe.his only used in code maintained by Alex so let him be the maintainer here too.Reported-by Andre Przywara <[email protected]>Cc Alexander Graf <[email protected]>Signed-off-by Heinrich Schuchardt <[email protected]>
Quoted 9 lines
--- MAINTAINERS | 2 ++ arch/arm/lib/crt0_aarch64_efi.S | 4 +++- arch/arm/lib/crt0_arm_efi.S | 4 +++- include/asm-generic/pe.h | 21 +++++++++++++++++++++ include/pe.h | 8 ++------ lib/efi_loader/efi_image_loader.c | 2 +- 6 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 include/asm-generic/pe.h
I tested that it at least fixed the build failure with v2018.03-rc1, though I haven't tested the resulting images actually boot.
Also haven't been able to successfully backported it to v2018.01...
With those caveats:
live well,
vagrant
Quoted 115 lines
MAINTAINERS +2
-0@@ -291,6 +291,8 @@ S: Maintained T: git git://github.com/agraf/u-boot.git F: doc/README.iscsi F: include/efi* +F: include/pe.h +F: include/asm-generic/pe.h F: lib/efi*/ F: test/py/tests/test_efi* F: cmd/bootefi.carch/arm/lib/crt0_aarch64_efi.S +3
-1@@ -8,6 +8,8 @@ * This file is taken and modified from the gnu-efi project. */ +#include <asm-generic/pe.h> + .section .text.head /*@@ -62,7 +64,7 @@ extra_header_fields: */ .long _start - ImageBase /* SizeOfHeaders */ .long 0 /* CheckSum */ - .short EFI_SUBSYSTEM /* Subsystem */ + .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ .short 0 /* DllCharacteristics */ .quad 0 /* SizeOfStackReserve */ .quad 0 /* SizeOfStackCommit */arch/arm/lib/crt0_arm_efi.S +3
-1@@ -8,6 +8,8 @@ * This file is taken and modified from the gnu-efi project. */ +#include <asm-generic/pe.h> + .section .text.head /*@@ -64,7 +66,7 @@ extra_header_fields: */ .long _start - image_base /* SizeOfHeaders */ .long 0 /* CheckSum */ - .short EFI_SUBSYSTEM /* Subsystem */ + .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ .short 0 /* DllCharacteristics */ .long 0 /* SizeOfStackReserve */ .long 0 /* SizeOfStackCommit */include/asm-generic/pe.h new file +21
-0@@ -0,0 +1,21 @@ +/* +* Portable Executable and Common Object Constants +* +* Copyright (c) 2018 Heinrich Schuchardt +* +* based on the "Microsoft Portable Executable and Common Object File Format +* Specification", revision 11, 2017-01-23 +* +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#ifndef _ASM_PE_H +#define _ASM_PE_H + +/* Subsystem type */ +#define IMAGE_SUBSYSTEM_EFI_APPLICATION 10 +#define IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER 11 +#define IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER 12 +#define IMAGE_SUBSYSTEM_EFI_ROM 13 + +#endif /* _ASM_PE_H */ include/pe.h +2
-6@@ -11,6 +11,8 @@ #ifndef _PE_H #define _PE_H +#include <asm-generic/pe.h> + typedef struct _IMAGE_DOS_HEADER { uint16_t e_magic; /* 00: MZ Header signature */ uint16_t e_cblp; /* 02: Bytes on last page of file */@@ -62,12 +64,6 @@ typedef struct _IMAGE_DATA_DIRECTORY {#define IMAGE_NUMBEROF_DIRECTORY_ENTRIES 16 -/* PE32+ Subsystem type for EFI images */ -#define IMAGE_SUBSYSTEM_EFI_APPLICATION 10 -#define IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER 11 -#define IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER 12 -#define IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVER 13 - typedef struct _IMAGE_OPTIONAL_HEADER64 { uint16_t Magic; /* 0x20b */ uint8_t MajorLinkerVersion;lib/efi_loader/efi_image_loader.c +0
-0
lib/efi_loader/efi_image_loader.c static void efi_set_code_and_data_type( · line 100
Quoted 8 lines
lib/efi_loader/efi_image_loader.c +1
-1@@ -94,7 +94,7 @@ static void efi_set_code_and_data_type( loaded_image_info->image_data_type =EFI_BOOT_SERVICES_DATA ;break ;case IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER : -case IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVER : +case IMAGE_SUBSYSTEM_EFI_ROM : loaded_image_info->image_code_type =EFI_RUNTIME_SERVICES_CODE ; loaded_image_info->image_data_type =EFI_RUNTIME_SERVICES_DATA ;break ;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180131/6bc7b640/attachment.sig>
PATCH 1/1 efi_loader: fix building crt0 on arm
In reply to Heinrich SchuchardtHi Heinrich,
Quoted 2 lines · Heinrich Schuchardt
Before the patch an undefined constant
EFI_SUBSYSTEMwas used in the crt0 code. The current version of binutils does not swallow the error.
Ah, indeed it was the switch to binutils 2.30 in my case that made the difference, not the GCC version. Thanks for pointing that out!
Quoted 11 lines
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=888403
The necessary constant
IMAGE_SUBSYSTEM_EFI_APPLICATIONis already defined inpe.h. So let's factor outasm-generic/pe.hfor the image subsystem constants and use it in our assembler code.
IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVERdoes not exist in the specification let's useIMAGE_SUBSYSTEM_EFI_ROMinstead.The include
pe.his only used in code maintained by Alex so let him be the maintainer here too.
Thanks very much for the quick handling!
The patch looks reasonable, the constants are correct. Just wondering how this worked before? EFI_SUBSYSTEM is clearly not defined anywhere, so does it really get silently translated to 0? Amazing...
So indeed that fixes it for me. Quickly boot tested on the SoPine64 with "bootefi hello".
Cheers,
Andre.
Quoted 137 lines
Reported-by Andre Przywara <[email protected]>Cc Alexander Graf <[email protected]>Signed-off-by Heinrich Schuchardt <[email protected]>--- MAINTAINERS | 2 ++ arch/arm/lib/crt0_aarch64_efi.S | 4 +++- arch/arm/lib/crt0_arm_efi.S | 4 +++- include/asm-generic/pe.h | 21 +++++++++++++++++++++ include/pe.h | 8 ++------ lib/efi_loader/efi_image_loader.c | 2 +- 6 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 include/asm-generic/pe.hMAINTAINERS +2
-0@@ -291,6 +291,8 @@ S: Maintained T: git git://github.com/agraf/u-boot.git F: doc/README.iscsi F: include/efi* +F: include/pe.h +F: include/asm-generic/pe.h F: lib/efi*/ F: test/py/tests/test_efi* F: cmd/bootefi.carch/arm/lib/crt0_aarch64_efi.S +3
-1@@ -8,6 +8,8 @@ * This file is taken and modified from the gnu-efi project. */ +#include <asm-generic/pe.h> + .section .text.head /*@@ -62,7 +64,7 @@ extra_header_fields: */ .long _start - ImageBase /* SizeOfHeaders */ .long 0 /* CheckSum */ - .short EFI_SUBSYSTEM /* Subsystem */ + .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ .short 0 /* DllCharacteristics */ .quad 0 /* SizeOfStackReserve */ .quad 0 /* SizeOfStackCommit */arch/arm/lib/crt0_arm_efi.S +3
-1@@ -8,6 +8,8 @@ * This file is taken and modified from the gnu-efi project. */ +#include <asm-generic/pe.h> + .section .text.head /*@@ -64,7 +66,7 @@ extra_header_fields: */ .long _start - image_base /* SizeOfHeaders */ .long 0 /* CheckSum */ - .short EFI_SUBSYSTEM /* Subsystem */ + .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ .short 0 /* DllCharacteristics */ .long 0 /* SizeOfStackReserve */ .long 0 /* SizeOfStackCommit */include/asm-generic/pe.h new file +21
-0@@ -0,0 +1,21 @@ +/* +* Portable Executable and Common Object Constants +* +* Copyright (c) 2018 Heinrich Schuchardt +* +* based on the "Microsoft Portable Executable and Common Object File Format +* Specification", revision 11, 2017-01-23 +* +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#ifndef _ASM_PE_H +#define _ASM_PE_H + +/* Subsystem type */ +#define IMAGE_SUBSYSTEM_EFI_APPLICATION 10 +#define IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER 11 +#define IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER 12 +#define IMAGE_SUBSYSTEM_EFI_ROM 13 + +#endif /* _ASM_PE_H */ include/pe.h +2
-6@@ -11,6 +11,8 @@ #ifndef _PE_H #define _PE_H +#include <asm-generic/pe.h> + typedef struct _IMAGE_DOS_HEADER { uint16_t e_magic; /* 00: MZ Header signature */ uint16_t e_cblp; /* 02: Bytes on last page of file */@@ -62,12 +64,6 @@ typedef struct _IMAGE_DATA_DIRECTORY {#define IMAGE_NUMBEROF_DIRECTORY_ENTRIES 16 -/* PE32+ Subsystem type for EFI images */ -#define IMAGE_SUBSYSTEM_EFI_APPLICATION 10 -#define IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER 11 -#define IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER 12 -#define IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVER 13 - typedef struct _IMAGE_OPTIONAL_HEADER64 { uint16_t Magic; /* 0x20b */ uint8_t MajorLinkerVersion;lib/efi_loader/efi_image_loader.c +1
-1@@ -94,7 +94,7 @@ static void efi_set_code_and_data_type( loaded_image_info->image_data_type =EFI_BOOT_SERVICES_DATA ;break ;case IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER : -case IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVER : +case IMAGE_SUBSYSTEM_EFI_ROM : loaded_image_info->image_code_type =EFI_RUNTIME_SERVICES_CODE ; loaded_image_info->image_data_type =EFI_RUNTIME_SERVICES_DATA ;break ;