[U-Boot] [PATCH 0/2] efi_loader: eliminate handle member

Up to now we have treated handles as separate objects to EFI objects. This confusion has lead to hidden bugs like those corrected by the first patch. By changing the efi_handle_t typedef we can avoid this problem in future.
As handles are pointers to EFI objects there is no need for a handle member in struct efi_object. This allows to simplify our coding.
Heinrich Schuchardt (2): efi_loader: typedef struct efi_object *efi_handle_t efi_loader: eliminate handle member
cmd/bootefi.c | 4 +- include/efi.h | 2 +- include/efi_api.h | 8 +-- include/efi_loader.h | 26 +++++--- lib/efi/efi.c | 2 +- lib/efi_loader/efi_boottime.c | 77 +++++++++++----------- lib/efi_loader/efi_console.c | 20 +++--- lib/efi_loader/efi_device_path.c | 2 +- lib/efi_loader/efi_disk.c | 8 +-- lib/efi_loader/efi_gop.c | 2 +- lib/efi_loader/efi_net.c | 6 +- lib/efi_selftest/efi_selftest_devicepath.c | 2 +- 12 files changed, 83 insertions(+), 76 deletions(-)

All our handles point to a struct efi_object. So let's define the efi_handle_t accordingly. This helps us to discover coding errors much more easily. This becomes evident by the corrections to the usage of handles in this patch.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- cmd/bootefi.c | 4 ++-- include/efi.h | 2 +- include/efi_api.h | 8 ++++---- lib/efi/efi.c | 2 +- lib/efi_loader/efi_boottime.c | 18 +++++++++--------- lib/efi_selftest/efi_selftest_devicepath.c | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 9c51a2cbd1..05eb168e4a 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -447,7 +447,7 @@ static efi_status_t do_bootefi_exec(void *efi, } #endif
- ret = efi_do_enter(image_handle, &systab, entry); + ret = efi_do_enter(&image_handle->parent, &systab, entry);
exit: /* image has returned, loaded-image obj goes *poof*: */ @@ -561,7 +561,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) /* Transfer environment variable efi_selftest as load options */ set_load_options(loaded_image_info, "efi_selftest"); /* Execute the test */ - r = efi_selftest(image_handle, &systab); + r = efi_selftest(&image_handle->parent, &systab); efi_restore_gd(); free(loaded_image_info->load_options); efi_delete_handle(&image_handle->parent); diff --git a/include/efi.h b/include/efi.h index b1deb609b4..b5e2c64f38 100644 --- a/include/efi.h +++ b/include/efi.h @@ -96,7 +96,7 @@ typedef struct { typedef unsigned long efi_status_t; typedef u64 efi_physical_addr_t; typedef u64 efi_virtual_addr_t; -typedef void *efi_handle_t; +typedef struct efi_object *efi_handle_t;
#define EFI_GUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, \ diff --git a/include/efi_api.h b/include/efi_api.h index c42df6900a..e6566bb358 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -86,10 +86,10 @@ struct efi_boot_services { efi_status_t (EFIAPI *check_event)(struct efi_event *event); #define EFI_NATIVE_INTERFACE 0x00000000 efi_status_t (EFIAPI *install_protocol_interface)( - void **handle, const efi_guid_t *protocol, + efi_handle_t *handle, const efi_guid_t *protocol, int protocol_interface_type, void *protocol_interface); efi_status_t (EFIAPI *reinstall_protocol_interface)( - void *handle, const efi_guid_t *protocol, + efi_handle_t handle, const efi_guid_t *protocol, void *old_interface, void *new_interface); efi_status_t (EFIAPI *uninstall_protocol_interface)( efi_handle_t handle, const efi_guid_t *protocol, @@ -165,9 +165,9 @@ struct efi_boot_services { efi_status_t (EFIAPI *locate_protocol)(const efi_guid_t *protocol, void *registration, void **protocol_interface); efi_status_t (EFIAPI *install_multiple_protocol_interfaces)( - void **handle, ...); + efi_handle_t *handle, ...); efi_status_t (EFIAPI *uninstall_multiple_protocol_interfaces)( - void *handle, ...); + efi_handle_t handle, ...); efi_status_t (EFIAPI *calculate_crc32)(const void *data, efi_uintn_t data_size, u32 *crc32); diff --git a/lib/efi/efi.c b/lib/efi/efi.c index c6639f96cc..2c6a50824f 100644 --- a/lib/efi/efi.c +++ b/lib/efi/efi.c @@ -69,7 +69,7 @@ int efi_init(struct efi_priv *priv, const char *banner, efi_handle_t image, efi_putc(priv, ' ');
ret = boot->open_protocol(priv->parent_image, &loaded_image_guid, - (void **)&loaded_image, &priv->parent_image, + (void **)&loaded_image, priv->parent_image, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); if (ret) { efi_puts(priv, "Failed to get loaded image protocol\n"); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 3bb7b70e08..20974f98f6 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1019,7 +1019,7 @@ efi_status_t efi_add_protocol(const efi_handle_t handle, * Return: status code */ static efi_status_t EFIAPI efi_install_protocol_interface( - void **handle, const efi_guid_t *protocol, + efi_handle_t *handle, const efi_guid_t *protocol, int protocol_interface_type, void *protocol_interface) { efi_status_t r; @@ -2279,8 +2279,8 @@ out: * * Return: status code */ -static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( - void **handle, ...) +static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces + (efi_handle_t *handle, ...) { EFI_ENTRY("%p", handle);
@@ -2316,7 +2316,7 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( 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, + EFI_CALL(efi_uninstall_protocol_interface(*handle, protocol, protocol_interface)); } efi_va_end(argptr); @@ -2339,7 +2339,7 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( * Return: status code */ static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces( - void *handle, ...) + efi_handle_t handle, ...) { EFI_ENTRY("%p", handle);
@@ -2554,10 +2554,10 @@ out: * * Return: status code */ -static efi_status_t EFIAPI efi_open_protocol( - void *handle, const efi_guid_t *protocol, - void **protocol_interface, void *agent_handle, - void *controller_handle, uint32_t attributes) +static efi_status_t EFIAPI efi_open_protocol + (efi_handle_t handle, const efi_guid_t *protocol, + void **protocol_interface, efi_handle_t agent_handle, + efi_handle_t controller_handle, uint32_t attributes) { struct efi_handler *handler; efi_status_t r = EFI_INVALID_PARAMETER; diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c index adcf531e90..105ce2c92b 100644 --- a/lib/efi_selftest/efi_selftest_devicepath.c +++ b/lib/efi_selftest/efi_selftest_devicepath.c @@ -257,7 +257,7 @@ static int teardown(void) static int execute(void) { struct efi_device_path *remaining_dp; - void *handle; + efi_handle_t handle; /* * This device path node ends with the letter 't' of 'u-boot'. * The following '.bin' does not belong to the node but is

A pointer to a struct efi_object is a handle. We do not need any handle member in this structure. Let's eliminate it.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 26 +++++++++----- lib/efi_loader/efi_boottime.c | 59 ++++++++++++++++---------------- lib/efi_loader/efi_console.c | 20 +++++------ lib/efi_loader/efi_device_path.c | 2 +- lib/efi_loader/efi_disk.c | 8 ++--- lib/efi_loader/efi_gop.c | 2 +- lib/efi_loader/efi_net.c | 6 ++-- 7 files changed, 65 insertions(+), 58 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 74df070316..6846bb03f3 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -173,20 +173,28 @@ struct efi_handler { struct list_head open_infos; };
-/* - * UEFI has a poor man's OO model where one "object" can be polymorphic and have - * multiple different protocols (classes) attached to it. +/** + * struct efi_object - dereferenced EFI handle + * + * @link: pointers to put the handle into a linked list + * @protocols: linked list with the protocol interfaces installed on this + * handle + * + * UEFI offers a flexible and expandable object model. The objects in the UEFI + * API are devices, drivers, and loaded images. struct efi_object is our storage + * structure for these objects. + * + * When including this structure into a larger structure always put it first so + * that when deleting a handle the whole encompassing structure can be freed. * - * This struct is the parent struct for all of our actual implementation objects - * that can include it to make themselves an EFI object + * A pointer to this structure is referred to as a handle. Typedef efi_handle_t + * has been created for such pointers. */ struct efi_object { /* Every UEFI object is part of a global object list */ struct list_head link; /* The list of protocols */ struct list_head protocols; - /* The object spawner can either use this for data or as identifier */ - void *handle; };
/** @@ -296,11 +304,11 @@ 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); /* Add a new object to the object list. */ -void efi_add_handle(struct efi_object *obj); +void efi_add_handle(efi_handle_t obj); /* Create handle */ efi_status_t efi_create_handle(efi_handle_t *handle); /* Delete handle */ -void efi_delete_handle(struct efi_object *obj); +void efi_delete_handle(efi_handle_t obj); /* Call this to validate a handle and find the EFI object for it */ struct efi_object *efi_search_obj(const efi_handle_t handle); /* Find a protocol on a handle */ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 20974f98f6..d1c4b26297 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -416,13 +416,12 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer) * * The protocols list is initialized. The object handle is set. */ -void efi_add_handle(struct efi_object *obj) +void efi_add_handle(efi_handle_t handle) { - if (!obj) + if (!handle) return; - INIT_LIST_HEAD(&obj->protocols); - obj->handle = obj; - list_add_tail(&obj->link, &efi_obj_list); + INIT_LIST_HEAD(&handle->protocols); + list_add_tail(&handle->link, &efi_obj_list); }
/** @@ -440,7 +439,7 @@ efi_status_t efi_create_handle(efi_handle_t *handle) return EFI_OUT_OF_RESOURCES;
efi_add_handle(obj); - *handle = obj->handle; + *handle = obj;
return EFI_SUCCESS; } @@ -536,13 +535,13 @@ efi_status_t efi_remove_all_protocols(const efi_handle_t handle) * * @obj: handle to delete */ -void efi_delete_handle(struct efi_object *obj) +void efi_delete_handle(efi_handle_t handle) { - if (!obj) + if (!handle) return; - efi_remove_all_protocols(obj->handle); - list_del(&obj->link); - free(obj); + efi_remove_all_protocols(handle); + list_del(&handle->link); + free(handle); }
/** @@ -927,7 +926,7 @@ struct efi_object *efi_search_obj(const efi_handle_t handle) struct efi_object *efiobj;
list_for_each_entry(efiobj, &efi_obj_list, link) { - if (efiobj->handle == handle) + if (efiobj == handle) return efiobj; }
@@ -1052,7 +1051,7 @@ out:
/** * efi_get_drivers() - get all drivers associated to a controller - * @efiobj: handle of the controller + * @handle: handle of the controller * @protocol: protocol GUID (optional) * @number_of_drivers: number of child controllers * @driver_handle_buffer: handles of the the drivers @@ -1061,7 +1060,7 @@ out: * * Return: status code */ -static efi_status_t efi_get_drivers(struct efi_object *efiobj, +static efi_status_t efi_get_drivers(efi_handle_t handle, const efi_guid_t *protocol, efi_uintn_t *number_of_drivers, efi_handle_t **driver_handle_buffer) @@ -1072,7 +1071,7 @@ static efi_status_t efi_get_drivers(struct efi_object *efiobj, bool duplicate;
/* Count all driver associations */ - list_for_each_entry(handler, &efiobj->protocols, link) { + list_for_each_entry(handler, &handle->protocols, link) { if (protocol && guidcmp(handler->guid, protocol)) continue; list_for_each_entry(item, &handler->open_infos, link) { @@ -1090,7 +1089,7 @@ static efi_status_t efi_get_drivers(struct efi_object *efiobj, if (!*driver_handle_buffer) return EFI_OUT_OF_RESOURCES; /* Collect unique driver handles */ - list_for_each_entry(handler, &efiobj->protocols, link) { + list_for_each_entry(handler, &handle->protocols, link) { if (protocol && guidcmp(handler->guid, protocol)) continue; list_for_each_entry(item, &handler->open_infos, link) { @@ -1117,7 +1116,7 @@ static efi_status_t efi_get_drivers(struct efi_object *efiobj,
/** * efi_disconnect_all_drivers() - disconnect all drivers from a controller - * @efiobj: handle of the controller + * @handle: handle of the controller * @protocol: protocol GUID (optional) * @child_handle: handle of the child to destroy * @@ -1128,16 +1127,16 @@ static efi_status_t efi_get_drivers(struct efi_object *efiobj, * * Return: status code */ -static efi_status_t efi_disconnect_all_drivers( - struct efi_object *efiobj, - const efi_guid_t *protocol, - efi_handle_t child_handle) +static efi_status_t efi_disconnect_all_drivers + (efi_handle_t handle, + const efi_guid_t *protocol, + efi_handle_t child_handle) { efi_uintn_t number_of_drivers; efi_handle_t *driver_handle_buffer; efi_status_t r, ret;
- ret = efi_get_drivers(efiobj, protocol, &number_of_drivers, + ret = efi_get_drivers(handle, protocol, &number_of_drivers, &driver_handle_buffer); if (ret != EFI_SUCCESS) return ret; @@ -1145,7 +1144,7 @@ static efi_status_t efi_disconnect_all_drivers( ret = EFI_NOT_FOUND; while (number_of_drivers) { r = EFI_CALL(efi_disconnect_controller( - efiobj->handle, + handle, driver_handle_buffer[--number_of_drivers], child_handle)); if (r == EFI_SUCCESS) @@ -1240,7 +1239,7 @@ static efi_status_t EFIAPI efi_register_protocol_notify( * @search_type: selection criterion * @protocol: GUID of the protocol * @search_key: registration key - * @efiobj: handle + * @handle: handle * * See the documentation of the LocateHandle service in the UEFI specification. * @@ -1248,7 +1247,7 @@ static efi_status_t EFIAPI efi_register_protocol_notify( */ static int efi_search(enum efi_locate_search_type search_type, const efi_guid_t *protocol, void *search_key, - struct efi_object *efiobj) + efi_handle_t handle) { efi_status_t ret;
@@ -1259,7 +1258,7 @@ static int efi_search(enum efi_locate_search_type search_type, /* TODO: RegisterProtocolNotify is not implemented yet */ return -1; case BY_PROTOCOL: - ret = efi_search_protocol(efiobj->handle, protocol, NULL); + ret = efi_search_protocol(handle, protocol, NULL); return (ret != EFI_SUCCESS); default: /* Invalid search type */ @@ -1331,7 +1330,7 @@ static efi_status_t efi_locate_handle( /* Then fill the array */ list_for_each_entry(efiobj, &efi_obj_list, link) { if (!efi_search(search_type, protocol, search_key, efiobj)) - *buffer++ = efiobj->handle; + *buffer++ = efiobj; }
return EFI_SUCCESS; @@ -1506,7 +1505,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, * When asking for the device path interface, return * bootefi_device_path */ - ret = efi_add_protocol(obj->parent.handle, + ret = efi_add_protocol(&obj->parent, &efi_guid_device_path, device_path); if (ret != EFI_SUCCESS) goto failure; @@ -1516,7 +1515,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, * When asking for the loaded_image interface, just * return handle which points to loaded_image_info */ - ret = efi_add_protocol(obj->parent.handle, + ret = efi_add_protocol(&obj->parent, &efi_guid_loaded_image, info); if (ret != EFI_SUCCESS) goto failure; @@ -2176,7 +2175,7 @@ static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol,
efiobj = list_entry(lhandle, struct efi_object, link);
- ret = efi_search_protocol(efiobj->handle, protocol, &handler); + ret = efi_search_protocol(efiobj, protocol, &handler); if (ret == EFI_SUCCESS) { *protocol_interface = handler->protocol_interface; return EFI_EXIT(EFI_SUCCESS); diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 7ecdbb1666..eb086bc219 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -1049,34 +1049,34 @@ static void EFIAPI efi_key_notify(struct efi_event *event, void *context) int efi_console_register(void) { efi_status_t r; - struct efi_object *efi_console_output_obj; - struct efi_object *efi_console_input_obj; + efi_handle_t console_output_handle; + efi_handle_t console_input_handle;
/* Set up mode information */ query_console_size();
/* Create handles */ - r = efi_create_handle((efi_handle_t *)&efi_console_output_obj); + r = efi_create_handle(&console_output_handle); if (r != EFI_SUCCESS) goto out_of_memory;
- r = efi_add_protocol(efi_console_output_obj->handle, + r = efi_add_protocol(console_output_handle, &efi_guid_text_output_protocol, &efi_con_out); if (r != EFI_SUCCESS) goto out_of_memory; - systab.con_out_handle = efi_console_output_obj->handle; - systab.stderr_handle = efi_console_output_obj->handle; + systab.con_out_handle = console_output_handle; + systab.stderr_handle = console_output_handle;
- r = efi_create_handle((efi_handle_t *)&efi_console_input_obj); + r = efi_create_handle(&console_input_handle); if (r != EFI_SUCCESS) goto out_of_memory;
- r = efi_add_protocol(efi_console_input_obj->handle, + r = efi_add_protocol(console_input_handle, &efi_guid_text_input_protocol, &efi_con_in); if (r != EFI_SUCCESS) goto out_of_memory; - systab.con_in_handle = efi_console_input_obj->handle; - r = efi_add_protocol(efi_console_input_obj->handle, + systab.con_in_handle = console_input_handle; + r = efi_add_protocol(console_input_handle, &efi_guid_text_input_ex_protocol, &efi_con_in_ex); if (r != EFI_SUCCESS) goto out_of_memory; diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 46a24f7882..2b5d067104 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -150,7 +150,7 @@ static struct efi_object *find_obj(struct efi_device_path *dp, bool short_path, struct efi_device_path *obj_dp; efi_status_t ret;
- ret = efi_search_protocol(efiobj->handle, + ret = efi_search_protocol(efiobj, &efi_guid_device_path, &handler); if (ret != EFI_SUCCESS) continue; diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 13fcc1b471..e62c2f3ccb 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -258,18 +258,18 @@ static efi_status_t efi_disk_add_dev( diskobj->dp = efi_dp_from_part(desc, part); } diskobj->part = part; - ret = efi_add_protocol(diskobj->parent.handle, &efi_block_io_guid, + ret = efi_add_protocol(&diskobj->parent, &efi_block_io_guid, &diskobj->ops); if (ret != EFI_SUCCESS) return ret; - ret = efi_add_protocol(diskobj->parent.handle, &efi_guid_device_path, + ret = efi_add_protocol(&diskobj->parent, &efi_guid_device_path, diskobj->dp); if (ret != EFI_SUCCESS) return ret; if (part >= 1) { diskobj->volume = efi_simple_file_system(desc, part, diskobj->dp); - ret = efi_add_protocol(diskobj->parent.handle, + ret = efi_add_protocol(&diskobj->parent, &efi_simple_file_system_protocol_guid, diskobj->volume); if (ret != EFI_SUCCESS) @@ -381,7 +381,7 @@ efi_status_t efi_disk_register(void)
/* Partitions show up as block devices in EFI */ disks += efi_disk_create_partitions( - disk->parent.handle, desc, if_typename, + &disk->parent, desc, if_typename, desc->devnum, dev->name); } #else diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index a4aa9bcf61..a13c626f6f 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -442,7 +442,7 @@ efi_status_t efi_gop_register(void) efi_add_handle(&gopobj->parent);
/* Fill in object data */ - ret = efi_add_protocol(gopobj->parent.handle, &efi_gop_guid, + ret = efi_add_protocol(&gopobj->parent, &efi_gop_guid, &gopobj->ops); if (ret != EFI_SUCCESS) { printf("ERROR: Failure adding gop protocol\n"); diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 034d0d2ed0..d4b39b9e76 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -329,15 +329,15 @@ efi_status_t efi_net_register(void) efi_add_handle(&netobj->parent);
/* Fill in object data */ - r = efi_add_protocol(netobj->parent.handle, &efi_net_guid, + r = efi_add_protocol(&netobj->parent, &efi_net_guid, &netobj->net); if (r != EFI_SUCCESS) goto failure_to_add_protocol; - r = efi_add_protocol(netobj->parent.handle, &efi_guid_device_path, + r = efi_add_protocol(&netobj->parent, &efi_guid_device_path, efi_dp_from_eth()); if (r != EFI_SUCCESS) goto failure_to_add_protocol; - r = efi_add_protocol(netobj->parent.handle, &efi_pxe_guid, + r = efi_add_protocol(&netobj->parent, &efi_pxe_guid, &netobj->pxe); if (r != EFI_SUCCESS) goto failure_to_add_protocol;
participants (1)
-
Heinrich Schuchardt