Patchwork [1/4] EFI stub: remove -fdata-sections

login
register
mail settings
Submitter Russell King
Date Nov. 6, 2018, 1:40 p.m.
Message ID <E1gK1aT-0000oL-Nn@rmk-PC.armlinux.org.uk>
Download mbox | patch
Permalink /patch/650567/
State New
Headers show

Comments

Russell King - Nov. 6, 2018, 1:40 p.m.
Remove -fdata-sections from the EFI stub build as this causes problems
for the ARM decompressor code.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/firmware/efi/libstub/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Ard Biesheuvel - Nov. 6, 2018, 2:11 p.m.
On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Remove -fdata-sections from the EFI stub build as this causes problems
> for the ARM decompressor code.
>
Just out of curiosity: what kind of problems?

In any case: it doesn't make sense to obsess about .data size
optimizations here, so

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/firmware/efi/libstub/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index c51627660dbb..98133d0577ab 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -15,7 +15,7 @@ cflags-$(CONFIG_X86)          += -m$(BITS) -D__KERNEL__ -O2 \
>  # disable the stackleak plugin
>  cflags-$(CONFIG_ARM64)         := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
>                                    $(DISABLE_STACKLEAK_PLUGIN)
> -cflags-$(CONFIG_ARM)           := $(subst -pg,,$(KBUILD_CFLAGS)) \
> +cflags-$(CONFIG_ARM)           := $(filter-out -pg -fdata-sections,$(KBUILD_CFLAGS)) \
>                                    -fno-builtin -fpic \
>                                    $(call cc-option,-mno-single-pic-base)
>
> --
> 2.7.4
>
Russell King - ARM Linux - Nov. 6, 2018, 2:21 p.m.
On Tue, Nov 06, 2018 at 03:11:18PM +0100, Ard Biesheuvel wrote:
> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > Remove -fdata-sections from the EFI stub build as this causes problems
> > for the ARM decompressor code.
> >
> Just out of curiosity: what kind of problems?

I'm afraid I don't remember - these patches are March 2018.  I suspect
it was a result of a randconfig build failing to link, as I don't have
any 32-bit ARM configs that have the EFI stub enabled.

In any case, this will break:

#
# ARM discards the .data section because it disallows r/w data in the
# decompressor. So move our .data to .data.efistub, which is preserved
# explicitly by the decompressor linker script.
#
STUBCOPY_FLAGS-$(CONFIG_ARM)    += --rename-section .data=.data.efistub

because with -fdata-sections enabled, there is no longer a single .data
section, but multiple .data.* sections.  Hence, the rename no longer
works, and all the EFI stub data ends up being discarded by the
decompressor link stage.
Ard Biesheuvel - Nov. 6, 2018, 2:22 p.m.
On 6 November 2018 at 15:21, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Nov 06, 2018 at 03:11:18PM +0100, Ard Biesheuvel wrote:
>> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> > Remove -fdata-sections from the EFI stub build as this causes problems
>> > for the ARM decompressor code.
>> >
>> Just out of curiosity: what kind of problems?
>
> I'm afraid I don't remember - these patches are March 2018.  I suspect
> it was a result of a randconfig build failing to link, as I don't have
> any 32-bit ARM configs that have the EFI stub enabled.
>
> In any case, this will break:
>
> #
> # ARM discards the .data section because it disallows r/w data in the
> # decompressor. So move our .data to .data.efistub, which is preserved
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM)    += --rename-section .data=.data.efistub
>
> because with -fdata-sections enabled, there is no longer a single .data
> section, but multiple .data.* sections.  Hence, the rename no longer
> works, and all the EFI stub data ends up being discarded by the
> decompressor link stage.
>

Ah, of course, that must be it.

Patch

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index c51627660dbb..98133d0577ab 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -15,7 +15,7 @@  cflags-$(CONFIG_X86)		+= -m$(BITS) -D__KERNEL__ -O2 \
 # disable the stackleak plugin
 cflags-$(CONFIG_ARM64)		:= $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
 				   $(DISABLE_STACKLEAK_PLUGIN)
-cflags-$(CONFIG_ARM)		:= $(subst -pg,,$(KBUILD_CFLAGS)) \
+cflags-$(CONFIG_ARM)		:= $(filter-out -pg -fdata-sections,$(KBUILD_CFLAGS)) \
 				   -fno-builtin -fpic \
 				   $(call cc-option,-mno-single-pic-base)