2020-04-16 01:12:02

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH 0/5] efi: Remove __efistub_global annotation

This patch series removes the need for annotating global data in the EFI
stub with __efistub_global for ARM32 and X86.

This is done by renaming the .data and .bss sections in the object files
linked into the EFI stub to .data.efistub and .bss.efistub respectively,
and including those sections into the compressed kernel's .data section
using its linker script.

The series is based on efi/next, rebased onto v5.7-rc1, plus two earlier
fixes for x86 EFI that are queued for v5.7.

Patches on top of v5.7-rc1:
In efi/next:

Ard Biesheuvel (2):
efi: clean up config table description arrays
efi: move arch_tables check to caller

Arvind Sankar (19):
efi/gop: Remove redundant current_fb_base
efi/gop: Move check for framebuffer before con_out
efi/gop: Get mode information outside the loop
efi/gop: Factor out locating the gop into a function
efi/gop: Slightly re-arrange logic of find_gop
efi/gop: Move variable declarations into loop block
efi/gop: Use helper macros for populating lfb_base
efi/gop: Use helper macros for find_bits
efi/gop: Remove unreachable code from setup_pixel_info
efi/gop: Add prototypes for query_mode and set_mode
efi/gop: Allow specifying mode number on command line
efi/gop: Allow specifying mode by <xres>x<yres>
efi/gop: Allow specifying depth as well as resolution
efi/gop: Allow automatically choosing the best mode

Additional:
Arvind Sankar (2):
efi/x86: Move efi stub globals from .bss to .data
efi/x86: Always relocate the kernel for EFI handover entry

Arvind Sankar (5):
efi/arm: Remove __efistub_global annotation
efi/libstub: Factor out relocation checking
efi/x86: Remove __efistub_global annotation
efi: Kill __efistub_global
efi/x86: Check for bad relocations

arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
arch/x86/boot/compressed/vmlinux.lds.S | 1 +
drivers/firmware/efi/libstub/Makefile | 43 +++++++++++++------
drivers/firmware/efi/libstub/arm-stub.c | 4 +-
.../firmware/efi/libstub/efi-stub-helper.c | 15 +++----
drivers/firmware/efi/libstub/efistub.h | 6 ---
drivers/firmware/efi/libstub/gop.c | 2 +-
drivers/firmware/efi/libstub/x86-stub.c | 2 +-
8 files changed, 42 insertions(+), 33 deletions(-)

--
2.24.1


2020-04-16 01:12:14

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH 3/5] efi/x86: Remove __efistub_global annotation

Instead of using __efistub_global to force variables into the .data
section, leave them in the .bss but pull the EFI stub's .bss section
into .data in the linker script for the compressed kernel.

Signed-off-by: Arvind Sankar <[email protected]>
---
arch/x86/boot/compressed/vmlinux.lds.S | 1 +
drivers/firmware/efi/libstub/Makefile | 12 ++++++++++--
drivers/firmware/efi/libstub/efistub.h | 4 ----
3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 508cfa6828c5..0dc5c2b9614b 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -52,6 +52,7 @@ SECTIONS
_data = . ;
*(.data)
*(.data.*)
+ *(.bss.efistub)
_edata = . ;
}
. = ALIGN(L1_CACHE_BYTES);
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index e5e76677f2da..0bb2916eb12b 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -73,8 +73,8 @@ CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
# a verification pass to see if any absolute relocations exist in any of the
# object files.
#
-extra-$(CONFIG_EFI_ARMSTUB) := $(lib-y)
-lib-$(CONFIG_EFI_ARMSTUB) := $(patsubst %.o,%.stub.o,$(lib-y))
+extra-y := $(lib-y)
+lib-y := $(patsubst %.o,%.stub.o,$(lib-y))

STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \
--prefix-symbols=__efistub_
@@ -89,6 +89,14 @@ STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
--rename-section .bss=.bss.efistub,load,alloc
STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS

+#
+# For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the
+# .bss section, so the .bss section of the EFI stub needs to be included in the
+# .data section of the compressed kernel to ensure initialization. Rename the
+# .bss section here so it's easy to pick out in the linker script.
+#
+STUBCOPY_FLAGS-$(CONFIG_X86) += --rename-section .bss=.bss.efistub,load,alloc
+
$(obj)/%.stub.o: $(obj)/%.o FORCE
$(call if_changed,stubcopy)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index a92d42ffd9f7..49651e20bb9f 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -25,11 +25,7 @@
#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
#endif

-#if defined(CONFIG_X86)
-#define __efistub_global __section(.data)
-#else
#define __efistub_global
-#endif

extern bool __pure nochunk(void);
extern bool __pure nokaslr(void);
--
2.24.1

2020-04-16 01:12:16

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH 4/5] efi: Kill __efistub_global

Now that both arm and x86 are using the linker script to place the EFI
stub's global variables in the correct section, remove __efistub_global.

Signed-off-by: Arvind Sankar <[email protected]>
---
drivers/firmware/efi/libstub/arm-stub.c | 4 ++--
drivers/firmware/efi/libstub/efi-stub-helper.c | 15 +++++++--------
drivers/firmware/efi/libstub/efistub.h | 2 --
drivers/firmware/efi/libstub/gop.c | 2 +-
drivers/firmware/efi/libstub/x86-stub.c | 2 +-
5 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 99a5cde7c2d8..bf42d6c742a8 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -36,9 +36,9 @@
#endif

static u64 virtmap_base = EFI_RT_VIRTUAL_BASE;
-static bool __efistub_global flat_va_mapping;
+static bool flat_va_mapping;

-static efi_system_table_t *__efistub_global sys_table;
+static efi_system_table_t *sys_table;

__pure efi_system_table_t *efi_system_table(void)
{
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index c6092b6038cf..14e56a64f208 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -12,14 +12,13 @@

#include "efistub.h"

-static bool __efistub_global efi_nochunk;
-static bool __efistub_global efi_nokaslr;
-static bool __efistub_global efi_noinitrd;
-static bool __efistub_global efi_quiet;
-static bool __efistub_global efi_novamap;
-static bool __efistub_global efi_nosoftreserve;
-static bool __efistub_global efi_disable_pci_dma =
- IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);
+static bool efi_nochunk;
+static bool efi_nokaslr;
+static bool efi_noinitrd;
+static bool efi_quiet;
+static bool efi_novamap;
+static bool efi_nosoftreserve;
+static bool efi_disable_pci_dma = IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);

bool __pure nochunk(void)
{
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 49651e20bb9f..f96c56596034 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -25,8 +25,6 @@
#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
#endif

-#define __efistub_global
-
extern bool __pure nochunk(void);
extern bool __pure nokaslr(void);
extern bool __pure noinitrd(void);
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index fa05a0b0adfd..216327d0b034 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -32,7 +32,7 @@ static struct {
u8 depth;
} res;
};
-} cmdline __efistub_global = { .option = EFI_CMDLINE_NONE };
+} cmdline = { .option = EFI_CMDLINE_NONE };

static bool parse_modenum(char *option, char **next)
{
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 7583e908852f..aedac3af4b5c 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -20,7 +20,7 @@
/* Maximum physical address for 64-bit kernel with 4-level paging */
#define MAXMEM_X86_64_4LEVEL (1ull << 46)

-static efi_system_table_t *sys_table __efistub_global;
+static efi_system_table_t *sys_table;
extern const bool efi_is64;
extern u32 image_offset;

--
2.24.1

2020-04-16 01:12:19

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH 5/5] efi/x86: Check for bad relocations

Add relocation checking for x86 as well to catch non-PC-relative
relocations that require runtime processing, since the EFI stub does not
do any runtime relocation processing.

This will catch, for example, data relocations created by static
initializers of pointers.

Signed-off-by: Arvind Sankar <[email protected]>
---
drivers/firmware/efi/libstub/Makefile | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 0bb2916eb12b..2aff59812a54 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -96,6 +96,8 @@ STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
# .bss section here so it's easy to pick out in the linker script.
#
STUBCOPY_FLAGS-$(CONFIG_X86) += --rename-section .bss=.bss.efistub,load,alloc
+STUBCOPY_RELOC-$(CONFIG_X86_32) := 'R_X86_32_(8|16|32)'
+STUBCOPY_RELOC-$(CONFIG_X86_64) := 'R_X86_64_(8|16|32|32S|64)'

$(obj)/%.stub.o: $(obj)/%.o FORCE
$(call if_changed,stubcopy)
@@ -107,16 +109,14 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE
# this time, use objcopy and leave all sections in place.
#

-cmd_stubrelocs_check-y = /bin/true
-
-cmd_stubrelocs_check-$(CONFIG_EFI_ARMSTUB) = \
+cmd_stubrelocs_check = \
$(STRIP) --strip-debug -o $@ $<; \
- if $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y); then \
+ if $(OBJDUMP) -r $@ | grep -E $(STUBCOPY_RELOC-y); then \
echo "$@: absolute symbol references not allowed in the EFI stub" >&2; \
/bin/false; \
fi

quiet_cmd_stubcopy = STUBCPY $@
cmd_stubcopy = \
- $(cmd_stubrelocs_check-y); \
+ $(cmd_stubrelocs_check); \
$(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@
--
2.24.1

2020-04-16 01:14:05

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH 1/5] efi/arm: Remove __efistub_global annotation

Instead of using __efistub_global to force variables into the .data
section, leave them in the .bss but pull the EFI stub's .bss section
into .data in the linker script for the compressed kernel.

Signed-off-by: Arvind Sankar <[email protected]>
---
arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
drivers/firmware/efi/libstub/Makefile | 7 ++++---
drivers/firmware/efi/libstub/efistub.h | 2 +-
3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
index b247f399de71..b6793c7932a9 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.S
+++ b/arch/arm/boot/compressed/vmlinux.lds.S
@@ -78,7 +78,7 @@ SECTIONS
* The EFI stub always executes from RAM, and runs strictly before the
* decompressor, so we can make an exception for its r/w data, and keep it
*/
- *(.data.efistub)
+ *(.data.efistub .bss.efistub)
__pecoff_data_end = .;

/*
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 094eabdecfe6..45ffe0822df1 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -100,8 +100,9 @@ quiet_cmd_stubcopy = STUBCPY $@

#
# 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.
+# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
+# which are preserved explicitly by the decompressor linker script.
#
-STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
+STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
+ --rename-section .bss=.bss.efistub,load,alloc
STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index bd0b86b63936..a92d42ffd9f7 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -25,7 +25,7 @@
#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
#endif

-#if defined(CONFIG_ARM) || defined(CONFIG_X86)
+#if defined(CONFIG_X86)
#define __efistub_global __section(.data)
#else
#define __efistub_global
--
2.24.1

2020-04-16 01:14:10

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH 2/5] efi/libstub: Factor out relocation checking

In preparation for using STUBCOPY for x86 as well, which doesn't require
relocation checking, move the checking code into its own variable so it
can be left out for x86.

Signed-off-by: Arvind Sankar <[email protected]>
---
drivers/firmware/efi/libstub/Makefile | 30 ++++++++++++++++-----------
1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 45ffe0822df1..e5e76677f2da 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -80,6 +80,15 @@ STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \
--prefix-symbols=__efistub_
STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS

+#
+# ARM discards the .data section because it disallows r/w data in the
+# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
+# which are preserved explicitly by the decompressor linker script.
+#
+STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
+ --rename-section .bss=.bss.efistub,load,alloc
+STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
+
$(obj)/%.stub.o: $(obj)/%.o FORCE
$(call if_changed,stubcopy)

@@ -89,20 +98,17 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE
# such relocations. If none are found, regenerate the output object, but
# this time, use objcopy and leave all sections in place.
#
-quiet_cmd_stubcopy = STUBCPY $@
- cmd_stubcopy = \
+
+cmd_stubrelocs_check-y = /bin/true
+
+cmd_stubrelocs_check-$(CONFIG_EFI_ARMSTUB) = \
$(STRIP) --strip-debug -o $@ $<; \
if $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y); then \
echo "$@: absolute symbol references not allowed in the EFI stub" >&2; \
/bin/false; \
- fi; \
- $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@
+ fi

-#
-# ARM discards the .data section because it disallows r/w data in the
-# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
-# which are preserved explicitly by the decompressor linker script.
-#
-STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
- --rename-section .bss=.bss.efistub,load,alloc
-STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
+quiet_cmd_stubcopy = STUBCPY $@
+ cmd_stubcopy = \
+ $(cmd_stubrelocs_check-y); \
+ $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@
--
2.24.1

2020-04-16 07:41:12

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 5/5] efi/x86: Check for bad relocations

On Thu, 16 Apr 2020 at 00:15, Arvind Sankar <[email protected]> wrote:
>
> Add relocation checking for x86 as well to catch non-PC-relative
> relocations that require runtime processing, since the EFI stub does not
> do any runtime relocation processing.
>
> This will catch, for example, data relocations created by static
> initializers of pointers.
>
> Signed-off-by: Arvind Sankar <[email protected]>
> ---
> drivers/firmware/efi/libstub/Makefile | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index 0bb2916eb12b..2aff59812a54 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -96,6 +96,8 @@ STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> # .bss section here so it's easy to pick out in the linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_X86) += --rename-section .bss=.bss.efistub,load,alloc
> +STUBCOPY_RELOC-$(CONFIG_X86_32) := 'R_X86_32_(8|16|32)'

This should be R_386_xxx

> +STUBCOPY_RELOC-$(CONFIG_X86_64) := 'R_X86_64_(8|16|32|32S|64)'
>

... and in general, I think we only need the native pointer sized ones, so

R_386_32
R_X86_64_64

> $(obj)/%.stub.o: $(obj)/%.o FORCE
> $(call if_changed,stubcopy)
> @@ -107,16 +109,14 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE
> # this time, use objcopy and leave all sections in place.
> #
>
> -cmd_stubrelocs_check-y = /bin/true
> -
> -cmd_stubrelocs_check-$(CONFIG_EFI_ARMSTUB) = \
> +cmd_stubrelocs_check = \
> $(STRIP) --strip-debug -o $@ $<; \
> - if $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y); then \
> + if $(OBJDUMP) -r $@ | grep -E $(STUBCOPY_RELOC-y); then \

... which means we don't need to -E either

> echo "$@: absolute symbol references not allowed in the EFI stub" >&2; \
> /bin/false; \
> fi
>
> quiet_cmd_stubcopy = STUBCPY $@
> cmd_stubcopy = \
> - $(cmd_stubrelocs_check-y); \
> + $(cmd_stubrelocs_check); \
> $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@
> --
> 2.24.1
>

Could we fold this into the previous x86 patch, and drop the one that
splits off the relocation check from stubcpy?

2020-04-16 07:53:14

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 3/5] efi/x86: Remove __efistub_global annotation

On Thu, 16 Apr 2020 at 00:15, Arvind Sankar <[email protected]> wrote:
>
> Instead of using __efistub_global to force variables into the .data
> section, leave them in the .bss but pull the EFI stub's .bss section
> into .data in the linker script for the compressed kernel.
>
> Signed-off-by: Arvind Sankar <[email protected]>

With the R_386_32/R_X86_64_64 check folded in:

Reviewed-by: Ard Biesheuvel <[email protected]>

> ---
> arch/x86/boot/compressed/vmlinux.lds.S | 1 +
> drivers/firmware/efi/libstub/Makefile | 12 ++++++++++--
> drivers/firmware/efi/libstub/efistub.h | 4 ----
> 3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> index 508cfa6828c5..0dc5c2b9614b 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -52,6 +52,7 @@ SECTIONS
> _data = . ;
> *(.data)
> *(.data.*)
> + *(.bss.efistub)
> _edata = . ;
> }
> . = ALIGN(L1_CACHE_BYTES);
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index e5e76677f2da..0bb2916eb12b 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -73,8 +73,8 @@ CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
> # a verification pass to see if any absolute relocations exist in any of the
> # object files.
> #
> -extra-$(CONFIG_EFI_ARMSTUB) := $(lib-y)
> -lib-$(CONFIG_EFI_ARMSTUB) := $(patsubst %.o,%.stub.o,$(lib-y))
> +extra-y := $(lib-y)
> +lib-y := $(patsubst %.o,%.stub.o,$(lib-y))
>
> STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \
> --prefix-symbols=__efistub_
> @@ -89,6 +89,14 @@ STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
> --rename-section .bss=.bss.efistub,load,alloc
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
>
> +#
> +# For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the
> +# .bss section, so the .bss section of the EFI stub needs to be included in the
> +# .data section of the compressed kernel to ensure initialization. Rename the
> +# .bss section here so it's easy to pick out in the linker script.
> +#
> +STUBCOPY_FLAGS-$(CONFIG_X86) += --rename-section .bss=.bss.efistub,load,alloc
> +
> $(obj)/%.stub.o: $(obj)/%.o FORCE
> $(call if_changed,stubcopy)
>
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index a92d42ffd9f7..49651e20bb9f 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -25,11 +25,7 @@
> #define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
> #endif
>
> -#if defined(CONFIG_X86)
> -#define __efistub_global __section(.data)
> -#else
> #define __efistub_global
> -#endif
>
> extern bool __pure nochunk(void);
> extern bool __pure nokaslr(void);
> --
> 2.24.1
>

2020-04-16 07:54:04

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 4/5] efi: Kill __efistub_global

On Thu, 16 Apr 2020 at 00:15, Arvind Sankar <[email protected]> wrote:
>
> Now that both arm and x86 are using the linker script to place the EFI
> stub's global variables in the correct section, remove __efistub_global.
>
> Signed-off-by: Arvind Sankar <[email protected]>

Reviewed-by: Ard Biesheuvel <[email protected]>

> ---
> drivers/firmware/efi/libstub/arm-stub.c | 4 ++--
> drivers/firmware/efi/libstub/efi-stub-helper.c | 15 +++++++--------
> drivers/firmware/efi/libstub/efistub.h | 2 --
> drivers/firmware/efi/libstub/gop.c | 2 +-
> drivers/firmware/efi/libstub/x86-stub.c | 2 +-
> 5 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 99a5cde7c2d8..bf42d6c742a8 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -36,9 +36,9 @@
> #endif
>
> static u64 virtmap_base = EFI_RT_VIRTUAL_BASE;
> -static bool __efistub_global flat_va_mapping;
> +static bool flat_va_mapping;
>
> -static efi_system_table_t *__efistub_global sys_table;
> +static efi_system_table_t *sys_table;
>
> __pure efi_system_table_t *efi_system_table(void)
> {
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index c6092b6038cf..14e56a64f208 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -12,14 +12,13 @@
>
> #include "efistub.h"
>
> -static bool __efistub_global efi_nochunk;
> -static bool __efistub_global efi_nokaslr;
> -static bool __efistub_global efi_noinitrd;
> -static bool __efistub_global efi_quiet;
> -static bool __efistub_global efi_novamap;
> -static bool __efistub_global efi_nosoftreserve;
> -static bool __efistub_global efi_disable_pci_dma =
> - IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);
> +static bool efi_nochunk;
> +static bool efi_nokaslr;
> +static bool efi_noinitrd;
> +static bool efi_quiet;
> +static bool efi_novamap;
> +static bool efi_nosoftreserve;
> +static bool efi_disable_pci_dma = IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);
>
> bool __pure nochunk(void)
> {
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 49651e20bb9f..f96c56596034 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -25,8 +25,6 @@
> #define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
> #endif
>
> -#define __efistub_global
> -
> extern bool __pure nochunk(void);
> extern bool __pure nokaslr(void);
> extern bool __pure noinitrd(void);
> diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
> index fa05a0b0adfd..216327d0b034 100644
> --- a/drivers/firmware/efi/libstub/gop.c
> +++ b/drivers/firmware/efi/libstub/gop.c
> @@ -32,7 +32,7 @@ static struct {
> u8 depth;
> } res;
> };
> -} cmdline __efistub_global = { .option = EFI_CMDLINE_NONE };
> +} cmdline = { .option = EFI_CMDLINE_NONE };
>
> static bool parse_modenum(char *option, char **next)
> {
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 7583e908852f..aedac3af4b5c 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -20,7 +20,7 @@
> /* Maximum physical address for 64-bit kernel with 4-level paging */
> #define MAXMEM_X86_64_4LEVEL (1ull << 46)
>
> -static efi_system_table_t *sys_table __efistub_global;
> +static efi_system_table_t *sys_table;
> extern const bool efi_is64;
> extern u32 image_offset;
>
> --
> 2.24.1
>

2020-04-16 07:54:24

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 1/5] efi/arm: Remove __efistub_global annotation

On Thu, 16 Apr 2020 at 00:15, Arvind Sankar <[email protected]> wrote:
>
> Instead of using __efistub_global to force variables into the .data
> section, leave them in the .bss but pull the EFI stub's .bss section
> into .data in the linker script for the compressed kernel.
>
> Signed-off-by: Arvind Sankar <[email protected]>

Reviewed-by: Ard Biesheuvel <[email protected]>

> ---
> arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
> drivers/firmware/efi/libstub/Makefile | 7 ++++---
> drivers/firmware/efi/libstub/efistub.h | 2 +-
> 3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> index b247f399de71..b6793c7932a9 100644
> --- a/arch/arm/boot/compressed/vmlinux.lds.S
> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> @@ -78,7 +78,7 @@ SECTIONS
> * The EFI stub always executes from RAM, and runs strictly before the
> * decompressor, so we can make an exception for its r/w data, and keep it
> */
> - *(.data.efistub)
> + *(.data.efistub .bss.efistub)
> __pecoff_data_end = .;
>
> /*
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index 094eabdecfe6..45ffe0822df1 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -100,8 +100,9 @@ quiet_cmd_stubcopy = STUBCPY $@
>
> #
> # 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.
> +# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
> +# which are preserved explicitly by the decompressor linker script.
> #
> -STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
> +STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
> + --rename-section .bss=.bss.efistub,load,alloc
> STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index bd0b86b63936..a92d42ffd9f7 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -25,7 +25,7 @@
> #define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
> #endif
>
> -#if defined(CONFIG_ARM) || defined(CONFIG_X86)
> +#if defined(CONFIG_X86)
> #define __efistub_global __section(.data)
> #else
> #define __efistub_global
> --
> 2.24.1
>

2020-04-16 14:53:24

by Arvind Sankar

[permalink] [raw]
Subject: Re: [PATCH 5/5] efi/x86: Check for bad relocations

On Thu, Apr 16, 2020 at 09:38:36AM +0200, Ard Biesheuvel wrote:
> On Thu, 16 Apr 2020 at 00:15, Arvind Sankar <[email protected]> wrote:
> >
> > Add relocation checking for x86 as well to catch non-PC-relative
> > relocations that require runtime processing, since the EFI stub does not
> > do any runtime relocation processing.
> >
> > This will catch, for example, data relocations created by static
> > initializers of pointers.
> >
> > Signed-off-by: Arvind Sankar <[email protected]>
> > ---
> > drivers/firmware/efi/libstub/Makefile | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> > index 0bb2916eb12b..2aff59812a54 100644
> > --- a/drivers/firmware/efi/libstub/Makefile
> > +++ b/drivers/firmware/efi/libstub/Makefile
> > @@ -96,6 +96,8 @@ STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
> > # .bss section here so it's easy to pick out in the linker script.
> > #
> > STUBCOPY_FLAGS-$(CONFIG_X86) += --rename-section .bss=.bss.efistub,load,alloc
> > +STUBCOPY_RELOC-$(CONFIG_X86_32) := 'R_X86_32_(8|16|32)'
>
> This should be R_386_xxx

Oops. I tested 64-bit but not 32-bit. I'll fix.

>
> > +STUBCOPY_RELOC-$(CONFIG_X86_64) := 'R_X86_64_(8|16|32|32S|64)'
> >
>
> ... and in general, I think we only need the native pointer sized ones, so
>
> R_386_32
> R_X86_64_64

Ok.

>
> > $(obj)/%.stub.o: $(obj)/%.o FORCE
> > $(call if_changed,stubcopy)
> > @@ -107,16 +109,14 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE
> > # this time, use objcopy and leave all sections in place.
> > #
> >
> > -cmd_stubrelocs_check-y = /bin/true
> > -
> > -cmd_stubrelocs_check-$(CONFIG_EFI_ARMSTUB) = \
> > +cmd_stubrelocs_check = \
> > $(STRIP) --strip-debug -o $@ $<; \
> > - if $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y); then \
> > + if $(OBJDUMP) -r $@ | grep -E $(STUBCOPY_RELOC-y); then \
>
> ... which means we don't need to -E either
>
> > echo "$@: absolute symbol references not allowed in the EFI stub" >&2; \
> > /bin/false; \
> > fi
> >
> > quiet_cmd_stubcopy = STUBCPY $@
> > cmd_stubcopy = \
> > - $(cmd_stubrelocs_check-y); \
> > + $(cmd_stubrelocs_check); \
> > $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@
> > --
> > 2.24.1
> >
>
> Could we fold this into the previous x86 patch, and drop the one that
> splits off the relocation check from stubcpy?

Will do.

2020-04-16 20:34:57

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH v2 0/3] efi: Remove __efistub_global annotation

This patch series removes the need for annotating global data in the EFI
stub with __efistub_global for ARM32 and X86.

This is done by renaming the .data and .bss sections in the object files
linked into the EFI stub to .data.efistub and .bss.efistub respectively,
and including those sections into the compressed kernel's .data section
using its linker script.

Changes from v1:
- drop patch 2 and squash patches 3 and 5 for x86
- fix R_X86 -> R_386
- only check native relocation size (32-bit for R386 and 64-bit for
RX86_64)

The series is based on efi/next, rebased onto v5.7-rc1, plus two earlier
fixes for x86 EFI that are queued for v5.7.

Patches on top of v5.7-rc1:
In efi/next:

Ard Biesheuvel (2):
efi: clean up config table description arrays
efi: move arch_tables check to caller

Arvind Sankar (19):
efi/gop: Remove redundant current_fb_base
efi/gop: Move check for framebuffer before con_out
efi/gop: Get mode information outside the loop
efi/gop: Factor out locating the gop into a function
efi/gop: Slightly re-arrange logic of find_gop
efi/gop: Move variable declarations into loop block
efi/gop: Use helper macros for populating lfb_base
efi/gop: Use helper macros for find_bits
efi/gop: Remove unreachable code from setup_pixel_info
efi/gop: Add prototypes for query_mode and set_mode
efi/gop: Allow specifying mode number on command line
efi/gop: Allow specifying mode by <xres>x<yres>
efi/gop: Allow specifying depth as well as resolution
efi/gop: Allow automatically choosing the best mode

Additional:
Arvind Sankar (2):
efi/x86: Move efi stub globals from .bss to .data
efi/x86: Always relocate the kernel for EFI handover entry

Arvind Sankar (3):
efi/arm: Remove __efistub_global annotation
efi/x86: Remove __efistub_global and add relocation check
efi: Kill __efistub_global

arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
arch/x86/boot/compressed/vmlinux.lds.S | 1 +
drivers/firmware/efi/libstub/Makefile | 31 +++++++++++++------
drivers/firmware/efi/libstub/arm-stub.c | 4 +--
.../firmware/efi/libstub/efi-stub-helper.c | 15 +++++----
drivers/firmware/efi/libstub/efistub.h | 6 ----
drivers/firmware/efi/libstub/gop.c | 2 +-
drivers/firmware/efi/libstub/x86-stub.c | 2 +-
8 files changed, 34 insertions(+), 29 deletions(-)

--
2.25.3

2020-04-16 20:34:58

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH v2 2/3] efi/x86: Remove __efistub_global and add relocation check

Instead of using __efistub_global to force variables into the .data
section, leave them in the .bss but pull the EFI stub's .bss section
into .data in the linker script for the compressed kernel.

Add relocation checking for x86 as well to catch non-PC-relative
relocations that require runtime processing, since the EFI stub does not
do any runtime relocation processing.

This will catch, for example, data relocations created by static
initializers of pointers.

Signed-off-by: Arvind Sankar <[email protected]>
---
arch/x86/boot/compressed/vmlinux.lds.S | 1 +
drivers/firmware/efi/libstub/Makefile | 32 +++++++++++++++++---------
drivers/firmware/efi/libstub/efistub.h | 4 ----
3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 508cfa6828c5..0dc5c2b9614b 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -52,6 +52,7 @@ SECTIONS
_data = . ;
*(.data)
*(.data.*)
+ *(.bss.efistub)
_edata = . ;
}
. = ALIGN(L1_CACHE_BYTES);
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 45ffe0822df1..069d117d5451 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -73,13 +73,32 @@ CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
# a verification pass to see if any absolute relocations exist in any of the
# object files.
#
-extra-$(CONFIG_EFI_ARMSTUB) := $(lib-y)
-lib-$(CONFIG_EFI_ARMSTUB) := $(patsubst %.o,%.stub.o,$(lib-y))
+extra-y := $(lib-y)
+lib-y := $(patsubst %.o,%.stub.o,$(lib-y))

STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \
--prefix-symbols=__efistub_
STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS

+#
+# ARM discards the .data section because it disallows r/w data in the
+# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
+# which are preserved explicitly by the decompressor linker script.
+#
+STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
+ --rename-section .bss=.bss.efistub,load,alloc
+STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
+
+#
+# For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the
+# .bss section, so the .bss section of the EFI stub needs to be included in the
+# .data section of the compressed kernel to ensure initialization. Rename the
+# .bss section here so it's easy to pick out in the linker script.
+#
+STUBCOPY_FLAGS-$(CONFIG_X86) += --rename-section .bss=.bss.efistub,load,alloc
+STUBCOPY_RELOC-$(CONFIG_X86_32) := R_386_32
+STUBCOPY_RELOC-$(CONFIG_X86_64) := R_X86_64_64
+
$(obj)/%.stub.o: $(obj)/%.o FORCE
$(call if_changed,stubcopy)

@@ -97,12 +116,3 @@ quiet_cmd_stubcopy = STUBCPY $@
/bin/false; \
fi; \
$(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@
-
-#
-# ARM discards the .data section because it disallows r/w data in the
-# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
-# which are preserved explicitly by the decompressor linker script.
-#
-STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
- --rename-section .bss=.bss.efistub,load,alloc
-STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index a92d42ffd9f7..49651e20bb9f 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -25,11 +25,7 @@
#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
#endif

-#if defined(CONFIG_X86)
-#define __efistub_global __section(.data)
-#else
#define __efistub_global
-#endif

extern bool __pure nochunk(void);
extern bool __pure nokaslr(void);
--
2.25.3

2020-04-16 20:34:58

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH v2 3/3] efi: Kill __efistub_global

Now that both arm and x86 are using the linker script to place the EFI
stub's global variables in the correct section, remove __efistub_global.

Signed-off-by: Arvind Sankar <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/libstub/arm-stub.c | 4 ++--
drivers/firmware/efi/libstub/efi-stub-helper.c | 15 +++++++--------
drivers/firmware/efi/libstub/efistub.h | 2 --
drivers/firmware/efi/libstub/gop.c | 2 +-
drivers/firmware/efi/libstub/x86-stub.c | 2 +-
5 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 99a5cde7c2d8..bf42d6c742a8 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -36,9 +36,9 @@
#endif

static u64 virtmap_base = EFI_RT_VIRTUAL_BASE;
-static bool __efistub_global flat_va_mapping;
+static bool flat_va_mapping;

-static efi_system_table_t *__efistub_global sys_table;
+static efi_system_table_t *sys_table;

__pure efi_system_table_t *efi_system_table(void)
{
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index c6092b6038cf..14e56a64f208 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -12,14 +12,13 @@

#include "efistub.h"

-static bool __efistub_global efi_nochunk;
-static bool __efistub_global efi_nokaslr;
-static bool __efistub_global efi_noinitrd;
-static bool __efistub_global efi_quiet;
-static bool __efistub_global efi_novamap;
-static bool __efistub_global efi_nosoftreserve;
-static bool __efistub_global efi_disable_pci_dma =
- IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);
+static bool efi_nochunk;
+static bool efi_nokaslr;
+static bool efi_noinitrd;
+static bool efi_quiet;
+static bool efi_novamap;
+static bool efi_nosoftreserve;
+static bool efi_disable_pci_dma = IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);

bool __pure nochunk(void)
{
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 49651e20bb9f..f96c56596034 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -25,8 +25,6 @@
#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
#endif

-#define __efistub_global
-
extern bool __pure nochunk(void);
extern bool __pure nokaslr(void);
extern bool __pure noinitrd(void);
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index fa05a0b0adfd..216327d0b034 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -32,7 +32,7 @@ static struct {
u8 depth;
} res;
};
-} cmdline __efistub_global = { .option = EFI_CMDLINE_NONE };
+} cmdline = { .option = EFI_CMDLINE_NONE };

static bool parse_modenum(char *option, char **next)
{
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 7583e908852f..aedac3af4b5c 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -20,7 +20,7 @@
/* Maximum physical address for 64-bit kernel with 4-level paging */
#define MAXMEM_X86_64_4LEVEL (1ull << 46)

-static efi_system_table_t *sys_table __efistub_global;
+static efi_system_table_t *sys_table;
extern const bool efi_is64;
extern u32 image_offset;

--
2.25.3

2020-04-16 20:35:06

by Arvind Sankar

[permalink] [raw]
Subject: [PATCH v2 1/3] efi/arm: Remove __efistub_global annotation

Instead of using __efistub_global to force variables into the .data
section, leave them in the .bss but pull the EFI stub's .bss section
into .data in the linker script for the compressed kernel.

Signed-off-by: Arvind Sankar <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
---
arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
drivers/firmware/efi/libstub/Makefile | 7 ++++---
drivers/firmware/efi/libstub/efistub.h | 2 +-
3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
index b247f399de71..b6793c7932a9 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.S
+++ b/arch/arm/boot/compressed/vmlinux.lds.S
@@ -78,7 +78,7 @@ SECTIONS
* The EFI stub always executes from RAM, and runs strictly before the
* decompressor, so we can make an exception for its r/w data, and keep it
*/
- *(.data.efistub)
+ *(.data.efistub .bss.efistub)
__pecoff_data_end = .;

/*
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 094eabdecfe6..45ffe0822df1 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -100,8 +100,9 @@ quiet_cmd_stubcopy = STUBCPY $@

#
# 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.
+# decompressor. So move our .data to .data.efistub and .bss to .bss.efistub,
+# which are preserved explicitly by the decompressor linker script.
#
-STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub
+STUBCOPY_FLAGS-$(CONFIG_ARM) += --rename-section .data=.data.efistub \
+ --rename-section .bss=.bss.efistub,load,alloc
STUBCOPY_RELOC-$(CONFIG_ARM) := R_ARM_ABS
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index bd0b86b63936..a92d42ffd9f7 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -25,7 +25,7 @@
#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
#endif

-#if defined(CONFIG_ARM) || defined(CONFIG_X86)
+#if defined(CONFIG_X86)
#define __efistub_global __section(.data)
#else
#define __efistub_global
--
2.25.3

2020-04-16 20:38:58

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] efi: Remove __efistub_global annotation

On Thu, 16 Apr 2020 at 17:12, Arvind Sankar <[email protected]> wrote:
>
> This patch series removes the need for annotating global data in the EFI
> stub with __efistub_global for ARM32 and X86.
>
> This is done by renaming the .data and .bss sections in the object files
> linked into the EFI stub to .data.efistub and .bss.efistub respectively,
> and including those sections into the compressed kernel's .data section
> using its linker script.
>
> Changes from v1:
> - drop patch 2 and squash patches 3 and 5 for x86
> - fix R_X86 -> R_386
> - only check native relocation size (32-bit for R386 and 64-bit for
> RX86_64)
>

Thanks Arvind. I have queued these up now.

Atish, I have queued up the first 2 patches of your RISC-V EFI stbu
series as well. Please base your next version on

https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next

Thanks,
Ard.