[{"id":1745651,"web_url":"http://patchwork.ozlabs.org/comment/1745651/","msgid":"","list_archive_url":null,"date":"2017-08-12T13:37:34","subject":"Re: [U-Boot] [PATCH v3 2/3] efi_loader: open_info in OpenProtocol, \n\tCloseProtocol","submitter":{"id":1212,"url":"http://patchwork.ozlabs.org/api/people/1212/","name":"Alexander Graf","email":"agraf@suse.de"},"content":"On 05.08.17 22:32, Heinrich Schuchardt wrote:\n> efi_open_protocol and close_protocol have to keep track of\n> opened protocols.\n> \n> So we add an array open_info to each protocol of each handle.\n> \n> Cc: Rob Clark \n> Signed-off-by: Heinrich Schuchardt \n> ---\n> v3:\n> \tuse EFI_CALL to avoid wrapper for efi_disconnect_controller\n> \tuse list_for_each_entry\n> \tmove variable declarations out of loops\n> v2:\n> \tnew patch\n> ---\n> include/efi_loader.h | 1 +\n> lib/efi_loader/efi_boottime.c | 164 +++++++++++++++++++++++++++++++++++++++---\n> 2 files changed, 155 insertions(+), 10 deletions(-)\n> \n> diff --git a/include/efi_loader.h b/include/efi_loader.h\n> index 553c615f11..222b251a38 100644\n> --- a/include/efi_loader.h\n> +++ b/include/efi_loader.h\n> @@ -88,6 +88,7 @@ extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;\n> struct efi_handler {\n> \tconst efi_guid_t *guid;\n> \tvoid *protocol_interface;\n> +\tstruct efi_open_protocol_info_entry open_info[4];\n> };\n> \n> /*\n> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c\n> index ebb557abb2..e969814476 100644\n> --- a/lib/efi_loader/efi_boottime.c\n> +++ b/lib/efi_loader/efi_boottime.c\n> @@ -898,23 +898,78 @@ static efi_status_t EFIAPI efi_connect_controller(\n> \treturn EFI_EXIT(EFI_NOT_FOUND);\n> }\n> \n> -static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle,\n> -\t\t\t\t\t\t void *driver_image_handle,\n> -\t\t\t\t\t\t void *child_handle)\n> +static efi_status_t EFIAPI efi_disconnect_controller(\n> +\t\t\t\t\t\tvoid *controller_handle,\n> +\t\t\t\t\t\tvoid *driver_image_handle,\n> +\t\t\t\t\t\tvoid *child_handle)\n> {\n> \tEFI_ENTRY(\"%p, %p, %p\", controller_handle, driver_image_handle,\n> \t\t child_handle);\n> \treturn EFI_EXIT(EFI_INVALID_PARAMETER);\n> }\n> \n> +static efi_status_t efi_close_protocol_int(struct efi_handler *protocol,\n\nPlease either wrap _ext or use EFI_CALL :).\n\n> +\t\t\t\t\t void *agent_handle,\n> +\t\t\t\t\t void *controller_handle)\n> +{\n> +\tsize_t i;\n> +\tstruct efi_open_protocol_info_entry *open_info;\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n> +\t\topen_info = &protocol->open_info[i];\n> +\n> +\t\tif (!open_info->open_count)\n> +\t\t\tcontinue;\n> +\n> +\t\tif (open_info->agent_handle == agent_handle &&\n> +\t\t open_info->controller_handle ==\n> +\t\t controller_handle) {\n> +\t\t\topen_info->open_count--;\n> +\t\t\treturn EFI_SUCCESS;\n> +\t\t}\n> +\t}\n> +\treturn EFI_NOT_FOUND;\n> +}\n> +\n> static efi_status_t EFIAPI efi_close_protocol(void *handle,\n> \t\t\t\t\t efi_guid_t *protocol,\n> \t\t\t\t\t void *agent_handle,\n> \t\t\t\t\t void *controller_handle)\n> {\n> +\tstruct efi_object *efiobj;\n> +\tsize_t i;\n> +\tefi_status_t r = EFI_NOT_FOUND;\n> +\n> \tEFI_ENTRY(\"%p, %p, %p, %p\", handle, protocol, agent_handle,\n> \t\t controller_handle);\n> -\treturn EFI_EXIT(EFI_NOT_FOUND);\n> +\n> +\tif (!handle || !protocol || !agent_handle) {\n> +\t\tr = EFI_INVALID_PARAMETER;\n> +\t\tgoto out;\n> +\t}\n> +\n> +\tEFI_PRINT_GUID(\"protocol:\", protocol);\n> +\n> +\tlist_for_each_entry(efiobj, &efi_obj_list, link) {\n> +\t\tif (efiobj->handle != handle)\n> +\t\t\tcontinue;\n> +\n> +\t\tfor (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {\n> +\t\t\tstruct efi_handler *handler = &efiobj->protocols[i];\n> +\t\t\tconst efi_guid_t *hprotocol = handler->guid;\n> +\t\t\tif (!hprotocol)\n> +\t\t\t\tcontinue;\n> +\t\t\tif (!guidcmp(hprotocol, protocol)) {\n> +\t\t\t\tr = efi_close_protocol_int(handler,\n> +\t\t\t\t\t\t\t agent_handle,\n> +\t\t\t\t\t\t\t controller_handle);\n> +\t\t\t\tgoto out;\n> +\t\t\t}\n> +\t\t}\n> +\t\tgoto out;\n> +\t}\n> +out:\n> +\treturn EFI_EXIT(r);\n> }\n> \n> static efi_status_t EFIAPI efi_open_protocol_information(efi_handle_t handle,\n> @@ -1119,6 +1174,96 @@ static void EFIAPI efi_set_mem(void *buffer, unsigned long size, uint8_t value)\n> \tmemset(buffer, value, size);\n> }\n> \n> +static efi_status_t efi_open_protocol_int(\n\nSame here.\n\n\nAlex\n\n> +\t\t\tstruct efi_handler *protocol,\n> +\t\t\tvoid **protocol_interface, void *agent_handle,\n> +\t\t\tvoid *controller_handle, uint32_t attributes)\n> +{\n> +\tbool opened_exclusive = false;\n> +\tbool opened_by_driver = false;\n> +\tint i;\n> +\tstruct efi_open_protocol_info_entry *open_info;\n> +\tstruct efi_open_protocol_info_entry *match = NULL;\n> +\n> +\tif (attributes !=\n> +\t EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {\n> +\t\t*protocol_interface = NULL;\n> +\t}\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n> +\t\topen_info = &protocol->open_info[i];\n> +\n> +\t\tif (!open_info->open_count)\n> +\t\t\tcontinue;\n> +\t\tif (open_info->agent_handle == agent_handle) {\n> +\t\t\tif ((attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) &&\n> +\t\t\t (open_info->attributes == attributes))\n> +\t\t\t\treturn EFI_ALREADY_STARTED;\n> +\t\t\tif (open_info->controller_handle == controller_handle)\n> +\t\t\t\tmatch = open_info;\n> +\t\t}\n> +\t\tif (open_info->attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE)\n> +\t\t\topened_exclusive = true;\n> +\t}\n> +\n> +\tif (attributes &\n> +\t (EFI_OPEN_PROTOCOL_EXCLUSIVE | EFI_OPEN_PROTOCOL_BY_DRIVER) &&\n> +\t opened_exclusive)\n> +\t\treturn EFI_ACCESS_DENIED;\n> +\n> +\tif (attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE) {\n> +\t\tfor (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n> +\t\t\topen_info = &protocol->open_info[i];\n> +\n> +\t\t\tif (!open_info->open_count)\n> +\t\t\t\tcontinue;\n> +\t\t\tif (open_info->attributes ==\n> +\t\t\t\t\tEFI_OPEN_PROTOCOL_BY_DRIVER)\n> +\t\t\t\tEFI_CALL(efi_disconnect_controller(\n> +\t\t\t\t\t\topen_info->controller_handle,\n> +\t\t\t\t\t\topen_info->agent_handle,\n> +\t\t\t\t\t\tNULL));\n> +\t\t}\n> +\t\topened_by_driver = false;\n> +\t\tfor (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n> +\t\t\topen_info = &protocol->open_info[i];\n> +\n> +\t\t\tif (!open_info->open_count)\n> +\t\t\t\tcontinue;\n> +\t\t\tif (open_info->attributes & EFI_OPEN_PROTOCOL_BY_DRIVER)\n> +\t\t\t\topened_by_driver = true;\n> +\t\t}\n> +\t\tif (opened_by_driver)\n> +\t\t\treturn EFI_ACCESS_DENIED;\n> +\t\tif (match && !match->open_count)\n> +\t\t\tmatch = NULL;\n> +\t}\n> +\n> +\t/*\n> +\t * Find an empty slot.\n> +\t */\n> +\tif (!match) {\n> +\t\tfor (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n> +\t\t\topen_info = &protocol->open_info[i];\n> +\n> +\t\t\tif (!open_info->open_count) {\n> +\t\t\t\tmatch = open_info;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\tif (!match)\n> +\t\treturn EFI_OUT_OF_RESOURCES;\n> +\n> +\tmatch->agent_handle = agent_handle;\n> +\tmatch->controller_handle = controller_handle;\n> +\tmatch->attributes = attributes;\n> +\tmatch->open_count++;\n> +\t*protocol_interface = protocol->protocol_interface;\n> +\n> +\treturn EFI_SUCCESS;\n> +}\n> +\n> static efi_status_t EFIAPI efi_open_protocol(\n> \t\t\tvoid *handle, efi_guid_t *protocol,\n> \t\t\tvoid **protocol_interface, void *agent_handle,\n> @@ -1173,12 +1318,11 @@ static efi_status_t EFIAPI efi_open_protocol(\n> \t\t\tif (!hprotocol)\n> \t\t\t\tcontinue;\n> \t\t\tif (!guidcmp(hprotocol, protocol)) {\n> -\t\t\t\tif (attributes !=\n> -\t\t\t\t EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {\n> -\t\t\t\t\t*protocol_interface =\n> -\t\t\t\t\t\thandler->protocol_interface;\n> -\t\t\t\t}\n> -\t\t\t\tr = EFI_SUCCESS;\n> +\t\t\t\tr = efi_open_protocol_int(handler,\n> +\t\t\t\t\t\t\t protocol_interface,\n> +\t\t\t\t\t\t\t agent_handle,\n> +\t\t\t\t\t\t\t controller_handle,\n> +\t\t\t\t\t\t\t attributes);\n> \t\t\t\tgoto out;\n> \t\t\t}\n> \t\t}\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xV30J6gC1z9t32\n\tfor ;\n\tSat, 12 Aug 2017 23:40:16 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid A6C4CC21E56; Sat, 12 Aug 2017 13:38:56 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id AEA27C21E16;\n\tSat, 12 Aug 2017 13:38:53 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 71084C21EEB; Sat, 12 Aug 2017 13:37:35 +0000 (UTC)","from mx1.suse.de (mx2.suse.de [195.135.220.15])\n\tby lists.denx.de (Postfix) with ESMTPS id 8D905C21EE1\n\tfor ; Sat, 12 Aug 2017 13:37:35 +0000 (UTC)","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 49473ABB1;\n\tSat, 12 Aug 2017 13:37:35 +0000 (UTC)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED\n\tautolearn=unavailable autolearn_force=no version=3.4.0","X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","To":"Heinrich Schuchardt ","References":"<20170805203230.19796-1-xypron.glpk@gmx.de>\n\t<20170805203230.19796-3-xypron.glpk@gmx.de>","From":"Alexander Graf ","Message-ID":"","Date":"Sat, 12 Aug 2017 15:37:34 +0200","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170805203230.19796-3-xypron.glpk@gmx.de>","Content-Language":"en-US","Cc":"u-boot@lists.denx.de","Subject":"Re: [U-Boot] [PATCH v3 2/3] efi_loader: open_info in OpenProtocol, \n\tCloseProtocol","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1745814,"web_url":"http://patchwork.ozlabs.org/comment/1745814/","msgid":"","list_archive_url":null,"date":"2017-08-13T11:09:11","subject":"Re: [U-Boot] [PATCH v3 2/3] efi_loader: open_info in OpenProtocol, \n\tCloseProtocol","submitter":{"id":61270,"url":"http://patchwork.ozlabs.org/api/people/61270/","name":"Heinrich Schuchardt","email":"xypron.glpk@gmx.de"},"content":"On 08/12/2017 03:37 PM, Alexander Graf wrote:\n> \n> \n> On 05.08.17 22:32, Heinrich Schuchardt wrote:\n>> efi_open_protocol and close_protocol have to keep track of\n>> opened protocols.\n>>\n>> So we add an array open_info to each protocol of each handle.\n>>\n>> Cc: Rob Clark \n>> Signed-off-by: Heinrich Schuchardt \n>> ---\n>> v3:\n>> use EFI_CALL to avoid wrapper for efi_disconnect_controller\n>> use list_for_each_entry\n>> move variable declarations out of loops\n>> v2:\n>> new patch\n>> ---\n>> include/efi_loader.h | 1 +\n>> lib/efi_loader/efi_boottime.c | 164\n>> +++++++++++++++++++++++++++++++++++++++---\n>> 2 files changed, 155 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/include/efi_loader.h b/include/efi_loader.h\n>> index 553c615f11..222b251a38 100644\n>> --- a/include/efi_loader.h\n>> +++ b/include/efi_loader.h\n>> @@ -88,6 +88,7 @@ extern unsigned int __efi_runtime_rel_start,\n>> __efi_runtime_rel_stop;\n>> struct efi_handler {\n>> const efi_guid_t *guid;\n>> void *protocol_interface;\n>> + struct efi_open_protocol_info_entry open_info[4];\n>> };\n>> /*\n>> diff --git a/lib/efi_loader/efi_boottime.c\n>> b/lib/efi_loader/efi_boottime.c\n>> index ebb557abb2..e969814476 100644\n>> --- a/lib/efi_loader/efi_boottime.c\n>> +++ b/lib/efi_loader/efi_boottime.c\n>> @@ -898,23 +898,78 @@ static efi_status_t EFIAPI efi_connect_controller(\n>> return EFI_EXIT(EFI_NOT_FOUND);\n>> }\n>> -static efi_status_t EFIAPI efi_disconnect_controller(void\n>> *controller_handle,\n>> - void *driver_image_handle,\n>> - void *child_handle)\n>> +static efi_status_t EFIAPI efi_disconnect_controller(\n>> + void *controller_handle,\n>> + void *driver_image_handle,\n>> + void *child_handle)\n>> {\n>> EFI_ENTRY(\"%p, %p, %p\", controller_handle, driver_image_handle,\n>> child_handle);\n>> return EFI_EXIT(EFI_INVALID_PARAMETER);\n>> }\n>> +static efi_status_t efi_close_protocol_int(struct efi_handler\n>> *protocol,\n> \n> Please either wrap _ext or use EFI_CALL :).\n\nWhy?\n\nFunction efi_close_protocol_int is only used to avoid lines over 80\ncharacters in efi_disconnect_controller. It is not called from anywhere\nelse.\n\nShould I add some comment in the code or in the commit message?\n\n> \n>> + void *agent_handle,\n>> + void *controller_handle)\n>> +{\n>> + size_t i;\n>> + struct efi_open_protocol_info_entry *open_info;\n>> +\n>> + for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n>> + open_info = &protocol->open_info[i];\n>> +\n>> + if (!open_info->open_count)\n>> + continue;\n>> +\n>> + if (open_info->agent_handle == agent_handle &&\n>> + open_info->controller_handle ==\n>> + controller_handle) {\n>> + open_info->open_count--;\n>> + return EFI_SUCCESS;\n>> + }\n>> + }\n>> + return EFI_NOT_FOUND;\n>> +}\n>> +\n>> static efi_status_t EFIAPI efi_close_protocol(void *handle,\n>> efi_guid_t *protocol,\n>> void *agent_handle,\n>> void *controller_handle)\n>> {\n>> + struct efi_object *efiobj;\n>> + size_t i;\n>> + efi_status_t r = EFI_NOT_FOUND;\n>> +\n>> EFI_ENTRY(\"%p, %p, %p, %p\", handle, protocol, agent_handle,\n>> controller_handle);\n>> - return EFI_EXIT(EFI_NOT_FOUND);\n>> +\n>> + if (!handle || !protocol || !agent_handle) {\n>> + r = EFI_INVALID_PARAMETER;\n>> + goto out;\n>> + }\n>> +\n>> + EFI_PRINT_GUID(\"protocol:\", protocol);\n>> +\n>> + list_for_each_entry(efiobj, &efi_obj_list, link) {\n>> + if (efiobj->handle != handle)\n>> + continue;\n>> +\n>> + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {\n>> + struct efi_handler *handler = &efiobj->protocols[i];\n>> + const efi_guid_t *hprotocol = handler->guid;\n>> + if (!hprotocol)\n>> + continue;\n>> + if (!guidcmp(hprotocol, protocol)) {\n>> + r = efi_close_protocol_int(handler,\n>> + agent_handle,\n>> + controller_handle);\n>> + goto out;\n>> + }\n>> + }\n>> + goto out;\n>> + }\n>> +out:\n>> + return EFI_EXIT(r);\n>> }\n>> static efi_status_t EFIAPI\n>> efi_open_protocol_information(efi_handle_t handle,\n>> @@ -1119,6 +1174,96 @@ static void EFIAPI efi_set_mem(void *buffer,\n>> unsigned long size, uint8_t value)\n>> memset(buffer, value, size);\n>> }\n>> +static efi_status_t efi_open_protocol_int(\n> \n> Same here.\n> \n> \n> Alex\n\nSee above.\n\nWas the rest of the patch ok for you?\n\nRegards\n\nHeinrich\n\n> \n>> + struct efi_handler *protocol,\n>> + void **protocol_interface, void *agent_handle,\n>> + void *controller_handle, uint32_t attributes)\n>> +{\n>> + bool opened_exclusive = false;\n>> + bool opened_by_driver = false;\n>> + int i;\n>> + struct efi_open_protocol_info_entry *open_info;\n>> + struct efi_open_protocol_info_entry *match = NULL;\n>> +\n>> + if (attributes !=\n>> + EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {\n>> + *protocol_interface = NULL;\n>> + }\n>> +\n>> + for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n>> + open_info = &protocol->open_info[i];\n>> +\n>> + if (!open_info->open_count)\n>> + continue;\n>> + if (open_info->agent_handle == agent_handle) {\n>> + if ((attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) &&\n>> + (open_info->attributes == attributes))\n>> + return EFI_ALREADY_STARTED;\n>> + if (open_info->controller_handle == controller_handle)\n>> + match = open_info;\n>> + }\n>> + if (open_info->attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE)\n>> + opened_exclusive = true;\n>> + }\n>> +\n>> + if (attributes &\n>> + (EFI_OPEN_PROTOCOL_EXCLUSIVE | EFI_OPEN_PROTOCOL_BY_DRIVER) &&\n>> + opened_exclusive)\n>> + return EFI_ACCESS_DENIED;\n>> +\n>> + if (attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE) {\n>> + for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n>> + open_info = &protocol->open_info[i];\n>> +\n>> + if (!open_info->open_count)\n>> + continue;\n>> + if (open_info->attributes ==\n>> + EFI_OPEN_PROTOCOL_BY_DRIVER)\n>> + EFI_CALL(efi_disconnect_controller(\n>> + open_info->controller_handle,\n>> + open_info->agent_handle,\n>> + NULL));\n>> + }\n>> + opened_by_driver = false;\n>> + for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n>> + open_info = &protocol->open_info[i];\n>> +\n>> + if (!open_info->open_count)\n>> + continue;\n>> + if (open_info->attributes & EFI_OPEN_PROTOCOL_BY_DRIVER)\n>> + opened_by_driver = true;\n>> + }\n>> + if (opened_by_driver)\n>> + return EFI_ACCESS_DENIED;\n>> + if (match && !match->open_count)\n>> + match = NULL;\n>> + }\n>> +\n>> + /*\n>> + * Find an empty slot.\n>> + */\n>> + if (!match) {\n>> + for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n>> + open_info = &protocol->open_info[i];\n>> +\n>> + if (!open_info->open_count) {\n>> + match = open_info;\n>> + break;\n>> + }\n>> + }\n>> + }\n>> + if (!match)\n>> + return EFI_OUT_OF_RESOURCES;\n>> +\n>> + match->agent_handle = agent_handle;\n>> + match->controller_handle = controller_handle;\n>> + match->attributes = attributes;\n>> + match->open_count++;\n>> + *protocol_interface = protocol->protocol_interface;\n>> +\n>> + return EFI_SUCCESS;\n>> +}\n>> +\n>> static efi_status_t EFIAPI efi_open_protocol(\n>> void *handle, efi_guid_t *protocol,\n>> void **protocol_interface, void *agent_handle,\n>> @@ -1173,12 +1318,11 @@ static efi_status_t EFIAPI efi_open_protocol(\n>> if (!hprotocol)\n>> continue;\n>> if (!guidcmp(hprotocol, protocol)) {\n>> - if (attributes !=\n>> - EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {\n>> - *protocol_interface =\n>> - handler->protocol_interface;\n>> - }\n>> - r = EFI_SUCCESS;\n>> + r = efi_open_protocol_int(handler,\n>> + protocol_interface,\n>> + agent_handle,\n>> + controller_handle,\n>> + attributes);\n>> goto out;\n>> }\n>> }\n>>\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xVbc16Fsfz9sR9\n\tfor ;\n\tSun, 13 Aug 2017 21:09:37 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 65E82C21E35; Sun, 13 Aug 2017 11:09:33 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 059C8C21DD4;\n\tSun, 13 Aug 2017 11:09:30 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 6BF73C21C6A; Sun, 13 Aug 2017 11:09:28 +0000 (UTC)","from mout.gmx.net (mout.gmx.net [212.227.15.15])\n\tby lists.denx.de (Postfix) with ESMTPS id 5B4B9C21C6A\n\tfor ; Sun, 13 Aug 2017 11:09:27 +0000 (UTC)","from [192.168.123.55] ([84.118.154.110]) by mail.gmx.com (mrgmx002\n\t[212.227.17.190]) with ESMTPSA (Nemesis) id\n\t0LtrKX-1dZbJQ3WhJ-011A0R; Sun, 13 Aug 2017 13:09:25 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.7 required=5.0 tests=FREEMAIL_FROM,\n\tRCVD_IN_DNSWL_LOW,\n\tRCVD_IN_MSPIKE_H2 autolearn=unavailable autolearn_force=no\n\tversion=3.4.0","To":"Alexander Graf ","References":"<20170805203230.19796-1-xypron.glpk@gmx.de>\n\t<20170805203230.19796-3-xypron.glpk@gmx.de>\n\t","From":"Heinrich Schuchardt ","Message-ID":"","Date":"Sun, 13 Aug 2017 13:09:11 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","X-Provags-ID":"V03:K0:jCmtvjNsH2pvSw8CreL/x651oGUkELr4zF3GOPcLt65RHbQ6Ehz\n\tTChOK6wmt/UP6i/ahZuvN84LMoDBz9dX79Fy1FWorbITc7Os8UkA54RuUCmoDIzrrYC2PQf\n\tgC5jYwZ1phoYqiXrv/3jm0Fj9FFLt+S8atBAKkoR4pBkz9Ahf6ulQF3SYSgU0yoN7DYPbia\n\tyQOtDpyJs3IcRdAF/Qjsw==","X-UI-Out-Filterresults":"notjunk:1; V01:K0:C6t30WhspB4=:VW1jBcwoGn0bCqMtbNDCOK\n\t/U05uc6L64xrVVKLl8kWZYwpx1Qf36JqpakXNditl0KpKH0n7TmDLTLG4xlnqiAAhGAuz2DAm\n\tSVbK0lrXDkS+CGZEB99YZYBuuXhyoHgS5hY8xvWiiJZITb5MoNLeMWoQygyaMoHsrCwCCyLXf\n\tNxjWZrxdfK9S5NI+K040Jeh2UQwfcDwE7YsCIP590DkFGkOEo6ypWEhXeh+tjYlv9HV7XstRD\n\thfIRZQ5gEZq3VRwV53f/s4Tvk0v/h7uA0HhS/Cqb6LgpFoHDp1UOxXTajncr0yVSP1rQoct8L\n\takpmw8ehQPsH0HSwHtNhAdLF+nZFSkfA5I85IoYkdbLAx/Ba/Q4mKnPD1ZCuJcbHVxpBkjn1h\n\tKoZrCBqT5XX0bKztBX9T+BROw1McKZ6pOAeGjjTJGsiGvMXQgoHex36+qm0cew415oT9T+IMF\n\t2XFdltpPP3rJHHRNxvYqhFQbHFSKGgkR4J9V8sF0mNms4EsjgNtLoIlWTL/ssUVEHa4silxV5\n\tBMvAQCfiP+vWGtIHt3vbQdUPGUhnrxbEmVYM5BTRC6tCC6vbk4tL7uSdq2ZVIfWypKS4vx+2o\n\tmzZ94Kpzw+zA2MbK8/rC5EklZw/kdJHl5gC0Q73E0W469f8WuGgyyJNU2uLV9KXmxNFXBg4NR\n\tZikZd4degYNHU6v/yNx85ujR8IyJTS7np1ovz8BQnK4QSRfhxQj0B29xGDqiAf354EmTLNV99\n\tZu4mt8ifqzLZvyGmE6nGdaiaf5IbaJsG+g7XRHKQkh2vpxp0m9ocIh8xJ/9Lp7fpIR4xCTGEj\n\tEa+RICP0v4TrFrLgKEv0iGQmB33PLSU62Z2WitY0Kkl0pho+ao=","Cc":"u-boot@lists.denx.de","Subject":"Re: [U-Boot] [PATCH v3 2/3] efi_loader: open_info in OpenProtocol, \n\tCloseProtocol","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1745925,"web_url":"http://patchwork.ozlabs.org/comment/1745925/","msgid":"<0d1a13b1-6bb6-4041-5b73-e5afd43ee929@suse.de>","list_archive_url":null,"date":"2017-08-13T19:23:10","subject":"Re: [U-Boot] [PATCH v3 2/3] efi_loader: open_info in OpenProtocol, \n\tCloseProtocol","submitter":{"id":1212,"url":"http://patchwork.ozlabs.org/api/people/1212/","name":"Alexander Graf","email":"agraf@suse.de"},"content":"On 13.08.17 13:09, Heinrich Schuchardt wrote:\n> On 08/12/2017 03:37 PM, Alexander Graf wrote:\n>>\n>>\n>> On 05.08.17 22:32, Heinrich Schuchardt wrote:\n>>> efi_open_protocol and close_protocol have to keep track of\n>>> opened protocols.\n>>>\n>>> So we add an array open_info to each protocol of each handle.\n>>>\n>>> Cc: Rob Clark \n>>> Signed-off-by: Heinrich Schuchardt \n>>> ---\n>>> v3:\n>>> use EFI_CALL to avoid wrapper for efi_disconnect_controller\n>>> use list_for_each_entry\n>>> move variable declarations out of loops\n>>> v2:\n>>> new patch\n>>> ---\n>>> include/efi_loader.h | 1 +\n>>> lib/efi_loader/efi_boottime.c | 164\n>>> +++++++++++++++++++++++++++++++++++++++---\n>>> 2 files changed, 155 insertions(+), 10 deletions(-)\n>>>\n>>> diff --git a/include/efi_loader.h b/include/efi_loader.h\n>>> index 553c615f11..222b251a38 100644\n>>> --- a/include/efi_loader.h\n>>> +++ b/include/efi_loader.h\n>>> @@ -88,6 +88,7 @@ extern unsigned int __efi_runtime_rel_start,\n>>> __efi_runtime_rel_stop;\n>>> struct efi_handler {\n>>> const efi_guid_t *guid;\n>>> void *protocol_interface;\n>>> + struct efi_open_protocol_info_entry open_info[4];\n>>> };\n>>> /*\n>>> diff --git a/lib/efi_loader/efi_boottime.c\n>>> b/lib/efi_loader/efi_boottime.c\n>>> index ebb557abb2..e969814476 100644\n>>> --- a/lib/efi_loader/efi_boottime.c\n>>> +++ b/lib/efi_loader/efi_boottime.c\n>>> @@ -898,23 +898,78 @@ static efi_status_t EFIAPI efi_connect_controller(\n>>> return EFI_EXIT(EFI_NOT_FOUND);\n>>> }\n>>> -static efi_status_t EFIAPI efi_disconnect_controller(void\n>>> *controller_handle,\n>>> - void *driver_image_handle,\n>>> - void *child_handle)\n>>> +static efi_status_t EFIAPI efi_disconnect_controller(\n>>> + void *controller_handle,\n>>> + void *driver_image_handle,\n>>> + void *child_handle)\n>>> {\n>>> EFI_ENTRY(\"%p, %p, %p\", controller_handle, driver_image_handle,\n>>> child_handle);\n>>> return EFI_EXIT(EFI_INVALID_PARAMETER);\n>>> }\n>>> +static efi_status_t efi_close_protocol_int(struct efi_handler\n>>> *protocol,\n>>\n>> Please either wrap _ext or use EFI_CALL :).\n> \n> Why?\n> \n> Function efi_close_protocol_int is only used to avoid lines over 80\n> characters in efi_disconnect_controller. It is not called from anywhere\n> else.\n> \n> Should I add some comment in the code or in the commit message?\n\nAh, now I see. No, I think the function name is misleading, so that \nneeds change :). How about efi_close_one_protocol()?\n\n> \n>>\n>>> + void *agent_handle,\n>>> + void *controller_handle)\n>>> +{\n>>> + size_t i;\n>>> + struct efi_open_protocol_info_entry *open_info;\n>>> +\n>>> + for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {\n>>> + open_info = &protocol->open_info[i];\n>>> +\n>>> + if (!open_info->open_count)\n>>> + continue;\n>>> +\n>>> + if (open_info->agent_handle == agent_handle &&\n>>> + open_info->controller_handle ==\n>>> + controller_handle) {\n>>> + open_info->open_count--;\n>>> + return EFI_SUCCESS;\n>>> + }\n>>> + }\n>>> + return EFI_NOT_FOUND;\n>>> +}\n>>> +\n>>> static efi_status_t EFIAPI efi_close_protocol(void *handle,\n>>> efi_guid_t *protocol,\n>>> void *agent_handle,\n>>> void *controller_handle)\n>>> {\n>>> + struct efi_object *efiobj;\n>>> + size_t i;\n>>> + efi_status_t r = EFI_NOT_FOUND;\n>>> +\n>>> EFI_ENTRY(\"%p, %p, %p, %p\", handle, protocol, agent_handle,\n>>> controller_handle);\n>>> - return EFI_EXIT(EFI_NOT_FOUND);\n>>> +\n>>> + if (!handle || !protocol || !agent_handle) {\n>>> + r = EFI_INVALID_PARAMETER;\n>>> + goto out;\n>>> + }\n>>> +\n>>> + EFI_PRINT_GUID(\"protocol:\", protocol);\n>>> +\n>>> + list_for_each_entry(efiobj, &efi_obj_list, link) {\n>>> + if (efiobj->handle != handle)\n>>> + continue;\n>>> +\n>>> + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {\n>>> + struct efi_handler *handler = &efiobj->protocols[i];\n>>> + const efi_guid_t *hprotocol = handler->guid;\n>>> + if (!hprotocol)\n>>> + continue;\n>>> + if (!guidcmp(hprotocol, protocol)) {\n>>> + r = efi_close_protocol_int(handler,\n>>> + agent_handle,\n>>> + controller_handle);\n>>> + goto out;\n>>> + }\n>>> + }\n>>> + goto out;\n>>> + }\n>>> +out:\n>>> + return EFI_EXIT(r);\n>>> }\n>>> static efi_status_t EFIAPI\n>>> efi_open_protocol_information(efi_handle_t handle,\n>>> @@ -1119,6 +1174,96 @@ static void EFIAPI efi_set_mem(void *buffer,\n>>> unsigned long size, uint8_t value)\n>>> memset(buffer, value, size);\n>>> }\n>>> +static efi_status_t efi_open_protocol_int(\n>>\n>> Same here.\n>>\n>>\n>> Alex\n> \n> See above.\n> \n> Was the rest of the patch ok for you?\n\nI didn't spot anything obviously bad, but that doesn't usually mean \nmuch. My review foo isn't quite as good as others'. When applying I \nwould push the patches through some more detailed testing though.\n\n\nAlex","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xVpYr3f13z9s83\n\tfor ;\n\tMon, 14 Aug 2017 05:23:26 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 97CE8C21D82; Sun, 13 Aug 2017 19:23:17 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id E3C6AC21C2B;\n\tSun, 13 Aug 2017 19:23:14 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 0A217C21C51; Sun, 13 Aug 2017 19:23:12 +0000 (UTC)","from mx1.suse.de (mx2.suse.de [195.135.220.15])\n\tby lists.denx.de (Postfix) with ESMTPS id 94716C21C26\n\tfor ; Sun, 13 Aug 2017 19:23:12 +0000 (UTC)","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 04A95ABB3;\n\tSun, 13 Aug 2017 19:23:12 +0000 (UTC)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED\n\tautolearn=unavailable autolearn_force=no version=3.4.0","X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","To":"Heinrich Schuchardt ","References":"<20170805203230.19796-1-xypron.glpk@gmx.de>\n\t<20170805203230.19796-3-xypron.glpk@gmx.de>\n\t\n\t","From":"Alexander Graf ","Message-ID":"<0d1a13b1-6bb6-4041-5b73-e5afd43ee929@suse.de>","Date":"Sun, 13 Aug 2017 21:23:10 +0200","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","Cc":"u-boot@lists.denx.de","Subject":"Re: [U-Boot] [PATCH v3 2/3] efi_loader: open_info in OpenProtocol, \n\tCloseProtocol","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]