On 08/27/2017 12:51 AM, Heinrich Schuchardt wrote:

8 protocols per efi_object is insufficient for iPXE.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

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 6f71a6202b..e8fb4fbb0a 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -99,8 +99,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 8 "protocols" an object can be accessed through */
• struct efi_handler protocols[8];

• /* We support up to 16 "protocols" an object can be accessed through */
• struct efi_handler protocols[16];

Can you try to convert it into a list instead? Leif tried to make the UEFI Shell work and stumbled over the same limitation, so I guess a static array won't cut it for long.

Alex

On 01.09.17 03:45, Heinrich Schuchardt wrote:

On 08/31/2017 04:01 PM, Alexander Graf wrote:

...

On 08/27/2017 12:51 AM, Heinrich Schuchardt wrote:

...

8 protocols per efi_object is insufficient for iPXE.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

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 6f71a6202b..e8fb4fbb0a 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -99,8 +99,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 8 "protocols" an object can be accessed

through */

• struct efi_handler protocols[8];

• /* We support up to 16 "protocols" an object can be accessed

through */

• struct efi_handler protocols[16];

Can you try to convert it into a list instead? Leif tried to make the UEFI Shell work and stumbled over the same limitation, so I guess a static array won't cut it for long.

Alex

Hello Alex,

is it safe to call malloc and free before efi_exit_boot_services?

Yes :). Before exiting boot services we're basically in normal U-Boot space with a U-Boot owned malloc region that we can allocate from.

Currently we do not check that boottime services are not called after efi_exit_boot_services. Shouldn't every call to boottime services fail

Yes, IIUC it's simply illegal to call them afterwards.

thereafter? The spec says: "After successfully calling ExitBootServices(), all boot services in the system are terminated."

I'm not sure. I'd be very surprised to see a payload actually call any boot service after exiting boot services. Runtime services is a different question, but those are very special anyway.

We definitively do not want to call malloc at runtime because all available memory (except for EFI_MEMORY_RUNTIME) is managed by the operating system.

Yes, but efi objects only exist during boot time. Runtime services don't expose protocols or objects.

Alex

Am 02.09.2017 um 20:14 schrieb Rob Clark robdclark@gmail.com:

...

On Thu, Aug 31, 2017 at 10:01 AM, Alexander Graf agraf@suse.de wrote:

...

On 08/27/2017 12:51 AM, Heinrich Schuchardt wrote:

8 protocols per efi_object is insufficient for iPXE.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

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 6f71a6202b..e8fb4fbb0a 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -99,8 +99,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 8 "protocols" an object can be accessed
  

through */

•   struct efi_handler protocols[8];
  

•   /* We support up to 16 "protocols" an object can be accessed
  

through */

•   struct efi_handler protocols[16];
  

Can you try to convert it into a list instead? Leif tried to make the UEFI Shell work and stumbled over the same limitation, so I guess a static array won't cut it for long.

Can we go w/ fixed 16 protocols length for now? A list is a definitely a better option, and it will be easier after "efi_loader: refactor boot device and loaded_image handling" (which gets rid of the statically initialized efi_object's). After that we can drop the fixed length array and add an 'void append_protocol(efiobj, guid, handle)' helper fairly easily.

Sure, of course :)

Alex

BR, -R

Hi Heinrich,

On 27 August 2017 at 06:51, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

Check the parameters in efi_locate_handle.

Use list_for_each_entry instead of list_for_each.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_boottime.c | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

nits below

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b5538e0769..570a5ea186 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -599,6 +599,7 @@ static int efi_search(enum efi_locate_search_type search_type, case all_handles: return 0; case by_register_notify:

•           /* RegisterProtocolNotify is not implemented yet */
            return -1;
    case by_protocol:
            for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
  

@@ -617,16 +618,38 @@ static efi_status_t efi_locate_handle( efi_guid_t *protocol, void *search_key, unsigned long *buffer_size, efi_handle_t *buffer)

function needs a comment

{

•   struct list_head *lhandle;
  

•   struct efi_object *efiobj;
    unsigned long size = 0;
  
  

•   /* Check parameters */
  

•   switch (search_type) {
  

•   case all_handles:
  

•           break;
  

•   case by_register_notify:
  

•           if (!search_key)
  

•                   return EFI_INVALID_PARAMETER;
  

•           /* RegisterProtocolNotify is not implemented yet */
  

•           return EFI_UNSUPPORTED;
  

•   case by_protocol:
  

•           if (!protocol)
  

•                   return EFI_INVALID_PARAMETER;
  

•           break;
  

•   default:
  

•           return EFI_INVALID_PARAMETER;
  

•   }
  

•   /*
  

•    * efi_locate_handle_buffer uses this function for
  

•    * the calculation of the necessary buffer size.
  

•    * So do not require a buffer for buffersize == 0.
  

•    */
  

•   if (!buffer_size || (*buffer_size && !buffer))
  

•           return EFI_INVALID_PARAMETER;
  

•   /* Count how much space we need */
  

•   list_for_each(lhandle, &efi_obj_list) {
  

•           struct efi_object *efiobj;
  

•           efiobj = list_entry(lhandle, struct efi_object, link);
  

•           if (!efi_search(search_type, protocol, search_key, efiobj)) {
  

•   list_for_each_entry(efiobj, &efi_obj_list, link) {
  

•           if (!efi_search(search_type, protocol, search_key, efiobj))
                    size += sizeof(void*);
  

•           }
    }
  
    if (*buffer_size < size) {
  

@@ -639,12 +662,9 @@ static efi_status_t efi_locate_handle( return EFI_NOT_FOUND;

    /* Then fill the array */

•   list_for_each(lhandle, &efi_obj_list) {
  

•           struct efi_object *efiobj;
  

•           efiobj = list_entry(lhandle, struct efi_object, link);
  

•           if (!efi_search(search_type, protocol, search_key, efiobj)) {
  

•   list_for_each_entry(efiobj, &efi_obj_list, link) {
  

•           if (!efi_search(search_type, protocol, search_key, efiobj))
                    *(buffer++) = efiobj->handle;
  

*buffer++

•           }
    }
  
    return EFI_SUCCESS;
  

-- 2.14.1

Regards, Simon

On 27 August 2017 at 06:51, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

In multiple functions we are searching for the protocol of a handle. This patch provides a new function efi_search_protocol that we can use to avoid duplicating code.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_boottime.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)

Reviewed-by: Simon Glass sjg@chromium.org

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b643d299b9..9dae02daca 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -453,6 +453,31 @@ static efi_status_t EFIAPI efi_check_event(struct efi_event *event) return EFI_EXIT(EFI_INVALID_PARAMETER); }

+static efi_status_t efi_search_protocol(void *handle, efi_guid_t *protocol_guid,

•                                   struct efi_handler **handler)
  

Needs a function comment

+{

•   struct efi_object *efiobj;
  

•   size_t i;
  

•   struct efi_handler *protocol;
  

•   if (!handle || !protocol_guid)
  

•           return EFI_INVALID_PARAMETER;
  

•   efiobj = efi_search_obj(handle);
  

•   if (!efiobj)
  

•           return EFI_INVALID_PARAMETER;
  

•   for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
  

•           protocol = &efiobj->protocols[i];
  

•           if (!protocol->guid)
  

•                   continue;
  

•           if (!guidcmp(protocol->guid, protocol_guid)) {
  

•                   if (handler)
  

•                           *handler = protocol;
  

•                   return EFI_SUCCESS;
  

•           }
  

•   }
  

•   return EFI_NOT_FOUND;
  

+}

static efi_status_t EFIAPI efi_install_protocol_interface(void **handle, efi_guid_t *protocol, int protocol_interface_type, void *protocol_interface) -- 2.14.1

On 27 August 2017 at 06:51, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

In efi_install_protocol_interface support creating a new handle.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_boottime.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

The use of void ** seems odd. Is that mandated by EFI? Is there no way to use a proper type?

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 96cb1fa410..9f8d64659f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -238,6 +238,23 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer) return EFI_EXIT(r); }

+static efi_status_t efi_create_handle(void **handle) +{

•   struct efi_object *obj;
  

•   efi_status_t r;
  

•   r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES,
  

•                         sizeof(struct efi_object),
  

•                         (void **)&obj);
  

•   if (r != EFI_SUCCESS)
  

•           return r;
  

•   memset(obj, 0, sizeof(struct efi_object));
  

•   obj->handle = obj;
  

•   list_add_tail(&obj->link, &efi_obj_list);
  

•   *handle = obj;
  

•   return r;
  

+}

/*

• Our event capabilities are very limited. Only a small limited
• number of events is allowed to coexist.

@@ -497,8 +514,9 @@ static efi_status_t EFIAPI efi_install_protocol_interface(void **handle,

    /* Create new handle if requested. */
    if (!*handle) {

•           r = EFI_OUT_OF_RESOURCES;
  

•           goto out;
  

•           r = efi_create_handle(handle);
  

•           if (r != EFI_SUCCESS)
  

•                   goto out;
    }
  
    /* Find object. */
  

-- 2.14.1

On 27 August 2017 at 06:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

efi_open_protocol and efi_close_protocol have to keep track of opened protocols.

efi_close_protocol has to mark the appropriate entry as empty.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_boottime.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass sjg@chromium.org

Again I wonder if you can use a utility function to find the slot.

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 294bc1f138..c9aec597a2 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -944,9 +944,40 @@ static efi_status_t EFIAPI efi_close_protocol(void *handle, void *agent_handle, void *controller_handle) {

•   struct efi_handler *handler;
  

•   size_t i;
  

•   struct efi_open_protocol_info_entry *open_info;
  

•   efi_status_t r;
  

•   EFI_ENTRY("%p, %p, %p, %p", handle, protocol, agent_handle,
              controller_handle);
  

•   return EFI_EXIT(EFI_NOT_FOUND);
  

•   if (!agent_handle) {
  

•           r = EFI_INVALID_PARAMETER;
  

•           goto out;
  

•   }
  

•   r = efi_search_protocol(handle, protocol, &handler);
  

•   if (r != EFI_SUCCESS)
  

•           goto out;
  

•   for (i = 0; i < ARRAY_SIZE(handler->open_info); ++i) {
  

•           open_info = &handler->open_info[i];
  

•           if (!open_info->open_count)
  

•                   continue;
  

•           if (open_info->agent_handle == agent_handle &&
  

•               open_info->controller_handle ==
  

•               controller_handle) {
  

•                   open_info->open_count--;
  

•                   r = EFI_SUCCESS;
  

•                   goto out;
  

•           }
  

•   }
  

•   r = EFI_NOT_FOUND;
  

+out:

•   return EFI_EXIT(r);
  

}

static efi_status_t EFIAPI efi_open_protocol_information(efi_handle_t handle,

2.14.1

Hi Heinrich,

On 27 August 2017 at 06:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

efi_open_protocol_information provides the agent and controller handles as well as the attributes and open count of an protocol on a handle.

Cc: Rob Clark robdclark@gmail.com Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

---

lib/efi_loader/efi_boottime.c | 55 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass sjg@chromium.org

I can't help wondering if this would be better as a linked list?

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index c9aec597a2..23b8894e73 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -985,9 +985,62 @@ static efi_status_t EFIAPI efi_open_protocol_information(efi_handle_t handle, struct efi_open_protocol_info_entry **entry_buffer, unsigned long *entry_count) {

•   unsigned long buffer_size;
  

•   unsigned long count;
  

•   struct efi_handler *handler;
  

•   size_t i;
  

•   efi_status_t r;
  

•   EFI_ENTRY("%p, %p, %p, %p", handle, protocol, entry_buffer,
              entry_count);
  

•   return EFI_EXIT(EFI_NOT_FOUND);
  

•   /* Check parameters */
  

•   if (!handle || !protocol || !entry_buffer) {
  

•           r = EFI_INVALID_PARAMETER;
  

•           goto out;
  

•   }
  

•   /* Find the protocol */
  

•   r = efi_search_protocol(handle, protocol, &handler);
  

•   if (r != EFI_SUCCESS)
  

•           goto out;
  

•   *entry_buffer = NULL;
  

•   /* Count entries */
  

•   count = 0;
  

•   for (i = 0; i < ARRAY_SIZE(handler->open_info); ++i) {
  

•           struct efi_open_protocol_info_entry *open_info =
  

•                   &handler->open_info[i];
  

•           if (open_info->open_count)
  

•                   ++count;
  

•   }
  

•   *entry_count = count;
  

•   if (!count) {
  

•           r = EFI_SUCCESS;
  

•           goto out;
  

•   }
  

•   /* Copy entries */
  

•   buffer_size = count * sizeof(struct efi_open_protocol_info_entry);
  

•   r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, buffer_size,
  

•                         (void **)entry_buffer);
  

•   if (r != EFI_SUCCESS)
  

•           goto out;
  

•   count = 0;
  

•   for (i = 0; i < ARRAY_SIZE(handler->open_info); ++i) {
  

•           struct efi_open_protocol_info_entry *open_info =
  

•                   &handler->open_info[i];
  

•           if (!open_info->open_count)
  

•                   continue;
  

•           (*entry_buffer)[count] = *open_info;
  

•           ++count;
  

•   }
  

+out:

•   return EFI_EXIT(r);
  

}

static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,

2.14.1

We need efi_open_protocol and efi_close_protocol for implementing other functions. So they shouldn't be static.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 9 +++++++++ lib/efi_loader/efi_boottime.c | 9 ++++----- 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 2c3360534b..2a98bf66b8 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -181,6 +181,15 @@ efi_status_t efi_set_timer(struct efi_event *event, enum efi_timer_delay type, uint64_t trigger_time); /* Call this to signal an event */ void efi_signal_event(struct efi_event *event); +/* Call this with EFI_CALL to close a protocol */ +efi_status_t EFIAPI efi_close_protocol(void *handle, efi_guid_t *protocol, + void *agent_handle, + void *controller_handle); +/* Call this with EFI_CALL to open a protocol */ +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);

/* Generic EFI memory allocator, call this to get memory */ void *efi_alloc(uint64_t len, int memory_type); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 23b8894e73..ad8733d3e5 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -939,10 +939,9 @@ static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle, return EFI_EXIT(EFI_INVALID_PARAMETER); }

-static efi_status_t EFIAPI efi_close_protocol(void *handle, - efi_guid_t *protocol, - void *agent_handle, - void *controller_handle) +efi_status_t EFIAPI efi_close_protocol(void *handle, efi_guid_t *protocol, + void *agent_handle, + void *controller_handle) { struct efi_handler *handler; size_t i; @@ -1326,7 +1325,7 @@ static efi_status_t efi_protocol_open( return EFI_SUCCESS; }

-static efi_status_t EFIAPI efi_open_protocol( +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)

We need to call some boottime services internally. Our GUIDs are stored as const efi_guid_t *.

The boottime services never change GUIDs. So we can define the parameters as const efi_guid_t *.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_api.h | 44 +++++++++++++++++++++---------------------- include/efi_loader.h | 4 ++-- lib/efi_loader/efi_boottime.c | 37 ++++++++++++++++++------------------ 3 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h index ec1b321e8e..8efc8dfab8 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -74,25 +74,25 @@ struct efi_boot_services { efi_status_t (EFIAPI *close_event)(struct efi_event *event); efi_status_t (EFIAPI *check_event)(struct efi_event *event); #define EFI_NATIVE_INTERFACE 0x00000000 - efi_status_t (EFIAPI *install_protocol_interface)( - void **handle, efi_guid_t *protocol, + efi_status_t (EFIAPI * install_protocol_interface)( + void **handle, const efi_guid_t *protocol, int protocol_interface_type, void *protocol_interface); - efi_status_t (EFIAPI *reinstall_protocol_interface)( - void *handle, efi_guid_t *protocol, + efi_status_t (EFIAPI * reinstall_protocol_interface)( + void *handle, const efi_guid_t *protocol, void *old_interface, void *new_interface); - efi_status_t (EFIAPI *uninstall_protocol_interface)(void *handle, - efi_guid_t *protocol, void *protocol_interface); - efi_status_t (EFIAPI *handle_protocol)(efi_handle_t, efi_guid_t *, - void **); + efi_status_t (EFIAPI * uninstall_protocol_interface)(void *handle, + const efi_guid_t *protocol, void *protocol_interface); + efi_status_t (EFIAPI * handle_protocol)(efi_handle_t, + const efi_guid_t *, void **); void *reserved; - efi_status_t (EFIAPI *register_protocol_notify)( - efi_guid_t *protocol, struct efi_event *event, + efi_status_t (EFIAPI * register_protocol_notify)( + const efi_guid_t *protocol, struct efi_event *event, void **registration); - efi_status_t (EFIAPI *locate_handle)( + efi_status_t (EFIAPI * locate_handle)( enum efi_locate_search_type search_type, - efi_guid_t *protocol, void *search_key, + const efi_guid_t *protocol, void *search_key, unsigned long *buffer_size, efi_handle_t *buffer); - efi_status_t (EFIAPI *locate_device_path)(efi_guid_t *protocol, + efi_status_t (EFIAPI * locate_device_path)(const efi_guid_t *protocol, struct efi_device_path **device_path, efi_handle_t *device); efi_status_t (EFIAPI *install_configuration_table)( @@ -128,25 +128,25 @@ struct efi_boot_services { #define EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER 0x00000008 #define EFI_OPEN_PROTOCOL_BY_DRIVER 0x00000010 #define EFI_OPEN_PROTOCOL_EXCLUSIVE 0x00000020 - efi_status_t (EFIAPI *open_protocol)(efi_handle_t handle, - efi_guid_t *protocol, void **interface, + efi_status_t (EFIAPI * open_protocol)(efi_handle_t handle, + const efi_guid_t *protocol, void **interface, efi_handle_t agent_handle, efi_handle_t controller_handle, u32 attributes); - efi_status_t (EFIAPI *close_protocol)(void *handle, - efi_guid_t *protocol, void *agent_handle, + efi_status_t (EFIAPI * close_protocol)(void *handle, + const efi_guid_t *protocol, void *agent_handle, void *controller_handle); - efi_status_t(EFIAPI *open_protocol_information)(efi_handle_t handle, - efi_guid_t *protocol, + efi_status_t(EFIAPI * open_protocol_information)(efi_handle_t handle, + const efi_guid_t *protocol, struct efi_open_protocol_info_entry **entry_buffer, unsigned long *entry_count); efi_status_t (EFIAPI *protocols_per_handle)(efi_handle_t handle, efi_guid_t ***protocol_buffer, unsigned long *protocols_buffer_count); - efi_status_t (EFIAPI *locate_handle_buffer) ( + efi_status_t (EFIAPI * locate_handle_buffer) ( enum efi_locate_search_type search_type, - efi_guid_t *protocol, void *search_key, + const efi_guid_t *protocol, void *search_key, unsigned long *no_handles, efi_handle_t **buffer); - efi_status_t (EFIAPI *locate_protocol)(efi_guid_t *protocol, + efi_status_t (EFIAPI * locate_protocol)(const efi_guid_t *protocol, void *registration, void **protocol_interface); efi_status_t (EFIAPI *install_multiple_protocol_interfaces)( void **handle, ...); diff --git a/include/efi_loader.h b/include/efi_loader.h index 2a98bf66b8..9c68246c7c 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -182,12 +182,12 @@ efi_status_t efi_set_timer(struct efi_event *event, enum efi_timer_delay type, /* Call this to signal an event */ void efi_signal_event(struct efi_event *event); /* Call this with EFI_CALL to close a protocol */ -efi_status_t EFIAPI efi_close_protocol(void *handle, efi_guid_t *protocol, +efi_status_t EFIAPI efi_close_protocol(void *handle, const efi_guid_t *protocol, void *agent_handle, void *controller_handle); /* Call this with EFI_CALL to open a protocol */ efi_status_t EFIAPI efi_open_protocol( - void *handle, efi_guid_t *protocol, + void *handle, const efi_guid_t *protocol, void **protocol_interface, void *agent_handle, void *controller_handle, uint32_t attributes);

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index ad8733d3e5..5a73ea5cd0 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -470,7 +470,8 @@ static efi_status_t EFIAPI efi_check_event(struct efi_event *event) return EFI_EXIT(EFI_INVALID_PARAMETER); }

-static efi_status_t efi_search_protocol(void *handle, efi_guid_t *protocol_guid, +static efi_status_t efi_search_protocol(void *handle, + const efi_guid_t *protocol_guid, struct efi_handler **handler) { struct efi_object *efiobj; @@ -496,7 +497,7 @@ static efi_status_t efi_search_protocol(void *handle, efi_guid_t *protocol_guid, }

static efi_status_t EFIAPI efi_install_protocol_interface(void **handle, - efi_guid_t *protocol, int protocol_interface_type, + const efi_guid_t *protocol, int protocol_interface_type, void *protocol_interface) { int i; @@ -551,7 +552,7 @@ out: }

static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle, - efi_guid_t *protocol, void *old_interface, + const efi_guid_t *protocol, void *old_interface, void *new_interface) { EFI_ENTRY("%p, %p, %p, %p", handle, protocol, old_interface, @@ -560,7 +561,7 @@ 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) + const efi_guid_t *protocol, void *protocol_interface) { struct efi_handler *handler; efi_status_t r; @@ -589,16 +590,16 @@ out: return EFI_EXIT(r); }

-static efi_status_t EFIAPI efi_register_protocol_notify(efi_guid_t *protocol, - struct efi_event *event, - void **registration) +static efi_status_t EFIAPI efi_register_protocol_notify( + const efi_guid_t *protocol, struct efi_event *event, + void **registration) { EFI_ENTRY("%p, %p, %p", protocol, event, registration); return EFI_EXIT(EFI_OUT_OF_RESOURCES); }

static int efi_search(enum efi_locate_search_type search_type, - efi_guid_t *protocol, void *search_key, + const efi_guid_t *protocol, void *search_key, struct efi_object *efiobj) { int i; @@ -623,7 +624,7 @@ static int efi_search(enum efi_locate_search_type search_type,

static efi_status_t efi_locate_handle( enum efi_locate_search_type search_type, - efi_guid_t *protocol, void *search_key, + const efi_guid_t *protocol, void *search_key, unsigned long *buffer_size, efi_handle_t *buffer) { struct efi_object *efiobj; @@ -680,7 +681,7 @@ static efi_status_t efi_locate_handle(

static efi_status_t EFIAPI efi_locate_handle_ext( enum efi_locate_search_type search_type, - efi_guid_t *protocol, void *search_key, + const 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, @@ -690,7 +691,7 @@ static efi_status_t EFIAPI efi_locate_handle_ext( buffer_size, buffer)); }

-static efi_status_t EFIAPI efi_locate_device_path(efi_guid_t *protocol, +static efi_status_t EFIAPI efi_locate_device_path(const efi_guid_t *protocol, struct efi_device_path **device_path, efi_handle_t *device) { @@ -939,7 +940,7 @@ static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle, return EFI_EXIT(EFI_INVALID_PARAMETER); }

-efi_status_t EFIAPI efi_close_protocol(void *handle, efi_guid_t *protocol, +efi_status_t EFIAPI efi_close_protocol(void *handle, const efi_guid_t *protocol, void *agent_handle, void *controller_handle) { @@ -980,7 +981,7 @@ out: }

static efi_status_t EFIAPI efi_open_protocol_information(efi_handle_t handle, - efi_guid_t *protocol, + const efi_guid_t *protocol, struct efi_open_protocol_info_entry **entry_buffer, unsigned long *entry_count) { @@ -1097,7 +1098,7 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,

static efi_status_t EFIAPI efi_locate_handle_buffer( enum efi_locate_search_type search_type, - efi_guid_t *protocol, void *search_key, + const efi_guid_t *protocol, void *search_key, unsigned long *no_handles, efi_handle_t **buffer) { efi_status_t r; @@ -1128,7 +1129,7 @@ out: return EFI_EXIT(r); }

-static efi_status_t EFIAPI efi_locate_protocol(efi_guid_t *protocol, +static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol, void *registration, void **protocol_interface) { @@ -1167,7 +1168,7 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( EFI_ENTRY("%p", handle);

va_list argptr; - efi_guid_t *protocol; + const efi_guid_t *protocol; void *protocol_interface; efi_status_t r = EFI_SUCCESS; int i = 0; @@ -1326,7 +1327,7 @@ static efi_status_t efi_protocol_open( }

efi_status_t EFIAPI efi_open_protocol( - void *handle, efi_guid_t *protocol, + void *handle, const efi_guid_t *protocol, void **protocol_interface, void *agent_handle, void *controller_handle, uint32_t attributes) { @@ -1372,7 +1373,7 @@ out: }

static efi_status_t EFIAPI efi_handle_protocol(void *handle, - efi_guid_t *protocol, + const efi_guid_t *protocol, void **protocol_interface) { return efi_open_protocol(handle, protocol, protocol_interface, NULL,

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_api.h | 22 ++++++++ include/efi_loader.h | 1 + lib/efi_loader/efi_boottime.c | 119 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 141 insertions(+), 1 deletion(-)

diff --git a/include/efi_api.h b/include/efi_api.h index 8efc8dfab8..b2838125d7 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -609,4 +609,26 @@ struct efi_pxe { struct efi_pxe_mode *mode; };

+#define EFI_DRIVER_BINDING_PROTOCOL_GUID \ + EFI_GUID(0x18a031ab, 0xb443, 0x4d1a,\ + 0xa5, 0xc0, 0x0c, 0x09, 0x26, 0x1e, 0x9f, 0x71) +struct efi_driver_binding_protocol { + efi_status_t (EFIAPI * supported)( + struct efi_driver_binding_protocol *this, + efi_handle_t controller_handle, + struct efi_device_path *remaining_device_path); + efi_status_t (EFIAPI * start)( + struct efi_driver_binding_protocol *this, + efi_handle_t controller_handle, + struct efi_device_path *remaining_device_path); + efi_status_t (EFIAPI * stop)( + struct efi_driver_binding_protocol *this, + efi_handle_t controller_handle, + UINTN number_of_children, + efi_handle_t child_handle_buffer); + u32 version; + efi_handle_t image_handle; + efi_handle_t driver_binding_handle; +}; + #endif diff --git a/include/efi_loader.h b/include/efi_loader.h index 9c68246c7c..f9f33e1d01 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -74,6 +74,7 @@ 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_driver_binding_protocol; extern const efi_guid_t efi_guid_loaded_image; extern const efi_guid_t efi_guid_device_path_to_text_protocol;

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 5a73ea5cd0..1069da7d79 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -18,6 +18,14 @@

DECLARE_GLOBAL_DATA_PTR;

+static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol, + void *registration, + void **protocol_interface); +static efi_status_t EFIAPI efi_locate_handle_buffer( + enum efi_locate_search_type search_type, + const efi_guid_t *protocol, void *search_key, + unsigned long *no_handles, efi_handle_t **buffer); + /* This list contains all the EFI objects our payload has access to */ LIST_HEAD(efi_obj_list);

@@ -49,6 +57,9 @@ static struct efi_configuration_table __efi_runtime_data efi_conf_table[2]; static volatile void *efi_gd, *app_gd; #endif

+const efi_guid_t efi_guid_driver_binding_protocol = + EFI_DRIVER_BINDING_PROTOCOL_GUID; + static int entry_count; static int nesting_level;

@@ -920,15 +931,121 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout, return efi_unsupported(__func__); }

+static efi_status_t efi_bind_controller( + efi_handle_t controller_handle, + efi_handle_t driver_image_handle, + struct efi_device_path *remain_device_path) +{ + struct efi_driver_binding_protocol *binding_protocol; + efi_status_t r; + + r = EFI_CALL(efi_open_protocol(driver_image_handle, + &efi_guid_driver_binding_protocol, + (void **)&binding_protocol, + driver_image_handle, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); + if (r != EFI_SUCCESS) + return r; + r = EFI_CALL(binding_protocol->supported(binding_protocol, + controller_handle, + remain_device_path)); + if (r == EFI_SUCCESS) + 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)); + return r; +} + +static efi_status_t efi_connect_single_controller( + efi_handle_t controller_handle, + efi_handle_t *driver_image_handle, + struct efi_device_path *remain_device_path) +{ + efi_handle_t *buffer; + unsigned long count; + size_t i; + efi_status_t r; + size_t connected = 0; + + /* Get buffer with all handles with driver binding protocol */ + r = EFI_CALL(efi_locate_handle_buffer(by_protocol, + &efi_guid_driver_binding_protocol, + NULL, &count, &buffer)); + if (r != EFI_SUCCESS) + return r; + + /* Context Override */ + if (driver_image_handle) { + for (; *driver_image_handle; ++driver_image_handle) { + for (i = 0; i < count; ++i) { + if (buffer[i] == *driver_image_handle) { + buffer[i] = NULL; + r = efi_bind_controller( + controller_handle, + *driver_image_handle, + remain_device_path); + if (r == EFI_SUCCESS) + ++connected; + } + } + } + } + + /* + * Some overrides are not yet implemented: + * Platform Driver Override + * Driver Family Override Search + * Driver Family Override Search + * Bus Specific Driver Override + */ + + /* Driver Binding Search */ + for (i = 0; i < count; ++i) { + if (buffer[i]) { + r = efi_bind_controller(controller_handle, + buffer[i], + remain_device_path); + if (r == EFI_SUCCESS) + ++connected; + } + } + + efi_free_pool(buffer); + if (!connected) + return EFI_NOT_FOUND; + return EFI_SUCCESS; +} + static efi_status_t EFIAPI efi_connect_controller( efi_handle_t controller_handle, efi_handle_t *driver_image_handle, struct efi_device_path *remain_device_path, bool recursive) { + efi_status_t r; + EFI_ENTRY("%p, %p, %p, %d", controller_handle, driver_image_handle, remain_device_path, recursive); - return EFI_EXIT(EFI_NOT_FOUND); + + if (!controller_handle) { + r = EFI_INVALID_PARAMETER; + goto out; + } + + if (recursive) { + r = EFI_UNSUPPORTED; + goto out; + } + + r = efi_connect_single_controller(controller_handle, + driver_image_handle, + remain_device_path); + +out: + return EFI_EXIT(r); }

static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle,

On 08/31/2017 02:52 PM, Simon Glass wrote:

On 27 August 2017 at 06:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

include/efi_api.h | 22 ++++++++ include/efi_loader.h | 1 + lib/efi_loader/efi_boottime.c | 119 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 141 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass sjg@chromium.org

Please see below.

...

diff --git a/include/efi_api.h b/include/efi_api.h index 8efc8dfab8..b2838125d7 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -609,4 +609,26 @@ struct efi_pxe { struct efi_pxe_mode *mode; };

+#define EFI_DRIVER_BINDING_PROTOCOL_GUID \

•   EFI_GUID(0x18a031ab, 0xb443, 0x4d1a,\
  

•            0xa5, 0xc0, 0x0c, 0x09, 0x26, 0x1e, 0x9f, 0x71)
  

+struct efi_driver_binding_protocol {

•   efi_status_t (EFIAPI * supported)(
  

I think the * should be up against 'supported'. Similarly below.

This is what checkpatch wants to see. Your suggestion leads to

ERROR: need consistent spacing around '*' (ctx:WxV) #25: FILE: include/efi_api.h:616: + efi_status_t (EFIAPI *supported)(

So I prefer not change this before checkpatch.pl is not giving a different result.

...

•                   struct efi_driver_binding_protocol *this,
  

•                   efi_handle_t controller_handle,
  

•                   struct efi_device_path *remaining_device_path);
  

•   efi_status_t (EFIAPI * start)(
  

•                   struct efi_driver_binding_protocol *this,
  

•                   efi_handle_t controller_handle,
  

•                   struct efi_device_path *remaining_device_path);
  

•   efi_status_t (EFIAPI * stop)(
  

•                   struct efi_driver_binding_protocol *this,
  

•                   efi_handle_t controller_handle,
  

•                   UINTN number_of_children,
  

•                   efi_handle_t child_handle_buffer);
  

These should have function comments.

...

•   u32 version;
  

•   efi_handle_t image_handle;
  

•   efi_handle_t driver_binding_handle;
  

+};

#endif diff --git a/include/efi_loader.h b/include/efi_loader.h index 9c68246c7c..f9f33e1d01 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -74,6 +74,7 @@ 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_driver_binding_protocol;

comment for this?

The GUIDs are defined in the UEFI spec. So maybe we should just put a comment above all of these.

Regards

Heinrich

...

extern const efi_guid_t efi_guid_loaded_image; extern const efi_guid_t efi_guid_device_path_to_text_protocol;

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 5a73ea5cd0..1069da7d79 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -18,6 +18,14 @@

DECLARE_GLOBAL_DATA_PTR;

+static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol,

•                                          void *registration,
  

•                                          void **protocol_interface);
  

+static efi_status_t EFIAPI efi_locate_handle_buffer(

•                   enum efi_locate_search_type search_type,
  

•                   const efi_guid_t *protocol, void *search_key,
  

•                   unsigned long *no_handles, efi_handle_t **buffer);
  

Is this a forward decl? Can you reorder (in a separate patch) to avoid it?

Add a missing comment.

Reported-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- Patch is directly applicable to efi-next.

Reported by Simon Re: [PATCH 15/23] efi_loader: implement ConnectController https://lists.denx.de/pipermail/u-boot/2017-August/304372.html --- include/efi_loader.h | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 46d684f6df..bc48bf4e11 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -67,6 +67,9 @@ extern 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;

+/* + * GUIDs of EFI protocols + */ 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;

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 77 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1069da7d79..c5a17b6252 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1052,9 +1052,84 @@ static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle, void *driver_image_handle, void *child_handle) { + struct efi_driver_binding_protocol *binding_protocol; + efi_handle_t child_handle_buffer; + unsigned long driver_count; + efi_handle_t *driver_handle_buffer; + size_t i; + UINTN number_of_children; + efi_status_t r; + size_t stop_count = 0; + EFI_ENTRY("%p, %p, %p", controller_handle, driver_image_handle, child_handle); - return EFI_EXIT(EFI_INVALID_PARAMETER); + + if (!efi_search_obj(controller_handle)) { + r = EFI_INVALID_PARAMETER; + goto out; + } + + /* Create list of driver handles */ + if (driver_image_handle) { + driver_handle_buffer = &driver_image_handle, + driver_count = 1; + /* Check that the handle supports driver binding protocol */ + r = efi_search_protocol(driver_image_handle, + &efi_guid_driver_binding_protocol, + NULL); + } else { + /* Get buffer with all handles with driver binding protocol */ + r = EFI_CALL(efi_locate_handle_buffer( + by_protocol, &efi_guid_driver_binding_protocol, + NULL, &driver_count, &driver_handle_buffer)); + } + if (r != EFI_SUCCESS) + goto out; + + /* Create list of child handles */ + if (child_handle) { + number_of_children = 1; + child_handle_buffer = &child_handle; + } else { + /* + * We do not fully support child handles. + * + * It is unclear from which handle and which protocols the + * list of child controllers should be collected. + */ + number_of_children = 0; + child_handle_buffer = NULL; + } + + for (i = 0; i < driver_count; ++i) { + r = EFI_CALL(efi_open_protocol( + driver_handle_buffer[i], + &efi_guid_driver_binding_protocol, + (void **)&binding_protocol, + driver_handle_buffer[i], NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); + if (r != EFI_SUCCESS) + continue; + + r = EFI_CALL(binding_protocol->stop(binding_protocol, + controller_handle, + number_of_children, + child_handle_buffer)); + if (r == EFI_SUCCESS) + ++stop_count; + EFI_CALL(efi_close_protocol(driver_handle_buffer[i], + &efi_guid_driver_binding_protocol, + driver_handle_buffer[i], NULL)); + } + + if (driver_image_handle) + efi_free_pool(driver_handle_buffer); + if (stop_count) + r = EFI_SUCCESS; + else + r = EFI_NOT_FOUND; +out: + return EFI_EXIT(r); }

efi_status_t EFIAPI efi_close_protocol(void *handle, const efi_guid_t *protocol,

On 08/31/2017 02:52 PM, Simon Glass wrote:

On 27 August 2017 at 06:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_boottime.c | 77 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1069da7d79..c5a17b6252 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1052,9 +1052,84 @@ static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle, void *driver_image_handle, void *child_handle)

Can you add a function comment?

Hello Simon,

the API functions (here DisconnectController) are documented in the UEFI spec. Is your idea that we should duplicate part of this information for all API functions? Or do you miss a comment on implementation details?

...

{

•   struct efi_driver_binding_protocol *binding_protocol;
  

•   efi_handle_t child_handle_buffer;
  

•   unsigned long driver_count;
  

•   efi_handle_t *driver_handle_buffer;
  

•   size_t i;
  

•   UINTN number_of_children;
  

Can we somehow use a lower-case type for this? Otherwise U-Boot will start to look like EFI and I will have to start taking antidepressants.

In different places the EFI API requires a bitness dependent unsigned integer.

Shall we globally rename UINTN to uintn? This is my patch to blame: 503f2695548 (efi_loader: correct size for tpl level)

Regards

Heinrich

...

•   efi_status_t r;
  

•   size_t stop_count = 0;
  

•   EFI_ENTRY("%p, %p, %p", controller_handle, driver_image_handle,
              child_handle);
  

•   return EFI_EXIT(EFI_INVALID_PARAMETER);
  

•   if (!efi_search_obj(controller_handle)) {
  

•           r = EFI_INVALID_PARAMETER;
  

•           goto out;
  

•   }
  

•   /* Create list of driver handles */
  

•   if (driver_image_handle) {
  

•           driver_handle_buffer = &driver_image_handle,
  

•           driver_count = 1;
  

•           /* Check that the handle supports driver binding protocol */
  

•           r = efi_search_protocol(driver_image_handle,
  

•                                   &efi_guid_driver_binding_protocol,
  

•                                   NULL);
  

•   } else {
  

•           /* Get buffer with all handles with driver binding protocol */
  

•           r = EFI_CALL(efi_locate_handle_buffer(
  

•                        by_protocol, &efi_guid_driver_binding_protocol,
  

•                        NULL, &driver_count, &driver_handle_buffer));
  

•   }
  

•   if (r != EFI_SUCCESS)
  

•           goto out;
  

•   /* Create list of child handles */
  

•   if (child_handle) {
  

•           number_of_children = 1;
  

•           child_handle_buffer = &child_handle;
  

•   } else {
  

•           /*
  

•            * We do not fully support child handles.
  

•            *
  

•            * It is unclear from which handle and which protocols the
  

•            * list of child controllers should be collected.
  

•            */
  

•           number_of_children = 0;
  

•           child_handle_buffer = NULL;
  

•   }
  

•   for (i = 0; i < driver_count; ++i) {
  

•           r = EFI_CALL(efi_open_protocol(
  

•                        driver_handle_buffer[i],
  

•                        &efi_guid_driver_binding_protocol,
  

•                        (void **)&binding_protocol,
  

•                        driver_handle_buffer[i], NULL,
  

•                        EFI_OPEN_PROTOCOL_GET_PROTOCOL));
  

•           if (r != EFI_SUCCESS)
  

•                   continue;
  

•           r = EFI_CALL(binding_protocol->stop(binding_protocol,
  

•                                               controller_handle,
  

•                                               number_of_children,
  

•                                               child_handle_buffer));
  

•           if (r == EFI_SUCCESS)
  

•                   ++stop_count;
  

•           EFI_CALL(efi_close_protocol(driver_handle_buffer[i],
  

•                                       &efi_guid_driver_binding_protocol,
  

•                                       driver_handle_buffer[i], NULL));
  

•   }
  

•   if (driver_image_handle)
  

•           efi_free_pool(driver_handle_buffer);
  

•   if (stop_count)
  

•           r = EFI_SUCCESS;
  

•   else
  

•           r = EFI_NOT_FOUND;
  

+out:

•   return EFI_EXIT(r);
  

}

efi_status_t EFIAPI efi_close_protocol(void *handle, const efi_guid_t *protocol,

2.14.1

On Wed, Sep 20, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:

Hi Heinrich,

On 15 September 2017 at 00:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 08/31/2017 02:52 PM, Simon Glass wrote:

...

On 27 August 2017 at 06:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de

lib/efi_loader/efi_boottime.c | 77 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1069da7d79..c5a17b6252 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1052,9 +1052,84 @@ static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle, void *driver_image_handle, void *child_handle)

Can you add a function comment?

Hello Simon,

the API functions (here DisconnectController) are documented in the UEFI spec. Is your idea that we should duplicate part of this information for all API functions? Or do you miss a comment on implementation details?

I think the code in U-Boot should stand alone, so arguments should be described here along with a one-line function description. The args are all void which is not good, but makes it even more important to document.

couple notes, fwiw..

1) As someone else implementing parts of UEFI interface, I'd find link to section in spec (or perhaps to http://wiki.phoenix.com/) to be more useful than re-writing the spec in our own words (and possibly getting it wrong)

2) Leif introduced (iirc, in the stub HII or unicode patch) efi_handle_t, and efi_string_t (and maybe we should add efi_char_t).. which we should probably use more extensively. Although I haven't wanted to go on a major housecleaning while we still have so many patches pending on list.. but would be a nice cleanup to make at some point.

BR, -R

...

...

...

{

•   struct efi_driver_binding_protocol *binding_protocol;
  

•   efi_handle_t child_handle_buffer;
  

•   unsigned long driver_count;
  

•   efi_handle_t *driver_handle_buffer;
  

•   size_t i;
  

•   UINTN number_of_children;
  

Can we somehow use a lower-case type for this? Otherwise U-Boot will start to look like EFI and I will have to start taking antidepressants.

In different places the EFI API requires a bitness dependent unsigned integer.

Shall we globally rename UINTN to uintn? This is my patch to blame: 503f2695548 (efi_loader: correct size for tpl level)

What does bitness-dependent mean? Do you mean 32-bit? It looks like this is just passed to a function, so could be uint or instead?

If it is 32-bit then uint32_t should do.

Regards, Simon

The length of a MAC address is 6. We have to set this length in the EFI_SIMPLE_NETWORK_MODE structure of the EFI_SIMPLE_NETWORK_PROTOCOL.

Without this patch iPXE fails to initialize the network with error message SNP MAC(001e0633bcbf,0x0) has invalid hardware address length 0

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_net.c | 1 + 1 file changed, 1 insertion(+)

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 0b949d86e8..75d7974b0e 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -259,6 +259,7 @@ int efi_net_register(void **handle) netobj->dp_end = dp_end; memcpy(netobj->dp_mac.mac.addr, eth_get_ethaddr(), 6); memcpy(netobj->net_mode.current_address.mac_addr, eth_get_ethaddr(), 6); + netobj->net_mode.hwaddr_size = 6; netobj->net_mode.max_packet_size = PKTSIZE;

netobj->pxe.mode = &netobj->pxe_mode;

U-Boot does not implement all functions of the simple network protocol. The unimplemented functions return either of EFI_SUCCESS and EFI_INVALID_PARAMETER.

The UEFI spec foresees to return EFI_UNSUPPORTED in these cases.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_net.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 75d7974b0e..38a3a4fac4 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -81,9 +81,7 @@ static efi_status_t EFIAPI efi_net_receive_filters( EFI_ENTRY("%p, %x, %x, %x, %lx, %p", this, enable, disable, reset_mcast_filter, mcast_filter_count, mcast_filter);

- /* XXX Do we care? */ - - return EFI_EXIT(EFI_SUCCESS); + return EFI_EXIT(EFI_UNSUPPORTED); }

static efi_status_t EFIAPI efi_net_station_address( @@ -92,7 +90,7 @@ static efi_status_t EFIAPI efi_net_station_address( { EFI_ENTRY("%p, %x, %p", this, reset, new_mac);

- return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_EXIT(EFI_UNSUPPORTED); }

static efi_status_t EFIAPI efi_net_statistics(struct efi_simple_network *this, @@ -101,7 +99,7 @@ static efi_status_t EFIAPI efi_net_statistics(struct efi_simple_network *this, { EFI_ENTRY("%p, %x, %p, %p", this, reset, stat_size, stat_table);

- return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_EXIT(EFI_UNSUPPORTED); }

static efi_status_t EFIAPI efi_net_mcastiptomac(struct efi_simple_network *this, @@ -121,7 +119,7 @@ static efi_status_t EFIAPI efi_net_nvdata(struct efi_simple_network *this, EFI_ENTRY("%p, %x, %lx, %lx, %p", this, read_write, offset, buffer_size, buffer);

- return EFI_EXIT(EFI_INVALID_PARAMETER); + return EFI_EXIT(EFI_UNSUPPORTED); }

static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this,

Remove extraneous commas. Add comment.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_api.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h index b2838125d7..029b57ca5e 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -530,11 +530,12 @@ struct efi_simple_network_mode { u8 media_present; };

-#define EFI_SIMPLE_NETWORK_RECEIVE_UNICAST 0x01, -#define EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST 0x02, -#define EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST 0x04, -#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS 0x08, -#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST 0x10, +/* receive_filters bit mask */ +#define EFI_SIMPLE_NETWORK_RECEIVE_UNICAST 0x01 +#define EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST 0x02 +#define EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST 0x04 +#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS 0x08 +#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST 0x10

struct efi_simple_network {

Hi Heinrich,

On 27 August 2017 at 06:53, Heinrich Schuchardt xypron.glpk@gmx.de 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 xypron.glpk@gmx.de

include/efi_loader.h | 1 + lib/efi_loader/efi_boottime.c | 130 +++++++++++++++++++++++++++++++----------- 2 files changed, 99 insertions(+), 32 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

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 = &protocol->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 = &protocol->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 = &protocol->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 = &protocol->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;
  

•   struct efi_handler *handler;
    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 (!handle || !protocol ||
  

•       (!protocol_interface && attributes !=
  

•        EFI_OPEN_PROTOCOL_TEST_PROTOCOL)) {
  

•   if (!protocol_interface && attributes !=
  

•       EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
            goto out;
  

•   }
  
    switch (attributes) {
    case EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL:
  

@@ -1191,33 +1278,12 @@ static efi_status_t EFIAPI efi_open_protocol( 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 (attributes !=
  

•                               EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
  

•                                   *protocol_interface =
  

•                                           handler->protocol_interface;
  

•                           }
  

•                           r = EFI_SUCCESS;
  

•                           goto out;
  

•                   }
  

•           }
  

•           goto unsupported;
  

•   }
  

•   r = efi_search_protocol(handle, protocol, &handler);
  

•   if (r != EFI_SUCCESS)
  

•           goto out;
  
  

-unsupported:

•   r = EFI_UNSUPPORTED;
  

•   r = efi_protocol_open(handler, protocol_interface, agent_handle,
  

•                         controller_handle, attributes);
  

out: return EFI_EXIT(r); } -- 2.14.1

The returned interrupt status was wrong.

As out transmit buffer is empty we need to always set EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT.

When we have received a packet we need to set EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT.

Furthermore we should call efi_timer_check() to handle events.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_api.h | 6 ++++++ lib/efi_loader/efi_net.c | 11 ++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h index 029b57ca5e..7d2355c033 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -537,6 +537,12 @@ struct efi_simple_network_mode { #define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS 0x08 #define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST 0x10

+/* interrupt status bit mask */ +#define EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT 0x01 +#define EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT 0x02 +#define EFI_SIMPLE_NETWORK_COMMAND_INTERRUPT 0x04 +#define EFI_SIMPLE_NETWORK_SOFTWARE_INTERRUPT 0x08 + struct efi_simple_network { u64 revision; diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 7659109386..e15f403c20 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -127,9 +127,14 @@ static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this, { EFI_ENTRY("%p, %p, %p", this, int_status, txbuf);

- /* We send packets synchronously, so nothing is outstanding */ - if (int_status) - *int_status = 0; + efi_timer_check(); + + if (int_status) { + /* We send packets synchronously, so nothing is outstanding */ + *int_status = EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; + if (new_rx_packet) + *int_status |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; + } if (txbuf) *txbuf = new_tx_packet;

The parent_handle of the loaded image must be set. Add the file path protocol from the provided parameter. Set system table. Add parameter checks.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index c5a17b6252..477809e4ca 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -774,27 +774,54 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, }; struct efi_loaded_image *info; struct efi_object *obj; + efi_status_t r;

EFI_ENTRY("%d, %p, %p, %p, %ld, %p", boot_policy, parent_image, file_path, source_buffer, source_size, image_handle); + + /* We do not support loading by device path, yet. */ + if (!source_buffer) { + r = EFI_NOT_FOUND; + goto out; + } + if (!parent_image || !image_handle) { + r = EFI_INVALID_PARAMETER; + goto out; + } + info = malloc(sizeof(*info)); + if (!info) { + r = EFI_OUT_OF_RESOURCES; + goto out; + } loaded_image_info_obj.protocols[0].protocol_interface = info; + loaded_image_info_obj.protocols[1].protocol_interface = file_path; obj = malloc(sizeof(loaded_image_info_obj)); + if (!obj) { + free(info); + r = EFI_OUT_OF_RESOURCES; + goto out; + } memset(info, 0, sizeof(*info)); memcpy(obj, &loaded_image_info_obj, sizeof(loaded_image_info_obj)); obj->handle = info; + info->system_table = &systab; + info->parent_handle = parent_image; info->file_path = file_path; info->reserved = efi_load_pe(source_buffer, info); if (!info->reserved) { free(info); free(obj); - return EFI_EXIT(EFI_UNSUPPORTED); + r = EFI_UNSUPPORTED; + goto out; }

*image_handle = info; list_add_tail(&obj->link, &efi_obj_list);

- return EFI_EXIT(EFI_SUCCESS); + r = EFI_SUCCESS; +out: + return EFI_EXIT(r); }

static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,

The watchdog is initialized with a 5 minute timeout period. It can be reset by SetWatchdogTimer. It is stopped by ExitBoottimeServices.

Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- cmd/bootefi.c | 1 + include/efi_loader.h | 4 +++ lib/efi_loader/Makefile | 2 +- lib/efi_loader/efi_boottime.c | 3 ++- lib/efi_loader/efi_watchdog.c | 58 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 lib/efi_loader/efi_watchdog.c

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 3196d86040..47771f87cc 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -132,6 +132,7 @@ static void efi_init_obj_list(void) #ifdef CONFIG_GENERATE_SMBIOS_TABLE efi_smbios_register(); #endif + efi_watchdog_register();

/* Initialize EFI runtime services */ efi_reset_system_init(); diff --git a/include/efi_loader.h b/include/efi_loader.h index f9f33e1d01..0c1f4e21ca 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -150,11 +150,15 @@ int efi_disk_register(void); int efi_gop_register(void); /* Called by bootefi to make the network interface available */ int efi_net_register(void **handle); +/* Called by bootefi to make the watchdog available */ +int efi_watchdog_register(void); /* Called by bootefi to make SMBIOS tables available */ 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); +/* Called by efi_set_watchdog_timer to reset the timer */ +efi_status_t efi_set_watchdog(unsigned long timeout);

/* Called from places to check whether a timer expired */ void efi_timer_check(void); diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 30bf343a36..6bca05aeb4 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 efi_device_path_to_text.o +obj-y += efi_memory.o efi_device_path_to_text.o efi_watchdog.o obj-$(CONFIG_LCD) += efi_gop.o obj-$(CONFIG_DM_VIDEO) += efi_gop.o obj-$(CONFIG_PARTITIONS) += efi_disk.o diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 477809e4ca..8f06209794 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -928,6 +928,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle, bootm_disable_interrupts();

/* Give the payload some time to boot */ + efi_set_watchdog(0); WATCHDOG_RESET();

return EFI_EXIT(EFI_SUCCESS); @@ -955,7 +956,7 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout, { EFI_ENTRY("%ld, 0x%"PRIx64", %ld, %p", timeout, watchdog_code, data_size, watchdog_data); - return efi_unsupported(__func__); + return EFI_EXIT(efi_set_watchdog(timeout)); }

static efi_status_t efi_bind_controller( diff --git a/lib/efi_loader/efi_watchdog.c b/lib/efi_loader/efi_watchdog.c new file mode 100644 index 0000000000..58c098e8bd --- /dev/null +++ b/lib/efi_loader/efi_watchdog.c @@ -0,0 +1,58 @@ +/* + * EFI device path interface + * + * Copyright (c) 2017 Heinrich Schuchardt + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <efi_loader.h> + +static struct efi_event *watchdog_timer_event; + +static void EFIAPI efi_watchdog_timer_notify(struct efi_event *event, + void *context) +{ + EFI_ENTRY("%p, %p", event, context); + + printf("\nEFI: Watchdog timeout\n"); + EFI_CALL_VOID(efi_reset_system(EFI_RESET_COLD, EFI_SUCCESS, 0, NULL)); + + EFI_EXIT(EFI_UNSUPPORTED); +} + +efi_status_t efi_set_watchdog(unsigned long timeout) +{ + efi_status_t r; + + if (timeout) + /* Reset watchdog */ + r = efi_set_timer(watchdog_timer_event, EFI_TIMER_RELATIVE, + 10000000 * timeout); + else + /* Deactivate watchdog */ + r = efi_set_timer(watchdog_timer_event, EFI_TIMER_STOP, 0); + return r; +} + +/* This gets called from do_bootefi_exec(). */ +int efi_watchdog_register(void) +{ + efi_status_t r; + + r = efi_create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, TPL_CALLBACK, + efi_watchdog_timer_notify, NULL, + &watchdog_timer_event); + if (r != EFI_SUCCESS) { + printf("ERROR: Failed to register watchdog event\n"); + return r; + } + /* Set watchdog to trigger after 5 minutes */ + r = efi_set_watchdog(300); + if (r != EFI_SUCCESS) { + printf("ERROR: Failed to set watchdog timer\n"); + return r; + } + return 0; +}