[U-Boot] [PATCH 12/13] efi_loader: manage protocols in a linked list
Heinrich Schuchardt
xypron.glpk at gmx.de
Wed Nov 8 18:50:06 UTC 2017
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 at 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
More information about the U-Boot
mailing list