[PATCH v3 0/7] Add LoadFile2 and riscv Linux loader

[PATCH v3 0/7] Add LoadFile2 and riscv Linux loader

[Nikita Ermakov]

Hello,

This is the third version of the patch series. In v3 the series was rebased
on top of the master branch.

This series contains patches to add support for LoadFile2 protocol to load
initrd on EFI systems. Also it contains patches to load Linux kernel with EFI
stub on riscv platforms and unites arm and riscv codes together into common
loader code for EFI systems.

Ard Biesheuvel (3):
  efi: add definition of LoadFile2 protocol
  efi: implemented LoadFile2 initrd loading protocol for Linux
  linux: ignore FDT unless we need to modify it

Atish Patra (3):
  loader: Move arm64 linux loader to common code
  RISC-V: Update image header
  RISC-V: Use common linux loader

Nikita Ermakov (1):
  loader: drop argv[] argument in grub_initrd_load()

 grub-core/Makefile.core.def               |   8 +-
 grub-core/commands/efi/lsefi.c            |   1 +
 grub-core/loader/arm/linux.c              |   2 +-
 grub-core/loader/efi/fdt.c                |   7 +-
 grub-core/loader/{arm64 => efi}/linux.c   | 143 +++++++++++++++++++---
 grub-core/loader/i386/linux.c             |   2 +-
 grub-core/loader/i386/pc/linux.c          |   2 +-
 grub-core/loader/i386/xen.c               |   3 +-
 grub-core/loader/ia64/efi/linux.c         |   2 +-
 grub-core/loader/linux.c                  |   4 +-
 grub-core/loader/mips/linux.c             |   2 +-
 grub-core/loader/powerpc/ieee1275/linux.c |   2 +-
 grub-core/loader/riscv/linux.c            |  59 ---------
 grub-core/loader/sparc64/ieee1275/linux.c |   2 +-
 include/grub/arm/linux.h                  |   2 +-
 include/grub/arm64/linux.h                |   2 +-
 include/grub/efi/api.h                    |  15 +++
 include/grub/linux.h                      |   2 +-
 include/grub/riscv32/linux.h              |  16 +--
 include/grub/riscv64/linux.h              |  16 +--
 20 files changed, 185 insertions(+), 107 deletions(-)
 rename grub-core/loader/{arm64 => efi}/linux.c (72%)
 delete mode 100644 grub-core/loader/riscv/linux.c

-- 
2.33.1

[PATCH v3 1/7] loader: drop argv[] argument in grub_initrd_load()

[Nikita Ermakov]

In the case of an error grub_initrd_load() uses argv[] to print the
filename that caused the error. It is also possible to obtain the
filename from the file handles and there is no need to duplicate that
information in argv[], so let's drop it.

Signed-off-by: Nikita Ermakov <arei@altlinux.org>
---
 grub-core/loader/arm/linux.c              | 2 +-
 grub-core/loader/arm64/linux.c            | 2 +-
 grub-core/loader/i386/linux.c             | 2 +-
 grub-core/loader/i386/pc/linux.c          | 2 +-
 grub-core/loader/i386/xen.c               | 3 +--
 grub-core/loader/ia64/efi/linux.c         | 2 +-
 grub-core/loader/linux.c                  | 4 ++--
 grub-core/loader/mips/linux.c             | 2 +-
 grub-core/loader/powerpc/ieee1275/linux.c | 2 +-
 grub-core/loader/sparc64/ieee1275/linux.c | 2 +-
 include/grub/linux.h                      | 2 +-
 11 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/grub-core/loader/arm/linux.c b/grub-core/loader/arm/linux.c
index ed23dc71e..1f7ab7578 100644
--- a/grub-core/loader/arm/linux.c
+++ b/grub-core/loader/arm/linux.c
@@ -422,7 +422,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
   grub_dprintf ("loader", "Loading initrd to 0x%08x\n",
 		(grub_addr_t) initrd_start);
 
-  if (grub_initrd_load (&initrd_ctx, argv, (void *) initrd_start))
+  if (grub_initrd_load (&initrd_ctx, (void *) initrd_start))
     goto fail;
 
   initrd_end = initrd_start + size;
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index ef3e9f944..aed7a200b 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -266,7 +266,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
       goto fail;
     }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
     goto fail;
 
   initrd_start = (grub_addr_t) initrd_mem;
diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index 9f74a96b1..f30a1586a 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -1107,7 +1107,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
     initrd_mem_target = get_physical_target_address (ch);
   }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
     goto fail;
 
   grub_dprintf ("linux", "Initrd, addr=0x%x, size=0x%x\n",
diff --git a/grub-core/loader/i386/pc/linux.c b/grub-core/loader/i386/pc/linux.c
index 2a2995201..efeeeb206 100644
--- a/grub-core/loader/i386/pc/linux.c
+++ b/grub-core/loader/i386/pc/linux.c
@@ -462,7 +462,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
     initrd_addr = get_physical_target_address (ch);
   }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_chunk))
+  if (grub_initrd_load (&initrd_ctx, initrd_chunk))
     goto fail;
 
   lh->ramdisk_image = initrd_addr;
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index cd24874ca..3b856e842 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -809,8 +809,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
       if (err)
 	goto fail;
 
-      if (grub_initrd_load (&initrd_ctx, argv,
-			    get_virtual_current_address (ch)))
+      if (grub_initrd_load (&initrd_ctx, get_virtual_current_address (ch)))
 	goto fail;
     }
 
diff --git a/grub-core/loader/ia64/efi/linux.c b/grub-core/loader/ia64/efi/linux.c
index 7987fd1ba..8873b7a55 100644
--- a/grub-core/loader/ia64/efi/linux.c
+++ b/grub-core/loader/ia64/efi/linux.c
@@ -563,7 +563,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
   grub_dprintf ("linux", "[addr=0x%lx, size=0x%lx]\n",
 		(grub_uint64_t) initrd_mem, initrd_size);
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_mem))
+  if (grub_initrd_load (&initrd_ctx, initrd_mem))
     goto fail;
  fail:
   grub_initrd_close (&initrd_ctx);
diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index 3fe390f17..d19df670e 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -271,7 +271,7 @@ grub_initrd_close (struct grub_linux_initrd_context *initrd_ctx)
 
 grub_err_t
 grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
-		  char *argv[], void *target)
+		  void *target)
 {
   grub_uint8_t *ptr = target;
   int i;
@@ -317,7 +317,7 @@ grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
 	{
 	  if (!grub_errno)
 	    grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file %s"),
-			argv[i]);
+			initrd_ctx->components[i].file->name);
 	  grub_initrd_close (initrd_ctx);
 	  return grub_errno;
 	}
diff --git a/grub-core/loader/mips/linux.c b/grub-core/loader/mips/linux.c
index e4ed95921..94594721d 100644
--- a/grub-core/loader/mips/linux.c
+++ b/grub-core/loader/mips/linux.c
@@ -452,7 +452,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
     initrd_dest = get_physical_target_address (ch) | 0x80000000;
   }
 
-  if (grub_initrd_load (&initrd_ctx, argv, initrd_src))
+  if (grub_initrd_load (&initrd_ctx, initrd_src))
     goto fail;
 
 #ifdef GRUB_MACHINE_MIPS_QEMU_MIPS
diff --git a/grub-core/loader/powerpc/ieee1275/linux.c b/grub-core/loader/powerpc/ieee1275/linux.c
index 6fdd86313..e6d071508 100644
--- a/grub-core/loader/powerpc/ieee1275/linux.c
+++ b/grub-core/loader/powerpc/ieee1275/linux.c
@@ -349,7 +349,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
 
   grub_dprintf ("loader", "Loading initrd at 0x%x, size 0x%x\n", addr, size);
 
-  if (grub_initrd_load (&initrd_ctx, argv, (void *) addr))
+  if (grub_initrd_load (&initrd_ctx, (void *) addr))
     goto fail;
 
   initrd_addr = addr;
diff --git a/grub-core/loader/sparc64/ieee1275/linux.c b/grub-core/loader/sparc64/ieee1275/linux.c
index bb47ee0cc..ac2206f3c 100644
--- a/grub-core/loader/sparc64/ieee1275/linux.c
+++ b/grub-core/loader/sparc64/ieee1275/linux.c
@@ -413,7 +413,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
   grub_dprintf ("loader", "Loading initrd at vaddr 0x%lx, paddr 0x%lx, size 0x%lx\n",
 		addr, paddr, size);
 
-  if (grub_initrd_load (&initrd_ctx, argv, (void *) addr))
+  if (grub_initrd_load (&initrd_ctx, (void *) addr))
     goto fail;
 
   initrd_addr = addr;
diff --git a/include/grub/linux.h b/include/grub/linux.h
index 594a3f307..a96ac2048 100644
--- a/include/grub/linux.h
+++ b/include/grub/linux.h
@@ -21,4 +21,4 @@ grub_initrd_close (struct grub_linux_initrd_context *initrd_ctx);
 
 grub_err_t
 grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
-		  char *argv[], void *target);
+		  void *target);
-- 
2.33.1

[PATCH v3 2/7] efi: add definition of LoadFile2 protocol

[Nikita Ermakov]

From: Ard Biesheuvel <ard.biesheuvel@arm.com>

Incorporate the EFI_LOAD_FILE2_PROTOCOL GUID and C types from the
UEFI spec.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Nikita Ermakov <arei@altlinux.org>
---
 grub-core/commands/efi/lsefi.c |  1 +
 include/grub/efi/api.h         | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
index d1ce99af4..4085f5df2 100644
--- a/grub-core/commands/efi/lsefi.c
+++ b/grub-core/commands/efi/lsefi.c
@@ -55,6 +55,7 @@ struct known_protocol
     { GRUB_EFI_ABSOLUTE_POINTER_PROTOCOL_GUID, "absolute pointer" },
     { GRUB_EFI_DRIVER_BINDING_PROTOCOL_GUID, "EFI driver binding" },
     { GRUB_EFI_LOAD_FILE_PROTOCOL_GUID, "load file" },
+    { GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID, "load file2" },
     { GRUB_EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID, "simple FS" },
     { GRUB_EFI_TAPE_IO_PROTOCOL_GUID, "tape I/O" },
     { GRUB_EFI_UNICODE_COLLATION_PROTOCOL_GUID, "unicode collation" },
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index f1a52210c..3de0a7d12 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -149,6 +149,11 @@
     { 0x8E, 0x3F, 0x00, 0xA0, 0xC9, 0x69, 0x72, 0x3B } \
   }
 
+#define GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID \
+  { 0x4006c0c1, 0xfcb3, 0x403e, \
+    { 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d } \
+  }
+
 #define GRUB_EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID \
   { 0x0964e5b22, 0x6459, 0x11d2, \
     { 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b } \
@@ -1729,6 +1734,16 @@ struct grub_efi_rng_protocol
 };
 typedef struct grub_efi_rng_protocol grub_efi_rng_protocol_t;
 
+struct grub_efi_load_file2
+{
+  grub_efi_status_t (*load_file)(struct grub_efi_load_file2 *this,
+				 grub_efi_device_path_t *file_path,
+				 grub_efi_boolean_t boot_policy,
+				 grub_efi_uintn_t *buffer_size,
+				 void *buffer);
+};
+typedef struct grub_efi_load_file2 grub_efi_load_file2_t;
+
 #if (GRUB_TARGET_SIZEOF_VOID_P == 4) || defined (__ia64__) \
   || defined (__aarch64__) || defined (__MINGW64__) || defined (__CYGWIN__) \
   || defined(__riscv)
-- 
2.33.1

[PATCH v3 3/7] efi: implemented LoadFile2 initrd loading protocol for Linux

[Nikita Ermakov]

From: Ard Biesheuvel <ard.biesheuvel@arm.com>

Recent Linux kernels will invoke the LoadFile2 protocol installed on
a well-known vendor media path to load the initrd if it is exposed by
the firmware. Using this method is preferred for two reasons:
- the Linux kernel is in charge of allocating the memory, and so it can
  implement any placement policy it wants (given that these tend to
  change between kernel versions),
- it is no longer necessary to modify the device tree provided by the
  firmware.

So let's install this protocol when handling the 'initrd' command if
such a recent kernel was detected (based on the PE/COFF image version),
and defer loading the initrd contents until the point where the kernel
invokes the LoadFile2 protocol.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Nikita Ermakov <arei@altlinux.org>
---
 grub-core/loader/arm64/linux.c | 117 ++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 1 deletion(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index aed7a200b..6b03455d1 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -48,9 +48,18 @@ static grub_uint32_t cmdline_size;
 static grub_addr_t initrd_start;
 static grub_addr_t initrd_end;
 
+static struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
+static grub_efi_handle_t initrd_lf2_handle;
+static int initrd_use_loadfile2;
+static grub_efi_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
+static grub_efi_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
+
 grub_err_t
 grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
 {
+  struct grub_pe32_coff_header *coff_header;
+  struct grub_pe32_optional_header *optional_header;
+
   if (lh->magic != GRUB_LINUX_ARMXX_MAGIC_SIGNATURE)
     return grub_error(GRUB_ERR_BAD_OS, "invalid magic number");
 
@@ -61,6 +70,21 @@ grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
   grub_dprintf ("linux", "UEFI stub kernel:\n");
   grub_dprintf ("linux", "PE/COFF header @ %08x\n", lh->hdr_offset);
 
+  coff_header = (struct grub_pe32_coff_header *)((unsigned long)lh + lh->hdr_offset);
+  optional_header = (struct grub_pe32_optional_header *)(coff_header + 1);
+
+  /*
+   * Linux kernels built for any architecture are guaranteed to support the
+   * LoadFile2 based initrd loading protocol if the image version is >= 1.
+   */
+  if (optional_header->major_image_version >= 1)
+    initrd_use_loadfile2 = 1;
+   else
+    initrd_use_loadfile2 = 0;
+
+  grub_dprintf ("linux", "LoadFile2 initrd loading %sabled\n",
+		initrd_use_loadfile2 ? "en" : "dis");
+
   return GRUB_ERR_NONE;
 }
 
@@ -230,13 +254,86 @@ allocate_initrd_mem (int initrd_pages)
 				       GRUB_EFI_LOADER_DATA);
 }
 
+struct initrd_media_device_path {
+  grub_efi_vendor_media_device_path_t	vendor;
+  grub_efi_device_path_t		end;
+} GRUB_PACKED;
+
+#define LINUX_EFI_INITRD_MEDIA_GUID  \
+  { 0x5568e427, 0x68fc, 0x4f3d, \
+    { 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68 } \
+  }
+
+static struct initrd_media_device_path initrd_lf2_device_path = {
+  {
+    {
+      GRUB_EFI_MEDIA_DEVICE_PATH_TYPE,
+      GRUB_EFI_VENDOR_MEDIA_DEVICE_PATH_SUBTYPE,
+      sizeof(grub_efi_vendor_media_device_path_t),
+    },
+    LINUX_EFI_INITRD_MEDIA_GUID
+  }, {
+    GRUB_EFI_END_DEVICE_PATH_TYPE,
+    GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE,
+    sizeof(grub_efi_device_path_t)
+  }
+};
+
+static grub_efi_status_t
+grub_efi_initrd_load_file2(grub_efi_load_file2_t *this,
+                           grub_efi_device_path_t *device_path,
+                           grub_efi_boolean_t boot_policy,
+                           grub_efi_uintn_t *buffer_size,
+                           void *buffer);
+
+static grub_efi_load_file2_t initrd_lf2 = {
+  grub_efi_initrd_load_file2
+};
+
+static grub_efi_status_t
+grub_efi_initrd_load_file2(grub_efi_load_file2_t *this,
+			   grub_efi_device_path_t *device_path,
+			   grub_efi_boolean_t boot_policy,
+			   grub_efi_uintn_t *buffer_size,
+			   void *buffer)
+{
+  grub_efi_status_t status = GRUB_EFI_SUCCESS;
+  grub_efi_uintn_t initrd_size;
+
+  if (!this || this != &initrd_lf2 || !buffer_size)
+    return GRUB_EFI_INVALID_PARAMETER;
+
+  if (device_path->type != GRUB_EFI_END_DEVICE_PATH_TYPE ||
+      device_path->subtype != GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE)
+    return GRUB_EFI_NOT_FOUND;
+
+  if (boot_policy)
+    return GRUB_EFI_UNSUPPORTED;
+
+  initrd_size = grub_get_initrd_size (&initrd_ctx);
+  if (!buffer || *buffer_size < initrd_size)
+    {
+      *buffer_size = initrd_size;
+      return GRUB_EFI_BUFFER_TOO_SMALL;
+    }
+
+  grub_dprintf ("linux", "Providing initrd via LOAD_FILE2_PROTOCOL\n");
+
+  if (grub_initrd_load (&initrd_ctx, buffer))
+    status = GRUB_EFI_LOAD_ERROR;
+
+  grub_initrd_close (&initrd_ctx);
+  return status;
+}
+
 static grub_err_t
 grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
 		 int argc, char *argv[])
 {
-  struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
   int initrd_size, initrd_pages;
   void *initrd_mem = NULL;
+  grub_efi_boot_services_t *b;
+  grub_efi_status_t status;
 
   if (argc == 0)
     {
@@ -254,6 +351,24 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
   if (grub_initrd_init (argc, argv, &initrd_ctx))
     goto fail;
 
+  if (initrd_use_loadfile2 && !initrd_lf2_handle)
+    {
+      b = grub_efi_system_table->boot_services;
+      status = b->install_multiple_protocol_interfaces (&initrd_lf2_handle,
+							&load_file2_guid,
+							&initrd_lf2,
+							&device_path_guid,
+							&initrd_lf2_device_path,
+							NULL);
+      if (status == GRUB_EFI_OUT_OF_RESOURCES)
+        {
+	  grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+	  return grub_errno;
+	}
+      grub_dprintf ("linux", "LoadFile2 initrd loading protocol installed\n");
+      return GRUB_ERR_NONE;
+    }
+
   initrd_size = grub_get_initrd_size (&initrd_ctx);
   grub_dprintf ("linux", "Loading initrd\n");
 
-- 
2.33.1

[PATCH v3 4/7] linux: ignore FDT unless we need to modify it

[Nikita Ermakov]

From: Ard Biesheuvel <ard.biesheuvel@arm.com>

Now that we implemented supported for the LoadFile2 protocol for initrd
loading, there is no longer a need to pass the initrd parameters via
the device tree. This means there is no longer a reason to update the
device tree in the first place, and so we can ignore it entirely.

The only remaining reason to deal with the devicetree is if we are
using the 'devicetree' command to load one from disk, so tweak the
logic in grub_fdt_install() to take that into account.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 grub-core/loader/arm64/linux.c | 22 +++++++++++-----------
 grub-core/loader/efi/fdt.c     |  7 +++++--
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 6b03455d1..65f1275fb 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -95,21 +95,21 @@ finalize_params_linux (void)
 
   void *fdt;
 
-  fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
+  /* Set initrd info */
+  if (initrd_start && initrd_end > initrd_start)
+    {
+      fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
 
-  if (!fdt)
-    goto failure;
+      if (!fdt)
+	goto failure;
 
-  node = grub_fdt_find_subnode (fdt, 0, "chosen");
-  if (node < 0)
-    node = grub_fdt_add_subnode (fdt, 0, "chosen");
+      node = grub_fdt_find_subnode (fdt, 0, "chosen");
+      if (node < 0)
+	node = grub_fdt_add_subnode (fdt, 0, "chosen");
 
-  if (node < 1)
-    goto failure;
+      if (node < 1)
+	goto failure;
 
-  /* Set initrd info */
-  if (initrd_start && initrd_end > initrd_start)
-    {
       grub_dprintf ("linux", "Initrd @ %p-%p\n",
 		    (void *) initrd_start, (void *) initrd_end);
 
diff --git a/grub-core/loader/efi/fdt.c b/grub-core/loader/efi/fdt.c
index c86f283d7..771d455c7 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -89,13 +89,16 @@ grub_fdt_install (void)
   grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
   grub_efi_status_t status;
 
+  if (!fdt && !loaded_fdt)
+    return GRUB_ERR_NONE;
+
   b = grub_efi_system_table->boot_services;
-  status = b->install_configuration_table (&fdt_guid, fdt);
+  status = b->install_configuration_table (&fdt_guid, fdt ?: loaded_fdt);
   if (status != GRUB_EFI_SUCCESS)
     return grub_error (GRUB_ERR_IO, "failed to install FDT");
 
   grub_dprintf ("fdt", "Installed/updated FDT configuration table @ %p\n",
-		fdt);
+		fdt ?: loaded_fdt);
   return GRUB_ERR_NONE;
 }
 
-- 
2.33.1

[PATCH v3 5/7] loader: Move arm64 linux loader to common code

[Nikita Ermakov]

From: Atish Patra <atish.patra@wdc.com>

ARM64 linux loader code is written in such a way that it can be reused
across different architectures without much change. Move it to common
code so that RISC-V doesn't have to define a separate loader.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 grub-core/Makefile.core.def             | 4 ++--
 grub-core/loader/{arm64 => efi}/linux.c | 2 +-
 include/grub/arm/linux.h                | 2 +-
 include/grub/arm64/linux.h              | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)
 rename grub-core/loader/{arm64 => efi}/linux.c (99%)

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 8022e1c0a..b36cf663a 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1806,9 +1806,9 @@ module = {
   sparc64_ieee1275 = loader/sparc64/ieee1275/linux.c;
   ia64_efi = loader/ia64/efi/linux.c;
   arm_coreboot = loader/arm/linux.c;
-  arm_efi = loader/arm64/linux.c;
+  arm_efi = loader/efi/linux.c;
   arm_uboot = loader/arm/linux.c;
-  arm64 = loader/arm64/linux.c;
+  arm64 = loader/efi/linux.c;
   riscv32 = loader/riscv/linux.c;
   riscv64 = loader/riscv/linux.c;
   common = loader/linux.c;
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/efi/linux.c
similarity index 99%
rename from grub-core/loader/arm64/linux.c
rename to grub-core/loader/efi/linux.c
index 65f1275fb..60f0fa264 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/efi/linux.c
@@ -60,7 +60,7 @@ grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
   struct grub_pe32_coff_header *coff_header;
   struct grub_pe32_optional_header *optional_header;
 
-  if (lh->magic != GRUB_LINUX_ARMXX_MAGIC_SIGNATURE)
+  if (lh->magic != GRUB_LINUX_ARCH_MAGIC_SIGNATURE)
     return grub_error(GRUB_ERR_BAD_OS, "invalid magic number");
 
   if ((lh->code0 & 0xffff) != GRUB_PE32_MAGIC)
diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index bcd5a7eb1..8c13978d2 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -35,7 +35,7 @@ struct linux_arm_kernel_header {
 };
 
 #if defined(__arm__)
-# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM_MAGIC_SIGNATURE
+# define GRUB_LINUX_ARCH_MAGIC_SIGNATURE GRUB_LINUX_ARM_MAGIC_SIGNATURE
 # define linux_arch_kernel_header linux_arm_kernel_header
 #endif
 
diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
index 7e22b4ab6..effd870ef 100644
--- a/include/grub/arm64/linux.h
+++ b/include/grub/arm64/linux.h
@@ -39,7 +39,7 @@ struct linux_arm64_kernel_header
 };
 
 #if defined(__aarch64__)
-# define GRUB_LINUX_ARMXX_MAGIC_SIGNATURE GRUB_LINUX_ARM64_MAGIC_SIGNATURE
+# define GRUB_LINUX_ARCH_MAGIC_SIGNATURE GRUB_LINUX_ARM64_MAGIC_SIGNATURE
 # define linux_arch_kernel_header linux_arm64_kernel_header
 #endif
 
-- 
2.33.1

[PATCH v3 6/7] RISC-V: Update image header

[Nikita Ermakov]

From: Atish Patra <atish.patra@wdc.com>

Update the RISC-V Linux kernel image headers as per the current header.

Reference:
<Linux kernel source>/Documentation/riscv/boot-image-header.rst

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 include/grub/riscv32/linux.h | 15 ++++++++-------
 include/grub/riscv64/linux.h | 15 ++++++++-------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h
index 512b777c8..de0dbdcd1 100644
--- a/include/grub/riscv32/linux.h
+++ b/include/grub/riscv32/linux.h
@@ -19,20 +19,21 @@
 #ifndef GRUB_RISCV32_LINUX_HEADER
 #define GRUB_RISCV32_LINUX_HEADER 1
 
-#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */
+#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x05435352 /* 'RSC\0x5' */
 
-/* From linux/Documentation/riscv/booting.txt */
+/* From linux/Documentation/riscv/boot-image-header.rst */
 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 text_offset;	/* Image load offset, little endian */
+  grub_uint64_t image_size;	/* Effective Image size, little endian */
+  grub_uint64_t flags;		/* kernel flags, little endian */
+  grub_uint32_t version;	/* Version of this header */
+  grub_uint32_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 magic;		/* Magic number, little endian, "RSC\x05" */
   grub_uint32_t hdr_offset;	/* Offset of PE/COFF header */
 };
 
diff --git a/include/grub/riscv64/linux.h b/include/grub/riscv64/linux.h
index 3630c30fb..7c28bc922 100644
--- a/include/grub/riscv64/linux.h
+++ b/include/grub/riscv64/linux.h
@@ -19,22 +19,23 @@
 #ifndef GRUB_RISCV64_LINUX_HEADER
 #define GRUB_RISCV64_LINUX_HEADER 1
 
-#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */
+#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x05435352 /* 'RSC\0x5' */
 
 #define GRUB_EFI_PE_MAGIC	0x5A4D
 
-/* From linux/Documentation/riscv/booting.txt */
+/* From linux/Documentation/riscv/boot-image-header.rst */
 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 text_offset;	/* Image load offset, little endian */
+  grub_uint64_t image_size;	/* Effective Image size, little endian */
+  grub_uint64_t flags;		/* kernel flags, little endian */
+  grub_uint32_t version;	/* Version of this header */
+  grub_uint32_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 magic;		/* Magic number, little endian, "RSC\x05" */
   grub_uint32_t hdr_offset;	/* Offset of PE/COFF header */
 };
 
-- 
2.33.1

[PATCH v3 7/7] RISC-V: Use common linux loader

[Nikita Ermakov]

From: Atish Patra <atish.patra@wdc.com>

RISC-V doesn't have to do anything very different from other architectures
to loader EFI stub linux kernel. As a result, just use the common linux
loader instead of defining a RISC-V specific linux loader.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 grub-core/Makefile.core.def    |  4 +--
 grub-core/loader/riscv/linux.c | 59 ----------------------------------
 include/grub/riscv32/linux.h   |  1 +
 include/grub/riscv64/linux.h   |  1 +
 4 files changed, 4 insertions(+), 61 deletions(-)
 delete mode 100644 grub-core/loader/riscv/linux.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index b36cf663a..0536575bc 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1809,8 +1809,8 @@ module = {
   arm_efi = loader/efi/linux.c;
   arm_uboot = loader/arm/linux.c;
   arm64 = loader/efi/linux.c;
-  riscv32 = loader/riscv/linux.c;
-  riscv64 = loader/riscv/linux.c;
+  riscv32 = loader/efi/linux.c;
+  riscv64 = loader/efi/linux.c;
   common = loader/linux.c;
   common = lib/cmdline.c;
   enable = noemu;
diff --git a/grub-core/loader/riscv/linux.c b/grub-core/loader/riscv/linux.c
deleted file mode 100644
index d17c488e1..000000000
--- a/grub-core/loader/riscv/linux.c
+++ /dev/null
@@ -1,59 +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/>.
- */
-
-#include <grub/command.h>
-#include <grub/dl.h>
-#include <grub/lib/cmdline.h>
-
-GRUB_MOD_LICENSE ("GPLv3+");
-
-static grub_err_t
-grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
-		 int argc __attribute__ ((unused)),
-		 char *argv[] __attribute__ ((unused)))
-{
-  grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("Linux not supported yet"));
-
-  return grub_errno;
-}
-
-static grub_err_t
-grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
-		int argc __attribute__ ((unused)),
-		char *argv[] __attribute__ ((unused)))
-{
-  grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("Linux not supported yet"));
-
-  return grub_errno;
-}
-
-static grub_command_t cmd_linux, cmd_initrd;
-
-GRUB_MOD_INIT (linux)
-{
-  cmd_linux = grub_register_command ("linux", grub_cmd_linux, 0,
-				     N_("Load Linux."));
-  cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd, 0,
-				      N_("Load initrd."));
-}
-
-GRUB_MOD_FINI (linux)
-{
-  grub_unregister_command (cmd_linux);
-  grub_unregister_command (cmd_initrd);
-}
diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h
index de0dbdcd1..706c69087 100644
--- a/include/grub/riscv32/linux.h
+++ b/include/grub/riscv32/linux.h
@@ -38,5 +38,6 @@ struct linux_riscv_kernel_header
 };
 
 #define linux_arch_kernel_header linux_riscv_kernel_header
+# define GRUB_LINUX_ARCH_MAGIC_SIGNATURE GRUB_LINUX_RISCV_MAGIC_SIGNATURE
 
 #endif /* ! GRUB_RISCV32_LINUX_HEADER */
diff --git a/include/grub/riscv64/linux.h b/include/grub/riscv64/linux.h
index 7c28bc922..88d5df781 100644
--- a/include/grub/riscv64/linux.h
+++ b/include/grub/riscv64/linux.h
@@ -40,5 +40,6 @@ struct linux_riscv_kernel_header
 };
 
 #define linux_arch_kernel_header linux_riscv_kernel_header
+# define GRUB_LINUX_ARCH_MAGIC_SIGNATURE GRUB_LINUX_RISCV_MAGIC_SIGNATURE
 
 #endif /* ! GRUB_RISCV64_LINUX_HEADER */
-- 
2.33.1

Re: [PATCH v3 0/7] Add LoadFile2 and riscv Linux loader

[Heinrich Schuchardt]

On 10/28/21 22:31, Nikita Ermakov wrote:
> Hello,
> 
> This is the third version of the patch series. In v3 the series was rebased
> on top of the master branch.
> 
> This series contains patches to add support for LoadFile2 protocol to load
> initrd on EFI systems. Also it contains patches to load Linux kernel with EFI
> stub on riscv platforms and unites arm and riscv codes together into common
> loader code for EFI systems.

Thanks for rebasing. I found no difference to the rebasing I had done in 
my private repo and successfully used for testing.

Best regards

Heinrich

Re: [PATCH v3 0/7] Add LoadFile2 and riscv Linux loader

[Fu Wei]

Hi Nikita,
Great thanks for updating V3, I have tried them on RISC-V Hardware
(Starfive StarLight/VisionFive, and Allwinner D1 Nezha), they are
working well, boot into Fedora.
My repo is here :
https://github.com/tekkamanninja/grub/tree/riscv_devel_Nikita_V3

For whole patchset:
Tested-by: Fu Wei <tekkamanninja@gmail.com>

Nikita Ermakov <arei@altlinux.org> 于2021年10月29日周五 上午4:32写道：
>
> Hello,
>
> This is the third version of the patch series. In v3 the series was rebased
> on top of the master branch.
>
> This series contains patches to add support for LoadFile2 protocol to load
> initrd on EFI systems. Also it contains patches to load Linux kernel with EFI
> stub on riscv platforms and unites arm and riscv codes together into common
> loader code for EFI systems.
>
> Ard Biesheuvel (3):
>   efi: add definition of LoadFile2 protocol
>   efi: implemented LoadFile2 initrd loading protocol for Linux
>   linux: ignore FDT unless we need to modify it
>
> Atish Patra (3):
>   loader: Move arm64 linux loader to common code
>   RISC-V: Update image header
>   RISC-V: Use common linux loader
>
> Nikita Ermakov (1):
>   loader: drop argv[] argument in grub_initrd_load()
>
>  grub-core/Makefile.core.def               |   8 +-
>  grub-core/commands/efi/lsefi.c            |   1 +
>  grub-core/loader/arm/linux.c              |   2 +-
>  grub-core/loader/efi/fdt.c                |   7 +-
>  grub-core/loader/{arm64 => efi}/linux.c   | 143 +++++++++++++++++++---
>  grub-core/loader/i386/linux.c             |   2 +-
>  grub-core/loader/i386/pc/linux.c          |   2 +-
>  grub-core/loader/i386/xen.c               |   3 +-
>  grub-core/loader/ia64/efi/linux.c         |   2 +-
>  grub-core/loader/linux.c                  |   4 +-
>  grub-core/loader/mips/linux.c             |   2 +-
>  grub-core/loader/powerpc/ieee1275/linux.c |   2 +-
>  grub-core/loader/riscv/linux.c            |  59 ---------
>  grub-core/loader/sparc64/ieee1275/linux.c |   2 +-
>  include/grub/arm/linux.h                  |   2 +-
>  include/grub/arm64/linux.h                |   2 +-
>  include/grub/efi/api.h                    |  15 +++
>  include/grub/linux.h                      |   2 +-
>  include/grub/riscv32/linux.h              |  16 +--
>  include/grub/riscv64/linux.h              |  16 +--
>  20 files changed, 185 insertions(+), 107 deletions(-)
>  rename grub-core/loader/{arm64 => efi}/linux.c (72%)
>  delete mode 100644 grub-core/loader/riscv/linux.c
>
> --
> 2.33.1
>

Re: [PATCH v3 1/7] loader: drop argv[] argument in grub_initrd_load()

[Daniel Kiper]

On Thu, Oct 28, 2021 at 11:31:13PM +0300, Nikita Ermakov wrote:
> In the case of an error grub_initrd_load() uses argv[] to print the
> filename that caused the error. It is also possible to obtain the
> filename from the file handles and there is no need to duplicate that
> information in argv[], so let's drop it.
>
> Signed-off-by: Nikita Ermakov <arei@altlinux.org>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH v3 2/7] efi: add definition of LoadFile2 protocol

[Daniel Kiper]

On Thu, Oct 28, 2021 at 11:31:15PM +0300, Nikita Ermakov wrote:
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
>
> Incorporate the EFI_LOAD_FILE2_PROTOCOL GUID and C types from the
> UEFI spec.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Nikita Ermakov <arei@altlinux.org>
> ---
>  grub-core/commands/efi/lsefi.c |  1 +
>  include/grub/efi/api.h         | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
> index d1ce99af4..4085f5df2 100644
> --- a/grub-core/commands/efi/lsefi.c
> +++ b/grub-core/commands/efi/lsefi.c
> @@ -55,6 +55,7 @@ struct known_protocol
>      { GRUB_EFI_ABSOLUTE_POINTER_PROTOCOL_GUID, "absolute pointer" },
>      { GRUB_EFI_DRIVER_BINDING_PROTOCOL_GUID, "EFI driver binding" },
>      { GRUB_EFI_LOAD_FILE_PROTOCOL_GUID, "load file" },
> +    { GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID, "load file2" },
>      { GRUB_EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID, "simple FS" },
>      { GRUB_EFI_TAPE_IO_PROTOCOL_GUID, "tape I/O" },
>      { GRUB_EFI_UNICODE_COLLATION_PROTOCOL_GUID, "unicode collation" },
> diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> index f1a52210c..3de0a7d12 100644
> --- a/include/grub/efi/api.h
> +++ b/include/grub/efi/api.h
> @@ -149,6 +149,11 @@
>      { 0x8E, 0x3F, 0x00, 0xA0, 0xC9, 0x69, 0x72, 0x3B } \
>    }
>
> +#define GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID \
> +  { 0x4006c0c1, 0xfcb3, 0x403e, \
> +    { 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d } \
> +  }
> +
>  #define GRUB_EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID \
>    { 0x0964e5b22, 0x6459, 0x11d2, \
>      { 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b } \
> @@ -1729,6 +1734,16 @@ struct grub_efi_rng_protocol
>  };
>  typedef struct grub_efi_rng_protocol grub_efi_rng_protocol_t;
>
> +struct grub_efi_load_file2
> +{
> +  grub_efi_status_t (*load_file)(struct grub_efi_load_file2 *this,
> +				 grub_efi_device_path_t *file_path,
> +				 grub_efi_boolean_t boot_policy,
> +				 grub_efi_uintn_t *buffer_size,
> +				 void *buffer);
> +};
> +typedef struct grub_efi_load_file2 grub_efi_load_file2_t;
> +
>  #if (GRUB_TARGET_SIZEOF_VOID_P == 4) || defined (__ia64__) \
>    || defined (__aarch64__) || defined (__MINGW64__) || defined (__CYGWIN__) \
>    || defined(__riscv)

This changes should be part of the patch #3.

Daniel

Re: [PATCH v3 3/7] efi: implemented LoadFile2 initrd loading protocol for Linux

[Daniel Kiper]

On Thu, Oct 28, 2021 at 11:31:16PM +0300, Nikita Ermakov wrote:
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
>
> Recent Linux kernels will invoke the LoadFile2 protocol installed on
> a well-known vendor media path to load the initrd if it is exposed by
> the firmware. Using this method is preferred for two reasons:
> - the Linux kernel is in charge of allocating the memory, and so it can
>   implement any placement policy it wants (given that these tend to
>   change between kernel versions),
> - it is no longer necessary to modify the device tree provided by the
>   firmware.
>
> So let's install this protocol when handling the 'initrd' command if
> such a recent kernel was detected (based on the PE/COFF image version),
> and defer loading the initrd contents until the point where the kernel
> invokes the LoadFile2 protocol.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Signed-off-by: Nikita Ermakov <arei@altlinux.org>
> ---
>  grub-core/loader/arm64/linux.c | 117 ++++++++++++++++++++++++++++++++-
>  1 file changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index aed7a200b..6b03455d1 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -48,9 +48,18 @@ static grub_uint32_t cmdline_size;
>  static grub_addr_t initrd_start;
>  static grub_addr_t initrd_end;
>
> +static struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
> +static grub_efi_handle_t initrd_lf2_handle;
> +static int initrd_use_loadfile2;

Please explicitly initialize these two variables.

> +static grub_efi_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
> +static grub_efi_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
> +
>  grub_err_t
>  grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
>  {
> +  struct grub_pe32_coff_header *coff_header;
> +  struct grub_pe32_optional_header *optional_header;
> +
>    if (lh->magic != GRUB_LINUX_ARMXX_MAGIC_SIGNATURE)
>      return grub_error(GRUB_ERR_BAD_OS, "invalid magic number");
>
> @@ -61,6 +70,21 @@ grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
>    grub_dprintf ("linux", "UEFI stub kernel:\n");
>    grub_dprintf ("linux", "PE/COFF header @ %08x\n", lh->hdr_offset);
>
> +  coff_header = (struct grub_pe32_coff_header *)((unsigned long)lh + lh->hdr_offset);

coff_header = (struct grub_pe32_coff_header *) ((grub_addr_t) lh + lh->hdr_offset);

... please note missing spaces too.

> +  optional_header = (struct grub_pe32_optional_header *)(coff_header + 1);

(struct grub_pe32_optional_header *) (coff_header + 1);

> +  /*
> +   * Linux kernels built for any architecture are guaranteed to support the
> +   * LoadFile2 based initrd loading protocol if the image version is >= 1.
> +   */
> +  if (optional_header->major_image_version >= 1)
> +    initrd_use_loadfile2 = 1;
> +   else
> +    initrd_use_loadfile2 = 0;
> +
> +  grub_dprintf ("linux", "LoadFile2 initrd loading %sabled\n",
> +		initrd_use_loadfile2 ? "en" : "dis");
> +
>    return GRUB_ERR_NONE;
>  }
>
> @@ -230,13 +254,86 @@ allocate_initrd_mem (int initrd_pages)
>  				       GRUB_EFI_LOADER_DATA);
>  }
>
> +struct initrd_media_device_path {
> +  grub_efi_vendor_media_device_path_t	vendor;
> +  grub_efi_device_path_t		end;
> +} GRUB_PACKED;

typedef struct initrd_media_device_path initrd_media_device_path_t;

And please use initrd_media_device_path_t instead of "struct initrd_media_device_path".

> +
> +#define LINUX_EFI_INITRD_MEDIA_GUID  \
> +  { 0x5568e427, 0x68fc, 0x4f3d, \
> +    { 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68 } \
> +  }

Please move struct initrd_media_device_path and
LINUX_EFI_INITRD_MEDIA_GUID definitions to the include/grub/efi/api.h file.

> +static struct initrd_media_device_path initrd_lf2_device_path = {
> +  {
> +    {
> +      GRUB_EFI_MEDIA_DEVICE_PATH_TYPE,
> +      GRUB_EFI_VENDOR_MEDIA_DEVICE_PATH_SUBTYPE,
> +      sizeof(grub_efi_vendor_media_device_path_t),
> +    },
> +    LINUX_EFI_INITRD_MEDIA_GUID
> +  }, {
> +    GRUB_EFI_END_DEVICE_PATH_TYPE,
> +    GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    sizeof(grub_efi_device_path_t)
> +  }
> +};

This declaration/initialization should be behind device_path_guid declaration.

> +static grub_efi_status_t
> +grub_efi_initrd_load_file2(grub_efi_load_file2_t *this,
> +                           grub_efi_device_path_t *device_path,
> +                           grub_efi_boolean_t boot_policy,
> +                           grub_efi_uintn_t *buffer_size,
> +                           void *buffer);

Please drop this...

> +
> +static grub_efi_load_file2_t initrd_lf2 = {
> +  grub_efi_initrd_load_file2
> +};

... and move this behind grub_efi_initrd_load_file2() definition.

> +static grub_efi_status_t
> +grub_efi_initrd_load_file2(grub_efi_load_file2_t *this,
> +			   grub_efi_device_path_t *device_path,
> +			   grub_efi_boolean_t boot_policy,
> +			   grub_efi_uintn_t *buffer_size,
> +			   void *buffer)

This works only because UEFI RISC-V/ARM and "normal" RISC-V/ARM calling
conventions are the same. This will not work on x86_64. So, we have to
introduce something like __grub_efi_api macro. It should be empty for
at least for RISC-V and ARM but it should be "__attribute__ ((ms_abi))"
for at least for x86_64. I think the __grub_efi_api macro should be
defined in include/grub/efi/api.h next to efi_call_* definitions.

> +{
> +  grub_efi_status_t status = GRUB_EFI_SUCCESS;
> +  grub_efi_uintn_t initrd_size;
> +
> +  if (!this || this != &initrd_lf2 || !buffer_size)

if (this == NULL || this != &initrd_lf2 || buffer_size == NULL)

Please use NULL explicitly below too...

> +    return GRUB_EFI_INVALID_PARAMETER;
> +
> +  if (device_path->type != GRUB_EFI_END_DEVICE_PATH_TYPE ||
> +      device_path->subtype != GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE)
> +    return GRUB_EFI_NOT_FOUND;
> +
> +  if (boot_policy)
> +    return GRUB_EFI_UNSUPPORTED;
> +
> +  initrd_size = grub_get_initrd_size (&initrd_ctx);
> +  if (!buffer || *buffer_size < initrd_size)
> +    {
> +      *buffer_size = initrd_size;
> +      return GRUB_EFI_BUFFER_TOO_SMALL;
> +    }
> +
> +  grub_dprintf ("linux", "Providing initrd via LOAD_FILE2_PROTOCOL\n");

s/LOAD_FILE2_PROTOCOL/EFI_LOAD_FILE2_PROTOCOL/

> +  if (grub_initrd_load (&initrd_ctx, buffer))

if (grub_initrd_load (&initrd_ctx, buffer) != GRUB_ERR_NONE))

> +    status = GRUB_EFI_LOAD_ERROR;

According to the UEFI spec this should be GRUB_EFI_DEVICE_ERROR.

> +  grub_initrd_close (&initrd_ctx);
> +  return status;
> +}
> +
>  static grub_err_t
>  grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
>  		 int argc, char *argv[])
>  {
> -  struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
>    int initrd_size, initrd_pages;
>    void *initrd_mem = NULL;
> +  grub_efi_boot_services_t *b;

grub_efi_boot_services_t *b = grub_efi_system_table->boot_services;

> +  grub_efi_status_t status;
>
>    if (argc == 0)
>      {
> @@ -254,6 +351,24 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
>    if (grub_initrd_init (argc, argv, &initrd_ctx))
>      goto fail;
>
> +  if (initrd_use_loadfile2 && !initrd_lf2_handle)
> +    {
> +      b = grub_efi_system_table->boot_services;

... then you can drop this.

> +      status = b->install_multiple_protocol_interfaces (&initrd_lf2_handle,
> +							&load_file2_guid,
> +							&initrd_lf2,
> +							&device_path_guid,
> +							&initrd_lf2_device_path,
> +							NULL);
> +      if (status == GRUB_EFI_OUT_OF_RESOURCES)
> +        {
> +	  grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +	  return grub_errno;

s/return grub_errno/goto fail/ otherwise your are leaking memory.

Or you can move protocols installation to the beginning of the function
and return immediately in case of error.

> +	}

I think we should not ignore the other UEFI errors here.

> +      grub_dprintf ("linux", "LoadFile2 initrd loading protocol installed\n");
> +      return GRUB_ERR_NONE;
> +    }
> +
>    initrd_size = grub_get_initrd_size (&initrd_ctx);
>    grub_dprintf ("linux", "Loading initrd\n");

Daniel

Re: [PATCH v3 4/7] linux: ignore FDT unless we need to modify it

[Daniel Kiper]

On Thu, Oct 28, 2021 at 11:31:17PM +0300, Nikita Ermakov wrote:
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
>
> Now that we implemented supported for the LoadFile2 protocol for initrd
> loading, there is no longer a need to pass the initrd parameters via
> the device tree. This means there is no longer a reason to update the
> device tree in the first place, and so we can ignore it entirely.
>
> The only remaining reason to deal with the devicetree is if we are
> using the 'devicetree' command to load one from disk, so tweak the
> logic in grub_fdt_install() to take that into account.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  grub-core/loader/arm64/linux.c | 22 +++++++++++-----------
>  grub-core/loader/efi/fdt.c     |  7 +++++--
>  2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index 6b03455d1..65f1275fb 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -95,21 +95,21 @@ finalize_params_linux (void)
>
>    void *fdt;
>
> -  fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
> +  /* Set initrd info */
> +  if (initrd_start && initrd_end > initrd_start)
> +    {
> +      fdt = grub_fdt_load (GRUB_EFI_LINUX_FDT_EXTRA_SPACE);
>
> -  if (!fdt)
> -    goto failure;
> +      if (!fdt)
> +	goto failure;
>
> -  node = grub_fdt_find_subnode (fdt, 0, "chosen");
> -  if (node < 0)
> -    node = grub_fdt_add_subnode (fdt, 0, "chosen");
> +      node = grub_fdt_find_subnode (fdt, 0, "chosen");
> +      if (node < 0)
> +	node = grub_fdt_add_subnode (fdt, 0, "chosen");
>
> -  if (node < 1)
> -    goto failure;
> +      if (node < 1)
> +	goto failure;
>
> -  /* Set initrd info */
> -  if (initrd_start && initrd_end > initrd_start)
> -    {
>        grub_dprintf ("linux", "Initrd @ %p-%p\n",
>  		    (void *) initrd_start, (void *) initrd_end);

You are saying in the commit message that this code is not needed any
longer but effectively it stay as is. Something is wrong here. I think
the commit message should be improved or the initrd FDT code should be
removed entirely.

Daniel

Re: [PATCH v3 5/7] loader: Move arm64 linux loader to common code

[Daniel Kiper]

On Thu, Oct 28, 2021 at 11:31:18PM +0300, Nikita Ermakov wrote:
> From: Atish Patra <atish.patra@wdc.com>
>
> ARM64 linux loader code is written in such a way that it can be reused
> across different architectures without much change. Move it to common
> code so that RISC-V doesn't have to define a separate loader.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH v3 6/7] RISC-V: Update image header

[Daniel Kiper]

On Thu, Oct 28, 2021 at 11:31:19PM +0300, Nikita Ermakov wrote:
> From: Atish Patra <atish.patra@wdc.com>
>
> Update the RISC-V Linux kernel image headers as per the current header.
>
> Reference:
> <Linux kernel source>/Documentation/riscv/boot-image-header.rst

May I ask you to put the Linux kernel commit ID and its subject of the
boot spec which you are referencing here?

> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  include/grub/riscv32/linux.h | 15 ++++++++-------
>  include/grub/riscv64/linux.h | 15 ++++++++-------
>  2 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h
> index 512b777c8..de0dbdcd1 100644
> --- a/include/grub/riscv32/linux.h
> +++ b/include/grub/riscv32/linux.h
> @@ -19,20 +19,21 @@
>  #ifndef GRUB_RISCV32_LINUX_HEADER
>  #define GRUB_RISCV32_LINUX_HEADER 1
>
> -#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */
> +#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x05435352 /* 'RSC\0x5' */
>
> -/* From linux/Documentation/riscv/booting.txt */
> +/* From linux/Documentation/riscv/boot-image-header.rst */
>  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 text_offset;	/* Image load offset, little endian */
> +  grub_uint64_t image_size;	/* Effective Image size, little endian */
> +  grub_uint64_t flags;		/* kernel flags, little endian */
> +  grub_uint32_t version;	/* Version of this header */
> +  grub_uint32_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 magic;		/* Magic number, little endian, "RSC\x05" */

This does not agree with the latest spec in the Linux kernel.

Daniel

Re: [PATCH v3 7/7] RISC-V: Use common linux loader

[Daniel Kiper]

On Thu, Oct 28, 2021 at 11:31:20PM +0300, Nikita Ermakov wrote:
> From: Atish Patra <atish.patra@wdc.com>
>
> RISC-V doesn't have to do anything very different from other architectures
> to loader EFI stub linux kernel. As a result, just use the common linux
> loader instead of defining a RISC-V specific linux loader.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  grub-core/Makefile.core.def    |  4 +--
>  grub-core/loader/riscv/linux.c | 59 ----------------------------------
>  include/grub/riscv32/linux.h   |  1 +
>  include/grub/riscv64/linux.h   |  1 +
>  4 files changed, 4 insertions(+), 61 deletions(-)
>  delete mode 100644 grub-core/loader/riscv/linux.c
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index b36cf663a..0536575bc 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1809,8 +1809,8 @@ module = {
>    arm_efi = loader/efi/linux.c;
>    arm_uboot = loader/arm/linux.c;
>    arm64 = loader/efi/linux.c;
> -  riscv32 = loader/riscv/linux.c;
> -  riscv64 = loader/riscv/linux.c;
> +  riscv32 = loader/efi/linux.c;
> +  riscv64 = loader/efi/linux.c;
>    common = loader/linux.c;
>    common = lib/cmdline.c;
>    enable = noemu;
> diff --git a/grub-core/loader/riscv/linux.c b/grub-core/loader/riscv/linux.c
> deleted file mode 100644
> index d17c488e1..000000000
> --- a/grub-core/loader/riscv/linux.c
> +++ /dev/null
> @@ -1,59 +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/>.
> - */
> -
> -#include <grub/command.h>
> -#include <grub/dl.h>
> -#include <grub/lib/cmdline.h>
> -
> -GRUB_MOD_LICENSE ("GPLv3+");
> -
> -static grub_err_t
> -grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
> -		 int argc __attribute__ ((unused)),
> -		 char *argv[] __attribute__ ((unused)))
> -{
> -  grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("Linux not supported yet"));
> -
> -  return grub_errno;
> -}
> -
> -static grub_err_t
> -grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
> -		int argc __attribute__ ((unused)),
> -		char *argv[] __attribute__ ((unused)))
> -{
> -  grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("Linux not supported yet"));
> -
> -  return grub_errno;
> -}
> -
> -static grub_command_t cmd_linux, cmd_initrd;
> -
> -GRUB_MOD_INIT (linux)
> -{
> -  cmd_linux = grub_register_command ("linux", grub_cmd_linux, 0,
> -				     N_("Load Linux."));
> -  cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd, 0,
> -				      N_("Load initrd."));
> -}
> -
> -GRUB_MOD_FINI (linux)
> -{
> -  grub_unregister_command (cmd_linux);
> -  grub_unregister_command (cmd_initrd);
> -}
> diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h
> index de0dbdcd1..706c69087 100644
> --- a/include/grub/riscv32/linux.h
> +++ b/include/grub/riscv32/linux.h
> @@ -38,5 +38,6 @@ struct linux_riscv_kernel_header
>  };
>
>  #define linux_arch_kernel_header linux_riscv_kernel_header
> +# define GRUB_LINUX_ARCH_MAGIC_SIGNATURE GRUB_LINUX_RISCV_MAGIC_SIGNATURE

Please put this definition before linux_arch_kernel_header definition.

>  #endif /* ! GRUB_RISCV32_LINUX_HEADER */
> diff --git a/include/grub/riscv64/linux.h b/include/grub/riscv64/linux.h
> index 7c28bc922..88d5df781 100644
> --- a/include/grub/riscv64/linux.h
> +++ b/include/grub/riscv64/linux.h
> @@ -40,5 +40,6 @@ struct linux_riscv_kernel_header
>  };
>
>  #define linux_arch_kernel_header linux_riscv_kernel_header
> +# define GRUB_LINUX_ARCH_MAGIC_SIGNATURE GRUB_LINUX_RISCV_MAGIC_SIGNATURE

Ditto.

Daniel

Re: [PATCH v3 3/7] efi: implemented LoadFile2 initrd loading protocol for Linux

[dann frazier]

On Thu, Oct 28, 2021 at 11:31:16PM +0300, Nikita Ermakov wrote:
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> 
> Recent Linux kernels will invoke the LoadFile2 protocol installed on
> a well-known vendor media path to load the initrd if it is exposed by
> the firmware. Using this method is preferred for two reasons:
> - the Linux kernel is in charge of allocating the memory, and so it can
>   implement any placement policy it wants (given that these tend to
>   change between kernel versions),
> - it is no longer necessary to modify the device tree provided by the
>   firmware.
> 
> So let's install this protocol when handling the 'initrd' command if
> such a recent kernel was detected (based on the PE/COFF image version),
> and defer loading the initrd contents until the point where the kernel
> invokes the LoadFile2 protocol.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Signed-off-by: Nikita Ermakov <arei@altlinux.org>
> ---
>  grub-core/loader/arm64/linux.c | 117 ++++++++++++++++++++++++++++++++-
>  1 file changed, 116 insertions(+), 1 deletion(-)
> 

fyi, Ubuntu currently includes this patch in our in-devel release, but
we'll likely have to disable it for arm64 due to a regression. We found
that it casuse our PXE boot install tests to fail because
grub_net_fini_hw() is called before the TFTP transfer for the
initramfs is attempted. More info here:

  https://launchpad.net/bugs/1967562

 -dann