Re: [PATCH] arm64/efi: fix variable 'si' set but not used
On Tue, Jul 30, 2019 at 05:23:48PM -0400, Qian Cai wrote:
> GCC throws out this warning on arm64.
>
> drivers/firmware/efi/libstub/arm-stub.c: In function 'efi_entry':
> drivers/firmware/efi/libstub/arm-stub.c:132:22: warning: variable 'si'
> set but not used [-Wunused-but-set-variable]
>
> Fix it by making free_screen_info() a static inline function.
>
> Signed-off-by: Qian Cai
> ---
> arch/arm64/include/asm/efi.h | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index 8e79ce9c3f5c..76a144702586 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -105,7 +105,11 @@ static inline unsigned long
> efi_get_max_initrd_addr(unsigned long dram_base,
> ((protocol##_t *)instance)->f(instance, ##__VA_ARGS__)
>
> #define alloc_screen_info(x...) _info
> -#define free_screen_info(x...)
> +
> +static inline void free_screen_info(efi_system_table_t *sys_table_arg,
> + struct screen_info *si)
> +{
> +}
Is this issue caused by the EFI stub being built with non-default
CFLAGS?
The toplevel Makefile specifies -Wno-unused-but-set-variable, which
would silence this warning.
It's debatable whether calling an empty inline function "uses" the
arguments, so I think your patch only silences the warning by accident:
different GCC versions, or clang, might still warn.
I wonder if we're missing any other options that would make sense for
the EFI stub.
[...]
Cheers
---Dave
Re: "arm64: Silence gcc warnings about arch ABI drift" breaks clang
On Fri, Jun 07, 2019 at 08:40:10AM -0700, Nathan Chancellor wrote: > On Fri, Jun 07, 2019 at 11:26:11AM -0400, Qian Cai wrote: > > On Fri, 2019-06-07 at 16:25 +0100, Will Deacon wrote: > > > On Fri, Jun 07, 2019 at 11:22:45AM -0400, Qian Cai wrote: > > > > The linux-next commit "arm64: Silence gcc warnings about arch ABI > > > > drift" [1] > > > > breaks clang build where it screams that unknown option "-Wno-psabi" and > > > > generates errors below, > > > > > > So that can be easily fixed with cc-option... > > > > > > > [1] > > > > https://lore.kernel.org/linux-arm-kernel/1559817223-32585-1-git-send-ema > > > > il-D > > > > ave.mar...@arm.com/ > > > > > > > > ./drivers/firmware/efi/libstub/arm-stub.stub.o: In function > > > > `install_memreserve_table': > > > > ./linux/drivers/firmware/efi/libstub/arm-stub.c:73: undefined reference > > > > to > > > > `__efistub___stack_chk_guard' > > > > ./linux/drivers/firmware/efi/libstub/arm-stub.c:73: undefined reference > > > > to > > > > `__efistub___stack_chk_guard' > > > > ./linux/drivers/firmware/efi/libstub/arm-stub.c:93: undefined reference > > > > to > > > > `__efistub___stack_chk_guard' > > > > ./linux/drivers/firmware/efi/libstub/arm-stub.c:93: undefined reference > > > > to > > > > `__efistub___stack_chk_guard' > > > > ./linux/drivers/firmware/efi/libstub/arm-stub.c:94: undefined reference > > > > to > > > > `__efistub___stack_chk_fail > > > > > > ... but this looks unrelated. Are you saying you don't see these errors if > > > you revert Dave's patch? > > > > Yes. > > I suspect the reason for this is -Wunknown-warning-option causes > cc-option to fail. I see some disabled warnings like > -Waddress-of-packed-member and -Wunused-const-variable when -Wno-psabi > is unconditionally added. > > I'll do some further triage but I think the obvious fix as Will > suggested is to use cc-disable-warning. I'll send a patch. Thanks for that. Cheers ---Dave
Re: [PATCH 16/16] Add config EFI_STUB for ARM to Kconfig
On Fri, Aug 09, 2013 at 04:26:17PM -0700, Roy Franz wrote: Signed-off-by: Roy Franz roy.fr...@linaro.org --- arch/arm/Kconfig | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 43594d5..8607d03 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1805,6 +1805,17 @@ config UACCESS_WITH_MEMCPY However, if the CPU data cache is using a write-allocate mode, this option is unlikely to provide any performance gain. +config EFI_STUB + bool EFI stub support + depends on EFI !CPU_BIG_ENDIAN, in case you didn't get around to that. Sooner or later, someone may try to fix that, so there should be a comment somewhere explaining what is broken for BE. Either in efi-stub.c or in the commit message accompanying this patch, I guess. Cheers ---Dave + ---help--- + This kernel feature allows a zImage to be loaded directly + by EFI firmware without the use of a bootloader. A PE/COFF + header is added to the zImage in a way that makes the binary + both a Linux zImage and an PE/COFF executable that can be + executed directly by EFI firmware. + See Documentation/efi-stub.txt for more information. + config SECCOMP bool prompt Enable seccomp to safely compute untrusted bytecode -- 1.7.10.4 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/16] Add EFI stub for ARM
On Fri, Aug 09, 2013 at 04:26:16PM -0700, Roy Franz wrote: This patch adds EFI stub support for the ARM Linux kernel. The EFI stub operations similarly to the x86 stub: it is a shim between the EFI firmware and the normal zImage entry point, and sets up the environment that the zImage is expecting. This includes loading the initrd (optionaly) and device tree from the system partition based on the kernel command line. The stub updates the device tree as necessary, including adding reserved memory regions and adding entries for EFI runtime services. The PE/COFF MZ header at offset 0 results in the first instruction being an add that corrupts r5, which is not used by the zImage interface. Thanks for the update -- a few more comments, nothing major. Signed-off-by: Roy Franz roy.fr...@linaro.org --- arch/arm/boot/compressed/Makefile | 15 +- arch/arm/boot/compressed/efi-header.S | 111 arch/arm/boot/compressed/efi-stub.c | 448 + arch/arm/boot/compressed/efi-stub.h |5 + arch/arm/boot/compressed/head.S | 90 ++- 5 files changed, 661 insertions(+), 8 deletions(-) create mode 100644 arch/arm/boot/compressed/efi-header.S create mode 100644 arch/arm/boot/compressed/efi-stub.c create mode 100644 arch/arm/boot/compressed/efi-stub.h diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index 7ac1610..5fad8bd 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -103,11 +103,22 @@ libfdt_objs := $(addsuffix .o, $(basename $(libfdt))) $(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/% $(call cmd,shipped) -$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \ +$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o efi-stub.o): \ $(addprefix $(obj)/,$(libfdt_hdrs)) ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y) -OBJS += $(libfdt_objs) atags_to_fdt.o +OBJS += atags_to_fdt.o +USE_LIBFDT = y +endif + +ifeq ($(CONFIG_EFI_STUB),y) +CFLAGS_efi-stub.o += -DTEXT_OFFSET=$(TEXT_OFFSET) +OBJS += efi-stub.o +USE_LIBFDT = y +endif + +ifeq ($(USE_LIBFDT),y) +OBJS += $(libfdt_objs) endif targets := vmlinux vmlinux.lds \ diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S new file mode 100644 index 000..6965e0f --- /dev/null +++ b/arch/arm/boot/compressed/efi-header.S @@ -0,0 +1,111 @@ +@ Copyright (C) 2013 Linaro Ltd; roy.fr...@linaro.org +@ +@ This file contains the PE/COFF header that is part of the +@ EFI stub. +@ + + .org0x3c + @ + @ The PE header can be anywhere in the file, but for + @ simplicity we keep it together with the MSDOS header + @ The offset to the PE/COFF header needs to be at offset + @ 0x3C in the MSDOS header. + @ The only 2 fields of the MSDOS header that are used are this + @ PE/COFF offset, and the MZ bytes at offset 0x0. + @ + .long pe_header @ Offset to the PE header. + + .align 3 +pe_header: + .ascii PE + .short 0 + +coff_header: + .short 0x01c2 @ ARM or Thumb + .short 2 @ nr_sections + .long 0 @ TimeDateStamp + .long 0 @ PointerToSymbolTable + .long 1 @ NumberOfSymbols + .short section_table - optional_header @ SizeOfOptionalHeader + .short 0x306 @ Characteristics. + @ IMAGE_FILE_32BIT_MACHINE | + @ IMAGE_FILE_DEBUG_STRIPPED | + @ IMAGE_FILE_EXECUTABLE_IMAGE | + @ IMAGE_FILE_LINE_NUMS_STRIPPED + +optional_header: + .short 0x10b @ PE32 format + .byte 0x02@ MajorLinkerVersion + .byte 0x14@ MinorLinkerVersion + + .long _edata - efi_stub_entry @ SizeOfCode + + .long 0 @ SizeOfInitializedData + .long 0 @ SizeOfUninitializedData + + .long efi_stub_entry @ AddressOfEntryPoint + .long efi_stub_entry @ BaseOfCode + .long 0 @ data + +extra_header_fields: + .long 0 @ ImageBase + .long 0x20@ SectionAlignment + .long 0x20@ FileAlignment + .short 0 @ MajorOperatingSystemVersion + .short 0 @ MinorOperatingSystemVersion + .short 0 @
Re: [PATCH 1/7] EFI stub documentation updates
On Fri, Aug 02, 2013 at 02:29:02PM -0700, Roy Franz wrote: The ARM kernel also has an EFI stub which works largely the same way as the x86 stub, so move the documentation out of x86 directory and update to reflect that it is generic, and add ARM specific text. Signed-off-by: Roy Franz roy.fr...@linaro.org --- Documentation/efi-stub.txt | 78 Documentation/x86/efi-stub.txt | 65 - arch/x86/Kconfig |2 +- 3 files changed, 79 insertions(+), 66 deletions(-) create mode 100644 Documentation/efi-stub.txt delete mode 100644 Documentation/x86/efi-stub.txt diff --git a/Documentation/efi-stub.txt b/Documentation/efi-stub.txt new file mode 100644 index 000..7837df1 --- /dev/null +++ b/Documentation/efi-stub.txt @@ -0,0 +1,78 @@ + The EFI Boot Stub + --- + +On the x86 and ARM platforms, a bzImage can masquerade as a PE/COFF image, +thereby convincing EFI firmware loaders to load it as an EFI +executable. The code that modifies the bzImage header, along with the Minor nit, I don't think there is such a thing as bzImage for ARM. Cheers ---Dave +EFI-specific entry point that the firmware loader jumps to are +collectively known as the EFI boot stub, and live in +arch/x86/boot/header.S and arch/x86/boot/compressed/eboot.c, +respectively. For ARM the EFI stub is implemented in +arch/arm/boot/compressed/efi-header.S and +arch/arm/boot/compressed/efi-stub.c. EFI stub code that is shared +between architectures is in drivers/firmware/efi/efi-stub-helper.c. + +By using the EFI boot stub it's possible to boot a Linux kernel +without the use of a conventional EFI boot loader, such as grub or +elilo. Since the EFI boot stub performs the jobs of a boot loader, in +a certain sense it *IS* the boot loader. + +The EFI boot stub is enabled with the CONFIG_EFI_STUB kernel option. + + + How to install bzImage.efi + +The bzImage located in arch/x86/boot/bzImage must be copied to the EFI +System Partiion (ESP) and renamed with the extension .efi. Without +the extension the EFI firmware loader will refuse to execute it. It's +not possible to execute bzImage.efi from the usual Linux file systems +because EFI firmware doesn't have support for them. For ARM the +arch/arm/boot/zImage should be copied to the system partition, and it +may not need to be renamed. + + + Passing kernel parameters from the EFI shell + +Arguments to the kernel can be passed after bzImage.efi, e.g. + + fs0: bzImage.efi console=ttyS0 root=/dev/sda4 + + + The initrd= option + +Like most boot loaders, the EFI stub allows the user to specify +multiple initrd files using the initrd= option. This is the only EFI +stub-specific command line parameter, everything else is passed to the +kernel when it boots. + +The path to the initrd file must be an absolute path from the +beginning of the ESP, relative path names do not work. Also, the path +is an EFI-style path and directory elements must be separated with +backslashes (\). For example, given the following directory layout, + +fs0: + Kernels\ + bzImage.efi + initrd-large.img + + Ramdisks\ + initrd-small.img + initrd-medium.img + +to boot with the initrd-large.img file if the current working +directory is fs0:\Kernels, the following command must be used, + + fs0:\Kernels bzImage.efi initrd=\Kernels\initrd-large.img + +Notice how bzImage.efi can be specified with a relative path. That's +because the image we're executing is interpreted by the EFI shell, +which understands relative paths, whereas the rest of the command line +is passed to bzImage.efi. + + + The dtb= option + +For the ARM architecture, we also need to be able to provide a device +tree to the kernel. This is done with the dtb= command line option, +and is process in the same manner as the initrd= option that is described +above. diff --git a/Documentation/x86/efi-stub.txt b/Documentation/x86/efi-stub.txt deleted file mode 100644 index 44e6bb6..000 --- a/Documentation/x86/efi-stub.txt +++ /dev/null @@ -1,65 +0,0 @@ - The EFI Boot Stub - --- - -On the x86 platform, a bzImage can masquerade as a PE/COFF image, -thereby convincing EFI firmware loaders to load it as an EFI -executable. The code that modifies the bzImage header, along with the -EFI-specific entry point that the firmware loader jumps to are -collectively known as the EFI boot stub, and live in -arch/x86/boot/header.S and arch/x86/boot/compressed/eboot.c, -respectively. - -By using the EFI boot stub it's possible to boot a Linux kernel -without the use of a conventional EFI boot loader, such as grub or -elilo. Since the EFI boot stub
