* [PATCH 1/4] loader/linux: permit NULL argument for argv[] in grub_initrd_load()
2020-10-23 12:08 [PATCH 0/4] linux: implement LoadFile2 initrd loading Ard Biesheuvel
@ 2020-10-23 12:08 ` Ard Biesheuvel
2020-10-23 12:08 ` [PATCH 2/4] efi: add definition of LoadFile2 protocol Ard Biesheuvel
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-10-23 12:08 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, grub-devel, daniel.kiper, leif
grub_initrd_load() takes a char *argv[] argument which is only used
when an error occurs, to print the name of the file that caused the
error. In order to be able to split initrd loading from handling the
initrd command, let's permit argv to be NULL, and fall back to the
file names recorded in the file handles.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
grub-core/loader/linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index 3fe390f17ec6..8c01381778c6 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -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]);
+ argv ? argv[i] : initrd_ctx->components[i].file->name);
grub_initrd_close (initrd_ctx);
return grub_errno;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/4] efi: add definition of LoadFile2 protocol
2020-10-23 12:08 [PATCH 0/4] linux: implement LoadFile2 initrd loading Ard Biesheuvel
2020-10-23 12:08 ` [PATCH 1/4] loader/linux: permit NULL argument for argv[] in grub_initrd_load() Ard Biesheuvel
@ 2020-10-23 12:08 ` Ard Biesheuvel
2020-10-23 12:08 ` [PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux Ard Biesheuvel
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-10-23 12:08 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, grub-devel, daniel.kiper, leif
Incorporate the EFI_LOAD_FILE2_PROTOCOL GUID and C types from the
UEFI spec.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
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 d1ce99af4389..145cbfcdaf5c 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 file 2" },
{ 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 1dcaa12f59e8..8bc040d9251b 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 } \
@@ -1694,6 +1699,16 @@ struct grub_efi_block_io
};
typedef struct grub_efi_block_io grub_efi_block_io_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.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux
2020-10-23 12:08 [PATCH 0/4] linux: implement LoadFile2 initrd loading Ard Biesheuvel
2020-10-23 12:08 ` [PATCH 1/4] loader/linux: permit NULL argument for argv[] in grub_initrd_load() Ard Biesheuvel
2020-10-23 12:08 ` [PATCH 2/4] efi: add definition of LoadFile2 protocol Ard Biesheuvel
@ 2020-10-23 12:08 ` Ard Biesheuvel
2020-10-23 12:32 ` Leif Lindholm
2020-10-23 12:08 ` [PATCH 4/4] linux: ignore FDT unless we need to modify it Ard Biesheuvel
2020-10-23 12:50 ` [PATCH 0/4] linux: implement LoadFile2 initrd loading Leif Lindholm
4 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2020-10-23 12:08 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, grub-devel, daniel.kiper, leif
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>
---
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 ef3e9f9444ca..285422c7bd43 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -48,9 +48,16 @@ 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;
+
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 +68,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 +252,88 @@ 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", "Loading initrd via LOAD_FILE2_PROTOCOL\n");
+
+ if (grub_initrd_load (&initrd_ctx, NULL, 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_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
+ grub_efi_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
+ 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.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux
2020-10-23 12:08 ` [PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux Ard Biesheuvel
@ 2020-10-23 12:32 ` Leif Lindholm
2020-10-23 17:02 ` Ard Biesheuvel
0 siblings, 1 reply; 11+ messages in thread
From: Leif Lindholm @ 2020-10-23 12:32 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, grub-devel, daniel.kiper
initrd typo in subject
minor style comments below
On Fri, Oct 23, 2020 at 14:08:24 +0200, Ard Biesheuvel wrote:
> 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>
> ---
> 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 ef3e9f9444ca..285422c7bd43 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -48,9 +48,16 @@ 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;
> +
> 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 +68,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
funky indentation
> + initrd_use_loadfile2 = 0;
> +
> + grub_dprintf ("linux", "LoadFile2 initrd loading %sabled\n",
> + initrd_use_loadfile2 ? "en" : "dis");
> +
> return GRUB_ERR_NONE;
> }
>
> @@ -230,13 +252,88 @@ 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,
Static functions don't need global prefixes (i.e., initrd_load_file2
would be sufficient).
> + 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", "Loading initrd via LOAD_FILE2_PROTOCOL\n");
> +
> + if (grub_initrd_load (&initrd_ctx, NULL, 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_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
> + grub_efi_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
> + 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)
> + {
indented by spaces
> + grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> + return grub_errno;
> + }
indented with tab
/
Leif
> + 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.17.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux
2020-10-23 12:32 ` Leif Lindholm
@ 2020-10-23 17:02 ` Ard Biesheuvel
0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-10-23 17:02 UTC (permalink / raw)
To: Leif Lindholm; +Cc: Ard Biesheuvel, linux-efi, grub-devel, Daniel Kiper
On Fri, 23 Oct 2020 at 14:32, Leif Lindholm <leif@nuviainc.com> wrote:
>
> initrd typo in subject
>
> minor style comments below
>
> On Fri, Oct 23, 2020 at 14:08:24 +0200, Ard Biesheuvel wrote:
> > 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>
> > ---
> > 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 ef3e9f9444ca..285422c7bd43 100644
> > --- a/grub-core/loader/arm64/linux.c
> > +++ b/grub-core/loader/arm64/linux.c
> > @@ -48,9 +48,16 @@ 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;
> > +
> > 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 +68,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
>
> funky indentation
>
> > + initrd_use_loadfile2 = 0;
> > +
> > + grub_dprintf ("linux", "LoadFile2 initrd loading %sabled\n",
> > + initrd_use_loadfile2 ? "en" : "dis");
> > +
> > return GRUB_ERR_NONE;
> > }
> >
> > @@ -230,13 +252,88 @@ 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,
>
> Static functions don't need global prefixes (i.e., initrd_load_file2
> would be sufficient).
>
> > + 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", "Loading initrd via LOAD_FILE2_PROTOCOL\n");
> > +
> > + if (grub_initrd_load (&initrd_ctx, NULL, 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_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
> > + grub_efi_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
> > + 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)
> > + {
>
> indented by spaces
>
> > + grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> > + return grub_errno;
> > + }
>
> indented with tab
>
Thanks. I'll fix that up for the next revision. There's actually a bug
in this patch which I need to fix too: the code that checks the
PE/COFF header for the image version doesn't actually load the entire
header :-(
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] linux: ignore FDT unless we need to modify it
2020-10-23 12:08 [PATCH 0/4] linux: implement LoadFile2 initrd loading Ard Biesheuvel
` (2 preceding siblings ...)
2020-10-23 12:08 ` [PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux Ard Biesheuvel
@ 2020-10-23 12:08 ` Ard Biesheuvel
2020-10-23 12:47 ` Leif Lindholm
2020-10-23 12:50 ` [PATCH 0/4] linux: implement LoadFile2 initrd loading Leif Lindholm
4 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2020-10-23 12:08 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, grub-devel, daniel.kiper, leif
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 285422c7bd43..9e282b6660fe 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -93,21 +93,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 ee9c5592c700..ab900b27d927 100644
--- a/grub-core/loader/efi/fdt.c
+++ b/grub-core/loader/efi/fdt.c
@@ -85,13 +85,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.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] linux: ignore FDT unless we need to modify it
2020-10-23 12:08 ` [PATCH 4/4] linux: ignore FDT unless we need to modify it Ard Biesheuvel
@ 2020-10-23 12:47 ` Leif Lindholm
2020-10-23 13:12 ` Ard Biesheuvel
0 siblings, 1 reply; 11+ messages in thread
From: Leif Lindholm @ 2020-10-23 12:47 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, grub-devel, daniel.kiper
On Fri, Oct 23, 2020 at 14:08:25 +0200, Ard Biesheuvel wrote:
> 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.
There is a change in behaviour here which I don't think matters, but
I'll call it out anyway:
If there was ever a kernel out there with an EFI stub that depended on
a chosen node existing in the DT, and the one provide by firmware did
not contain one, that setup would break from this *if* it didn't use
an initrd.
/
Leif
> 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 285422c7bd43..9e282b6660fe 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -93,21 +93,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 ee9c5592c700..ab900b27d927 100644
> --- a/grub-core/loader/efi/fdt.c
> +++ b/grub-core/loader/efi/fdt.c
> @@ -85,13 +85,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.17.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] linux: ignore FDT unless we need to modify it
2020-10-23 12:47 ` Leif Lindholm
@ 2020-10-23 13:12 ` Ard Biesheuvel
2020-10-23 13:16 ` Leif Lindholm
0 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2020-10-23 13:12 UTC (permalink / raw)
To: Leif Lindholm; +Cc: Ard Biesheuvel, linux-efi, grub-devel, Daniel Kiper
On Fri, 23 Oct 2020 at 14:47, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Fri, Oct 23, 2020 at 14:08:25 +0200, Ard Biesheuvel wrote:
> > 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.
>
> There is a change in behaviour here which I don't think matters, but
> I'll call it out anyway:
> If there was ever a kernel out there with an EFI stub that depended on
> a chosen node existing in the DT, and the one provide by firmware did
> not contain one, that setup would break from this *if* it didn't use
> an initrd.
>
I checked the Linux source, and the original code contributed by Roy
already contained the logic to create the /chosen node if it wants
there already. So we should be fine here.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] linux: ignore FDT unless we need to modify it
2020-10-23 13:12 ` Ard Biesheuvel
@ 2020-10-23 13:16 ` Leif Lindholm
0 siblings, 0 replies; 11+ messages in thread
From: Leif Lindholm @ 2020-10-23 13:16 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Ard Biesheuvel, linux-efi, grub-devel, Daniel Kiper
On Fri, Oct 23, 2020 at 15:12:50 +0200, Ard Biesheuvel wrote:
> On Fri, 23 Oct 2020 at 14:47, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > On Fri, Oct 23, 2020 at 14:08:25 +0200, Ard Biesheuvel wrote:
> > > 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.
> >
> > There is a change in behaviour here which I don't think matters, but
> > I'll call it out anyway:
> > If there was ever a kernel out there with an EFI stub that depended on
> > a chosen node existing in the DT, and the one provide by firmware did
> > not contain one, that setup would break from this *if* it didn't use
> > an initrd.
>
> I checked the Linux source, and the original code contributed by Roy
> already contained the logic to create the /chosen node if it wants
> there already. So we should be fine here.
Excellent. Then, with this information now in a public archive for any
unfortunate souls doing anything crazy non-linux to find:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] linux: implement LoadFile2 initrd loading
2020-10-23 12:08 [PATCH 0/4] linux: implement LoadFile2 initrd loading Ard Biesheuvel
` (3 preceding siblings ...)
2020-10-23 12:08 ` [PATCH 4/4] linux: ignore FDT unless we need to modify it Ard Biesheuvel
@ 2020-10-23 12:50 ` Leif Lindholm
4 siblings, 0 replies; 11+ messages in thread
From: Leif Lindholm @ 2020-10-23 12:50 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, grub-devel, daniel.kiper
On Fri, Oct 23, 2020 at 14:08:21 +0200, Ard Biesheuvel wrote:
> This implements the LoadFile2 initrd loading protocol, which is
> essentially a callback face into the bootloader to load the initrd
> data into a caller provided buffer. This means the bootloader no
> longer has to contain any policy regarding where to load the initrd
> (which differs between architectures and kernel versions) and no
> longer has to manipulate arch specific data structures such as DT
> or struct bootparams to inform the OS where the initrd resides in
> memory.
>
> Sample output from booting a recent Linux/arm64 kernel:
>
> grub> insmod part_msdos
> grub> linux (hd0,msdos1)/Image
> grub> initrd (hd0,msdos1)/initrd.img
> grub> boot
> EFI stub: Booting Linux Kernel...
> EFI stub: EFI_RNG_PROTOCOL unavailable, KASLR will be disabled
> EFI stub: Generating empty DTB
> EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
> EFI stub: Exiting boot services and installing virtual address map...
> [ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x411fd070]
I don't review enough grub code to be certain I've caught all aspects
of style adherence, so with that in mind, for 1-2/4:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
For 3-4/4, I did have some minor comments, but this is a really great
feature and I would like to see it merged.
/
Leif
> Cc: grub-devel@gnu.org
> Cc: daniel.kiper@oracle.com
> Cc: leif@nuviainc.com
>
> Ard Biesheuvel (4):
> loader/linux: permit NULL argument for argv[] in grub_initrd_load()
> efi: add definition of LoadFile2 protocol
> efi: implemented LoadFile2 initr loading protocol for Linux
> linux: ignore FDT unless we need to modify it
>
> grub-core/commands/efi/lsefi.c | 1 +
> grub-core/loader/arm64/linux.c | 139 ++++++++++++++++++--
> grub-core/loader/efi/fdt.c | 7 +-
> grub-core/loader/linux.c | 2 +-
> include/grub/efi/api.h | 15 +++
> 5 files changed, 149 insertions(+), 15 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread