[U-Boot] [PATCH 1/1] efi_loader: fix building crt0 on arm
Andre Przywara
andre.przywara at arm.com
Fri Feb 2 12:05:42 UTC 2018
Hi Heinrich,
On 31/01/18 18:45, Heinrich Schuchardt wrote:
> Before the patch an undefined constant EFI_SUBSYSTEM was 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!
> 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.
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".
Reviewed-by: Andre Przywara <andre.przywara at arm.com>
Tested-by: Andre Przywara <andre.przywara at arm.com>
Cheers,
Andre.
> Reported-by: Andre Przywara <andre.przywara at arm.com>
> Cc: Alexander Graf <agraf at suse.de>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
> 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
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0aecc18a6c..879b41c97e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -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
> diff --git a/arch/arm/lib/crt0_aarch64_efi.S b/arch/arm/lib/crt0_aarch64_efi.S
> index 52056469be..9b0e894f8a 100644
> --- a/arch/arm/lib/crt0_aarch64_efi.S
> +++ b/arch/arm/lib/crt0_aarch64_efi.S
> @@ -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 */
> diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S
> index 967c885982..af55bba4ba 100644
> --- a/arch/arm/lib/crt0_arm_efi.S
> +++ b/arch/arm/lib/crt0_arm_efi.S
> @@ -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 */
> diff --git a/include/asm-generic/pe.h b/include/asm-generic/pe.h
> new file mode 100644
> index 0000000000..d1683f238a
> --- /dev/null
> +++ b/include/asm-generic/pe.h
> @@ -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 */
> diff --git a/include/pe.h b/include/pe.h
> index 4ef3e92efa..c3a19cef76 100644
> --- a/include/pe.h
> +++ b/include/pe.h
> @@ -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;
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index 9d2214b481..cac64ba9fe 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -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;
>
More information about the U-Boot
mailing list