* [PATCH 0/6 v2] Loadfile2 for initrd loading
@ 2021-03-13 21:47 Ilias Apalodimas
2021-03-13 21:47 ` [PATCH 1/6 v2] efi_selftest: Remove loadfile2 for initrd selftests Ilias Apalodimas
` (6 more replies)
0 siblings, 7 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-13 21:47 UTC (permalink / raw)
To: u-boot
Hi!
This is v2 of [1]
Changes since v1:
- minor coding style fixes from Heinrich
- changed the DP format. Instead of VenMedia - 0x01 - initrd, we skip
the 0x01 between VenMedia and the first file.
- final device path is stripped in efi_get_dp_from_boot() instead of
get_initrd_fp()
- Fixed comments on documentation
[1] https://lists.denx.de/pipermail/u-boot/2021-March/443399.html
Ilias Apalodimas (6):
efi_selftest: Remove loadfile2 for initrd selftests
efi_loader: Add device path related functions for initrd via Boot####
efi_loader: Introduce helper functions for EFI
efi_loader: Replace config option for initrd loading
efidebug: add multiple device path instances on Boot####
doc: Update uefi documentation for initrd loading options
cmd/bootefi.c | 3 +
cmd/efidebug.c | 194 ++++++++++++---
doc/board/emulation/qemu_capsule_update.rst | 4 +-
doc/uefi/uefi.rst | 23 +-
include/efi_helper.h | 15 ++
include/efi_loader.h | 7 +
lib/efi_loader/Kconfig | 15 +-
lib/efi_loader/Makefile | 1 +
lib/efi_loader/efi_bootmgr.c | 19 +-
lib/efi_loader/efi_device_path.c | 99 +++++++-
lib/efi_loader/efi_helper.c | 133 +++++++++++
lib/efi_loader/efi_load_initrd.c | 189 +++++++++------
lib/efi_loader/efi_var_common.c | 33 +++
lib/efi_selftest/Makefile | 1 -
lib/efi_selftest/efi_selftest_load_initrd.c | 221 ------------------
.../test_efi_capsule/test_capsule_firmware.py | 6 +-
test/py/tests/test_efi_secboot/test_signed.py | 16 +-
.../test_efi_secboot/test_signed_intca.py | 8 +-
.../tests/test_efi_secboot/test_unsigned.py | 8 +-
19 files changed, 618 insertions(+), 377 deletions(-)
create mode 100644 include/efi_helper.h
create mode 100644 lib/efi_loader/efi_helper.c
delete mode 100644 lib/efi_selftest/efi_selftest_load_initrd.c
--
2.30.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/6 v2] efi_selftest: Remove loadfile2 for initrd selftests
2021-03-13 21:47 [PATCH 0/6 v2] Loadfile2 for initrd loading Ilias Apalodimas
@ 2021-03-13 21:47 ` Ilias Apalodimas
2021-03-13 21:47 ` [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot#### Ilias Apalodimas
` (5 subsequent siblings)
6 siblings, 0 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-13 21:47 UTC (permalink / raw)
To: u-boot
We are redefining how u-boot locates the initrd to load via the kernel
LoadFile2 protocol. This selftest is not relevant any more, so remove
it. A new one will be added later
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
lib/efi_selftest/Makefile | 1 -
lib/efi_selftest/efi_selftest_load_initrd.c | 221 --------------------
2 files changed, 222 deletions(-)
delete mode 100644 lib/efi_selftest/efi_selftest_load_initrd.c
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index b02fd56e0a79..50de581b7763 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -61,7 +61,6 @@ obj-$(CONFIG_CPU_V7) += efi_selftest_unaligned.o
obj-$(CONFIG_EFI_LOADER_HII) += efi_selftest_hii.o
obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_selftest_rng.o
obj-$(CONFIG_EFI_GET_TIME) += efi_selftest_rtc.o
-obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_selftest_load_initrd.o
obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_selftest_tcg2.o
ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
diff --git a/lib/efi_selftest/efi_selftest_load_initrd.c b/lib/efi_selftest/efi_selftest_load_initrd.c
deleted file mode 100644
index f591dcd2115e..000000000000
--- a/lib/efi_selftest/efi_selftest_load_initrd.c
+++ /dev/null
@@ -1,221 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * efi_selftest_load_initrd
- *
- * Copyright (c) 2020 Ilias Apalodimas <ilias.apalodimas@linaro.org>
- *
- * This test checks the FileLoad2 protocol.
- * A known file is read from the file system and verified.
- *
- * An example usage - given a file image with a file system in partition 1
- * holding file initrd - is:
- *
- * * Configure the sandbox with
- *
- * CONFIG_EFI_SELFTEST=y
- * CONFIG_EFI_LOAD_FILE2_INITRD=y
- * CONFIG_EFI_INITRD_FILESPEC="host 0:1 initrd"
- *
- * * Run ./u-boot and execute
- *
- * host bind 0 image
- * setenv efi_selftest load initrd
- * bootefi selftest
- *
- * This would provide a test output like:
- *
- * Testing EFI API implementation
- *
- * Selected test: 'load initrd'
- *
- * Setting up 'load initrd'
- * Setting up 'load initrd' succeeded
- *
- * Executing 'load initrd'
- * Loaded 12378613 bytes
- * CRC32 2997478465
- *
- * Now the size and CRC32 can be compared to the provided file.
- */
-
-#include <efi_selftest.h>
-#include <efi_loader.h>
-#include <efi_load_initrd.h>
-
-static struct efi_boot_services *boottime;
-
-static struct efi_initrd_dp dp = {
- .vendor = {
- {
- DEVICE_PATH_TYPE_MEDIA_DEVICE,
- DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
- sizeof(dp.vendor),
- },
- EFI_INITRD_MEDIA_GUID,
- },
- .end = {
- DEVICE_PATH_TYPE_END,
- DEVICE_PATH_SUB_TYPE_END,
- sizeof(dp.end),
- }
-};
-
-static struct efi_initrd_dp dp_invalid = {
- .vendor = {
- {
- DEVICE_PATH_TYPE_MEDIA_DEVICE,
- DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
- sizeof(dp.vendor),
- },
- EFI_INITRD_MEDIA_GUID,
- },
- .end = {
- 0x8f, /* invalid */
- 0xfe, /* invalid */
- sizeof(dp.end),
- }
-};
-
-static int setup(const efi_handle_t handle,
- const struct efi_system_table *systable)
-{
- boottime = systable->boottime;
-
- return EFI_ST_SUCCESS;
-}
-
-static int execute(void)
-{
- struct efi_load_file_protocol *lf2;
- struct efi_device_path *dp2, *dp2_invalid;
- efi_status_t status;
- efi_handle_t handle;
- char buffer[64];
- efi_uintn_t buffer_size;
- void *buf;
- u32 crc32;
-
- memset(buffer, 0, sizeof(buffer));
-
- dp2 = (struct efi_device_path *)&dp;
- status = boottime->locate_device_path(&efi_guid_load_file2_protocol,
- &dp2, &handle);
- if (status != EFI_SUCCESS) {
- efi_st_error("Unable to locate device path\n");
- return EFI_ST_FAILURE;
- }
-
- status = boottime->handle_protocol(handle,
- &efi_guid_load_file2_protocol,
- (void **)&lf2);
- if (status != EFI_SUCCESS) {
- efi_st_error("Unable to locate protocol\n");
- return EFI_ST_FAILURE;
- }
-
- /* Case 1:
- * buffer_size can't be NULL
- * protocol can't be NULL
- */
- status = lf2->load_file(lf2, dp2, false, NULL, &buffer);
- if (status != EFI_INVALID_PARAMETER) {
- efi_st_error("Buffer size can't be NULL\n");
- return EFI_ST_FAILURE;
- }
- buffer_size = sizeof(buffer);
- status = lf2->load_file(NULL, dp2, false, &buffer_size, &buffer);
- if (status != EFI_INVALID_PARAMETER) {
- efi_st_error("Protocol can't be NULL\n");
- return EFI_ST_FAILURE;
- }
-
- /*
- * Case 2: Match end node type/sub-type on device path
- */
- dp2_invalid = (struct efi_device_path *)&dp_invalid;
- buffer_size = sizeof(buffer);
- status = lf2->load_file(lf2, dp2_invalid, false, &buffer_size, &buffer);
- if (status != EFI_INVALID_PARAMETER) {
- efi_st_error("Invalid device path type must return EFI_INVALID_PARAMETER\n");
- return EFI_ST_FAILURE;
- }
-
- status = lf2->load_file(lf2, dp2_invalid, false, &buffer_size, &buffer);
- if (status != EFI_INVALID_PARAMETER) {
- efi_st_error("Invalid device path sub-type must return EFI_INVALID_PARAMETER\n");
- return EFI_ST_FAILURE;
- }
-
- /*
- * Case 3:
- * BootPolicy 'true' must return EFI_UNSUPPORTED
- */
- buffer_size = sizeof(buffer);
- status = lf2->load_file(lf2, dp2, true, &buffer_size, &buffer);
- if (status != EFI_UNSUPPORTED) {
- efi_st_error("BootPolicy true must return EFI_UNSUPPORTED\n");
- return EFI_ST_FAILURE;
- }
-
- /*
- * Case: Pass buffer size as zero, firmware must return
- * EFI_BUFFER_TOO_SMALL and an appropriate size
- */
- buffer_size = 0;
- status = lf2->load_file(lf2, dp2, false, &buffer_size, NULL);
- if (status != EFI_BUFFER_TOO_SMALL || !buffer_size) {
- efi_st_printf("buffer_size: %u\n", (unsigned int)buffer_size);
- efi_st_printf("status: %x\n", (unsigned int)status);
- efi_st_error("Buffer size not updated\n");
- return EFI_ST_FAILURE;
- }
-
- /*
- * Case: Pass buffer size as smaller than the file_size,
- * firmware must return * EFI_BUFFER_TOO_SMALL and an appropriate size
- */
- buffer_size = 1;
- status = lf2->load_file(lf2, dp2, false, &buffer_size, &buffer);
- if (status != EFI_BUFFER_TOO_SMALL || buffer_size <= 1) {
- efi_st_error("Buffer size not updated\n");
- return EFI_ST_FAILURE;
- }
-
- status = boottime->allocate_pool(EFI_BOOT_SERVICES_DATA, buffer_size,
- &buf);
- if (status != EFI_SUCCESS) {
- efi_st_error("Cannot allocate buffer\n");
- return EFI_ST_FAILURE;
- }
-
- /* Case: Pass correct buffer, load the file and verify checksum*/
- status = lf2->load_file(lf2, dp2, false, &buffer_size, buf);
- if (status != EFI_SUCCESS) {
- efi_st_error("Loading initrd failed\n");
- return EFI_ST_FAILURE;
- }
-
- efi_st_printf("Loaded %u bytes\n", (unsigned int)buffer_size);
- status = boottime->calculate_crc32(buf, buffer_size, &crc32);
- if (status != EFI_SUCCESS) {
- efi_st_error("Could not determine CRC32\n");
- return EFI_ST_FAILURE;
- }
- efi_st_printf("CRC32 %.8x\n", (unsigned int)crc32);
-
- status = boottime->free_pool(buf);
- if (status != EFI_SUCCESS) {
- efi_st_error("Cannot free buffer\n");
- return EFI_ST_FAILURE;
- }
-
- return EFI_ST_SUCCESS;
-}
-
-EFI_UNIT_TEST(load_initrd) = {
- .name = "load initrd",
- .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
- .setup = setup,
- .execute = execute,
- .on_request = true,
-};
--
2.30.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
2021-03-13 21:47 [PATCH 0/6 v2] Loadfile2 for initrd loading Ilias Apalodimas
2021-03-13 21:47 ` [PATCH 1/6 v2] efi_selftest: Remove loadfile2 for initrd selftests Ilias Apalodimas
@ 2021-03-13 21:47 ` Ilias Apalodimas
2021-03-14 7:19 ` Heinrich Schuchardt
2021-03-13 21:47 ` [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI Ilias Apalodimas
` (4 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-13 21:47 UTC (permalink / raw)
To: u-boot
On the following patches we allow for an initrd path to be stored in
Boot#### variables. Specifically we encode in the FIlePathList[] of
the EFI_LOAD_OPTIONS for each Boot#### variable.
The FilePathList[] array looks like this:
kernel - 0xff - VenMedia(initrd GUID) - initrd1 - 0x01 initrd2 - 0xff
So let's add the relevant functions to concatenate and retrieve a device
path based on a Vendor GUID.
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
include/efi_loader.h | 4 ++
lib/efi_loader/efi_device_path.c | 99 ++++++++++++++++++++++++++++++--
2 files changed, 98 insertions(+), 5 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index f470bbd636f4..eb11a8c7d4b1 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -738,6 +738,10 @@ struct efi_load_option {
const u8 *optional_data;
};
+struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
+ efi_uintn_t *size, efi_guid_t guid);
+struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2);
efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data,
efi_uintn_t *size);
unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index c9315dd45857..1f55c772dc83 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -282,11 +282,25 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp)
return ndp;
}
-struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
- const struct efi_device_path *dp2)
+/** _efi_dp_append() - Append or concatenate two device paths.
+ * Concatenated device path will be separated by a 0xff end
+ * node.
+ *
+ * @dp1: First device path
+ * @dp2: Second device path
+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ * value
+ */
+static struct efi_device_path *_efi_dp_append(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2,
+ bool concat)
{
struct efi_device_path *ret;
+ size_t end_size = sizeof(END);
+ if (concat)
+ end_size = 2 * sizeof(END);
if (!dp1 && !dp2) {
/* return an end node */
ret = efi_dp_dup(&END);
@@ -298,18 +312,56 @@ struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
/* both dp1 and dp2 are non-null */
unsigned sz1 = efi_dp_size(dp1);
unsigned sz2 = efi_dp_size(dp2);
- void *p = dp_alloc(sz1 + sz2 + sizeof(END));
+ void *p = dp_alloc(sz1 + sz2 + end_size);
if (!p)
return NULL;
+ ret = p;
memcpy(p, dp1, sz1);
+ p += sz1;
+
+ if (concat) {
+ memcpy(p, &END, sizeof(END));
+ p += sizeof(END);
+ }
+
/* the end node of the second device path has to be retained */
- memcpy(p + sz1, dp2, sz2 + sizeof(END));
- ret = p;
+ memcpy(p, dp2, sz2);
+ p += sz2;
+ memcpy(p, &END, sizeof(END));
}
return ret;
}
+/** efi_dp_append() - Append a device to an existing device path.
+ *
+ * @dp1: First device path
+ * @dp2: Second device path
+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ * value
+ */
+struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2)
+{
+ return _efi_dp_append(dp1, dp2, false);
+}
+
+/** efi_dp_concat() - Concatenate 2 device paths. The final device path will
+ * contain two device paths separated by and end node (0xff).
+ *
+ * @dp1: First device path
+ * @dp2: Second device path
+ *
+ * Return: concatenated device path or NULL. Caller must free the returned
+ * value
+ */
+struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
+ const struct efi_device_path *dp2)
+{
+ return _efi_dp_append(dp1, dp2, true);
+}
+
struct efi_device_path *efi_dp_append_node(const struct efi_device_path *dp,
const struct efi_device_path *node)
{
@@ -1160,3 +1212,40 @@ ssize_t efi_dp_check_length(const struct efi_device_path *dp,
dp = (const struct efi_device_path *)((const u8 *)dp + len);
}
}
+
+/**
+ * efi_dp_from_lo() - Get the instance of a VenMedia node in a
+ * multi-instance device path that matches
+ * a specific GUID. This kind of device paths
+ * is found in Boot#### options describing an
+ * initrd location
+ *
+ * @load_option: EFI_LOAD_OPTION containing a valid device path
+ * @size: size of the discovered device path
+ * @guid: guid to search for
+ *
+ * Return: device path including the VenMedia node or NULL.
+ * Caller must free the returned value
+ */
+struct
+efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
+ efi_uintn_t *size, efi_guid_t guid)
+{
+ struct efi_device_path *fp = lo->file_path;
+ struct efi_device_path_vendor *vendor;
+ int lo_len = lo->file_path_length;
+
+ for (; lo_len >= sizeof(struct efi_device_path);
+ lo_len -= fp->length, fp = (void *)fp + fp->length) {
+ if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
+ fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH)
+ continue;
+
+ vendor = (struct efi_device_path_vendor *)fp;
+ if (!guidcmp(&vendor->guid, &guid))
+ return efi_dp_dup(fp);
+ }
+ log_debug("VenMedia(%pUl) not found in %ls\n", &guid, lo->label);
+
+ return NULL;
+}
--
2.30.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI
2021-03-13 21:47 [PATCH 0/6 v2] Loadfile2 for initrd loading Ilias Apalodimas
2021-03-13 21:47 ` [PATCH 1/6 v2] efi_selftest: Remove loadfile2 for initrd selftests Ilias Apalodimas
2021-03-13 21:47 ` [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot#### Ilias Apalodimas
@ 2021-03-13 21:47 ` Ilias Apalodimas
2021-03-14 7:31 ` Heinrich Schuchardt
` (2 more replies)
2021-03-13 21:47 ` [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading Ilias Apalodimas
` (3 subsequent siblings)
6 siblings, 3 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-13 21:47 UTC (permalink / raw)
To: u-boot
A following patch introduces a different logic for loading initrd's
based on the EFI_LOAD_FILE2_PROTOCOL.
Since similar logic can be applied in the future for other system files
(i.e DTBs), let's add some helper functions which will retrieve and
parse file paths stored in EFI variables.
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
include/efi_helper.h | 15 ++++
include/efi_loader.h | 2 +
lib/efi_loader/Makefile | 1 +
lib/efi_loader/efi_helper.c | 133 ++++++++++++++++++++++++++++++++
lib/efi_loader/efi_var_common.c | 33 ++++++++
5 files changed, 184 insertions(+)
create mode 100644 include/efi_helper.h
create mode 100644 lib/efi_loader/efi_helper.c
diff --git a/include/efi_helper.h b/include/efi_helper.h
new file mode 100644
index 000000000000..97492c65b6d2
--- /dev/null
+++ b/include/efi_helper.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#if !defined _EFI_HELPER_H_
+#define _EFI_HELPER_H
+
+#include <efi.h>
+#include <efi_api.h>
+
+efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size);
+struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
+
+#endif
diff --git a/include/efi_loader.h b/include/efi_loader.h
index eb11a8c7d4b1..aa812a9a3052 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -717,6 +717,8 @@ efi_status_t EFIAPI efi_query_variable_info(
u64 *remaining_variable_storage_size,
u64 *maximum_variable_size);
+void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);
+
/*
* See section 3.1.3 in the v2.7 UEFI spec for more details on
* the layout of EFI_LOAD_OPTION. In short it is:
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 10b42e8847bf..da2741adecfa 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -23,6 +23,7 @@ endif
obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
obj-y += efi_boottime.o
+obj-y += efi_helper.o
obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
obj-y += efi_console.o
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
new file mode 100644
index 000000000000..df5bdc506dbe
--- /dev/null
+++ b/lib/efi_loader/efi_helper.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#define LOG_CATEGORY LOGC_EFI
+#include <common.h>
+#include <env.h>
+#include <malloc.h>
+#include <dm.h>
+#include <fs.h>
+#include <efi_helper.h>
+#include <efi_load_initrd.h>
+#include <efi_loader.h>
+#include <efi_variable.h>
+
+/**
+ * efi_create_current_boot_var() - Return Boot#### name were #### is replaced by
+ * the value of BootCurrent
+ *
+ * @var_name: variable name
+ * @var_name_size: size of var_name
+ *
+ * Return: Status code
+ */
+static efi_status_t efi_create_current_boot_var(u16 var_name[],
+ size_t var_name_size)
+{
+ efi_uintn_t boot_current_size;
+ efi_status_t ret;
+ u16 boot_current;
+ u16 *pos;
+
+ boot_current_size = sizeof(boot_current);
+ ret = efi_get_variable_int(L"BootCurrent",
+ &efi_global_variable_guid, NULL,
+ &boot_current_size, &boot_current, NULL);
+ if (ret != EFI_SUCCESS)
+ goto out;
+
+ pos = efi_create_indexed_name(var_name, var_name_size, "Boot",
+ boot_current);
+ if (!pos) {
+ ret = EFI_OUT_OF_RESOURCES;
+ goto out;
+ }
+
+out:
+ return ret;
+}
+
+/**
+ * efi_get_dp_from_boot() - Retrieve and return a device path from an EFI
+ * Boot### variable
+ *
+ * @guid: Vendor guid of the VenMedia DP
+ *
+ * Return: device path or NULL. Caller must free the returned value
+ */
+struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid)
+{
+ struct efi_device_path *file_path = NULL;
+ struct efi_device_path *tmp;
+ struct efi_load_option lo;
+ void *var_value = NULL;
+ efi_uintn_t size;
+ efi_status_t ret;
+ u16 var_name[16];
+
+ ret = efi_create_current_boot_var(var_name, sizeof(var_name));
+ if (ret != EFI_SUCCESS)
+ return NULL;
+
+ var_value = efi_get_var(var_name, &efi_global_variable_guid, &size);
+ if (!var_value)
+ return NULL;
+
+ ret = efi_deserialize_load_option(&lo, var_value, &size);
+ if (ret != EFI_SUCCESS)
+ goto out;
+
+ tmp = efi_dp_from_lo(&lo, &size, guid);
+ if (!tmp)
+ goto out;
+
+ /* efi_dp_dup will just return NULL if efi_dp_next is NULL */
+ file_path = efi_dp_dup(efi_dp_next(tmp));
+
+out:
+ efi_free_pool(tmp);
+ free(var_value);
+
+ return file_path;
+}
+
+/**
+ * efi_get_file_size() - Get the size of a file using an EFI file handle
+ *
+ * @handle: EFI file handle
+ * @size: buffer to fill in the discovered size
+ *
+ * Return: device path or NULL. Caller must free the returned value
+ */
+efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)
+{
+ struct efi_file_info *info = NULL;
+ efi_uintn_t bs = 0;
+ efi_status_t ret;
+
+ *size = 0;
+ ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
+ info));
+ if (ret != EFI_BUFFER_TOO_SMALL) {
+ ret = EFI_DEVICE_ERROR;
+ goto out;
+ }
+
+ info = malloc(bs);
+ if (!info) {
+ ret = EFI_OUT_OF_RESOURCES;
+ goto out;
+ }
+ ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
+ info));
+ if (ret != EFI_SUCCESS)
+ goto out;
+
+ *size = info->file_size;
+
+out:
+ free(info);
+ return ret;
+}
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 1c7459266a38..b11ed91a74a4 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -9,6 +9,7 @@
#include <common.h>
#include <efi_loader.h>
#include <efi_variable.h>
+#include <stdlib.h>
enum efi_secure_mode {
EFI_MODE_SETUP,
@@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)
}
return EFI_AUTH_VAR_NONE;
}
+
+/**
+ * efi_get_var() - read value of an EFI variable
+ *
+ * @name: variable name
+ * @start: vendor GUID
+ * @size: size of allocated buffer
+ *
+ * Return: buffer with variable data or NULL
+ */
+void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
+{
+ efi_status_t ret;
+ void *buf = NULL;
+
+ *size = 0;
+ ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ buf = malloc(*size);
+ if (!buf)
+ return NULL;
+ ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
+ }
+
+ if (ret != EFI_SUCCESS) {
+ free(buf);
+ *size = 0;
+ return NULL;
+ }
+
+ return buf;
+}
--
2.30.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading
2021-03-13 21:47 [PATCH 0/6 v2] Loadfile2 for initrd loading Ilias Apalodimas
` (2 preceding siblings ...)
2021-03-13 21:47 ` [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI Ilias Apalodimas
@ 2021-03-13 21:47 ` Ilias Apalodimas
2021-03-14 9:54 ` Heinrich Schuchardt
` (2 more replies)
2021-03-13 21:47 ` [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot#### Ilias Apalodimas
` (2 subsequent siblings)
6 siblings, 3 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-13 21:47 UTC (permalink / raw)
To: u-boot
Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd
unconditionally. Although we correctly return various EFI exit codes
depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the
kernel loader, only falls back to the cmdline interpreted initrd if the
protocol is not installed.
This creates a problem for EFI installers, since they won't be able to
load their own initrd and continue the installation. It also makes the
feature hard to use, since we can either have a single initrd or we have
to recompile u-boot if the filename changes.
So let's introduce a different logic that will decouple the initrd
path from the config option we currently have.
When defining a UEFI BootXXXX we can use the filepathlist and store
a file path pointing to our initrd. Specifically the EFI spec describes:
"The first element of the array is a device path that describes the device
and location of the Image for this load option. Other device paths may
optionally exist in the FilePathList, but their usage is OSV specific".
That means we can install a device path to our initrd(s) after the loaded
image looking like this:
VenMedia - initrd1 - end node (0x01) initrd2 - end node (0xff)
When the EFI application is launched through the bootmgr, we'll try to
interpret the extra device path. If that points to a file that exists on
our disk, we'll now install the load_file2 and the efi-stub will be able
to use it.
This opens up another path using U-Boot and defines a new boot flow.
A user will be able to control the kernel/initrd pairs without explicit
cmdline args or GRUB.
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
cmd/bootefi.c | 3 +
include/efi_loader.h | 1 +
lib/efi_loader/Kconfig | 15 +--
lib/efi_loader/efi_bootmgr.c | 19 +++-
lib/efi_loader/efi_load_initrd.c | 189 ++++++++++++++++++-------------
5 files changed, 136 insertions(+), 91 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 271b385edea6..cba81ffe75e4 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
free(load_options);
+ if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
+ efi_initrd_deregister();
+
return ret;
}
diff --git a/include/efi_loader.h b/include/efi_loader.h
index aa812a9a3052..9e57eb37ff28 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -431,6 +431,7 @@ efi_status_t efi_net_register(void);
/* Called by bootefi to make the watchdog available */
efi_status_t efi_watchdog_register(void);
efi_status_t efi_initrd_register(void);
+void efi_initrd_deregister(void);
/* Called by bootefi to make SMBIOS tables available */
/**
* efi_acpi_register() - write out ACPI tables
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e729f727df11..029f0e515f57 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD
bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
default n
help
- Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
- use to load the initial ramdisk. Once this is enabled using
- initrd=<ramdisk> will stop working.
-
-config EFI_INITRD_FILESPEC
- string "initramfs path"
- default "host 0:1 initrd"
- depends on EFI_LOAD_FILE2_INITRD
- help
- Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
+ Linux v5.7 and later can make use of this option. If the boot option
+ selected by the UEFI boot manager specifies an existing file to be used
+ as initial RAM disk, a Linux specific Load File2 protocol will be
+ installed and Linux 5.7+ will ignore any initrd=<ramdisk> command line
+ argument.
config EFI_SECURE_BOOT
bool "Enable EFI secure boot support"
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 25f5cebfdb67..46c8011344b9 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -118,11 +118,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
ret = efi_set_variable_int(L"BootCurrent",
&efi_global_variable_guid,
attributes, sizeof(n), &n, false);
- if (ret != EFI_SUCCESS) {
- if (EFI_CALL(efi_unload_image(*handle))
- != EFI_SUCCESS)
- log_err("Unloading image failed\n");
- goto error;
+ if (ret != EFI_SUCCESS)
+ goto unload;
+ /* try to register load file2 for initrd's */
+ if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
+ ret = efi_initrd_register();
+ if (ret != EFI_SUCCESS)
+ goto unload;
}
log_info("Booting: %ls\n", lo.label);
@@ -146,6 +148,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
error:
free(load_option);
+ return ret;
+
+unload:
+ if (EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
+ log_err("Unloading image failed\n");
+ free(load_option);
+
return ret;
}
diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
index b9ee8839054f..e76de30808e3 100644
--- a/lib/efi_loader/efi_load_initrd.c
+++ b/lib/efi_loader/efi_load_initrd.c
@@ -5,7 +5,9 @@
#include <common.h>
#include <efi_loader.h>
+#include <efi_helper.h>
#include <efi_load_initrd.h>
+#include <efi_variable.h>
#include <fs.h>
#include <malloc.h>
#include <mapmem.h>
@@ -39,41 +41,40 @@ static const struct efi_initrd_dp dp = {
}
};
+static efi_handle_t efi_initrd_handle;
+
/**
- * get_file_size() - retrieve the size of initramfs, set efi status on error
+ * get_initrd_fp() - Get initrd device path from a FilePathList device path
*
- * @dev: device to read from, e.g. "mmc"
- * @part: device partition, e.g. "0:1"
- * @file: name of file
- * @status: EFI exit code in case of failure
+ * @initrd_fp: the final initrd filepath
*
- * Return: size of file
+ * Return: status code. Caller must free initrd_fp
*/
-static loff_t get_file_size(const char *dev, const char *part, const char *file,
- efi_status_t *status)
+static efi_status_t get_initrd_fp(struct efi_device_path **initrd_fp)
{
- loff_t sz = 0;
- int ret;
-
- ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
- if (ret) {
- *status = EFI_NO_MEDIA;
- goto out;
- }
+ const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
+ struct efi_device_path *dp = NULL;
- ret = fs_size(file, &sz);
- if (ret) {
- sz = 0;
- *status = EFI_NOT_FOUND;
- goto out;
- }
+ /*
+ * if bootmgr is setup with and initrd, the device path will be
+ * in the FilePathList[] of our load options in Boot####.
+ * The first device path of the multi instance device path will
+ * start with a VenMedia and the initrds will follow.
+ *
+ * If the device path is not found return EFI_INVALID_PARAMETER.
+ * We can then use this specific return value and not install the
+ * protocol, while allowing the boot to continue
+ */
+ dp = efi_get_dp_from_boot(lf2_initrd_guid);
+ if (!dp)
+ return EFI_INVALID_PARAMETER;
-out:
- return sz;
+ *initrd_fp = dp;
+ return EFI_SUCCESS;
}
/**
- * efi_load_file2initrd() - load initial RAM disk
+ * efi_load_file2_initrd() - load initial RAM disk
*
* This function implements the LoadFile service of the EFI_LOAD_FILE2_PROTOCOL
* in order to load an initial RAM disk requested by the Linux kernel stub.
@@ -93,98 +94,120 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
struct efi_device_path *file_path, bool boot_policy,
efi_uintn_t *buffer_size, void *buffer)
{
- char *filespec;
- efi_status_t status = EFI_NOT_FOUND;
- loff_t file_sz = 0, read_sz = 0;
- char *dev, *part, *file;
- char *pos;
- int ret;
+ struct efi_device_path *initrd_fp = NULL;
+ efi_status_t ret = EFI_NOT_FOUND;
+ struct efi_file_handle *f;
+ efi_uintn_t bs;
EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
buffer_size, buffer);
- filespec = strdup(CONFIG_EFI_INITRD_FILESPEC);
- if (!filespec)
- goto out;
- pos = filespec;
-
if (!this || this != &efi_lf2_protocol ||
!buffer_size) {
- status = EFI_INVALID_PARAMETER;
+ ret = EFI_INVALID_PARAMETER;
goto out;
}
if (file_path->type != dp.end.type ||
file_path->sub_type != dp.end.sub_type) {
- status = EFI_INVALID_PARAMETER;
+ ret = EFI_INVALID_PARAMETER;
goto out;
}
if (boot_policy) {
- status = EFI_UNSUPPORTED;
+ ret = EFI_UNSUPPORTED;
goto out;
}
- /*
- * expect a string with three space separated parts:
- *
- * * a block device type, e.g. "mmc"
- * * a device and partition identifier, e.g. "0:1"
- * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
- */
- dev = strsep(&pos, " ");
- if (!dev)
+ ret = get_initrd_fp(&initrd_fp);
+ if (ret != EFI_SUCCESS)
goto out;
- part = strsep(&pos, " ");
- if (!part)
- goto out;
- file = strsep(&pos, " ");
- if (!file)
+
+ /* Open file */
+ f = efi_file_from_path(initrd_fp);
+ if (!f) {
+ EFI_PRINT("Can't find initrd specified in Boot####\n");
+ ret = EFI_NOT_FOUND;
goto out;
+ }
- file_sz = get_file_size(dev, part, file, &status);
- if (!file_sz)
+ /* Get file size */
+ ret = efi_get_file_size(f, &bs);
+ if (ret != EFI_SUCCESS)
goto out;
- if (!buffer || *buffer_size < file_sz) {
- status = EFI_BUFFER_TOO_SMALL;
- *buffer_size = file_sz;
+ if (!buffer || *buffer_size < bs) {
+ ret = EFI_BUFFER_TOO_SMALL;
+ *buffer_size = bs;
} else {
- ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
- if (ret) {
- status = EFI_NO_MEDIA;
- goto out;
- }
-
- ret = fs_read(file, map_to_sysmem(buffer), 0, *buffer_size,
- &read_sz);
- if (ret || read_sz != file_sz)
- goto out;
- *buffer_size = read_sz;
-
- status = EFI_SUCCESS;
+ ret = EFI_CALL(f->read(f, &bs, (void *)(uintptr_t)buffer));
+ *buffer_size = bs;
+ }
+
+out:
+ efi_free_pool(initrd_fp);
+ EFI_CALL(f->close(f));
+ return EFI_EXIT(ret);
+}
+
+/**
+ * check_initrd() - Determine if the file defined as an initrd in Boot####
+ * load_options device path is present
+ *
+ * Return: status code
+ */
+static efi_status_t check_initrd(void)
+{
+ struct efi_device_path *initrd_fp = NULL;
+ struct efi_file_handle *f;
+ efi_status_t ret;
+
+ ret = get_initrd_fp(&initrd_fp);
+ if (ret != EFI_SUCCESS)
+ goto out;
+
+ /*
+ * If the file is not found, but the file path is set, return an error
+ * and trigger the bootmgr fallback
+ */
+ f = efi_file_from_path(initrd_fp);
+ if (!f) {
+ EFI_PRINT("Can't find initrd specified in Boot####\n");
+ ret = EFI_NOT_FOUND;
+ goto out;
}
+ EFI_CALL(f->close(f));
+
out:
- free(filespec);
- return EFI_EXIT(status);
+ efi_free_pool(initrd_fp);
+ return ret;
}
/**
* efi_initrd_register() - create handle for loading initial RAM disk
*
* This function creates a new handle and installs a Linux specific vendor
- * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path
+ * device path and an EFI_LOAD_FILE2_PROTOCOL. Linux uses the device path
* to identify the handle and then calls the LoadFile service of the
- * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk.
+ * EFI_LOAD_FILE2_PROTOCOL to read the initial RAM disk.
*
* Return: status code
*/
efi_status_t efi_initrd_register(void)
{
- efi_handle_t efi_initrd_handle = NULL;
efi_status_t ret;
+ /*
+ * Allow the user to continue if Boot#### file path is not set for
+ * an initrd
+ */
+ ret = check_initrd();
+ if (ret == EFI_INVALID_PARAMETER)
+ return EFI_SUCCESS;
+ if (ret != EFI_SUCCESS)
+ return ret;
+
ret = EFI_CALL(efi_install_multiple_protocol_interfaces
(&efi_initrd_handle,
/* initramfs */
@@ -196,3 +219,17 @@ efi_status_t efi_initrd_register(void)
return ret;
}
+
+/**
+ * efi_initrd_register() - delete the handle for loading initial RAM disk
+ *
+ * This will delete the handle containing the Linux specific vendor device
+ * path and EFI_LOAD_FILE2_PROTOCOL for loading an initrd
+ *
+ * Return: status code
+ */
+void efi_initrd_deregister(void)
+{
+ efi_delete_handle(efi_initrd_handle);
+ efi_initrd_handle = NULL;
+}
--
2.30.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot####
2021-03-13 21:47 [PATCH 0/6 v2] Loadfile2 for initrd loading Ilias Apalodimas
` (3 preceding siblings ...)
2021-03-13 21:47 ` [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading Ilias Apalodimas
@ 2021-03-13 21:47 ` Ilias Apalodimas
2021-03-14 9:27 ` Heinrich Schuchardt
2021-03-14 10:14 ` Heinrich Schuchardt
2021-03-13 21:47 ` [PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options Ilias Apalodimas
2021-03-14 8:41 ` [PATCH 0/6 v2] Loadfile2 for initrd loading Heinrich Schuchardt
6 siblings, 2 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-13 21:47 UTC (permalink / raw)
To: u-boot
The UEFI spec allow a packed array of UEFI device paths in the
FilePathList[] of an EFI_LOAD_OPTION. The first file path must
describe the loaded image but the rest are OS specific.
Previous patches parse the device path and try to use the second
member of the array as an initrd. So let's modify efidebug slightly
and install the second file described in the command line as the
initrd device path.
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
cmd/efidebug.c | 194 ++++++++++++++----
doc/board/emulation/qemu_capsule_update.rst | 4 +-
doc/uefi/uefi.rst | 2 +-
.../test_efi_capsule/test_capsule_firmware.py | 6 +-
test/py/tests/test_efi_secboot/test_signed.py | 16 +-
.../test_efi_secboot/test_signed_intca.py | 8 +-
.../tests/test_efi_secboot/test_unsigned.py | 8 +-
7 files changed, 180 insertions(+), 58 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index bbbcb0a54643..223cffa389fb 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -9,6 +9,8 @@
#include <common.h>
#include <command.h>
#include <efi_dt_fixup.h>
+#include <efi_helper.h>
+#include <efi_load_initrd.h>
#include <efi_loader.h>
#include <efi_rng.h>
#include <exports.h>
@@ -19,6 +21,7 @@
#include <part.h>
#include <search.h>
#include <linux/ctype.h>
+#include <linux/err.h>
#define BS systab.boottime
#define RT systab.runtime
@@ -794,6 +797,66 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
return CMD_RET_SUCCESS;
}
+/**
+ * add_initrd_instance() - Append a device path to load_options pointing to an
+ * inirtd
+ *
+ * @dev: Device
+ * @part: Partition of thge disk
+ * @file: Filename
+ * @fp: Device Path containing the existing load_options
+ * @fp_size: New size of the device path after the addition
+ * Return: Pointer to the device path or ERR_PTR
+ *
+ */
+static
+struct efi_device_path *add_initrd_instance(const char *dev, const char *part,
+ const char *file,
+ const struct efi_device_path *fp,
+ efi_uintn_t *fp_size)
+{
+ struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL;
+ struct efi_device_path *final_fp = NULL, *initrd_dp = NULL;
+ efi_status_t ret;
+ const struct efi_initrd_dp id_dp = {
+ .vendor = {
+ {
+ DEVICE_PATH_TYPE_MEDIA_DEVICE,
+ DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
+ sizeof(id_dp.vendor),
+ },
+ EFI_INITRD_MEDIA_GUID,
+ },
+ .end = {
+ DEVICE_PATH_TYPE_END,
+ DEVICE_PATH_SUB_TYPE_END,
+ sizeof(id_dp.end),
+ }
+ };
+
+ ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp);
+ if (ret != EFI_SUCCESS) {
+ printf("Cannot create device path for \"%s %s\"\n", part, file);
+ goto out;
+ }
+
+ initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp,
+ tmp_fp);
+ if (!initrd_dp) {
+ printf("Cannot append media vendor device path path\n");
+ goto out;
+ }
+ final_fp = efi_dp_concat(fp, initrd_dp);
+ *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) +
+ (2 * sizeof(struct efi_device_path));
+
+out:
+ efi_free_pool(initrd_dp);
+ efi_free_pool(tmp_dp);
+ efi_free_pool(tmp_fp);
+ return final_fp ? final_fp : ERR_PTR(-EINVAL);
+}
+
/**
* do_efi_boot_add() - set UEFI load option
*
@@ -806,7 +869,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
*
* Implement efidebug "boot add" sub-command. Create or change UEFI load option.
*
- * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options>
+ * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file>
+ * -i <file> <interface2> <devnum2>[:<part>] <initrd>
+ * -s '<options>'
*/
static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
int argc, char *const argv[])
@@ -819,55 +884,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
size_t label_len, label_len16;
u16 *label;
struct efi_device_path *device_path = NULL, *file_path = NULL;
+ struct efi_device_path *final_fp = NULL;
struct efi_load_option lo;
void *data = NULL;
efi_uintn_t size;
+ efi_uintn_t fp_size;
efi_status_t ret;
int r = CMD_RET_SUCCESS;
-
- if (argc < 6 || argc > 7)
- return CMD_RET_USAGE;
-
- id = (int)simple_strtoul(argv[1], &endp, 16);
- if (*endp != '\0' || id > 0xffff)
- return CMD_RET_USAGE;
-
- sprintf(var_name, "Boot%04X", id);
- p = var_name16;
- utf8_utf16_strncpy(&p, var_name, 9);
+ int i;
guid = efi_global_variable_guid;
/* attributes */
lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */
+ lo.optional_data = NULL;
+ lo.label = NULL;
- /* label */
- label_len = strlen(argv[2]);
- label_len16 = utf8_utf16_strnlen(argv[2], label_len);
- label = malloc((label_len16 + 1) * sizeof(u16));
- if (!label)
- return CMD_RET_FAILURE;
- lo.label = label; /* label will be changed below */
- utf8_utf16_strncpy(&label, argv[2], label_len);
+ /* search for -b first since the rest of the arguments depends on that */
+ for (i = 0; i < argc; i++) {
+ if (!strcmp(argv[i], "-b")) {
+ if (argc < i + 5 || lo.label) {
+ r = CMD_RET_USAGE;
+ goto out;
+ }
+ id = (int)simple_strtoul(argv[i + 1], &endp, 16);
+ if (*endp != '\0' || id > 0xffff)
+ return CMD_RET_USAGE;
+
+ sprintf(var_name, "Boot%04X", id);
+ p = var_name16;
+ utf8_utf16_strncpy(&p, var_name, 9);
+
+ /* label */
+ label_len = strlen(argv[i + 2]);
+ label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len);
+ label = malloc((label_len16 + 1) * sizeof(u16));
+ if (!label)
+ return CMD_RET_FAILURE;
+ lo.label = label; /* label will be changed below */
+ utf8_utf16_strncpy(&label, argv[i + 2], label_len);
+
+ /* file path */
+ ret = efi_dp_from_name(argv[i + 3], argv[i + 4],
+ argv[i + 5], &device_path,
+ &file_path);
+ if (ret != EFI_SUCCESS) {
+ printf("Cannot create device path for \"%s %s\"\n",
+ argv[3], argv[4]);
+ r = CMD_RET_FAILURE;
+ goto out;
+ break;
+ }
+ fp_size = efi_dp_size(file_path) +
+ sizeof(struct efi_device_path);
+ }
+ }
- /* file path */
- ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
- &file_path);
- if (ret != EFI_SUCCESS) {
- printf("Cannot create device path for \"%s %s\"\n",
- argv[3], argv[4]);
+ if (!file_path) {
+ printf("You need to specify an image before an initrd.\n");
r = CMD_RET_FAILURE;
goto out;
}
- lo.file_path = file_path;
- lo.file_path_length = efi_dp_size(file_path)
- + sizeof(struct efi_device_path); /* for END */
- /* optional data */
- if (argc == 6)
- lo.optional_data = NULL;
- else
- lo.optional_data = (const u8 *)argv[6];
+ /* add now add initrd and extra data */
+ for (i = 0; i < argc; i++) {
+ if (!strcmp(argv[i], "-i")) {
+ if (argc < i + 3 || final_fp) {
+ r = CMD_RET_USAGE;
+ goto out;
+ }
+
+ final_fp = add_initrd_instance(argv[i + 1], argv[i + 2],
+ argv[i + 3], file_path,
+ &fp_size);
+ if (IS_ERR(final_fp)) {
+ r = CMD_RET_FAILURE;
+ goto out;
+ }
+
+ /* add_initrd_instance allocates a new device path */
+ efi_free_pool(file_path);
+ file_path = final_fp;
+ } else if (!strcmp(argv[i], "-s")) {
+ if (argc < i + 1 || lo.optional_data) {
+ r = CMD_RET_USAGE;
+ goto out;
+ }
+ lo.optional_data = (const u8 *)argv[i + 1];
+ }
+ }
+
+ lo.file_path = file_path;
+ lo.file_path_length = fp_size;
size = efi_serialize_load_option(&lo, (u8 **)&data);
if (!size) {
@@ -951,11 +1059,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag,
*/
static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size)
{
+ struct efi_device_path *initrd_path = NULL;
struct efi_load_option lo;
char *label, *p;
size_t label_len16, label_len;
u16 *dp_str;
efi_status_t ret;
+ efi_uintn_t initrd_dp_size;
+ const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
ret = efi_deserialize_load_option(&lo, data, size);
if (ret != EFI_SUCCESS) {
@@ -986,6 +1097,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size)
printf(" file_path: %ls\n", dp_str);
efi_free_pool(dp_str);
+ initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid);
+ if (initrd_path) {
+ dp_str = efi_dp_str(initrd_path);
+ printf(" initrd_path: %ls\n", dp_str);
+ efi_free_pool(dp_str);
+ efi_free_pool(initrd_path);
+ }
+
printf(" data:\n");
print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
lo.optional_data, *size, true);
@@ -1555,7 +1674,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag,
static char efidebug_help_text[] =
" - UEFI Shell-like interface to configure UEFI environment\n"
"\n"
- "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n"
+ "efidebug boot add "
+ "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> "
+ "-i <interface> <devnum>[:<part>] <initrd file path> "
+ "-s '<optional data>'\n"
" - set UEFI BootXXXX variable\n"
" <load options> will be passed to UEFI application\n"
"efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
@@ -1599,7 +1721,7 @@ static char efidebug_help_text[] =
#endif
U_BOOT_CMD(
- efidebug, 10, 0, do_efidebug,
+ efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug,
"Configure UEFI environment",
efidebug_help_text
);
diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst
index 9fec75f8f1c9..33ce4bcd32ea 100644
--- a/doc/board/emulation/qemu_capsule_update.rst
+++ b/doc/board/emulation/qemu_capsule_update.rst
@@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule
file. The BootNext, BootXXXX and OsIndications variables can be set
using the following commands::
- => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name>
+ => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
=> efidebug boot next 0
=> setenv -e -nv -bs -rt -v OsIndications =0x04
=> saveenv
@@ -198,7 +198,7 @@ command line::
3. Set the following environment and UEFI boot variables
=> setenv -e -nv -bs -rt -v OsIndications =0x04
- => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name>
+ => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
=> efidebug boot next 0
=> saveenv
diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
index 5a67737c1579..b3494c22e073 100644
--- a/doc/uefi/uefi.rst
+++ b/doc/uefi/uefi.rst
@@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board::
Set up boot parameters on your board::
- efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed ""
+ efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed ""
Now your board can run the signed image via the boot manager (see below).
You can also try this sequence by running Pytest, test_efi_secboot,
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
index f006fa95d650..e8b0a1575453 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
@@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object):
with u_boot_console.log.section('Test Case 1-a, before reboot'):
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
- 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
'efidebug boot order 1',
'env set -e OsIndications',
'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
@@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object):
with u_boot_console.log.section('Test Case 2-a, before reboot'):
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
- 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
'efidebug boot order 1',
'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
@@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object):
with u_boot_console.log.section('Test Case 3-a, before reboot'):
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
- 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
'efidebug boot order 1',
'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
index 863685e215b7..75f5ea772300 100644
--- a/test/py/tests/test_efi_secboot/test_signed.py
+++ b/test/py/tests/test_efi_secboot/test_signed.py
@@ -28,7 +28,7 @@ class TestEfiSignedImage(object):
# Test Case 1a, run signed image if no PK
output = u_boot_console.run_command_list([
'host bind 0 %s' % disk_img,
- 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
+ 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
'efidebug boot next 1',
'bootefi bootmgr'])
assert 'Hello, world!' in ''.join(output)
@@ -36,7 +36,7 @@ class TestEfiSignedImage(object):
with u_boot_console.log.section('Test Case 1b'):
# Test Case 1b, run unsigned image if no PK
output = u_boot_console.run_command_list([
- 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""',
'efidebug boot next 2',
'bootefi bootmgr'])
assert 'Hello, world!' in ''.join(output)
@@ -58,13 +58,13 @@ class TestEfiSignedImage(object):
'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
+ 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
'efidebug boot next 1',
'efidebug test bootmgr'])
assert('\'HELLO1\' failed' in ''.join(output))
assert('efi_start_image() returned: 26' in ''.join(output))
output = u_boot_console.run_command_list([
- 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""',
'efidebug boot next 2',
'efidebug test bootmgr'])
assert '\'HELLO2\' failed' in ''.join(output)
@@ -104,7 +104,7 @@ class TestEfiSignedImage(object):
'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
+ 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
'efidebug boot next 1',
'efidebug test bootmgr'])
assert '\'HELLO\' failed' in ''.join(output)
@@ -142,7 +142,7 @@ class TestEfiSignedImage(object):
'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
+ 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
'efidebug boot next 1',
'efidebug test bootmgr'])
assert '\'HELLO\' failed' in ''.join(output)
@@ -169,7 +169,7 @@ class TestEfiSignedImage(object):
'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""',
+ 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""',
'efidebug boot next 1',
'efidebug test bootmgr'])
assert 'Hello, world!' in ''.join(output)
@@ -227,7 +227,7 @@ class TestEfiSignedImage(object):
'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
+ 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
'efidebug boot next 1',
'bootefi bootmgr'])
assert 'Hello, world!' in ''.join(output)
diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
index 70d6be00e8a8..0849572a5143 100644
--- a/test/py/tests/test_efi_secboot/test_signed_intca.py
+++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
@@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object):
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
+ 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
'efidebug boot next 1',
'efidebug test bootmgr'])
assert '\'HELLO_a\' failed' in ''.join(output)
@@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object):
with u_boot_console.log.section('Test Case 1b'):
# Test Case 1b, signed and authenticated by root CA
output = u_boot_console.run_command_list([
- 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
+ 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
'efidebug boot next 2',
'bootefi bootmgr'])
assert 'Hello, world!' in ''.join(output)
@@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object):
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
+ 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
'efidebug boot next 1',
'efidebug test bootmgr'])
assert '\'HELLO_abc\' failed' in ''.join(output)
@@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object):
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
+ 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
'efidebug boot next 1',
'efidebug test bootmgr'])
assert 'Hello, world!' in ''.join(output)
diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py
index 56f56e19eb84..8e026f7566ad 100644
--- a/test/py/tests/test_efi_secboot/test_unsigned.py
+++ b/test/py/tests/test_efi_secboot/test_unsigned.py
@@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object):
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
'efidebug boot next 1',
'bootefi bootmgr'])
assert '\'HELLO\' failed' in ''.join(output)
@@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object):
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
'efidebug boot next 1',
'bootefi bootmgr'])
assert 'Hello, world!' in ''.join(output)
@@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object):
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
'efidebug boot next 1',
'bootefi bootmgr'])
assert '\'HELLO\' failed' in ''.join(output)
@@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object):
assert 'Failed to set EFI variable' not in ''.join(output)
output = u_boot_console.run_command_list([
- 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
+ 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
'efidebug boot next 1',
'bootefi bootmgr'])
assert '\'HELLO\' failed' in ''.join(output)
--
2.30.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options
2021-03-13 21:47 [PATCH 0/6 v2] Loadfile2 for initrd loading Ilias Apalodimas
` (4 preceding siblings ...)
2021-03-13 21:47 ` [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot#### Ilias Apalodimas
@ 2021-03-13 21:47 ` Ilias Apalodimas
2021-03-14 7:53 ` Heinrich Schuchardt
2021-03-14 8:41 ` [PATCH 0/6 v2] Loadfile2 for initrd loading Heinrich Schuchardt
6 siblings, 1 reply; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-13 21:47 UTC (permalink / raw)
To: u-boot
Document the command line options for efidebug and initrd loading
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
doc/uefi/uefi.rst | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
index b3494c22e073..76402bc5cfaa 100644
--- a/doc/uefi/uefi.rst
+++ b/doc/uefi/uefi.rst
@@ -180,6 +180,12 @@ Set up boot parameters on your board::
efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed ""
+Since kernel 5.7 there's an alternative way of loading an initrd using
+LoadFile2 protocol if CONFIG_EFI_LOAD_FILE2_INITRD is enabled.
+The initrd path can be specified with::
+
+ efidebug boot add -b ABE0 'kernel' mmc 0:1 Image -i mmc 0:1 initrd
+
Now your board can run the signed image via the boot manager (see below).
You can also try this sequence by running Pytest, test_efi_secboot,
on the sandbox
@@ -484,7 +490,20 @@ The load file 2 protocol can be used by the Linux kernel to load the initial
RAM disk. U-Boot can be configured to provide an implementation with::
EFI_LOAD_FILE2_INITRD=y
- EFI_INITRD_FILESPEC=interface dev:part path_to_initrd
+
+When the options is enabled the user can add the initrd path with the efidebug
+command.
+The Boot#### option has a FilePathList[] in it's EFI_LOAD_OPTION.
+The first element of the array (FilePathList[0]) is the loded image.
+When an initrd is specified the Device Path for the initrd is denoted by a
+VenMedia node with the EFI_INITRD_MEDIA_GUID. Each entry of the array is
+terminated by the 'end of entire device path' subtype. If the user wants to
+define multiple initrds those must by separated by the 'end of this instance'
+identifier of the end node (0x01).
+
+So our final format of the FilePathList[] is::
+
+ Loaded image - end node (0xff) - VenMedia - initrd1 - end node (0x01) initrd2 - end node (0xff)
Links
-----
--
2.30.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
2021-03-13 21:47 ` [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot#### Ilias Apalodimas
@ 2021-03-14 7:19 ` Heinrich Schuchardt
2021-03-14 7:32 ` Ilias Apalodimas
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 7:19 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> On the following patches we allow for an initrd path to be stored in
> Boot#### variables. Specifically we encode in the FIlePathList[] of
> the EFI_LOAD_OPTIONS for each Boot#### variable.
>
> The FilePathList[] array looks like this:
> kernel - 0xff - VenMedia(initrd GUID) - initrd1 - 0x01 initrd2 - 0xff
> So let's add the relevant functions to concatenate and retrieve a device
> path based on a Vendor GUID.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> include/efi_loader.h | 4 ++
> lib/efi_loader/efi_device_path.c | 99 ++++++++++++++++++++++++++++++--
> 2 files changed, 98 insertions(+), 5 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index f470bbd636f4..eb11a8c7d4b1 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -738,6 +738,10 @@ struct efi_load_option {
> const u8 *optional_data;
> };
>
> +struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
> + efi_uintn_t *size, efi_guid_t guid);
> +struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
> + const struct efi_device_path *dp2);
> efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data,
> efi_uintn_t *size);
> unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index c9315dd45857..1f55c772dc83 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -282,11 +282,25 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp)
> return ndp;
> }
>
> -struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
> - const struct efi_device_path *dp2)
> +/** _efi_dp_append() - Append or concatenate two device paths.
Please, add an empty line.
> + * Concatenated device path will be separated by a 0xff end
... by a sub-type 0xff end node.
> + * node.
> + *
> + * @dp1: First device path
> + * @dp2: Second device path
Please, describe argument @concat.
> + *
> + * Return: concatenated device path or NULL. Caller must free the returned
> + * value
> + */
> +static struct efi_device_path *_efi_dp_append(const struct efi_device_path *dp1,
This function name is very close to efi_dp_append. Maybe
efi_dp_append_or_concatenate() would be a better function name.
> + const struct efi_device_path *dp2,
> + bool concat)
> {
> struct efi_device_path *ret;
> + size_t end_size = sizeof(END);
>
> + if (concat)
> + end_size = 2 * sizeof(END);
> if (!dp1 && !dp2) {
> /* return an end node */
> ret = efi_dp_dup(&END);
> @@ -298,18 +312,56 @@ struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
> /* both dp1 and dp2 are non-null */
> unsigned sz1 = efi_dp_size(dp1);
> unsigned sz2 = efi_dp_size(dp2);
> - void *p = dp_alloc(sz1 + sz2 + sizeof(END));
> + void *p = dp_alloc(sz1 + sz2 + end_size);
> if (!p)
> return NULL;
> + ret = p;
> memcpy(p, dp1, sz1);
> + p += sz1;
> +
> + if (concat) {
> + memcpy(p, &END, sizeof(END));
> + p += sizeof(END);
> + }
> +
> /* the end node of the second device path has to be retained */
> - memcpy(p + sz1, dp2, sz2 + sizeof(END));
> - ret = p;
> + memcpy(p, dp2, sz2);
> + p += sz2;
> + memcpy(p, &END, sizeof(END));
> }
>
> return ret;
> }
>
> +/** efi_dp_append() - Append a device to an existing device path.
> + *
> + * @dp1: First device path
> + * @dp2: Second device path
> + *
> + * Return: concatenated device path or NULL. Caller must free the returned
> + * value
> + */
> +struct efi_device_path *efi_dp_append(const struct efi_device_path *dp1,
> + const struct efi_device_path *dp2)
> +{
> + return _efi_dp_append(dp1, dp2, false);
> +}
> +
> +/** efi_dp_concat() - Concatenate 2 device paths. The final device path will
> + * contain two device paths separated by and end node (0xff).
> + *
> + * @dp1: First device path
> + * @dp2: Second device path
> + *
> + * Return: concatenated device path or NULL. Caller must free the returned
> + * value
> + */
> +struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
> + const struct efi_device_path *dp2)
> +{
> + return _efi_dp_append(dp1, dp2, true);
> +}
> +
> struct efi_device_path *efi_dp_append_node(const struct efi_device_path *dp,
> const struct efi_device_path *node)
> {
> @@ -1160,3 +1212,40 @@ ssize_t efi_dp_check_length(const struct efi_device_path *dp,
> dp = (const struct efi_device_path *)((const u8 *)dp + len);
> }
> }
> +
> +/**
> + * efi_dp_from_lo() - Get the instance of a VenMedia node in a
> + * multi-instance device path that matches
> + * a specific GUID. This kind of device paths
> + * is found in Boot#### options describing an
> + * initrd location
> + *
> + * @load_option: EFI_LOAD_OPTION containing a valid device path
> + * @size: size of the discovered device path
> + * @guid: guid to search for
> + *
> + * Return: device path including the VenMedia node or NULL.
> + * Caller must free the returned value
> + */
> +struct
> +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
> + efi_uintn_t *size, efi_guid_t guid)
> +{
> + struct efi_device_path *fp = lo->file_path;
> + struct efi_device_path_vendor *vendor;
> + int lo_len = lo->file_path_length;
> +
> + for (; lo_len >= sizeof(struct efi_device_path);
> + lo_len -= fp->length, fp = (void *)fp + fp->length) {
> + if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
> + fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH)
> + continue;
The device path is provided by the user and may be constructed incorrectly.
lo_len might be negative here. Or the remaining device path might not
fit into lo_len.
Function efi_dp_check_length() can be used to check the size but it
currently accepts only positive values of maxlen. Maybe we should change
the type of maxlen to ssize() in that function.
Best regards
Heinrich
> +
> + vendor = (struct efi_device_path_vendor *)fp;
> + if (!guidcmp(&vendor->guid, &guid))
> + return efi_dp_dup(fp);
> + }
> + log_debug("VenMedia(%pUl) not found in %ls\n", &guid, lo->label);
> +
> + return NULL;
> +}
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI
2021-03-13 21:47 ` [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI Ilias Apalodimas
@ 2021-03-14 7:31 ` Heinrich Schuchardt
2021-03-14 7:34 ` Ilias Apalodimas
2021-03-14 8:49 ` Heinrich Schuchardt
2021-03-14 9:24 ` Heinrich Schuchardt
2 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 7:31 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> A following patch introduces a different logic for loading initrd's
> based on the EFI_LOAD_FILE2_PROTOCOL.
> Since similar logic can be applied in the future for other system files
> (i.e DTBs), let's add some helper functions which will retrieve and
> parse file paths stored in EFI variables.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> include/efi_helper.h | 15 ++++
> include/efi_loader.h | 2 +
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_helper.c | 133 ++++++++++++++++++++++++++++++++
> lib/efi_loader/efi_var_common.c | 33 ++++++++
> 5 files changed, 184 insertions(+)
> create mode 100644 include/efi_helper.h
> create mode 100644 lib/efi_loader/efi_helper.c
>
> diff --git a/include/efi_helper.h b/include/efi_helper.h
> new file mode 100644
> index 000000000000..97492c65b6d2
> --- /dev/null
> +++ b/include/efi_helper.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#if !defined _EFI_HELPER_H_
> +#define _EFI_HELPER_H
> +
> +#include <efi.h>
> +#include <efi_api.h>
> +
> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size);
> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
> +
> +#endif
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index eb11a8c7d4b1..aa812a9a3052 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -717,6 +717,8 @@ efi_status_t EFIAPI efi_query_variable_info(
> u64 *remaining_variable_storage_size,
> u64 *maximum_variable_size);
>
> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);
> +
> /*
> * See section 3.1.3 in the v2.7 UEFI spec for more details on
> * the layout of EFI_LOAD_OPTION. In short it is:
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 10b42e8847bf..da2741adecfa 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -23,6 +23,7 @@ endif
> obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
> obj-y += efi_boottime.o
> +obj-y += efi_helper.o
> obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> obj-y += efi_console.o
> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> new file mode 100644
> index 000000000000..df5bdc506dbe
> --- /dev/null
> +++ b/lib/efi_loader/efi_helper.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#define LOG_CATEGORY LOGC_EFI
> +#include <common.h>
> +#include <env.h>
> +#include <malloc.h>
> +#include <dm.h>
> +#include <fs.h>
> +#include <efi_helper.h>
> +#include <efi_load_initrd.h>
> +#include <efi_loader.h>
> +#include <efi_variable.h>
> +
> +/**
> + * efi_create_current_boot_var() - Return Boot#### name were #### is replaced by
> + * the value of BootCurrent
> + *
> + * @var_name: variable name
> + * @var_name_size: size of var_name
> + *
> + * Return: Status code
> + */
> +static efi_status_t efi_create_current_boot_var(u16 var_name[],
> + size_t var_name_size)
> +{
> + efi_uintn_t boot_current_size;
> + efi_status_t ret;
> + u16 boot_current;
> + u16 *pos;
> +
> + boot_current_size = sizeof(boot_current);
> + ret = efi_get_variable_int(L"BootCurrent",
> + &efi_global_variable_guid, NULL,
> + &boot_current_size, &boot_current, NULL);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + pos = efi_create_indexed_name(var_name, var_name_size, "Boot",
> + boot_current);
> + if (!pos) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * efi_get_dp_from_boot() - Retrieve and return a device path from an EFI
> + * Boot### variable
We could add a paragraph here:
A boot option may contain an array of device paths. We use a VenMedia()
with a specific GUID to identify the usage of the array members. This
function is use to extract a specific device path.
> + *
> + * @guid: Vendor guid of the VenMedia DP
vendor GUID of the VenMedia() device path node identifying the device path
> + *
> + * Return: device path or NULL. Caller must free the returned value
> + */
> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid)
> +{
> + struct efi_device_path *file_path = NULL;
> + struct efi_device_path *tmp;
> + struct efi_load_option lo;
> + void *var_value = NULL;
> + efi_uintn_t size;
> + efi_status_t ret;
> + u16 var_name[16];
> +
> + ret = efi_create_current_boot_var(var_name, sizeof(var_name));
> + if (ret != EFI_SUCCESS)
> + return NULL;
> +
> + var_value = efi_get_var(var_name, &efi_global_variable_guid, &size);
> + if (!var_value)
> + return NULL;
> +
> + ret = efi_deserialize_load_option(&lo, var_value, &size);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + tmp = efi_dp_from_lo(&lo, &size, guid);
> + if (!tmp)
> + goto out;
> +
> + /* efi_dp_dup will just return NULL if efi_dp_next is NULL */
> + file_path = efi_dp_dup(efi_dp_next(tmp));
> +
> +out:
> + efi_free_pool(tmp);
> + free(var_value);
> +
> + return file_path;
> +}
> +
> +/**
> + * efi_get_file_size() - Get the size of a file using an EFI file handle
> + *
> + * @handle: EFI file handle
> + * @size: buffer to fill in the discovered size
> + *
> + * Return: device path or NULL. Caller must free the returned value
> + */
> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)
Should this function be in lib/efi_loader/efi_file?
Best regards
Heinrich
> +{
> + struct efi_file_info *info = NULL;
> + efi_uintn_t bs = 0;
> + efi_status_t ret;
> +
> + *size = 0;
> + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> + info));
> + if (ret != EFI_BUFFER_TOO_SMALL) {
> + ret = EFI_DEVICE_ERROR;
> + goto out;
> + }
> +
> + info = malloc(bs);
> + if (!info) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> + info));
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + *size = info->file_size;
> +
> +out:
> + free(info);
> + return ret;
> +}
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index 1c7459266a38..b11ed91a74a4 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -9,6 +9,7 @@
> #include <common.h>
> #include <efi_loader.h>
> #include <efi_variable.h>
> +#include <stdlib.h>
>
> enum efi_secure_mode {
> EFI_MODE_SETUP,
> @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)
> }
> return EFI_AUTH_VAR_NONE;
> }
> +
> +/**
> + * efi_get_var() - read value of an EFI variable
> + *
> + * @name: variable name
> + * @start: vendor GUID
> + * @size: size of allocated buffer
> + *
> + * Return: buffer with variable data or NULL
> + */
> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
> +{
> + efi_status_t ret;
> + void *buf = NULL;
> +
> + *size = 0;
> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + buf = malloc(*size);
> + if (!buf)
> + return NULL;
> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> + }
> +
> + if (ret != EFI_SUCCESS) {
> + free(buf);
> + *size = 0;
> + return NULL;
> + }
> +
> + return buf;
> +}
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot####
2021-03-14 7:19 ` Heinrich Schuchardt
@ 2021-03-14 7:32 ` Ilias Apalodimas
0 siblings, 0 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-14 7:32 UTC (permalink / raw)
To: u-boot
On Sun, Mar 14, 2021 at 08:19:49AM +0100, Heinrich Schuchardt wrote:
> > + * Caller must free the returned value
[...]
> > + */
> > +struct
> > +efi_device_path *efi_dp_from_lo(struct efi_load_option *lo,
> > + efi_uintn_t *size, efi_guid_t guid)
> > +{
> > + struct efi_device_path *fp = lo->file_path;
> > + struct efi_device_path_vendor *vendor;
> > + int lo_len = lo->file_path_length;
> > +
> > + for (; lo_len >= sizeof(struct efi_device_path);
> > + lo_len -= fp->length, fp = (void *)fp + fp->length) {
> > + if (fp->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
> > + fp->sub_type != DEVICE_PATH_SUB_TYPE_VENDOR_PATH)
> > + continue;
>
> The device path is provided by the user and may be constructed incorrectly.
>
> lo_len might be negative here. Or the remaining device path might not
> fit into lo_len.
>
> Function efi_dp_check_length() can be used to check the size but it
> currently accepts only positive values of maxlen. Maybe we should change
> the type of maxlen to ssize() in that function.
>
Yea, I forgot to fix this one.
Regards
/Ilias
> Best regards
>
> Heinrich
>
> > +
> > + vendor = (struct efi_device_path_vendor *)fp;
> > + if (!guidcmp(&vendor->guid, &guid))
> > + return efi_dp_dup(fp);
> > + }
> > + log_debug("VenMedia(%pUl) not found in %ls\n", &guid, lo->label);
> > +
> > + return NULL;
> > +}
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI
2021-03-14 7:31 ` Heinrich Schuchardt
@ 2021-03-14 7:34 ` Ilias Apalodimas
2021-03-14 8:01 ` Heinrich Schuchardt
0 siblings, 1 reply; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-14 7:34 UTC (permalink / raw)
To: u-boot
Hi Heinrich,
On Sun, Mar 14, 2021 at 08:31:20AM +0100, Heinrich Schuchardt wrote:
> On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> > A following patch introduces a different logic for loading initrd's
> > based on the EFI_LOAD_FILE2_PROTOCOL.
> > +/**
> > + * efi_get_file_size() - Get the size of a file using an EFI file handle
> > + *
> > + * @handle: EFI file handle
> > + * @size: buffer to fill in the discovered size
> > + *
> > + * Return: device path or NULL. Caller must free the returned value
> > + */
> > +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)
>
> Should this function be in lib/efi_loader/efi_file?
there's already a statically defined efi_get_file_size() in that file. I can
rename that, just let me know what you prefer.
>
> Best regards
>
> Heinrich
>
> > +{
> > + struct efi_file_info *info = NULL;
> > + efi_uintn_t bs = 0;
> > + efi_status_t ret;
> > +
> > + *size = 0;
> > + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> > + info));
> > + if (ret != EFI_BUFFER_TOO_SMALL) {
> > + ret = EFI_DEVICE_ERROR;
> > + goto out;
> > + }
> > +
> > + info = malloc(bs);
> > + if (!info) {
> > + ret = EFI_OUT_OF_RESOURCES;
> > + goto out;
> > + }
> > + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> > + info));
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + *size = info->file_size;
> > +
> > +out:
> > + free(info);
> > + return ret;
> > +}
> > diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> > index 1c7459266a38..b11ed91a74a4 100644
> > --- a/lib/efi_loader/efi_var_common.c
> > +++ b/lib/efi_loader/efi_var_common.c
> > @@ -9,6 +9,7 @@
> > #include <common.h>
> > #include <efi_loader.h>
> > #include <efi_variable.h>
> > +#include <stdlib.h>
> >
> > enum efi_secure_mode {
> > EFI_MODE_SETUP,
> > @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)
> > }
> > return EFI_AUTH_VAR_NONE;
> > }
> > +
> > +/**
> > + * efi_get_var() - read value of an EFI variable
> > + *
> > + * @name: variable name
> > + * @start: vendor GUID
> > + * @size: size of allocated buffer
> > + *
> > + * Return: buffer with variable data or NULL
> > + */
> > +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
> > +{
> > + efi_status_t ret;
> > + void *buf = NULL;
> > +
> > + *size = 0;
> > + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > + buf = malloc(*size);
> > + if (!buf)
> > + return NULL;
> > + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> > + }
> > +
> > + if (ret != EFI_SUCCESS) {
> > + free(buf);
> > + *size = 0;
> > + return NULL;
> > + }
> > +
> > + return buf;
> > +}
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options
2021-03-13 21:47 ` [PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options Ilias Apalodimas
@ 2021-03-14 7:53 ` Heinrich Schuchardt
2021-03-14 9:02 ` Ilias Apalodimas
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 7:53 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> Document the command line options for efidebug and initrd loading
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> doc/uefi/uefi.rst | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
> index b3494c22e073..76402bc5cfaa 100644
> --- a/doc/uefi/uefi.rst
> +++ b/doc/uefi/uefi.rst
> @@ -180,6 +180,12 @@ Set up boot parameters on your board::
>
> efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed ""
>
> +Since kernel 5.7 there's an alternative way of loading an initrd using
> +LoadFile2 protocol if CONFIG_EFI_LOAD_FILE2_INITRD is enabled.
> +The initrd path can be specified with::
> +
> + efidebug boot add -b ABE0 'kernel' mmc 0:1 Image -i mmc 0:1 initrd
> +
> Now your board can run the signed image via the boot manager (see below).
> You can also try this sequence by running Pytest, test_efi_secboot,
> on the sandbox
> @@ -484,7 +490,20 @@ The load file 2 protocol can be used by the Linux kernel to load the initial
> RAM disk. U-Boot can be configured to provide an implementation with::
>
> EFI_LOAD_FILE2_INITRD=y
> - EFI_INITRD_FILESPEC=interface dev:part path_to_initrd
Thanks for adding the description. Some nitpicky comments below:
> +
> +When the options is enabled the user can add the initrd path with the efidebug
%s/options/option/
> +command.
> +The Boot#### option has a FilePathList[] in it's EFI_LOAD_OPTION.
Load options Boot#### have a FilePathList[] member.
> +The first element of the array (FilePathList[0]) is the loded image.
is the EFI binary to execute.
> +When an initrd is specified the Device Path for the initrd is denoted by a
> +VenMedia node with the EFI_INITRD_MEDIA_GUID. Each entry of the array is
> +terminated by the 'end of entire device path' subtype. If the user wants to
Below you have (0x01). So add (0xff) here.
> +define multiple initrds those must by separated by the 'end of this instance'
%s/initrds/initrds,/
> +identifier of the end node (0x01).
> +
> +So our final format of the FilePathList[] is::
> +
> + Loaded image - end node (0xff) - VenMedia - initrd1 - end node (0x01) initrd2 - end node (0xff)
Loaded image - end node (0xff) - VenMedia - initrd_1 -
[end node (0x01) - initrd_n ...] - end node (0xff)
There can be any number of initrds?
Best regards
Heinrich
>
> Links
> -----
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI
2021-03-14 7:34 ` Ilias Apalodimas
@ 2021-03-14 8:01 ` Heinrich Schuchardt
2021-03-14 8:07 ` Ilias Apalodimas
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 8:01 UTC (permalink / raw)
To: u-boot
On 3/14/21 8:34 AM, Ilias Apalodimas wrote:
> Hi Heinrich,
>
> On Sun, Mar 14, 2021 at 08:31:20AM +0100, Heinrich Schuchardt wrote:
>> On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
>>> A following patch introduces a different logic for loading initrd's
>>> based on the EFI_LOAD_FILE2_PROTOCOL.
>>> +/**
>>> + * efi_get_file_size() - Get the size of a file using an EFI file handle
>>> + *
>>> + * @handle: EFI file handle
>>> + * @size: buffer to fill in the discovered size
>>> + *
>>> + * Return: device path or NULL. Caller must free the returned value
>>> + */
>>> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)
>>
>> Should this function be in lib/efi_loader/efi_file?
>
> there's already a statically defined efi_get_file_size() in that file. I can
> rename that, just let me know what you prefer.
Unfortunately we cannot call the efi_get_file_size() function in
lib/efi_loader/efi_file.c directly because a driver might have installed
a different implementation of a simple file protocol.
We should have different names for the functions, e.g. efi_file_size()
and efi_get_file_size().
But still I think lib/efi_loader/efi_file.c would be a better place for
the new function. We will be able to reuse it in:
* efi_capsule_read_file()
* efi_load_image_from_file()
Best regards
Heinrich
>
>>
>> Best regards
>>
>> Heinrich
>>
>>> +{
>>> + struct efi_file_info *info = NULL;
>>> + efi_uintn_t bs = 0;
>>> + efi_status_t ret;
>>> +
>>> + *size = 0;
>>> + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
>>> + info));
>>> + if (ret != EFI_BUFFER_TOO_SMALL) {
>>> + ret = EFI_DEVICE_ERROR;
>>> + goto out;
>>> + }
>>> +
>>> + info = malloc(bs);
>>> + if (!info) {
>>> + ret = EFI_OUT_OF_RESOURCES;
>>> + goto out;
>>> + }
>>> + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
>>> + info));
>>> + if (ret != EFI_SUCCESS)
>>> + goto out;
>>> +
>>> + *size = info->file_size;
>>> +
>>> +out:
>>> + free(info);
>>> + return ret;
>>> +}
>>> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
>>> index 1c7459266a38..b11ed91a74a4 100644
>>> --- a/lib/efi_loader/efi_var_common.c
>>> +++ b/lib/efi_loader/efi_var_common.c
>>> @@ -9,6 +9,7 @@
>>> #include <common.h>
>>> #include <efi_loader.h>
>>> #include <efi_variable.h>
>>> +#include <stdlib.h>
>>>
>>> enum efi_secure_mode {
>>> EFI_MODE_SETUP,
>>> @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)
>>> }
>>> return EFI_AUTH_VAR_NONE;
>>> }
>>> +
>>> +/**
>>> + * efi_get_var() - read value of an EFI variable
>>> + *
>>> + * @name: variable name
>>> + * @start: vendor GUID
>>> + * @size: size of allocated buffer
>>> + *
>>> + * Return: buffer with variable data or NULL
>>> + */
>>> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
>>> +{
>>> + efi_status_t ret;
>>> + void *buf = NULL;
>>> +
>>> + *size = 0;
>>> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
>>> + if (ret == EFI_BUFFER_TOO_SMALL) {
>>> + buf = malloc(*size);
>>> + if (!buf)
>>> + return NULL;
>>> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
>>> + }
>>> +
>>> + if (ret != EFI_SUCCESS) {
>>> + free(buf);
>>> + *size = 0;
>>> + return NULL;
>>> + }
>>> +
>>> + return buf;
>>> +}
>>>
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI
2021-03-14 8:01 ` Heinrich Schuchardt
@ 2021-03-14 8:07 ` Ilias Apalodimas
0 siblings, 0 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-14 8:07 UTC (permalink / raw)
To: u-boot
On Sun, Mar 14, 2021 at 09:01:48AM +0100, Heinrich Schuchardt wrote:
> On 3/14/21 8:34 AM, Ilias Apalodimas wrote:
> > Hi Heinrich,
> >
> > On Sun, Mar 14, 2021 at 08:31:20AM +0100, Heinrich Schuchardt wrote:
> > > On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> > > > A following patch introduces a different logic for loading initrd's
> > > > based on the EFI_LOAD_FILE2_PROTOCOL.
> > > > +/**
> > > > + * efi_get_file_size() - Get the size of a file using an EFI file handle
> > > > + *
> > > > + * @handle: EFI file handle
> > > > + * @size: buffer to fill in the discovered size
> > > > + *
> > > > + * Return: device path or NULL. Caller must free the returned value
> > > > + */
> > > > +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)
> > >
> > > Should this function be in lib/efi_loader/efi_file?
> >
> > there's already a statically defined efi_get_file_size() in that file. I can
> > rename that, just let me know what you prefer.
>
> Unfortunately we cannot call the efi_get_file_size() function in
> lib/efi_loader/efi_file.c directly because a driver might have installed
> a different implementation of a simple file protocol.
>
> We should have different names for the functions, e.g. efi_file_size()
> and efi_get_file_size().
>
> But still I think lib/efi_loader/efi_file.c would be a better place for
> the new function. We will be able to reuse it in:
>
> * efi_capsule_read_file()
> * efi_load_image_from_file()
Sure, I'll rename this to efi_file_size() and move it in lib/efi_loader/efi_file.c
>
> Best regards
>
> Heinrich
>
> >
> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > > +{
> > > > + struct efi_file_info *info = NULL;
> > > > + efi_uintn_t bs = 0;
> > > > + efi_status_t ret;
> > > > +
> > > > + *size = 0;
> > > > + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> > > > + info));
> > > > + if (ret != EFI_BUFFER_TOO_SMALL) {
> > > > + ret = EFI_DEVICE_ERROR;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + info = malloc(bs);
> > > > + if (!info) {
> > > > + ret = EFI_OUT_OF_RESOURCES;
> > > > + goto out;
> > > > + }
> > > > + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> > > > + info));
> > > > + if (ret != EFI_SUCCESS)
> > > > + goto out;
> > > > +
> > > > + *size = info->file_size;
> > > > +
> > > > +out:
> > > > + free(info);
> > > > + return ret;
> > > > +}
> > > > diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> > > > index 1c7459266a38..b11ed91a74a4 100644
> > > > --- a/lib/efi_loader/efi_var_common.c
> > > > +++ b/lib/efi_loader/efi_var_common.c
> > > > @@ -9,6 +9,7 @@
> > > > #include <common.h>
> > > > #include <efi_loader.h>
> > > > #include <efi_variable.h>
> > > > +#include <stdlib.h>
> > > >
> > > > enum efi_secure_mode {
> > > > EFI_MODE_SETUP,
> > > > @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)
> > > > }
> > > > return EFI_AUTH_VAR_NONE;
> > > > }
> > > > +
> > > > +/**
> > > > + * efi_get_var() - read value of an EFI variable
> > > > + *
> > > > + * @name: variable name
> > > > + * @start: vendor GUID
> > > > + * @size: size of allocated buffer
> > > > + *
> > > > + * Return: buffer with variable data or NULL
> > > > + */
> > > > +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
> > > > +{
> > > > + efi_status_t ret;
> > > > + void *buf = NULL;
> > > > +
> > > > + *size = 0;
> > > > + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> > > > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > > > + buf = malloc(*size);
> > > > + if (!buf)
> > > > + return NULL;
> > > > + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> > > > + }
> > > > +
> > > > + if (ret != EFI_SUCCESS) {
> > > > + free(buf);
> > > > + *size = 0;
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + return buf;
> > > > +}
> > > >
> > >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 0/6 v2] Loadfile2 for initrd loading
2021-03-13 21:47 [PATCH 0/6 v2] Loadfile2 for initrd loading Ilias Apalodimas
` (5 preceding siblings ...)
2021-03-13 21:47 ` [PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options Ilias Apalodimas
@ 2021-03-14 8:41 ` Heinrich Schuchardt
6 siblings, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 8:41 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> Hi!
> This is v2 of [1]
>
> Changes since v1:
> - minor coding style fixes from Heinrich
> - changed the DP format. Instead of VenMedia - 0x01 - initrd, we skip
> the 0x01 between VenMedia and the first file.
> - final device path is stripped in efi_get_dp_from_boot() instead of
> get_initrd_fp()
> - Fixed comments on documentation
>
> [1] https://lists.denx.de/pipermail/u-boot/2021-March/443399.html
>
> Ilias Apalodimas (6):
> efi_selftest: Remove loadfile2 for initrd selftests
> efi_loader: Add device path related functions for initrd via Boot####
> efi_loader: Introduce helper functions for EFI
> efi_loader: Replace config option for initrd loading
> efidebug: add multiple device path instances on Boot####
> doc: Update uefi documentation for initrd loading options
For testing I used:
./u-boot -T
efidebug boot add \
-b d00d initrdtest host 0:1 initrddump.efi \
-i host 0:1 initrd
efidebug boot next d00d
bootefi bootmgr
load
exit
setenv -e BootD00D
poweroff
The same could be done in a Python test.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI
2021-03-13 21:47 ` [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI Ilias Apalodimas
2021-03-14 7:31 ` Heinrich Schuchardt
@ 2021-03-14 8:49 ` Heinrich Schuchardt
2021-03-14 9:24 ` Heinrich Schuchardt
2 siblings, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 8:49 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> A following patch introduces a different logic for loading initrd's
> based on the EFI_LOAD_FILE2_PROTOCOL.
> Since similar logic can be applied in the future for other system files
> (i.e DTBs), let's add some helper functions which will retrieve and
> parse file paths stored in EFI variables.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> include/efi_helper.h | 15 ++++
> include/efi_loader.h | 2 +
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_helper.c | 133 ++++++++++++++++++++++++++++++++
> lib/efi_loader/efi_var_common.c | 33 ++++++++
> 5 files changed, 184 insertions(+)
> create mode 100644 include/efi_helper.h
> create mode 100644 lib/efi_loader/efi_helper.c
>
> diff --git a/include/efi_helper.h b/include/efi_helper.h
> new file mode 100644
> index 000000000000..97492c65b6d2
> --- /dev/null
> +++ b/include/efi_helper.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#if !defined _EFI_HELPER_H_
> +#define _EFI_HELPER_H
%s/_EFI_HELPER_H/_EFI_HELPER_H_/
Best regards
Heirnich
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options
2021-03-14 7:53 ` Heinrich Schuchardt
@ 2021-03-14 9:02 ` Ilias Apalodimas
0 siblings, 0 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-14 9:02 UTC (permalink / raw)
To: u-boot
On Sun, 14 Mar 2021 at 09:53, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:
> On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> > Document the command line options for efidebug and initrd loading
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> > doc/uefi/uefi.rst | 21 ++++++++++++++++++++-
> > 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
> > index b3494c22e073..76402bc5cfaa 100644
> > --- a/doc/uefi/uefi.rst
> > +++ b/doc/uefi/uefi.rst
> > @@ -180,6 +180,12 @@ Set up boot parameters on your board::
> >
> > efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed ""
> >
> > +Since kernel 5.7 there's an alternative way of loading an initrd using
> > +LoadFile2 protocol if CONFIG_EFI_LOAD_FILE2_INITRD is enabled.
> > +The initrd path can be specified with::
> > +
> > + efidebug boot add -b ABE0 'kernel' mmc 0:1 Image -i mmc 0:1 initrd
> > +
> > Now your board can run the signed image via the boot manager (see
> below).
> > You can also try this sequence by running Pytest, test_efi_secboot,
> > on the sandbox
> > @@ -484,7 +490,20 @@ The load file 2 protocol can be used by the Linux
> kernel to load the initial
> > RAM disk. U-Boot can be configured to provide an implementation with::
> >
> > EFI_LOAD_FILE2_INITRD=y
> > - EFI_INITRD_FILESPEC=interface dev:part path_to_initrd
>
> Thanks for adding the description. Some nitpicky comments below:
>
> > +
> > +When the options is enabled the user can add the initrd path with the
> efidebug
>
> %s/options/option/
>
> > +command.
> > +The Boot#### option has a FilePathList[] in it's EFI_LOAD_OPTION.
>
> Load options Boot#### have a FilePathList[] member.
>
> > +The first element of the array (FilePathList[0]) is the loded image.
>
> is the EFI binary to execute.
>
> > +When an initrd is specified the Device Path for the initrd is denoted
> by a
> > +VenMedia node with the EFI_INITRD_MEDIA_GUID. Each entry of the array is
> > +terminated by the 'end of entire device path' subtype. If the user
> wants to
>
> Below you have (0x01). So add (0xff) here.
>
> > +define multiple initrds those must by separated by the 'end of this
> instance'
>
> %s/initrds/initrds,/
>
> > +identifier of the end node (0x01).
> > +
> > +So our final format of the FilePathList[] is::
> > +
> > + Loaded image - end node (0xff) - VenMedia - initrd1 - end node
> (0x01) initrd2 - end node (0xff)
>
> Loaded image - end node (0xff) - VenMedia - initrd_1 -
> [end node (0x01) - initrd_n ...] - end node (0xff)
>
> There can be any number of initrds?
>
I dont see any limitation on the spec.
>
> Best regards
>
> Heinrich
>
> >
> > Links
> > -----
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI
2021-03-13 21:47 ` [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI Ilias Apalodimas
2021-03-14 7:31 ` Heinrich Schuchardt
2021-03-14 8:49 ` Heinrich Schuchardt
@ 2021-03-14 9:24 ` Heinrich Schuchardt
2 siblings, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 9:24 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> A following patch introduces a different logic for loading initrd's
> based on the EFI_LOAD_FILE2_PROTOCOL.
> Since similar logic can be applied in the future for other system files
> (i.e DTBs), let's add some helper functions which will retrieve and
> parse file paths stored in EFI variables.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> include/efi_helper.h | 15 ++++
> include/efi_loader.h | 2 +
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_helper.c | 133 ++++++++++++++++++++++++++++++++
> lib/efi_loader/efi_var_common.c | 33 ++++++++
> 5 files changed, 184 insertions(+)
> create mode 100644 include/efi_helper.h
> create mode 100644 lib/efi_loader/efi_helper.c
>
> diff --git a/include/efi_helper.h b/include/efi_helper.h
> new file mode 100644
> index 000000000000..97492c65b6d2
> --- /dev/null
> +++ b/include/efi_helper.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#if !defined _EFI_HELPER_H_
> +#define _EFI_HELPER_H
> +
> +#include <efi.h>
> +#include <efi_api.h>
> +
> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size);
> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
> +
> +#endif
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index eb11a8c7d4b1..aa812a9a3052 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -717,6 +717,8 @@ efi_status_t EFIAPI efi_query_variable_info(
> u64 *remaining_variable_storage_size,
> u64 *maximum_variable_size);
>
> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);
> +
> /*
> * See section 3.1.3 in the v2.7 UEFI spec for more details on
> * the layout of EFI_LOAD_OPTION. In short it is:
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 10b42e8847bf..da2741adecfa 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -23,6 +23,7 @@ endif
> obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
> obj-y += efi_boottime.o
> +obj-y += efi_helper.o
> obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> obj-y += efi_console.o
> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> new file mode 100644
> index 000000000000..df5bdc506dbe
> --- /dev/null
> +++ b/lib/efi_loader/efi_helper.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#define LOG_CATEGORY LOGC_EFI
> +#include <common.h>
> +#include <env.h>
> +#include <malloc.h>
> +#include <dm.h>
> +#include <fs.h>
> +#include <efi_helper.h>
> +#include <efi_load_initrd.h>
> +#include <efi_loader.h>
> +#include <efi_variable.h>
> +
> +/**
> + * efi_create_current_boot_var() - Return Boot#### name were #### is replaced by
> + * the value of BootCurrent
> + *
> + * @var_name: variable name
> + * @var_name_size: size of var_name
> + *
> + * Return: Status code
> + */
> +static efi_status_t efi_create_current_boot_var(u16 var_name[],
> + size_t var_name_size)
> +{
> + efi_uintn_t boot_current_size;
> + efi_status_t ret;
> + u16 boot_current;
> + u16 *pos;
> +
> + boot_current_size = sizeof(boot_current);
> + ret = efi_get_variable_int(L"BootCurrent",
> + &efi_global_variable_guid, NULL,
> + &boot_current_size, &boot_current, NULL);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + pos = efi_create_indexed_name(var_name, var_name_size, "Boot",
> + boot_current);
> + if (!pos) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * efi_get_dp_from_boot() - Retrieve and return a device path from an EFI
> + * Boot### variable
> + *
> + * @guid: Vendor guid of the VenMedia DP
> + *
> + * Return: device path or NULL. Caller must free the returned value
> + */
> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid)
> +{
> + struct efi_device_path *file_path = NULL;
> + struct efi_device_path *tmp;
struct efi_device_path *tmp = NULL;
+lib/efi_loader/efi_helper.c:79:6: error: variable 'tmp' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
+ if (ret != EFI_SUCCESS)
+ ^~~~~~~~~~~~~~~~~~
+lib/efi_loader/efi_helper.c:90:16: note: uninitialized use occurs here
+ efi_free_pool(tmp);
+ ^~~
> + struct efi_load_option lo;
> + void *var_value = NULL;
> + efi_uintn_t size;
> + efi_status_t ret;
> + u16 var_name[16];
> +
> + ret = efi_create_current_boot_var(var_name, sizeof(var_name));
> + if (ret != EFI_SUCCESS)
> + return NULL;
> +
> + var_value = efi_get_var(var_name, &efi_global_variable_guid, &size);
> + if (!var_value)
> + return NULL;
> +
> + ret = efi_deserialize_load_option(&lo, var_value, &size);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + tmp = efi_dp_from_lo(&lo, &size, guid);
> + if (!tmp)
> + goto out;
> +
> + /* efi_dp_dup will just return NULL if efi_dp_next is NULL */
> + file_path = efi_dp_dup(efi_dp_next(tmp));
> +
> +out:
> + efi_free_pool(tmp);
> + free(var_value);
> +
> + return file_path;
> +}
> +
> +/**
> + * efi_get_file_size() - Get the size of a file using an EFI file handle
> + *
> + * @handle: EFI file handle
> + * @size: buffer to fill in the discovered size
> + *
> + * Return: device path or NULL. Caller must free the returned value
> + */
> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)
> +{
> + struct efi_file_info *info = NULL;
> + efi_uintn_t bs = 0;
> + efi_status_t ret;
> +
> + *size = 0;
> + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> + info));
> + if (ret != EFI_BUFFER_TOO_SMALL) {
> + ret = EFI_DEVICE_ERROR;
> + goto out;
> + }
> +
> + info = malloc(bs);
> + if (!info) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> + ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
> + info));
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + *size = info->file_size;
> +
> +out:
> + free(info);
> + return ret;
> +}
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index 1c7459266a38..b11ed91a74a4 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -9,6 +9,7 @@
> #include <common.h>
> #include <efi_loader.h>
> #include <efi_variable.h>
> +#include <stdlib.h>
>
> enum efi_secure_mode {
> EFI_MODE_SETUP,
> @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)
> }
> return EFI_AUTH_VAR_NONE;
> }
> +
> +/**
> + * efi_get_var() - read value of an EFI variable
> + *
> + * @name: variable name
> + * @start: vendor GUID
> + * @size: size of allocated buffer
> + *
> + * Return: buffer with variable data or NULL
> + */
> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
> +{
> + efi_status_t ret;
> + void *buf = NULL;
> +
> + *size = 0;
> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + buf = malloc(*size);
> + if (!buf)
> + return NULL;
> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> + }
> +
> + if (ret != EFI_SUCCESS) {
> + free(buf);
> + *size = 0;
> + return NULL;
> + }
> +
> + return buf;
> +}
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot####
2021-03-13 21:47 ` [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot#### Ilias Apalodimas
@ 2021-03-14 9:27 ` Heinrich Schuchardt
2021-03-14 9:42 ` Heinrich Schuchardt
2021-03-14 10:14 ` Heinrich Schuchardt
1 sibling, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 9:27 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> The UEFI spec allow a packed array of UEFI device paths in the
%s/allow/allows/
> FilePathList[] of an EFI_LOAD_OPTION. The first file path must
> describe the loaded image but the rest are OS specific.
>
> Previous patches parse the device path and try to use the second
> member of the array as an initrd. So let's modify efidebug slightly
> and install the second file described in the command line as the
> initrd device path.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> cmd/efidebug.c | 194 ++++++++++++++----
> doc/board/emulation/qemu_capsule_update.rst | 4 +-
> doc/uefi/uefi.rst | 2 +-
> .../test_efi_capsule/test_capsule_firmware.py | 6 +-
> test/py/tests/test_efi_secboot/test_signed.py | 16 +-
> .../test_efi_secboot/test_signed_intca.py | 8 +-
> .../tests/test_efi_secboot/test_unsigned.py | 8 +-
> 7 files changed, 180 insertions(+), 58 deletions(-)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index bbbcb0a54643..223cffa389fb 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -9,6 +9,8 @@
> #include <common.h>
> #include <command.h>
> #include <efi_dt_fixup.h>
> +#include <efi_helper.h>
> +#include <efi_load_initrd.h>
> #include <efi_loader.h>
> #include <efi_rng.h>
> #include <exports.h>
> @@ -19,6 +21,7 @@
> #include <part.h>
> #include <search.h>
> #include <linux/ctype.h>
> +#include <linux/err.h>
>
> #define BS systab.boottime
> #define RT systab.runtime
> @@ -794,6 +797,66 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
> return CMD_RET_SUCCESS;
> }
>
> +/**
> + * add_initrd_instance() - Append a device path to load_options pointing to an
> + * inirtd
> + *
> + * @dev: Device
> + * @part: Partition of thge disk
> + * @file: Filename
> + * @fp: Device Path containing the existing load_options
> + * @fp_size: New size of the device path after the addition
> + * Return: Pointer to the device path or ERR_PTR
NULL is good enough to signal a problem and saves a few bytes of code.
> + *
> + */
> +static
> +struct efi_device_path *add_initrd_instance(const char *dev, const char *part,
> + const char *file,
> + const struct efi_device_path *fp,
> + efi_uintn_t *fp_size)
> +{
> + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL;
> + struct efi_device_path *final_fp = NULL, *initrd_dp = NULL;
> + efi_status_t ret;
> + const struct efi_initrd_dp id_dp = {
> + .vendor = {
> + {
> + DEVICE_PATH_TYPE_MEDIA_DEVICE,
> + DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> + sizeof(id_dp.vendor),
> + },
> + EFI_INITRD_MEDIA_GUID,
> + },
> + .end = {
> + DEVICE_PATH_TYPE_END,
> + DEVICE_PATH_SUB_TYPE_END,
> + sizeof(id_dp.end),
> + }
> + };
> +
> + ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp);
> + if (ret != EFI_SUCCESS) {
> + printf("Cannot create device path for \"%s %s\"\n", part, file);
> + goto out;
> + }
> +
> + initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp,
> + tmp_fp);
> + if (!initrd_dp) {
> + printf("Cannot append media vendor device path path\n");
"Cannot add initrd\n" is less confusing for an end user.
But I guess that message should be moved to caller because
efi_dp_concat() might fail too (due to out of memory).
> + goto out;
> + }
> + final_fp = efi_dp_concat(fp, initrd_dp);
> + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) +
> + (2 * sizeof(struct efi_device_path));
> +
> +out:
> + efi_free_pool(initrd_dp);
> + efi_free_pool(tmp_dp);
> + efi_free_pool(tmp_fp);
> + return final_fp ? final_fp : ERR_PTR(-EINVAL);
Just return final_fp. NULL signals an error.
> +}
> +
> /**
> * do_efi_boot_add() - set UEFI load option
> *
> @@ -806,7 +869,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
> *
> * Implement efidebug "boot add" sub-command. Create or change UEFI load option.
> *
> - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options>
> + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file>
> + * -i <file> <interface2> <devnum2>[:<part>] <initrd>
> + * -s '<options>'
> */
> static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> @@ -819,55 +884,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
> size_t label_len, label_len16;
> u16 *label;
> struct efi_device_path *device_path = NULL, *file_path = NULL;
> + struct efi_device_path *final_fp = NULL;
> struct efi_load_option lo;
> void *data = NULL;
> efi_uintn_t size;
> + efi_uintn_t fp_size;
> efi_status_t ret;
> int r = CMD_RET_SUCCESS;
> -
> - if (argc < 6 || argc > 7)
> - return CMD_RET_USAGE;
> -
> - id = (int)simple_strtoul(argv[1], &endp, 16);
> - if (*endp != '\0' || id > 0xffff)
> - return CMD_RET_USAGE;
> -
> - sprintf(var_name, "Boot%04X", id);
> - p = var_name16;
> - utf8_utf16_strncpy(&p, var_name, 9);
> + int i;
>
> guid = efi_global_variable_guid;
>
> /* attributes */
> lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */
> + lo.optional_data = NULL;
> + lo.label = NULL;
>
> - /* label */
> - label_len = strlen(argv[2]);
> - label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> - label = malloc((label_len16 + 1) * sizeof(u16));
> - if (!label)
> - return CMD_RET_FAILURE;
> - lo.label = label; /* label will be changed below */
> - utf8_utf16_strncpy(&label, argv[2], label_len);
> + /* search for -b first since the rest of the arguments depends on that */
> + for (i = 0; i < argc; i++) {
> + if (!strcmp(argv[i], "-b")) {
> + if (argc < i + 5 || lo.label) {
> + r = CMD_RET_USAGE;
> + goto out;
> + }
> + id = (int)simple_strtoul(argv[i + 1], &endp, 16);
> + if (*endp != '\0' || id > 0xffff)
> + return CMD_RET_USAGE;
> +
> + sprintf(var_name, "Boot%04X", id);
> + p = var_name16;
> + utf8_utf16_strncpy(&p, var_name, 9);
> +
> + /* label */
> + label_len = strlen(argv[i + 2]);
> + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len);
> + label = malloc((label_len16 + 1) * sizeof(u16));
> + if (!label)
> + return CMD_RET_FAILURE;
> + lo.label = label; /* label will be changed below */
> + utf8_utf16_strncpy(&label, argv[i + 2], label_len);
> +
> + /* file path */
> + ret = efi_dp_from_name(argv[i + 3], argv[i + 4],
> + argv[i + 5], &device_path,
> + &file_path);
> + if (ret != EFI_SUCCESS) {
> + printf("Cannot create device path for \"%s %s\"\n",
> + argv[3], argv[4]);
> + r = CMD_RET_FAILURE;
> + goto out;
> + break;
> + }
> + fp_size = efi_dp_size(file_path) +
> + sizeof(struct efi_device_path);
> + }
> + }
>
> - /* file path */
> - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> - &file_path);
> - if (ret != EFI_SUCCESS) {
> - printf("Cannot create device path for \"%s %s\"\n",
> - argv[3], argv[4]);
> + if (!file_path) {
> + printf("You need to specify an image before an initrd.\n");
"Missing binary\n" is enough.
You must specify a binary even if there is not initrd.
> r = CMD_RET_FAILURE;
r = CMD_RET_USAGE;
> goto out;
> }
> - lo.file_path = file_path;
> - lo.file_path_length = efi_dp_size(file_path)
> - + sizeof(struct efi_device_path); /* for END */
>
> - /* optional data */
> - if (argc == 6)
> - lo.optional_data = NULL;
> - else
> - lo.optional_data = (const u8 *)argv[6];
> + /* add now add initrd and extra data */
> + for (i = 0; i < argc; i++) {
> + if (!strcmp(argv[i], "-i")) {
> + if (argc < i + 3 || final_fp) {
> + r = CMD_RET_USAGE;
> + goto out;
> + }
> +
> + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2],
> + argv[i + 3], file_path,
> + &fp_size);
> + if (IS_ERR(final_fp)) {
if (!final_fp) if we change add_intird_instance().
Error message could be here instead of in add_initrd_instance().
Best regards
Heinrich
> + r = CMD_RET_FAILURE;
> + goto out;
> + }
> +
> + /* add_initrd_instance allocates a new device path */
> + efi_free_pool(file_path);
> + file_path = final_fp;
> + } else if (!strcmp(argv[i], "-s")) {
> + if (argc < i + 1 || lo.optional_data) {
> + r = CMD_RET_USAGE;
> + goto out;
> + }
> + lo.optional_data = (const u8 *)argv[i + 1];
> + }
> + }
> +
> + lo.file_path = file_path;
> + lo.file_path_length = fp_size;
>
> size = efi_serialize_load_option(&lo, (u8 **)&data);
> if (!size) {
> @@ -951,11 +1059,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag,
> */
> static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size)
> {
> + struct efi_device_path *initrd_path = NULL;
> struct efi_load_option lo;
> char *label, *p;
> size_t label_len16, label_len;
> u16 *dp_str;
> efi_status_t ret;
> + efi_uintn_t initrd_dp_size;
> + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
>
> ret = efi_deserialize_load_option(&lo, data, size);
> if (ret != EFI_SUCCESS) {
> @@ -986,6 +1097,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size)
> printf(" file_path: %ls\n", dp_str);
> efi_free_pool(dp_str);
>
> + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid);
> + if (initrd_path) {
> + dp_str = efi_dp_str(initrd_path);
> + printf(" initrd_path: %ls\n", dp_str);
> + efi_free_pool(dp_str);
> + efi_free_pool(initrd_path);
> + }
> +
> printf(" data:\n");
> print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> lo.optional_data, *size, true);
> @@ -1555,7 +1674,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag,
> static char efidebug_help_text[] =
> " - UEFI Shell-like interface to configure UEFI environment\n"
> "\n"
> - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n"
> + "efidebug boot add "
> + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> "
> + "-i <interface> <devnum>[:<part>] <initrd file path> "
> + "-s '<optional data>'\n"
> " - set UEFI BootXXXX variable\n"
> " <load options> will be passed to UEFI application\n"
> "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
> @@ -1599,7 +1721,7 @@ static char efidebug_help_text[] =
> #endif
>
> U_BOOT_CMD(
> - efidebug, 10, 0, do_efidebug,
> + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug,
> "Configure UEFI environment",
> efidebug_help_text
> );
> diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst
> index 9fec75f8f1c9..33ce4bcd32ea 100644
> --- a/doc/board/emulation/qemu_capsule_update.rst
> +++ b/doc/board/emulation/qemu_capsule_update.rst
> @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule
> file. The BootNext, BootXXXX and OsIndications variables can be set
> using the following commands::
>
> - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name>
> + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
> => efidebug boot next 0
> => setenv -e -nv -bs -rt -v OsIndications =0x04
> => saveenv
> @@ -198,7 +198,7 @@ command line::
> 3. Set the following environment and UEFI boot variables
>
> => setenv -e -nv -bs -rt -v OsIndications =0x04
> - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name>
> + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
> => efidebug boot next 0
> => saveenv
>
> diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
> index 5a67737c1579..b3494c22e073 100644
> --- a/doc/uefi/uefi.rst
> +++ b/doc/uefi/uefi.rst
> @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board::
>
> Set up boot parameters on your board::
>
> - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed ""
> + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed ""
>
> Now your board can run the signed image via the boot manager (see below).
> You can also try this sequence by running Pytest, test_efi_secboot,
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> index f006fa95d650..e8b0a1575453 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object):
> with u_boot_console.log.section('Test Case 1-a, before reboot'):
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
> 'efidebug boot order 1',
> 'env set -e OsIndications',
> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
> @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object):
> with u_boot_console.log.section('Test Case 2-a, before reboot'):
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
> 'efidebug boot order 1',
> 'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
> @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object):
> with u_boot_console.log.section('Test Case 3-a, before reboot'):
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
> 'efidebug boot order 1',
> 'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
> diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
> index 863685e215b7..75f5ea772300 100644
> --- a/test/py/tests/test_efi_secboot/test_signed.py
> +++ b/test/py/tests/test_efi_secboot/test_signed.py
> @@ -28,7 +28,7 @@ class TestEfiSignedImage(object):
> # Test Case 1a, run signed image if no PK
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -36,7 +36,7 @@ class TestEfiSignedImage(object):
> with u_boot_console.log.section('Test Case 1b'):
> # Test Case 1b, run unsigned image if no PK
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""',
> 'efidebug boot next 2',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -58,13 +58,13 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert('\'HELLO1\' failed' in ''.join(output))
> assert('efi_start_image() returned: 26' in ''.join(output))
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""',
> 'efidebug boot next 2',
> 'efidebug test bootmgr'])
> assert '\'HELLO2\' failed' in ''.join(output)
> @@ -104,7 +104,7 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
> @@ -142,7 +142,7 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
> @@ -169,7 +169,7 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -227,7 +227,7 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
> index 70d6be00e8a8..0849572a5143 100644
> --- a/test/py/tests/test_efi_secboot/test_signed_intca.py
> +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
> @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
> + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert '\'HELLO_a\' failed' in ''.join(output)
> @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object):
> with u_boot_console.log.section('Test Case 1b'):
> # Test Case 1b, signed and authenticated by root CA
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
> + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
> 'efidebug boot next 2',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert '\'HELLO_abc\' failed' in ''.join(output)
> @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py
> index 56f56e19eb84..8e026f7566ad 100644
> --- a/test/py/tests/test_efi_secboot/test_unsigned.py
> +++ b/test/py/tests/test_efi_secboot/test_unsigned.py
> @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
> @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
> @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot####
2021-03-14 9:27 ` Heinrich Schuchardt
@ 2021-03-14 9:42 ` Heinrich Schuchardt
2021-03-14 9:44 ` Ilias Apalodimas
0 siblings, 1 reply; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 9:42 UTC (permalink / raw)
To: u-boot
On 3/14/21 10:27 AM, Heinrich Schuchardt wrote:
> On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
>> The UEFI spec allow a packed array of UEFI device paths in the
>
> %s/allow/allows/
>
>> FilePathList[] of an EFI_LOAD_OPTION. The first file path must
>> describe the loaded image but the rest are OS specific.
>>
>> Previous patches parse the device path and try to use the second
>> member of the array as an initrd. So let's modify efidebug slightly
>> and install the second file described in the command line as the
>> initrd device path.
>>
>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
<snip />
>> index 70d6be00e8a8..0849572a5143 100644
>> --- a/test/py/tests/test_efi_secboot/test_signed_intca.py
>> +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
>> @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object):
>> ????????????? assert 'Failed to set EFI variable' not in ''.join(output)
>>
>> ????????????? output = u_boot_console.run_command_list([
>> -??????????????? 'efidebug boot add 1 HELLO_a host 0:1
>> /helloworld.efi.signed_a ""',
>> +??????????????? 'efidebug boot add -b 1 HELLO_a host 0:1
>> /helloworld.efi.signed_a ""',
>> 'efidebug boot next 1',
>> 'efidebug test bootmgr'])
Why don't we use as syntax
'efidebug boot add HELLO_a -b 1 host 0:1 helloworld.efi.signed_a'?
The boot option number and label are not a property of the binary.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot####
2021-03-14 9:42 ` Heinrich Schuchardt
@ 2021-03-14 9:44 ` Ilias Apalodimas
0 siblings, 0 replies; 25+ messages in thread
From: Ilias Apalodimas @ 2021-03-14 9:44 UTC (permalink / raw)
To: u-boot
On Sun, Mar 14, 2021 at 10:42:06AM +0100, Heinrich Schuchardt wrote:
> On 3/14/21 10:27 AM, Heinrich Schuchardt wrote:
> > On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> > > The UEFI spec allow a packed array of UEFI device paths in the
> >
> > %s/allow/allows/
> >
> > > FilePathList[] of an EFI_LOAD_OPTION. The first file path must
> > > describe the loaded image but the rest are OS specific.
> > >
> > > Previous patches parse the device path and try to use the second
> > > member of the array as an initrd. So let's modify efidebug slightly
> > > and install the second file described in the command line as the
> > > initrd device path.
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> <snip />
>
> > > index 70d6be00e8a8..0849572a5143 100644
> > > --- a/test/py/tests/test_efi_secboot/test_signed_intca.py
> > > +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
> > > @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object):
> > > ????????????? assert 'Failed to set EFI variable' not in ''.join(output)
> > >
> > > ????????????? output = u_boot_console.run_command_list([
> > > -??????????????? 'efidebug boot add 1 HELLO_a host 0:1
> > > /helloworld.efi.signed_a ""',
> > > +??????????????? 'efidebug boot add -b 1 HELLO_a host 0:1
> >> /helloworld.efi.signed_a ""',
> >> 'efidebug boot next 1',
> >> 'efidebug test bootmgr'])
>
> Why don't we use as syntax
> 'efidebug boot add HELLO_a -b 1 host 0:1 helloworld.efi.signed_a'?
>
> The boot option number and label are not a property of the binary.
I just moved all aof the args we had when adding an executable to -b, since
those are need to define a boot option
>
> Best regards
>
> Heinrich
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading
2021-03-13 21:47 ` [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading Ilias Apalodimas
@ 2021-03-14 9:54 ` Heinrich Schuchardt
2021-03-14 10:08 ` Heinrich Schuchardt
2021-03-14 10:19 ` Heinrich Schuchardt
2 siblings, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 9:54 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd
> unconditionally. Although we correctly return various EFI exit codes
> depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the
> kernel loader, only falls back to the cmdline interpreted initrd if the
> protocol is not installed.
>
> This creates a problem for EFI installers, since they won't be able to
> load their own initrd and continue the installation. It also makes the
> feature hard to use, since we can either have a single initrd or we have
> to recompile u-boot if the filename changes.
>
> So let's introduce a different logic that will decouple the initrd
> path from the config option we currently have.
> When defining a UEFI BootXXXX we can use the filepathlist and store
> a file path pointing to our initrd. Specifically the EFI spec describes:
>
> "The first element of the array is a device path that describes the device
> and location of the Image for this load option. Other device paths may
> optionally exist in the FilePathList, but their usage is OSV specific".
> That means we can install a device path to our initrd(s) after the loaded
> image looking like this:
> VenMedia - initrd1 - end node (0x01) initrd2 - end node (0xff)
>
> When the EFI application is launched through the bootmgr, we'll try to
> interpret the extra device path. If that points to a file that exists on
> our disk, we'll now install the load_file2 and the efi-stub will be able
> to use it.
>
> This opens up another path using U-Boot and defines a new boot flow.
> A user will be able to control the kernel/initrd pairs without explicit
> cmdline args or GRUB.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> cmd/bootefi.c | 3 +
> include/efi_loader.h | 1 +
> lib/efi_loader/Kconfig | 15 +--
> lib/efi_loader/efi_bootmgr.c | 19 +++-
> lib/efi_loader/efi_load_initrd.c | 189 ++++++++++++++++++-------------
> 5 files changed, 136 insertions(+), 91 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 271b385edea6..cba81ffe75e4 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
>
> free(load_options);
>
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
> + efi_initrd_deregister();
> +
> return ret;
> }
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index aa812a9a3052..9e57eb37ff28 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -431,6 +431,7 @@ efi_status_t efi_net_register(void);
> /* Called by bootefi to make the watchdog available */
> efi_status_t efi_watchdog_register(void);
> efi_status_t efi_initrd_register(void);
> +void efi_initrd_deregister(void);
> /* Called by bootefi to make SMBIOS tables available */
> /**
> * efi_acpi_register() - write out ACPI tables
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e729f727df11..029f0e515f57 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD
> bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
> default n
> help
> - Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
> - use to load the initial ramdisk. Once this is enabled using
> - initrd=<ramdisk> will stop working.
> -
> -config EFI_INITRD_FILESPEC
> - string "initramfs path"
> - default "host 0:1 initrd"
> - depends on EFI_LOAD_FILE2_INITRD
> - help
> - Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> + Linux v5.7 and later can make use of this option. If the boot option
> + selected by the UEFI boot manager specifies an existing file to be used
> + as initial RAM disk, a Linux specific Load File2 protocol will be
> + installed and Linux 5.7+ will ignore any initrd=<ramdisk> command line
> + argument.
>
> config EFI_SECURE_BOOT
> bool "Enable EFI secure boot support"
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 25f5cebfdb67..46c8011344b9 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -118,11 +118,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
> ret = efi_set_variable_int(L"BootCurrent",
> &efi_global_variable_guid,
> attributes, sizeof(n), &n, false);
> - if (ret != EFI_SUCCESS) {
> - if (EFI_CALL(efi_unload_image(*handle))
> - != EFI_SUCCESS)
> - log_err("Unloading image failed\n");
> - goto error;
> + if (ret != EFI_SUCCESS)
> + goto unload;
> + /* try to register load file2 for initrd's */
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> + ret = efi_initrd_register();
> + if (ret != EFI_SUCCESS)
> + goto unload;
> }
>
> log_info("Booting: %ls\n", lo.label);
> @@ -146,6 +148,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
> error:
> free(load_option);
>
> + return ret;
> +
> +unload:
> + if (EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
> + log_err("Unloading image failed\n");
> + free(load_option);
> +
> return ret;
> }
>
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index b9ee8839054f..e76de30808e3 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -5,7 +5,9 @@
>
> #include <common.h>
> #include <efi_loader.h>
> +#include <efi_helper.h>
> #include <efi_load_initrd.h>
> +#include <efi_variable.h>
> #include <fs.h>
> #include <malloc.h>
> #include <mapmem.h>
> @@ -39,41 +41,40 @@ static const struct efi_initrd_dp dp = {
> }
> };
>
> +static efi_handle_t efi_initrd_handle;
> +
> /**
> - * get_file_size() - retrieve the size of initramfs, set efi status on error
> + * get_initrd_fp() - Get initrd device path from a FilePathList device path
> *
> - * @dev: device to read from, e.g. "mmc"
> - * @part: device partition, e.g. "0:1"
> - * @file: name of file
> - * @status: EFI exit code in case of failure
> + * @initrd_fp: the final initrd filepath
> *
> - * Return: size of file
> + * Return: status code. Caller must free initrd_fp
Reducing the indentation of the comments would make this more readable.
Otherwise
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> */
> -static loff_t get_file_size(const char *dev, const char *part, const char *file,
> - efi_status_t *status)
> +static efi_status_t get_initrd_fp(struct efi_device_path **initrd_fp)
> {
> - loff_t sz = 0;
> - int ret;
> -
> - ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> - if (ret) {
> - *status = EFI_NO_MEDIA;
> - goto out;
> - }
> + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
> + struct efi_device_path *dp = NULL;
>
> - ret = fs_size(file, &sz);
> - if (ret) {
> - sz = 0;
> - *status = EFI_NOT_FOUND;
> - goto out;
> - }
> + /*
> + * if bootmgr is setup with and initrd, the device path will be
> + * in the FilePathList[] of our load options in Boot####.
> + * The first device path of the multi instance device path will
> + * start with a VenMedia and the initrds will follow.
> + *
> + * If the device path is not found return EFI_INVALID_PARAMETER.
> + * We can then use this specific return value and not install the
> + * protocol, while allowing the boot to continue
> + */
> + dp = efi_get_dp_from_boot(lf2_initrd_guid);
> + if (!dp)
> + return EFI_INVALID_PARAMETER;
>
> -out:
> - return sz;
> + *initrd_fp = dp;
> + return EFI_SUCCESS;
> }
>
> /**
> - * efi_load_file2initrd() - load initial RAM disk
> + * efi_load_file2_initrd() - load initial RAM disk
> *
> * This function implements the LoadFile service of the EFI_LOAD_FILE2_PROTOCOL
> * in order to load an initial RAM disk requested by the Linux kernel stub.
> @@ -93,98 +94,120 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
> struct efi_device_path *file_path, bool boot_policy,
> efi_uintn_t *buffer_size, void *buffer)
> {
> - char *filespec;
> - efi_status_t status = EFI_NOT_FOUND;
> - loff_t file_sz = 0, read_sz = 0;
> - char *dev, *part, *file;
> - char *pos;
> - int ret;
> + struct efi_device_path *initrd_fp = NULL;
> + efi_status_t ret = EFI_NOT_FOUND;
> + struct efi_file_handle *f;
> + efi_uintn_t bs;
>
> EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
> buffer_size, buffer);
>
> - filespec = strdup(CONFIG_EFI_INITRD_FILESPEC);
> - if (!filespec)
> - goto out;
> - pos = filespec;
> -
> if (!this || this != &efi_lf2_protocol ||
> !buffer_size) {
> - status = EFI_INVALID_PARAMETER;
> + ret = EFI_INVALID_PARAMETER;
> goto out;
> }
>
> if (file_path->type != dp.end.type ||
> file_path->sub_type != dp.end.sub_type) {
> - status = EFI_INVALID_PARAMETER;
> + ret = EFI_INVALID_PARAMETER;
> goto out;
> }
>
> if (boot_policy) {
> - status = EFI_UNSUPPORTED;
> + ret = EFI_UNSUPPORTED;
> goto out;
> }
>
> - /*
> - * expect a string with three space separated parts:
> - *
> - * * a block device type, e.g. "mmc"
> - * * a device and partition identifier, e.g. "0:1"
> - * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
> - */
> - dev = strsep(&pos, " ");
> - if (!dev)
> + ret = get_initrd_fp(&initrd_fp);
> + if (ret != EFI_SUCCESS)
> goto out;
> - part = strsep(&pos, " ");
> - if (!part)
> - goto out;
> - file = strsep(&pos, " ");
> - if (!file)
> +
> + /* Open file */
> + f = efi_file_from_path(initrd_fp);
> + if (!f) {
> + EFI_PRINT("Can't find initrd specified in Boot####\n");
I think this deserves an log_err() or log_warning(). The user needs a
feedback what is going wrong.
> + ret = EFI_NOT_FOUND;
> goto out;
> + }
>
> - file_sz = get_file_size(dev, part, file, &status);
> - if (!file_sz)
> + /* Get file size */
> + ret = efi_get_file_size(f, &bs);
> + if (ret != EFI_SUCCESS)
> goto out;
>
> - if (!buffer || *buffer_size < file_sz) {
> - status = EFI_BUFFER_TOO_SMALL;
> - *buffer_size = file_sz;
> + if (!buffer || *buffer_size < bs) {
> + ret = EFI_BUFFER_TOO_SMALL;
> + *buffer_size = bs;
> } else {
> - ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> - if (ret) {
> - status = EFI_NO_MEDIA;
> - goto out;
> - }
> -
> - ret = fs_read(file, map_to_sysmem(buffer), 0, *buffer_size,
> - &read_sz);
> - if (ret || read_sz != file_sz)
> - goto out;
> - *buffer_size = read_sz;
> -
> - status = EFI_SUCCESS;
> + ret = EFI_CALL(f->read(f, &bs, (void *)(uintptr_t)buffer));
> + *buffer_size = bs;
> + }
> +
> +out:
> + efi_free_pool(initrd_fp);
> + EFI_CALL(f->close(f));
> + return EFI_EXIT(ret);
> +}
> +
> +/**
> + * check_initrd() - Determine if the file defined as an initrd in Boot####
> + * load_options device path is present
> + *
> + * Return: status code
> + */
> +static efi_status_t check_initrd(void)
> +{
> + struct efi_device_path *initrd_fp = NULL;
> + struct efi_file_handle *f;
> + efi_status_t ret;
> +
> + ret = get_initrd_fp(&initrd_fp);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + /*
> + * If the file is not found, but the file path is set, return an error
> + * and trigger the bootmgr fallback
> + */
> + f = efi_file_from_path(initrd_fp);
> + if (!f) {
> + EFI_PRINT("Can't find initrd specified in Boot####\n");
I think this deserves an log_err() or log_warning(). The user needs a
feedback what is going wrong.
> + ret = EFI_NOT_FOUND;
> + goto out;
> }
>
> + EFI_CALL(f->close(f));
> +
> out:
> - free(filespec);
> - return EFI_EXIT(status);
> + efi_free_pool(initrd_fp);
> + return ret;
> }
>
> /**
> * efi_initrd_register() - create handle for loading initial RAM disk
> *
> * This function creates a new handle and installs a Linux specific vendor
> - * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path
> + * device path and an EFI_LOAD_FILE2_PROTOCOL. Linux uses the device path
> * to identify the handle and then calls the LoadFile service of the
> - * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk.
> + * EFI_LOAD_FILE2_PROTOCOL to read the initial RAM disk.
> *
> * Return: status code
> */
> efi_status_t efi_initrd_register(void)
> {
> - efi_handle_t efi_initrd_handle = NULL;
> efi_status_t ret;
>
> + /*
> + * Allow the user to continue if Boot#### file path is not set for
> + * an initrd
> + */
> + ret = check_initrd();
> + if (ret == EFI_INVALID_PARAMETER)
> + return EFI_SUCCESS;
> + if (ret != EFI_SUCCESS)
> + return ret;
> +
> ret = EFI_CALL(efi_install_multiple_protocol_interfaces
> (&efi_initrd_handle,
> /* initramfs */
> @@ -196,3 +219,17 @@ efi_status_t efi_initrd_register(void)
>
> return ret;
> }
> +
> +/**
> + * efi_initrd_register() - delete the handle for loading initial RAM disk
> + *
> + * This will delete the handle containing the Linux specific vendor device
> + * path and EFI_LOAD_FILE2_PROTOCOL for loading an initrd
> + *
> + * Return: status code
> + */
> +void efi_initrd_deregister(void)
> +{
> + efi_delete_handle(efi_initrd_handle);
> + efi_initrd_handle = NULL;
> +}
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading
2021-03-13 21:47 ` [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading Ilias Apalodimas
2021-03-14 9:54 ` Heinrich Schuchardt
@ 2021-03-14 10:08 ` Heinrich Schuchardt
2021-03-14 10:19 ` Heinrich Schuchardt
2 siblings, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 10:08 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd
> unconditionally. Although we correctly return various EFI exit codes
> depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the
> kernel loader, only falls back to the cmdline interpreted initrd if the
> protocol is not installed.
>
> This creates a problem for EFI installers, since they won't be able to
> load their own initrd and continue the installation. It also makes the
> feature hard to use, since we can either have a single initrd or we have
> to recompile u-boot if the filename changes.
>
> So let's introduce a different logic that will decouple the initrd
> path from the config option we currently have.
> When defining a UEFI BootXXXX we can use the filepathlist and store
> a file path pointing to our initrd. Specifically the EFI spec describes:
>
> "The first element of the array is a device path that describes the device
> and location of the Image for this load option. Other device paths may
> optionally exist in the FilePathList, but their usage is OSV specific".
> That means we can install a device path to our initrd(s) after the loaded
> image looking like this:
> VenMedia - initrd1 - end node (0x01) initrd2 - end node (0xff)
>
> When the EFI application is launched through the bootmgr, we'll try to
> interpret the extra device path. If that points to a file that exists on
> our disk, we'll now install the load_file2 and the efi-stub will be able
> to use it.
>
> This opens up another path using U-Boot and defines a new boot flow.
> A user will be able to control the kernel/initrd pairs without explicit
> cmdline args or GRUB.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> cmd/bootefi.c | 3 +
> include/efi_loader.h | 1 +
> lib/efi_loader/Kconfig | 15 +--
> lib/efi_loader/efi_bootmgr.c | 19 +++-
> lib/efi_loader/efi_load_initrd.c | 189 ++++++++++++++++++-------------
> 5 files changed, 136 insertions(+), 91 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 271b385edea6..cba81ffe75e4 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
>
> free(load_options);
>
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
> + efi_initrd_deregister();
> +
> return ret;
> }
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index aa812a9a3052..9e57eb37ff28 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -431,6 +431,7 @@ efi_status_t efi_net_register(void);
> /* Called by bootefi to make the watchdog available */
> efi_status_t efi_watchdog_register(void);
> efi_status_t efi_initrd_register(void);
> +void efi_initrd_deregister(void);
> /* Called by bootefi to make SMBIOS tables available */
> /**
> * efi_acpi_register() - write out ACPI tables
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e729f727df11..029f0e515f57 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD
> bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
> default n
> help
> - Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
> - use to load the initial ramdisk. Once this is enabled using
> - initrd=<ramdisk> will stop working.
> -
> -config EFI_INITRD_FILESPEC
> - string "initramfs path"
> - default "host 0:1 initrd"
> - depends on EFI_LOAD_FILE2_INITRD
> - help
> - Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> + Linux v5.7 and later can make use of this option. If the boot option
> + selected by the UEFI boot manager specifies an existing file to be used
> + as initial RAM disk, a Linux specific Load File2 protocol will be
> + installed and Linux 5.7+ will ignore any initrd=<ramdisk> command line
> + argument.
>
> config EFI_SECURE_BOOT
> bool "Enable EFI secure boot support"
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 25f5cebfdb67..46c8011344b9 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -118,11 +118,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
> ret = efi_set_variable_int(L"BootCurrent",
> &efi_global_variable_guid,
> attributes, sizeof(n), &n, false);
> - if (ret != EFI_SUCCESS) {
> - if (EFI_CALL(efi_unload_image(*handle))
> - != EFI_SUCCESS)
> - log_err("Unloading image failed\n");
> - goto error;
> + if (ret != EFI_SUCCESS)
> + goto unload;
> + /* try to register load file2 for initrd's */
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> + ret = efi_initrd_register();
> + if (ret != EFI_SUCCESS)
> + goto unload;
> }
>
> log_info("Booting: %ls\n", lo.label);
> @@ -146,6 +148,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
> error:
> free(load_option);
>
> + return ret;
> +
> +unload:
> + if (EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
> + log_err("Unloading image failed\n");
> + free(load_option);
> +
> return ret;
> }
>
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index b9ee8839054f..e76de30808e3 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -5,7 +5,9 @@
>
> #include <common.h>
> #include <efi_loader.h>
> +#include <efi_helper.h>
> #include <efi_load_initrd.h>
> +#include <efi_variable.h>
> #include <fs.h>
> #include <malloc.h>
> #include <mapmem.h>
> @@ -39,41 +41,40 @@ static const struct efi_initrd_dp dp = {
> }
> };
>
> +static efi_handle_t efi_initrd_handle;
> +
> /**
> - * get_file_size() - retrieve the size of initramfs, set efi status on error
> + * get_initrd_fp() - Get initrd device path from a FilePathList device path
> *
> - * @dev: device to read from, e.g. "mmc"
> - * @part: device partition, e.g. "0:1"
> - * @file: name of file
> - * @status: EFI exit code in case of failure
> + * @initrd_fp: the final initrd filepath
> *
> - * Return: size of file
> + * Return: status code. Caller must free initrd_fp
> */
> -static loff_t get_file_size(const char *dev, const char *part, const char *file,
> - efi_status_t *status)
> +static efi_status_t get_initrd_fp(struct efi_device_path **initrd_fp)
> {
> - loff_t sz = 0;
> - int ret;
> -
> - ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> - if (ret) {
> - *status = EFI_NO_MEDIA;
> - goto out;
> - }
> + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
> + struct efi_device_path *dp = NULL;
>
> - ret = fs_size(file, &sz);
> - if (ret) {
> - sz = 0;
> - *status = EFI_NOT_FOUND;
> - goto out;
> - }
> + /*
> + * if bootmgr is setup with and initrd, the device path will be
> + * in the FilePathList[] of our load options in Boot####.
> + * The first device path of the multi instance device path will
> + * start with a VenMedia and the initrds will follow.
> + *
> + * If the device path is not found return EFI_INVALID_PARAMETER.
> + * We can then use this specific return value and not install the
> + * protocol, while allowing the boot to continue
> + */
> + dp = efi_get_dp_from_boot(lf2_initrd_guid);
> + if (!dp)
> + return EFI_INVALID_PARAMETER;
>
> -out:
> - return sz;
> + *initrd_fp = dp;
> + return EFI_SUCCESS;
> }
>
> /**
> - * efi_load_file2initrd() - load initial RAM disk
> + * efi_load_file2_initrd() - load initial RAM disk
> *
> * This function implements the LoadFile service of the EFI_LOAD_FILE2_PROTOCOL
> * in order to load an initial RAM disk requested by the Linux kernel stub.
> @@ -93,98 +94,120 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
> struct efi_device_path *file_path, bool boot_policy,
> efi_uintn_t *buffer_size, void *buffer)
> {
> - char *filespec;
> - efi_status_t status = EFI_NOT_FOUND;
> - loff_t file_sz = 0, read_sz = 0;
> - char *dev, *part, *file;
> - char *pos;
> - int ret;
> + struct efi_device_path *initrd_fp = NULL;
> + efi_status_t ret = EFI_NOT_FOUND;
> + struct efi_file_handle *f;
Please, initialize f to NULL.
+lib/efi_loader/efi_load_initrd.c:123:6: error: variable 'f' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
+ if (ret != EFI_SUCCESS)
+ ^~~~~~~~~~~~~~~~~~
+lib/efi_loader/efi_load_initrd.c:149:11: note: uninitialized use occurs
here
+ EFI_CALL(f->close(f));
+ ^
> + efi_uintn_t bs;
>
> EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
> buffer_size, buffer);
>
> - filespec = strdup(CONFIG_EFI_INITRD_FILESPEC);
> - if (!filespec)
> - goto out;
> - pos = filespec;
> -
> if (!this || this != &efi_lf2_protocol ||
> !buffer_size) {
> - status = EFI_INVALID_PARAMETER;
> + ret = EFI_INVALID_PARAMETER;
> goto out;
> }
>
> if (file_path->type != dp.end.type ||
> file_path->sub_type != dp.end.sub_type) {
> - status = EFI_INVALID_PARAMETER;
> + ret = EFI_INVALID_PARAMETER;
> goto out;
> }
>
> if (boot_policy) {
> - status = EFI_UNSUPPORTED;
> + ret = EFI_UNSUPPORTED;
> goto out;
> }
>
> - /*
> - * expect a string with three space separated parts:
> - *
> - * * a block device type, e.g. "mmc"
> - * * a device and partition identifier, e.g. "0:1"
> - * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
> - */
> - dev = strsep(&pos, " ");
> - if (!dev)
> + ret = get_initrd_fp(&initrd_fp);
> + if (ret != EFI_SUCCESS)
> goto out;
> - part = strsep(&pos, " ");
> - if (!part)
> - goto out;
> - file = strsep(&pos, " ");
> - if (!file)
> +
> + /* Open file */
> + f = efi_file_from_path(initrd_fp);
> + if (!f) {
> + EFI_PRINT("Can't find initrd specified in Boot####\n");
> + ret = EFI_NOT_FOUND;
> goto out;
> + }
>
> - file_sz = get_file_size(dev, part, file, &status);
> - if (!file_sz)
> + /* Get file size */
> + ret = efi_get_file_size(f, &bs);
> + if (ret != EFI_SUCCESS)
> goto out;
>
> - if (!buffer || *buffer_size < file_sz) {
> - status = EFI_BUFFER_TOO_SMALL;
> - *buffer_size = file_sz;
> + if (!buffer || *buffer_size < bs) {
> + ret = EFI_BUFFER_TOO_SMALL;
> + *buffer_size = bs;
> } else {
> - ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> - if (ret) {
> - status = EFI_NO_MEDIA;
> - goto out;
> - }
> -
> - ret = fs_read(file, map_to_sysmem(buffer), 0, *buffer_size,
> - &read_sz);
> - if (ret || read_sz != file_sz)
> - goto out;
> - *buffer_size = read_sz;
> -
> - status = EFI_SUCCESS;
> + ret = EFI_CALL(f->read(f, &bs, (void *)(uintptr_t)buffer));
> + *buffer_size = bs;
> + }
> +
> +out:
> + efi_free_pool(initrd_fp);
> + EFI_CALL(f->close(f));
f may be NULL. You must not dereference NULL.
Best regards
Heinrich
> + return EFI_EXIT(ret);
> +}
> +
> +/**
> + * check_initrd() - Determine if the file defined as an initrd in Boot####
> + * load_options device path is present
> + *
> + * Return: status code
> + */
> +static efi_status_t check_initrd(void)
> +{
> + struct efi_device_path *initrd_fp = NULL;
> + struct efi_file_handle *f;
> + efi_status_t ret;
> +
> + ret = get_initrd_fp(&initrd_fp);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + /*
> + * If the file is not found, but the file path is set, return an error
> + * and trigger the bootmgr fallback
> + */
> + f = efi_file_from_path(initrd_fp);
> + if (!f) {
> + EFI_PRINT("Can't find initrd specified in Boot####\n");
> + ret = EFI_NOT_FOUND;
> + goto out;
> }
>
> + EFI_CALL(f->close(f));
> +
> out:
> - free(filespec);
> - return EFI_EXIT(status);
> + efi_free_pool(initrd_fp);
> + return ret;
> }
>
> /**
> * efi_initrd_register() - create handle for loading initial RAM disk
> *
> * This function creates a new handle and installs a Linux specific vendor
> - * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path
> + * device path and an EFI_LOAD_FILE2_PROTOCOL. Linux uses the device path
> * to identify the handle and then calls the LoadFile service of the
> - * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk.
> + * EFI_LOAD_FILE2_PROTOCOL to read the initial RAM disk.
> *
> * Return: status code
> */
> efi_status_t efi_initrd_register(void)
> {
> - efi_handle_t efi_initrd_handle = NULL;
> efi_status_t ret;
>
> + /*
> + * Allow the user to continue if Boot#### file path is not set for
> + * an initrd
> + */
> + ret = check_initrd();
> + if (ret == EFI_INVALID_PARAMETER)
> + return EFI_SUCCESS;
> + if (ret != EFI_SUCCESS)
> + return ret;
> +
> ret = EFI_CALL(efi_install_multiple_protocol_interfaces
> (&efi_initrd_handle,
> /* initramfs */
> @@ -196,3 +219,17 @@ efi_status_t efi_initrd_register(void)
>
> return ret;
> }
> +
> +/**
> + * efi_initrd_register() - delete the handle for loading initial RAM disk
> + *
> + * This will delete the handle containing the Linux specific vendor device
> + * path and EFI_LOAD_FILE2_PROTOCOL for loading an initrd
> + *
> + * Return: status code
> + */
> +void efi_initrd_deregister(void)
> +{
> + efi_delete_handle(efi_initrd_handle);
> + efi_initrd_handle = NULL;
> +}
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot####
2021-03-13 21:47 ` [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot#### Ilias Apalodimas
2021-03-14 9:27 ` Heinrich Schuchardt
@ 2021-03-14 10:14 ` Heinrich Schuchardt
1 sibling, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 10:14 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> The UEFI spec allow a packed array of UEFI device paths in the
> FilePathList[] of an EFI_LOAD_OPTION. The first file path must
> describe the loaded image but the rest are OS specific.
>
> Previous patches parse the device path and try to use the second
> member of the array as an initrd. So let's modify efidebug slightly
> and install the second file described in the command line as the
> initrd device path.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> cmd/efidebug.c | 194 ++++++++++++++----
> doc/board/emulation/qemu_capsule_update.rst | 4 +-
> doc/uefi/uefi.rst | 2 +-
> .../test_efi_capsule/test_capsule_firmware.py | 6 +-
> test/py/tests/test_efi_secboot/test_signed.py | 16 +-
> .../test_efi_secboot/test_signed_intca.py | 8 +-
> .../tests/test_efi_secboot/test_unsigned.py | 8 +-
> 7 files changed, 180 insertions(+), 58 deletions(-)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index bbbcb0a54643..223cffa389fb 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -9,6 +9,8 @@
> #include <common.h>
> #include <command.h>
> #include <efi_dt_fixup.h>
> +#include <efi_helper.h>
> +#include <efi_load_initrd.h>
> #include <efi_loader.h>
> #include <efi_rng.h>
> #include <exports.h>
> @@ -19,6 +21,7 @@
> #include <part.h>
> #include <search.h>
> #include <linux/ctype.h>
> +#include <linux/err.h>
>
> #define BS systab.boottime
> #define RT systab.runtime
> @@ -794,6 +797,66 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
> return CMD_RET_SUCCESS;
> }
>
> +/**
> + * add_initrd_instance() - Append a device path to load_options pointing to an
> + * inirtd
> + *
> + * @dev: Device
> + * @part: Partition of thge disk
> + * @file: Filename
> + * @fp: Device Path containing the existing load_options
> + * @fp_size: New size of the device path after the addition
> + * Return: Pointer to the device path or ERR_PTR
> + *
> + */
> +static
> +struct efi_device_path *add_initrd_instance(const char *dev, const char *part,
> + const char *file,
> + const struct efi_device_path *fp,
> + efi_uintn_t *fp_size)
> +{
> + struct efi_device_path *tmp_dp = NULL, *tmp_fp = NULL;
> + struct efi_device_path *final_fp = NULL, *initrd_dp = NULL;
> + efi_status_t ret;
> + const struct efi_initrd_dp id_dp = {
> + .vendor = {
> + {
> + DEVICE_PATH_TYPE_MEDIA_DEVICE,
> + DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> + sizeof(id_dp.vendor),
> + },
> + EFI_INITRD_MEDIA_GUID,
> + },
> + .end = {
> + DEVICE_PATH_TYPE_END,
> + DEVICE_PATH_SUB_TYPE_END,
> + sizeof(id_dp.end),
> + }
> + };
> +
> + ret = efi_dp_from_name(dev, part, file, &tmp_dp, &tmp_fp);
> + if (ret != EFI_SUCCESS) {
> + printf("Cannot create device path for \"%s %s\"\n", part, file);
> + goto out;
> + }
> +
> + initrd_dp = efi_dp_append((const struct efi_device_path *)&id_dp,
> + tmp_fp);
> + if (!initrd_dp) {
> + printf("Cannot append media vendor device path path\n");
> + goto out;
> + }
> + final_fp = efi_dp_concat(fp, initrd_dp);
> + *fp_size = efi_dp_size(fp) + efi_dp_size(initrd_dp) +
> + (2 * sizeof(struct efi_device_path));
> +
> +out:
> + efi_free_pool(initrd_dp);
> + efi_free_pool(tmp_dp);
> + efi_free_pool(tmp_fp);
> + return final_fp ? final_fp : ERR_PTR(-EINVAL);
> +}
> +
> /**
> * do_efi_boot_add() - set UEFI load option
> *
> @@ -806,7 +869,9 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
> *
> * Implement efidebug "boot add" sub-command. Create or change UEFI load option.
> *
> - * efidebug boot add <id> <label> <interface> <devnum>[:<part>] <file> <options>
> + * efidebug boot add -b <id> <label> <interface> <devnum>[:<part>] <file>
> + * -i <file> <interface2> <devnum2>[:<part>] <initrd>
> + * -s '<options>'
> */
> static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> @@ -819,55 +884,98 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
> size_t label_len, label_len16;
> u16 *label;
> struct efi_device_path *device_path = NULL, *file_path = NULL;
> + struct efi_device_path *final_fp = NULL;
> struct efi_load_option lo;
> void *data = NULL;
> efi_uintn_t size;
> + efi_uintn_t fp_size;
> efi_status_t ret;
> int r = CMD_RET_SUCCESS;
> -
> - if (argc < 6 || argc > 7)
> - return CMD_RET_USAGE;
> -
> - id = (int)simple_strtoul(argv[1], &endp, 16);
> - if (*endp != '\0' || id > 0xffff)
> - return CMD_RET_USAGE;
> -
> - sprintf(var_name, "Boot%04X", id);
> - p = var_name16;
> - utf8_utf16_strncpy(&p, var_name, 9);
> + int i;
>
> guid = efi_global_variable_guid;
>
> /* attributes */
> lo.attributes = LOAD_OPTION_ACTIVE; /* always ACTIVE */
> + lo.optional_data = NULL;
> + lo.label = NULL;
>
> - /* label */
> - label_len = strlen(argv[2]);
> - label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> - label = malloc((label_len16 + 1) * sizeof(u16));
> - if (!label)
> - return CMD_RET_FAILURE;
> - lo.label = label; /* label will be changed below */
> - utf8_utf16_strncpy(&label, argv[2], label_len);
> + /* search for -b first since the rest of the arguments depends on that */
> + for (i = 0; i < argc; i++) {
> + if (!strcmp(argv[i], "-b")) {
> + if (argc < i + 5 || lo.label) {
> + r = CMD_RET_USAGE;
> + goto out;
> + }
> + id = (int)simple_strtoul(argv[i + 1], &endp, 16);
> + if (*endp != '\0' || id > 0xffff)
> + return CMD_RET_USAGE;
> +
> + sprintf(var_name, "Boot%04X", id);
> + p = var_name16;
> + utf8_utf16_strncpy(&p, var_name, 9);
> +
> + /* label */
> + label_len = strlen(argv[i + 2]);
> + label_len16 = utf8_utf16_strnlen(argv[i + 2], label_len);
> + label = malloc((label_len16 + 1) * sizeof(u16));
> + if (!label)
> + return CMD_RET_FAILURE;
> + lo.label = label; /* label will be changed below */
> + utf8_utf16_strncpy(&label, argv[i + 2], label_len);
> +
> + /* file path */
> + ret = efi_dp_from_name(argv[i + 3], argv[i + 4],
> + argv[i + 5], &device_path,
> + &file_path);
> + if (ret != EFI_SUCCESS) {
> + printf("Cannot create device path for \"%s %s\"\n",
> + argv[3], argv[4]);
> + r = CMD_RET_FAILURE;
> + goto out;
> + break;
This break is incorrectly indented and not needed after goto.
Cppcheck pointed me at it.
Best regards
Heinrich
> + }
> + fp_size = efi_dp_size(file_path) +
> + sizeof(struct efi_device_path);
> + }
> + }
>
> - /* file path */
> - ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> - &file_path);
> - if (ret != EFI_SUCCESS) {
> - printf("Cannot create device path for \"%s %s\"\n",
> - argv[3], argv[4]);
> + if (!file_path) {
> + printf("You need to specify an image before an initrd.\n");
> r = CMD_RET_FAILURE;
> goto out;
> }
> - lo.file_path = file_path;
> - lo.file_path_length = efi_dp_size(file_path)
> - + sizeof(struct efi_device_path); /* for END */
>
> - /* optional data */
> - if (argc == 6)
> - lo.optional_data = NULL;
> - else
> - lo.optional_data = (const u8 *)argv[6];
> + /* add now add initrd and extra data */
> + for (i = 0; i < argc; i++) {
> + if (!strcmp(argv[i], "-i")) {
> + if (argc < i + 3 || final_fp) {
> + r = CMD_RET_USAGE;
> + goto out;
> + }
> +
> + final_fp = add_initrd_instance(argv[i + 1], argv[i + 2],
> + argv[i + 3], file_path,
> + &fp_size);
> + if (IS_ERR(final_fp)) {
> + r = CMD_RET_FAILURE;
> + goto out;
> + }
> +
> + /* add_initrd_instance allocates a new device path */
> + efi_free_pool(file_path);
> + file_path = final_fp;
> + } else if (!strcmp(argv[i], "-s")) {
> + if (argc < i + 1 || lo.optional_data) {
> + r = CMD_RET_USAGE;
> + goto out;
> + }
> + lo.optional_data = (const u8 *)argv[i + 1];
> + }
> + }
> +
> + lo.file_path = file_path;
> + lo.file_path_length = fp_size;
>
> size = efi_serialize_load_option(&lo, (u8 **)&data);
> if (!size) {
> @@ -951,11 +1059,14 @@ static int do_efi_boot_rm(struct cmd_tbl *cmdtp, int flag,
> */
> static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size)
> {
> + struct efi_device_path *initrd_path = NULL;
> struct efi_load_option lo;
> char *label, *p;
> size_t label_len16, label_len;
> u16 *dp_str;
> efi_status_t ret;
> + efi_uintn_t initrd_dp_size;
> + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
>
> ret = efi_deserialize_load_option(&lo, data, size);
> if (ret != EFI_SUCCESS) {
> @@ -986,6 +1097,14 @@ static void show_efi_boot_opt_data(u16 *varname16, void *data, size_t *size)
> printf(" file_path: %ls\n", dp_str);
> efi_free_pool(dp_str);
>
> + initrd_path = efi_dp_from_lo(&lo, &initrd_dp_size, lf2_initrd_guid);
> + if (initrd_path) {
> + dp_str = efi_dp_str(initrd_path);
> + printf(" initrd_path: %ls\n", dp_str);
> + efi_free_pool(dp_str);
> + efi_free_pool(initrd_path);
> + }
> +
> printf(" data:\n");
> print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> lo.optional_data, *size, true);
> @@ -1555,7 +1674,10 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag,
> static char efidebug_help_text[] =
> " - UEFI Shell-like interface to configure UEFI environment\n"
> "\n"
> - "efidebug boot add <bootid> <label> <interface> <devnum>[:<part>] <file path> [<load options>]\n"
> + "efidebug boot add "
> + "-b <bootid> <label> <interface> <devnum>[:<part>] <file path> "
> + "-i <interface> <devnum>[:<part>] <initrd file path> "
> + "-s '<optional data>'\n"
> " - set UEFI BootXXXX variable\n"
> " <load options> will be passed to UEFI application\n"
> "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
> @@ -1599,7 +1721,7 @@ static char efidebug_help_text[] =
> #endif
>
> U_BOOT_CMD(
> - efidebug, 10, 0, do_efidebug,
> + efidebug, CONFIG_SYS_MAXARGS, 0, do_efidebug,
> "Configure UEFI environment",
> efidebug_help_text
> );
> diff --git a/doc/board/emulation/qemu_capsule_update.rst b/doc/board/emulation/qemu_capsule_update.rst
> index 9fec75f8f1c9..33ce4bcd32ea 100644
> --- a/doc/board/emulation/qemu_capsule_update.rst
> +++ b/doc/board/emulation/qemu_capsule_update.rst
> @@ -60,7 +60,7 @@ to be pointing to the EFI System Partition which contains the capsule
> file. The BootNext, BootXXXX and OsIndications variables can be set
> using the following commands::
>
> - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name>
> + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
> => efidebug boot next 0
> => setenv -e -nv -bs -rt -v OsIndications =0x04
> => saveenv
> @@ -198,7 +198,7 @@ command line::
> 3. Set the following environment and UEFI boot variables
>
> => setenv -e -nv -bs -rt -v OsIndications =0x04
> - => efidebug boot add 0 Boot0000 virtio 0:1 <capsule_file_name>
> + => efidebug boot add -b 0 Boot0000 virtio 0:1 <capsule_file_name>
> => efidebug boot next 0
> => saveenv
>
> diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
> index 5a67737c1579..b3494c22e073 100644
> --- a/doc/uefi/uefi.rst
> +++ b/doc/uefi/uefi.rst
> @@ -178,7 +178,7 @@ Now in U-Boot install the keys on your board::
>
> Set up boot parameters on your board::
>
> - efidebug boot add 1 HELLO mmc 0:1 /helloworld.efi.signed ""
> + efidebug boot add -b 1 HELLO mmc 0:1 /helloworld.efi.signed ""
>
> Now your board can run the signed image via the boot manager (see below).
> You can also try this sequence by running Pytest, test_efi_secboot,
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> index f006fa95d650..e8b0a1575453 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> @@ -39,7 +39,7 @@ class TestEfiCapsuleFirmwareFit(object):
> with u_boot_console.log.section('Test Case 1-a, before reboot'):
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
> 'efidebug boot order 1',
> 'env set -e OsIndications',
> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
> @@ -114,7 +114,7 @@ class TestEfiCapsuleFirmwareFit(object):
> with u_boot_console.log.section('Test Case 2-a, before reboot'):
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
> 'efidebug boot order 1',
> 'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
> @@ -188,7 +188,7 @@ class TestEfiCapsuleFirmwareFit(object):
> with u_boot_console.log.section('Test Case 3-a, before reboot'):
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> - 'efidebug boot add 1 TEST host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 TEST host 0:1 /helloworld.efi ""',
> 'efidebug boot order 1',
> 'env set -e -nv -bs -rt OsIndications =0x0000000000000004',
> 'env set dfu_alt_info "sf 0:0=u-boot-bin raw 0x100000 0x50000;u-boot-env raw 0x150000 0x200000"',
> diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
> index 863685e215b7..75f5ea772300 100644
> --- a/test/py/tests/test_efi_secboot/test_signed.py
> +++ b/test/py/tests/test_efi_secboot/test_signed.py
> @@ -28,7 +28,7 @@ class TestEfiSignedImage(object):
> # Test Case 1a, run signed image if no PK
> output = u_boot_console.run_command_list([
> 'host bind 0 %s' % disk_img,
> - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -36,7 +36,7 @@ class TestEfiSignedImage(object):
> with u_boot_console.log.section('Test Case 1b'):
> # Test Case 1b, run unsigned image if no PK
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""',
> 'efidebug boot next 2',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -58,13 +58,13 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert('\'HELLO1\' failed' in ''.join(output))
> assert('efi_start_image() returned: 26' in ''.join(output))
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 2 HELLO2 host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld.efi ""',
> 'efidebug boot next 2',
> 'efidebug test bootmgr'])
> assert '\'HELLO2\' failed' in ''.join(output)
> @@ -104,7 +104,7 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
> @@ -142,7 +142,7 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
> @@ -169,7 +169,7 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -227,7 +227,7 @@ class TestEfiSignedImage(object):
> 'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
> assert 'Failed to set EFI variable' not in ''.join(output)
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> diff --git a/test/py/tests/test_efi_secboot/test_signed_intca.py b/test/py/tests/test_efi_secboot/test_signed_intca.py
> index 70d6be00e8a8..0849572a5143 100644
> --- a/test/py/tests/test_efi_secboot/test_signed_intca.py
> +++ b/test/py/tests/test_efi_secboot/test_signed_intca.py
> @@ -39,7 +39,7 @@ class TestEfiSignedImageIntca(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
> + 'efidebug boot add -b 1 HELLO_a host 0:1 /helloworld.efi.signed_a ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert '\'HELLO_a\' failed' in ''.join(output)
> @@ -48,7 +48,7 @@ class TestEfiSignedImageIntca(object):
> with u_boot_console.log.section('Test Case 1b'):
> # Test Case 1b, signed and authenticated by root CA
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
> + 'efidebug boot add -b 2 HELLO_ab host 0:1 /helloworld.efi.signed_ab ""',
> 'efidebug boot next 2',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -70,7 +70,7 @@ class TestEfiSignedImageIntca(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert '\'HELLO_abc\' failed' in ''.join(output)
> @@ -116,7 +116,7 @@ class TestEfiSignedImageIntca(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> + 'efidebug boot add -b 1 HELLO_abc host 0:1 /helloworld.efi.signed_abc ""',
> 'efidebug boot next 1',
> 'efidebug test bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> diff --git a/test/py/tests/test_efi_secboot/test_unsigned.py b/test/py/tests/test_efi_secboot/test_unsigned.py
> index 56f56e19eb84..8e026f7566ad 100644
> --- a/test/py/tests/test_efi_secboot/test_unsigned.py
> +++ b/test/py/tests/test_efi_secboot/test_unsigned.py
> @@ -35,7 +35,7 @@ class TestEfiUnsignedImage(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
> @@ -64,7 +64,7 @@ class TestEfiUnsignedImage(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert 'Hello, world!' in ''.join(output)
> @@ -88,7 +88,7 @@ class TestEfiUnsignedImage(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
> @@ -106,7 +106,7 @@ class TestEfiUnsignedImage(object):
> assert 'Failed to set EFI variable' not in ''.join(output)
>
> output = u_boot_console.run_command_list([
> - 'efidebug boot add 1 HELLO host 0:1 /helloworld.efi ""',
> + 'efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi ""',
> 'efidebug boot next 1',
> 'bootefi bootmgr'])
> assert '\'HELLO\' failed' in ''.join(output)
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading
2021-03-13 21:47 ` [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading Ilias Apalodimas
2021-03-14 9:54 ` Heinrich Schuchardt
2021-03-14 10:08 ` Heinrich Schuchardt
@ 2021-03-14 10:19 ` Heinrich Schuchardt
2 siblings, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2021-03-14 10:19 UTC (permalink / raw)
To: u-boot
On 3/13/21 10:47 PM, Ilias Apalodimas wrote:
> Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd
> unconditionally. Although we correctly return various EFI exit codes
> depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the
> kernel loader, only falls back to the cmdline interpreted initrd if the
> protocol is not installed.
>
> This creates a problem for EFI installers, since they won't be able to
> load their own initrd and continue the installation. It also makes the
> feature hard to use, since we can either have a single initrd or we have
> to recompile u-boot if the filename changes.
>
> So let's introduce a different logic that will decouple the initrd
> path from the config option we currently have.
> When defining a UEFI BootXXXX we can use the filepathlist and store
> a file path pointing to our initrd. Specifically the EFI spec describes:
>
> "The first element of the array is a device path that describes the device
> and location of the Image for this load option. Other device paths may
> optionally exist in the FilePathList, but their usage is OSV specific".
> That means we can install a device path to our initrd(s) after the loaded
> image looking like this:
> VenMedia - initrd1 - end node (0x01) initrd2 - end node (0xff)
>
> When the EFI application is launched through the bootmgr, we'll try to
> interpret the extra device path. If that points to a file that exists on
> our disk, we'll now install the load_file2 and the efi-stub will be able
> to use it.
>
> This opens up another path using U-Boot and defines a new boot flow.
> A user will be able to control the kernel/initrd pairs without explicit
> cmdline args or GRUB.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> cmd/bootefi.c | 3 +
> include/efi_loader.h | 1 +
> lib/efi_loader/Kconfig | 15 +--
> lib/efi_loader/efi_bootmgr.c | 19 +++-
> lib/efi_loader/efi_load_initrd.c | 189 ++++++++++++++++++-------------
> 5 files changed, 136 insertions(+), 91 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 271b385edea6..cba81ffe75e4 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
>
> free(load_options);
>
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
> + efi_initrd_deregister();
> +
> return ret;
> }
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index aa812a9a3052..9e57eb37ff28 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -431,6 +431,7 @@ efi_status_t efi_net_register(void);
> /* Called by bootefi to make the watchdog available */
> efi_status_t efi_watchdog_register(void);
> efi_status_t efi_initrd_register(void);
> +void efi_initrd_deregister(void);
> /* Called by bootefi to make SMBIOS tables available */
> /**
> * efi_acpi_register() - write out ACPI tables
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e729f727df11..029f0e515f57 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD
> bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
> default n
> help
> - Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
> - use to load the initial ramdisk. Once this is enabled using
> - initrd=<ramdisk> will stop working.
> -
> -config EFI_INITRD_FILESPEC
> - string "initramfs path"
> - default "host 0:1 initrd"
> - depends on EFI_LOAD_FILE2_INITRD
> - help
> - Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> + Linux v5.7 and later can make use of this option. If the boot option
> + selected by the UEFI boot manager specifies an existing file to be used
> + as initial RAM disk, a Linux specific Load File2 protocol will be
> + installed and Linux 5.7+ will ignore any initrd=<ramdisk> command line
> + argument.
>
> config EFI_SECURE_BOOT
> bool "Enable EFI secure boot support"
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 25f5cebfdb67..46c8011344b9 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -118,11 +118,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
> ret = efi_set_variable_int(L"BootCurrent",
> &efi_global_variable_guid,
> attributes, sizeof(n), &n, false);
> - if (ret != EFI_SUCCESS) {
> - if (EFI_CALL(efi_unload_image(*handle))
> - != EFI_SUCCESS)
> - log_err("Unloading image failed\n");
> - goto error;
> + if (ret != EFI_SUCCESS)
> + goto unload;
> + /* try to register load file2 for initrd's */
> + if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> + ret = efi_initrd_register();
> + if (ret != EFI_SUCCESS)
> + goto unload;
> }
>
> log_info("Booting: %ls\n", lo.label);
> @@ -146,6 +148,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
> error:
> free(load_option);
>
> + return ret;
> +
> +unload:
> + if (EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
> + log_err("Unloading image failed\n");
> + free(load_option);
> +
> return ret;
> }
>
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index b9ee8839054f..e76de30808e3 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -5,7 +5,9 @@
>
> #include <common.h>
> #include <efi_loader.h>
> +#include <efi_helper.h>
> #include <efi_load_initrd.h>
> +#include <efi_variable.h>
> #include <fs.h>
> #include <malloc.h>
> #include <mapmem.h>
> @@ -39,41 +41,40 @@ static const struct efi_initrd_dp dp = {
> }
> };
>
> +static efi_handle_t efi_initrd_handle;
> +
> /**
> - * get_file_size() - retrieve the size of initramfs, set efi status on error
> + * get_initrd_fp() - Get initrd device path from a FilePathList device path
> *
> - * @dev: device to read from, e.g. "mmc"
> - * @part: device partition, e.g. "0:1"
> - * @file: name of file
> - * @status: EFI exit code in case of failure
> + * @initrd_fp: the final initrd filepath
> *
> - * Return: size of file
> + * Return: status code. Caller must free initrd_fp
> */
> -static loff_t get_file_size(const char *dev, const char *part, const char *file,
> - efi_status_t *status)
> +static efi_status_t get_initrd_fp(struct efi_device_path **initrd_fp)
> {
> - loff_t sz = 0;
> - int ret;
> -
> - ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> - if (ret) {
> - *status = EFI_NO_MEDIA;
> - goto out;
> - }
> + const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
> + struct efi_device_path *dp = NULL;
This variable name shadows the variable defined in line 28. I think we
should give the global variable a better name then simply dp. How about
dp_lf2_handle?
Best regards
Heinrich
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2021-03-14 10:19 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-13 21:47 [PATCH 0/6 v2] Loadfile2 for initrd loading Ilias Apalodimas
2021-03-13 21:47 ` [PATCH 1/6 v2] efi_selftest: Remove loadfile2 for initrd selftests Ilias Apalodimas
2021-03-13 21:47 ` [PATCH 2/6] efi_loader: Add device path related functions for initrd via Boot#### Ilias Apalodimas
2021-03-14 7:19 ` Heinrich Schuchardt
2021-03-14 7:32 ` Ilias Apalodimas
2021-03-13 21:47 ` [PATCH 3/6 v2] efi_loader: Introduce helper functions for EFI Ilias Apalodimas
2021-03-14 7:31 ` Heinrich Schuchardt
2021-03-14 7:34 ` Ilias Apalodimas
2021-03-14 8:01 ` Heinrich Schuchardt
2021-03-14 8:07 ` Ilias Apalodimas
2021-03-14 8:49 ` Heinrich Schuchardt
2021-03-14 9:24 ` Heinrich Schuchardt
2021-03-13 21:47 ` [PATCH 4/6 v2] efi_loader: Replace config option for initrd loading Ilias Apalodimas
2021-03-14 9:54 ` Heinrich Schuchardt
2021-03-14 10:08 ` Heinrich Schuchardt
2021-03-14 10:19 ` Heinrich Schuchardt
2021-03-13 21:47 ` [PATCH 5/6 v2] efidebug: add multiple device path instances on Boot#### Ilias Apalodimas
2021-03-14 9:27 ` Heinrich Schuchardt
2021-03-14 9:42 ` Heinrich Schuchardt
2021-03-14 9:44 ` Ilias Apalodimas
2021-03-14 10:14 ` Heinrich Schuchardt
2021-03-13 21:47 ` [PATCH 6/6 v2] doc: Update uefi documentation for initrd loading options Ilias Apalodimas
2021-03-14 7:53 ` Heinrich Schuchardt
2021-03-14 9:02 ` Ilias Apalodimas
2021-03-14 8:41 ` [PATCH 0/6 v2] Loadfile2 for initrd loading Heinrich Schuchardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.