Discussion:
[PATCH v3 0/6] efi: arm linux loader unification and correctness
Leif Lindholm
2018-06-27 17:17:14 UTC
Permalink
The existing linux loader for 32-bit ARM is really only a piggy-back
on the U-Boot loader, and for UEFI it's entirely possible to reuse
the same loader across multiple architectures.

This set will stop the ability to boot non-efistub kernels on arm-efi,
but that has really only ever worked by luck.

Also, both arm and arm64 have certain requirements with regards to
placement of the initrd, which weren't honored by the existing loader -
leading to theoretical issues on arm and some very observable ones on
arm64 systems with large amounts of RAM or large holes in the memory
map. So ensure the unified loader respects these requirements.

It would be difficult to provide a linear history of this set, but the
main change since v2 is the renaming/refactoring of linux headers to
be included for multiple architectures in a single source file
(which was actually submitted as a separate set), and the follow-on
changes this required for the unified apis.

Leif Lindholm (6):
efi: add central copy of grub_efi_find_mmap_size
efi: add grub_efi_get_ram_base() function for arm64
arm64 linux loader: rename functions and macros and move to common
headers
arm/efi: switch to arm64 linux loader
arm: delete unused efi support from loader/arm
efi: restrict arm/arm64 linux loader initrd placement

grub-core/Makefile.am | 1 -
grub-core/Makefile.core.def | 7 +-
grub-core/kern/arm/efi/misc.c | 202 --------------------------------------
grub-core/kern/efi/mm.c | 56 +++++++++++
grub-core/loader/arm/linux.c | 28 ------
grub-core/loader/arm64/linux.c | 53 ++++++++--
grub-core/loader/arm64/xen_boot.c | 10 +-
include/grub/arm/efi/loader.h | 26 -----
include/grub/arm/linux.h | 21 +---
include/grub/arm64/linux.h | 9 +-
include/grub/efi/efi.h | 6 ++
11 files changed, 125 insertions(+), 294 deletions(-)
delete mode 100644 grub-core/kern/arm/efi/misc.c
delete mode 100644 include/grub/arm/efi/loader.h
--
2.11.0
Leif Lindholm
2018-06-27 17:17:16 UTC
Permalink
Since ARM platforms do not have a common memory map, add a helper
function that finds the lowest address region with the EFI_MEMORY_WB
attribute set in the UEFI memory map.

Required for the arm64 efi linux loader to restrict the initrd
location to where it will be accessible by the kernel at runtime.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
grub-core/kern/efi/mm.c | 36 ++++++++++++++++++++++++++++++++++++
include/grub/efi/efi.h | 3 +++
2 files changed, 39 insertions(+)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index fd39d23b4..10ffa2c9b 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -629,3 +629,39 @@ grub_efi_mm_init (void)
grub_efi_free_pages ((grub_addr_t) memory_map,
2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
}
+
+#if defined (__aarch64__)
+grub_err_t
+grub_efi_get_ram_base(grub_addr_t *base_addr)
+{
+ grub_efi_memory_descriptor_t *memory_map;
+ grub_efi_memory_descriptor_t *desc;
+ grub_efi_uintn_t mmap_size;
+ grub_efi_uintn_t desc_size;
+ int ret;
+
+ mmap_size = grub_efi_find_mmap_size();
+
+ memory_map = grub_malloc (mmap_size);
+ if (! memory_map)
+ return GRUB_ERR_OUT_OF_MEMORY;
+ ret = grub_efi_get_memory_map (&mmap_size, memory_map, NULL,
+ &desc_size, NULL);
+
+ if (ret < 1)
+ return GRUB_ERR_BUG;
+
+ for (desc = memory_map, *base_addr = GRUB_UINT_MAX;
+ (grub_addr_t) desc < ((grub_addr_t) memory_map + mmap_size);
+ desc = NEXT_MEMORY_DESCRIPTOR (desc, desc_size))
+ {
+ if (desc->attribute & GRUB_EFI_MEMORY_WB)
+ if (desc->physical_start < *base_addr)
+ *base_addr = desc->physical_start;
+ }
+
+ grub_free(memory_map);
+
+ return GRUB_ERR_NONE;
+}
+#endif
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index 1021273c1..57db74b57 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -93,6 +93,9 @@ extern void (*EXPORT_VAR(grub_efi_net_config)) (grub_efi_handle_t hnd,
#if defined(__arm__) || defined(__aarch64__)
void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
#endif
+#if defined(__aarch64__)
+grub_err_t EXPORT_FUNC(grub_efi_get_ram_base)(grub_addr_t *);
+#endif

grub_addr_t grub_efi_modules_addr (void);
--
2.11.0
Daniel Kiper
2018-07-06 15:12:27 UTC
Permalink
Post by Leif Lindholm
Since ARM platforms do not have a common memory map, add a helper
function that finds the lowest address region with the EFI_MEMORY_WB
attribute set in the UEFI memory map.
Required for the arm64 efi linux loader to restrict the initrd
location to where it will be accessible by the kernel at runtime.
---
grub-core/kern/efi/mm.c | 36 ++++++++++++++++++++++++++++++++++++
include/grub/efi/efi.h | 3 +++
2 files changed, 39 insertions(+)
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index fd39d23b4..10ffa2c9b 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -629,3 +629,39 @@ grub_efi_mm_init (void)
grub_efi_free_pages ((grub_addr_t) memory_map,
2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
}
+
+#if defined (__aarch64__)
+grub_err_t
+grub_efi_get_ram_base(grub_addr_t *base_addr)
+{
+ grub_efi_memory_descriptor_t *memory_map;
+ grub_efi_memory_descriptor_t *desc;
grub_efi_memory_descriptor_t *desc, *memory_map;
Post by Leif Lindholm
+ grub_efi_uintn_t mmap_size;
+ grub_efi_uintn_t desc_size;
grub_efi_uintn_t desc_size, mmap_size;
Post by Leif Lindholm
+ int ret;
+
+ mmap_size = grub_efi_find_mmap_size();
+
+ memory_map = grub_malloc (mmap_size);
+ if (! memory_map)
+ return GRUB_ERR_OUT_OF_MEMORY;
+ ret = grub_efi_get_memory_map (&mmap_size, memory_map, NULL,
+ &desc_size, NULL);
+
+ if (ret < 1)
+ return GRUB_ERR_BUG;
+
+ for (desc = memory_map, *base_addr = GRUB_UINT_MAX;
+ (grub_addr_t) desc < ((grub_addr_t) memory_map + mmap_size);
+ desc = NEXT_MEMORY_DESCRIPTOR (desc, desc_size))
+ {
+ if (desc->attribute & GRUB_EFI_MEMORY_WB)
*base_addr = grub_min (*base_addr, desc->physical_start);
Post by Leif Lindholm
+ if (desc->physical_start < *base_addr)
+ *base_addr = desc->physical_start;
+ }
And then you can drop these curly brackets.

Daniel
Michel Hermier
2018-07-06 16:17:34 UTC
Permalink
Post by Daniel Kiper
Post by Leif Lindholm
Since ARM platforms do not have a common memory map, add a helper
function that finds the lowest address region with the EFI_MEMORY_WB
attribute set in the UEFI memory map.
Required for the arm64 efi linux loader to restrict the initrd
location to where it will be accessible by the kernel at runtime.
---
grub-core/kern/efi/mm.c | 36 ++++++++++++++++++++++++++++++++++++
include/grub/efi/efi.h | 3 +++
2 files changed, 39 insertions(+)
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index fd39d23b4..10ffa2c9b 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -629,3 +629,39 @@ grub_efi_mm_init (void)
grub_efi_free_pages ((grub_addr_t) memory_map,
2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
}
+
+#if defined (__aarch64__)
+grub_err_t
+grub_efi_get_ram_base(grub_addr_t *base_addr)
+{
+ grub_efi_memory_descriptor_t *memory_map;
+ grub_efi_memory_descriptor_t *desc;
grub_efi_memory_descriptor_t *desc, *memory_map;
For readability and ease of understanding, I think mmap_size should be
renamed to memory_map_size (or the other way around) to explicit/uniformize
naming.
Post by Daniel Kiper
Post by Leif Lindholm
+ grub_efi_uintn_t mmap_size;
+ grub_efi_uintn_t desc_size;
grub_efi_uintn_t desc_size, mmap_size;
Post by Leif Lindholm
+ int ret;
+
+ mmap_size = grub_efi_find_mmap_size();
+
+ memory_map = grub_malloc (mmap_size);
+ if (! memory_map)
+ return GRUB_ERR_OUT_OF_MEMORY;
+ ret = grub_efi_get_memory_map (&mmap_size, memory_map, NULL,
+ &desc_size, NULL);
+
+ if (ret < 1)
+ return GRUB_ERR_BUG;
+
+ for (desc = memory_map, *base_addr = GRUB_UINT_MAX;
+ (grub_addr_t) desc < ((grub_addr_t) memory_map + mmap_size);
+ desc = NEXT_MEMORY_DESCRIPTOR (desc, desc_size))
+ {
+ if (desc->attribute & GRUB_EFI_MEMORY_WB)
*base_addr = grub_min (*base_addr, desc->physical_start);
Post by Leif Lindholm
+ if (desc->physical_start < *base_addr)
+ *base_addr = desc->physical_start;
+ }
And then you can drop these curly brackets.
Daniel
_______________________________________________
Grub-devel mailing list
https://lists.gnu.org/mailman/listinfo/grub-devel
Leif Lindholm
2018-07-06 16:24:02 UTC
Permalink
Post by Daniel Kiper
Post by Leif Lindholm
Since ARM platforms do not have a common memory map, add a helper
function that finds the lowest address region with the EFI_MEMORY_WB
attribute set in the UEFI memory map.
Required for the arm64 efi linux loader to restrict the initrd
location to where it will be accessible by the kernel at runtime.
---
grub-core/kern/efi/mm.c | 36 ++++++++++++++++++++++++++++++++++++
include/grub/efi/efi.h | 3 +++
2 files changed, 39 insertions(+)
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index fd39d23b4..10ffa2c9b 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -629,3 +629,39 @@ grub_efi_mm_init (void)
grub_efi_free_pages ((grub_addr_t) memory_map,
2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
}
+
+#if defined (__aarch64__)
+grub_err_t
+grub_efi_get_ram_base(grub_addr_t *base_addr)
+{
+ grub_efi_memory_descriptor_t *memory_map;
+ grub_efi_memory_descriptor_t *desc;
grub_efi_memory_descriptor_t *desc, *memory_map;
Post by Leif Lindholm
+ grub_efi_uintn_t mmap_size;
+ grub_efi_uintn_t desc_size;
grub_efi_uintn_t desc_size, mmap_size;
Post by Leif Lindholm
+ int ret;
+
+ mmap_size = grub_efi_find_mmap_size();
+
+ memory_map = grub_malloc (mmap_size);
+ if (! memory_map)
+ return GRUB_ERR_OUT_OF_MEMORY;
+ ret = grub_efi_get_memory_map (&mmap_size, memory_map, NULL,
+ &desc_size, NULL);
+
+ if (ret < 1)
+ return GRUB_ERR_BUG;
+
+ for (desc = memory_map, *base_addr = GRUB_UINT_MAX;
+ (grub_addr_t) desc < ((grub_addr_t) memory_map + mmap_size);
+ desc = NEXT_MEMORY_DESCRIPTOR (desc, desc_size))
+ {
+ if (desc->attribute & GRUB_EFI_MEMORY_WB)
*base_addr = grub_min (*base_addr, desc->physical_start);
Post by Leif Lindholm
+ if (desc->physical_start < *base_addr)
+ *base_addr = desc->physical_start;
+ }
And then you can drop these curly brackets.
Sure, I'll fold these changes into v4.

/
Leif
Leif Lindholm
2018-06-27 17:17:19 UTC
Permalink
The 32-bit arm efi port now shares the 64-bit linux loader, so delete
the now unused bits from the 32-bit linux loader.

This in turn leaves the grub-core/kern/arm/efi/misc.c unused, so
delete that too.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
grub-core/Makefile.am | 1 -
grub-core/kern/arm/efi/misc.c | 202 ------------------------------------------
grub-core/loader/arm/linux.c | 28 ------
include/grub/arm/efi/loader.h | 26 ------
include/grub/arm/linux.h | 16 ----
5 files changed, 273 deletions(-)
delete mode 100644 grub-core/kern/arm/efi/misc.c
delete mode 100644 include/grub/arm/efi/loader.h

diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
index 104513847..f4ff62b76 100644
--- a/grub-core/Makefile.am
+++ b/grub-core/Makefile.am
@@ -254,7 +254,6 @@ KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/fdtbus.h
endif

if COND_arm_efi
-KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/arm/efi/loader.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/efi.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/disk.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/arm/system.h
diff --git a/grub-core/kern/arm/efi/misc.c b/grub-core/kern/arm/efi/misc.c
deleted file mode 100644
index c95e8299d..000000000
--- a/grub-core/kern/arm/efi/misc.c
+++ /dev/null
@@ -1,202 +0,0 @@
-/* misc.c - various system functions for an arm-based EFI system */
-/*
- * 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/>.
- */
-
-#include <grub/misc.h>
-#include <grub/mm.h>
-#include <grub/cpu/linux.h>
-#include <grub/cpu/system.h>
-#include <grub/efi/efi.h>
-#include <grub/machine/loader.h>
-
-static inline grub_size_t
-page_align (grub_size_t size)
-{
- return (size + (1 << 12) - 1) & (~((1 << 12) - 1));
-}
-
-/* Find the optimal number of pages for the memory map. Is it better to
- move this code to efi/mm.c? */
-static grub_efi_uintn_t
-find_mmap_size (void)
-{
- static grub_efi_uintn_t mmap_size = 0;
-
- if (mmap_size != 0)
- return mmap_size;
-
- mmap_size = (1 << 12);
- while (1)
- {
- int ret;
- grub_efi_memory_descriptor_t *mmap;
- grub_efi_uintn_t desc_size;
-
- mmap = grub_malloc (mmap_size);
- if (! mmap)
- return 0;
-
- ret = grub_efi_get_memory_map (&mmap_size, mmap, 0, &desc_size, 0);
- grub_free (mmap);
-
- if (ret < 0)
- {
- grub_error (GRUB_ERR_IO, "cannot get memory map");
- return 0;
- }
- else if (ret > 0)
- break;
-
- mmap_size += (1 << 12);
- }
-
- /* Increase the size a bit for safety, because GRUB allocates more on
- later, and EFI itself may allocate more. */
- mmap_size += (1 << 12);
-
- return page_align (mmap_size);
-}
-
-#define NEXT_MEMORY_DESCRIPTOR(desc, size) \
- ((grub_efi_memory_descriptor_t *) ((char *) (desc) + (size)))
-#define PAGE_SHIFT 12
-
-void *
-grub_efi_allocate_loader_memory (grub_uint32_t min_offset, grub_uint32_t size)
-{
- grub_efi_uintn_t desc_size;
- grub_efi_memory_descriptor_t *mmap, *mmap_end;
- grub_efi_uintn_t mmap_size, tmp_mmap_size;
- grub_efi_memory_descriptor_t *desc;
- void *mem = NULL;
- grub_addr_t min_start = 0;
-
- mmap_size = find_mmap_size();
- if (!mmap_size)
- return NULL;
-
- mmap = grub_malloc(mmap_size);
- if (!mmap)
- return NULL;
-
- tmp_mmap_size = mmap_size;
- if (grub_efi_get_memory_map (&tmp_mmap_size, mmap, 0, &desc_size, 0) <= 0)
- {
- grub_error (GRUB_ERR_IO, "cannot get memory map");
- goto fail;
- }
-
- mmap_end = NEXT_MEMORY_DESCRIPTOR (mmap, tmp_mmap_size);
- /* Find lowest accessible RAM location */
- {
- int found = 0;
- for (desc = mmap ; !found && (desc < mmap_end) ;
- desc = NEXT_MEMORY_DESCRIPTOR(desc, desc_size))
- {
- switch (desc->type)
- {
- case GRUB_EFI_CONVENTIONAL_MEMORY:
- case GRUB_EFI_LOADER_CODE:
- case GRUB_EFI_LOADER_DATA:
- min_start = desc->physical_start + min_offset;
- found = 1;
- break;
- default:
- break;
- }
- }
- }
-
- /* First, find free pages for the real mode code
- and the memory map buffer. */
- for (desc = mmap ; desc < mmap_end ;
- desc = NEXT_MEMORY_DESCRIPTOR(desc, desc_size))
- {
- grub_uint64_t start, end;
-
- grub_dprintf("mm", "%s: 0x%08x bytes @ 0x%08x\n",
- __FUNCTION__,
- (grub_uint32_t) (desc->num_pages << PAGE_SHIFT),
- (grub_uint32_t) (desc->physical_start));
-
- if (desc->type != GRUB_EFI_CONVENTIONAL_MEMORY)
- continue;
-
- start = desc->physical_start;
- end = start + (desc->num_pages << PAGE_SHIFT);
- grub_dprintf("mm", "%s: start=0x%016llx, end=0x%016llx\n",
- __FUNCTION__, start, end);
- start = start < min_start ? min_start : start;
- if (start + size > end)
- continue;
- grub_dprintf("mm", "%s: let's allocate some (0x%x) pages @ 0x%08x...\n",
- __FUNCTION__, (size >> PAGE_SHIFT), (grub_addr_t) start);
- mem = grub_efi_allocate_fixed (start, (size >> PAGE_SHIFT) + 1);
- grub_dprintf("mm", "%s: retval=0x%08x\n",
- __FUNCTION__, (grub_addr_t) mem);
- if (! mem)
- {
- grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
- goto fail;
- }
- break;
- }
-
- if (! mem)
- {
- grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory");
- goto fail;
- }
-
- grub_free (mmap);
- return mem;
-
- fail:
- grub_free (mmap);
- return NULL;
-}
-
-grub_err_t
-grub_efi_prepare_platform (void)
-{
- grub_efi_uintn_t mmap_size;
- grub_efi_uintn_t map_key;
- grub_efi_uintn_t desc_size;
- grub_efi_uint32_t desc_version;
- grub_efi_memory_descriptor_t *mmap_buf;
- grub_err_t err;
-
- /*
- * Cloned from IA64
- * Must be done after grub_machine_fini because map_key is used by
- *exit_boot_services.
- */
- mmap_size = find_mmap_size ();
- if (! mmap_size)
- return GRUB_ERR_OUT_OF_MEMORY;
- mmap_buf = grub_efi_allocate_any_pages (page_align (mmap_size) >> 12);
- if (! mmap_buf)
- return GRUB_ERR_OUT_OF_MEMORY;
-
- err = grub_efi_finish_boot_services (&mmap_size, mmap_buf, &map_key,
- &desc_size, &desc_version);
- if (err != GRUB_ERR_NONE)
- return err;
-
- return GRUB_ERR_NONE;
-}
diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c
index 9f43e41bb..b4f609d2d 100644
--- a/grub-core/loader/arm/linux.c
+++ b/grub-core/loader/arm/linux.c
@@ -290,15 +290,6 @@ linux_boot (void)
*/
linuxmain = (kernel_entry_t) linux_addr;

-#ifdef GRUB_MACHINE_EFI
- {
- grub_err_t err;
- err = grub_efi_prepare_platform();
- if (err != GRUB_ERR_NONE)
- return err;
- }
-#endif
-
grub_arm_disable_caches_mmu ();

linuxmain (0, machine_type, target_fdt);
@@ -317,13 +308,7 @@ linux_load (const char *filename, grub_file_t file)

size = grub_file_size (file);

-#ifdef GRUB_MACHINE_EFI
- linux_addr = (grub_addr_t) grub_efi_allocate_loader_memory (LINUX_PHYS_OFFSET, size);
- if (!linux_addr)
- return grub_errno;
-#else
linux_addr = LINUX_ADDRESS;
-#endif
grub_dprintf ("loader", "Loading Linux to 0x%08x\n",
(grub_addr_t) linux_addr);

@@ -428,20 +413,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),

size = grub_get_initrd_size (&initrd_ctx);

-#ifdef GRUB_MACHINE_EFI
- if (initrd_start)
- grub_efi_free_pages (initrd_start,
- (initrd_end - initrd_start + 0xfff) >> 12);
- initrd_start = (grub_addr_t) grub_efi_allocate_loader_memory (LINUX_INITRD_PHYS_OFFSET, size);
-
- if (!initrd_start)
- {
- grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
- goto fail;
- }
-#else
initrd_start = LINUX_INITRD_ADDRESS;
-#endif

grub_dprintf ("loader", "Loading initrd to 0x%08x\n",
(grub_addr_t) initrd_start);
diff --git a/include/grub/arm/efi/loader.h b/include/grub/arm/efi/loader.h
deleted file mode 100644
index 4bab18e83..000000000
--- a/include/grub/arm/efi/loader.h
+++ /dev/null
@@ -1,26 +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_LOADER_MACHINE_HEADER
-#define GRUB_LOADER_MACHINE_HEADER 1
-
-grub_err_t EXPORT_FUNC (grub_efi_prepare_platform) (void);
-void * EXPORT_FUNC (grub_efi_allocate_loader_memory) (grub_uint32_t min_offset,
- grub_uint32_t size);
-
-#endif /* ! GRUB_LOADER_MACHINE_HEADER */
diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index 54658af1b..9a4a5fb7e 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -46,20 +46,6 @@ struct linux_arm_kernel_header {
# define LINUX_FDT_ADDRESS (LINUX_INITRD_ADDRESS - 0x10000)
# define grub_arm_firmware_get_boot_data grub_uboot_get_boot_data
# define grub_arm_firmware_get_machine_type grub_uboot_get_machine_type
-#elif defined GRUB_MACHINE_EFI
-# include <grub/efi/efi.h>
-# include <grub/arm/efi/loader.h>
-/* On UEFI platforms - load the images at the lowest available address not
- less than *_PHYS_OFFSET from the first available memory location. */
-# define LINUX_PHYS_OFFSET (0x00008000)
-# define LINUX_INITRD_PHYS_OFFSET (LINUX_PHYS_OFFSET + 0x02000000)
-# define LINUX_FDT_PHYS_OFFSET (LINUX_INITRD_PHYS_OFFSET - 0x10000)
-# define grub_arm_firmware_get_boot_data (grub_addr_t)grub_efi_get_firmware_fdt
-static inline grub_uint32_t
-grub_arm_firmware_get_machine_type (void)
-{
- return GRUB_ARM_MACHINE_TYPE_FDT;
-}
#elif defined (GRUB_MACHINE_COREBOOT)
#include <grub/fdtbus.h>
#include <grub/arm/coreboot/kernel.h>
@@ -78,6 +64,4 @@ grub_arm_firmware_get_machine_type (void)
}
#endif

-#define FDT_ADDITIONAL_ENTRIES_SIZE 0x300
-
#endif /* ! GRUB_ARM_LINUX_HEADER */
--
2.11.0
Daniel Kiper
2018-07-06 15:53:33 UTC
Permalink
Post by Leif Lindholm
The 32-bit arm efi port now shares the 64-bit linux loader, so delete
the now unused bits from the 32-bit linux loader.
This in turn leaves the grub-core/kern/arm/efi/misc.c unused, so
delete that too.
LGTM.

Daniel
Leif Lindholm
2018-06-27 17:17:18 UTC
Permalink
Switch over to the EFI-stub aware arm64 loader for 32-bit ARM platforms.

This *WILL* stop non-efistub Linux kernels from booting on arm-efi.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
grub-core/Makefile.core.def | 7 ++++---
grub-core/kern/efi/mm.c | 2 +-
include/grub/arm/linux.h | 5 +++++
include/grub/efi/efi.h | 2 --
4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index fc4767f19..9590e87d9 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -229,7 +229,6 @@ kernel = {
ia64_efi = kern/ia64/cache.c;

arm_efi = kern/arm/efi/init.c;
- arm_efi = kern/arm/efi/misc.c;
arm_efi = kern/efi/fdt.c;

arm64_efi = kern/arm64/efi/init.c;
@@ -1693,7 +1692,9 @@ module = {
powerpc_ieee1275 = loader/powerpc/ieee1275/linux.c;
sparc64_ieee1275 = loader/sparc64/ieee1275/linux.c;
ia64_efi = loader/ia64/efi/linux.c;
- arm = loader/arm/linux.c;
+ arm_coreboot = loader/arm/linux.c;
+ arm_efi = loader/arm64/linux.c;
+ arm_uboot = loader/arm/linux.c;
arm64 = loader/arm64/linux.c;
common = loader/linux.c;
common = lib/cmdline.c;
@@ -1702,7 +1703,7 @@ module = {

module = {
name = fdt;
- arm64 = loader/efi/fdt.c;
+ efi = loader/efi/fdt.c;
common = lib/fdt.c;
enable = fdt;
};
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 10ffa2c9b..b6a3f40f7 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -630,7 +630,7 @@ grub_efi_mm_init (void)
2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
}

-#if defined (__aarch64__)
+#if defined (__aarch64__) || defined (__arm__)
grub_err_t
grub_efi_get_ram_base(grub_addr_t *base_addr)
{
diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index cceb9c4a9..54658af1b 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -34,6 +34,11 @@ struct linux_arm_kernel_header {
grub_uint32_t hdr_offset;
};

+#if defined(__arm__)
+# define LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM_MAGIC_SIGNATURE
+# define linux_armxx_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 5888bf6ac..e75c730c0 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -92,8 +92,6 @@ extern void (*EXPORT_VAR(grub_efi_net_config)) (grub_efi_handle_t hnd,

#if defined(__arm__) || defined(__aarch64__)
void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
-#endif
-#if defined(__aarch64__)
grub_err_t EXPORT_FUNC(grub_efi_get_ram_base)(grub_addr_t *);
#include <grub/cpu/linux.h>
grub_err_t grub_efi_linux_check_image(struct linux_armxx_kernel_header *lh);
--
2.11.0
Daniel Kiper
2018-07-06 15:48:21 UTC
Permalink
Post by Leif Lindholm
Switch over to the EFI-stub aware arm64 loader for 32-bit ARM platforms.
Hmmm... Does it mean that ARM64 EFI stub can work on 32-bit ARM platforms?

Daniel
Leif Lindholm
2018-07-06 16:30:59 UTC
Permalink
Post by Daniel Kiper
Post by Leif Lindholm
Switch over to the EFI-stub aware arm64 loader for 32-bit ARM platforms.
Hmmm... Does it mean that ARM64 EFI stub can work on 32-bit ARM platforms?
No, the architecture explicitly prevents it.

But they share much of the code in the kernel, so the linux loader can
share most of the code in grub.

/
Leif
Daniel Kiper
2018-07-06 17:00:57 UTC
Permalink
Post by Leif Lindholm
Post by Daniel Kiper
Post by Leif Lindholm
Switch over to the EFI-stub aware arm64 loader for 32-bit ARM platforms.
Hmmm... Does it mean that ARM64 EFI stub can work on 32-bit ARM platforms?
No, the architecture explicitly prevents it.
But they share much of the code in the kernel, so the linux loader can
share most of the code in grub.
Ahhh... OK then. However, please tell us this in the commit message
because at first sight it is not clear.

Daniel
Leif Lindholm
2018-06-27 17:17:15 UTC
Permalink
There are several implementations of this function in the tree.
Add a central version in grub-core/efi/mm.c.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
grub-core/kern/efi/mm.c | 20 ++++++++++++++++++++
include/grub/efi/efi.h | 1 +
2 files changed, 21 insertions(+)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index c48e9b5c7..fd39d23b4 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -286,6 +286,26 @@ grub_efi_finish_boot_services (grub_efi_uintn_t *outbuf_size, void *outbuf,
return GRUB_ERR_NONE;
}

+/* To obtain the UEFI memory map, we must pass a buffer of sufficient size
+ to hold the entire map. This function returns a sane start value for
+ buffer size. */
+grub_efi_uintn_t
+grub_efi_find_mmap_size (void)
+{
+ grub_efi_uintn_t mmap_size = 0;
+ grub_efi_uintn_t desc_size;
+
+ if (grub_efi_get_memory_map (&mmap_size, NULL, NULL, &desc_size, 0) < 0)
+ {
+ grub_error (GRUB_ERR_IO, "cannot get EFI memory map size");
+ return 0;
+ }
+
+ /* Add an extra page, since UEFI can alter the memory map itself on
+ callbacks or explicit calls, including console output. */
+ return ALIGN_UP (mmap_size + GRUB_EFI_PAGE_SIZE, GRUB_EFI_PAGE_SIZE);
+}
+
/* Get the memory map as defined in the EFI spec. Return 1 if successful,
return 0 if partial, or return -1 if an error occurs. */
int
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index c996913e5..1021273c1 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -49,6 +49,7 @@ void *
EXPORT_FUNC(grub_efi_allocate_any_pages) (grub_efi_uintn_t pages);
void EXPORT_FUNC(grub_efi_free_pages) (grub_efi_physical_address_t address,
grub_efi_uintn_t pages);
+grub_efi_uintn_t EXPORT_FUNC(grub_efi_find_mmap_size) (void);
int
EXPORT_FUNC(grub_efi_get_memory_map) (grub_efi_uintn_t *memory_map_size,
grub_efi_memory_descriptor_t *memory_map,
--
2.11.0
Daniel Kiper
2018-07-06 15:00:45 UTC
Permalink
Post by Leif Lindholm
There are several implementations of this function in the tree.
Add a central version in grub-core/efi/mm.c.
I am happy with the code itself but I am not sure why you are
not replacing these "several implementations of this function"
in the existing code. I would like to see a patch which does that.

Daniel
Leif Lindholm
2018-07-06 16:21:17 UTC
Permalink
Post by Daniel Kiper
Post by Leif Lindholm
There are several implementations of this function in the tree.
Add a central version in grub-core/efi/mm.c.
I am happy with the code itself but I am not sure why you are
not replacing these "several implementations of this function"
in the existing code. I would like to see a patch which does that.
I am happy to submit further cleanup patches once this set gets in,
but this is a fundamental change in behaviour compared to those other
bits of code (which I do not exercise or use). It is also a
fundamental change in behaviour compared to the previous version
submitted. I feel a temporary duplication minimises risk of further
disruption.

Best Regards,

Leif
Daniel Kiper
2018-07-06 16:55:51 UTC
Permalink
Post by Leif Lindholm
Post by Daniel Kiper
Post by Leif Lindholm
There are several implementations of this function in the tree.
Add a central version in grub-core/efi/mm.c.
I am happy with the code itself but I am not sure why you are
not replacing these "several implementations of this function"
in the existing code. I would like to see a patch which does that.
I am happy to submit further cleanup patches once this set gets in,
but this is a fundamental change in behaviour compared to those other
bits of code (which I do not exercise or use). It is also a
fundamental change in behaviour compared to the previous version
submitted. I feel a temporary duplication minimises risk of further
disruption.
OK, I will take this patch if you promise to fix this in separate patch
series later. Later means weeks not years. Of course I am happy to help
with this.

Daniel
Leif Lindholm
2018-07-06 17:13:30 UTC
Permalink
Post by Daniel Kiper
Post by Leif Lindholm
Post by Daniel Kiper
Post by Leif Lindholm
There are several implementations of this function in the tree.
Add a central version in grub-core/efi/mm.c.
I am happy with the code itself but I am not sure why you are
not replacing these "several implementations of this function"
in the existing code. I would like to see a patch which does that.
I am happy to submit further cleanup patches once this set gets in,
but this is a fundamental change in behaviour compared to those other
bits of code (which I do not exercise or use). It is also a
fundamental change in behaviour compared to the previous version
submitted. I feel a temporary duplication minimises risk of further
disruption.
OK, I will take this patch if you promise to fix this in separate patch
series later. Later means weeks not years. Of course I am happy to help
with this.
I promise to send an RFC out next week.
After that, I'll be basically offline for two weeks.

What I cannot promise is validating my patches outside of QEMU/Ovmf,
or figuring out special workarounds if it turns out this new mechanism
does not work on some (broken) UEFI implementations. If we come across
anything like that, then I would suggest we keep the duplication.

The RFC will remove all caching of the memory map, which always felt a
bit dubious to me.

/
Leif
Daniel Kiper
2018-07-06 18:26:36 UTC
Permalink
Post by Leif Lindholm
Post by Daniel Kiper
Post by Leif Lindholm
Post by Daniel Kiper
Post by Leif Lindholm
There are several implementations of this function in the tree.
Add a central version in grub-core/efi/mm.c.
I am happy with the code itself but I am not sure why you are
not replacing these "several implementations of this function"
in the existing code. I would like to see a patch which does that.
I am happy to submit further cleanup patches once this set gets in,
but this is a fundamental change in behaviour compared to those other
bits of code (which I do not exercise or use). It is also a
fundamental change in behaviour compared to the previous version
submitted. I feel a temporary duplication minimises risk of further
disruption.
OK, I will take this patch if you promise to fix this in separate patch
series later. Later means weeks not years. Of course I am happy to help
with this.
I promise to send an RFC out next week.
After that, I'll be basically offline for two weeks.
What I cannot promise is validating my patches outside of QEMU/Ovmf,
or figuring out special workarounds if it turns out this new mechanism
does not work on some (broken) UEFI implementations. If we come across
anything like that, then I would suggest we keep the duplication.
The RFC will remove all caching of the memory map, which always felt a
bit dubious to me.
That works for me. Thank you!

Have a nice weekend,

Daniel

Leif Lindholm
2018-06-27 17:17:20 UTC
Permalink
The 32-bit arm Linux kernel is built as a zImage, which self-decompresses
down to near start of RAM. In order for an initrd/initramfs to be
accessible, it needs to be placed within the first ~768MB of RAM.
The initrd loader built into the kernel EFI stub restricts this down to
512MB for simplicity - so enable the same restriction in grub.

For arm64, the requirement is within a 1GB aligned 32GB window also
covering the (runtime) kernel image. Since the EFI stub loader itself
will attempt to relocate to near start of RAM, force initrd to be loaded
completely within the first 32GB of RAM.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
grub-core/loader/arm64/linux.c | 39 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 577fbda54..0d550182f 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -193,6 +193,42 @@ grub_linux_unload (void)
return GRUB_ERR_NONE;
}

+/*
+ * This function returns a pointer to a legally allocated initrd buffer,
+ * or NULL if unsuccessful
+ */
+static void *
+allocate_initrd_mem (int initrd_pages)
+{
+ grub_addr_t max_addr;
+
+ if (grub_efi_get_ram_base (&max_addr) != GRUB_ERR_NONE)
+ return NULL;
+
+ /*
+ * As per linux/Documentation/arm/Booting
+ * ARM initrd needs to be covered by kernel linear mapping,
+ * so place it in the first 512MB of DRAM.
+ *
+ * As per linux/Documentation/arm64/booting.txt
+ * ARM64 initrd needs to be contained entirely within a 1GB aligned window
+ * of up to 32GB of size that covers the kernel image as well.
+ * Since the EFI stub loader will attempt to load the kernel near start of
+ * RAM, place the buffer in the first 32GB of RAM.
+ */
+#ifdef __arm__
+#define INITRD_MAX_ADDRESS_OFFSET (512U * 1024 * 1024)
+#else /* __aarch64__ */
+#define INITRD_MAX_ADDRESS_OFFSET (32ULL * 1024 * 1024 * 1024)
+#endif
+
+ max_addr += INITRD_MAX_ADDRESS_OFFSET - 1;
+
+ return grub_efi_allocate_pages_real (max_addr, initrd_pages,
+ GRUB_EFI_ALLOCATE_MAX_ADDRESS,
+ GRUB_EFI_LOADER_DATA);
+}
+
static grub_err_t
grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
int argc, char *argv[])
@@ -221,7 +257,8 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
grub_dprintf ("linux", "Loading initrd\n");

initrd_pages = (GRUB_EFI_BYTES_TO_PAGES (initrd_size));
- initrd_mem = grub_efi_allocate_any_pages (initrd_pages);
+ initrd_mem = allocate_initrd_mem (initrd_pages);
+
if (!initrd_mem)
{
grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
--
2.11.0
Daniel Kiper
2018-07-06 16:00:11 UTC
Permalink
Post by Leif Lindholm
The 32-bit arm Linux kernel is built as a zImage, which self-decompresses
down to near start of RAM. In order for an initrd/initramfs to be
accessible, it needs to be placed within the first ~768MB of RAM.
The initrd loader built into the kernel EFI stub restricts this down to
512MB for simplicity - so enable the same restriction in grub.
For arm64, the requirement is within a 1GB aligned 32GB window also
covering the (runtime) kernel image. Since the EFI stub loader itself
will attempt to relocate to near start of RAM, force initrd to be loaded
completely within the first 32GB of RAM.
---
grub-core/loader/arm64/linux.c | 39 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 577fbda54..0d550182f 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -193,6 +193,42 @@ grub_linux_unload (void)
return GRUB_ERR_NONE;
}
+/*
+ * This function returns a pointer to a legally allocated initrd buffer,
+ * or NULL if unsuccessful
+ */
+static void *
+allocate_initrd_mem (int initrd_pages)
+{
+ grub_addr_t max_addr;
+
+ if (grub_efi_get_ram_base (&max_addr) != GRUB_ERR_NONE)
+ return NULL;
+
+ /*
+ * As per linux/Documentation/arm/Booting
+ * ARM initrd needs to be covered by kernel linear mapping,
+ * so place it in the first 512MB of DRAM.
+ *
+ * As per linux/Documentation/arm64/booting.txt
+ * ARM64 initrd needs to be contained entirely within a 1GB aligned window
+ * of up to 32GB of size that covers the kernel image as well.
+ * Since the EFI stub loader will attempt to load the kernel near start of
+ * RAM, place the buffer in the first 32GB of RAM.
+ */
+#ifdef __arm__
+#define INITRD_MAX_ADDRESS_OFFSET (512U * 1024 * 1024)
+#else /* __aarch64__ */
+#define INITRD_MAX_ADDRESS_OFFSET (32ULL * 1024 * 1024 * 1024)
+#endif
May I ask you to move these definitions together with the comment before
the allocate_initrd_mem(). Or even to the beginning of the file just
behind GRUB_MOD_LICENSE(). I prefer the latter. Though I am not
insisting on it.

Daniel
Leif Lindholm
2018-07-06 16:33:54 UTC
Permalink
Post by Daniel Kiper
Post by Leif Lindholm
The 32-bit arm Linux kernel is built as a zImage, which self-decompresses
down to near start of RAM. In order for an initrd/initramfs to be
accessible, it needs to be placed within the first ~768MB of RAM.
The initrd loader built into the kernel EFI stub restricts this down to
512MB for simplicity - so enable the same restriction in grub.
For arm64, the requirement is within a 1GB aligned 32GB window also
covering the (runtime) kernel image. Since the EFI stub loader itself
will attempt to relocate to near start of RAM, force initrd to be loaded
completely within the first 32GB of RAM.
---
grub-core/loader/arm64/linux.c | 39 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 577fbda54..0d550182f 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -193,6 +193,42 @@ grub_linux_unload (void)
return GRUB_ERR_NONE;
}
+/*
+ * This function returns a pointer to a legally allocated initrd buffer,
+ * or NULL if unsuccessful
+ */
+static void *
+allocate_initrd_mem (int initrd_pages)
+{
+ grub_addr_t max_addr;
+
+ if (grub_efi_get_ram_base (&max_addr) != GRUB_ERR_NONE)
+ return NULL;
+
+ /*
+ * As per linux/Documentation/arm/Booting
+ * ARM initrd needs to be covered by kernel linear mapping,
+ * so place it in the first 512MB of DRAM.
+ *
+ * As per linux/Documentation/arm64/booting.txt
+ * ARM64 initrd needs to be contained entirely within a 1GB aligned window
+ * of up to 32GB of size that covers the kernel image as well.
+ * Since the EFI stub loader will attempt to load the kernel near start of
+ * RAM, place the buffer in the first 32GB of RAM.
+ */
+#ifdef __arm__
+#define INITRD_MAX_ADDRESS_OFFSET (512U * 1024 * 1024)
+#else /* __aarch64__ */
+#define INITRD_MAX_ADDRESS_OFFSET (32ULL * 1024 * 1024 * 1024)
+#endif
May I ask you to move these definitions together with the comment before
the allocate_initrd_mem(). Or even to the beginning of the file just
behind GRUB_MOD_LICENSE(). I prefer the latter. Though I am not
insisting on it.
I'm OK with either. I stuck it here because it is information that is
_only_ relevant to this particular piece of code. So my preference
leans the other way. Will update for v4.

/
Leif
Leif Lindholm
2018-06-27 17:17:17 UTC
Permalink
In preparation for using the linux loader for 32-bit and 64-bit platforms,
rename grub_arm64*/GRUB_ARM64* to grub_efi*/GRUB_EFI*.

Move prototypes for now-common functions to efi/efi.h.

Signed-off-by: Leif Lindholm <***@linaro.org>
---
grub-core/loader/arm64/linux.c | 14 +++++++-------
grub-core/loader/arm64/xen_boot.c | 10 +++++-----
include/grub/arm64/linux.h | 9 ++++-----
include/grub/efi/efi.h | 4 ++++
4 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index ebe1e730d..577fbda54 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -48,9 +48,9 @@ static grub_addr_t initrd_start;
static grub_addr_t initrd_end;

grub_err_t
-grub_arm64_uefi_check_image (struct linux_arm64_kernel_header * lh)
+grub_efi_linux_check_image (struct linux_armxx_kernel_header * lh)
{
- if (lh->magic != GRUB_LINUX_ARM64_MAGIC_SIGNATURE)
+ if (lh->magic != LINUX_ARMXX_MAGIC_SIGNATURE)
return grub_error(GRUB_ERR_BAD_OS, "invalid magic number");

if ((lh->code0 & 0xffff) != GRUB_PE32_MAGIC)
@@ -109,7 +109,7 @@ failure:
}

grub_err_t
-grub_arm64_uefi_boot_image (grub_addr_t addr, grub_size_t size, char *args)
+grub_efi_linux_boot_image (grub_addr_t addr, grub_size_t size, char *args)
{
grub_efi_memory_mapped_device_path_t *mempath;
grub_efi_handle_t image_handle;
@@ -172,8 +172,8 @@ grub_linux_boot (void)
if (finalize_params_linux () != GRUB_ERR_NONE)
return grub_errno;

- return (grub_arm64_uefi_boot_image((grub_addr_t)kernel_addr,
- kernel_size, linux_args));
+ return (grub_efi_linux_boot_image((grub_addr_t)kernel_addr,
+ kernel_size, linux_args));
}

static grub_err_t
@@ -249,7 +249,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
int argc, char *argv[])
{
grub_file_t file = 0;
- struct linux_arm64_kernel_header lh;
+ struct linux_armxx_kernel_header lh;

grub_dl_ref (my_mod);

@@ -268,7 +268,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
if (grub_file_read (file, &lh, sizeof (lh)) < (long) sizeof (lh))
return grub_errno;

- if (grub_arm64_uefi_check_image (&lh) != GRUB_ERR_NONE)
+ if (grub_efi_linux_check_image (&lh) != GRUB_ERR_NONE)
goto fail;

grub_loader_unset();
diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64/xen_boot.c
index 258264c79..b86704757 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -265,9 +265,9 @@ xen_boot (void)
if (err)
return err;

- return grub_arm64_uefi_boot_image (xen_hypervisor->start,
- xen_hypervisor->size,
- xen_hypervisor->cmdline);
+ return grub_efi_linux_boot_image (xen_hypervisor->start,
+ xen_hypervisor->size,
+ xen_hypervisor->cmdline);
}

static void
@@ -468,8 +468,8 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)),

if (grub_file_read (file, &sh, sizeof (sh)) != (long) sizeof (sh))
goto fail;
- if (grub_arm64_uefi_check_image
- ((struct linux_arm64_kernel_header *) &sh) != GRUB_ERR_NONE)
+ if (grub_efi_linux_check_image
+ ((struct linux_armxx_kernel_header *) &sh) != GRUB_ERR_NONE)
goto fail;
grub_file_seek (file, 0);

diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
index b06347624..4c0d8d239 100644
--- a/include/grub/arm64/linux.h
+++ b/include/grub/arm64/linux.h
@@ -19,8 +19,6 @@
#ifndef GRUB_ARM64_LINUX_HEADER
#define GRUB_ARM64_LINUX_HEADER 1

-#include <grub/efi/efi.h>
-
#define GRUB_LINUX_ARM64_MAGIC_SIGNATURE 0x644d5241 /* 'ARM\x64' */

/* From linux/Documentation/arm64/booting.txt */
@@ -38,8 +36,9 @@ struct linux_arm64_kernel_header
grub_uint32_t hdr_offset; /* Offset of PE/COFF header */
};

-grub_err_t grub_arm64_uefi_check_image (struct linux_arm64_kernel_header *lh);
-grub_err_t grub_arm64_uefi_boot_image (grub_addr_t addr, grub_size_t size,
- char *args);
+#if defined(__aarch64__)
+# define LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM64_MAGIC_SIGNATURE
+# define linux_armxx_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 57db74b57..5888bf6ac 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -95,6 +95,10 @@ void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
#endif
#if defined(__aarch64__)
grub_err_t EXPORT_FUNC(grub_efi_get_ram_base)(grub_addr_t *);
+#include <grub/cpu/linux.h>
+grub_err_t grub_efi_linux_check_image(struct linux_armxx_kernel_header *lh);
+grub_err_t grub_efi_linux_boot_image(grub_addr_t addr, grub_size_t size,
+ char *args);
#endif

grub_addr_t grub_efi_modules_addr (void);
--
2.11.0
Daniel Kiper
2018-07-06 15:28:17 UTC
Permalink
Post by Leif Lindholm
In preparation for using the linux loader for 32-bit and 64-bit platforms,
rename grub_arm64*/GRUB_ARM64* to grub_efi*/GRUB_EFI*.
Move prototypes for now-common functions to efi/efi.h.
---
grub-core/loader/arm64/linux.c | 14 +++++++-------
grub-core/loader/arm64/xen_boot.c | 10 +++++-----
include/grub/arm64/linux.h | 9 ++++-----
include/grub/efi/efi.h | 4 ++++
4 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index ebe1e730d..577fbda54 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -48,9 +48,9 @@ static grub_addr_t initrd_start;
static grub_addr_t initrd_end;
grub_err_t
-grub_arm64_uefi_check_image (struct linux_arm64_kernel_header * lh)
+grub_efi_linux_check_image (struct linux_armxx_kernel_header * lh)
s/grub_efi_linux_check_image/grub_armxx_efi_linux_check_image/

This and functions below seems to be ARM specific, so, I would
add "_armxx_" to the name as above. Same below please.
Post by Leif Lindholm
{
- if (lh->magic != GRUB_LINUX_ARM64_MAGIC_SIGNATURE)
+ if (lh->magic != LINUX_ARMXX_MAGIC_SIGNATURE)
s/LINUX_ARMXX_MAGIC_SIGNATURE/GRUB_LINUX_ARMXX_MAGIC_SIGNATURE/g

Hmmm... Why do you drop "GRUB_" prefix?
Post by Leif Lindholm
return grub_error(GRUB_ERR_BAD_OS, "invalid magic number");
if ((lh->code0 & 0xffff) != GRUB_PE32_MAGIC)
}
grub_err_t
-grub_arm64_uefi_boot_image (grub_addr_t addr, grub_size_t size, char *args)
+grub_efi_linux_boot_image (grub_addr_t addr, grub_size_t size, char *args)
s/grub_efi_linux_boot_image/grub_armxx_efi_linux_boot_image/g

...and below please...

Daniel
Leif Lindholm
2018-07-06 16:28:12 UTC
Permalink
Post by Daniel Kiper
Post by Leif Lindholm
In preparation for using the linux loader for 32-bit and 64-bit platforms,
rename grub_arm64*/GRUB_ARM64* to grub_efi*/GRUB_EFI*.
Move prototypes for now-common functions to efi/efi.h.
---
grub-core/loader/arm64/linux.c | 14 +++++++-------
grub-core/loader/arm64/xen_boot.c | 10 +++++-----
include/grub/arm64/linux.h | 9 ++++-----
include/grub/efi/efi.h | 4 ++++
4 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index ebe1e730d..577fbda54 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -48,9 +48,9 @@ static grub_addr_t initrd_start;
static grub_addr_t initrd_end;
grub_err_t
-grub_arm64_uefi_check_image (struct linux_arm64_kernel_header * lh)
+grub_efi_linux_check_image (struct linux_armxx_kernel_header * lh)
s/grub_efi_linux_check_image/grub_armxx_efi_linux_check_image/
This and functions below seems to be ARM specific, so, I would
add "_armxx_" to the name as above. Same below please.
Yeah, fair point.
Post by Daniel Kiper
Post by Leif Lindholm
{
- if (lh->magic != GRUB_LINUX_ARM64_MAGIC_SIGNATURE)
+ if (lh->magic != LINUX_ARMXX_MAGIC_SIGNATURE)
s/LINUX_ARMXX_MAGIC_SIGNATURE/GRUB_LINUX_ARMXX_MAGIC_SIGNATURE/g
Hmmm... Why do you drop "GRUB_" prefix?
I stumbled a bit around here tbh.
My rationale was a bit like for struct linux_*_kernel_header instead
of grub_linux_*_kernel_header.

I don't mind changing.
Post by Daniel Kiper
Post by Leif Lindholm
return grub_error(GRUB_ERR_BAD_OS, "invalid magic number");
if ((lh->code0 & 0xffff) != GRUB_PE32_MAGIC)
}
grub_err_t
-grub_arm64_uefi_boot_image (grub_addr_t addr, grub_size_t size, char *args)
+grub_efi_linux_boot_image (grub_addr_t addr, grub_size_t size, char *args)
s/grub_efi_linux_boot_image/grub_armxx_efi_linux_boot_image/g
...and below please...
Sure.

/
Leif
Loading...