efistub: convert into static library

This moves definitions depended upon both by code under arch/x86/boot and under drivers/firmware/efi to <asm/efi.h>. This is in preparation of turning the stub code under drivers/firmware/efi into a static library.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/x86/boot/compressed/eboot.c |  5 +----
 arch/x86/boot/compressed/eboot.h | 16 ----------------
 arch/x86/include/asm/efi.h       | 25 +++++++++++++++++++++++++
 3 files changed, 26 insertions(+), 20 deletions(-)

arch/x86/boot/compressed/eboot.c +1 -4

index 0331d765c2bb..2fd5e2643623 100644
@@ -19,10 +19,7 @@
 
 static efi_system_table_t *sys_table;
 
-static struct efi_config *efi_early;
-
-#define efi_call_early(f, ...)						\
-	efi_early->call(efi_early->f, __VA_ARGS__);
+struct efi_config *efi_early;
 
 #define BOOT_SERVICES(bits)						\
 static void setup_boot_services##bits(struct efi_config *c)		\

arch/x86/boot/compressed/eboot.h +0 -16

index c88c31ecad12..d487e727f1ec 100644
@@ -103,20 +103,4 @@ struct efi_uga_draw_protocol {
 	void *blt;
 };
 
-struct efi_config {
-	u64 image_handle;
-	u64 table;
-	u64 allocate_pool;
-	u64 allocate_pages;
-	u64 get_memory_map;
-	u64 free_pool;
-	u64 free_pages;
-	u64 locate_handle;
-	u64 handle_protocol;
-	u64 exit_boot_services;
-	u64 text_output;
-	efi_status_t (*call)(unsigned long, ...);
-	bool is64;
-} __packed;
-
 #endif /* BOOT_COMPRESSED_EBOOT_H */

arch/x86/include/asm/efi.h +14 -0

index 1eb5f6433ad8..55059a50a01f 100644
@@ -156,6 +156,31 @@ static inline efi_status_t efi_thunk_set_virtual_address_map(
 	return EFI_SUCCESS;
 }
 #endif /* CONFIG_EFI_MIXED */
+
+
+/* arch specific definitions used by the stub code */
+
+struct efi_config {
+	u64 image_handle;
+	u64 table;
+	u64 allocate_pool;
+	u64 allocate_pages;
+	u64 get_memory_map;
+	u64 free_pool;
+	u64 free_pages;
+	u64 locate_handle;
+	u64 handle_protocol;

arch/x86/include/asm/efi.h +1 -0

+
 #else
 /*
  * IF EFI is not configured, have the EFI calls return -ENOSYS.

Quoted 78 lines · Ard Biesheuvel

This moves definitions depended upon both by code under arch/x86/boot and under drivers/firmware/efi to <asm/efi.h>. This is in preparation of turning the stub code under drivers/firmware/efi into a static library.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/x86/boot/compressed/eboot.c |  5 +----
 arch/x86/boot/compressed/eboot.h | 16 ----------------
 arch/x86/include/asm/efi.h       | 25 +++++++++++++++++++++++++
 3 files changed, 26 insertions(+), 20 deletions(-)

arch/x86/boot/compressed/eboot.c +1 -4

index 0331d765c2bb..2fd5e2643623 100644
@@ -19,10 +19,7 @@
 
 static efi_system_table_t *sys_table;
 
-static struct efi_config *efi_early;
-
-#define efi_call_early(f, ...)                                               \
-     efi_early->call(efi_early->f, __VA_ARGS__);
+struct efi_config *efi_early;
 
 #define BOOT_SERVICES(bits)                                          \
 static void setup_boot_services##bits(struct efi_config *c)          \

arch/x86/boot/compressed/eboot.h +0 -16

index c88c31ecad12..d487e727f1ec 100644
@@ -103,20 +103,4 @@ struct efi_uga_draw_protocol {
      void *blt;
 };
 
-struct efi_config {
-     u64 image_handle;
-     u64 table;
-     u64 allocate_pool;
-     u64 allocate_pages;
-     u64 get_memory_map;
-     u64 free_pool;
-     u64 free_pages;
-     u64 locate_handle;
-     u64 handle_protocol;
-     u64 exit_boot_services;
-     u64 text_output;
-     efi_status_t (*call)(unsigned long, ...);
-     bool is64;
-} __packed;
-
 #endif /* BOOT_COMPRESSED_EBOOT_H */

arch/x86/include/asm/efi.h +18 -0

index 1eb5f6433ad8..55059a50a01f 100644
@@ -156,6 +156,31 @@ static inline efi_status_t efi_thunk_set_virtual_address_map(
      return EFI_SUCCESS;
 }
 #endif /* CONFIG_EFI_MIXED */
+
+
+/* arch specific definitions used by the stub code */
+
+struct efi_config {
+     u64 image_handle;
+     u64 table;
+     u64 allocate_pool;
+     u64 allocate_pages;
+     u64 get_memory_map;
+     u64 free_pool;
+     u64 free_pages;
+     u64 locate_handle;
+     u64 handle_protocol;
+     u64 exit_boot_services;
+     u64 text_output;
+     efi_status_t (*call)(unsigned long, ...);
+     bool is64;

arch/x86/include/asm/efi.h static inline efi_status_t efi_thunk_set_virtual_address_map( · line 182

Quoted 6 lines

arch/x86/include/asm/efi.h +6 -0

+} __packed;
+
+extern struct efi_config *efi_early;
+
+#define efi_call_early(f, ...)                                               \
+     efi_early->call(efi_early->f, __VA_ARGS__);

That shouldn't have the trailing ;

In order to move from the #include "../../../xxxxx.c" anti-pattern used by both the x86 and arm64 versions of the stub to a static library linked into either the kernel proper (arm64) or a separate boot executable (x86), there is some prepatory work required.

This patch does the following:
- move forward declarations of functions shared between the arch specific and the generic parts of the stub to include/linux/efi.h
- move forward declarations of functions shared between various .c files of the generic stub code to a new local header file called "efistub.h"
- add #includes to all .c files which were formerly relying on the #includor to include the correct header files
- remove all static modifiers from functions which will need to be externally visible once we move to a static library

Signed-off-by Ard Biesheuvel <[email protected]>

include/linux/efi.h +3 -0

index 0ceb816bdfc2..3a64f2f85821 100644
@@ -1163,4 +1163,46 @@ static inline void
 efi_runtime_map_setup(void *map, int nr_entries, u32 desc_size) {}
 #endif
 
+/* prototypes shared between arch specific and generic stub code */
+
+#define pr_efi(sys_table, msg)     efi_printk(sys_table, "EFI stub: "msg)

Quoted 16 lines · Ard Biesheuvel

In order to move from the #include "../../../xxxxx.c" anti-pattern used by both the x86 and arm64 versions of the stub to a static library linked into either the kernel proper (arm64) or a separate boot executable (x86), there is some prepatory work required.

This patch does the following:
- move forward declarations of functions shared between the arch specific and the generic parts of the stub to include/linux/efi.h
- move forward declarations of functions shared between various .c files of the generic stub code to a new local header file called "efistub.h"
- add #includes to all .c files which were formerly relying on the #includor to include the correct header files
- remove all static modifiers from functions which will need to be externally visible once we move to a static library

Signed-off-by Ard Biesheuvel <[email protected]>

[...]

Quoted 11 lines

include/linux/efi.h +3 -0

index 0ceb816bdfc2..3a64f2f85821 100644
@@ -1163,4 +1163,46 @@ static inline void
 efi_runtime_map_setup(void *map, int nr_entries, u32 desc_size) {}
 #endif
 
+/* prototypes shared between arch specific and generic stub code */
+
+#define pr_efi(sys_table, msg)     efi_printk(sys_table, "EFI stub: "msg)

include/linux/efi.h static inline void · line 1178

Quoted 10 lines

include/linux/efi.h +10 -0

+#define pr_efi_err(sys_table, msg) efi_printk(sys_table, "EFI stub: ERROR: "msg)
+
+void efi_printk(efi_system_table_t *sys_table_arg, char *str);
+
+void efi_free(efi_system_table_t *sys_table_arg, unsigned long size,
+           unsigned long addr);
+
+char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
+                       efi_loaded_image_t *image, int *cmd_line_len);
+

We've been very good so far at not splattering include/linux/efi.h with any of the EFI boot stub prototypes, and it would be awesome if we didn't have to start now.

Is there any way we could avoid doing this? Arguably everything should be in the new efistub.h, no?

Quoted 46 lines · Matt Fleming

Quoted 16 lines · Ard Biesheuvel

In order to move from the #include "../../../xxxxx.c" anti-pattern used by both the x86 and arm64 versions of the stub to a static library linked into either the kernel proper (arm64) or a separate boot executable (x86), there is some prepatory work required.

This patch does the following:
- move forward declarations of functions shared between the arch specific and the generic parts of the stub to include/linux/efi.h
- move forward declarations of functions shared between various .c files of the generic stub code to a new local header file called "efistub.h"
- add #includes to all .c files which were formerly relying on the #includor to include the correct header files
- remove all static modifiers from functions which will need to be externally visible once we move to a static library

Signed-off-by Ard Biesheuvel <[email protected]>

[...]

Quoted 11 lines

include/linux/efi.h +3 -0

index 0ceb816bdfc2..3a64f2f85821 100644
@@ -1163,4 +1163,46 @@ static inline void
 efi_runtime_map_setup(void *map, int nr_entries, u32 desc_size) {}
 #endif
 
+/* prototypes shared between arch specific and generic stub code */
+
+#define pr_efi(sys_table, msg)     efi_printk(sys_table, "EFI stub: "msg)

include/linux/efi.h static inline void · line 1178

Quoted 10 lines

include/linux/efi.h +10 -0

+#define pr_efi_err(sys_table, msg) efi_printk(sys_table, "EFI stub: ERROR: "msg)
+
+void efi_printk(efi_system_table_t *sys_table_arg, char *str);
+
+void efi_free(efi_system_table_t *sys_table_arg, unsigned long size,
+           unsigned long addr);
+
+char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
+                       efi_loaded_image_t *image, int *cmd_line_len);
+

We've been very good so far at not splattering include/linux/efi.h with any of the EFI boot stub prototypes, and it would be awesome if we didn't have to start now.

Quoted 3 lines

Is there any way we could avoid doing this? Arguably everything should be in the new efistub.h, no?

There are bits that are shared between code under arch/xxx and drivers/firmware/efi, and what those bits are is different between the archs. I used <asm/efi.h> just for convenience, but perhaps we could add <asm/efistub.h> for each arch, and #include it in both "efistub.h" under drivers/firmware/efi and the stub bits that live under arch/xxx? Then any other users of <asm/efi.h> won't have to see it.

... but that wasn't your question ....

The reason this can't live in "efistub.h" is that it gets included by bits under arch/xxx
So perhaps moving the local "efistub.h" to <linux/efistub.h> would be appropriate, yes.

This patch changes both x86 and arm64 efistub implementations from #including shared .c files under drivers/firmware/efi to building the shared code as a static library.

The x86 code uses a stub built into the boot executable which uncompresses the kernel at boot time. In this case, the library is linked into the decompressor.

In the arm64 case, the stub is part of the kernel proper so the library is linked into the kernel proper as well.

Signed-off-by Ard Biesheuvel <[email protected]>

Quoted 11 lines · Ard Biesheuvel

This patch changes both x86 and arm64 efistub implementations from #including shared .c files under drivers/firmware/efi to building the shared code as a static library.

The x86 code uses a stub built into the boot executable which uncompresses the kernel at boot time. In this case, the library is linked into the decompressor.

In the arm64 case, the stub is part of the kernel proper so the library is linked into the kernel proper as well.

Signed-off-by Ard Biesheuvel <[email protected]>

OK, this breaks the ia64 build because of the following...

/drivers/firmware/efi/libstub/efi-stub-helper.c:14:21: fatal error: asm/efi.h: No such file or directory

drivers/firmware/efi/Makefile line 7

Quoted 13 lines

drivers/firmware/efi/Makefile +1 -1

index e1096539eedb..d9abdbc962f1 100644
@@ -1,7 +1,7 @@
 #
 # Makefile for linux kernel
 #
-obj-$(CONFIG_EFI)                    += efi.o vars.o
+obj-$(CONFIG_EFI)                    += efi.o vars.o libstub/
 obj-$(CONFIG_EFI_VARS)                       += efivars.o
 obj-$(CONFIG_EFI_VARS_PSTORE)                += efi-pstore.o
 obj-$(CONFIG_UEFI_CPER)                      += cper.o

I guess what we need is CONFIG_EFI_LIBSTUB selected by both CONFIG_EFI_STUB (for x86) and CONFIG_EFI_ARMSTUB (for arm64)? e.g.

obj-$(CONFIG_EFI_LIBSTUB) libstub/

Quoted 34 lines · Matt Fleming

Quoted 11 lines · Ard Biesheuvel

This patch changes both x86 and arm64 efistub implementations from #including shared .c files under drivers/firmware/efi to building the shared code as a static library.

The x86 code uses a stub built into the boot executable which uncompresses the kernel at boot time. In this case, the library is linked into the decompressor.

In the arm64 case, the stub is part of the kernel proper so the library is linked into the kernel proper as well.

Signed-off-by Ard Biesheuvel <[email protected]>

OK, this breaks the ia64 build because of the following...

/drivers/firmware/efi/libstub/efi-stub-helper.c:14:21: fatal error: asm/efi.h: No such file or directory

drivers/firmware/efi/Makefile line 7

Quoted 13 lines

drivers/firmware/efi/Makefile +1 -1

index e1096539eedb..d9abdbc962f1 100644
@@ -1,7 +1,7 @@
 #
 # Makefile for linux kernel
 #
-obj-$(CONFIG_EFI)                    += efi.o vars.o
+obj-$(CONFIG_EFI)                    += efi.o vars.o libstub/
 obj-$(CONFIG_EFI_VARS)                       += efivars.o
 obj-$(CONFIG_EFI_VARS_PSTORE)                += efi-pstore.o
 obj-$(CONFIG_UEFI_CPER)                      += cper.o

I guess what we need is CONFIG_EFI_LIBSTUB selected by both CONFIG_EFI_STUB (for x86) and CONFIG_EFI_ARMSTUB (for arm64)? e.g.

Quoted 2 lines

obj-$(CONFIG_EFI_LIBSTUB) libstub/

Yes, that seems the appropriate way to deal with this.
Let me respin so I can fix the other thing I mentioned yesterday as well.

--
Ard.

This is v2 of the series to change the #include "../../../../xxx.c" pattern into a static library linked into either the kernel (arm64) or a separate boot decompressor (x86, ARM).

Changes since v1:
- added patch #1 to change EFI_ERROR, it is not a result code defined by UEFI so it should only be returned by get_dram_base() and efi_entry()
- added a section to libstub Makefile to clean CFLAGS of stack protecter and other options that are inappropriate for the stub
- rebased onto the UEFI Runtime Services NEON patches (re)posted earlier today

Ard Biesheuvel (5):
efi/arm64: avoid EFI_ERROR as a generic return code efi/x86: efistub: move shared dependencies to <asm/efi.h> efi/arm64: efistub: move shared dependencies to <asm/efi.h>
efi: efistub: refactor stub components
efi: efistub: convert into static library

Quoted 11 lines · Ard Biesheuvel

This is v2 of the series to change the #include "../../../../xxx.c" pattern into a static library linked into either the kernel (arm64) or a separate boot decompressor (x86, ARM).

Changes since v1:
- added patch #1 to change EFI_ERROR, it is not a result code defined by UEFI so it should only be returned by get_dram_base() and efi_entry()
- added a section to libstub Makefile to clean CFLAGS of stack protecter and other options that are inappropriate for the stub
- rebased onto the UEFI Runtime Services NEON patches (re)posted earlier today

Quoted 6 lines

Ard Biesheuvel (5):
efi/arm64: avoid EFI_ERROR as a generic return code efi/x86: efistub: move shared dependencies to <asm/efi.h> efi/arm64: efistub: move shared dependencies to <asm/efi.h>
efi: efistub: refactor stub components
efi: efistub: convert into static library

Thanks Ard. I've picked these up.

OK, thanks. One tiny snag that I would like to fix up once we have some Acks to add: I just noticed that i removed the 'static' from handle_kernel_image()'s definition from arm64 in 4/5, but removed it from the forward declaration in 5/5, which likely breaks bisect for arm64 if you happen to end up right in the middle.