Re: [U-Boot] [PATCH 10/23] efi_loader: open_info in OpenProtocol

[Simon Glass]

Hi Heinrich,

On 27 August 2017 at 06:53, Heinrich Schuchardt  wrote:
> efi_open_protocol and close_protocol have to keep track of
> opened protocols.
>
> So we add an array open_info to each protocol of each handle.
>
> OpenProtocol has enter the agent and controller handle
> information into this array.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  include/efi_loader.h  |   1 +
>  lib/efi_loader/efi_boottime.c | 130 
> +++---
>  2 files changed, 99 insertions(+), 32 deletions(-)
>

Reviewed-by: Simon Glass 

Please see below.

> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 193fca24ce..2c3360534b 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -87,6 +87,7 @@ extern unsigned int __efi_runtime_rel_start, 
> __efi_runtime_rel_stop;
>  struct efi_handler {
> const efi_guid_t *guid;
> void *protocol_interface;
> +   struct efi_open_protocol_info_entry open_info[4];
>  };
>
>  /*
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index a483b827cd..294bc1f138 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1152,24 +1152,111 @@ static void EFIAPI efi_set_mem(void *buffer, 
> unsigned long size, uint8_t value)
> memset(buffer, value, size);
>  }
>
> +static efi_status_t efi_protocol_open(
> +   struct efi_handler *protocol,
> +   void **protocol_interface, void *agent_handle,
> +   void *controller_handle, uint32_t attributes)
> +{
> +   bool opened_exclusive = false;
> +   bool opened_by_driver = false;
> +   int i;
> +   struct efi_open_protocol_info_entry *open_info;
> +   struct efi_open_protocol_info_entry *match = NULL;
> +
> +   if (attributes !=
> +   EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
> +   *protocol_interface = NULL;
> +   }
> +
> +   for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {
> +   open_info = >open_info[i];
> +
> +   if (!open_info->open_count)
> +   continue;
> +   if (open_info->agent_handle == agent_handle) {
> +   if ((attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) &&
> +   (open_info->attributes == attributes))
> +   return EFI_ALREADY_STARTED;
> +   if (open_info->controller_handle == controller_handle)
> +   match = open_info;
> +   }
> +   if (open_info->attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE)
> +   opened_exclusive = true;
> +   }
> +
> +   if (attributes &
> +   (EFI_OPEN_PROTOCOL_EXCLUSIVE | EFI_OPEN_PROTOCOL_BY_DRIVER) &&
> +   opened_exclusive)
> +   return EFI_ACCESS_DENIED;
> +
> +   if (attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE) {
> +   for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {

Is it possible to put this search in a utility function with suitable
parameters and call it? This function is very long.

> +   open_info = >open_info[i];
> +
> +   if (!open_info->open_count)
> +   continue;
> +   if (open_info->attributes ==
> +   EFI_OPEN_PROTOCOL_BY_DRIVER)
> +   EFI_CALL(efi_disconnect_controller(
> +   open_info->controller_handle,
> +   open_info->agent_handle,
> +   NULL));
> +   }
> +   opened_by_driver = false;
> +   for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {
> +   open_info = >open_info[i];
> +
> +   if (!open_info->open_count)
> +   continue;
> +   if (open_info->attributes & 
> EFI_OPEN_PROTOCOL_BY_DRIVER)
> +   opened_by_driver = true;
> +   }
> +   if (opened_by_driver)
> +   return EFI_ACCESS_DENIED;
> +   if (match && !match->open_count)
> +   match = NULL;
> +   }
> +
> +   /*
> +* Find an empty slot.
> +*/
> +   if (!match) {
> +   for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {
> +   open_info = >open_info[i];
> +
> +   if (!open_info->open_count) {
> +   match = open_info;
> +   break;
> +   }
> +   }
> +   }
> +   if (!match)
> +   return EFI_OUT_OF_RESOURCES;
> +
> +   match->agent_handle = agent_handle;
> +   

[U-Boot] [PATCH 10/23] efi_loader: open_info in OpenProtocol

[Heinrich Schuchardt]

efi_open_protocol and close_protocol have to keep track of
opened protocols.

So we add an array open_info to each protocol of each handle.

OpenProtocol has enter the agent and controller handle
information into this array.

Signed-off-by: Heinrich Schuchardt 
---
 include/efi_loader.h  |   1 +
 lib/efi_loader/efi_boottime.c | 130 +++---
 2 files changed, 99 insertions(+), 32 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 193fca24ce..2c3360534b 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -87,6 +87,7 @@ extern unsigned int __efi_runtime_rel_start, 
__efi_runtime_rel_stop;
 struct efi_handler {
const efi_guid_t *guid;
void *protocol_interface;
+   struct efi_open_protocol_info_entry open_info[4];
 };
 
 /*
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index a483b827cd..294bc1f138 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1152,24 +1152,111 @@ static void EFIAPI efi_set_mem(void *buffer, unsigned 
long size, uint8_t value)
memset(buffer, value, size);
 }
 
+static efi_status_t efi_protocol_open(
+   struct efi_handler *protocol,
+   void **protocol_interface, void *agent_handle,
+   void *controller_handle, uint32_t attributes)
+{
+   bool opened_exclusive = false;
+   bool opened_by_driver = false;
+   int i;
+   struct efi_open_protocol_info_entry *open_info;
+   struct efi_open_protocol_info_entry *match = NULL;
+
+   if (attributes !=
+   EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
+   *protocol_interface = NULL;
+   }
+
+   for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {
+   open_info = >open_info[i];
+
+   if (!open_info->open_count)
+   continue;
+   if (open_info->agent_handle == agent_handle) {
+   if ((attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) &&
+   (open_info->attributes == attributes))
+   return EFI_ALREADY_STARTED;
+   if (open_info->controller_handle == controller_handle)
+   match = open_info;
+   }
+   if (open_info->attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE)
+   opened_exclusive = true;
+   }
+
+   if (attributes &
+   (EFI_OPEN_PROTOCOL_EXCLUSIVE | EFI_OPEN_PROTOCOL_BY_DRIVER) &&
+   opened_exclusive)
+   return EFI_ACCESS_DENIED;
+
+   if (attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE) {
+   for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {
+   open_info = >open_info[i];
+
+   if (!open_info->open_count)
+   continue;
+   if (open_info->attributes ==
+   EFI_OPEN_PROTOCOL_BY_DRIVER)
+   EFI_CALL(efi_disconnect_controller(
+   open_info->controller_handle,
+   open_info->agent_handle,
+   NULL));
+   }
+   opened_by_driver = false;
+   for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {
+   open_info = >open_info[i];
+
+   if (!open_info->open_count)
+   continue;
+   if (open_info->attributes & EFI_OPEN_PROTOCOL_BY_DRIVER)
+   opened_by_driver = true;
+   }
+   if (opened_by_driver)
+   return EFI_ACCESS_DENIED;
+   if (match && !match->open_count)
+   match = NULL;
+   }
+
+   /*
+* Find an empty slot.
+*/
+   if (!match) {
+   for (i = 0; i < ARRAY_SIZE(protocol->open_info); ++i) {
+   open_info = >open_info[i];
+
+   if (!open_info->open_count) {
+   match = open_info;
+   break;
+   }
+   }
+   }
+   if (!match)
+   return EFI_OUT_OF_RESOURCES;
+
+   match->agent_handle = agent_handle;
+   match->controller_handle = controller_handle;
+   match->attributes = attributes;
+   match->open_count++;
+   *protocol_interface = protocol->protocol_interface;
+
+   return EFI_SUCCESS;
+}
+
 static efi_status_t EFIAPI efi_open_protocol(
void *handle, efi_guid_t *protocol,
void **protocol_interface, void *agent_handle,
void *controller_handle, uint32_t attributes)
 {
-   struct list_head *lhandle;
-   int i;
+