On 11/08/2017 04:43 PM, Alexander Graf wrote:
On 11/01/2017 09:31 AM, Heinrich Schuchardt wrote:
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
include/efi_loader.h | 6 ++- lib/efi_loader/efi_boottime.c | 107 ++++++++++++++++++++---------------------- lib/efi_loader/efi_disk.c | 1 + lib/efi_loader/efi_gop.c | 1 + lib/efi_loader/efi_net.c | 1 + 5 files changed, 58 insertions(+), 58 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index e1f0af3496..a73bbc1269 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -101,6 +101,8 @@ extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; * interface (usually a struct with callback functions), this struct maps the * protocol GUID to the respective protocol interface */ struct efi_handler { + /* Link to the list of protocols of a handle */ + struct list_head link; const efi_guid_t *guid; void *protocol_interface; }; @@ -115,8 +117,8 @@ struct efi_handler { struct efi_object { /* Every UEFI object is part of a global object list */ struct list_head link; - /* We support up to 16 "protocols" an object can be accessed through */ - struct efi_handler protocols[16]; + /* The list of protocols */ + struct list_head protocols; /* The object spawner can either use this for data or as identifier */ void *handle; }; diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 2b3db162a1..cee0cb1390 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -339,6 +339,7 @@ efi_status_t efi_create_handle(void **handle) return r; memset(obj, 0, sizeof(struct efi_object)); obj->handle = obj; + INIT_LIST_HEAD(&obj->protocols); list_add_tail(&obj->link, &efi_obj_list); *handle = obj; return r; @@ -715,18 +716,17 @@ efi_status_t efi_search_protocol(const void *handle, struct efi_handler **handler) { struct efi_object *efiobj; - size_t i; - struct efi_handler *protocol; + struct list_head *lhandle; 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; + list_for_each(lhandle, &efiobj->protocols) { + struct efi_handler *protocol;
+ protocol = list_entry(lhandle, struct efi_handler, link); if (!guidcmp(protocol->guid, protocol_guid)) { if (handler) *handler = protocol; @@ -750,7 +750,6 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol, struct efi_object *efiobj; struct efi_handler *handler; efi_status_t ret; - size_t i; efiobj = efi_search_obj(handle); if (!efiobj) @@ -761,16 +760,10 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol, 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; + handler->guid = protocol; + handler->protocol_interface = protocol_interface; + list_add_tail(&handler->link, &efiobj->protocols); + return EFI_SUCCESS; } /* @@ -790,10 +783,10 @@ efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol, 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; + if (guidcmp(handler->guid, protocol)) + return EFI_INVALID_PARAMETER; + list_del(&handler->link); + free(handler); return EFI_SUCCESS; } @@ -806,17 +799,22 @@ efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol, efi_status_t efi_remove_all_protocols(const void *handle) { struct efi_object *efiobj; - struct efi_handler *handler; - size_t i; + struct list_head *lhandle; + struct list_head *pos; efiobj = efi_search_obj(handle); if (!efiobj) return EFI_INVALID_PARAMETER; + list_for_each_safe(lhandle, pos, &efiobj->protocols) { + struct efi_handler *protocol; + efi_status_t ret; - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - handler = &efiobj->protocols[i]; - handler->guid = NULL; - handler->protocol_interface = NULL; + protocol = list_entry(lhandle, struct efi_handler, link);
+ ret = efi_remove_protocol(handle, protocol->guid, + protocol->protocol_interface); + if (ret != EFI_SUCCESS) + return ret; } return EFI_SUCCESS; } @@ -1171,6 +1169,7 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob if (device_path) info->device_handle = efi_dp_find_obj(device_path, NULL); + INIT_LIST_HEAD(&obj->protocols); list_add_tail(&obj->link, &efi_obj_list); /* * When asking for the device path interface, return @@ -1648,8 +1647,7 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle, { unsigned long buffer_size; struct efi_object *efiobj; - unsigned long i, j; - struct list_head *lhandle; + struct list_head *protocol_handle; efi_status_t r; EFI_ENTRY("%p, %p, %p", handle, protocol_buffer, @@ -1660,36 +1658,33 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle, *protocol_buffer = NULL; *protocol_buffer_count = 0; - list_for_each(lhandle, &efi_obj_list) { - efiobj = list_entry(lhandle, struct efi_object, link); - if (efiobj->handle != handle) - continue; + efiobj = efi_search_obj(handle); + if (!efiobj) + return EFI_EXIT(EFI_INVALID_PARAMETER); - /* Count protocols */ - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { - if (efiobj->protocols[i].guid) - ++*protocol_buffer_count; - } - /* Copy guids */ - if (*protocol_buffer_count) { - buffer_size = sizeof(efi_guid_t *) * - *protocol_buffer_count; - r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, - buffer_size, - (void **)protocol_buffer); - if (r != EFI_SUCCESS) - return EFI_EXIT(r); - j = 0; - for (i = 0; i < ARRAY_SIZE(efiobj->protocols); ++i) { - if (efiobj->protocols[i].guid) { - (*protocol_buffer)[j] = (void *) - efiobj->protocols[i].guid; - ++j; - } - } + /* Count protocols */ + list_for_each(protocol_handle, &efiobj->protocols) { + ++*protocol_buffer_count; + }
+ /* Copy guids */ + if (*protocol_buffer_count) { + size_t j = 0;
+ buffer_size = sizeof(efi_guid_t *) * *protocol_buffer_count; + r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, buffer_size, + (void **)protocol_buffer); + if (r != EFI_SUCCESS) + return EFI_EXIT(r); + list_for_each(protocol_handle, &efiobj->protocols) { + struct efi_handler *protocol;
+ protocol = list_entry(protocol_handle, + struct efi_handler, link); + (*protocol_buffer)[j] = (void *)protocol->guid; + ++j; } - break; } return EFI_EXIT(EFI_SUCCESS); diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 1d6cf3122f..af8db40e19 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -224,6 +224,7 @@ static void efi_disk_add_dev(const char *name, goto out_of_memory; /* Hook up to the device list */ + INIT_LIST_HEAD(&diskobj->parent.protocols); list_add_tail(&diskobj->parent.link, &efi_obj_list);
This seems to have become quite a common theme now. Maybe it's about time we add a helper function to manually add a precreated object to the object list (and clear the protocol list along the way)?
Currently we have quite different ways to initialize parent->handle:
lib/efi_loader/efi_net.c:303: netobj->parent.handle = &netobj->net; lib/efi_loader/efi_gop.c:187: gopobj->parent.handle = &gopobj->ops; lib/efi_loader/efi_disk.c:233: diskobj->parent.handle = diskobj; lib/efi_loader/efi_boottime.c:1166 obj->handle = info; lib/efi_loader/efi_boottime.c:341: obj->handle = obj;
I could not find any place where we actually dereference a handle. So shouldn't such a helper function also set obj.handle = &obj?
I would prefer to put the introduction of the helper function into a separate patch.
So could you, please, merge patches 2-13 of the series. And I will follow up with:
New version of patch 1 which is really standalone. Fix ups for this patch and the previous patch series.
Regards
Heinrich