| Message ID | 20170711200625.7108-2-xypron.glpk@gmx.de |
|---|---|
| State | Accepted |
| Delegated to: | Alexander Graf |
| Headers | show |
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.glpk@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
Hello Alex, I had no problem compiling against efi-next yesterday. It contains the patch. Where can I find the git tree that does not compile? Best regards Heinrich Schuchardt Am 18.07.17 um 15:12 schrieb Alexander Graf > 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.glpk@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
On 07/18/2017 03:30 PM, Heinrich Schuchardt wrote: > Hello Alex, > > I had no problem compiling against efi-next yesterday. It contains the patch. > > Where can I find the git tree that does not compile? https://travis-ci.org/agraf/u-boot/builds/254762424?utm_source=email&utm_medium=notification Alex
On Tue, Jul 11, 2017 at 4:06 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> 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.glpk@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, btw, I probably should have looked at this patchset earlier.. in general, I like it, since it removes a lot of boilerplate. But there are some cases where ->open() allows deferred allocation. For example, I'm working on efi_file and simple-file-system-protocol implementation. A file object can have a device-path accessed by opening device-path protocol. 99% of the time this isn't used, so the ->open() approach lets me defer constructing a file's device-path. How would you feel if I re-introduced ->open() as an optional thing (where the normal case if open is NULL would be to just return protocol_interface)? BR, -R > }, > { > /* > @@ -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; > gopobj->parent.handle = &gopobj->ops; > gopobj->ops.query_mode = gop_query_mode; > gopobj->ops.set_mode = gop_set_mode; > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c > index bc8e04d6d9..749093c656 100644 > --- a/lib/efi_loader/efi_image_loader.c > +++ b/lib/efi_loader/efi_image_loader.c > @@ -18,14 +18,6 @@ DECLARE_GLOBAL_DATA_PTR; > const efi_guid_t efi_guid_device_path = DEVICE_PATH_GUID; > const efi_guid_t efi_guid_loaded_image = LOADED_IMAGE_GUID; > > -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) > -{ > - *protocol_interface = handle; > - return EFI_SUCCESS; > -} > - > static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION *rel, > unsigned long rel_size, void *efi_reloc) > { > diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c > index 604ac6e040..0b949d86e8 100644 > --- a/lib/efi_loader/efi_net.c > +++ b/lib/efi_loader/efi_net.c > @@ -199,30 +199,6 @@ static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this, > return EFI_EXIT(EFI_SUCCESS); > } > > -static efi_status_t EFIAPI efi_net_open_dp(void *handle, efi_guid_t *protocol, > - void **protocol_interface, void *agent_handle, > - void *controller_handle, uint32_t attributes) > -{ > - struct efi_simple_network *net = handle; > - struct efi_net_obj *netobj = container_of(net, struct efi_net_obj, net); > - > - *protocol_interface = &netobj->dp_mac; > - > - return EFI_SUCCESS; > -} > - > -static efi_status_t EFIAPI efi_net_open_pxe(void *handle, efi_guid_t *protocol, > - void **protocol_interface, void *agent_handle, > - void *controller_handle, uint32_t attributes) > -{ > - struct efi_simple_network *net = handle; > - struct efi_net_obj *netobj = container_of(net, struct efi_net_obj, net); > - > - *protocol_interface = &netobj->pxe; > - > - return EFI_SUCCESS; > -} > - > void efi_net_set_dhcp_ack(void *pkt, int len) > { > int maxsize = sizeof(*dhcp_ack); > @@ -258,11 +234,11 @@ int efi_net_register(void **handle) > > /* Fill in object data */ > netobj->parent.protocols[0].guid = &efi_net_guid; > - netobj->parent.protocols[0].open = efi_return_handle; > + netobj->parent.protocols[0].protocol_interface = &netobj->net; > netobj->parent.protocols[1].guid = &efi_guid_device_path; > - netobj->parent.protocols[1].open = efi_net_open_dp; > + netobj->parent.protocols[1].protocol_interface = &netobj->dp_mac; > netobj->parent.protocols[2].guid = &efi_pxe_guid; > - netobj->parent.protocols[2].open = efi_net_open_pxe; > + netobj->parent.protocols[2].protocol_interface = &netobj->pxe; > netobj->parent.handle = &netobj->net; > netobj->net.start = efi_net_start; > netobj->net.stop = efi_net_stop; > -- > 2.11.0 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On 24.07.17 11:48, Rob Clark wrote: > On Tue, Jul 11, 2017 at 4:06 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> 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.glpk@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, > > btw, I probably should have looked at this patchset earlier.. in > general, I like it, since it removes a lot of boilerplate. But there > are some cases where ->open() allows deferred allocation. For > example, I'm working on efi_file and simple-file-system-protocol > implementation. A file object can have a device-path accessed by > opening device-path protocol. 99% of the time this isn't used, so the > ->open() approach lets me defer constructing a file's device-path. > > How would you feel if I re-introduced ->open() as an optional thing > (where the normal case if open is NULL would be to just return > protocol_interface)? Sounds very reasonable to me. I also think that first converting everything to data-driven is a good thing, as it makes sure we have everything that doesn't need to be deferred explicit. So yes, please base an optional open method on top :). Alex
On 07/24/2017 11:48 AM, Rob Clark wrote: > On Tue, Jul 11, 2017 at 4:06 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> 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.glpk@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, > > btw, I probably should have looked at this patchset earlier.. in > general, I like it, since it removes a lot of boilerplate. But there > are some cases where ->open() allows deferred allocation. For > example, I'm working on efi_file and simple-file-system-protocol > implementation. A file object can have a device-path accessed by > opening device-path protocol. 99% of the time this isn't used, so the > ->open() approach lets me defer constructing a file's device-path. > > How would you feel if I re-introduced ->open() as an optional thing > (where the normal case if open is NULL would be to just return > protocol_interface)? > > BR, > -R > Hello Rob, a big gap we currently have in the EFI implementation is the missing handling of lock entries in OpenProtocol which have to be stored per protocol as a list of EFI_OPEN_PROTOCOL_INFORMATION_ENTRYs. Could you, please, elaborate how OpenProtocolInformation, ConnectController, and DisconnectController shall work together with your suggested open() approach. We need be able to iterate efficiently over the EFI_OPEN_PROTOCOL_INFORMATION_ENTRYs in these functions. I fear your proposal will make the code overly complicated here. Best regards Heinrich
On Mon, Jul 24, 2017 at 1:11 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 07/24/2017 11:48 AM, Rob Clark wrote: >> On Tue, Jul 11, 2017 at 4:06 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> 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.glpk@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, >> >> btw, I probably should have looked at this patchset earlier.. in >> general, I like it, since it removes a lot of boilerplate. But there >> are some cases where ->open() allows deferred allocation. For >> example, I'm working on efi_file and simple-file-system-protocol >> implementation. A file object can have a device-path accessed by >> opening device-path protocol. 99% of the time this isn't used, so the >> ->open() approach lets me defer constructing a file's device-path. >> >> How would you feel if I re-introduced ->open() as an optional thing >> (where the normal case if open is NULL would be to just return >> protocol_interface)? >> >> BR, >> -R >> > > > Hello Rob, > > a big gap we currently have in the EFI implementation is the missing > handling of lock entries in OpenProtocol which have to be stored per > protocol as a list of EFI_OPEN_PROTOCOL_INFORMATION_ENTRYs. > > Could you, please, elaborate how OpenProtocolInformation, > ConnectController, and DisconnectController shall work together with > your suggested open() approach. > > We need be able to iterate efficiently over the > EFI_OPEN_PROTOCOL_INFORMATION_ENTRYs in these functions. > > I fear your proposal will make the code overly complicated here. > I guess I'm not too sure what you see the issue would be, but I'm also unfamiliar with what needs to be implemented. The patch I sent just (on first OpenProtocol()) calls ->open() to resolve the actual protocol interface. Afaiu, ->OpenProtocol() always have to be called first. And ofc it is completely optional. It avoids having to up-front create the simple-file-system, and avoids having to construct a device-path for each efi_file object (when it is almost never accessed). BR, -R
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; gopobj->parent.handle = &gopobj->ops; gopobj->ops.query_mode = gop_query_mode; gopobj->ops.set_mode = gop_set_mode; diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index bc8e04d6d9..749093c656 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -18,14 +18,6 @@ DECLARE_GLOBAL_DATA_PTR; const efi_guid_t efi_guid_device_path = DEVICE_PATH_GUID; const efi_guid_t efi_guid_loaded_image = LOADED_IMAGE_GUID; -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) -{ - *protocol_interface = handle; - return EFI_SUCCESS; -} - static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION *rel, unsigned long rel_size, void *efi_reloc) { diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 604ac6e040..0b949d86e8 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -199,30 +199,6 @@ static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this, return EFI_EXIT(EFI_SUCCESS); } -static efi_status_t EFIAPI efi_net_open_dp(void *handle, efi_guid_t *protocol, - void **protocol_interface, void *agent_handle, - void *controller_handle, uint32_t attributes) -{ - struct efi_simple_network *net = handle; - struct efi_net_obj *netobj = container_of(net, struct efi_net_obj, net); - - *protocol_interface = &netobj->dp_mac; - - return EFI_SUCCESS; -} - -static efi_status_t EFIAPI efi_net_open_pxe(void *handle, efi_guid_t *protocol, - void **protocol_interface, void *agent_handle, - void *controller_handle, uint32_t attributes) -{ - struct efi_simple_network *net = handle; - struct efi_net_obj *netobj = container_of(net, struct efi_net_obj, net); - - *protocol_interface = &netobj->pxe; - - return EFI_SUCCESS; -} - void efi_net_set_dhcp_ack(void *pkt, int len) { int maxsize = sizeof(*dhcp_ack); @@ -258,11 +234,11 @@ int efi_net_register(void **handle) /* Fill in object data */ netobj->parent.protocols[0].guid = &efi_net_guid; - netobj->parent.protocols[0].open = efi_return_handle; + netobj->parent.protocols[0].protocol_interface = &netobj->net; netobj->parent.protocols[1].guid = &efi_guid_device_path; - netobj->parent.protocols[1].open = efi_net_open_dp; + netobj->parent.protocols[1].protocol_interface = &netobj->dp_mac; netobj->parent.protocols[2].guid = &efi_pxe_guid; - netobj->parent.protocols[2].open = efi_net_open_pxe; + netobj->parent.protocols[2].protocol_interface = &netobj->pxe; netobj->parent.handle = &netobj->net; netobj->net.start = efi_net_start; netobj->net.stop = efi_net_stop;
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.glpk@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(-)