On Fri, Jan 20, 2023 at 05:17:13PM -0800, Atish Patra wrote: > The arch specific image header details are not very useful as > most of the grub just looks at the PE/COFF spec parameters (PE32 magic > and header offset). > > Remove the arch specific images headers and define a generic > arch headers that provide enough PE/COFF fields for grub to parse kernel > images correctly. > > Signed-off-by: Atish Patra <ati...@rivosinc.com> > --- > grub-core/commands/file.c | 8 +++--- > grub-core/loader/arm64/xen_boot.c | 3 +- > grub-core/loader/efi/linux.c | 1 - > include/grub/arm64/linux.h | 48 ------------------------------- > include/grub/efi/efi.h | 11 ++++++- > include/grub/riscv32/linux.h | 41 -------------------------- > include/grub/riscv64/linux.h | 43 --------------------------- > 7 files changed, 15 insertions(+), 140 deletions(-) > delete mode 100644 include/grub/arm64/linux.h > delete mode 100644 include/grub/riscv32/linux.h > delete mode 100644 include/grub/riscv64/linux.h
Sadly this patch broke normal ARM builds. I had to apply following fix... diff --git a/grub-core/commands/file.c b/grub-core/commands/file.c index 9ba0e5eca..db9fdc5f2 100644 --- a/grub-core/commands/file.c +++ b/grub-core/commands/file.c @@ -391,7 +391,7 @@ grub_cmd_file (grub_extcmd_context_t ctxt, int argc, char **args) } case IS_ARM_LINUX: { - struct linux_arm_kernel_header lh; + struct linux_arch_kernel_header lh; if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh)) break; diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c index f00b538eb..19ddedbc2 100644 --- a/grub-core/loader/arm/linux.c +++ b/grub-core/loader/arm/linux.c @@ -26,6 +26,7 @@ #include <grub/command.h> #include <grub/cache.h> #include <grub/cpu/linux.h> +#include <grub/efi/efi.h> #include <grub/lib/cmdline.h> #include <grub/linux.h> #include <grub/verify.h> @@ -304,7 +305,7 @@ linux_boot (void) static grub_err_t linux_load (const char *filename, grub_file_t file) { - struct linux_arm_kernel_header *lh; + struct linux_arch_kernel_header *lh; int size; size = grub_file_size (file); diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h index f38e695b1..5b8fb14e0 100644 --- a/include/grub/arm/linux.h +++ b/include/grub/arm/linux.h @@ -24,26 +24,6 @@ #include <grub/efi/pe32.h> -#define GRUB_LINUX_ARM_MAGIC_SIGNATURE 0x016f2818 - -struct linux_arm_kernel_header { - grub_uint32_t code0; - grub_uint32_t reserved1[8]; - grub_uint32_t magic; - grub_uint32_t start; /* _start */ - grub_uint32_t end; /* _edata */ - grub_uint32_t reserved2[3]; - grub_uint32_t hdr_offset; -#if defined GRUB_MACHINE_EFI - struct grub_pe_image_header pe_image_header; -#endif -}; - -#if defined(__arm__) -# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM_MAGIC_SIGNATURE -# define linux_arch_kernel_header linux_arm_kernel_header -#endif - #if defined GRUB_MACHINE_UBOOT # include <grub/uboot/uboot.h> # define LINUX_ADDRESS (start_of_ram + 0x8000) diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h index b9e7f6724..329c4f9b2 100644 --- a/include/grub/efi/efi.h +++ b/include/grub/efi/efi.h @@ -25,13 +25,15 @@ #include <grub/efi/api.h> #include <grub/efi/pe32.h> +#define GRUB_LINUX_ARM_MAGIC_SIGNATURE 0x016f2818 + struct linux_arch_kernel_header { - grub_uint32_t code0; - grub_uint32_t code1; - grub_uint64_t reserved[6]; - grub_uint32_t reserved1; - grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ - struct grub_pe_image_header pe_image_header; + grub_uint32_t code0; + grub_uint32_t code1; + grub_uint64_t reserved[6]; + grub_uint32_t magic; + grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ + struct grub_pe_image_header pe_image_header; }; So, final version of this patch will look like this... From d6f32defa2523a9145fae839ebcdfff4f406dde4 Mon Sep 17 00:00:00 2001 From: Atish Patra <ati...@rivosinc.com> Date: Fri, 20 Jan 2023 17:17:13 -0800 Subject: [PATCH 2/3] efi: Remove arch specific image headers for RISC-V & ARM64 The arch specific image header details are not very useful as most of the GRUB just looks at the PE/COFF spec parameters (PE32 magic and header offset). Remove the arch specific images headers and define a generic arch headers that provide enough PE/COFF fields for GRUB to parse kernel images correctly. Signed-off-by: Atish Patra <ati...@rivosinc.com> Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> --- grub-core/commands/file.c | 10 ++++---- grub-core/loader/arm/linux.c | 3 ++- grub-core/loader/arm64/xen_boot.c | 3 +-- grub-core/loader/efi/linux.c | 1 - include/grub/arm/linux.h | 20 ---------------- include/grub/arm64/linux.h | 48 --------------------------------------- include/grub/efi/efi.h | 13 ++++++++++- include/grub/riscv32/linux.h | 41 --------------------------------- include/grub/riscv64/linux.h | 43 ----------------------------------- 9 files changed, 20 insertions(+), 162 deletions(-) delete mode 100644 include/grub/arm64/linux.h delete mode 100644 include/grub/riscv32/linux.h delete mode 100644 include/grub/riscv64/linux.h diff --git a/grub-core/commands/file.c b/grub-core/commands/file.c index 9de00061e..db9fdc5f2 100644 --- a/grub-core/commands/file.c +++ b/grub-core/commands/file.c @@ -25,10 +25,10 @@ #include <grub/i18n.h> #include <grub/file.h> #include <grub/elf.h> +#include <grub/efi/efi.h> #include <grub/xen_file.h> #include <grub/efi/pe32.h> #include <grub/arm/linux.h> -#include <grub/arm64/linux.h> #include <grub/i386/linux.h> #include <grub/xnu.h> #include <grub/machoload.h> @@ -391,7 +391,7 @@ grub_cmd_file (grub_extcmd_context_t ctxt, int argc, char **args) } case IS_ARM_LINUX: { - struct linux_arm_kernel_header lh; + struct linux_arch_kernel_header lh; if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh)) break; @@ -412,13 +412,13 @@ grub_cmd_file (grub_extcmd_context_t ctxt, int argc, char **args) } case IS_ARM64_LINUX: { - struct linux_arm64_kernel_header lh; + struct linux_arch_kernel_header lh; if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh)) break; - if (lh.magic == - grub_cpu_to_le32_compile_time (GRUB_LINUX_ARM64_MAGIC_SIGNATURE)) + if (lh.pe_image_header.coff_header.machine == + grub_cpu_to_le32_compile_time (GRUB_PE32_MACHINE_ARM64)) { ret = 1; break; diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c index f00b538eb..19ddedbc2 100644 --- a/grub-core/loader/arm/linux.c +++ b/grub-core/loader/arm/linux.c @@ -26,6 +26,7 @@ #include <grub/command.h> #include <grub/cache.h> #include <grub/cpu/linux.h> +#include <grub/efi/efi.h> #include <grub/lib/cmdline.h> #include <grub/linux.h> #include <grub/verify.h> @@ -304,7 +305,7 @@ linux_boot (void) static grub_err_t linux_load (const char *filename, grub_file_t file) { - struct linux_arm_kernel_header *lh; + struct linux_arch_kernel_header *lh; int size; size = grub_file_size (file); diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c index 763d87dcd..26e1472c9 100644 --- a/grub-core/loader/arm64/xen_boot.c +++ b/grub-core/loader/arm64/xen_boot.c @@ -27,7 +27,6 @@ #include <grub/misc.h> #include <grub/mm.h> #include <grub/types.h> -#include <grub/cpu/linux.h> #include <grub/efi/efi.h> #include <grub/efi/fdtload.h> #include <grub/efi/memory.h> @@ -439,7 +438,7 @@ static grub_err_t grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)), int argc, char *argv[]) { - struct linux_arm64_kernel_header lh; + struct linux_arch_kernel_header lh; grub_file_t file = NULL; grub_dl_ref (my_mod); diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c index 48ab34a25..15e068654 100644 --- a/grub-core/loader/efi/linux.c +++ b/grub-core/loader/efi/linux.c @@ -25,7 +25,6 @@ #include <grub/loader.h> #include <grub/mm.h> #include <grub/types.h> -#include <grub/cpu/linux.h> #include <grub/efi/efi.h> #include <grub/efi/fdtload.h> #include <grub/efi/memory.h> diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h index f38e695b1..5b8fb14e0 100644 --- a/include/grub/arm/linux.h +++ b/include/grub/arm/linux.h @@ -24,26 +24,6 @@ #include <grub/efi/pe32.h> -#define GRUB_LINUX_ARM_MAGIC_SIGNATURE 0x016f2818 - -struct linux_arm_kernel_header { - grub_uint32_t code0; - grub_uint32_t reserved1[8]; - grub_uint32_t magic; - grub_uint32_t start; /* _start */ - grub_uint32_t end; /* _edata */ - grub_uint32_t reserved2[3]; - grub_uint32_t hdr_offset; -#if defined GRUB_MACHINE_EFI - struct grub_pe_image_header pe_image_header; -#endif -}; - -#if defined(__arm__) -# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM_MAGIC_SIGNATURE -# define linux_arch_kernel_header linux_arm_kernel_header -#endif - #if defined GRUB_MACHINE_UBOOT # include <grub/uboot/uboot.h> # define LINUX_ADDRESS (start_of_ram + 0x8000) diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h deleted file mode 100644 index 3da71a512..000000000 --- a/include/grub/arm64/linux.h +++ /dev/null @@ -1,48 +0,0 @@ -/* - * GRUB -- GRand Unified Bootloader - * Copyright (C) 2013 Free Software Foundation, Inc. - * - * GRUB is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * GRUB is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with GRUB. If not, see <http://www.gnu.org/licenses/>. - */ - -#ifndef GRUB_ARM64_LINUX_HEADER -#define GRUB_ARM64_LINUX_HEADER 1 - -#include <grub/types.h> -#include <grub/efi/pe32.h> - -#define GRUB_LINUX_ARM64_MAGIC_SIGNATURE 0x644d5241 /* 'ARM\x64' */ - -/* From linux/Documentation/arm64/booting.txt */ -struct linux_arm64_kernel_header -{ - grub_uint32_t code0; /* Executable code */ - grub_uint32_t code1; /* Executable code */ - grub_uint64_t text_offset; /* Image load offset */ - grub_uint64_t res0; /* reserved */ - grub_uint64_t res1; /* reserved */ - grub_uint64_t res2; /* reserved */ - grub_uint64_t res3; /* reserved */ - grub_uint64_t res4; /* reserved */ - grub_uint32_t magic; /* Magic number, little endian, "ARM\x64" */ - grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ - struct grub_pe_image_header pe_image_header; -}; - -#if defined(__aarch64__) -# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM64_MAGIC_SIGNATURE -# define linux_arch_kernel_header linux_arm64_kernel_header -#endif - -#endif /* ! GRUB_ARM64_LINUX_HEADER */ diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h index e61272de5..329c4f9b2 100644 --- a/include/grub/efi/efi.h +++ b/include/grub/efi/efi.h @@ -23,6 +23,18 @@ #include <grub/types.h> #include <grub/dl.h> #include <grub/efi/api.h> +#include <grub/efi/pe32.h> + +#define GRUB_LINUX_ARM_MAGIC_SIGNATURE 0x016f2818 + +struct linux_arch_kernel_header { + grub_uint32_t code0; + grub_uint32_t code1; + grub_uint64_t reserved[6]; + grub_uint32_t magic; + grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ + struct grub_pe_image_header pe_image_header; +}; /* Functions. */ void *EXPORT_FUNC(grub_efi_locate_protocol) (grub_efi_guid_t *protocol, @@ -101,7 +113,6 @@ extern void (*EXPORT_VAR(grub_efi_net_config)) (grub_efi_handle_t hnd, #if defined(__arm__) || defined(__aarch64__) || defined(__riscv) void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void); grub_err_t EXPORT_FUNC(grub_efi_get_ram_base)(grub_addr_t *); -#include <grub/cpu/linux.h> #include <grub/file.h> grub_err_t grub_arch_efi_linux_load_image_header(grub_file_t file, struct linux_arch_kernel_header *lh); diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h deleted file mode 100644 index 512b777c8..000000000 --- a/include/grub/riscv32/linux.h +++ /dev/null @@ -1,41 +0,0 @@ -/* - * GRUB -- GRand Unified Bootloader - * Copyright (C) 2018 Free Software Foundation, Inc. - * - * GRUB is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * GRUB is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with GRUB. If not, see <http://www.gnu.org/licenses/>. - */ - -#ifndef GRUB_RISCV32_LINUX_HEADER -#define GRUB_RISCV32_LINUX_HEADER 1 - -#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */ - -/* From linux/Documentation/riscv/booting.txt */ -struct linux_riscv_kernel_header -{ - grub_uint32_t code0; /* Executable code */ - grub_uint32_t code1; /* Executable code */ - grub_uint64_t text_offset; /* Image load offset */ - grub_uint64_t res0; /* reserved */ - grub_uint64_t res1; /* reserved */ - grub_uint64_t res2; /* reserved */ - grub_uint64_t res3; /* reserved */ - grub_uint64_t res4; /* reserved */ - grub_uint32_t magic; /* Magic number, little endian, "RSCV" */ - grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ -}; - -#define linux_arch_kernel_header linux_riscv_kernel_header - -#endif /* ! GRUB_RISCV32_LINUX_HEADER */ diff --git a/include/grub/riscv64/linux.h b/include/grub/riscv64/linux.h deleted file mode 100644 index 3630c30fb..000000000 --- a/include/grub/riscv64/linux.h +++ /dev/null @@ -1,43 +0,0 @@ -/* - * GRUB -- GRand Unified Bootloader - * Copyright (C) 2018 Free Software Foundation, Inc. - * - * GRUB is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * GRUB is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with GRUB. If not, see <http://www.gnu.org/licenses/>. - */ - -#ifndef GRUB_RISCV64_LINUX_HEADER -#define GRUB_RISCV64_LINUX_HEADER 1 - -#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */ - -#define GRUB_EFI_PE_MAGIC 0x5A4D - -/* From linux/Documentation/riscv/booting.txt */ -struct linux_riscv_kernel_header -{ - grub_uint32_t code0; /* Executable code */ - grub_uint32_t code1; /* Executable code */ - grub_uint64_t text_offset; /* Image load offset */ - grub_uint64_t res0; /* reserved */ - grub_uint64_t res1; /* reserved */ - grub_uint64_t res2; /* reserved */ - grub_uint64_t res3; /* reserved */ - grub_uint64_t res4; /* reserved */ - grub_uint32_t magic; /* Magic number, little endian, "RSCV" */ - grub_uint32_t hdr_offset; /* Offset of PE/COFF header */ -}; - -#define linux_arch_kernel_header linux_riscv_kernel_header - -#endif /* ! GRUB_RISCV64_LINUX_HEADER */ -- 2.11.0 Please check I did not make any mistake. If my fix is correct then I will push the patches with it applied. Though even after this there is still a problem with ARM64 Linux kernel detection code in grub-core/commands/file.c:grub_cmd_file(). The lh.pe_image_header.coff_header.machine field can be in different place of the PE file. I think the logic should be aligned with grub-core/loader/efi/linux.c:grub_arch_efi_linux_load_image_header(). If you could do that it would be nice. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel