From 947d4f132b4d67a0d15859b354ed3fc71410105b Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 29 Sep 2022 22:27:06 +0000 Subject: [PATCH 01/21] regmap: fix range checks On the 32bit ARM sandbox 'dm ut dm_test_devm_regmap' fails with an abort. This is due to incorrect range checks. On 32-bit systems the size of size_t and int is both 32 bit. The expression (offset + val_len) is bound to overflow if offset == -1. Add an overflow check. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- drivers/core/regmap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c index 5f98f85cfce..5ccbf9abb8a 100644 --- a/drivers/core/regmap.c +++ b/drivers/core/regmap.c @@ -399,7 +399,7 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset, range = &map->ranges[range_num]; offset <<= map->reg_offset_shift; - if (offset + val_len > range->size) { + if (offset + val_len > range->size || offset + val_len < offset) { debug("%s: offset/size combination invalid\n", __func__); return -ERANGE; } @@ -538,7 +538,7 @@ int regmap_raw_write_range(struct regmap *map, uint range_num, uint offset, range = &map->ranges[range_num]; offset <<= map->reg_offset_shift; - if (offset + val_len > range->size) { + if (offset + val_len > range->size || offset + val_len < offset) { debug("%s: offset/size combination invalid\n", __func__); return -ERANGE; } From 932f72d68efa4f38846f90f258e07f7bb1a2cd70 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 4 Oct 2022 22:02:46 +0200 Subject: [PATCH 02/21] doc: typo 'it it' in doc/develop/package/index.rst %s/it it/it/ Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- doc/develop/package/index.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/develop/package/index.rst b/doc/develop/package/index.rst index 9374be2e62c..4f448313f95 100644 --- a/doc/develop/package/index.rst +++ b/doc/develop/package/index.rst @@ -4,7 +4,7 @@ Package U-Boot ============== U-Boot uses Flat Image Tree (FIT) as a standard file format for packaging -images that it it reads and boots. Documentation about FIT is available at +images that it reads and boots. Documentation about FIT is available at doc/uImage.FIT U-Boot also provides binman for cases not covered by FIT. Examples include From 39434a9b25dcfc0f6b5aba59e9b6d691468891a4 Mon Sep 17 00:00:00 2001 From: Paul Barker Date: Wed, 5 Oct 2022 13:18:35 +0100 Subject: [PATCH 03/21] efi: Add string conversion helper Signed-off-by: Paul Barker Reviewed-by: Heinrich Schuchardt --- include/efi_loader.h | 3 ++- lib/efi_loader/efi_string.c | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index ad01395b39c..8c45e3ee259 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -1014,9 +1014,10 @@ struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, /* runtime implementation of memcpy() */ void efi_memcpy_runtime(void *dest, const void *src, size_t n); -/* commonly used helper function */ +/* commonly used helper functions */ u16 *efi_create_indexed_name(u16 *buffer, size_t buffer_size, const char *name, unsigned int index); +efi_string_t efi_convert_string(const char *str); extern const struct efi_firmware_management_protocol efi_fmp_fit; extern const struct efi_firmware_management_protocol efi_fmp_raw; diff --git a/lib/efi_loader/efi_string.c b/lib/efi_loader/efi_string.c index 8bf1e493b89..e21e09c9461 100644 --- a/lib/efi_loader/efi_string.c +++ b/lib/efi_loader/efi_string.c @@ -8,6 +8,7 @@ #include #include #include +#include /** * efi_create_indexed_name - create a string name with an index @@ -41,3 +42,26 @@ u16 *efi_create_indexed_name(u16 *buffer, size_t buffer_size, const char *name, return p; } + +/** + * efi_convert_string - Convert an ASCII or UTF-8 string to UTF-16 + * @str: String to be converted + * + * Return: Converted string in UTF-16 format. The caller is responsible for + * freeing this string when it is no longer needed. + */ +efi_string_t efi_convert_string(const char *str) +{ + efi_string_t str_16, tmp; + size_t sz_16; + + sz_16 = utf8_utf16_strlen(str); + str_16 = calloc(sz_16 + 1, sizeof(u16)); + if (!str_16) + return NULL; + + tmp = str_16; + utf8_utf16_strcpy(&tmp, str); + + return str_16; +} From 57b2d300afc987f53fdbccee583f267f17f65654 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 6 Oct 2022 06:52:51 +0200 Subject: [PATCH 04/21] cmd: simplify do_env_set_efi() Use efi_convert_string() to convert a UTF-8 to a UTF-16 string. Signed-off-by: Heinrich Schuchardt --- cmd/nvedit_efi.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c index 770877c5272..24944ab81e2 100644 --- a/cmd/nvedit_efi.c +++ b/cmd/nvedit_efi.c @@ -382,8 +382,7 @@ int do_env_set_efi(struct cmd_tbl *cmdtp, int flag, int argc, efi_guid_t guid; u32 attributes; bool default_guid, verbose, value_on_memory; - u16 *var_name16 = NULL, *p; - size_t len; + u16 *var_name16; efi_status_t ret; if (argc == 1) @@ -487,18 +486,15 @@ int do_env_set_efi(struct cmd_tbl *cmdtp, int flag, int argc, 16, 1, value, size, true); } - len = utf8_utf16_strnlen(var_name, strlen(var_name)); - var_name16 = malloc((len + 1) * 2); + var_name16 = efi_convert_string(var_name); if (!var_name16) { printf("## Out of memory\n"); ret = CMD_RET_FAILURE; goto out; } - p = var_name16; - utf8_utf16_strncpy(&p, var_name, len + 1); - ret = efi_set_variable_int(var_name16, &guid, attributes, size, value, true); + free(var_name16); unmap_sysmem(value); if (ret == EFI_SUCCESS) { ret = CMD_RET_SUCCESS; @@ -533,7 +529,6 @@ out: unmap_sysmem(value); else free(value); - free(var_name16); return ret; } From 80fadf4af6413f9668f6cfaea39f7a101103ce9e Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 6 Oct 2022 06:48:02 +0200 Subject: [PATCH 05/21] cmd: simplify do_efi_boot_add() Use efi_convert_string() to convert a UTF-8 to a UTF-16 string. Signed-off-by: Heinrich Schuchardt --- cmd/efidebug.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 84e6ff55650..25b5c47f3e3 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -812,7 +812,6 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, char *endp; u16 var_name16[9]; efi_guid_t guid; - size_t label_len, label_len16; u16 *label; struct efi_device_path *file_path = NULL; struct efi_device_path *fp_free = NULL; @@ -859,13 +858,10 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, "Boot", id); /* label */ - label_len = strlen(argv[2]); - label_len16 = utf8_utf16_strnlen(argv[2], label_len); - label = malloc((label_len16 + 1) * sizeof(u16)); + label = efi_convert_string(argv[2]); if (!label) return CMD_RET_FAILURE; lo.label = label; /* label will be changed below */ - utf8_utf16_strncpy(&label, argv[2], label_len); /* file path */ ret = efi_dp_from_name(argv[3], argv[4], argv[5], From 01caf28778bfe04838bd4c548ccc975f5a2f59ee Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 6 Oct 2022 13:36:02 +0200 Subject: [PATCH 06/21] efi_loader: efi_dp_part_node check dp_alloc return value dp_alloc() may return NULL. This needs to be caught. Fixes: 98d48bdf415e ("efi_loader: provide a function to create a partition node") Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_device_path.c | 3 ++- lib/efi_loader/efi_disk.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ebffb771228..acae007f26f 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -936,7 +936,8 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part) dpsize = sizeof(struct efi_device_path_hard_drive_path); buf = dp_alloc(dpsize); - dp_part_node(buf, desc, part); + if (buf) + dp_part_node(buf, desc, part); return buf; } diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 39ea1a68a68..79b28097b6c 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -415,6 +415,11 @@ static efi_status_t efi_disk_add_dev( struct efi_handler *handler; void *protocol_interface; + if (!node) { + ret = EFI_OUT_OF_RESOURCES; + goto error; + } + /* Parent must expose EFI_BLOCK_IO_PROTOCOL */ ret = efi_search_protocol(parent, &efi_block_io_guid, &handler); if (ret != EFI_SUCCESS) From 874490c7ec7a05a429b951720f11a3b966ec0572 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 1 Oct 2022 20:55:14 +0200 Subject: [PATCH 07/21] test: fix some pylint errors in test_efi_secboot * Remove unused import * Provide module docstring Signed-off-by: Heinrich Schuchardt Acked-by: Ilias Apalodimas --- test/py/tests/test_efi_secboot/conftest.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py index db6b8d301f8..406131cb45e 100644 --- a/test/py/tests/test_efi_secboot/conftest.py +++ b/test/py/tests/test_efi_secboot/conftest.py @@ -2,18 +2,12 @@ # Copyright (c) 2019, Linaro Limited # Author: AKASHI Takahiro -import os -import os.path -from subprocess import call, check_call, check_output, CalledProcessError +""" Fixture for UEFI secure boot test """ + +from subprocess import call, check_call, CalledProcessError import pytest from defs import * - -# -# Fixture for UEFI secure boot test -# - - @pytest.fixture(scope='session') def efi_boot_env(request, u_boot_config): """Set up a file system to be used in UEFI secure boot test. From 16b27b67c5002c13d84bdf68727954ec765f0731 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 3 Oct 2022 09:47:51 +0200 Subject: [PATCH 08/21] efi_loader: function to unlink udevice and handle When deleting a device or a handle we must remove the link between the two to avoid dangling references. Provide function efi_unlink_dev() for this purpose. Signed-off-by: Heinrich Schuchardt --- include/efi_loader.h | 1 + lib/efi_loader/efi_helper.c | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/include/efi_loader.h b/include/efi_loader.h index 8c45e3ee259..6f78f774047 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -708,6 +708,7 @@ const char *guid_to_sha_str(const efi_guid_t *guid); int algo_to_len(const char *algo); int efi_link_dev(efi_handle_t handle, struct udevice *dev); +int efi_unlink_dev(efi_handle_t handle); /** * efi_size_in_pages() - convert size in bytes to size in pages diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index 8ed564e2619..c71e87d1180 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -171,3 +171,22 @@ int efi_link_dev(efi_handle_t handle, struct udevice *dev) handle->dev = dev; return dev_tag_set_ptr(dev, DM_TAG_EFI, handle); } + +/** + * efi_unlink_dev() - unlink udevice and handle + * + * @handle: EFI handle to unlink + * + * Return: 0 on success, negative on failure + */ +int efi_unlink_dev(efi_handle_t handle) +{ + int ret; + + ret = dev_tag_del(handle->dev, DM_TAG_EFI); + if (ret) + return ret; + handle->dev = NULL; + + return 0; +} From 43a5891c66c8fe961999415b051c827ce1b543c4 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 3 Oct 2022 10:35:35 +0200 Subject: [PATCH 09/21] efi_driver: fix error handling If creating the block device fails, * delete all created objects and references * close the protocol interface on the controller Signed-off-by: Heinrich Schuchardt --- include/efi_driver.h | 2 +- lib/efi_driver/efi_block_device.c | 58 +++++++++++++++++-------------- lib/efi_driver/efi_uclass.c | 23 ++++++------ 3 files changed, 45 insertions(+), 38 deletions(-) diff --git a/include/efi_driver.h b/include/efi_driver.h index 2b62219c5bf..dc0c1c7ac07 100644 --- a/include/efi_driver.h +++ b/include/efi_driver.h @@ -25,7 +25,7 @@ struct efi_driver_ops { const efi_guid_t *protocol; const efi_guid_t *child_protocol; - int (*bind)(efi_handle_t handle, void *interface); + efi_status_t (*bind)(efi_handle_t handle, void *interface); }; /* diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index 3177ab67b81..9ccc1485907 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -112,12 +112,13 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, * * @handle: handle * @interface: block io protocol - * Return: 0 = success + * Return: status code */ -static int efi_bl_bind(efi_handle_t handle, void *interface) +static efi_status_t efi_bl_bind(efi_handle_t handle, void *interface) { - struct udevice *bdev, *parent = dm_root(); - int ret, devnum; + struct udevice *bdev = NULL, *parent = dm_root(); + efi_status_t ret; + int devnum; char *name; struct efi_object *obj = efi_search_obj(handle); struct efi_block_io *io = interface; @@ -125,28 +126,28 @@ static int efi_bl_bind(efi_handle_t handle, void *interface) EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io); - if (!obj) - return -ENOENT; + if (!obj || !interface) + return EFI_INVALID_PARAMETER; devnum = blk_find_max_devnum(UCLASS_EFI_LOADER); if (devnum == -ENODEV) devnum = 0; else if (devnum < 0) - return devnum; + return EFI_OUT_OF_RESOURCES; name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */ if (!name) - return -ENOMEM; + return EFI_OUT_OF_RESOURCES; sprintf(name, "efiblk#%d", devnum); /* Create driver model udevice for the EFI block io device */ - ret = blk_create_device(parent, "efi_blk", name, UCLASS_EFI_LOADER, - devnum, io->media->block_size, - (lbaint_t)io->media->last_block, &bdev); - if (ret) - return ret; - if (!bdev) - return -ENOENT; + if (blk_create_device(parent, "efi_blk", name, UCLASS_EFI_LOADER, + devnum, io->media->block_size, + (lbaint_t)io->media->last_block, &bdev)) { + ret = EFI_OUT_OF_RESOURCES; + free(name); + goto err; + } /* Set the DM_FLAG_NAME_ALLOCED flag to avoid a memory leak */ device_set_name_alloced(bdev); @@ -154,20 +155,25 @@ static int efi_bl_bind(efi_handle_t handle, void *interface) plat->handle = handle; plat->io = interface; - /* - * FIXME: necessary because we won't do almost nothing in - * efi_disk_create() when called from device_probe(). - */ - if (efi_link_dev(handle, bdev)) - /* FIXME: cleanup for bdev */ - return ret; + if (efi_link_dev(handle, bdev)) { + ret = EFI_OUT_OF_RESOURCES; + goto err; + } - ret = device_probe(bdev); - if (ret) - return ret; + if (device_probe(bdev)) { + ret = EFI_DEVICE_ERROR; + goto err; + } EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name); - return 0; + return EFI_SUCCESS; + +err: + efi_unlink_dev(handle); + if (bdev) + device_unbind(bdev); + + return ret; } /* Block device driver operators */ diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index 74dd0032437..5a285aad898 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -11,7 +11,7 @@ * The uclass provides the bind, start, and stop entry points for the driver * binding protocol. * - * In bind() and stop() it checks if the controller implements the protocol + * In supported() and bind() it checks if the controller implements the protocol * supported by the EFI driver. In the start() function it calls the bind() * function of the EFI driver. In the stop() function it destroys the child * controllers. @@ -144,18 +144,19 @@ static efi_status_t EFIAPI efi_uc_start( goto out; } ret = check_node_type(controller_handle); - if (ret != EFI_SUCCESS) { - r = EFI_CALL(systab.boottime->close_protocol( - controller_handle, bp->ops->protocol, - this->driver_binding_handle, - controller_handle)); - if (r != EFI_SUCCESS) - EFI_PRINT("Failure to close handle\n"); + if (ret != EFI_SUCCESS) + goto err; + ret = bp->ops->bind(controller_handle, interface); + if (ret == EFI_SUCCESS) goto out; - } - /* TODO: driver specific stuff */ - bp->ops->bind(controller_handle, interface); +err: + r = EFI_CALL(systab.boottime->close_protocol( + controller_handle, bp->ops->protocol, + this->driver_binding_handle, + controller_handle)); + if (r != EFI_SUCCESS) + EFI_PRINT("Failure to close handle\n"); out: return EFI_EXIT(ret); From 8b1641680d220e7e6cf467f7e2d627c4cbd66436 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 4 Oct 2022 12:50:51 +0200 Subject: [PATCH 10/21] efi_driver: simplify efi_uc_stop(), call efi_free_pool() We have exported efi_free_pool(). There is no need to use EFI_CALL(). Signed-off-by: Heinrich Schuchardt --- lib/efi_driver/efi_uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index 5a285aad898..aabee0e2601 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -246,7 +246,7 @@ static efi_status_t EFIAPI efi_uc_stop( goto out; } } - ret = EFI_CALL(systab.boottime->free_pool(entry_buffer)); + ret = efi_free_pool(entry_buffer); if (ret != EFI_SUCCESS) log_err("Cannot free EFI memory pool\n"); From f3290be388aa12971ba3cabc969b8d3c94ea7035 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 4 Oct 2022 15:31:17 +0200 Subject: [PATCH 11/21] cmd: simplify command efidebug Currently we have subcommands 'efidebug dh' which shows protocols per handle and 'efidebug devices' which shows the device path. None shows which U-Boot device matches the handle. Change 'efidebug dh' to show the device path and the U-Boot device if any is associated with the handle. Remove 'efidebug devices'. Old output of 'efidebug dh': Handle Protocols ================ ==================== 000000001b22e690 Device Path, Block IO 000000001b22e800 Device Path, Block IO, system, Simple File System New output of 'efidebug dh': 000000001b22e690 (host0) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,00) Block IO 000000001b22e800 (host0:1) /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,00)/HD(1,GPT,7e5c17c5-3f5f-49d0-ae96-511b21d7f273,0x800,0x3f7df) Block IO system Simple File System Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- cmd/efidebug.c | 102 ++++++++----------------------------------------- 1 file changed, 15 insertions(+), 87 deletions(-) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 25b5c47f3e3..4b49f30d937 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -344,79 +345,11 @@ static int do_efi_capsule(struct cmd_tbl *cmdtp, int flag, } #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ -/** - * efi_get_device_path_text() - get device path text - * - * Return the text representation of the device path of a handle. - * - * @handle: handle of UEFI device - * Return: - * Pointer to the device path text or NULL. - * The caller is responsible for calling FreePool(). - */ -static u16 *efi_get_device_path_text(efi_handle_t handle) -{ - struct efi_handler *handler; - efi_status_t ret; - - ret = efi_search_protocol(handle, &efi_guid_device_path, &handler); - if (ret == EFI_SUCCESS && handler->protocol_interface) { - struct efi_device_path *dp = handler->protocol_interface; - - return efi_dp_str(dp); - } else { - return NULL; - } -} - #define EFI_HANDLE_WIDTH ((int)sizeof(efi_handle_t) * 2) static const char spc[] = " "; static const char sep[] = "================"; -/** - * do_efi_show_devices() - show UEFI devices - * - * @cmdtp: Command table - * @flag: Command flag - * @argc: Number of arguments - * @argv: Argument array - * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure - * - * Implement efidebug "devices" sub-command. - * Show all UEFI devices and their information. - */ -static int do_efi_show_devices(struct cmd_tbl *cmdtp, int flag, - int argc, char *const argv[]) -{ - efi_handle_t *handles; - efi_uintn_t num, i; - u16 *dev_path_text; - efi_status_t ret; - - ret = EFI_CALL(efi_locate_handle_buffer(ALL_HANDLES, NULL, NULL, - &num, &handles)); - if (ret != EFI_SUCCESS) - return CMD_RET_FAILURE; - - if (!num) - return CMD_RET_SUCCESS; - - printf("Device%.*s Device Path\n", EFI_HANDLE_WIDTH - 6, spc); - printf("%.*s ====================\n", EFI_HANDLE_WIDTH, sep); - for (i = 0; i < num; i++) { - dev_path_text = efi_get_device_path_text(handles[i]); - if (dev_path_text) { - printf("%p %ls\n", handles[i], dev_path_text); - efi_free_pool(dev_path_text); - } - } - - efi_free_pool(handles); - - return CMD_RET_SUCCESS; -} - /** * efi_get_driver_handle_info() - get information of UEFI driver * @@ -535,26 +468,25 @@ static int do_efi_show_handles(struct cmd_tbl *cmdtp, int flag, if (!num) return CMD_RET_SUCCESS; - printf("Handle%.*s Protocols\n", EFI_HANDLE_WIDTH - 6, spc); - printf("%.*s ====================\n", EFI_HANDLE_WIDTH, sep); for (i = 0; i < num; i++) { - printf("%p", handles[i]); + struct efi_handler *handler; + + printf("\n%p", handles[i]); + if (handles[i]->dev) + printf(" (%s)", handles[i]->dev->name); + printf("\n"); + /* Print device path */ + ret = efi_search_protocol(handles[i], &efi_guid_device_path, + &handler); + if (ret == EFI_SUCCESS) + printf(" %pD\n", handler->protocol_interface); ret = EFI_CALL(BS->protocols_per_handle(handles[i], &guid, &count)); - if (ret || !count) { - putc('\n'); - continue; - } - + /* Print other protocols */ for (j = 0; j < count; j++) { - if (j) - printf(", "); - else - putc(' '); - - printf("%pUs", guid[j]); + if (guidcmp(guid[j], &efi_guid_device_path)) + printf(" %pUs\n", guid[j]); } - putc('\n'); } efi_free_pool(handles); @@ -1535,8 +1467,6 @@ static struct cmd_tbl cmd_efidebug_sub[] = { U_BOOT_CMD_MKENT(capsule, CONFIG_SYS_MAXARGS, 1, do_efi_capsule, "", ""), #endif - U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices, - "", ""), U_BOOT_CMD_MKENT(drivers, CONFIG_SYS_MAXARGS, 1, do_efi_show_drivers, "", ""), U_BOOT_CMD_MKENT(dh, CONFIG_SYS_MAXARGS, 1, do_efi_show_handles, @@ -1626,8 +1556,6 @@ static char efidebug_help_text[] = #endif "\n" #endif - "efidebug devices\n" - " - show UEFI devices\n" "efidebug drivers\n" " - show UEFI drivers\n" "efidebug dh\n" From a6d4f704ad4ae91f66d1901877f072f762812c39 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 4 Oct 2022 16:19:30 +0200 Subject: [PATCH 12/21] efi_driver: carve out function to create block device * Carve out function efi_bl_create_block_device() from efi_bl_bind(). * Add a check for U-Boot devices to efi_bl_bind(). Signed-off-by: Heinrich Schuchardt --- lib/efi_driver/efi_block_device.c | 32 ++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index 9ccc1485907..10f3cb90c43 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -114,21 +114,16 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, * @interface: block io protocol * Return: status code */ -static efi_status_t efi_bl_bind(efi_handle_t handle, void *interface) +static efi_status_t +efi_bl_create_block_device(efi_handle_t handle, void *interface) { struct udevice *bdev = NULL, *parent = dm_root(); efi_status_t ret; int devnum; char *name; - struct efi_object *obj = efi_search_obj(handle); struct efi_block_io *io = interface; struct efi_blk_plat *plat; - EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io); - - if (!obj || !interface) - return EFI_INVALID_PARAMETER; - devnum = blk_find_max_devnum(UCLASS_EFI_LOADER); if (devnum == -ENODEV) devnum = 0; @@ -176,6 +171,29 @@ err: return ret; } +/** + * efi_bl_bind() - bind to a block io protocol + * + * @handle: handle + * @interface: block io protocol + * Return: status code + */ +static efi_status_t efi_bl_bind(efi_handle_t handle, void *interface) +{ + efi_status_t ret = EFI_SUCCESS; + struct efi_object *obj = efi_search_obj(handle); + + EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, interface); + + if (!obj || !interface) + return EFI_INVALID_PARAMETER; + + if (!handle->dev) + ret = efi_bl_create_block_device(handle, interface); + + return ret; +} + /* Block device driver operators */ static const struct blk_ops efi_blk_ops = { .read = efi_bl_read, From df31fedd3929c18bd64192d1ec9b839b6295efd6 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 4 Oct 2022 18:28:24 +0200 Subject: [PATCH 13/21] doc: documentation of EFI driver binding protocol * Convert code comments in include/efi_driver.h to Sphinx style. * Add include/efi_driver.h to the HTML documentation. Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- doc/api/efi.rst | 6 ++++++ include/efi_driver.h | 19 ++++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/doc/api/efi.rst b/doc/api/efi.rst index 2b96783828b..43d6f936fb0 100644 --- a/doc/api/efi.rst +++ b/doc/api/efi.rst @@ -172,6 +172,12 @@ Firmware management protocol .. kernel-doc:: lib/efi_loader/efi_firmware.c :internal: +Driver binding protocol +~~~~~~~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: include/efi_driver.h + :internal: + Unit testing ------------ diff --git a/include/efi_driver.h b/include/efi_driver.h index dc0c1c7ac07..de38abe83bd 100644 --- a/include/efi_driver.h +++ b/include/efi_driver.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /* - * EFI application loader + * Internal structures for the EFI driver binding protocol * * Copyright (c) 2017 Heinrich Schuchardt */ @@ -10,16 +10,16 @@ #include -/* - * Operations supported by an EFI driver with respect to the EFI uclass +/** + * struct efi_driver_ops - operations support by an EFI driver * - * @protocol The GUID of the protocol which is consumed by the + * @protocol: The GUID of the protocol which is consumed by the * driver. This GUID is used by the EFI uclass in the * supports() and start() methods of the * EFI_DRIVER_BINDING_PROTOCOL. - * @child_protocol Protocol supported by the child handles generated by + * @child_protocol: Protocol supported by the child handles generated by * the EFI driver. - * @bind Function called by the EFI uclass to attach the + * @bind: Function called by the EFI uclass to attach the * driver to EFI driver to a handle. */ struct efi_driver_ops { @@ -28,8 +28,13 @@ struct efi_driver_ops { efi_status_t (*bind)(efi_handle_t handle, void *interface); }; -/* +/** + * struct efi_driver_binding_extended_protocol - extended driver binding protocol + * * This structure adds internal fields to the driver binding protocol. + * + * @bp: driver binding protocol + * @ops: operations supported by the driver */ struct efi_driver_binding_extended_protocol { struct efi_driver_binding_protocol bp; From 939f204c5a37e87052b1967cbd6971109b7176e7 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 4 Oct 2022 18:53:34 +0200 Subject: [PATCH 14/21] efi_driver: reformat efi_block_device.c * use Sphinx documentation style * correct indentation Signed-off-by: Heinrich Schuchardt --- lib/efi_driver/efi_block_device.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index 10f3cb90c43..e9eabbde58d 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -37,11 +37,11 @@ #include #include -/* - * EFI attributes of the udevice handled by this driver. +/** + * struct efi_blk_plat - attributes of a block device * - * handle handle of the controller on which this driver is installed - * io block io protocol proxied by this driver + * @handle: handle of the controller on which this driver is installed + * @io: block io protocol proxied by this driver */ struct efi_blk_plat { efi_handle_t handle; @@ -49,7 +49,7 @@ struct efi_blk_plat { }; /** - * Read from block device + * efi_bl_read() - read from block device * * @dev: device * @blknr: first block to be read @@ -78,7 +78,7 @@ static ulong efi_bl_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, } /** - * Write to block device + * efi_bl_write() - write to block device * * @dev: device * @blknr: first block to be write @@ -108,7 +108,7 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, } /** - * Create a block device for a handle + * efi_bl_create_block_device() - create a block device for a handle * * @handle: handle * @interface: block io protocol @@ -202,9 +202,9 @@ static const struct blk_ops efi_blk_ops = { /* Identify as block device driver */ U_BOOT_DRIVER(efi_blk) = { - .name = "efi_blk", - .id = UCLASS_BLK, - .ops = &efi_blk_ops, + .name = "efi_blk", + .id = UCLASS_BLK, + .ops = &efi_blk_ops, .plat_auto = sizeof(struct efi_blk_plat), }; From ec4f675f9ebec2535f2cd050aed7f9c106a5bee9 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 4 Oct 2022 19:12:59 +0200 Subject: [PATCH 15/21] efi_driver: provide driver binding protocol to bind function DisconnectController() is based on the open protocol information created when the driver opens a protocol with BY_CHILD_CONTROLLER or BY_DRIVER. To create an open protocol information it is required to supply the handle of the driver as agent handle. This information is available as field DriverBindingHandle in the driver binding protocol. Signed-off-by: Heinrich Schuchardt --- include/efi_driver.h | 29 +++++++++++++++-------------- lib/efi_driver/efi_block_device.c | 5 ++++- lib/efi_driver/efi_uclass.c | 2 +- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/include/efi_driver.h b/include/efi_driver.h index de38abe83bd..71e0d3194ea 100644 --- a/include/efi_driver.h +++ b/include/efi_driver.h @@ -10,6 +10,19 @@ #include +/** + * struct efi_driver_binding_extended_protocol - extended driver binding protocol + * + * This structure adds internal fields to the driver binding protocol. + * + * @bp: driver binding protocol + * @ops: operations supported by the driver + */ +struct efi_driver_binding_extended_protocol { + struct efi_driver_binding_protocol bp; + const struct efi_driver_ops *ops; +}; + /** * struct efi_driver_ops - operations support by an EFI driver * @@ -25,20 +38,8 @@ struct efi_driver_ops { const efi_guid_t *protocol; const efi_guid_t *child_protocol; - efi_status_t (*bind)(efi_handle_t handle, void *interface); -}; - -/** - * struct efi_driver_binding_extended_protocol - extended driver binding protocol - * - * This structure adds internal fields to the driver binding protocol. - * - * @bp: driver binding protocol - * @ops: operations supported by the driver - */ -struct efi_driver_binding_extended_protocol { - struct efi_driver_binding_protocol bp; - const struct efi_driver_ops *ops; + efi_status_t (*bind)(struct efi_driver_binding_extended_protocol *this, + efi_handle_t handle, void *interface); }; #endif /* _EFI_DRIVER_H */ diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index e9eabbde58d..f440067f702 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -174,11 +174,14 @@ err: /** * efi_bl_bind() - bind to a block io protocol * + * @this: driver binding protocol * @handle: handle * @interface: block io protocol * Return: status code */ -static efi_status_t efi_bl_bind(efi_handle_t handle, void *interface) +static efi_status_t efi_bl_bind( + struct efi_driver_binding_extended_protocol *this, + efi_handle_t handle, void *interface) { efi_status_t ret = EFI_SUCCESS; struct efi_object *obj = efi_search_obj(handle); diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index aabee0e2601..0a16c594e3a 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -146,7 +146,7 @@ static efi_status_t EFIAPI efi_uc_start( ret = check_node_type(controller_handle); if (ret != EFI_SUCCESS) goto err; - ret = bp->ops->bind(controller_handle, interface); + ret = bp->ops->bind(bp, controller_handle, interface); if (ret == EFI_SUCCESS) goto out; From 8f8fe1d458664aaa15fa82de78dfdb0eca74b2ca Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 5 Oct 2022 11:28:47 +0200 Subject: [PATCH 16/21] efi_driver: add init function to EFI block driver For handling added and removed block devices we need to register events which has to be done when the driver is installed. This patch only creates an empty init function that will be filled with code later on. The function needs to be called before any EFI block devices are used. Move the efi_driver_init() call to early init. Signed-off-by: Heinrich Schuchardt --- include/efi_driver.h | 3 +++ lib/efi_driver/efi_block_device.c | 12 ++++++++++++ lib/efi_driver/efi_uclass.c | 17 ++++++++++++----- lib/efi_loader/efi_setup.c | 10 +++++----- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/include/efi_driver.h b/include/efi_driver.h index 71e0d3194ea..63a95e4cf80 100644 --- a/include/efi_driver.h +++ b/include/efi_driver.h @@ -32,12 +32,15 @@ struct efi_driver_binding_extended_protocol { * EFI_DRIVER_BINDING_PROTOCOL. * @child_protocol: Protocol supported by the child handles generated by * the EFI driver. + * @init: Function called by the EFI uclass after installing the + * driver binding protocol. * @bind: Function called by the EFI uclass to attach the * driver to EFI driver to a handle. */ struct efi_driver_ops { const efi_guid_t *protocol; const efi_guid_t *child_protocol; + efi_status_t (*init)(struct efi_driver_binding_extended_protocol *this); efi_status_t (*bind)(struct efi_driver_binding_extended_protocol *this, efi_handle_t handle, void *interface); }; diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index f440067f702..bd3688848ba 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -197,6 +197,17 @@ static efi_status_t efi_bl_bind( return ret; } +/** + * efi_bl_init() - initialize block device driver + * + * @this: extended driver binding protocol + */ +static efi_status_t +efi_bl_init(struct efi_driver_binding_extended_protocol *this) +{ + return EFI_SUCCESS; +} + /* Block device driver operators */ static const struct blk_ops efi_blk_ops = { .read = efi_bl_read, @@ -215,6 +226,7 @@ U_BOOT_DRIVER(efi_blk) = { static const struct efi_driver_ops driver_ops = { .protocol = &efi_block_io_guid, .child_protocol = &efi_block_io_guid, + .init = efi_bl_init, .bind = efi_bl_bind, }; diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index 0a16c594e3a..2193f8493fd 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -284,7 +284,7 @@ static efi_status_t efi_add_driver(struct driver *drv) bp->bp.start = efi_uc_start; bp->bp.stop = efi_uc_stop; bp->bp.version = 0xffffffff; - bp->ops = drv->ops; + bp->ops = ops; ret = efi_create_handle(&bp->bp.driver_binding_handle); if (ret != EFI_SUCCESS) { @@ -294,13 +294,20 @@ static efi_status_t efi_add_driver(struct driver *drv) bp->bp.image_handle = bp->bp.driver_binding_handle; ret = efi_add_protocol(bp->bp.driver_binding_handle, &efi_guid_driver_binding_protocol, bp); - if (ret != EFI_SUCCESS) { - efi_delete_handle(bp->bp.driver_binding_handle); - free(bp); - goto out; + if (ret != EFI_SUCCESS) + goto err; + if (ops->init) { + ret = ops->init(bp); + if (ret != EFI_SUCCESS) + goto err; } out: return ret; + +err: + efi_delete_handle(bp->bp.driver_binding_handle); + free(bp); + return ret; } /** diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index c633fcd91e3..113d5d5d56a 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -198,6 +198,11 @@ static efi_status_t __efi_init_early(void) if (ret != EFI_SUCCESS) goto out; + /* Initialize EFI driver uclass */ + ret = efi_driver_init(); + if (ret != EFI_SUCCESS) + goto out; + ret = efi_disk_init(); out: return ret; @@ -319,11 +324,6 @@ efi_status_t efi_init_obj_list(void) if (ret != EFI_SUCCESS) goto out; - /* Initialize EFI driver uclass */ - ret = efi_driver_init(); - if (ret != EFI_SUCCESS) - goto out; - if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT)) { ret = efi_load_capsule_drivers(); if (ret != EFI_SUCCESS) From 564e55c7f4a335abb766c921eb2ae1e05ce1253c Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 6 Oct 2022 07:28:19 +0200 Subject: [PATCH 17/21] efi_selftest: rename event_notify A function event_notify() exists. We should not use the same name for and EFI event. Rename events in unit tests. Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_selftest/efi_selftest_events.c | 18 +++---- .../efi_selftest_exitbootservices.c | 6 +-- lib/efi_selftest/efi_selftest_tpl.c | 47 ++++++++++--------- lib/efi_selftest/efi_selftest_watchdog.c | 30 ++++++------ 4 files changed, 55 insertions(+), 46 deletions(-) diff --git a/lib/efi_selftest/efi_selftest_events.c b/lib/efi_selftest/efi_selftest_events.c index 90071536a27..743a6b9154f 100644 --- a/lib/efi_selftest/efi_selftest_events.c +++ b/lib/efi_selftest/efi_selftest_events.c @@ -11,7 +11,7 @@ #include -static struct efi_event *event_notify; +static struct efi_event *efi_st_event_notify; static struct efi_event *event_wait; static unsigned int timer_ticks; static struct efi_boot_services *boottime; @@ -50,7 +50,7 @@ static int setup(const efi_handle_t handle, ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, TPL_CALLBACK, notify, (void *)&timer_ticks, - &event_notify); + &efi_st_event_notify); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); return EFI_ST_FAILURE; @@ -75,9 +75,9 @@ static int teardown(void) { efi_status_t ret; - if (event_notify) { - ret = boottime->close_event(event_notify); - event_notify = NULL; + if (efi_st_event_notify) { + ret = boottime->close_event(efi_st_event_notify); + efi_st_event_notify = NULL; if (ret != EFI_SUCCESS) { efi_st_error("could not close event\n"); return EFI_ST_FAILURE; @@ -112,7 +112,8 @@ static int execute(void) /* Set 10 ms timer */ timer_ticks = 0; - ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000); + ret = boottime->set_timer(efi_st_event_notify, EFI_TIMER_PERIODIC, + 100000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; @@ -146,14 +147,15 @@ static int execute(void) efi_st_error("Incorrect timing of events\n"); return EFI_ST_FAILURE; } - ret = boottime->set_timer(event_notify, EFI_TIMER_STOP, 0); + ret = boottime->set_timer(efi_st_event_notify, EFI_TIMER_STOP, 0); if (ret != EFI_SUCCESS) { efi_st_error("Could not cancel timer\n"); return EFI_ST_FAILURE; } /* Set 10 ms timer */ timer_ticks = 0; - ret = boottime->set_timer(event_notify, EFI_TIMER_RELATIVE, 100000); + ret = boottime->set_timer(efi_st_event_notify, EFI_TIMER_RELATIVE, + 100000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; diff --git a/lib/efi_selftest/efi_selftest_exitbootservices.c b/lib/efi_selftest/efi_selftest_exitbootservices.c index f5e0d9da89b..11b43fdd90b 100644 --- a/lib/efi_selftest/efi_selftest_exitbootservices.c +++ b/lib/efi_selftest/efi_selftest_exitbootservices.c @@ -26,7 +26,7 @@ struct notification_context { }; static struct efi_boot_services *boottime; -static struct efi_event *event_notify; +static struct efi_event *efi_st_event_notify; struct notification_record record; struct notification_context context_before = { @@ -75,7 +75,7 @@ static int setup(const efi_handle_t handle, ret = boottime->create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK, ebs_notify, &context, - &event_notify); + &efi_st_event_notify); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); return EFI_ST_FAILURE; @@ -83,7 +83,7 @@ static int setup(const efi_handle_t handle, ret = boottime->create_event_ex(0, TPL_CALLBACK, ebs_notify, &context_before, &guid_before_exit_boot_services, - &event_notify); + &efi_st_event_notify); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); return EFI_ST_FAILURE; diff --git a/lib/efi_selftest/efi_selftest_tpl.c b/lib/efi_selftest/efi_selftest_tpl.c index f4e467267e5..909c78a1c23 100644 --- a/lib/efi_selftest/efi_selftest_tpl.c +++ b/lib/efi_selftest/efi_selftest_tpl.c @@ -10,8 +10,8 @@ #include -static struct efi_event *event_notify; -static struct efi_event *event_wait; +static struct efi_event *efi_st_event_notify; +static struct efi_event *efi_st_event_wait; static unsigned int notification_count; static struct efi_boot_services *boottime; @@ -49,13 +49,14 @@ static int setup(const efi_handle_t handle, ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, TPL_CALLBACK, notify, (void *)¬ification_count, - &event_notify); + &efi_st_event_notify); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); return EFI_ST_FAILURE; } ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_WAIT, - TPL_NOTIFY, notify, NULL, &event_wait); + TPL_NOTIFY, notify, NULL, + &efi_st_event_wait); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); return EFI_ST_FAILURE; @@ -74,17 +75,17 @@ static int teardown(void) { efi_status_t ret; - if (event_notify) { - ret = boottime->close_event(event_notify); - event_notify = NULL; + if (efi_st_event_notify) { + ret = boottime->close_event(efi_st_event_notify); + efi_st_event_notify = NULL; if (ret != EFI_SUCCESS) { efi_st_error("could not close event\n"); return EFI_ST_FAILURE; } } - if (event_wait) { - ret = boottime->close_event(event_wait); - event_wait = NULL; + if (efi_st_event_wait) { + ret = boottime->close_event(efi_st_event_wait); + efi_st_event_wait = NULL; if (ret != EFI_SUCCESS) { efi_st_error("could not close event\n"); return EFI_ST_FAILURE; @@ -116,24 +117,26 @@ static int execute(void) /* Set 10 ms timer */ notification_count = 0; - ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000); + ret = boottime->set_timer(efi_st_event_notify, EFI_TIMER_PERIODIC, + 100000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; } /* Set 100 ms timer */ - ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000); + ret = boottime->set_timer(efi_st_event_wait, EFI_TIMER_RELATIVE, + 1000000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; } index = 5; - ret = boottime->wait_for_event(1, &event_wait, &index); + ret = boottime->wait_for_event(1, &efi_st_event_wait, &index); if (ret != EFI_SUCCESS) { efi_st_error("Could not wait for event\n"); return EFI_ST_FAILURE; } - ret = boottime->check_event(event_wait); + ret = boottime->check_event(efi_st_event_wait); if (ret != EFI_NOT_READY) { efi_st_error("Signaled state was not cleared.\n"); efi_st_printf("ret = %u\n", (unsigned int)ret); @@ -150,7 +153,7 @@ static int execute(void) efi_st_error("Incorrect timing of events\n"); return EFI_ST_FAILURE; } - ret = boottime->set_timer(event_notify, EFI_TIMER_STOP, 0); + ret = boottime->set_timer(efi_st_event_notify, EFI_TIMER_STOP, 0); if (ret != EFI_SUCCESS) { efi_st_error("Could not cancel timer\n"); return EFI_ST_FAILURE; @@ -163,19 +166,21 @@ static int execute(void) } /* Set 10 ms timer */ notification_count = 0; - ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000); + ret = boottime->set_timer(efi_st_event_notify, EFI_TIMER_PERIODIC, + 100000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; } /* Set 100 ms timer */ - ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000); + ret = boottime->set_timer(efi_st_event_wait, EFI_TIMER_RELATIVE, + 1000000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; } do { - ret = boottime->check_event(event_wait); + ret = boottime->check_event(efi_st_event_wait); } while (ret == EFI_NOT_READY); if (ret != EFI_SUCCESS) { efi_st_error("Could not check event\n"); @@ -189,14 +194,14 @@ static int execute(void) return EFI_ST_FAILURE; } /* Set 1 ms timer */ - ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000); + ret = boottime->set_timer(efi_st_event_wait, EFI_TIMER_RELATIVE, 1000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; } /* Restore the old TPL level */ boottime->restore_tpl(TPL_APPLICATION); - ret = boottime->wait_for_event(1, &event_wait, &index); + ret = boottime->wait_for_event(1, &efi_st_event_wait, &index); if (ret != EFI_SUCCESS) { efi_st_error("Could not wait for event\n"); return EFI_ST_FAILURE; @@ -208,7 +213,7 @@ static int execute(void) efi_st_error("Queued timer event did not fire\n"); return EFI_ST_FAILURE; } - ret = boottime->set_timer(event_wait, EFI_TIMER_STOP, 0); + ret = boottime->set_timer(efi_st_event_wait, EFI_TIMER_STOP, 0); if (ret != EFI_SUCCESS) { efi_st_error("Could not cancel timer\n"); return EFI_ST_FAILURE; diff --git a/lib/efi_selftest/efi_selftest_watchdog.c b/lib/efi_selftest/efi_selftest_watchdog.c index a352d4a5adf..4d7ed5a54bb 100644 --- a/lib/efi_selftest/efi_selftest_watchdog.c +++ b/lib/efi_selftest/efi_selftest_watchdog.c @@ -28,8 +28,8 @@ struct notify_context { unsigned int timer_ticks; }; -static struct efi_event *event_notify; -static struct efi_event *event_wait; +static struct efi_event *efi_st_event_notify; +static struct efi_event *efi_st_event_wait; static struct efi_boot_services *boottime; static struct notify_context notification_context; static bool watchdog_reset; @@ -79,13 +79,14 @@ static int setup(const efi_handle_t handle, ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, TPL_CALLBACK, notify, (void *)¬ification_context, - &event_notify); + &efi_st_event_notify); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); return EFI_ST_FAILURE; } ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_WAIT, - TPL_CALLBACK, notify, NULL, &event_wait); + TPL_CALLBACK, notify, NULL, + &efi_st_event_wait); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); return EFI_ST_FAILURE; @@ -138,17 +139,17 @@ static int teardown(void) efi_st_error("Setting watchdog timer failed\n"); return EFI_ST_FAILURE; } - if (event_notify) { - ret = boottime->close_event(event_notify); - event_notify = NULL; + if (efi_st_event_notify) { + ret = boottime->close_event(efi_st_event_notify); + efi_st_event_notify = NULL; if (ret != EFI_SUCCESS) { efi_st_error("Could not close event\n"); return EFI_ST_FAILURE; } } - if (event_wait) { - ret = boottime->close_event(event_wait); - event_wait = NULL; + if (efi_st_event_wait) { + ret = boottime->close_event(efi_st_event_wait); + efi_st_event_wait = NULL; if (ret != EFI_SUCCESS) { efi_st_error("Could not close event\n"); return EFI_ST_FAILURE; @@ -181,21 +182,22 @@ static int execute(void) } if (watchdog_reset) { /* Set 600 ms timer */ - ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, - 6000000); + ret = boottime->set_timer(efi_st_event_notify, + EFI_TIMER_PERIODIC, 6000000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; } } /* Set 1350 ms timer */ - ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 13500000); + ret = boottime->set_timer(efi_st_event_wait, EFI_TIMER_RELATIVE, + 13500000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); return EFI_ST_FAILURE; } - ret = boottime->wait_for_event(1, &event_wait, &index); + ret = boottime->wait_for_event(1, &efi_st_event_wait, &index); if (ret != EFI_SUCCESS) { efi_st_error("Could not wait for event\n"); return EFI_ST_FAILURE; From f05911a126bd6b8536c8d43fd6c1d837008fcda1 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 6 Oct 2022 07:29:41 +0200 Subject: [PATCH 18/21] efi_driver: move event registration to driver Move the registration of events for the addition and removal of block devices to the block device driver. Here we can add a reference to the EFI Driver Binding protocol as context. Signed-off-by: Heinrich Schuchardt --- include/efi_loader.h | 7 +++++-- lib/efi_driver/efi_block_device.c | 16 ++++++++++++++++ lib/efi_loader/efi_disk.c | 25 ++----------------------- lib/efi_loader/efi_setup.c | 4 ---- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index 6f78f774047..15e7680af95 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -544,8 +545,6 @@ void efi_carve_out_dt_rsv(void *fdt); void efi_try_purge_kaslr_seed(void *fdt); /* Called by bootefi to make console interface available */ efi_status_t efi_console_register(void); -/* Called by efi_init_early() to add block devices when probed */ -efi_status_t efi_disk_init(void); /* Called by efi_init_obj_list() to proble all block devices */ efi_status_t efi_disks_register(void); /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ @@ -749,6 +748,10 @@ efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end, /* Called by board init to initialize the EFI drivers */ efi_status_t efi_driver_init(void); +/* Called when a block device is added */ +int efi_disk_probe(void *ctx, struct event *event); +/* Called when a block device is removed */ +int efi_disk_remove(void *ctx, struct event *event); /* Called by board init to initialize the EFI memory map */ int efi_memory_init(void); /* Adds new or overrides configuration table entry to the system table */ diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index bd3688848ba..add00eeebbe 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -205,6 +205,22 @@ static efi_status_t efi_bl_bind( static efi_status_t efi_bl_init(struct efi_driver_binding_extended_protocol *this) { + int ret; + + ret = event_register("efi_disk add", EVT_DM_POST_PROBE, + efi_disk_probe, this); + if (ret) { + log_err("Event registration for efi_disk add failed\n"); + return EFI_OUT_OF_RESOURCES; + } + + ret = event_register("efi_disk del", EVT_DM_PRE_REMOVE, + efi_disk_remove, this); + if (ret) { + log_err("Event registration for efi_disk del failed\n"); + return EFI_OUT_OF_RESOURCES; + } + return EFI_SUCCESS; } diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 79b28097b6c..a04ab338fc6 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -625,7 +625,7 @@ static int efi_disk_create_part(struct udevice *dev) * * @return 0 on success, -1 otherwise */ -static int efi_disk_probe(void *ctx, struct event *event) +int efi_disk_probe(void *ctx, struct event *event) { struct udevice *dev; enum uclass_id id; @@ -729,7 +729,7 @@ static int efi_disk_delete_part(struct udevice *dev) * * @return 0 on success, -1 otherwise */ -static int efi_disk_remove(void *ctx, struct event *event) +int efi_disk_remove(void *ctx, struct event *event) { enum uclass_id id; struct udevice *dev; @@ -745,27 +745,6 @@ static int efi_disk_remove(void *ctx, struct event *event) return 0; } -efi_status_t efi_disk_init(void) -{ - int ret; - - ret = event_register("efi_disk add", EVT_DM_POST_PROBE, - efi_disk_probe, NULL); - if (ret) { - log_err("Event registration for efi_disk add failed\n"); - return EFI_OUT_OF_RESOURCES; - } - - ret = event_register("efi_disk del", EVT_DM_PRE_REMOVE, - efi_disk_remove, NULL); - if (ret) { - log_err("Event registration for efi_disk del failed\n"); - return EFI_OUT_OF_RESOURCES; - } - - return EFI_SUCCESS; -} - /** * efi_disk_get_device_name() - get U-Boot device name associated with EFI handle * diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index 113d5d5d56a..9d7189336dc 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -200,10 +200,6 @@ static efi_status_t __efi_init_early(void) /* Initialize EFI driver uclass */ ret = efi_driver_init(); - if (ret != EFI_SUCCESS) - goto out; - - ret = efi_disk_init(); out: return ret; } From 1af680d5bc2e659a9bd532cf5c0009dd6d5bbdc3 Mon Sep 17 00:00:00 2001 From: Ilias Apalodimas Date: Thu, 6 Oct 2022 11:41:24 +0300 Subject: [PATCH 19/21] MAINTAINERS: get rid of the optee variables entry Since I am co-maintaining EFI with Heinrich remove the special entry for EFI variable storage via OP-TEE Signed-off-by: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt --- MAINTAINERS | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index b4b185aacd4..a26b36c7c20 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -871,6 +871,7 @@ F: include/cp437.h F: include/efi* F: include/pe.h F: include/asm-generic/pe.h +F: include/mm_communication.h F: lib/charset.c F: lib/efi*/ F: test/lib/efi_* @@ -884,12 +885,6 @@ F: tools/efivar.py F: tools/file2include.c F: tools/mkeficapsule.c -EFI VARIABLES VIA OP-TEE -M: Ilias Apalodimas -S: Maintained -F: lib/efi_loader/efi_variable_tee.c -F: include/mm_communication.h - ENVIRONMENT M: Joe Hershberger R: Wolfgang Denk From 05c4c9e21ae6f45ba1091917fc55f3ebc3916909 Mon Sep 17 00:00:00 2001 From: Ilias Apalodimas Date: Thu, 6 Oct 2022 16:08:46 +0300 Subject: [PATCH 20/21] efi_loader: define internal implementations of install/uninstallmultiple A following patch is cleaning up the core EFI code trying to remove sequences of efi_create_handle, efi_add_protocol. Although this works fine there's a problem with the latter since it is usually combined with efi_delete_handle() which blindly removes all protocols on a handle and deletes the handle. We should try to adhere to the EFI spec which only deletes a handle if the last instance of a protocol has been removed. Another problem is that efi_delete_handle() never checks for opened protocols, but the EFI spec defines that the caller is responsible for ensuring that there are no references to a protocol interface that is going to be removed. So let's fix this by replacing all callsites of efi_create_handle(), efi_add_protocol() , efi_delete_handle() with Install/UninstallMultipleProtocol. In order to do that redefine functions that can be used by the U-Boot proper internally and add '_ext' variants that will be used from the EFI API Signed-off-by: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt --- include/efi.h | 2 + include/efi_loader.h | 6 +- lib/efi_loader/efi_boottime.c | 274 +++++++++++++++++++++---------- lib/efi_loader/efi_capsule.c | 15 +- lib/efi_loader/efi_console.c | 14 +- lib/efi_loader/efi_disk.c | 10 +- lib/efi_loader/efi_load_initrd.c | 15 +- lib/efi_loader/efi_root_node.c | 48 +++--- 8 files changed, 248 insertions(+), 136 deletions(-) diff --git a/include/efi.h b/include/efi.h index 6159f34ad2b..42f4e58a917 100644 --- a/include/efi.h +++ b/include/efi.h @@ -37,12 +37,14 @@ #define EFIAPI __attribute__((ms_abi)) #define efi_va_list __builtin_ms_va_list #define efi_va_start __builtin_ms_va_start +#define efi_va_copy __builtin_ms_va_copy #define efi_va_arg __builtin_va_arg #define efi_va_end __builtin_ms_va_end #else #define EFIAPI asmlinkage #define efi_va_list va_list #define efi_va_start va_start +#define efi_va_copy va_copy #define efi_va_arg va_arg #define efi_va_end va_end #endif /* __x86_64__ */ diff --git a/include/efi_loader.h b/include/efi_loader.h index 15e7680af95..69d6d004f4d 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -654,8 +654,10 @@ efi_status_t efi_remove_protocol(const efi_handle_t handle, /* Delete all protocols from a handle */ efi_status_t efi_remove_all_protocols(const efi_handle_t handle); /* Install multiple protocol interfaces */ -efi_status_t EFIAPI efi_install_multiple_protocol_interfaces - (efi_handle_t *handle, ...); +efi_status_t EFIAPI +efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...); +efi_status_t EFIAPI +efi_uninstall_multiple_protocol_interfaces(efi_handle_t handle, ...); /* Get handles that support a given protocol */ efi_status_t EFIAPI efi_locate_handle_buffer( enum efi_locate_search_type search_type, diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1bfd094e89f..e776d25830f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2589,6 +2589,75 @@ found: return EFI_EXIT(EFI_SUCCESS); } +/** + * efi_install_multiple_protocol_interfaces_int() - Install multiple protocol + * interfaces + * @handle: handle on which the protocol interfaces shall be installed + * @argptr: va_list of args + * + * Core functionality of efi_install_multiple_protocol_interfaces + * Must not be called directly + * + * Return: status code + */ +static efi_status_t EFIAPI +efi_install_multiple_protocol_interfaces_int(efi_handle_t *handle, + efi_va_list argptr) +{ + const efi_guid_t *protocol; + void *protocol_interface; + efi_handle_t old_handle; + efi_status_t ret = EFI_SUCCESS; + int i = 0; + efi_va_list argptr_copy; + + if (!handle) + return EFI_INVALID_PARAMETER; + + efi_va_copy(argptr_copy, argptr); + for (;;) { + protocol = efi_va_arg(argptr, efi_guid_t*); + if (!protocol) + break; + protocol_interface = efi_va_arg(argptr, void*); + /* Check that a device path has not been installed before */ + if (!guidcmp(protocol, &efi_guid_device_path)) { + struct efi_device_path *dp = protocol_interface; + + ret = EFI_CALL(efi_locate_device_path(protocol, &dp, + &old_handle)); + if (ret == EFI_SUCCESS && + dp->type == DEVICE_PATH_TYPE_END) { + EFI_PRINT("Path %pD already installed\n", + protocol_interface); + ret = EFI_ALREADY_STARTED; + break; + } + } + ret = EFI_CALL(efi_install_protocol_interface(handle, protocol, + EFI_NATIVE_INTERFACE, + protocol_interface)); + if (ret != EFI_SUCCESS) + break; + i++; + } + if (ret == EFI_SUCCESS) + goto out; + + /* If an error occurred undo all changes. */ + for (; i; --i) { + protocol = efi_va_arg(argptr_copy, efi_guid_t*); + protocol_interface = efi_va_arg(argptr_copy, void*); + EFI_CALL(efi_uninstall_protocol_interface(*handle, protocol, + protocol_interface)); + } + +out: + efi_va_end(argptr_copy); + return ret; + +} + /** * efi_install_multiple_protocol_interfaces() - Install multiple protocol * interfaces @@ -2596,6 +2665,32 @@ found: * @...: NULL terminated argument list with pairs of protocol GUIDS and * interfaces * + * + * This is the function for internal usage in U-Boot. For the API function + * implementing the InstallMultipleProtocol service see + * efi_install_multiple_protocol_interfaces_ext() + * + * Return: status code + */ +efi_status_t EFIAPI +efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...) +{ + efi_status_t ret; + efi_va_list argptr; + + efi_va_start(argptr, handle); + ret = efi_install_multiple_protocol_interfaces_int(handle, argptr); + efi_va_end(argptr); + return ret; +} + +/** + * efi_install_multiple_protocol_interfaces_ext() - Install multiple protocol + * interfaces + * @handle: handle on which the protocol interfaces shall be installed + * @...: NULL terminated argument list with pairs of protocol GUIDS and + * interfaces + * * This function implements the MultipleProtocolInterfaces service. * * See the Unified Extensible Firmware Interface (UEFI) specification for @@ -2603,64 +2698,84 @@ found: * * Return: status code */ -efi_status_t EFIAPI efi_install_multiple_protocol_interfaces - (efi_handle_t *handle, ...) +static efi_status_t EFIAPI +efi_install_multiple_protocol_interfaces_ext(efi_handle_t *handle, ...) { EFI_ENTRY("%p", handle); - + efi_status_t ret; efi_va_list argptr; - const efi_guid_t *protocol; - void *protocol_interface; - efi_handle_t old_handle; - efi_status_t r = EFI_SUCCESS; - int i = 0; - - if (!handle) - return EFI_EXIT(EFI_INVALID_PARAMETER); efi_va_start(argptr, handle); + ret = efi_install_multiple_protocol_interfaces_int(handle, argptr); + efi_va_end(argptr); + return EFI_EXIT(ret); +} + +/** + * efi_uninstall_multiple_protocol_interfaces_int() - wrapper for uninstall + * multiple protocol + * interfaces + * @handle: handle from which the protocol interfaces shall be removed + * @argptr: va_list of args + * + * Core functionality of efi_uninstall_multiple_protocol_interfaces + * Must not be called directly + * + * Return: status code + */ +static efi_status_t EFIAPI +efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle, + efi_va_list argptr) +{ + const efi_guid_t *protocol; + void *protocol_interface; + efi_status_t ret; + size_t i = 0; + efi_va_list argptr_copy; + + if (!handle) + return EFI_INVALID_PARAMETER; + + efi_va_copy(argptr_copy, argptr); for (;;) { protocol = efi_va_arg(argptr, efi_guid_t*); if (!protocol) break; protocol_interface = efi_va_arg(argptr, void*); - /* Check that a device path has not been installed before */ - if (!guidcmp(protocol, &efi_guid_device_path)) { - struct efi_device_path *dp = protocol_interface; - - r = EFI_CALL(efi_locate_device_path(protocol, &dp, - &old_handle)); - if (r == EFI_SUCCESS && - dp->type == DEVICE_PATH_TYPE_END) { - EFI_PRINT("Path %pD already installed\n", - protocol_interface); - r = EFI_ALREADY_STARTED; - break; - } - } - r = EFI_CALL(efi_install_protocol_interface( - handle, protocol, - EFI_NATIVE_INTERFACE, - protocol_interface)); - if (r != EFI_SUCCESS) + ret = efi_uninstall_protocol(handle, protocol, + protocol_interface); + if (ret != EFI_SUCCESS) break; i++; } - efi_va_end(argptr); - if (r == EFI_SUCCESS) - return EFI_EXIT(r); + if (ret == EFI_SUCCESS) { + /* If the last protocol has been removed, delete the handle. */ + if (list_empty(&handle->protocols)) { + list_del(&handle->link); + free(handle); + } + goto out; + } /* If an error occurred undo all changes. */ - efi_va_start(argptr, handle); for (; i; --i) { - protocol = efi_va_arg(argptr, efi_guid_t*); - protocol_interface = efi_va_arg(argptr, void*); - EFI_CALL(efi_uninstall_protocol_interface(*handle, protocol, - protocol_interface)); + protocol = efi_va_arg(argptr_copy, efi_guid_t*); + protocol_interface = efi_va_arg(argptr_copy, void*); + EFI_CALL(efi_install_protocol_interface(&handle, protocol, + EFI_NATIVE_INTERFACE, + protocol_interface)); } - efi_va_end(argptr); + /* + * If any errors are generated while the protocol interfaces are being + * uninstalled, then the protocols uninstalled prior to the error will + * be reinstalled using InstallProtocolInterface() and the status code + * EFI_INVALID_PARAMETER is returned. + */ + ret = EFI_INVALID_PARAMETER; - return EFI_EXIT(r); +out: + efi_va_end(argptr_copy); + return ret; } /** @@ -2672,60 +2787,49 @@ efi_status_t EFIAPI efi_install_multiple_protocol_interfaces * * This function implements the UninstallMultipleProtocolInterfaces service. * + * This is the function for internal usage in U-Boot. For the API function + * implementing the UninstallMultipleProtocolInterfaces service see + * efi_uninstall_multiple_protocol_interfaces_ext() + * + * Return: status code + */ +efi_status_t EFIAPI +efi_uninstall_multiple_protocol_interfaces(efi_handle_t handle, ...) +{ + efi_status_t ret; + efi_va_list argptr; + + efi_va_start(argptr, handle); + ret = efi_uninstall_multiple_protocol_interfaces_int(handle, argptr); + efi_va_end(argptr); + return ret; +} + +/** + * efi_uninstall_multiple_protocol_interfaces_ext() - uninstall multiple protocol + * interfaces + * @handle: handle from which the protocol interfaces shall be removed + * @...: NULL terminated argument list with pairs of protocol GUIDS and + * interfaces + * + * This function implements the UninstallMultipleProtocolInterfaces service. + * * See the Unified Extensible Firmware Interface (UEFI) specification for * details. * * Return: status code */ -static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces( - efi_handle_t handle, ...) +static efi_status_t EFIAPI +efi_uninstall_multiple_protocol_interfaces_ext(efi_handle_t handle, ...) { EFI_ENTRY("%p", handle); - + efi_status_t ret; efi_va_list argptr; - const efi_guid_t *protocol; - void *protocol_interface; - efi_status_t r = EFI_SUCCESS; - size_t i = 0; - - if (!handle) - return EFI_EXIT(EFI_INVALID_PARAMETER); efi_va_start(argptr, handle); - for (;;) { - protocol = efi_va_arg(argptr, efi_guid_t*); - if (!protocol) - break; - protocol_interface = efi_va_arg(argptr, void*); - r = efi_uninstall_protocol(handle, protocol, - protocol_interface); - if (r != EFI_SUCCESS) - break; - i++; - } + ret = efi_uninstall_multiple_protocol_interfaces_int(handle, argptr); efi_va_end(argptr); - if (r == EFI_SUCCESS) { - /* If the last protocol has been removed, delete the handle. */ - if (list_empty(&handle->protocols)) { - list_del(&handle->link); - free(handle); - } - return EFI_EXIT(r); - } - - /* If an error occurred undo all changes. */ - efi_va_start(argptr, handle); - for (; i; --i) { - protocol = efi_va_arg(argptr, efi_guid_t*); - protocol_interface = efi_va_arg(argptr, void*); - EFI_CALL(efi_install_protocol_interface(&handle, protocol, - EFI_NATIVE_INTERFACE, - protocol_interface)); - } - efi_va_end(argptr); - - /* In case of an error always return EFI_INVALID_PARAMETER */ - return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_EXIT(ret); } /** @@ -3785,9 +3889,9 @@ static struct efi_boot_services efi_boot_services = { .locate_handle_buffer = efi_locate_handle_buffer, .locate_protocol = efi_locate_protocol, .install_multiple_protocol_interfaces = - efi_install_multiple_protocol_interfaces, + efi_install_multiple_protocol_interfaces_ext, .uninstall_multiple_protocol_interfaces = - efi_uninstall_multiple_protocol_interfaces, + efi_uninstall_multiple_protocol_interfaces_ext, .calculate_crc32 = efi_calculate_crc32, .copy_mem = efi_copy_mem, .set_mem = efi_set_mem, diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index a6b98f066a0..b6bd2d6af88 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -636,17 +636,18 @@ efi_status_t __weak efi_load_capsule_drivers(void) if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_FIT)) { handle = NULL; - ret = EFI_CALL(efi_install_multiple_protocol_interfaces( - &handle, &efi_guid_firmware_management_protocol, - &efi_fmp_fit, NULL)); + ret = efi_install_multiple_protocol_interfaces(&handle, + &efi_guid_firmware_management_protocol, + &efi_fmp_fit, + NULL); } if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)) { handle = NULL; - ret = EFI_CALL(efi_install_multiple_protocol_interfaces( - &handle, - &efi_guid_firmware_management_protocol, - &efi_fmp_raw, NULL)); + ret = efi_install_multiple_protocol_interfaces(&handle, + &efi_guid_firmware_management_protocol, + &efi_fmp_raw, + NULL); } return ret; diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index cf9fbd9cb54..3354b217a9a 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -1278,12 +1278,14 @@ efi_status_t efi_console_register(void) struct efi_device_path *dp; /* Install protocols on root node */ - r = EFI_CALL(efi_install_multiple_protocol_interfaces - (&efi_root, - &efi_guid_text_output_protocol, &efi_con_out, - &efi_guid_text_input_protocol, &efi_con_in, - &efi_guid_text_input_ex_protocol, &efi_con_in_ex, - NULL)); + r = efi_install_multiple_protocol_interfaces(&efi_root, + &efi_guid_text_output_protocol, + &efi_con_out, + &efi_guid_text_input_protocol, + &efi_con_in, + &efi_guid_text_input_ex_protocol, + &efi_con_in_ex, + NULL); /* Create console node and install device path protocols */ if (CONFIG_IS_ENABLED(DM_SERIAL)) { diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index a04ab338fc6..e6a356b5897 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -454,10 +454,12 @@ static efi_status_t efi_disk_add_dev( * in this case. */ handle = &diskobj->header; - ret = EFI_CALL(efi_install_multiple_protocol_interfaces( - &handle, &efi_guid_device_path, diskobj->dp, - &efi_block_io_guid, &diskobj->ops, - guid, NULL, NULL)); + ret = efi_install_multiple_protocol_interfaces(&handle, + &efi_guid_device_path, + diskobj->dp, + &efi_block_io_guid, + &diskobj->ops, guid, + NULL, NULL); if (ret != EFI_SUCCESS) goto error; diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c index 3d6044f7604..87fde3f88c2 100644 --- a/lib/efi_loader/efi_load_initrd.c +++ b/lib/efi_loader/efi_load_initrd.c @@ -208,14 +208,13 @@ efi_status_t efi_initrd_register(void) if (ret != EFI_SUCCESS) return ret; - ret = EFI_CALL(efi_install_multiple_protocol_interfaces - (&efi_initrd_handle, - /* initramfs */ - &efi_guid_device_path, &dp_lf2_handle, - /* LOAD_FILE2 */ - &efi_guid_load_file2_protocol, - (void *)&efi_lf2_protocol, - NULL)); + ret = efi_install_multiple_protocol_interfaces(&efi_initrd_handle, + /* initramfs */ + &efi_guid_device_path, &dp_lf2_handle, + /* LOAD_FILE2 */ + &efi_guid_load_file2_protocol, + (void *)&efi_lf2_protocol, + NULL); return ret; } diff --git a/lib/efi_loader/efi_root_node.c b/lib/efi_loader/efi_root_node.c index 739c6867f41..a4eb6f493dc 100644 --- a/lib/efi_loader/efi_root_node.c +++ b/lib/efi_loader/efi_root_node.c @@ -49,38 +49,38 @@ efi_status_t efi_root_node_register(void) dp->end.length = sizeof(struct efi_device_path); /* Create root node and install protocols */ - ret = EFI_CALL(efi_install_multiple_protocol_interfaces - (&efi_root, - /* Device path protocol */ - &efi_guid_device_path, dp, + ret = efi_install_multiple_protocol_interfaces + (&efi_root, + /* Device path protocol */ + &efi_guid_device_path, dp, #if CONFIG_IS_ENABLED(EFI_DEVICE_PATH_TO_TEXT) - /* Device path to text protocol */ - &efi_guid_device_path_to_text_protocol, - (void *)&efi_device_path_to_text, + /* Device path to text protocol */ + &efi_guid_device_path_to_text_protocol, + &efi_device_path_to_text, #endif -#ifdef CONFIG_EFI_DEVICE_PATH_UTIL - /* Device path utilities protocol */ - &efi_guid_device_path_utilities_protocol, - (void *)&efi_device_path_utilities, +#if CONFIG_IS_ENABLED(EFI_DEVICE_PATH_UTIL) + /* Device path utilities protocol */ + &efi_guid_device_path_utilities_protocol, + &efi_device_path_utilities, #endif -#ifdef CONFIG_EFI_DT_FIXUP - /* Device-tree fix-up protocol */ - &efi_guid_dt_fixup_protocol, - (void *)&efi_dt_fixup_prot, +#if CONFIG_IS_ENABLED(EFI_DT_FIXUP) + /* Device-tree fix-up protocol */ + &efi_guid_dt_fixup_protocol, + &efi_dt_fixup_prot, #endif #if CONFIG_IS_ENABLED(EFI_UNICODE_COLLATION_PROTOCOL2) - &efi_guid_unicode_collation_protocol2, - (void *)&efi_unicode_collation_protocol2, + &efi_guid_unicode_collation_protocol2, + &efi_unicode_collation_protocol2, #endif #if CONFIG_IS_ENABLED(EFI_LOADER_HII) - /* HII string protocol */ - &efi_guid_hii_string_protocol, - (void *)&efi_hii_string, - /* HII database protocol */ - &efi_guid_hii_database_protocol, - (void *)&efi_hii_database, + /* HII string protocol */ + &efi_guid_hii_string_protocol, + &efi_hii_string, + /* HII database protocol */ + &efi_guid_hii_database_protocol, + &efi_hii_database, #endif - NULL)); + NULL); efi_root->type = EFI_OBJECT_TYPE_U_BOOT_FIRMWARE; return ret; } From a75e8355eaa561ebd6128c92a90288d5d7c1f060 Mon Sep 17 00:00:00 2001 From: Ilias Apalodimas Date: Thu, 6 Oct 2022 16:08:44 +0300 Subject: [PATCH 21/21] cmd: replace efi_create_handle/add_protocol with InstallMultipleProtocol In general handles should only be deleted if the last remaining protocol is removed. Instead of explicitly calling efi_create_handle -> efi_add_protocol -> efi_delete_handle which blindly removes all protocols from a handle before removing it, use InstallMultiple/UninstallMultiple which adheres to the EFI spec and only deletes a handle if there are no additional protocols present Signed-off-by: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt --- cmd/bootefi.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 3041873afbb..b93c0d3d4c0 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -492,7 +492,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) efi_handle_t mem_handle = NULL, handle; struct efi_device_path *file_path = NULL; struct efi_device_path *msg_path; - efi_status_t ret; + efi_status_t ret, ret2; u16 *load_options; if (!bootefi_device_path || !bootefi_image_path) { @@ -509,12 +509,9 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) * Make sure that device for device_path exist * in load_image(). Otherwise, shell and grub will fail. */ - ret = efi_create_handle(&mem_handle); - if (ret != EFI_SUCCESS) - goto out; - - ret = efi_add_protocol(mem_handle, &efi_guid_device_path, - file_path); + ret = efi_install_multiple_protocol_interfaces(&mem_handle, + &efi_guid_device_path, + file_path, NULL); if (ret != EFI_SUCCESS) goto out; msg_path = file_path; @@ -542,9 +539,11 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) ret = do_bootefi_exec(handle, load_options); out: - efi_delete_handle(mem_handle); + ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle, + &efi_guid_device_path, + file_path, NULL); efi_free_pool(file_path); - return ret; + return (ret != EFI_SUCCESS) ? ret : ret2; } #ifdef CONFIG_CMD_BOOTEFI_SELFTEST