[PATCH 0/2] efi_loader: internal CloseProtocol

[PATCH 0/2] efi_loader: internal CloseProtocol

[Heinrich Schuchardt]

Get rid of EFI_CALL() for invoking CloseProtocol().

Heinrich Schuchardt (2):
  efi_loader: internal CloseProtocol
  efi_driver: use efi_close_protocol

 include/efi_loader.h          |  9 ++--
 lib/efi_driver/efi_uclass.c   | 25 +++++-----
 lib/efi_loader/efi_boottime.c | 90 ++++++++++++++++++++++-------------
 3 files changed, 72 insertions(+), 52 deletions(-)

-- 
2.37.2

[PATCH 1/2] efi_loader: internal CloseProtocol

[Heinrich Schuchardt]

Allow avoiding using EFI_CALL() when closing a protocol by providing an
internal function.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 include/efi_loader.h          |  9 ++--
 lib/efi_loader/efi_boottime.c | 90 ++++++++++++++++++++++-------------
 2 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index c70d977298..1bac3f49a3 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -661,11 +661,10 @@ efi_status_t EFIAPI efi_locate_handle_buffer(
 			enum efi_locate_search_type search_type,
 			const efi_guid_t *protocol, void *search_key,
 			efi_uintn_t *no_handles, efi_handle_t **buffer);
-/* Close an previously opened protocol interface */
-efi_status_t EFIAPI efi_close_protocol(efi_handle_t handle,
-				       const efi_guid_t *protocol,
-				       efi_handle_t agent_handle,
-				       efi_handle_t controller_handle);
+/* Close a previously opened protocol interface */
+efi_status_t efi_close_protocol(efi_handle_t handle, const efi_guid_t *protocol,
+				efi_handle_t agent_handle,
+				efi_handle_t controller_handle);
 /* Open a protocol interface */
 efi_status_t EFIAPI efi_handle_protocol(efi_handle_t handle,
 					const efi_guid_t *protocol,
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index e776d25830..9151aa771a 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1993,7 +1993,7 @@ efi_status_t efi_load_image_from_path(bool boot_policy,
 	if (ret != EFI_SUCCESS)
 		efi_free_pages(addr, pages);
 out:
-	EFI_CALL(efi_close_protocol(device, guid, efi_root, NULL));
+	efi_close_protocol(device, guid, efi_root, NULL);
 	if (ret == EFI_SUCCESS) {
 		*buffer = (void *)(uintptr_t)addr;
 		*size = buffer_size;
@@ -2290,45 +2290,70 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
  * @agent_handle:      handle of the driver
  * @controller_handle: handle of the controller
  *
- * This function implements the CloseProtocol service.
+ * This is the function implementing the CloseProtocol service is for internal
+ * usage in U-Boot. For API usage wrapper efi_close_protocol_ext() is provided.
  *
  * See the Unified Extensible Firmware Interface (UEFI) specification for
  * details.
  *
  * Return: status code
  */
-efi_status_t EFIAPI efi_close_protocol(efi_handle_t handle,
-				       const efi_guid_t *protocol,
-				       efi_handle_t agent_handle,
-				       efi_handle_t controller_handle)
+efi_status_t efi_close_protocol(efi_handle_t handle, const efi_guid_t *protocol,
+				efi_handle_t agent_handle,
+				efi_handle_t controller_handle)
 {
 	struct efi_handler *handler;
 	struct efi_open_protocol_info_item *item;
 	struct efi_open_protocol_info_item *pos;
-	efi_status_t r;
-
-	EFI_ENTRY("%p, %pUs, %p, %p", handle, protocol, agent_handle,
-		  controller_handle);
+	efi_status_t ret;
 
 	if (!efi_search_obj(agent_handle) ||
-	    (controller_handle && !efi_search_obj(controller_handle))) {
-		r = EFI_INVALID_PARAMETER;
-		goto out;
-	}
-	r = efi_search_protocol(handle, protocol, &handler);
-	if (r != EFI_SUCCESS)
-		goto out;
+	    (controller_handle && !efi_search_obj(controller_handle)))
+		return EFI_INVALID_PARAMETER;
+	ret = efi_search_protocol(handle, protocol, &handler);
+	if (ret != EFI_SUCCESS)
+		return ret;
 
-	r = EFI_NOT_FOUND;
+	ret = EFI_NOT_FOUND;
 	list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
 		if (item->info.agent_handle == agent_handle &&
 		    item->info.controller_handle == controller_handle) {
 			efi_delete_open_info(item);
-			r = EFI_SUCCESS;
+			ret = EFI_SUCCESS;
 		}
 	}
-out:
-	return EFI_EXIT(r);
+
+	return ret;
+}
+
+/**
+ * efi_close_protocol_ext() - close a protocol
+ * @handle:            handle on which the protocol shall be closed
+ * @protocol:          GUID of the protocol to close
+ * @agent_handle:      handle of the driver
+ * @controller_handle: handle of the controller
+ *
+ * This function implements the CloseProtocol service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * Return: status code
+ */
+efi_status_t EFIAPI efi_close_protocol_ext(efi_handle_t handle,
+					   const efi_guid_t *protocol,
+					   efi_handle_t agent_handle,
+					   efi_handle_t controller_handle)
+{
+	efi_status_t ret;
+
+	EFI_ENTRY("%p, %pUs, %p, %p", handle, protocol, agent_handle,
+		  controller_handle);
+
+	ret = efi_close_protocol(handle, protocol,
+				 agent_handle, controller_handle);
+
+	return EFI_EXIT(ret);
 }
 
 /**
@@ -3213,11 +3238,10 @@ close_next:
 				if (info->info.agent_handle !=
 				    (efi_handle_t)image_obj)
 					continue;
-				r = EFI_CALL(efi_close_protocol
-						(efiobj, &protocol->guid,
-						 info->info.agent_handle,
-						 info->info.controller_handle
-						));
+				r = efi_close_protocol(
+						efiobj, &protocol->guid,
+						info->info.agent_handle,
+						info->info.controller_handle);
 				if (r !=  EFI_SUCCESS)
 					ret = r;
 				/*
@@ -3481,9 +3505,9 @@ static efi_status_t efi_bind_controller(
 		r = EFI_CALL(binding_protocol->start(binding_protocol,
 						     controller_handle,
 						     remain_device_path));
-	EFI_CALL(efi_close_protocol(driver_image_handle,
-				    &efi_guid_driver_binding_protocol,
-				    driver_image_handle, NULL));
+	efi_close_protocol(driver_image_handle,
+			   &efi_guid_driver_binding_protocol,
+			   driver_image_handle, NULL);
 	return r;
 }
 
@@ -3834,9 +3858,9 @@ static efi_status_t EFIAPI efi_disconnect_controller(
 			goto out;
 		}
 	}
-	EFI_CALL(efi_close_protocol(driver_image_handle,
-				    &efi_guid_driver_binding_protocol,
-				    driver_image_handle, NULL));
+	efi_close_protocol(driver_image_handle,
+			   &efi_guid_driver_binding_protocol,
+			   driver_image_handle, NULL);
 	r = EFI_SUCCESS;
 out:
 	if (!child_handle)
@@ -3883,7 +3907,7 @@ static struct efi_boot_services efi_boot_services = {
 	.connect_controller = efi_connect_controller,
 	.disconnect_controller = efi_disconnect_controller,
 	.open_protocol = efi_open_protocol,
-	.close_protocol = efi_close_protocol,
+	.close_protocol = efi_close_protocol_ext,
 	.open_protocol_information = efi_open_protocol_information,
 	.protocols_per_handle = efi_protocols_per_handle,
 	.locate_handle_buffer = efi_locate_handle_buffer,
-- 
2.37.2

[PATCH 2/2] efi_driver: use efi_close_protocol

[Heinrich Schuchardt]

Avoid EFI_CALL() by using efi_close_protocol().

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/efi_driver/efi_uclass.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index 45610d1aa3..45f9351988 100644
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -97,10 +97,9 @@ static efi_status_t EFIAPI efi_uc_supported(
 
 	ret = check_node_type(controller_handle);
 
-	r = EFI_CALL(systab.boottime->close_protocol(
-				controller_handle, bp->ops->protocol,
-				this->driver_binding_handle,
-				controller_handle));
+	r = efi_close_protocol(controller_handle, bp->ops->protocol,
+			       this->driver_binding_handle,
+			       controller_handle);
 	if (r != EFI_SUCCESS)
 		ret = EFI_UNSUPPORTED;
 out:
@@ -151,10 +150,9 @@ static efi_status_t EFIAPI efi_uc_start(
 		goto out;
 
 err:
-	r = EFI_CALL(systab.boottime->close_protocol(
-			controller_handle, bp->ops->protocol,
-			this->driver_binding_handle,
-			controller_handle));
+	r = efi_close_protocol(controller_handle, bp->ops->protocol,
+			       this->driver_binding_handle,
+			       controller_handle);
 	if (r != EFI_SUCCESS)
 		EFI_PRINT("Failure to close handle\n");
 
@@ -177,9 +175,8 @@ static efi_status_t disconnect_child(efi_handle_t controller_handle,
 	efi_guid_t *guid_controller = NULL;
 	efi_guid_t *guid_child_controller = NULL;
 
-	ret = EFI_CALL(systab.boottime->close_protocol(
-				controller_handle, guid_controller,
-				child_handle, child_handle));
+	ret = efi_close_protocol(controller_handle, guid_controller,
+				 child_handle, child_handle);
 	if (ret != EFI_SUCCESS) {
 		EFI_PRINT("Cannot close protocol\n");
 		return ret;
@@ -252,9 +249,9 @@ static efi_status_t EFIAPI efi_uc_stop(
 		log_err("Cannot free EFI memory pool\n");
 
 	/* Detach driver from controller */
-	ret = EFI_CALL(systab.boottime->close_protocol(
-			controller_handle, bp->ops->protocol,
-			this->driver_binding_handle, controller_handle));
+	ret = efi_close_protocol(controller_handle, bp->ops->protocol,
+				 this->driver_binding_handle,
+				 controller_handle);
 out:
 	return EFI_EXIT(ret);
 }
-- 
2.37.2

Re: [PATCH 1/2] efi_loader: internal CloseProtocol

[Ilias Apalodimas]

On Sat, Oct 08, 2022 at 09:14:41AM +0200, Heinrich Schuchardt wrote:
> Allow avoiding using EFI_CALL() when closing a protocol by providing an
> internal function.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  include/efi_loader.h          |  9 ++--
>  lib/efi_loader/efi_boottime.c | 90 ++++++++++++++++++++++-------------
>  2 files changed, 61 insertions(+), 38 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index c70d977298..1bac3f49a3 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -661,11 +661,10 @@ efi_status_t EFIAPI efi_locate_handle_buffer(
>  			enum efi_locate_search_type search_type,
>  			const efi_guid_t *protocol, void *search_key,
>  			efi_uintn_t *no_handles, efi_handle_t **buffer);
> -/* Close an previously opened protocol interface */
> -efi_status_t EFIAPI efi_close_protocol(efi_handle_t handle,
> -				       const efi_guid_t *protocol,
> -				       efi_handle_t agent_handle,
> -				       efi_handle_t controller_handle);
> +/* Close a previously opened protocol interface */
> +efi_status_t efi_close_protocol(efi_handle_t handle, const efi_guid_t *protocol,
> +				efi_handle_t agent_handle,
> +				efi_handle_t controller_handle);
>  /* Open a protocol interface */
>  efi_status_t EFIAPI efi_handle_protocol(efi_handle_t handle,
>  					const efi_guid_t *protocol,
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index e776d25830..9151aa771a 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1993,7 +1993,7 @@ efi_status_t efi_load_image_from_path(bool boot_policy,
>  	if (ret != EFI_SUCCESS)
>  		efi_free_pages(addr, pages);
>  out:
> -	EFI_CALL(efi_close_protocol(device, guid, efi_root, NULL));
> +	efi_close_protocol(device, guid, efi_root, NULL);
>  	if (ret == EFI_SUCCESS) {
>  		*buffer = (void *)(uintptr_t)addr;
>  		*size = buffer_size;
> @@ -2290,45 +2290,70 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
>   * @agent_handle:      handle of the driver
>   * @controller_handle: handle of the controller
>   *
> - * This function implements the CloseProtocol service.
> + * This is the function implementing the CloseProtocol service is for internal
> + * usage in U-Boot. For API usage wrapper efi_close_protocol_ext() is provided.
>   *
>   * See the Unified Extensible Firmware Interface (UEFI) specification for
>   * details.
>   *
>   * Return: status code
>   */
> -efi_status_t EFIAPI efi_close_protocol(efi_handle_t handle,
> -				       const efi_guid_t *protocol,
> -				       efi_handle_t agent_handle,
> -				       efi_handle_t controller_handle)
> +efi_status_t efi_close_protocol(efi_handle_t handle, const efi_guid_t *protocol,
> +				efi_handle_t agent_handle,
> +				efi_handle_t controller_handle)
>  {
>  	struct efi_handler *handler;
>  	struct efi_open_protocol_info_item *item;
>  	struct efi_open_protocol_info_item *pos;
> -	efi_status_t r;
> -
> -	EFI_ENTRY("%p, %pUs, %p, %p", handle, protocol, agent_handle,
> -		  controller_handle);
> +	efi_status_t ret;
>  
>  	if (!efi_search_obj(agent_handle) ||
> -	    (controller_handle && !efi_search_obj(controller_handle))) {
> -		r = EFI_INVALID_PARAMETER;
> -		goto out;
> -	}
> -	r = efi_search_protocol(handle, protocol, &handler);
> -	if (r != EFI_SUCCESS)
> -		goto out;
> +	    (controller_handle && !efi_search_obj(controller_handle)))
> +		return EFI_INVALID_PARAMETER;
> +	ret = efi_search_protocol(handle, protocol, &handler);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
>  
> -	r = EFI_NOT_FOUND;
> +	ret = EFI_NOT_FOUND;
>  	list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
>  		if (item->info.agent_handle == agent_handle &&
>  		    item->info.controller_handle == controller_handle) {
>  			efi_delete_open_info(item);
> -			r = EFI_SUCCESS;
> +			ret = EFI_SUCCESS;
>  		}
>  	}
> -out:
> -	return EFI_EXIT(r);
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_close_protocol_ext() - close a protocol
> + * @handle:            handle on which the protocol shall be closed
> + * @protocol:          GUID of the protocol to close
> + * @agent_handle:      handle of the driver
> + * @controller_handle: handle of the controller
> + *
> + * This function implements the CloseProtocol service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * Return: status code
> + */
> +efi_status_t EFIAPI efi_close_protocol_ext(efi_handle_t handle,
> +					   const efi_guid_t *protocol,
> +					   efi_handle_t agent_handle,
> +					   efi_handle_t controller_handle)
> +{

This needs to be static 

Other than that 
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

> +	efi_status_t ret;
> +
> +	EFI_ENTRY("%p, %pUs, %p, %p", handle, protocol, agent_handle,
> +		  controller_handle);
> +
> +	ret = efi_close_protocol(handle, protocol,
> +				 agent_handle, controller_handle);
> +
> +	return EFI_EXIT(ret);
>  }
>  
>  /**
> @@ -3213,11 +3238,10 @@ close_next:
>  				if (info->info.agent_handle !=
>  				    (efi_handle_t)image_obj)
>  					continue;
> -				r = EFI_CALL(efi_close_protocol
> -						(efiobj, &protocol->guid,
> -						 info->info.agent_handle,
> -						 info->info.controller_handle
> -						));
> +				r = efi_close_protocol(
> +						efiobj, &protocol->guid,
> +						info->info.agent_handle,
> +						info->info.controller_handle);
>  				if (r !=  EFI_SUCCESS)
>  					ret = r;
>  				/*
> @@ -3481,9 +3505,9 @@ static efi_status_t efi_bind_controller(
>  		r = EFI_CALL(binding_protocol->start(binding_protocol,
>  						     controller_handle,
>  						     remain_device_path));
> -	EFI_CALL(efi_close_protocol(driver_image_handle,
> -				    &efi_guid_driver_binding_protocol,
> -				    driver_image_handle, NULL));
> +	efi_close_protocol(driver_image_handle,
> +			   &efi_guid_driver_binding_protocol,
> +			   driver_image_handle, NULL);
>  	return r;
>  }
>  
> @@ -3834,9 +3858,9 @@ static efi_status_t EFIAPI efi_disconnect_controller(
>  			goto out;
>  		}
>  	}
> -	EFI_CALL(efi_close_protocol(driver_image_handle,
> -				    &efi_guid_driver_binding_protocol,
> -				    driver_image_handle, NULL));
> +	efi_close_protocol(driver_image_handle,
> +			   &efi_guid_driver_binding_protocol,
> +			   driver_image_handle, NULL);
>  	r = EFI_SUCCESS;
>  out:
>  	if (!child_handle)
> @@ -3883,7 +3907,7 @@ static struct efi_boot_services efi_boot_services = {
>  	.connect_controller = efi_connect_controller,
>  	.disconnect_controller = efi_disconnect_controller,
>  	.open_protocol = efi_open_protocol,
> -	.close_protocol = efi_close_protocol,
> +	.close_protocol = efi_close_protocol_ext,
>  	.open_protocol_information = efi_open_protocol_information,
>  	.protocols_per_handle = efi_protocols_per_handle,
>  	.locate_handle_buffer = efi_locate_handle_buffer,
> -- 
> 2.37.2
> 

Re: [PATCH 2/2] efi_driver: use efi_close_protocol

[Ilias Apalodimas]

On Sat, Oct 08, 2022 at 09:14:42AM +0200, Heinrich Schuchardt wrote:
> Avoid EFI_CALL() by using efi_close_protocol().
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  lib/efi_driver/efi_uclass.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> index 45610d1aa3..45f9351988 100644
> --- a/lib/efi_driver/efi_uclass.c
> +++ b/lib/efi_driver/efi_uclass.c
> @@ -97,10 +97,9 @@ static efi_status_t EFIAPI efi_uc_supported(
>  
>  	ret = check_node_type(controller_handle);
>  
> -	r = EFI_CALL(systab.boottime->close_protocol(
> -				controller_handle, bp->ops->protocol,
> -				this->driver_binding_handle,
> -				controller_handle));
> +	r = efi_close_protocol(controller_handle, bp->ops->protocol,
> +			       this->driver_binding_handle,
> +			       controller_handle);
>  	if (r != EFI_SUCCESS)
>  		ret = EFI_UNSUPPORTED;
>  out:
> @@ -151,10 +150,9 @@ static efi_status_t EFIAPI efi_uc_start(
>  		goto out;
>  
>  err:
> -	r = EFI_CALL(systab.boottime->close_protocol(
> -			controller_handle, bp->ops->protocol,
> -			this->driver_binding_handle,
> -			controller_handle));
> +	r = efi_close_protocol(controller_handle, bp->ops->protocol,
> +			       this->driver_binding_handle,
> +			       controller_handle);
>  	if (r != EFI_SUCCESS)
>  		EFI_PRINT("Failure to close handle\n");
>  
> @@ -177,9 +175,8 @@ static efi_status_t disconnect_child(efi_handle_t controller_handle,
>  	efi_guid_t *guid_controller = NULL;
>  	efi_guid_t *guid_child_controller = NULL;
>  
> -	ret = EFI_CALL(systab.boottime->close_protocol(
> -				controller_handle, guid_controller,
> -				child_handle, child_handle));
> +	ret = efi_close_protocol(controller_handle, guid_controller,
> +				 child_handle, child_handle);
>  	if (ret != EFI_SUCCESS) {
>  		EFI_PRINT("Cannot close protocol\n");
>  		return ret;
> @@ -252,9 +249,9 @@ static efi_status_t EFIAPI efi_uc_stop(
>  		log_err("Cannot free EFI memory pool\n");
>  
>  	/* Detach driver from controller */
> -	ret = EFI_CALL(systab.boottime->close_protocol(
> -			controller_handle, bp->ops->protocol,
> -			this->driver_binding_handle, controller_handle));
> +	ret = efi_close_protocol(controller_handle, bp->ops->protocol,
> +				 this->driver_binding_handle,
> +				 controller_handle);
>  out:
>  	return EFI_EXIT(ret);
>  }
> -- 
> 2.37.2
> 
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>