[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [v7 PATCH 2/3] efi: Remove arch specific image headers for RISC-V &
|
From: |
Daniel Kiper |
|
Subject: |
Re: [v7 PATCH 2/3] efi: Remove arch specific image headers for RISC-V & ARM64 |
|
Date: |
Thu, 2 Feb 2023 21:12:18 +0100 |
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 <atishp@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 <atishp@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 <atishp@rivosinc.com>
Reviewed-by: Daniel Kiper <daniel.kiper@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
- Re: [v7 PATCH 2/3] efi: Remove arch specific image headers for RISC-V & ARM64,
Daniel Kiper <=