[U-Boot] [PATCH v2 00/12] efi_loader: protocol services

Currently only a small fraction of the UEFI specification is implemented in U-Boot. It is sufficient to load grub but it fails on iPXE.
With this patch series all EFI protocol services are implemented that are needed to reach the console prompt of the bin-<ARCH>-efi/snponly.efi target of iPXE.
Futher patches for event services are needed to use the iPXE console. These will be provided in a separate patch series.
The 1st patch refactors efi_open_protocol to enable the implementation of InstallProtocolInterface. This patch eliminates the open protocol functions in efi_disk, efi_gop, and efi_net.
The 2nd patch adds missing parameter checks to efi_open_protocol.
The 3rd patch implements InstallProtocolInteface.
The 4th patch partially implements UninstallProtocolInterface.
The 5th patch refactors efi_install_protocol_interface to make it callable internally.
The 6th patch refactors efi_uninstall_protocol_interface to make it callable internally.
The 7th patch implements InstallMultipleProtocolInterfaces by calling efi_install_protocol_interface.
The 8th patch refactors efi_locate_handle to make it callable internally.
The 9th patch implements LocateHandleBuffer.
The 10th patch increases the number of protocols per handle.
The 11th patch reimplements efi_locate_protocol.
The 12th patch implements the device path to text protocol.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
--- v2 Remove implementation of efi_return_handle in 1st patch. Correctly check GUIDs in 3rd patch. Check if protocol != NULL to avoid illegal memory access in 4th patch. Add patches 10-12. ---
Heinrich Schuchardt (12): efi_loader: refactor efi_open_protocol efi_loader: efi_open_protocol: parameter checks efi_loader: implement InstallProtocolInterface efi_loader: implement UninstallProtocolInterface efi_loader: refactor efi_install_protocol_interface efi_loader: refactor efi_uninstall_protocol_interface efi_loader: implement InstallMultipleProtocolInterfaces efi_loader: refactor efi_locate_handle efi_loader: implement LocateHandleBuffer efi_loader: provide a sufficient number of protocols efi_loader: reimplement efi_locate_protocol efi_loader: implement EFI_DEVICE_PATH_TO_TEXT_PROTOCOL
cmd/bootefi.c | 24 +-- include/efi_api.h | 26 ++- include/efi_loader.h | 33 +--- lib/efi_loader/Makefile | 2 +- lib/efi_loader/efi_boottime.c | 261 +++++++++++++++++++++++++++---- lib/efi_loader/efi_device_path_to_text.c | 68 ++++++++ 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 +--- 10 files changed, 350 insertions(+), 133 deletions(-) create mode 100644 lib/efi_loader/efi_device_path_to_text.c

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; 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;

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.
- */
struct efi_handler { const efi_guid_t *guid;
- protocol GUID to the respective protocol interface */
- 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,
/* Called from places to check whether a timer expired */ void efi_timer_check(void); /* PE loader implementation */uint32_t attributes);
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.
- */
struct efi_handler { const efi_guid_t *guid;
- protocol GUID to the respective protocol interface */
- 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,
/* Called from places to check whether a timer expired */ void efi_timer_check(void); /* PE loader implementation */uint32_t attributes);
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...
Alex

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.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.
- */
struct efi_handler { const efi_guid_t *guid;
- protocol GUID to the respective protocol interface */
- 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,
/* Called from places to check whether a timer expired */ void efi_timer_check(void); /* PE loader implementation */uint32_t attributes);
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

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

Add all parameter checks for function efi_open_protocol that do not depend on a locking table.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 no change --- lib/efi_loader/efi_boottime.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 5c72f92474..22e9e6001d 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -718,15 +718,35 @@ static efi_status_t EFIAPI efi_open_protocol( { struct list_head *lhandle; int i; - efi_status_t r = EFI_UNSUPPORTED; + efi_status_t r = EFI_INVALID_PARAMETER;
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; + if (!handle || !protocol || + (!protocol_interface && attributes != + EFI_OPEN_PROTOCOL_TEST_PROTOCOL)) { + goto out; + } + + switch (attributes) { + case EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL: + case EFI_OPEN_PROTOCOL_GET_PROTOCOL: + case EFI_OPEN_PROTOCOL_TEST_PROTOCOL: + break; + case EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER: + if (controller_handle == handle) + goto out; + case EFI_OPEN_PROTOCOL_BY_DRIVER: + case EFI_OPEN_PROTOCOL_BY_DRIVER | EFI_OPEN_PROTOCOL_EXCLUSIVE: + if (controller_handle == NULL) + goto out; + case EFI_OPEN_PROTOCOL_EXCLUSIVE: + if (agent_handle == NULL) + goto out; + break; + default: goto out; }
@@ -752,8 +772,11 @@ static efi_status_t EFIAPI efi_open_protocol( goto out; } } + goto unsupported; }
+unsupported: + r = EFI_UNSUPPORTED; out: return EFI_EXIT(r); }

efi_install_protocol_interface up to now only returned an error code.
The patch implements the UEFI specification for InstallProtocolInterface with the exception that it will not create new handles.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 Correctly check GUIDs. --- include/efi_api.h | 2 +- lib/efi_loader/efi_boottime.c | 54 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/include/efi_api.h b/include/efi_api.h index f071b36b53..42cd47ff08 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -58,7 +58,7 @@ struct efi_boot_services { efi_status_t (EFIAPI *signal_event)(void *event); efi_status_t (EFIAPI *close_event)(void *event); efi_status_t (EFIAPI *check_event)(void *event); - +#define EFI_NATIVE_INTERFACE 0x00000000 efi_status_t (EFIAPI *install_protocol_interface)( void **handle, efi_guid_t *protocol, int protocol_interface_type, void *protocol_interface); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index f11aa551d2..97b2dce8b3 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -304,10 +304,62 @@ static efi_status_t EFIAPI efi_install_protocol_interface(void **handle, efi_guid_t *protocol, int protocol_interface_type, void *protocol_interface) { + struct list_head *lhandle; + int i; + efi_status_t r; + EFI_ENTRY("%p, %p, %d, %p", handle, protocol, protocol_interface_type, protocol_interface); - return EFI_EXIT(EFI_OUT_OF_RESOURCES); + + if (!handle || !protocol || + protocol_interface_type != EFI_NATIVE_INTERFACE) { + r = EFI_INVALID_PARAMETER; + goto out; + } + + /* Create new handle if requested. */ + if (!*handle) { + r = EFI_OUT_OF_RESOURCES; + goto out; + } + /* Find object. */ + list_for_each(lhandle, &efi_obj_list) { + struct efi_object *efiobj; + efiobj = list_entry(lhandle, struct efi_object, link); + + if (efiobj->handle != *handle) + continue; + /* Check if protocol is already installed on the handle. */ + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { + struct efi_handler *handler = &efiobj->protocols[i]; + + if (!handler->guid) + continue; + if (!guidcmp(handler->guid, protocol)) { + r = EFI_INVALID_PARAMETER; + goto out; + } + } + /* Install protocol in first empty slot. */ + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { + struct efi_handler *handler = &efiobj->protocols[i]; + + if (handler->guid) + continue; + + handler->guid = protocol; + handler->protocol_interface = protocol_interface; + r = EFI_SUCCESS; + goto out; + } + r = EFI_OUT_OF_RESOURCES; + goto out; + } + r = EFI_INVALID_PARAMETER; +out: + return EFI_EXIT(r); } + static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle, efi_guid_t *protocol, void *old_interface, void *new_interface)

Without the patch efi_uninstall_protocol_interface always returns an error.
With the patch protocols without interface can be uninstalled.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 Check if protocol != NULL to avoid illegal memory access. --- lib/efi_loader/efi_boottime.c | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 97b2dce8b3..6c64b94e4a 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -372,8 +372,44 @@ static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle, static efi_status_t EFIAPI efi_uninstall_protocol_interface(void *handle, efi_guid_t *protocol, void *protocol_interface) { + struct list_head *lhandle; + int i; + efi_status_t r = EFI_NOT_FOUND; + EFI_ENTRY("%p, %p, %p", handle, protocol, protocol_interface); - return EFI_EXIT(EFI_NOT_FOUND); + + if (!handle || !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); + + if (efiobj->handle != handle) + continue; + + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { + struct efi_handler *handler = &efiobj->protocols[i]; + const efi_guid_t *hprotocol = handler->guid; + + if (!hprotocol) + continue; + if (!guidcmp(hprotocol, protocol)) { + if (handler->protocol_interface) { + r = EFI_ACCESS_DENIED; + } else { + handler->guid = 0; + r = EFI_SUCCESS; + } + goto out; + } + } + } + +out: + return EFI_EXIT(r); }
static efi_status_t EFIAPI efi_register_protocol_notify(efi_guid_t *protocol, @@ -814,7 +850,7 @@ static efi_status_t EFIAPI efi_open_protocol( struct efi_handler *handler = &efiobj->protocols[i]; const efi_guid_t *hprotocol = handler->guid; if (!hprotocol) - break; + continue; if (!guidcmp(hprotocol, protocol)) { if (attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {

For the implementation of InstallMultipleProtocolInterfaces we need to call efi_install_protocol_interface. In internal calls we should not pass through EFI_EXIT.
The patch introduces a wrapper function efi_install_protocol_interface_ext.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 no change --- lib/efi_loader/efi_boottime.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b3a59b4219..0075ccaaa2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -307,9 +307,6 @@ static efi_status_t EFIAPI efi_install_protocol_interface(void **handle, int i; efi_status_t r;
- EFI_ENTRY("%p, %p, %d, %p", handle, protocol, protocol_interface_type, - protocol_interface); - if (!handle || !protocol || protocol_interface_type != EFI_NATIVE_INTERFACE) { r = EFI_INVALID_PARAMETER; @@ -354,7 +351,19 @@ static efi_status_t EFIAPI efi_install_protocol_interface(void **handle, } r = EFI_INVALID_PARAMETER; out: - return EFI_EXIT(r); + return r; +} + +static efi_status_t EFIAPI efi_install_protocol_interface_ext(void **handle, + efi_guid_t *protocol, int protocol_interface_type, + void *protocol_interface) +{ + EFI_ENTRY("%p, %p, %d, %p", handle, protocol, protocol_interface_type, + protocol_interface); + + return EFI_EXIT(efi_install_protocol_interface(handle, protocol, + protocol_interface_type, + protocol_interface)); }
static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle, @@ -889,7 +898,7 @@ static const struct efi_boot_services efi_boot_services = { .signal_event = efi_signal_event, .close_event = efi_close_event, .check_event = efi_check_event, - .install_protocol_interface = efi_install_protocol_interface, + .install_protocol_interface = efi_install_protocol_interface_ext, .reinstall_protocol_interface = efi_reinstall_protocol_interface, .uninstall_protocol_interface = efi_uninstall_protocol_interface, .handle_protocol = efi_handle_protocol,

For the implementation of UninstallMultipleProtocolInterfaces we need to call efi_uninstall_protocol_interface. In internal calls we should not pass through EFI_EXIT.
The patch introduces a wrapper function efi_uninstall_protocol_interface_ext.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 no change --- lib/efi_loader/efi_boottime.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 0075ccaaa2..607287d01f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -382,8 +382,6 @@ static efi_status_t EFIAPI efi_uninstall_protocol_interface(void *handle, int i; efi_status_t r = EFI_NOT_FOUND;
- EFI_ENTRY("%p, %p, %p", handle, protocol, protocol_interface); - if (!handle || !protocol) { r = EFI_INVALID_PARAMETER; goto out; @@ -412,7 +410,16 @@ static efi_status_t EFIAPI efi_uninstall_protocol_interface(void *handle, }
out: - return EFI_EXIT(r); + return r; +} + +static efi_status_t EFIAPI efi_uninstall_protocol_interface_ext(void *handle, + efi_guid_t *protocol, void *protocol_interface) +{ + EFI_ENTRY("%p, %p, %p", handle, protocol, protocol_interface); + + return EFI_EXIT(efi_uninstall_protocol_interface(handle, protocol, + protocol_interface)); }
static efi_status_t EFIAPI efi_register_protocol_notify(efi_guid_t *protocol, @@ -900,7 +907,7 @@ static const struct efi_boot_services efi_boot_services = { .check_event = efi_check_event, .install_protocol_interface = efi_install_protocol_interface_ext, .reinstall_protocol_interface = efi_reinstall_protocol_interface, - .uninstall_protocol_interface = efi_uninstall_protocol_interface, + .uninstall_protocol_interface = efi_uninstall_protocol_interface_ext, .handle_protocol = efi_handle_protocol, .reserved = NULL, .register_protocol_notify = efi_register_protocol_notify,

Implement InstallMultipleProtocolInterfaces in function efi_install_multiple_protocol_interfaces by repeatedly calling efi_install_protocol_interface.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 no change --- lib/efi_loader/efi_boottime.c | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 607287d01f..74b9620ad8 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -778,7 +778,44 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( void **handle, ...) { EFI_ENTRY("%p", handle); - return EFI_EXIT(EFI_OUT_OF_RESOURCES); + + va_list argptr; + efi_guid_t *protocol; + void *protocol_interface; + efi_status_t r = EFI_SUCCESS; + int i = 0; + + if (!handle) + return EFI_EXIT(EFI_INVALID_PARAMETER); + + va_start(argptr, handle); + for (;;) { + protocol = va_arg(argptr, efi_guid_t*); + if (!protocol) + break; + protocol_interface = va_arg(argptr, void*); + r = efi_install_protocol_interface(handle, protocol, + EFI_NATIVE_INTERFACE, + protocol_interface); + if (r != EFI_SUCCESS) + break; + i++; + } + va_end(argptr); + if (r == EFI_SUCCESS) + return EFI_EXIT(r); + + /* If an error occured undo all changes. */ + va_start(argptr, handle); + for (; i; --i) { + protocol = va_arg(argptr, efi_guid_t*); + protocol_interface = va_arg(argptr, void*); + efi_uninstall_protocol_interface(handle, protocol, + protocol_interface); + } + va_end(argptr); + + return EFI_EXIT(r); }
static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces(

To implement LocateHandleBuffer we need to call efi_locate_handle internally without running through EFI_EXIT.
So put EFI_ENTRY and EFI_EXIT into a new wrapper function efi_locate_handle_ext.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 no change --- lib/efi_loader/efi_boottime.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 74b9620ad8..888207681c 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -461,9 +461,6 @@ static efi_status_t EFIAPI efi_locate_handle( struct list_head *lhandle; unsigned long size = 0;
- EFI_ENTRY("%d, %p, %p, %p, %p", search_type, protocol, search_key, - buffer_size, buffer); - /* Count how much space we need */ list_for_each(lhandle, &efi_obj_list) { struct efi_object *efiobj; @@ -475,7 +472,7 @@ static efi_status_t EFIAPI efi_locate_handle(
if (*buffer_size < size) { *buffer_size = size; - return EFI_EXIT(EFI_BUFFER_TOO_SMALL); + return EFI_BUFFER_TOO_SMALL; }
/* Then fill the array */ @@ -488,7 +485,19 @@ static efi_status_t EFIAPI efi_locate_handle( }
*buffer_size = size; - return EFI_EXIT(EFI_SUCCESS); + return EFI_SUCCESS; +} + +static efi_status_t EFIAPI efi_locate_handle_ext( + enum efi_locate_search_type search_type, + efi_guid_t *protocol, void *search_key, + unsigned long *buffer_size, efi_handle_t *buffer) +{ + EFI_ENTRY("%d, %p, %p, %p, %p", search_type, protocol, search_key, + buffer_size, buffer); + + return EFI_EXIT(efi_locate_handle(search_type, protocol, search_key, + buffer_size, buffer)); }
static efi_status_t EFIAPI efi_locate_device_path(efi_guid_t *protocol, @@ -948,7 +957,7 @@ static const struct efi_boot_services efi_boot_services = { .handle_protocol = efi_handle_protocol, .reserved = NULL, .register_protocol_notify = efi_register_protocol_notify, - .locate_handle = efi_locate_handle, + .locate_handle = efi_locate_handle_ext, .locate_device_path = efi_locate_device_path, .install_configuration_table = efi_install_configuration_table_ext, .load_image = efi_load_image,

UEFI boot service LocateHandleBuffer is implemented by calling efi_allocate_handle and efi_locate_handle.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 no change --- lib/efi_loader/efi_boottime.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 888207681c..3060c25a2a 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -753,9 +753,32 @@ static efi_status_t EFIAPI efi_locate_handle_buffer( efi_guid_t *protocol, void *search_key, unsigned long *no_handles, efi_handle_t **buffer) { + efi_status_t r; + unsigned long buffer_size = 0; + EFI_ENTRY("%d, %p, %p, %p, %p", search_type, protocol, search_key, no_handles, buffer); - return EFI_EXIT(EFI_NOT_FOUND); + + if (!no_handles || !buffer) { + r = EFI_INVALID_PARAMETER; + goto out; + } + *no_handles = 0; + *buffer = NULL; + r = efi_locate_handle(search_type, protocol, search_key, &buffer_size, + *buffer); + if (r != EFI_BUFFER_TOO_SMALL) + goto out; + r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, buffer_size, + (void **)buffer); + if (r != EFI_SUCCESS) + goto out; + r = efi_locate_handle(search_type, protocol, search_key, &buffer_size, + *buffer); + if (r == EFI_SUCCESS) + *no_handles = buffer_size / sizeof(void *); +out: + return EFI_EXIT(r); }
static struct efi_class_map efi_class_maps[] = {

Four protocols per object is too few to run iPXE.
Let's raise the number of protocols per object to eight.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 new patch --- include/efi_loader.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index c620652307..989e5809ba 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -65,8 +65,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 4 "protocols" an object can be accessed through */ - struct efi_handler protocols[4]; + /* We support up to 8 "protocols" an object can be accessed through */ + struct efi_handler protocols[8]; /* The object spawner can either use this for data or as identifier */ void *handle; };

The UEFI specification requires that LocateProtol finds the first handle supporting the protocol and to return a pointer to its interface.
So we have to assign the protocols to an efi_object and not use any separate storage.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 new patch --- cmd/bootefi.c | 4 ++++ include/efi_loader.h | 10 ---------- lib/efi_loader/efi_boottime.c | 32 ++++++++++++++++++++------------ 3 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index d80cb0723f..0bac05f1c6 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -80,6 +80,10 @@ static struct efi_object loaded_image_info_obj = { .guid = &efi_guid_device_path, .protocol_interface = bootefi_device_path, }, + { + .guid = &efi_guid_console_control, + .protocol_interface = (void *) &efi_console_control + }, }, };
diff --git a/include/efi_loader.h b/include/efi_loader.h index 989e5809ba..6ea6e9ee4d 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -37,16 +37,6 @@ extern unsigned int __efi_runtime_start, __efi_runtime_stop; extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
/* - * While UEFI objects can have callbacks, you can also call functions on - * protocols (classes) themselves. This struct maps a protocol GUID to its - * interface (usually a struct with callback functions). - */ -struct efi_class_map { - const efi_guid_t *guid; - const void *interface; -}; - -/* * 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 interface */ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 48c606d0b7..bf277bd5a3 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -787,27 +787,35 @@ out: return EFI_EXIT(r); }
-static struct efi_class_map efi_class_maps[] = { - { - .guid = &efi_guid_console_control, - .interface = &efi_console_control - }, -}; - static efi_status_t EFIAPI efi_locate_protocol(efi_guid_t *protocol, void *registration, void **protocol_interface) { + struct list_head *lhandle; int i;
EFI_ENTRY("%p, %p, %p", protocol, registration, protocol_interface); - for (i = 0; i < ARRAY_SIZE(efi_class_maps); i++) { - struct efi_class_map *curmap = &efi_class_maps[i]; - if (!guidcmp(protocol, curmap->guid)) { - *protocol_interface = (void*)curmap->interface; - return EFI_EXIT(EFI_SUCCESS); + + if (!protocol || !protocol_interface) + return EFI_EXIT(EFI_INVALID_PARAMETER); + + list_for_each(lhandle, &efi_obj_list) { + struct efi_object *efiobj; + + efiobj = list_entry(lhandle, struct efi_object, link); + for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) { + struct efi_handler *handler = &efiobj->protocols[i]; + + if (!handler->guid) + continue; + if (!guidcmp(handler->guid, protocol)) { + *protocol_interface = + handler->protocol_interface; + return EFI_EXIT(EFI_SUCCESS); + } } } + *protocol_interface = NULL;
return EFI_EXIT(EFI_NOT_FOUND); }

ConvertPathToText is implemented for * type 4 - media device path * subtype 4 - file path
This is the kind of device path we hand out for block devices.
All other cases may be implemented later.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 new patch --- cmd/bootefi.c | 4 ++ include/efi_api.h | 24 +++++++++++ include/efi_loader.h | 2 + lib/efi_loader/Makefile | 2 +- lib/efi_loader/efi_device_path_to_text.c | 68 ++++++++++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 lib/efi_loader/efi_device_path_to_text.c
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 4e73059cb3..d1e782cfe9 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -84,6 +84,10 @@ static struct efi_object loaded_image_info_obj = { .guid = &efi_guid_console_control, .protocol_interface = (void *) &efi_console_control }, + { + .guid = &efi_guid_device_path_to_text_protocol, + .protocol_interface = (void *) &efi_device_path_to_text + }, }, };
diff --git a/include/efi_api.h b/include/efi_api.h index 42cd47ff08..ea63e80b47 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -395,6 +395,30 @@ struct efi_console_control_protocol uint16_t *password); };
+#define EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID \ + EFI_GUID(0x8b843e20, 0x8132, 0x4852, \ + 0x90, 0xcc, 0x55, 0x1a, 0x4e, 0x4a, 0x7f, 0x1c) + +struct efi_device_path_protocol +{ + uint8_t type; + uint8_t sub_type; + uint16_t length; + uint8_t data[]; +}; + +struct efi_device_path_to_text_protocol +{ + uint16_t *(EFIAPI *convert_device_node_to_text)( + struct efi_device_path_protocol *device_node, + bool display_only, + bool allow_shortcuts); + uint16_t *(EFIAPI *convert_device_path_to_text)( + struct efi_device_path_protocol *device_path, + bool display_only, + bool allow_shortcuts); +}; + #define EFI_GOP_GUID \ EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \ 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a) diff --git a/include/efi_loader.h b/include/efi_loader.h index 6ea6e9ee4d..d7847d23e5 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -28,10 +28,12 @@ extern struct efi_system_table systab; extern const struct efi_simple_text_output_protocol efi_con_out; extern const struct efi_simple_input_interface efi_con_in; extern const struct efi_console_control_protocol efi_console_control; +extern const struct efi_device_path_to_text_protocol efi_device_path_to_text;
extern const efi_guid_t efi_guid_console_control; extern const efi_guid_t efi_guid_device_path; extern const efi_guid_t efi_guid_loaded_image; +extern const efi_guid_t efi_guid_device_path_to_text_protocol;
extern unsigned int __efi_runtime_start, __efi_runtime_stop; extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index fa8b91a526..3fc2371896 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -15,7 +15,7 @@ always := $(efiprogs-y)
obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o -obj-y += efi_memory.o +obj-y += efi_memory.o efi_device_path_to_text.o obj-$(CONFIG_LCD) += efi_gop.o obj-$(CONFIG_PARTITIONS) += efi_disk.o obj-$(CONFIG_NET) += efi_net.o diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c new file mode 100644 index 0000000000..d6d2e3c35f --- /dev/null +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -0,0 +1,68 @@ +/* + * EFI device path interface + * + * Copyright (c) 2017 Heinrich Schuchardt + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <efi_loader.h> + +#define MEDIA_DEVICE_PATH 4 +#define FILE_PATH_MEDIA_DEVICE_PATH 4 + +const efi_guid_t efi_guid_device_path_to_text_protocol = + EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; + +uint16_t *efi_convert_device_node_to_text( + struct efi_device_path_protocol *device_node, + bool display_only, + bool allow_shortcuts) { + + EFI_ENTRY("%p, %d, %d", device_node, display_only, allow_shortcuts); + + EFI_EXIT(EFI_UNSUPPORTED); + return NULL; +} + +uint16_t *efi_convert_device_path_to_text( + struct efi_device_path_protocol *device_path, + bool display_only, + bool allow_shortcuts) +{ + EFI_ENTRY("%p, %d, %d", device_path, display_only, allow_shortcuts); + + unsigned long buffer_size; + efi_status_t r; + uint16_t *buffer = NULL; + + switch (device_path->type) { + case MEDIA_DEVICE_PATH: + switch (device_path->sub_type) { + case FILE_PATH_MEDIA_DEVICE_PATH: + buffer_size = device_path->length - 4; + r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, + buffer_size, (void **) &buffer); + if (r == EFI_SUCCESS) + memcpy(buffer, device_path->data, buffer_size); + break; + + } + } + + if (buffer) { + EFI_EXIT(EFI_SUCCESS); + } else { + debug("type %d, subtype %d\n", + device_path->type, device_path->sub_type); + EFI_EXIT(EFI_UNSUPPORTED); + } + + return buffer; +} + +const struct efi_device_path_to_text_protocol efi_device_path_to_text = { + .convert_device_node_to_text = efi_convert_device_node_to_text, + .convert_device_path_to_text = efi_convert_device_path_to_text, +};
participants (3)
-
Alexander Graf
-
Heinrich Schuchardt
-
Rob Clark