[U-Boot] [PATCH 00/18] efi_loader: refactor protocols management
This patch series starts refactoring the EFI boottime services to enable the move from an array of protocols to a linke list.
The UninstallMultpleProtolInterfaces service is implemented.
Some bug fixes are provided.
This series is to be merged after the series efi_loader: clean up protocol services http://patchwork.ozlabs.org/project/uboot/list/?series=9531 https://lists.denx.de/pipermail/u-boot/2017-October/309913.html
Heinrich Schuchardt (18): efi_loader: fix typo efi_install_multiple_protocol_interfaces efi_loader: debug output efi_install_protocol_interface efi_loader implement UninstallMultipleProtocolInterfaces efi_loader: efi_gop: check calloc return value efi_loader: efi_disk: check return value of calloc efi_loader: efi_net: check return value of calloc efi_loader: efi_dp_match should have const arguments efi_loader: make efi_create_handle non-static efi_loader: argument of efi_search_obj should be const efi_loader: efi_gop: use correct types for parameters efi_selftest: test for graphics output protocol efi_loader: helper functions for protocol management efi_loader: simplify efi_install_protocol_interface efi_loader: simplify efi_search efi_loader: simplify efi_uninstall_protocol_interface efi_loader: simplify efi_locate_protocol efi_loader: refactor efi_setup_loaded_image efi_loader: efi_console: use helper functions
include/efi_api.h | 10 +- include/efi_loader.h | 31 ++-- lib/efi_loader/efi_boottime.c | 324 ++++++++++++++++++++++++------------ lib/efi_loader/efi_console.c | 48 ++++-- lib/efi_loader/efi_device_path.c | 3 +- lib/efi_loader/efi_disk.c | 13 ++ lib/efi_loader/efi_gop.c | 18 +- lib/efi_loader/efi_net.c | 4 + lib/efi_selftest/Makefile | 3 + lib/efi_selftest/efi_selftest_gop.c | 95 +++++++++++ 10 files changed, 409 insertions(+), 140 deletions(-) create mode 100644 lib/efi_selftest/efi_selftest_gop.c
%s/occured/occurred/g
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index bdbed32656..871a8769ea 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1778,7 +1778,7 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( if (r == EFI_SUCCESS) return EFI_EXIT(r);
- /* If an error occured undo all changes. */ + /* If an error occurred undo all changes. */ va_start(argptr, handle); for (; i; --i) { protocol = va_arg(argptr, efi_guid_t*);
efi_install_protocol_interface should provide the created or provided handle in the debug output.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 871a8769ea..f2f9e87635 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -732,6 +732,11 @@ static efi_status_t EFIAPI efi_install_protocol_interface( r = efi_create_handle(handle); if (r != EFI_SUCCESS) goto out; + debug("%sEFI: new handle %p\n", indent_string(nesting_level), + *handle); + } else { + debug("%sEFI: handle %p\n", indent_string(nesting_level), + *handle); } /* Find object. */ list_for_each(lhandle, &efi_obj_list) {
Implement UninstallMultipleProtocolInterfaces. The efi_uninstall_multipled_protocol_interfaces tries to uninstall protocols one by one. If an error occurs all uninstalled protocols are reinstalled.
As the implementation efi_uninstall_protocol_interface is still incomplete the function will fail.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index f2f9e87635..f7e8367091 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1812,7 +1812,45 @@ static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces( void *handle, ...) { EFI_ENTRY("%p", handle); - return EFI_EXIT(EFI_INVALID_PARAMETER); + + 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); + + va_start(argptr, handle); + for (;;) { + protocol = va_arg(argptr, efi_guid_t*); + if (!protocol) + break; + protocol_interface = va_arg(argptr, void*); + r = EFI_CALL(efi_uninstall_protocol_interface( + handle, protocol, + protocol_interface)); + if (r != EFI_SUCCESS) + break; + i++; + } + va_end(argptr); + if (r == EFI_SUCCESS) + return EFI_EXIT(r); + + /* If an error occurred undo all changes. */ + va_start(argptr, handle); + for (; i; --i) { + protocol = va_arg(argptr, efi_guid_t*); + protocol_interface = va_arg(argptr, void*); + EFI_CALL(efi_install_protocol_interface(&handle, protocol, + EFI_NATIVE_INTERFACE, + protocol_interface)); + } + va_end(argptr); + + return EFI_EXIT(r); }
/*
Calloc may return NULL. We have to check the return value.
Fixes: be8d324191f efi_loader: Add GOP support Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_gop.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 411a8c9226..85b5391ed1 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -173,6 +173,10 @@ int efi_gop_register(void) }
gopobj = calloc(1, sizeof(*gopobj)); + if (!gopobj) { + printf("ERROR: Out of memory\n"); + return 1; + }
/* Fill in object data */ gopobj->parent.protocols[0].guid = &efi_gop_guid;
Calloc may return NULL. We should check the return value.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_disk.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index e61dbc8058..c6f0d732c1 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -196,6 +196,15 @@ efi_fs_from_path(struct efi_device_path *fp) return diskobj->volume; }
+/* + * Create a device for a disk + * + * @name not used + * @if_typename interface name for block device + * @desc internal block device + * @dev_index device index for block device + * @offset offset into disk for simple partitions + */ static void efi_disk_add_dev(const char *name, const char *if_typename, struct blk_desc *desc, @@ -210,6 +219,10 @@ static void efi_disk_add_dev(const char *name, return;
diskobj = calloc(1, sizeof(*diskobj)); + if (!diskobj) { + printf("ERROR: Out of memory\n"); + return; + }
/* Fill in object data */ diskobj->dp = efi_dp_from_part(desc, part);
Calloc may return NULL. So we must check the return value.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_net.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 432d9a99a2..a7b101e830 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -292,6 +292,10 @@ int efi_net_register(void)
/* We only expose the "active" eth device, so one is enough */ netobj = calloc(1, sizeof(*netobj)); + if (!netobj) { + printf("ERROR: Out of memory\n"); + return 1; + }
/* Fill in object data */ netobj->parent.protocols[0].guid = &efi_net_guid;
efi_dp_match does not change its arguments. So they should be marked as const.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 3 ++- lib/efi_loader/efi_device_path.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 2bcca3dfd8..5fe17b0c09 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -252,7 +252,8 @@ extern void *efi_bounce_buffer;
struct efi_device_path *efi_dp_next(const struct efi_device_path *dp); -int efi_dp_match(struct efi_device_path *a, struct efi_device_path *b); +int efi_dp_match(const struct efi_device_path *a, + const struct efi_device_path *b); struct efi_object *efi_dp_find_obj(struct efi_device_path *dp, struct efi_device_path **rem); unsigned efi_dp_size(const struct efi_device_path *dp); diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index f6e368e029..024877161b 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -68,7 +68,8 @@ struct efi_device_path *efi_dp_next(const struct efi_device_path *dp) * representing a device with one representing a file on the device, or * a device with a parent device. */ -int efi_dp_match(struct efi_device_path *a, struct efi_device_path *b) +int efi_dp_match(const struct efi_device_path *a, + const struct efi_device_path *b) { while (1) { int ret;
Export function efi_create_handle.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 2 ++ lib/efi_loader/efi_boottime.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 5fe17b0c09..f42dceaf51 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -192,6 +192,8 @@ void efi_restore_gd(void); void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map); /* Call this to set the current device name */ void efi_set_bootdev(const char *dev, const char *devnr, const char *path); +/* Create handle */ +efi_status_t efi_create_handle(void **handle); /* Call this to validate a handle and find the EFI object for it */ struct efi_object *efi_search_obj(void *handle); /* Call this to create an event */ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index f7e8367091..a87e23cd8e 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -321,7 +321,13 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer) return EFI_EXIT(r); }
-static efi_status_t efi_create_handle(void **handle) +/* + * Create handle. + * + * @handle new handle + * @return status code + */ +efi_status_t efi_create_handle(void **handle) { struct efi_object *obj; efi_status_t r;
The argument of efi_search_obj is not changed so it should be marked as const.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 2 +- lib/efi_loader/efi_boottime.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index f42dceaf51..a5aae1b87e 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -195,7 +195,7 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path); /* Create handle */ efi_status_t efi_create_handle(void **handle); /* Call this to validate a handle and find the EFI object for it */ -struct efi_object *efi_search_obj(void *handle); +struct efi_object *efi_search_obj(const void *handle); /* Call this to create an event */ efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl, void (EFIAPI *notify_function) ( diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index a87e23cd8e..c96c2be7b9 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -690,7 +690,7 @@ static efi_status_t EFIAPI efi_check_event(struct efi_event *event) * @handle handle to find * @return EFI object */ -struct efi_object *efi_search_obj(void *handle) +struct efi_object *efi_search_obj(const void *handle) { struct efi_object *efiobj;
Use efi_uintn_t instead of unsigned long.
EFI_GRAPHICS_OUTPUT_BLT_OPERATION is an enum. If we don't define an enum we have to pass it as u32.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_api.h | 10 +++++----- lib/efi_loader/efi_gop.c | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/efi_api.h b/include/efi_api.h index e0991b6eca..584016dc30 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -585,14 +585,14 @@ struct efi_gop_mode struct efi_gop { efi_status_t (EFIAPI *query_mode)(struct efi_gop *this, u32 mode_number, - unsigned long *size_of_info, + efi_uintn_t *size_of_info, struct efi_gop_mode_info **info); efi_status_t (EFIAPI *set_mode)(struct efi_gop *this, u32 mode_number); efi_status_t (EFIAPI *blt)(struct efi_gop *this, void *buffer, - unsigned long operation, unsigned long sx, - unsigned long sy, unsigned long dx, - unsigned long dy, unsigned long width, - unsigned long height, unsigned long delta); + u32 operation, efi_uintn_t sx, + efi_uintn_t sy, efi_uintn_t dx, + efi_uintn_t dy, efi_uintn_t width, + efi_uintn_t height, efi_uintn_t delta); struct efi_gop_mode *mode; };
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 85b5391ed1..7370eeee37 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -32,7 +32,7 @@ struct efi_gop_obj { };
static efi_status_t EFIAPI gop_query_mode(struct efi_gop *this, u32 mode_number, - unsigned long *size_of_info, + efi_uintn_t *size_of_info, struct efi_gop_mode_info **info) { struct efi_gop_obj *gopobj; @@ -56,17 +56,17 @@ static efi_status_t EFIAPI gop_set_mode(struct efi_gop *this, u32 mode_number) return EFI_EXIT(EFI_SUCCESS); }
-static efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer, - unsigned long operation, unsigned long sx, - unsigned long sy, unsigned long dx, - unsigned long dy, unsigned long width, - unsigned long height, unsigned long delta) +efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer, + u32 operation, efi_uintn_t sx, + efi_uintn_t sy, efi_uintn_t dx, + efi_uintn_t dy, efi_uintn_t width, + efi_uintn_t height, efi_uintn_t delta) { struct efi_gop_obj *gopobj = container_of(this, struct efi_gop_obj, ops); int i, j, line_len16, line_len32; void *fb;
- EFI_ENTRY("%p, %p, %lx, %lx, %lx, %lx, %lx, %lx, %lx, %lx", this, + EFI_ENTRY("%p, %p, %u, %zu, %zu, %zu, %zu, %zu, %zu, %zu", this, buffer, operation, sx, sy, dx, dy, width, height, delta);
if (operation != EFI_BLT_BUFFER_TO_VIDEO)
Supply a test for the graphics output protocol.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_selftest/Makefile | 3 ++ lib/efi_selftest/efi_selftest_gop.c | 95 +++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 lib/efi_selftest/efi_selftest_gop.c
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index acd184db4b..1851c17db6 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -15,6 +15,8 @@ CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI) CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI) CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI) CFLAGS_REMOVE_efi_selftest_exitbootservices.o := $(CFLAGS_NON_EFI) +CFLAGS_efi_selftest_gop.o := $(CFLAGS_EFI) +CFLAGS_REMOVE_efi_selftest_gop.o := $(CFLAGS_NON_EFI) CFLAGS_efi_selftest_manageprotocols.o := $(CFLAGS_EFI) CFLAGS_REMOVE_efi_selftest_manageprotocols.o := $(CFLAGS_NON_EFI) CFLAGS_efi_selftest_snp.o := $(CFLAGS_EFI) @@ -33,6 +35,7 @@ efi_selftest.o \ efi_selftest_console.o \ efi_selftest_events.o \ efi_selftest_exitbootservices.o \ +efi_selftest_gop.o \ efi_selftest_manageprotocols.o \ efi_selftest_snp.o \ efi_selftest_textoutput.o \ diff --git a/lib/efi_selftest/efi_selftest_gop.c b/lib/efi_selftest/efi_selftest_gop.c new file mode 100644 index 0000000000..2a0553b115 --- /dev/null +++ b/lib/efi_selftest/efi_selftest_gop.c @@ -0,0 +1,95 @@ +/* + * efi_selftest_gop + * + * Copyright (c) 2017 Heinrich Schuchardt xypron.glpk@gmx.de + * + * SPDX-License-Identifier: GPL-2.0+ + * + * Test the graphical output protocol. + */ + +#include <efi_selftest.h> + +static struct efi_boot_services *boottime; +static efi_guid_t efi_gop_guid = EFI_GOP_GUID; +static struct efi_gop *gop; + +/* + * Setup unit test. + * + * @handle: handle of the loaded image + * @systable: system table + * @return: EFI_ST_SUCCESS for success + */ +static int setup(const efi_handle_t handle, + const struct efi_system_table *systable) +{ + efi_status_t ret; + + boottime = systable->boottime; + + ret = boottime->locate_protocol(&efi_gop_guid, NULL, (void **)&gop); + if (ret != EFI_SUCCESS) { + gop = NULL; + efi_st_printf("Graphical output protocol is not available.\n"); + } + + return EFI_ST_SUCCESS; +} + +/* + * Tear down unit test. + * + * @return: EFI_ST_SUCCESS for success + */ +static int teardown(void) +{ + return EFI_ST_SUCCESS; +} + +/* + * Execute unit test. + * + * @return: EFI_ST_SUCCESS for success + */ +static int execute(void) +{ + efi_status_t ret; + u32 i, max_mode; + efi_uintn_t size; + struct efi_gop_mode_info *info; + + if (!gop) + return EFI_ST_SUCCESS; + + if (!gop->mode) { + efi_st_error("EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE missing\n"); + return EFI_ST_FAILURE; + } + max_mode = gop->mode->max_mode; + if (!max_mode) { + efi_st_error("No graphical mode available\n"); + return EFI_ST_FAILURE; + } + efi_st_printf("Number of available modes: %u\n", max_mode); + + for (i = 0; i < max_mode; ++i) { + ret = gop->query_mode(gop, i, &size, &info); + if (ret != EFI_SUCCESS) { + efi_st_printf("Could not query mode %u\n", i); + return EFI_ST_FAILURE; + } + efi_st_printf("Mode %u: %u x %u\n", + i, info->width, info->height); + } + + return EFI_ST_SUCCESS; +} + +EFI_UNIT_TEST(gop) = { + .name = "graphical output", + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, + .teardown = teardown, +};
This patch provides helper functions to manage protocols. efi_search_protocol - find a protocol on a handle efi_add_protocol - install a protocol on a handle efi_remove_protocol - remove a protocol from a handle efi_remove_all_protocols - remove all protocols from a handle
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 12 +++++ lib/efi_loader/efi_boottime.c | 119 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h index a5aae1b87e..934f64dccb 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -196,6 +196,18 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path); efi_status_t efi_create_handle(void **handle); /* Call this to validate a handle and find the EFI object for it */ struct efi_object *efi_search_obj(const void *handle); +/* Find a protocol on a handle */ +efi_status_t efi_search_protocol(const void *handle, + const efi_guid_t *protocol_guid, + struct efi_handler **handler); +/* Install new protocol on a handle */ +efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol, + void *protocol_interface); +/* Delete protocol from a handle */ +efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol, + void *protocol_interface); +/* Delete all protocols from a handle */ +efi_status_t efi_remove_all_protocols(const void *handle); /* Call this to create an event */ efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl, void (EFIAPI *notify_function) ( diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index c96c2be7b9..d242ce858b 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -703,6 +703,125 @@ struct efi_object *efi_search_obj(const void *handle) }
/* + * Find a protocol on a handle. + * + * @handle handle + * @protocol_guid GUID of the protocol + * @handler reference to the protocol + * @return status code + */ +efi_status_t efi_search_protocol(const void *handle, + const efi_guid_t *protocol_guid, + struct efi_handler **handler) +{ + struct efi_object *efiobj; + size_t i; + struct efi_handler *protocol; + + if (!handle || !protocol_guid) + return EFI_INVALID_PARAMETER; + efiobj = efi_search_obj(handle); + if (!efiobj) + return EFI_INVALID_PARAMETER; + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { + protocol = &efiobj->protocols[i]; + if (!protocol->guid) + continue; + if (!guidcmp(protocol->guid, protocol_guid)) { + if (handler) + *handler = protocol; + return EFI_SUCCESS; + } + } + return EFI_NOT_FOUND; +} + +/* + * Install new protocol on a handle. + * + * @handle handle on which the protocol shall be installed + * @protocol GUID of the protocol to be installed + * @protocol_interface interface of the protocol implementation + * @return status code + */ +efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol, + void *protocol_interface) +{ + struct efi_object *efiobj; + struct efi_handler *handler; + efi_status_t ret; + size_t i; + + efiobj = efi_search_obj(handle); + if (!efiobj) + return EFI_INVALID_PARAMETER; + ret = efi_search_protocol(handle, protocol, NULL); + if (ret != EFI_NOT_FOUND) + return EFI_INVALID_PARAMETER; + handler = calloc(1, sizeof(struct efi_handler)); + if (!handler) + return EFI_OUT_OF_RESOURCES; + /* Install protocol in first empty slot. */ + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { + handler = &efiobj->protocols[i]; + if (handler->guid) + continue; + handler->guid = protocol; + handler->protocol_interface = protocol_interface; + return EFI_SUCCESS; + } + return EFI_OUT_OF_RESOURCES; +} + +/* + * Delete protocol from a handle. + * + * @handle handle from which the protocol shall be deleted + * @protocol GUID of the protocol to be deleted + * @protocol_interface interface of the protocol implementation + * @return status code + */ +efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol, + void *protocol_interface) +{ + struct efi_handler *handler; + efi_status_t ret; + + ret = efi_search_protocol(handle, protocol, &handler); + if (ret != EFI_SUCCESS) + return ret; + if (handler->protocol_interface != protocol_interface) + return EFI_NOT_FOUND; + handler->guid = NULL; + handler->protocol_interface = NULL; + return EFI_SUCCESS; +} + +/* + * Delete all protocols from a handle. + * + * @handle handle from which the protocols shall be deleted + * @return status code + */ +efi_status_t efi_remove_all_protocols(const void *handle) +{ + struct efi_object *efiobj; + struct efi_handler *handler; + size_t i; + + efiobj = efi_search_obj(handle); + if (!efiobj) + return EFI_INVALID_PARAMETER; + + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { + handler = &efiobj->protocols[i]; + handler->guid = NULL; + handler->protocol_interface = NULL; + } + return EFI_SUCCESS; +} + +/* * Install protocol interface. * * This function implements the InstallProtocolInterface service.
Use helper functio efi_add_protocol.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 38 ++------------------------------------ 1 file changed, 2 insertions(+), 36 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index d242ce858b..dd9ac40fe7 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -839,8 +839,6 @@ static efi_status_t EFIAPI efi_install_protocol_interface( void **handle, const efi_guid_t *protocol, int protocol_interface_type, void *protocol_interface) { - struct list_head *lhandle; - int i; efi_status_t r;
EFI_ENTRY("%p, %pUl, %d, %p", handle, protocol, protocol_interface_type, @@ -863,40 +861,8 @@ static efi_status_t EFIAPI efi_install_protocol_interface( debug("%sEFI: handle %p\n", indent_string(nesting_level), *handle); } - /* Find object. */ - list_for_each(lhandle, &efi_obj_list) { - struct efi_object *efiobj; - efiobj = list_entry(lhandle, struct efi_object, link); - - if (efiobj->handle != *handle) - continue; - /* Check if protocol is already installed on the handle. */ - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - struct efi_handler *handler = &efiobj->protocols[i]; - - if (!handler->guid) - continue; - if (!guidcmp(handler->guid, protocol)) { - r = EFI_INVALID_PARAMETER; - goto out; - } - } - /* Install protocol in first empty slot. */ - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - struct efi_handler *handler = &efiobj->protocols[i]; - - if (handler->guid) - continue; - - handler->guid = protocol; - handler->protocol_interface = protocol_interface; - r = EFI_SUCCESS; - goto out; - } - r = EFI_OUT_OF_RESOURCES; - goto out; - } - r = EFI_INVALID_PARAMETER; + /* Add new protocol */ + r = efi_add_protocol(*handle, protocol, protocol_interface); out: return EFI_EXIT(r); }
Use helper function efi_search_protocol in efi_search. Add missing comments. Put default handling into default branch of switch statement.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index dd9ac40fe7..84b9891272 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -983,24 +983,21 @@ static int efi_search(enum efi_locate_search_type search_type, const efi_guid_t *protocol, void *search_key, struct efi_object *efiobj) { - int i; + efi_status_t ret;
switch (search_type) { case ALL_HANDLES: return 0; case BY_REGISTER_NOTIFY: - /* RegisterProtocolNotify is not implemented yet */ + /* TODO: RegisterProtocolNotify is not implemented yet */ return -1; case BY_PROTOCOL: - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - const efi_guid_t *guid = efiobj->protocols[i].guid; - if (guid && !guidcmp(guid, protocol)) - return 0; - } + ret = efi_search_protocol(efiobj->handle, protocol, NULL); + return (ret != EFI_SUCCESS); + default: + /* Invalid search type */ return -1; } - - return -1; }
/*
Use function efi_search_obj, efi_search_protocol and efi_remove_protocol to simplify the coding.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 38 +++++++++++--------------------------- 1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 84b9891272..e4bcd4421f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -906,9 +906,8 @@ static efi_status_t EFIAPI efi_uninstall_protocol_interface( void *handle, const efi_guid_t *protocol, void *protocol_interface) { - struct list_head *lhandle; - int i; - efi_status_t r = EFI_NOT_FOUND; + struct efi_handler *handler; + efi_status_t r;
EFI_ENTRY("%p, %pUl, %p", handle, protocol, protocol_interface);
@@ -917,31 +916,16 @@ static efi_status_t EFIAPI efi_uninstall_protocol_interface( goto out; }
- list_for_each(lhandle, &efi_obj_list) { - struct efi_object *efiobj; - efiobj = list_entry(lhandle, struct efi_object, link); - - if (efiobj->handle != handle) - continue; - - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - struct efi_handler *handler = &efiobj->protocols[i]; - const efi_guid_t *hprotocol = handler->guid; - - if (!hprotocol) - continue; - if (!guidcmp(hprotocol, protocol)) { - if (handler->protocol_interface) { - r = EFI_ACCESS_DENIED; - } else { - handler->guid = 0; - r = EFI_SUCCESS; - } - goto out; - } - } + /* Find the protocol on the handle */ + r = efi_search_protocol(handle, protocol, &handler); + if (r != EFI_SUCCESS) + goto out; + if (handler->protocol_interface) { + /* TODO disconnect controllers */ + r = EFI_ACCESS_DENIED; + } else { + r = efi_remove_protocol(handle, protocol, protocol_interface); } - out: return EFI_EXIT(r); }
Use helper function efi_search_protocol.
Do not print protocol guid twice in debug mode.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index e4bcd4421f..a526280e4c 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1781,29 +1781,23 @@ static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol, void **protocol_interface) { struct list_head *lhandle; - int i; + efi_status_t ret;
EFI_ENTRY("%pUl, %p, %p", protocol, registration, protocol_interface);
if (!protocol || !protocol_interface) return EFI_EXIT(EFI_INVALID_PARAMETER);
- EFI_PRINT_GUID("protocol", protocol); - list_for_each(lhandle, &efi_obj_list) { struct efi_object *efiobj; + struct efi_handler *handler;
efiobj = list_entry(lhandle, struct efi_object, link); - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - struct efi_handler *handler = &efiobj->protocols[i];
- if (!handler->guid) - continue; - if (!guidcmp(handler->guid, protocol)) { - *protocol_interface = - handler->protocol_interface; - return EFI_EXIT(EFI_SUCCESS); - } + ret = efi_search_protocol(efiobj->handle, protocol, &handler); + if (ret == EFI_SUCCESS) { + *protocol_interface = handler->protocol_interface; + return EFI_EXIT(EFI_SUCCESS); } } *protocol_interface = NULL;
Use helper functions to add protocols.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index a526280e4c..3f23e6d638 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1193,34 +1193,45 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob struct efi_device_path *device_path, struct efi_device_path *file_path) { + efi_status_t ret; + obj->handle = info;
+ info->file_path = file_path; + if (device_path) + info->device_handle = efi_dp_find_obj(device_path, NULL); + + list_add_tail(&obj->link, &efi_obj_list); /* * When asking for the device path interface, return * bootefi_device_path */ - obj->protocols[0].guid = &efi_guid_device_path; - obj->protocols[0].protocol_interface = device_path; + ret = efi_add_protocol(obj->handle, &efi_guid_device_path, device_path); + if (ret != EFI_SUCCESS) + goto failure;
/* * When asking for the loaded_image interface, just * return handle which points to loaded_image_info */ - obj->protocols[1].guid = &efi_guid_loaded_image; - obj->protocols[1].protocol_interface = info; - - obj->protocols[2].guid = &efi_guid_console_control; - obj->protocols[2].protocol_interface = (void *)&efi_console_control; + ret = efi_add_protocol(obj->handle, &efi_guid_loaded_image, info); + if (ret != EFI_SUCCESS) + goto failure;
- obj->protocols[3].guid = &efi_guid_device_path_to_text_protocol; - obj->protocols[3].protocol_interface = - (void *)&efi_device_path_to_text; + ret = efi_add_protocol(obj->handle, &efi_guid_console_control, + (void *)&efi_console_control); + if (ret != EFI_SUCCESS) + goto failure;
- info->file_path = file_path; - if (device_path) - info->device_handle = efi_dp_find_obj(device_path, NULL); + ret = efi_add_protocol(obj->handle, + &efi_guid_device_path_to_text_protocol, + (void *)&efi_device_path_to_text); + if (ret != EFI_SUCCESS) + goto failure;
- list_add_tail(&obj->link, &efi_obj_list); + return; +failure: + printf("ERROR: Failure to install protocols for loaded image\n"); }
/*
On 10/26/2017 07:25 PM, Heinrich Schuchardt wrote:
Use helper functions to add protocols.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_boottime.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index a526280e4c..3f23e6d638 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1193,34 +1193,45 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob struct efi_device_path *device_path, struct efi_device_path *file_path) {
efi_status_t ret;
obj->handle = info;
info->file_path = file_path;
if (device_path)
info->device_handle = efi_dp_find_obj(device_path, NULL);list_add_tail(&obj->link, &efi_obj_list); /*
- When asking for the device path interface, return
- bootefi_device_path
*/
- obj->protocols[0].guid = &efi_guid_device_path;
- obj->protocols[0].protocol_interface = device_path;
- ret = efi_add_protocol(obj->handle, &efi_guid_device_path, device_path);
Can we always assume that there are no other protocols installed already? The current code simply overrides them if there are any.
Alex
if (ret != EFI_SUCCESS)
goto failure;/*
- When asking for the loaded_image interface, just
- return handle which points to loaded_image_info
*/
- obj->protocols[1].guid = &efi_guid_loaded_image;
- obj->protocols[1].protocol_interface = info;
- obj->protocols[2].guid = &efi_guid_console_control;
- obj->protocols[2].protocol_interface = (void *)&efi_console_control;
- ret = efi_add_protocol(obj->handle, &efi_guid_loaded_image, info);
- if (ret != EFI_SUCCESS)
goto failure;
- obj->protocols[3].guid = &efi_guid_device_path_to_text_protocol;
- obj->protocols[3].protocol_interface =
(void *)&efi_device_path_to_text;
- ret = efi_add_protocol(obj->handle, &efi_guid_console_control,
(void *)&efi_console_control);- if (ret != EFI_SUCCESS)
goto failure;
- info->file_path = file_path;
- if (device_path)
info->device_handle = efi_dp_find_obj(device_path, NULL);
- ret = efi_add_protocol(obj->handle,
&efi_guid_device_path_to_text_protocol,(void *)&efi_device_path_to_text);- if (ret != EFI_SUCCESS)
goto failure;
- list_add_tail(&obj->link, &efi_obj_list);
- return;
+failure:
printf("ERROR: Failure to install protocols for loaded image\n"); }
/*
On 11/08/2017 04:18 PM, Alexander Graf wrote:
On 10/26/2017 07:25 PM, Heinrich Schuchardt wrote:
Use helper functions to add protocols.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_boottime.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index a526280e4c..3f23e6d638 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1193,34 +1193,45 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob struct efi_device_path *device_path, struct efi_device_path *file_path) { + efi_status_t ret;
obj->handle = info; + info->file_path = file_path; + if (device_path) + info->device_handle = efi_dp_find_obj(device_path, NULL);
+ list_add_tail(&obj->link, &efi_obj_list); /* * When asking for the device path interface, return * bootefi_device_path */ - obj->protocols[0].guid = &efi_guid_device_path; - obj->protocols[0].protocol_interface = device_path; + ret = efi_add_protocol(obj->handle, &efi_guid_device_path, device_path);
Can we always assume that there are no other protocols installed already? The current code simply overrides them if there are any.
efi_setup_loaded_image is called from efi_load_image with a newly allocated obj, from do_bootefi_exit and do_bootefi with an initial local obj. So there cannot be any protocol existing.
We unlink the image from the object list after use so none of the protocols will be available when calling the next EFI application.
Best regards
Heinrich
Alex
+ if (ret != EFI_SUCCESS) + goto failure; /* * When asking for the loaded_image interface, just * return handle which points to loaded_image_info */ - obj->protocols[1].guid = &efi_guid_loaded_image; - obj->protocols[1].protocol_interface = info;
- obj->protocols[2].guid = &efi_guid_console_control; - obj->protocols[2].protocol_interface = (void *)&efi_console_control; + ret = efi_add_protocol(obj->handle, &efi_guid_loaded_image, info); + if (ret != EFI_SUCCESS) + goto failure; - obj->protocols[3].guid = &efi_guid_device_path_to_text_protocol; - obj->protocols[3].protocol_interface = - (void *)&efi_device_path_to_text; + ret = efi_add_protocol(obj->handle, &efi_guid_console_control, + (void *)&efi_console_control); + if (ret != EFI_SUCCESS) + goto failure; - info->file_path = file_path; - if (device_path) - info->device_handle = efi_dp_find_obj(device_path, NULL); + ret = efi_add_protocol(obj->handle, + &efi_guid_device_path_to_text_protocol, + (void *)&efi_device_path_to_text); + if (ret != EFI_SUCCESS) + goto failure; - list_add_tail(&obj->link, &efi_obj_list); + return; +failure: + printf("ERROR: Failure to install protocols for loaded image\n"); } /*
Use helper functions efi_created_handle and efi_add_protocol for creating the console handles and instaling the respective protocols.
This change is needed if we want to move from an array of protocols to a linked list of protocols.
Eliminate EFI_PROTOCOL_OBJECT which is not used anymore.
Currently we have not defined protocol interfaces to be const. So efi_con_out and efi_console_control cannot be defined as const.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 12 ++--------- lib/efi_loader/efi_console.c | 48 ++++++++++++++++++++++++++++++++------------ 2 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 934f64dccb..e1f0af3496 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -78,9 +78,9 @@ const char *__efi_nesting_dec(void); extern struct efi_runtime_services efi_runtime_services; extern struct efi_system_table systab;
-extern const struct efi_simple_text_output_protocol efi_con_out; +extern struct efi_simple_text_output_protocol efi_con_out; extern struct efi_simple_input_interface efi_con_in; -extern const struct efi_console_control_protocol efi_console_control; +extern struct efi_console_control_protocol efi_console_control; extern const struct efi_device_path_to_text_protocol efi_device_path_to_text;
uint16_t *efi_dp_str(struct efi_device_path *dp); @@ -121,14 +121,6 @@ struct efi_object { void *handle; };
-#define EFI_PROTOCOL_OBJECT(_guid, _protocol) (struct efi_object){ \ - .protocols = {{ \ - .guid = &(_guid), \ - .protocol_interface = (void *)(_protocol), \ - }}, \ - .handle = (void *)(_protocol), \ -} - /** * struct efi_event * diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 01732aafea..98497db612 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -46,6 +46,10 @@ static struct cout_mode efi_cout_modes[] = { };
const efi_guid_t efi_guid_console_control = CONSOLE_CONTROL_GUID; +const efi_guid_t efi_guid_text_output_protocol = + EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID; +const efi_guid_t efi_guid_text_input_protocol = + EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID;
#define cESC '\x1b' #define ESC "\x1b" @@ -81,7 +85,7 @@ static efi_status_t EFIAPI efi_cin_lock_std_in( return EFI_EXIT(EFI_UNSUPPORTED); }
-const struct efi_console_control_protocol efi_console_control = { +struct efi_console_control_protocol efi_console_control = { .get_mode = efi_cin_get_mode, .set_mode = efi_cin_set_mode, .lock_std_in = efi_cin_lock_std_in, @@ -374,7 +378,7 @@ static efi_status_t EFIAPI efi_cout_enable_cursor( return EFI_EXIT(EFI_SUCCESS); }
-const struct efi_simple_text_output_protocol efi_con_out = { +struct efi_simple_text_output_protocol efi_con_out = { .reset = efi_cout_reset, .output_string = efi_cout_output_string, .test_string = efi_cout_test_string, @@ -490,23 +494,38 @@ static void EFIAPI efi_console_timer_notify(struct efi_event *event, }
-static struct efi_object efi_console_control_obj = - EFI_PROTOCOL_OBJECT(efi_guid_console_control, &efi_console_control); -static struct efi_object efi_console_output_obj = - EFI_PROTOCOL_OBJECT(EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID, &efi_con_out); -static struct efi_object efi_console_input_obj = - EFI_PROTOCOL_OBJECT(EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID, &efi_con_in); - /* This gets called from do_bootefi_exec(). */ int efi_console_register(void) { efi_status_t r; + struct efi_object *efi_console_control_obj; + struct efi_object *efi_console_output_obj; + struct efi_object *efi_console_input_obj;
- /* Hook up to the device list */ - list_add_tail(&efi_console_control_obj.link, &efi_obj_list); - list_add_tail(&efi_console_output_obj.link, &efi_obj_list); - list_add_tail(&efi_console_input_obj.link, &efi_obj_list); + /* Create handles */ + r = efi_create_handle((void **)&efi_console_control_obj); + if (r != EFI_SUCCESS) + goto out_of_memory; + r = efi_add_protocol(efi_console_control_obj->handle, + &efi_guid_console_control, &efi_console_control); + if (r != EFI_SUCCESS) + goto out_of_memory; + r = efi_create_handle((void **)&efi_console_output_obj); + if (r != EFI_SUCCESS) + goto out_of_memory; + r = efi_add_protocol(efi_console_output_obj->handle, + &efi_guid_text_output_protocol, &efi_con_out); + if (r != EFI_SUCCESS) + goto out_of_memory; + r = efi_create_handle((void **)&efi_console_input_obj); + if (r != EFI_SUCCESS) + goto out_of_memory; + r = efi_add_protocol(efi_console_input_obj->handle, + &efi_guid_text_input_protocol, &efi_con_in); + if (r != EFI_SUCCESS) + goto out_of_memory;
+ /* Create console events */ r = efi_create_event(EVT_NOTIFY_WAIT, TPL_CALLBACK, efi_key_notify, NULL, &efi_con_in.wait_for_key); if (r != EFI_SUCCESS) { @@ -525,4 +544,7 @@ int efi_console_register(void) if (r != EFI_SUCCESS) printf("ERROR: Failed to set console timer\n"); return r; +out_of_memory: + printf("ERROR: Out of meemory\n"); + return r; }
participants (2)
-
Alexander Graf -
Heinrich Schuchardt