On 07/18/2017 03:12 PM, Alexander Graf wrote:
> On 07/11/2017 10:06 PM, Heinrich Schuchardt wrote:
>> efi_open_protocol was implemented to call a protocol specific open
>> function to retrieve the protocol interface.
>>
>> The UEFI specification does not know of such a function.
>>
>> It is not possible to implement InstallProtocolInterface with the
>> current design.
>>
>> With the patch the protocol interface itself is stored in the list
>> of installed protocols of an efi_object instead of an open function.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
>> ---
>> v2
>> Remove implementation of efi_return_handle.
>> ---
>> cmd/bootefi.c | 14 +++-----------
>> include/efi_loader.h | 17 ++---------------
>> lib/efi_loader/efi_boottime.c | 18 ++++++++++++++----
>> lib/efi_loader/efi_disk.c | 29 +++--------------------------
>> lib/efi_loader/efi_gop.c | 2 +-
>> lib/efi_loader/efi_image_loader.c | 8 --------
>> lib/efi_loader/efi_net.c | 30 +++---------------------------
>> 7 files changed, 26 insertions(+), 92 deletions(-)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 7ddeead671..7cf0129a18 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -54,14 +54,6 @@ static struct efi_device_path_file_path
>> bootefi_device_path[] = {
>> }
>> };
>> -static efi_status_t EFIAPI bootefi_open_dp(void *handle, efi_guid_t
>> *protocol,
>> - void **protocol_interface, void *agent_handle,
>> - void *controller_handle, uint32_t attributes)
>> -{
>> - *protocol_interface = bootefi_device_path;
>> - return EFI_SUCCESS;
>> -}
>> -
>> /* The EFI loaded_image interface for the image executed via
>> "bootefi" */
>> static struct efi_loaded_image loaded_image_info = {
>> .device_handle = bootefi_device_path,
>> @@ -78,7 +70,7 @@ static struct efi_object loaded_image_info_obj = {
>> * return handle which points to loaded_image_info
>> */
>> .guid = &efi_guid_loaded_image,
>> - .open = &efi_return_handle,
>> + .protocol_interface = &loaded_image_info,
>> },
>> {
>> /*
>> @@ -86,7 +78,7 @@ static struct efi_object loaded_image_info_obj = {
>> * bootefi_device_path
>> */
>> .guid = &efi_guid_device_path,
>> - .open = &bootefi_open_dp,
>> + .protocol_interface = bootefi_device_path,
>> },
>> },
>> };
>> @@ -99,7 +91,7 @@ static struct efi_object bootefi_device_obj = {
>> /* When asking for the device path interface, return
>> * bootefi_device_path */
>> .guid = &efi_guid_device_path,
>> - .open = &bootefi_open_dp,
>> + .protocol_interface = bootefi_device_path
>> }
>> },
>> };
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 99619f53a9..c620652307 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -49,15 +49,10 @@ struct efi_class_map {
>> /*
>> * When the UEFI payload wants to open a protocol on an object to
>> get its
>> * interface (usually a struct with callback functions), this struct
>> maps the
>> - * protocol GUID to the respective protocol handler open function for
>> that
>> - * object protocol combination.
>> - */
>> + * protocol GUID to the respective protocol interface */
>> struct efi_handler {
>> const efi_guid_t *guid;
>> - efi_status_t (EFIAPI *open)(void *handle,
>> - efi_guid_t *protocol, void **protocol_interface,
>> - void *agent_handle, void *controller_handle,
>> - uint32_t attributes);
>> + void *protocol_interface;
>> };
>> /*
>> @@ -91,14 +86,6 @@ void efi_smbios_register(void);
>> /* Called by networking code to memorize the dhcp ack package */
>> void efi_net_set_dhcp_ack(void *pkt, int len);
>> -/*
>> - * Stub implementation for a protocol opener that just returns the
>> handle as
>> - * interface
>> - */
>> -efi_status_t EFIAPI efi_return_handle(void *handle,
>> - efi_guid_t *protocol, void **protocol_interface,
>> - void *agent_handle, void *controller_handle,
>> - uint32_t attributes);
>> /* Called from places to check whether a timer expired */
>> void efi_timer_check(void);
>> /* PE loader implementation */
>> diff --git a/lib/efi_loader/efi_boottime.c
>> b/lib/efi_loader/efi_boottime.c
>> index 2ecec60281..0f67dc2037 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -443,7 +443,6 @@ static efi_status_t EFIAPI efi_load_image(bool
>> boot_policy,
>> .protocols = {
>> {
>> .guid = &efi_guid_loaded_image,
>> - .open = &efi_return_handle,
>> },
>> },
>> };
>> @@ -453,6 +452,7 @@ static efi_status_t EFIAPI efi_load_image(bool
>> boot_policy,
>> EFI_ENTRY("%d, %p, %p, %p, %ld, %p", boot_policy, parent_image,
>> file_path, source_buffer, source_size, image_handle);
>> info = malloc(sizeof(*info));
>> + loaded_image_info_obj.protocols[0].protocol_interface = info;
>> obj = malloc(sizeof(loaded_image_info_obj));
>> memset(info, 0, sizeof(*info));
>> memcpy(obj, &loaded_image_info_obj, sizeof(loaded_image_info_obj));
>> @@ -724,6 +724,13 @@ static efi_status_t EFIAPI efi_open_protocol(
>> EFI_ENTRY("%p, %p, %p, %p, %p, 0x%x", handle, protocol,
>> protocol_interface, agent_handle, controller_handle,
>> attributes);
>> +
>> + if (!protocol_interface && attributes !=
>> + EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
>> + r = EFI_INVALID_PARAMETER;
>> + goto out;
>> + }
>> +
>> list_for_each(lhandle, &efi_obj_list) {
>> struct efi_object *efiobj;
>> efiobj = list_entry(lhandle, struct efi_object, link);
>> @@ -737,9 +744,12 @@ static efi_status_t EFIAPI efi_open_protocol(
>> if (!hprotocol)
>> break;
>> if (!guidcmp(hprotocol, protocol)) {
>> - r = handler->open(handle, protocol,
>> - protocol_interface, agent_handle,
>> - controller_handle, attributes);
>> + if (attributes !=
>> + EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
>> + *protocol_interface =
>> + handler->protocol_interface;
>> + }
>> + r = EFI_SUCCESS;
>> goto out;
>> }
>> }
>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> index 4e8e7d0ad6..7f8970496f 100644
>> --- a/lib/efi_loader/efi_disk.c
>> +++ b/lib/efi_loader/efi_disk.c
>> @@ -35,29 +35,6 @@ struct efi_disk_obj {
>> const struct blk_desc *desc;
>> };
>> -static efi_status_t EFIAPI efi_disk_open_block(void *handle,
>> - efi_guid_t *protocol, void **protocol_interface,
>> - void *agent_handle, void *controller_handle,
>> - uint32_t attributes)
>> -{
>> - struct efi_disk_obj *diskobj = handle;
>> -
>> - *protocol_interface = &diskobj->ops;
>> -
>> - return EFI_SUCCESS;
>> -}
>> -
>> -static efi_status_t EFIAPI efi_disk_open_dp(void *handle, efi_guid_t
>> *protocol,
>> - void **protocol_interface, void *agent_handle,
>> - void *controller_handle, uint32_t attributes)
>> -{
>> - struct efi_disk_obj *diskobj = handle;
>> -
>> - *protocol_interface = diskobj->dp;
>> -
>> - return EFI_SUCCESS;
>> -}
>> -
>> static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this,
>> char extended_verification)
>> {
>> @@ -210,10 +187,11 @@ static void efi_disk_add_dev(const char *name,
>> diskobj = calloc(1, objlen);
>> /* Fill in object data */
>> + dp = (void *)&diskobj[1];
>> diskobj->parent.protocols[0].guid = &efi_block_io_guid;
>> - diskobj->parent.protocols[0].open = efi_disk_open_block;
>> + diskobj->parent.protocols[0].protocol_interface = &diskobj->ops;
>> diskobj->parent.protocols[1].guid = &efi_guid_device_path;
>> - diskobj->parent.protocols[1].open = efi_disk_open_dp;
>> + diskobj->parent.protocols[1].protocol_interface = dp;
>> diskobj->parent.handle = diskobj;
>> diskobj->ops = block_io_disk_template;
>> diskobj->ifname = if_typename;
>> @@ -230,7 +208,6 @@ static void efi_disk_add_dev(const char *name,
>> diskobj->ops.media = &diskobj->media;
>> /* Fill in device path */
>> - dp = (void*)&diskobj[1];
>> diskobj->dp = dp;
>> dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>> dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>> diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
>> index 286ad83097..c491ca0815 100644
>> --- a/lib/efi_loader/efi_gop.c
>> +++ b/lib/efi_loader/efi_gop.c
>> @@ -172,7 +172,7 @@ int efi_gop_register(void)
>> /* Fill in object data */
>> gopobj->parent.protocols[0].guid = &efi_gop_guid;
>> - gopobj->parent.protocols[0].open = efi_return_handle;
>> + gopobj->parent.protocols[0].open = &gopobj->ops;
>
> This doesn't compile. I assume you meant .protocol_interface =
> &gopobj->ops? If so, I can fix it up in my tree...
>
>
> Alex
>
>
Yes your analysis was correct.
I should have compiled on a system with CONFIG_LCD for testing.
Best regards
Heinrich Schuchardt
---
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
index c491ca0815..db1fd18a34 100644
--- a/lib/efi_loader/efi_gop.c
+++ b/lib/efi_loader/efi_gop.c
@@ -172,7 +172,7 @@ int efi_gop_register(void)
/* Fill in object data */
gopobj->parent.protocols[0].guid = &efi_gop_guid;
- gopobj->parent.protocols[0].open = &gopobj->ops;
+ gopobj->parent.protocols[0].protocol_interface = &gopobj->ops;
gopobj->parent.handle = &gopobj->ops;
gopobj->ops.query_mode = gop_query_mode;
gopobj->ops.set_mode = gop_set_mode;
--
2.13.2
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
