Re: [PATCH v2 4/5] efi: efistub: refactor stub components

[Ard Biesheuvel]

On 1 July 2014 17:11, Matt Fleming m...@console-pimps.org wrote:
 On Thu, 26 Jun, at 04:23:36PM, Ard Biesheuvel wrote:
 In order to move from the #include ../../../x.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 ard.biesheu...@linaro.org

 [...]

 diff --git a/include/linux/efi.h b/include/linux/efi.h
 index 0ceb816bdfc2..3a64f2f85821 100644
 --- a/include/linux/efi.h
 +++ b/include/linux/efi.h
 @@ -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)
 +#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?


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.

-- 
Ard.
--
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

[PATCH v2 4/5] efi: efistub: refactor stub components

[Ard Biesheuvel]

In order to move from the #include ../../../x.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 ard.biesheu...@linaro.org
---
 arch/arm64/kernel/efi-stub.c   | 29 -
 arch/x86/boot/compressed/eboot.c   | 13 +++---
 drivers/firmware/efi/arm-stub.c| 18 ++---
 drivers/firmware/efi/efi-stub-helper.c | 74 +-
 drivers/firmware/efi/efistub.h | 42 +++
 drivers/firmware/efi/fdt.c | 20 +
 include/linux/efi.h| 42 +++
 7 files changed, 157 insertions(+), 81 deletions(-)
 create mode 100644 drivers/firmware/efi/efistub.h

diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
index 23cbde4324b1..e4999021b07d 100644
--- a/arch/arm64/kernel/efi-stub.c
+++ b/arch/arm64/kernel/efi-stub.c
@@ -11,36 +11,21 @@
  */
 #include linux/efi.h
 #include asm/efi.h
-#include linux/libfdt.h
 #include asm/sections.h
 
-static void efi_char16_printk(efi_system_table_t *sys_table_arg,
- efi_char16_t *str);
-
-static efi_status_t efi_open_volume(efi_system_table_t *sys_table,
-   void *__image, void **__fh);
-static efi_status_t efi_file_close(void *handle);
-
-static efi_status_t
-efi_file_read(void *handle, unsigned long *size, void *addr);
-
-static efi_status_t
-efi_file_size(efi_system_table_t *sys_table, void *__fh,
- efi_char16_t *filename_16, void **handle, u64 *file_sz);
-
 /* Include shared EFI stub code */
 #include ../../../drivers/firmware/efi/efi-stub-helper.c
 #include ../../../drivers/firmware/efi/fdt.c
 #include ../../../drivers/firmware/efi/arm-stub.c
 
 
-static efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
-   unsigned long *image_addr,
-   unsigned long *image_size,
-   unsigned long *reserve_addr,
-   unsigned long *reserve_size,
-   unsigned long dram_base,
-   efi_loaded_image_t *image)
+efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
+unsigned long *image_addr,
+unsigned long *image_size,
+unsigned long *reserve_addr,
+unsigned long *reserve_size,
+unsigned long dram_base,
+efi_loaded_image_t *image)
 {
efi_status_t status;
unsigned long kernel_size, kernel_memsize = 0;
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 2fd5e2643623..d338c134c659 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -45,8 +45,7 @@ static void setup_boot_services##bits(struct efi_config *c)   
\
 BOOT_SERVICES(32);
 BOOT_SERVICES(64);
 
-static void efi_printk(efi_system_table_t *, char *);
-static void efi_char16_printk(efi_system_table_t *, efi_char16_t *);
+void efi_char16_printk(efi_system_table_t *, efi_char16_t *);
 
 static efi_status_t
 __file_size32(void *__fh, efi_char16_t *filename_16,
@@ -153,7 +152,7 @@ grow:
 
return status;
 }
-static efi_status_t
+efi_status_t
 efi_file_size(efi_system_table_t *sys_table, void *__fh,
  efi_char16_t *filename_16, void **handle, u64 *file_sz)
 {
@@ -163,7 +162,7 @@ efi_file_size(efi_system_table_t *sys_table, void *__fh,
return __file_size32(__fh, filename_16, handle, file_sz);
 }
 
-static inline efi_status_t
+efi_status_t
 efi_file_read(void *handle, unsigned long *size, void *addr)
 {
unsigned long func;
@@ -181,7 +180,7 @@ efi_file_read(void *handle, unsigned long *size, void *addr)
}
 }
 
-static inline efi_status_t efi_file_close(void *handle)
+efi_status_t efi_file_close(void *handle)
 {
if (efi_early-is64) {
efi_file_handle_64_t *fh = handle;
@@ -246,7 +245,7 @@ static inline efi_status_t __open_volume64(void *__image, 
void **__fh)
return status;
 }
 
-static inline efi_status_t
+efi_status_t