[U-Boot] [PATCH 00/16] efi_loader: implement driver management

[U-Boot] [PATCH 00/16] efi_loader: implement driver management

[Heinrich Schuchardt]

This series implements the OpenProtocolInformation, ConnectController,
and DisconnectController boot services.

The EFI application creating a new device will call ConnectController
to request the installation of all necessary drivers. Before
deleting the device it will call DisconectController to remove all
child controllers and to detach the driver.

E.g. iPXE may be used to connect an iSCSI target. It then creates a
handle by installing the EFI_BLOCK_IO_PROTOCOL. It calls
ConnectController and expects U-Boot to create the partition handles
and to provide the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. (The driver
connecting to the EFI_BLOCK_IO_PROTOCOL is not yet implemented in
U-Boot.)

A self test is provided for the driver management.
The bootefi selftest now creates colored output.
Two bug fixes are included.

To complete the implementation of the EFI boot services the following
future work is needed:
* manage events in a linked list (see Rob's proposal)
* implement ReinstallProtocol and RegisterProtocolNotify


Heinrich Schuchardt (16):
  efi_selftest: colored test output
  efi_loader: simplify efi_remove_all_protocols
  efi_selftest: do not try to close device path protocol
  efi_loader: list of open protocol infos
  efi_loader: open_info in OpenProtocol
  efi_loader: open_info in CloseProtocol
  efi_loader: implement OpenProtocolInformation
  efi_loader: debug output installed device path
  efi_loader: implement ConnectController
  efi_loader: fix signature of efi_disconnect_controller
  efi_loader: implement DisconnectController
  efi_loader: disconnect controllers in UninstallProtocol
  efi_selftest: remove todo in manage protocols
  efi_selftest: remove todo in device path test
  efi_selftest: test for (Dis)ConnectController
  efi_loader: consistently use efi_handle_t for handles

 cmd/bootefi.c                                   |  10 +-
 include/efi_api.h                               |  48 +-
 include/efi_loader.h                            |  31 +-
 include/efi_selftest.h                          |  27 +-
 lib/efi_loader/efi_boottime.c                   | 757 +++++++++++++++++++++---
 lib/efi_loader/efi_console.c                    |   6 +-
 lib/efi_selftest/Makefile                       |   1 +
 lib/efi_selftest/efi_selftest.c                 |  25 +-
 lib/efi_selftest/efi_selftest_console.c         |  13 +-
 lib/efi_selftest/efi_selftest_controllers.c     | 385 ++++++++++++
 lib/efi_selftest/efi_selftest_devicepath.c      |  48 +-
 lib/efi_selftest/efi_selftest_manageprotocols.c |  22 +-
 12 files changed, 1209 insertions(+), 164 deletions(-)
 create mode 100644 lib/efi_selftest/efi_selftest_controllers.c

-- 
2.14.2

[U-Boot] [PATCH 01/16] efi_selftest: colored test output

[Heinrich Schuchardt]

Add color coding to output:
test section    blue
success         green
errors          red
todo            yellow
summary         white
others          light gray

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_selftest.h                  | 27 +++++++++++++++++----------
 lib/efi_selftest/efi_selftest.c         | 25 ++++++++++++++-----------
 lib/efi_selftest/efi_selftest_console.c | 13 +++++++++----
 3 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/include/efi_selftest.h b/include/efi_selftest.h
index be5ba4bfa9..344f28ed36 100644
--- a/include/efi_selftest.h
+++ b/include/efi_selftest.h
@@ -18,14 +18,20 @@
 #define EFI_ST_SUCCESS 0
 #define EFI_ST_FAILURE 1
 
+/*
+ * Prints a message.
+ */
+#define efi_st_printf(...) \
+	(efi_st_printc(-1, __VA_ARGS__))
+
 /*
  * Prints an error message.
  *
  * @...	format string followed by fields to print
  */
 #define efi_st_error(...) \
-	(efi_st_printf("%s(%u):\nERROR: ", __FILE__, __LINE__), \
-	efi_st_printf(__VA_ARGS__)) \
+	(efi_st_printc(EFI_LIGHTRED, "%s(%u):\nERROR: ", __FILE__, __LINE__), \
+	efi_st_printc(EFI_LIGHTRED, __VA_ARGS__))
 
 /*
  * Prints a TODO message.
@@ -33,8 +39,8 @@
  * @...	format string followed by fields to print
  */
 #define efi_st_todo(...) \
-	(efi_st_printf("%s(%u):\nTODO: ", __FILE__, __LINE__), \
-	efi_st_printf(__VA_ARGS__)) \
+	(efi_st_printc(EFI_YELLOW, "%s(%u):\nTODO: ", __FILE__, __LINE__), \
+	efi_st_printc(EFI_YELLOW, __VA_ARGS__)) \
 
 /*
  * A test may be setup and executed at boottime,
@@ -61,14 +67,15 @@ extern struct efi_simple_input_interface *con_in;
 void efi_st_exit_boot_services(void);
 
 /*
- * Print a pointer to an u16 string
+ * Print a colored message
  *
- * @pointer: pointer
- * @buf: pointer to buffer address
- * on return position of terminating zero word
+ * @color	color
+ * @fmt		printf format
+ * @...		arguments to be printed
+ *		on return position of terminating zero word
  */
-void efi_st_printf(const char *fmt, ...)
-		 __attribute__ ((format (__printf__, 1, 2)));
+void efi_st_printc(int color, const char *fmt, ...)
+		 __attribute__ ((format (__printf__, 2, 3)));
 
 /*
  * Compare memory.
diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c
index 4e5a12c47c..fc5ef254a1 100644
--- a/lib/efi_selftest/efi_selftest.c
+++ b/lib/efi_selftest/efi_selftest.c
@@ -65,7 +65,7 @@ void efi_st_exit_boot_services(void)
 		efi_st_error("ExitBootServices did not return EFI_SUCCESS\n");
 		return;
 	}
-	efi_st_printf("\nBoot services terminated\n");
+	efi_st_printc(EFI_WHITE, "\nBoot services terminated\n");
 }
 
 /*
@@ -81,13 +81,14 @@ static int setup(struct efi_unit_test *test, unsigned int *failures)
 
 	if (!test->setup)
 		return EFI_ST_SUCCESS;
-	efi_st_printf("\nSetting up '%s'\n", test->name);
+	efi_st_printc(EFI_LIGHTBLUE, "\nSetting up '%s'\n", test->name);
 	ret = test->setup(handle, systable);
 	if (ret != EFI_ST_SUCCESS) {
 		efi_st_error("Setting up '%s' failed\n", test->name);
 		++*failures;
 	} else {
-		efi_st_printf("Setting up '%s' succeeded\n", test->name);
+		efi_st_printc(EFI_LIGHTGREEN,
+			      "Setting up '%s' succeeded\n", test->name);
 	}
 	return ret;
 }
@@ -105,13 +106,14 @@ static int execute(struct efi_unit_test *test, unsigned int *failures)
 
 	if (!test->execute)
 		return EFI_ST_SUCCESS;
-	efi_st_printf("\nExecuting '%s'\n", test->name);
+	efi_st_printc(EFI_LIGHTBLUE, "\nExecuting '%s'\n", test->name);
 	ret = test->execute();
 	if (ret != EFI_ST_SUCCESS) {
 		efi_st_error("Executing '%s' failed\n", test->name);
 		++*failures;
 	} else {
-		efi_st_printf("Executing '%s' succeeded\n", test->name);
+		efi_st_printc(EFI_LIGHTGREEN,
+			      "Executing '%s' succeeded\n", test->name);
 	}
 	return ret;
 }
@@ -129,13 +131,14 @@ static int teardown(struct efi_unit_test *test, unsigned int *failures)
 
 	if (!test->teardown)
 		return EFI_ST_SUCCESS;
-	efi_st_printf("\nTearing down '%s'\n", test->name);
+	efi_st_printc(EFI_LIGHTBLUE, "\nTearing down '%s'\n", test->name);
 	ret = test->teardown();
 	if (ret != EFI_ST_SUCCESS) {
 		efi_st_error("Tearing down '%s' failed\n", test->name);
 		++*failures;
 	} else {
-		efi_st_printf("Tearing down '%s' succeeded\n", test->name);
+		efi_st_printc(EFI_LIGHTGREEN,
+			      "Tearing down '%s' succeeded\n", test->name);
 	}
 	return ret;
 }
@@ -262,12 +265,12 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
 		}
 	}
 
-	efi_st_printf("\nTesting EFI API implementation\n");
+	efi_st_printc(EFI_WHITE, "\nTesting EFI API implementation\n");
 
 	if (testname)
-		efi_st_printf("\nSelected test: '%ps'\n", testname);
+		efi_st_printc(EFI_WHITE, "\nSelected test: '%ps'\n", testname);
 	else
-		efi_st_printf("\nNumber of tests to execute: %u\n",
+		efi_st_printc(EFI_WHITE, "\nNumber of tests to execute: %u\n",
 			      ll_entry_count(struct efi_unit_test,
 					     efi_unit_test));
 
@@ -291,7 +294,7 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
 			&failures);
 
 	/* Give feedback */
-	efi_st_printf("\nSummary: %u failures\n\n", failures);
+	efi_st_printc(EFI_WHITE, "\nSummary: %u failures\n\n", failures);
 
 	/* Reset system */
 	efi_st_printf("Preparing for reset. Press any key.\n");
diff --git a/lib/efi_selftest/efi_selftest_console.c b/lib/efi_selftest/efi_selftest_console.c
index 6a7fd20da5..7920c961ba 100644
--- a/lib/efi_selftest/efi_selftest_console.c
+++ b/lib/efi_selftest/efi_selftest_console.c
@@ -130,12 +130,13 @@ static void int2dec(s32 value, u16 **buf)
 }
 
 /*
- * Print a formatted string to the EFI console
+ * Print a colored formatted string to the EFI console
  *
- * @fmt: format string
- * @...: optional arguments
+ * @color	color
+ * @fmt		format string
+ * @...		optional arguments
  */
-void efi_st_printf(const char *fmt, ...)
+void efi_st_printc(int color, const char *fmt, ...)
 {
 	va_list args;
 	u16 buf[160];
@@ -215,7 +216,11 @@ void efi_st_printf(const char *fmt, ...)
 	}
 	va_end(args);
 	*pos = 0;
+	if (color >= 0)
+		con_out->set_attribute(con_out, (unsigned long)color);
 	con_out->output_string(con_out, buf);
+	if (color >= 0)
+		con_out->set_attribute(con_out, EFI_LIGHTGRAY);
 }
 
 /*
-- 
2.14.2

[U-Boot] [PATCH 02/16] efi_loader: simplify efi_remove_all_protocols

[Heinrich Schuchardt]

Replace list_for_each_safe() and list_entry() by
list_for_each_entry_safe().

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_boottime.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 02bc9fdcf0..99eb36c306 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -425,18 +425,15 @@ efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol,
 efi_status_t efi_remove_all_protocols(const void *handle)
 {
 	struct efi_object *efiobj;
-	struct list_head *lhandle;
-	struct list_head *pos;
+	struct efi_handler *protocol;
+	struct efi_handler *pos;
 
 	efiobj = efi_search_obj(handle);
 	if (!efiobj)
 		return EFI_INVALID_PARAMETER;
-	list_for_each_safe(lhandle, pos, &efiobj->protocols) {
-		struct efi_handler *protocol;
+	list_for_each_entry_safe(protocol, pos, &efiobj->protocols, link) {
 		efi_status_t ret;
 
-		protocol = list_entry(lhandle, struct efi_handler, link);
-
 		ret = efi_remove_protocol(handle, protocol->guid,
 					  protocol->protocol_interface);
 		if (ret != EFI_SUCCESS)
-- 
2.14.2

[U-Boot] [PATCH 03/16] efi_selftest: do not try to close device path protocol

[Heinrich Schuchardt]

CloseProtocol cannot be called without agent handle.

There is no need to close the device path protocol if
it has been opened without agent handle.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_selftest/efi_selftest_devicepath.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c
index 1ab54ebb37..d9a6697892 100644
--- a/lib/efi_selftest/efi_selftest_devicepath.c
+++ b/lib/efi_selftest/efi_selftest_devicepath.c
@@ -299,10 +299,10 @@ static int execute(void)
 			efi_st_error("FreePool failed\n");
 			return EFI_ST_FAILURE;
 		}
-		ret = boottime->close_protocol(handles[i], &guid_device_path,
-					       NULL, NULL);
-		if (ret != EFI_SUCCESS)
-			efi_st_todo("Cannot close device path protocol.\n");
+		/*
+		 * CloseProtocol cannot be called without agent handle.
+		 * There is no need to close the device path protocol.
+		 */
 	}
 	ret = boottime->free_pool(handles);
 	if (ret != EFI_SUCCESS) {
-- 
2.14.2

[U-Boot] [PATCH 04/16] efi_loader: list of open protocol infos

[Heinrich Schuchardt]

Add a list of open protocol infos to each protocol of a handle.

Provide helper functions to access the list items.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_loader.h          | 15 ++++++++++++++-
 lib/efi_loader/efi_boottime.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 6185055e78..637e6e166d 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -96,15 +96,28 @@ extern const efi_guid_t efi_file_info_guid;
 extern unsigned int __efi_runtime_start, __efi_runtime_stop;
 extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
 
+/*
+ * When a protocol is opened a open protocol info entry is created.
+ * These are maintained in a list.
+ */
+struct efi_open_protocol_info_item {
+	/* Link to the list of open protocol info entries of a protocol */
+	struct list_head link;
+	struct efi_open_protocol_info_entry info;
+};
+
 /*
  * When the UEFI payload wants to open a protocol on an object to get its
  * interface (usually a struct with callback functions), this struct maps the
- * protocol GUID to the respective protocol interface */
+ * protocol GUID to the respective protocol interface
+ */
 struct efi_handler {
 	/* Link to the list of protocols of a handle */
 	struct list_head link;
 	const efi_guid_t *guid;
 	void *protocol_interface;
+	/* Link to the list of open protocol info items */
+	struct list_head open_infos;
 };
 
 /*
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 99eb36c306..f5ec2a8866 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -814,6 +814,40 @@ struct efi_object *efi_search_obj(const void *handle)
 	return NULL;
 }
 
+/*
+ * Create open protocol info entry and add it to a protocol.
+ *
+ * @handler	handler of a protocol
+ * @return	open protocol info entry
+ */
+static struct efi_open_protocol_info_entry *efi_create_open_info(
+			struct efi_handler *handler)
+{
+	struct efi_open_protocol_info_item *item;
+
+	item = calloc(1, sizeof(struct efi_open_protocol_info_item));
+	if (!item)
+		return NULL;
+	/* Append the item to the open protocol info list. */
+	list_add_tail(&item->link, &handler->open_infos);
+
+	return &item->info;
+}
+
+/*
+ * Remove an open protocol info entry from a protocol.
+ *
+ * @handler	handler of a protocol
+ * @return	status code
+ */
+static efi_status_t efi_delete_open_info(
+			struct efi_open_protocol_info_item *item)
+{
+	list_del(&item->link);
+	free(item);
+	return EFI_SUCCESS;
+}
+
 /*
  * Install new protocol on a handle.
  *
@@ -840,6 +874,7 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol,
 		return EFI_OUT_OF_RESOURCES;
 	handler->guid = protocol;
 	handler->protocol_interface = protocol_interface;
+	INIT_LIST_HEAD(&handler->open_infos);
 	list_add_tail(&handler->link, &efiobj->protocols);
 	return EFI_SUCCESS;
 }
-- 
2.14.2

[U-Boot] [PATCH 05/16] efi_loader: open_info in OpenProtocol

[Heinrich Schuchardt]

efi_open_protocol has to keep track of opened protocols.

OpenProtocol enters the agent and controller handle
information into this list.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_boottime.c | 107 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 103 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index f5ec2a8866..bb80c6066a 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2100,6 +2100,101 @@ static void EFIAPI efi_set_mem(void *buffer, size_t size, uint8_t value)
 	EFI_EXIT(EFI_SUCCESS);
 }
 
+/*
+ * Open protocol interface on a handle.
+ *
+ * @handler		handler of a protocol
+ * @protocol_interface	interface implementing the protocol
+ * @agent_handle	handle of the driver
+ * @controller_handle	handle of the controller
+ * @attributes		attributes indicating how to open the protocol
+ * @return		status code
+ */
+static efi_status_t efi_protocol_open(
+			struct efi_handler *handler,
+			void **protocol_interface, void *agent_handle,
+			void *controller_handle, uint32_t attributes)
+{
+	struct efi_open_protocol_info_item *item;
+	struct efi_open_protocol_info_entry *match = NULL;
+	bool opened_by_driver = false;
+	bool opened_exclusive = false;
+
+	/* If there is no agent, only return the interface */
+	if (!agent_handle)
+		goto out;
+
+	/* For TEST_PROTOCOL ignore interface attribute */
+	if (attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
+		*protocol_interface = NULL;
+
+	/*
+	 * Check if the protocol is already opened by a driver with the same
+	 * attributes or opened exclusively
+	 */
+	list_for_each_entry(item, &handler->open_infos, link) {
+		if (item->info.agent_handle == agent_handle) {
+			if ((attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) &&
+			    (item->info.attributes == attributes))
+				return EFI_ALREADY_STARTED;
+		}
+		if (item->info.attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE)
+			opened_exclusive = true;
+	}
+
+	/* Only one controller can open the protocol exclusively */
+	if (opened_exclusive && attributes &
+	    (EFI_OPEN_PROTOCOL_EXCLUSIVE | EFI_OPEN_PROTOCOL_BY_DRIVER))
+		return EFI_ACCESS_DENIED;
+
+	/* Prepare exclusive opening */
+	if (attributes & EFI_OPEN_PROTOCOL_EXCLUSIVE) {
+		/* Try to disconnect controllers */
+		list_for_each_entry(item, &handler->open_infos, link) {
+			if (item->info.attributes ==
+					EFI_OPEN_PROTOCOL_BY_DRIVER)
+				EFI_CALL(efi_disconnect_controller(
+						item->info.controller_handle,
+						item->info.agent_handle,
+						NULL));
+		}
+		opened_by_driver = false;
+		/* Check if all controllers are disconnected */
+		list_for_each_entry(item, &handler->open_infos, link) {
+			if (item->info.attributes & EFI_OPEN_PROTOCOL_BY_DRIVER)
+				opened_by_driver = true;
+		}
+		/* Only one controller can be conncected */
+		if (opened_by_driver)
+			return EFI_ACCESS_DENIED;
+	}
+
+	/* Find existing entry */
+	list_for_each_entry(item, &handler->open_infos, link) {
+		if (item->info.agent_handle == agent_handle &&
+		    item->info.controller_handle == controller_handle)
+			match = &item->info;
+	}
+	/* None found, create one */
+	if (!match) {
+		match = efi_create_open_info(handler);
+		if (!match)
+			return EFI_OUT_OF_RESOURCES;
+	}
+
+	match->agent_handle = agent_handle;
+	match->controller_handle = controller_handle;
+	match->attributes = attributes;
+	match->open_count++;
+
+out:
+	/* For TEST_PROTOCOL ignore interface attribute. */
+	if (attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
+		*protocol_interface = handler->protocol_interface;
+
+	return EFI_SUCCESS;
+}
+
 /*
  * Open protocol interface on a handle.
  *
@@ -2141,12 +2236,16 @@ static efi_status_t EFIAPI efi_open_protocol(
 	case EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER:
 		if (controller_handle == handle)
 			goto out;
+		/* fall-through */
 	case EFI_OPEN_PROTOCOL_BY_DRIVER:
 	case EFI_OPEN_PROTOCOL_BY_DRIVER | EFI_OPEN_PROTOCOL_EXCLUSIVE:
-		if (controller_handle == NULL)
+		/* Check that the controller handle is valid */
+		if (!efi_search_obj(controller_handle))
 			goto out;
+		/* fall-through */
 	case EFI_OPEN_PROTOCOL_EXCLUSIVE:
-		if (agent_handle == NULL)
+		/* Check that the agent handle is valid */
+		if (!efi_search_obj(agent_handle))
 			goto out;
 		break;
 	default:
@@ -2157,8 +2256,8 @@ static efi_status_t EFIAPI efi_open_protocol(
 	if (r != EFI_SUCCESS)
 		goto out;
 
-	if (attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
-		*protocol_interface = handler->protocol_interface;
+	r = efi_protocol_open(handler, protocol_interface, agent_handle,
+			      controller_handle, attributes);
 out:
 	return EFI_EXIT(r);
 }
-- 
2.14.2

[U-Boot] [PATCH 06/16] efi_loader: open_info in CloseProtocol

[Heinrich Schuchardt]

efi_open_protocol and efi_close_protocol have to keep track of
opened protocols.

Check if the protocol was opened for the same agent and
controller.

Remove all open protocol information for this pair.

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index bb80c6066a..a527e33141 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1675,9 +1675,33 @@ static efi_status_t EFIAPI efi_close_protocol(void *handle,
 					      void *agent_handle,
 					      void *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, %pUl, %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;
+
+	r = 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;
+			break;
+		}
+	}
+out:
+	return EFI_EXIT(r);
 }
 
 /*
-- 
2.14.2

[U-Boot] [PATCH 07/16] efi_loader: implement OpenProtocolInformation

[Heinrich Schuchardt]

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

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index a527e33141..44c9da0a7c 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1722,9 +1722,48 @@ static efi_status_t EFIAPI efi_open_protocol_information(efi_handle_t handle,
 			struct efi_open_protocol_info_entry **entry_buffer,
 			efi_uintn_t *entry_count)
 {
+	unsigned long buffer_size;
+	unsigned long count;
+	struct efi_handler *handler;
+	struct efi_open_protocol_info_item *item;
+	efi_status_t r;
+
 	EFI_ENTRY("%p, %pUl, %p, %p", handle, protocol, entry_buffer,
 		  entry_count);
-	return EFI_EXIT(EFI_NOT_FOUND);
+
+	/* Check parameters */
+	if (!entry_buffer) {
+		r = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+	r = efi_search_protocol(handle, protocol, &handler);
+	if (r != EFI_SUCCESS)
+		goto out;
+
+	/* Count entries */
+	count = 0;
+	list_for_each_entry(item, &handler->open_infos, link) {
+		++count;
+	}
+	*entry_count = count;
+	*entry_buffer = NULL;
+	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;
+	list_for_each_entry_reverse(item, &handler->open_infos, link) {
+		if (item->info.open_count)
+			(*entry_buffer)[--count] = item->info;
+	}
+out:
+	return EFI_EXIT(r);
 }
 
 /*
-- 
2.14.2

[U-Boot] [PATCH 08/16] efi_loader: debug output installed device path

[Heinrich Schuchardt]

When a device path protocol is installed write the device
path to the console in debug mode.

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 44c9da0a7c..b5d6808bf7 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -876,6 +876,13 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol,
 	handler->protocol_interface = protocol_interface;
 	INIT_LIST_HEAD(&handler->open_infos);
 	list_add_tail(&handler->link, &efiobj->protocols);
+#if _DEBUG == 1
+	if (!guidcmp(&efi_guid_device_path, protocol)) {
+		debug("%sEFI installed device path: %ls\n", __efi_nesting(),
+		      efi_dp_str((struct efi_device_path *)
+				 protocol_interface));
+	}
+#endif
 	return EFI_SUCCESS;
 }
 
-- 
2.14.2

[U-Boot] [PATCH 09/16] efi_loader: implement ConnectController

[Heinrich Schuchardt]

Implement the ConnectController boot service.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_api.h             |  22 ++++++
 include/efi_loader.h          |   2 +
 lib/efi_loader/efi_boottime.c | 178 ++++++++++++++++++++++++++++++++++++------
 3 files changed, 178 insertions(+), 24 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 46963f2891..81e580dbbc 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -805,4 +805,26 @@ struct efi_file_info {
 	s16 file_name[0];
 };
 
+#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,
+			efi_uintn_t 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 637e6e166d..9e1ae8866b 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -88,6 +88,8 @@ uint16_t *efi_dp_str(struct efi_device_path *dp);
 extern const efi_guid_t efi_global_variable_guid;
 extern const efi_guid_t efi_guid_console_control;
 extern const efi_guid_t efi_guid_device_path;
+/* GUID of the EFI_DRIVER_BINDING_PROTOCOL */
+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;
 extern const efi_guid_t efi_simple_file_system_protocol_guid;
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index b5d6808bf7..6cc0659eb9 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -56,6 +56,9 @@ static volatile void *efi_gd, *app_gd;
 
 static int entry_count;
 static int nesting_level;
+/* GUID of the EFI_DRIVER_BINDING_PROTOCOL */
+const efi_guid_t efi_guid_driver_binding_protocol =
+			EFI_DRIVER_BINDING_PROTOCOL_GUID;
 
 /* Called on every callback entry */
 int __efi_entry_check(void)
@@ -1619,30 +1622,6 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
 	return EFI_EXIT(efi_set_watchdog(timeout));
 }
 
-/*
- * Connect a controller to a driver.
- *
- * This function implements the ConnectController service.
- * See the Unified Extensible Firmware Interface (UEFI) specification
- * for details.
- *
- * @controller_handle	handle of the controller
- * @driver_image_handle	handle of the driver
- * @remain_device_path	device path of a child controller
- * @recursive		true to connect all child controllers
- * @return		status code
- */
-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_ENTRY("%p, %p, %p, %d", controller_handle, driver_image_handle,
-		  remain_device_path, recursive);
-	return EFI_EXIT(EFI_NOT_FOUND);
-}
-
 /*
  * Disconnect a controller from a driver.
  *
@@ -2352,6 +2331,157 @@ static efi_status_t EFIAPI efi_handle_protocol(void *handle,
 				 NULL, EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL);
 }
 
+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;
+	size_t 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;
+				}
+			}
+		}
+	}
+
+	/*
+	 * TODO: Some overrides are not yet implemented:
+	 * - Platform Driver Override
+	 * - 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;
+}
+
+/*
+ * Connect a controller to a driver.
+ *
+ * This function implements the ConnectController service.
+ * See the Unified Extensible Firmware Interface (UEFI) specification
+ * for details.
+ *
+ * @controller_handle	handle of the controller
+ * @driver_image_handle	handle of the driver
+ * @remain_device_path	device path of a child controller
+ * @recursive		true to connect all child controllers
+ * @return		status code
+ */
+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_status_t ret = EFI_NOT_FOUND;
+	struct efi_object *efiobj;
+
+	EFI_ENTRY("%p, %p, %p, %d", controller_handle, driver_image_handle,
+		  remain_device_path, recursive);
+
+	efiobj = efi_search_obj(controller_handle);
+	if (!efiobj) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	r = efi_connect_single_controller(controller_handle,
+					  driver_image_handle,
+					  remain_device_path);
+	if (r == EFI_SUCCESS)
+		ret = EFI_SUCCESS;
+	if (recursive) {
+		struct efi_handler *handler;
+		struct efi_open_protocol_info_item *item;
+
+		list_for_each_entry(handler, &efiobj->protocols, link) {
+			list_for_each_entry(item, &handler->open_infos, link) {
+				if (item->info.attributes &
+				    EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) {
+					r = EFI_CALL(efi_connect_controller(
+						item->info.controller_handle,
+						driver_image_handle,
+						remain_device_path,
+						recursive));
+					if (r == EFI_SUCCESS)
+						ret = EFI_SUCCESS;
+				}
+			}
+		}
+	}
+	/*  Check for child controller specified by end node */
+	if (ret != EFI_SUCCESS && remain_device_path &&
+	    remain_device_path->type == DEVICE_PATH_TYPE_END)
+		ret = EFI_SUCCESS;
+out:
+	return EFI_EXIT(ret);
+}
+
 static const struct efi_boot_services efi_boot_services = {
 	.hdr = {
 		.headersize = sizeof(struct efi_table_hdr),
-- 
2.14.2

[U-Boot] [PATCH 10/16] efi_loader: fix signature of efi_disconnect_controller

[Heinrich Schuchardt]

Handles should be passed as efi_handle_t and not as void *.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_api.h             | 6 ++++--
 lib/efi_loader/efi_boottime.c | 7 ++++---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 81e580dbbc..7164492f83 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -125,8 +125,10 @@ struct efi_boot_services {
 			efi_handle_t *driver_image_handle,
 			struct efi_device_path *remaining_device_path,
 			bool recursive);
-	efi_status_t (EFIAPI *disconnect_controller)(void *controller_handle,
-			void *driver_image_handle, void *child_handle);
+	efi_status_t (EFIAPI *disconnect_controller)(
+			efi_handle_t controller_handle,
+			efi_handle_t driver_image_handle,
+			efi_handle_t child_handle);
 #define EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL  0x00000001
 #define EFI_OPEN_PROTOCOL_GET_PROTOCOL        0x00000002
 #define EFI_OPEN_PROTOCOL_TEST_PROTOCOL       0x00000004
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 6cc0659eb9..3094b26e5d 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1634,9 +1634,10 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
  * @child_handle	handle of the child to destroy
  * @return		status code
  */
-static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle,
-						     void *driver_image_handle,
-						     void *child_handle)
+static efi_status_t EFIAPI efi_disconnect_controller(
+				efi_handle_t controller_handle,
+				efi_handle_t driver_image_handle,
+				efi_handle_t child_handle)
 {
 	EFI_ENTRY("%p, %p, %p", controller_handle, driver_image_handle,
 		  child_handle);
-- 
2.14.2

[U-Boot] [PATCH 11/16] efi_loader: implement DisconnectController

[Heinrich Schuchardt]

Unfortunately we need a forward declaration because both
OpenProtocol and CloseProtocol have to call DisconnectController.
And DisconnectController calls both OpenProtcol and CloseProtocol.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_boottime.c | 283 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 261 insertions(+), 22 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 3094b26e5d..bf60b79e93 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -60,6 +60,10 @@ static int nesting_level;
 const efi_guid_t efi_guid_driver_binding_protocol =
 			EFI_DRIVER_BINDING_PROTOCOL_GUID;
 
+static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle,
+						     void *driver_image_handle,
+						     void *child_handle);
+
 /* Called on every callback entry */
 int __efi_entry_check(void)
 {
@@ -958,6 +962,108 @@ static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle,
 	return EFI_EXIT(EFI_ACCESS_DENIED);
 }
 
+/*
+ * Get all drivers associated to a controller.
+ * The allocated buffer has to be freed with free().
+ *
+ * @efiobj			handle of the controller
+ * @protocol			protocol guid (optional)
+ * @number_of_drivers		number of child controllers
+ * @driver_handle_buffer	handles of the the drivers
+ * @return			status code
+ */
+static efi_status_t efi_get_drivers(struct efi_object *efiobj,
+				    const efi_guid_t *protocol,
+				    efi_uintn_t *number_of_drivers,
+				    efi_handle_t **driver_handle_buffer)
+{
+	struct efi_handler *handler;
+	struct efi_open_protocol_info_item *item;
+	efi_uintn_t count = 0, i;
+	bool duplicate;
+
+	/* Count all driver associations */
+	list_for_each_entry(handler, &efiobj->protocols, link) {
+		if (protocol && guidcmp(handler->guid, protocol))
+			continue;
+		list_for_each_entry(item, &handler->open_infos, link) {
+			if (item->info.attributes &
+			    EFI_OPEN_PROTOCOL_BY_DRIVER)
+				++count;
+		}
+	}
+	/*
+	 * Create buffer. In case of duplicate driver assignments the buffer
+	 * will be too large. But that does not harm.
+	 */
+	*number_of_drivers = 0;
+	*driver_handle_buffer = calloc(count, sizeof(efi_handle_t));
+	if (!*driver_handle_buffer)
+		return EFI_OUT_OF_RESOURCES;
+	/* Collect unique driver handles */
+	list_for_each_entry(handler, &efiobj->protocols, link) {
+		if (protocol && guidcmp(handler->guid, protocol))
+			continue;
+		list_for_each_entry(item, &handler->open_infos, link) {
+			if (item->info.attributes &
+			    EFI_OPEN_PROTOCOL_BY_DRIVER) {
+				/* Check this is a new driver */
+				duplicate = false;
+				for (i = 0; i < *number_of_drivers; ++i) {
+					if ((*driver_handle_buffer)[i] ==
+					    item->info.agent_handle)
+						duplicate = true;
+				}
+				/* Copy handle to buffer */
+				if (!duplicate) {
+					i = (*number_of_drivers)++;
+					(*driver_handle_buffer)[i] =
+						item->info.agent_handle;
+				}
+			}
+		}
+	}
+	return EFI_SUCCESS;
+}
+
+/*
+ * Disconnect all drivers from a controller.
+ *
+ * This function implements the DisconnectController service.
+ * See the Unified Extensible Firmware Interface (UEFI) specification
+ * for details.
+ *
+ * @efiobj		handle of the controller
+ * @protocol		protocol guid (optional)
+ * @child_handle	handle of the child to destroy
+ * @return		status code
+ */
+static efi_status_t efi_disconnect_all_drivers(
+				struct efi_object *efiobj,
+				const efi_guid_t *protocol,
+				efi_handle_t child_handle)
+{
+	efi_uintn_t number_of_drivers;
+	efi_handle_t *driver_handle_buffer;
+	efi_status_t r, ret = EFI_NOT_FOUND;
+
+	ret = efi_get_drivers(efiobj, protocol, &number_of_drivers,
+			      &driver_handle_buffer);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	while (number_of_drivers) {
+		r = EFI_CALL(efi_disconnect_controller(
+				efiobj->handle,
+				driver_handle_buffer[--number_of_drivers],
+				child_handle));
+		if (r == EFI_SUCCESS)
+			ret = r;
+	}
+	free(driver_handle_buffer);
+	return ret;
+}
+
 /*
  * Uninstall protocol interface.
  *
@@ -1622,28 +1728,6 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
 	return EFI_EXIT(efi_set_watchdog(timeout));
 }
 
-/*
- * Disconnect a controller from a driver.
- *
- * This function implements the DisconnectController service.
- * See the Unified Extensible Firmware Interface (UEFI) specification
- * for details.
- *
- * @controller_handle	handle of the controller
- * @driver_image_handle handle of the driver
- * @child_handle	handle of the child to destroy
- * @return		status code
- */
-static efi_status_t EFIAPI efi_disconnect_controller(
-				efi_handle_t controller_handle,
-				efi_handle_t driver_image_handle,
-				efi_handle_t child_handle)
-{
-	EFI_ENTRY("%p, %p, %p", controller_handle, driver_image_handle,
-		  child_handle);
-	return EFI_EXIT(EFI_INVALID_PARAMETER);
-}
-
 /*
  * Close a protocol.
  *
@@ -2483,6 +2567,161 @@ out:
 	return EFI_EXIT(ret);
 }
 
+/*
+ * Get all child controllers associated to a driver.
+ * The allocated buffer has to be freed with free().
+ *
+ * @efiobj			handle of the controller
+ * @driver_handle		handle of the driver
+ * @number_of_children		number of child controllers
+ * @child_handle_buffer		handles of the the child controllers
+ */
+static efi_status_t efi_get_child_controllers(
+				struct efi_object *efiobj,
+				efi_handle_t driver_handle,
+				efi_uintn_t *number_of_children,
+				efi_handle_t **child_handle_buffer)
+{
+	struct efi_handler *handler;
+	struct efi_open_protocol_info_item *item;
+	efi_uintn_t count = 0, i;
+	bool duplicate;
+
+	/* Count all child controller associations */
+	list_for_each_entry(handler, &efiobj->protocols, link) {
+		list_for_each_entry(item, &handler->open_infos, link) {
+			if (item->info.agent_handle == driver_handle &&
+			    item->info.attributes &
+			    EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER)
+				++count;
+		}
+	}
+	/*
+	 * Create buffer. In case of duplicate child controller assignments
+	 * the buffer will be too large. But that does not harm.
+	 */
+	*number_of_children = 0;
+	*child_handle_buffer = calloc(count, sizeof(efi_handle_t));
+	if (!*child_handle_buffer)
+		return EFI_OUT_OF_RESOURCES;
+	/* Copy unique child handles */
+	list_for_each_entry(handler, &efiobj->protocols, link) {
+		list_for_each_entry(item, &handler->open_infos, link) {
+			if (item->info.agent_handle == driver_handle &&
+			    item->info.attributes &
+			    EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) {
+				/* Check this is a new child controller */
+				duplicate = false;
+				for (i = 0; i < *number_of_children; ++i) {
+					if ((*child_handle_buffer)[i] ==
+					    item->info.controller_handle)
+						duplicate = true;
+				}
+				/* Copy handle to buffer */
+				if (!duplicate) {
+					i = (*number_of_children)++;
+					(*child_handle_buffer)[i] =
+						item->info.controller_handle;
+				}
+			}
+		}
+	}
+	return EFI_SUCCESS;
+}
+
+/*
+ * Disconnect a controller from a driver.
+ *
+ * This function implements the DisconnectController service.
+ * See the Unified Extensible Firmware Interface (UEFI) specification
+ * for details.
+ *
+ * @controller_handle	handle of the controller
+ * @driver_image_handle handle of the driver
+ * @child_handle	handle of the child to destroy
+ * @return		status code
+ */
+static efi_status_t EFIAPI efi_disconnect_controller(
+				efi_handle_t controller_handle,
+				efi_handle_t driver_image_handle,
+				efi_handle_t child_handle)
+{
+	struct efi_driver_binding_protocol *binding_protocol;
+	efi_handle_t *child_handle_buffer = NULL;
+	size_t number_of_children = 0;
+	efi_status_t r;
+	size_t stop_count = 0;
+	struct efi_object *efiobj;
+
+	EFI_ENTRY("%p, %p, %p", controller_handle, driver_image_handle,
+		  child_handle);
+
+	efiobj = efi_search_obj(controller_handle);
+	if (!efiobj) {
+		r = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	if (child_handle && !efi_search_obj(child_handle)) {
+		r = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	/* If no driver handle is supplied, disconnect all drivers */
+	if (!driver_image_handle) {
+		r = efi_disconnect_all_drivers(efiobj, NULL, child_handle);
+		goto out;
+	}
+
+	/* Create list of child handles */
+	if (child_handle) {
+		number_of_children = 1;
+		child_handle_buffer = &child_handle;
+	} else {
+		efi_get_child_controllers(efiobj,
+					  driver_image_handle,
+					  &number_of_children,
+					  &child_handle_buffer);
+	}
+
+	/* Get the driver binding protocol */
+	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)
+		goto out;
+	/* Remove the children */
+	if (number_of_children) {
+		r = EFI_CALL(binding_protocol->stop(binding_protocol,
+						    controller_handle,
+						    number_of_children,
+						    child_handle_buffer));
+		if (r == EFI_SUCCESS)
+			++stop_count;
+	}
+	/* Remove the driver */
+	if (!child_handle)
+		r = EFI_CALL(binding_protocol->stop(binding_protocol,
+						    controller_handle,
+						    0, NULL));
+	if (r == EFI_SUCCESS)
+		++stop_count;
+	EFI_CALL(efi_close_protocol(driver_image_handle,
+				    &efi_guid_driver_binding_protocol,
+				    driver_image_handle, NULL));
+
+	if (stop_count)
+		r = EFI_SUCCESS;
+	else
+		r = EFI_NOT_FOUND;
+out:
+	if (!child_handle)
+		free(child_handle_buffer);
+	return EFI_EXIT(r);
+}
+
 static const struct efi_boot_services efi_boot_services = {
 	.hdr = {
 		.headersize = sizeof(struct efi_table_hdr),
-- 
2.14.2

[U-Boot] [PATCH 12/16] efi_loader: disconnect controllers in UninstallProtocol

[Heinrich Schuchardt]

The UninstallProtocol boot service should first try to
disconnect controllers that have been connected with
EFI_OPEN_PROTOCOL_BY_DRIVER.

If the protocol is still opened by an agent, it should be
closed.

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index bf60b79e93..18022ca734 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1080,26 +1080,43 @@ static efi_status_t EFIAPI efi_uninstall_protocol_interface(
 				void *handle, const efi_guid_t *protocol,
 				void *protocol_interface)
 {
+	struct efi_object *efiobj;
 	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, %pUl, %p", handle, protocol, protocol_interface);
 
-	if (!handle || !protocol) {
+	/* Check handle */
+	efiobj = efi_search_obj(handle);
+	if (!efiobj) {
 		r = EFI_INVALID_PARAMETER;
 		goto out;
 	}
-
 	/* Find the protocol on the handle */
 	r = efi_search_protocol(handle, protocol, &handler);
 	if (r != EFI_SUCCESS)
 		goto out;
-	if (handler->protocol_interface) {
-		/* TODO disconnect controllers */
+	/* Disconnect controllers */
+	efi_disconnect_all_drivers(efiobj, protocol, NULL);
+	if (!list_empty(&handler->open_infos)) {
 		r =  EFI_ACCESS_DENIED;
-	} else {
-		r = efi_remove_protocol(handle, protocol, protocol_interface);
+		goto out;
+	}
+	/* Close protocol */
+	list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
+		if (item->info.attributes ==
+			EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL ||
+		    item->info.attributes == EFI_OPEN_PROTOCOL_GET_PROTOCOL ||
+		    item->info.attributes == EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
+			list_del(&item->link);
+	}
+	if (!list_empty(&handler->open_infos)) {
+		r =  EFI_ACCESS_DENIED;
+		goto out;
 	}
+	r = efi_remove_protocol(handle, protocol, protocol_interface);
 out:
 	return EFI_EXIT(r);
 }
-- 
2.14.2

[U-Boot] [PATCH 13/16] efi_selftest: remove todo in manage protocols

[Heinrich Schuchardt]

The installation of UninstallProtocols is functional now.
So we do not expect errors when calling it.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_selftest/efi_selftest_manageprotocols.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_manageprotocols.c b/lib/efi_selftest/efi_selftest_manageprotocols.c
index f20f1528d4..874f86102d 100644
--- a/lib/efi_selftest/efi_selftest_manageprotocols.c
+++ b/lib/efi_selftest/efi_selftest_manageprotocols.c
@@ -194,7 +194,7 @@ static int execute(void)
 						&guid3, &interface3,
 						NULL);
 	if (ret == EFI_SUCCESS) {
-		efi_st_todo("UninstallMultipleProtocolInterfaces did not catch error\n");
+		efi_st_error("UninstallMultipleProtocolInterfaces did not catch error\n");
 		return EFI_ST_FAILURE;
 	}
 
@@ -273,8 +273,8 @@ static int execute(void)
 						&guid2, &interface2,
 						NULL);
 	if (ret != EFI_SUCCESS) {
-		efi_st_todo("UninstallMultipleProtocolInterfaces failed\n");
-		/* This test is known to fail due to missing implementation */
+		efi_st_error("UninstallMultipleProtocolInterfaces failed\n");
+		return EFI_ST_FAILURE;
 	}
 	/*
 	 * Check that the protocols are really uninstalled.
@@ -287,8 +287,8 @@ static int execute(void)
 		return EFI_ST_FAILURE;
 	}
 	if (count != 1) {
-		efi_st_todo("UninstallMultipleProtocolInterfaces failed to uninstall protocols\n");
-		/* This test is known to fail due to missing implementation */
+		efi_st_error("UninstallMultipleProtocolInterfaces failed to uninstall protocols\n");
+		return EFI_ST_FAILURE;
 	}
 	ret = find_in_buffer(handle1, count, buffer);
 	if (ret != EFI_SUCCESS) {
@@ -327,19 +327,19 @@ static int execute(void)
 	ret = boottime->uninstall_protocol_interface(handle1, &guid1,
 						     &interface1);
 	if (ret != EFI_SUCCESS) {
-		efi_st_todo("UninstallProtocolInterface failed\n");
-		/* This test is known to fail due to missing implementation */
+		efi_st_error("UninstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
 	}
 	ret = boottime->handle_protocol(handle1, &guid1, (void **)&interface);
 	if (ret == EFI_SUCCESS) {
-		efi_st_todo("UninstallProtocolInterface failed\n");
-		/* This test is known to fail due to missing implementation */
+		efi_st_error("UninstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
 	}
 	ret = boottime->uninstall_protocol_interface(handle1, &guid3,
 						     &interface1);
 	if (ret != EFI_SUCCESS) {
-		efi_st_todo("UninstallProtocolInterface failed\n");
-		/* This test is known to fail due to missing implementation */
+		efi_st_error("UninstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
 	}
 
 	return EFI_ST_SUCCESS;
-- 
2.14.2

[U-Boot] [PATCH 14/16] efi_selftest: remove todo in device path test

[Heinrich Schuchardt]

The installation of UninstallProtocol is functional now.
So we do not expect errors when calling it.

Call UninstallProtocol with correct level of indirection
for parameter handle.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_selftest/efi_selftest_devicepath.c | 40 +++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c
index d9a6697892..2f8f3c7f15 100644
--- a/lib/efi_selftest/efi_selftest_devicepath.c
+++ b/lib/efi_selftest/efi_selftest_devicepath.c
@@ -192,31 +192,41 @@ static int teardown(void)
 {
 	efi_status_t ret;
 
-	ret = boottime->uninstall_protocol_interface(&handle1,
+	ret = boottime->uninstall_protocol_interface(handle1,
 						     &guid_device_path,
 						     dp1);
-	if (ret != EFI_SUCCESS)
-		efi_st_todo("UninstallProtocolInterface failed\n");
-	ret = boottime->uninstall_protocol_interface(&handle1,
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("UninstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->uninstall_protocol_interface(handle1,
 						     &guid_protocol,
 						     &interface);
-	if (ret != EFI_SUCCESS)
-		efi_st_todo("UninstallProtocolInterface failed\n");
-	ret = boottime->uninstall_protocol_interface(&handle2,
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("UninstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->uninstall_protocol_interface(handle2,
 						     &guid_device_path,
 						     dp2);
-	if (ret != EFI_SUCCESS)
-		efi_st_todo("UninstallProtocolInterface failed\n");
-	ret = boottime->uninstall_protocol_interface(&handle2,
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("UninstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->uninstall_protocol_interface(handle2,
 						     &guid_protocol,
 						     &interface);
-	if (ret != EFI_SUCCESS)
-		efi_st_todo("UninstallProtocolInterface failed\n");
-	ret = boottime->uninstall_protocol_interface(&handle3,
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("UninstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->uninstall_protocol_interface(handle3,
 						     &guid_device_path,
 						     dp3);
-	if (ret != EFI_SUCCESS)
-		efi_st_todo("UninstallProtocolInterface failed\n");
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("UninstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
 	if (dp1) {
 		ret = boottime->free_pool(dp1);
 		if (ret != EFI_SUCCESS) {
-- 
2.14.2

[U-Boot] [PATCH 15/16] efi_selftest: test for (Dis)ConnectController

[Heinrich Schuchardt]

This unit test checks the following protocol services:
ConnectController, DisconnectController,
InstallProtocol, UninstallProtocol,
OpenProtocol, CloseProtcol, OpenProtocolInformation

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_selftest/Makefile                   |   1 +
 lib/efi_selftest/efi_selftest_controllers.c | 385 ++++++++++++++++++++++++++++
 2 files changed, 386 insertions(+)
 create mode 100644 lib/efi_selftest/efi_selftest_controllers.c

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 837e86228e..e549553c82 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -9,6 +9,7 @@
 
 obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \
 efi_selftest.o \
+efi_selftest_controllers.o \
 efi_selftest_console.o \
 efi_selftest_devicepath.o \
 efi_selftest_events.o \
diff --git a/lib/efi_selftest/efi_selftest_controllers.c b/lib/efi_selftest/efi_selftest_controllers.c
new file mode 100644
index 0000000000..9908c85c44
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_controllers.c
@@ -0,0 +1,385 @@
+/*
+ * efi_selftest_controllers
+ *
+ * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ *
+ * This unit test checks the following protocol services:
+ * ConnectController, DisconnectController,
+ * InstallProtocol, UninstallProtocol,
+ * OpenProtocol, CloseProtcol, OpenProtocolInformation
+ */
+
+#include <efi_selftest.h>
+
+#define NUMBER_OF_CHILD_CONTROLLERS 4
+
+static struct efi_boot_services *boottime;
+const efi_guid_t guid_driver_binding_protocol =
+			EFI_DRIVER_BINDING_PROTOCOL_GUID;
+static efi_guid_t guid_controller =
+	EFI_GUID(0xe6ab1d96, 0x6bff, 0xdb42,
+		 0xaa, 0x05, 0xc8, 0x1f, 0x7f, 0x45, 0x26, 0x34);
+static efi_guid_t guid_child_controller =
+	EFI_GUID(0x1d41f6f5, 0x2c41, 0xddfb,
+		 0xe2, 0x9b, 0xb8, 0x0e, 0x2e, 0xe8, 0x3a, 0x85);
+static efi_handle_t handle_controller;
+static efi_handle_t handle_child_controller[NUMBER_OF_CHILD_CONTROLLERS];
+static efi_handle_t handle_driver;
+
+/*
+ * Count child controllers
+ *
+ * @handle	handle on which child controllers are installed
+ * @protocol	protocol for which the child controlles where installed
+ * @count	number of child controllers
+ * @return	status code
+ */
+static efi_status_t count_child_controllers(efi_handle_t handle,
+					    efi_guid_t *protocol,
+					    efi_uintn_t *count)
+{
+	efi_status_t ret;
+	efi_uintn_t entry_count;
+	struct efi_open_protocol_info_entry *entry_buffer;
+
+	*count = 0;
+	ret = boottime->open_protocol_information(handle, protocol,
+						  &entry_buffer, &entry_count);
+	if (ret != EFI_SUCCESS)
+		return ret;
+	if (!entry_count)
+		return EFI_SUCCESS;
+	while (entry_count) {
+		if (entry_buffer[--entry_count].attributes &
+		    EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER)
+			++*count;
+	}
+	ret = boottime->free_pool(entry_buffer);
+	if (ret != EFI_SUCCESS)
+		efi_st_error("Cannot free buffer\n");
+	return ret;
+}
+
+/*
+ * Check if the driver supports the controller.
+ *
+ * @this			driver binding protocol
+ * @controller_handle		handle of the controller
+ * @remaining_device_path	path specifying the child controller
+ * @return			status code
+ */
+static 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 ret;
+	void *interface;
+
+	ret = boottime->open_protocol(
+			controller_handle, &guid_controller,
+			&interface, handle_driver,
+			controller_handle, EFI_OPEN_PROTOCOL_BY_DRIVER);
+	switch (ret) {
+	case EFI_ACCESS_DENIED:
+	case EFI_ALREADY_STARTED:
+		return ret;
+	case EFI_SUCCESS:
+		break;
+	default:
+		return EFI_UNSUPPORTED;
+	}
+	ret = boottime->close_protocol(
+				controller_handle, &guid_controller,
+				handle_driver, controller_handle);
+	if (ret != EFI_SUCCESS)
+		ret = EFI_UNSUPPORTED;
+	return ret;
+}
+
+/*
+ * Create child controllers and attach driver.
+ *
+ * @this			driver binding protocol
+ * @controller_handle		handle of the controller
+ * @remaining_device_path	path specifying the child controller
+ * @return			status code
+ */
+static efi_status_t EFIAPI start(
+		struct efi_driver_binding_protocol *this,
+		efi_handle_t controller_handle,
+		struct efi_device_path *remaining_device_path)
+{
+	size_t i;
+	efi_status_t ret;
+	void *interface;
+
+	/* Attach driver to controller */
+	ret = boottime->open_protocol(
+			controller_handle, &guid_controller,
+			&interface, handle_driver,
+			controller_handle, EFI_OPEN_PROTOCOL_BY_DRIVER);
+	switch (ret) {
+	case EFI_ACCESS_DENIED:
+	case EFI_ALREADY_STARTED:
+		return ret;
+	case EFI_SUCCESS:
+		break;
+	default:
+		return EFI_UNSUPPORTED;
+	}
+
+	/* Create child controllers */
+	for (i = 0; i < NUMBER_OF_CHILD_CONTROLLERS; ++i) {
+		ret = boottime->install_protocol_interface(
+			&handle_child_controller[i], &guid_child_controller,
+			EFI_NATIVE_INTERFACE, NULL);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("InstallProtocolInterface failed\n");
+			return EFI_ST_FAILURE;
+		}
+		ret = boottime->open_protocol(
+			controller_handle, &guid_controller,
+			&interface, handle_child_controller[i],
+			handle_child_controller[i],
+			EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("OpenProtocol failed\n");
+			return EFI_ST_FAILURE;
+		}
+	}
+	return ret;
+}
+
+/*
+ * Remove a single child controller from the parent controller.
+ *
+ * @controller_handle	parent controller
+ * @child_handle	child controller
+ * @return		status code
+ */
+static efi_status_t disconnect_child(efi_handle_t controller_handle,
+				     efi_handle_t child_handle)
+{
+	efi_status_t ret;
+
+	ret = boottime->close_protocol(
+				controller_handle, &guid_controller,
+				child_handle, child_handle);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Cannot close protocol\n");
+		return ret;
+	}
+	ret = boottime->uninstall_protocol_interface(
+				child_handle, &guid_child_controller, NULL);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Cannot uninstall protocol interface\n");
+		return ret;
+	}
+	return ret;
+}
+
+/*
+ * Remove child controllers and disconnect the controller.
+ *
+ * @this			driver binding protocol
+ * @controller_handle		handle of the controller
+ * @number_of_children		number of child controllers to remove
+ * @child_handle_buffer		handles of the child controllers to remove
+ * @return			status code
+ */
+static efi_status_t EFIAPI stop(
+		struct efi_driver_binding_protocol *this,
+		efi_handle_t controller_handle,
+		size_t number_of_children,
+		efi_handle_t *child_handle_buffer)
+{
+	efi_status_t ret;
+	efi_uintn_t count;
+	struct efi_open_protocol_info_entry *entry_buffer;
+
+	/* Destroy provided child controllers */
+	if (number_of_children) {
+		efi_uintn_t i;
+
+		for (i = 0; i < number_of_children; ++i) {
+			ret = disconnect_child(controller_handle,
+					       child_handle_buffer[i]);
+			if (ret != EFI_SUCCESS)
+				return ret;
+		}
+		return EFI_SUCCESS;
+	}
+
+	/* Destroy all children */
+	ret = boottime->open_protocol_information(
+					controller_handle, &guid_controller,
+					&entry_buffer, &count);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("OpenProtocolInformation failed\n");
+		return ret;
+	}
+	while (count) {
+		if (entry_buffer[--count].attributes &
+		    EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) {
+			ret = disconnect_child(
+					controller_handle,
+					entry_buffer[count].agent_handle);
+			if (ret != EFI_SUCCESS)
+				return ret;
+		}
+	}
+	ret = boottime->free_pool(entry_buffer);
+	if (ret != EFI_SUCCESS)
+		efi_st_error("Cannot free buffer\n");
+
+	/* Detach driver from controller */
+	ret = boottime->close_protocol(
+			controller_handle, &guid_controller,
+			handle_driver, controller_handle);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Cannot close protocol\n");
+		return ret;
+	}
+	return EFI_SUCCESS;
+}
+
+/* Driver binding protocol interface */
+struct efi_driver_binding_protocol binding_interface = {
+	supported,
+	start,
+	stop,
+	0xffffffff,
+	NULL,
+	NULL,
+	};
+
+/*
+ * Setup unit test.
+ *
+ * @handle	handle of the loaded image
+ * @systable	system table
+ */
+static int setup(const efi_handle_t img_handle,
+		 const struct efi_system_table *systable)
+{
+	efi_status_t ret;
+
+	boottime = systable->boottime;
+
+	/* Create controller handle */
+	ret = boottime->install_protocol_interface(
+			&handle_controller, &guid_controller,
+			EFI_NATIVE_INTERFACE, NULL);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	/* Create driver handle */
+	ret = boottime->install_protocol_interface(
+			&handle_driver,  &guid_driver_binding_protocol,
+			EFI_NATIVE_INTERFACE, &binding_interface);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * Execute unit test.
+ *
+ * The number of child controllers is checked after each of the following
+ * actions:
+ *
+ * Connect a controller to a driver.
+ * Disconnect and destroy a child controller.
+ * Disconnect and destroy the remaining child controllers.
+ *
+ * Connect a controller to a driver.
+ * Uninstall the driver protocol from the controller.
+ */
+static int execute(void)
+{
+	efi_status_t ret;
+	efi_uintn_t count;
+
+	/* Connect controller to driver */
+	ret = boottime->connect_controller(handle_controller, NULL, NULL, 1);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to connect controller\n");
+		return EFI_ST_FAILURE;
+	}
+	/* Check number of child controllers */
+	ret = count_child_controllers(handle_controller, &guid_controller,
+				      &count);
+	if (ret != EFI_SUCCESS || count != NUMBER_OF_CHILD_CONTROLLERS) {
+		efi_st_error("Number of children %u != %u\n",
+			     (unsigned int)count, NUMBER_OF_CHILD_CONTROLLERS);
+	}
+	/* Destroy second child controller */
+	ret = boottime->disconnect_controller(handle_controller,
+					      handle_driver,
+					      handle_child_controller[1]);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to disconnect child controller\n");
+		return EFI_ST_FAILURE;
+	}
+	/* Check number of child controllers */
+	ret = count_child_controllers(handle_controller, &guid_controller,
+				      &count);
+	if (ret != EFI_SUCCESS || count != NUMBER_OF_CHILD_CONTROLLERS - 1) {
+		efi_st_error("Destroying single child controller failed\n");
+		return EFI_ST_FAILURE;
+	}
+	/* Destroy remaining child controllers and disconnect controller */
+	ret = boottime->disconnect_controller(handle_controller, NULL, NULL);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to disconnect controller\n");
+		return EFI_ST_FAILURE;
+	}
+	/* Check number of child controllers */
+	ret = count_child_controllers(handle_controller, &guid_controller,
+				      &count);
+	if (ret != EFI_SUCCESS || count) {
+		efi_st_error("Destroying child controllers failed\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/* Connect controller to driver */
+	ret = boottime->connect_controller(handle_controller, NULL, NULL, 1);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to connect controller\n");
+		return EFI_ST_FAILURE;
+	}
+	/* Check number of child controllers */
+	ret = count_child_controllers(handle_controller, &guid_controller,
+				      &count);
+	if (ret != EFI_SUCCESS || count != NUMBER_OF_CHILD_CONTROLLERS) {
+		efi_st_error("Number of children %u != %u\n",
+			     (unsigned int)count, NUMBER_OF_CHILD_CONTROLLERS);
+	}
+	/* Uninstall controller protocol */
+	ret = boottime->uninstall_protocol_interface(handle_controller,
+						     &guid_controller, NULL);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to uninstall protocols\n");
+		return EFI_ST_FAILURE;
+	}
+	/* Check number of child controllers */
+	ret = count_child_controllers(handle_controller, &guid_controller,
+				      &count);
+	if (ret == EFI_SUCCESS)
+		efi_st_error("Uninstall failed\n");
+	return EFI_ST_SUCCESS;
+}
+
+EFI_UNIT_TEST(controllers) = {
+	.name = "controllers",
+	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+	.setup = setup,
+	.execute = execute,
+};
-- 
2.14.2

[U-Boot] [PATCH 16/16] efi_loader: consistently use efi_handle_t for handles

[Heinrich Schuchardt]

We should consistently use the efi_handle_t typedef when
referring to handles.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 cmd/bootefi.c                 | 10 ++++-----
 include/efi_api.h             | 20 ++++++++++--------
 include/efi_loader.h          | 14 +++++++------
 lib/efi_loader/efi_boottime.c | 49 +++++++++++++++++++++++--------------------
 lib/efi_loader/efi_console.c  |  6 +++---
 5 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 78ff109835..97a4f269ae 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -122,8 +122,8 @@ static void *copy_fdt(void *fdt)
 }
 
 static efi_status_t efi_do_enter(
-			void *image_handle, struct efi_system_table *st,
-			asmlinkage ulong (*entry)(void *image_handle,
+			efi_handle_t image_handle, struct efi_system_table *st,
+			asmlinkage ulong (*entry)(efi_handle_t image_handle,
 						  struct efi_system_table *st))
 {
 	efi_status_t ret = EFI_LOAD_ERROR;
@@ -136,8 +136,8 @@ static efi_status_t efi_do_enter(
 
 #ifdef CONFIG_ARM64
 static efi_status_t efi_run_in_el2(asmlinkage ulong (*entry)(
-			void *image_handle, struct efi_system_table *st),
-			void *image_handle, struct efi_system_table *st)
+			efi_handle_t image_handle, struct efi_system_table *st),
+			efi_handle_t image_handle, struct efi_system_table *st)
 {
 	/* Enable caches again */
 	dcache_enable();
@@ -159,7 +159,7 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt,
 	struct efi_device_path *memdp = NULL;
 	ulong ret;
 
-	ulong (*entry)(void *image_handle, struct efi_system_table *st)
+	ulong (*entry)(efi_handle_t image_handle, struct efi_system_table *st)
 		asmlinkage;
 	ulong fdt_pages, fdt_size, fdt_start, fdt_end;
 	const efi_guid_t fdt_guid = EFI_FDT_GUID;
diff --git a/include/efi_api.h b/include/efi_api.h
index 7164492f83..502fffed20 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -84,11 +84,12 @@ struct efi_boot_services {
 	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,
-			const efi_guid_t *protocol, void *protocol_interface);
-	efi_status_t (EFIAPI *handle_protocol)(efi_handle_t,
-					       const efi_guid_t *protocol,
-					       void **protocol_interface);
+	efi_status_t (EFIAPI *uninstall_protocol_interface)(
+			efi_handle_t handle, const efi_guid_t *protocol,
+			void *protocol_interface);
+	efi_status_t (EFIAPI *handle_protocol)(
+			efi_handle_t handle, const efi_guid_t *protocol,
+			void **protocol_interface);
 	void *reserved;
 	efi_status_t (EFIAPI *register_protocol_notify)(
 			const efi_guid_t *protocol, struct efi_event *event,
@@ -113,7 +114,7 @@ struct efi_boot_services {
 	efi_status_t (EFIAPI *exit)(efi_handle_t handle,
 				    efi_status_t exit_status,
 				    unsigned long exitdata_size, s16 *exitdata);
-	efi_status_t (EFIAPI *unload_image)(void *image_handle);
+	efi_status_t (EFIAPI *unload_image)(efi_handle_t image_handle);
 	efi_status_t (EFIAPI *exit_boot_services)(efi_handle_t, unsigned long);
 
 	efi_status_t (EFIAPI *get_next_monotonic_count)(u64 *count);
@@ -139,9 +140,10 @@ struct efi_boot_services {
 			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,
-			const efi_guid_t *protocol, void *agent_handle,
-			void *controller_handle);
+	efi_status_t (EFIAPI *close_protocol)(
+			efi_handle_t handle, const efi_guid_t *protocol,
+			efi_handle_t agent_handle,
+			efi_handle_t controller_handle);
 	efi_status_t(EFIAPI *open_protocol_information)(efi_handle_t handle,
 			const efi_guid_t *protocol,
 			struct efi_open_protocol_info_entry **entry_buffer,
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9e1ae8866b..1411782879 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -204,23 +204,25 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path);
 /* Add a new object to the object list. */
 void efi_add_handle(struct efi_object *obj);
 /* Create handle */
-efi_status_t efi_create_handle(void **handle);
+efi_status_t efi_create_handle(efi_handle_t *handle);
 /* Delete handle */
 void efi_delete_handle(struct efi_object *obj);
 /* Call this to validate a handle and find the EFI object for it */
-struct efi_object *efi_search_obj(const void *handle);
+struct efi_object *efi_search_obj(const efi_handle_t handle);
 /* Find a protocol on a handle */
-efi_status_t efi_search_protocol(const void *handle,
+efi_status_t efi_search_protocol(const efi_handle_t handle,
 				 const efi_guid_t *protocol_guid,
 				 struct efi_handler **handler);
 /* Install new protocol on a handle */
-efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol,
+efi_status_t efi_add_protocol(const efi_handle_t handle,
+			      const efi_guid_t *protocol,
 			      void *protocol_interface);
 /* Delete protocol from a handle */
-efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol,
+efi_status_t efi_remove_protocol(const efi_handle_t handle,
+				 const efi_guid_t *protocol,
 				 void *protocol_interface);
 /* Delete all protocols from a handle */
-efi_status_t efi_remove_all_protocols(const void *handle);
+efi_status_t efi_remove_all_protocols(const efi_handle_t handle);
 /* Call this to create an event */
 efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl,
 			      void (EFIAPI *notify_function) (
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 18022ca734..bb637e20bd 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -60,9 +60,10 @@ static int nesting_level;
 const efi_guid_t efi_guid_driver_binding_protocol =
 			EFI_DRIVER_BINDING_PROTOCOL_GUID;
 
-static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle,
-						     void *driver_image_handle,
-						     void *child_handle);
+static efi_status_t EFIAPI efi_disconnect_controller(
+					efi_handle_t controller_handle,
+					efi_handle_t driver_image_handle,
+					efi_handle_t child_handle);
 
 /* Called on every callback entry */
 int __efi_entry_check(void)
@@ -351,7 +352,7 @@ void efi_add_handle(struct efi_object *obj)
  * @handle	new handle
  * @return	status code
  */
-efi_status_t efi_create_handle(void **handle)
+efi_status_t efi_create_handle(efi_handle_t *handle)
 {
 	struct efi_object *obj;
 	efi_status_t r;
@@ -374,7 +375,7 @@ efi_status_t efi_create_handle(void **handle)
  * @handler		reference to the protocol
  * @return		status code
  */
-efi_status_t efi_search_protocol(const void *handle,
+efi_status_t efi_search_protocol(const efi_handle_t handle,
 				 const efi_guid_t *protocol_guid,
 				 struct efi_handler **handler)
 {
@@ -407,7 +408,8 @@ efi_status_t efi_search_protocol(const void *handle,
  * @protocol_interface		interface of the protocol implementation
  * @return			status code
  */
-efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol,
+efi_status_t efi_remove_protocol(const efi_handle_t handle,
+				 const efi_guid_t *protocol,
 				 void *protocol_interface)
 {
 	struct efi_handler *handler;
@@ -429,7 +431,7 @@ efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol,
  * @handle	handle from which the protocols shall be deleted
  * @return	status code
  */
-efi_status_t efi_remove_all_protocols(const void *handle)
+efi_status_t efi_remove_all_protocols(const efi_handle_t handle)
 {
 	struct efi_object *efiobj;
 	struct efi_handler *protocol;
@@ -809,7 +811,7 @@ static efi_status_t EFIAPI efi_check_event(struct efi_event *event)
  * @handle	handle to find
  * @return	EFI object
  */
-struct efi_object *efi_search_obj(const void *handle)
+struct efi_object *efi_search_obj(const efi_handle_t handle)
 {
 	struct efi_object *efiobj;
 
@@ -863,7 +865,8 @@ static efi_status_t efi_delete_open_info(
  * @protocol_interface		interface of the protocol implementation
  * @return			status code
  */
-efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol,
+efi_status_t efi_add_protocol(const efi_handle_t handle,
+			      const efi_guid_t *protocol,
 			      void *protocol_interface)
 {
 	struct efi_object *efiobj;
@@ -953,9 +956,9 @@ out:
  * @new_interface		interface to be installed
  * @return			status code
  */
-static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle,
-			const efi_guid_t *protocol, void *old_interface,
-			void *new_interface)
+static efi_status_t EFIAPI efi_reinstall_protocol_interface(
+			efi_handle_t handle, const efi_guid_t *protocol,
+			void *old_interface, void *new_interface)
 {
 	EFI_ENTRY("%p, %pUl, %p, %p", handle, protocol, old_interface,
 		  new_interface);
@@ -1077,7 +1080,7 @@ static efi_status_t efi_disconnect_all_drivers(
  * @return			status code
  */
 static efi_status_t EFIAPI efi_uninstall_protocol_interface(
-				void *handle, const efi_guid_t *protocol,
+				efi_handle_t handle, const efi_guid_t *protocol,
 				void *protocol_interface)
 {
 	struct efi_object *efiobj;
@@ -1534,7 +1537,7 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
 					   unsigned long *exit_data_size,
 					   s16 **exit_data)
 {
-	ulong (*entry)(void *image_handle, struct efi_system_table *st);
+	ulong (*entry)(efi_handle_t image_handle, struct efi_system_table *st);
 	struct efi_loaded_image *info = image_handle;
 
 	EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data);
@@ -1616,7 +1619,7 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle,
  * @image_handle	handle of the image to be unloaded
  * @return		status code
  */
-static efi_status_t EFIAPI efi_unload_image(void *image_handle)
+static efi_status_t EFIAPI efi_unload_image(efi_handle_t image_handle)
 {
 	struct efi_object *efiobj;
 
@@ -1654,7 +1657,7 @@ static void efi_exit_caches(void)
  * @map_key		key of the memory map
  * @return		status code
  */
-static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
+static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
 						  unsigned long map_key)
 {
 	int i;
@@ -1758,10 +1761,10 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
  * @controller_handle	handle of the controller
  * @return		status code
  */
-static efi_status_t EFIAPI efi_close_protocol(void *handle,
+static efi_status_t EFIAPI efi_close_protocol(efi_handle_t handle,
 					      const efi_guid_t *protocol,
-					      void *agent_handle,
-					      void *controller_handle)
+					      efi_handle_t agent_handle,
+					      efi_handle_t controller_handle)
 {
 	struct efi_handler *handler;
 	struct efi_open_protocol_info_item *item;
@@ -1866,8 +1869,8 @@ out:
  * @protocol_buffer_count	number of entries in the buffer
  * @return			status code
  */
-static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,
-			efi_guid_t ***protocol_buffer,
+static efi_status_t EFIAPI efi_protocols_per_handle(
+			efi_handle_t handle, efi_guid_t ***protocol_buffer,
 			efi_uintn_t *protocol_buffer_count)
 {
 	unsigned long buffer_size;
@@ -1957,7 +1960,7 @@ static efi_status_t EFIAPI efi_locate_handle_buffer(
 	r = efi_locate_handle(search_type, protocol, search_key, &buffer_size,
 			      *buffer);
 	if (r == EFI_SUCCESS)
-		*no_handles = buffer_size / sizeof(void *);
+		*no_handles = buffer_size / sizeof(efi_handle_t);
 out:
 	return EFI_EXIT(r);
 }
@@ -2425,7 +2428,7 @@ out:
  * @protocol_interface  interface implementing the protocol
  * @return		status code
  */
-static efi_status_t EFIAPI efi_handle_protocol(void *handle,
+static efi_status_t EFIAPI efi_handle_protocol(efi_handle_t handle,
 					       const efi_guid_t *protocol,
 					       void **protocol_interface)
 {
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 98497db612..56b079cee8 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -503,21 +503,21 @@ int efi_console_register(void)
 	struct efi_object *efi_console_input_obj;
 
 	/* Create handles */
-	r = efi_create_handle((void **)&efi_console_control_obj);
+	r = efi_create_handle((efi_handle_t *)&efi_console_control_obj);
 	if (r != EFI_SUCCESS)
 		goto out_of_memory;
 	r = efi_add_protocol(efi_console_control_obj->handle,
 			     &efi_guid_console_control, &efi_console_control);
 	if (r != EFI_SUCCESS)
 		goto out_of_memory;
-	r = efi_create_handle((void **)&efi_console_output_obj);
+	r = efi_create_handle((efi_handle_t *)&efi_console_output_obj);
 	if (r != EFI_SUCCESS)
 		goto out_of_memory;
 	r = efi_add_protocol(efi_console_output_obj->handle,
 			     &efi_guid_text_output_protocol, &efi_con_out);
 	if (r != EFI_SUCCESS)
 		goto out_of_memory;
-	r = efi_create_handle((void **)&efi_console_input_obj);
+	r = efi_create_handle((efi_handle_t *)&efi_console_input_obj);
 	if (r != EFI_SUCCESS)
 		goto out_of_memory;
 	r = efi_add_protocol(efi_console_input_obj->handle,
-- 
2.14.2

[U-Boot] [PATCH 01/16] efi_selftest: colored test output

[Simon Glass]

Hi Heinrich,

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Add color coding to output:
> test section    blue
> success         green
> errors          red
> todo            yellow
> summary         white
> others          light gray
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_selftest.h                  | 27 +++++++++++++++++----------
>  lib/efi_selftest/efi_selftest.c         | 25 ++++++++++++++-----------
>  lib/efi_selftest/efi_selftest_console.c | 13 +++++++++----
>  3 files changed, 40 insertions(+), 25 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

comments below

>
> diff --git a/include/efi_selftest.h b/include/efi_selftest.h
> index be5ba4bfa9..344f28ed36 100644
> --- a/include/efi_selftest.h
> +++ b/include/efi_selftest.h
> @@ -18,14 +18,20 @@
>  #define EFI_ST_SUCCESS 0
>  #define EFI_ST_FAILURE 1
>
> +/*
> + * Prints a message.
> + */
> +#define efi_st_printf(...) \
> +       (efi_st_printc(-1, __VA_ARGS__))
> +
>  /*
>   * Prints an error message.
>   *
>   * @...        format string followed by fields to print
>   */
>  #define efi_st_error(...) \
> -       (efi_st_printf("%s(%u):\nERROR: ", __FILE__, __LINE__), \
> -       efi_st_printf(__VA_ARGS__)) \
> +       (efi_st_printc(EFI_LIGHTRED, "%s(%u):\nERROR: ", __FILE__, __LINE__), \
> +       efi_st_printc(EFI_LIGHTRED, __VA_ARGS__))
>
>  /*
>   * Prints a TODO message.
> @@ -33,8 +39,8 @@
>   * @...        format string followed by fields to print
>   */
>  #define efi_st_todo(...) \
> -       (efi_st_printf("%s(%u):\nTODO: ", __FILE__, __LINE__), \
> -       efi_st_printf(__VA_ARGS__)) \
> +       (efi_st_printc(EFI_YELLOW, "%s(%u):\nTODO: ", __FILE__, __LINE__), \
> +       efi_st_printc(EFI_YELLOW, __VA_ARGS__)) \
>
>  /*
>   * A test may be setup and executed at boottime,
> @@ -61,14 +67,15 @@ extern struct efi_simple_input_interface *con_in;
>  void efi_st_exit_boot_services(void);
>
>  /*
> - * Print a pointer to an u16 string
> + * Print a colored message
>   *
> - * @pointer: pointer
> - * @buf: pointer to buffer address
> - * on return position of terminating zero word
> + * @color      color

Can you please point to the enum to use? Also it seems like -1 means
no colour? That should bementioned also

> + * @fmt                printf format
> + * @...                arguments to be printed
> + *             on return position of terminating zero word
>   */
> -void efi_st_printf(const char *fmt, ...)
> -                __attribute__ ((format (__printf__, 1, 2)));
> +void efi_st_printc(int color, const char *fmt, ...)
> +                __attribute__ ((format (__printf__, 2, 3)));
>
>  /*
>   * Compare memory.
> diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c
> index 4e5a12c47c..fc5ef254a1 100644
> --- a/lib/efi_selftest/efi_selftest.c
> +++ b/lib/efi_selftest/efi_selftest.c
> @@ -65,7 +65,7 @@ void efi_st_exit_boot_services(void)
>                 efi_st_error("ExitBootServices did not return EFI_SUCCESS\n");
>                 return;
>         }
> -       efi_st_printf("\nBoot services terminated\n");
> +       efi_st_printc(EFI_WHITE, "\nBoot services terminated\n");
>  }
>
>  /*
> @@ -81,13 +81,14 @@ static int setup(struct efi_unit_test *test, unsigned int *failures)
>
>         if (!test->setup)
>                 return EFI_ST_SUCCESS;
> -       efi_st_printf("\nSetting up '%s'\n", test->name);
> +       efi_st_printc(EFI_LIGHTBLUE, "\nSetting up '%s'\n", test->name);
>         ret = test->setup(handle, systable);
>         if (ret != EFI_ST_SUCCESS) {
>                 efi_st_error("Setting up '%s' failed\n", test->name);
>                 ++*failures;
>         } else {
> -               efi_st_printf("Setting up '%s' succeeded\n", test->name);
> +               efi_st_printc(EFI_LIGHTGREEN,
> +                             "Setting up '%s' succeeded\n", test->name);
>         }
>         return ret;
>  }
> @@ -105,13 +106,14 @@ static int execute(struct efi_unit_test *test, unsigned int *failures)
>
>         if (!test->execute)
>                 return EFI_ST_SUCCESS;
> -       efi_st_printf("\nExecuting '%s'\n", test->name);
> +       efi_st_printc(EFI_LIGHTBLUE, "\nExecuting '%s'\n", test->name);
>         ret = test->execute();
>         if (ret != EFI_ST_SUCCESS) {
>                 efi_st_error("Executing '%s' failed\n", test->name);
>                 ++*failures;
>         } else {
> -               efi_st_printf("Executing '%s' succeeded\n", test->name);
> +               efi_st_printc(EFI_LIGHTGREEN,
> +                             "Executing '%s' succeeded\n", test->name);
>         }
>         return ret;
>  }
> @@ -129,13 +131,14 @@ static int teardown(struct efi_unit_test *test, unsigned int *failures)
>
>         if (!test->teardown)
>                 return EFI_ST_SUCCESS;
> -       efi_st_printf("\nTearing down '%s'\n", test->name);
> +       efi_st_printc(EFI_LIGHTBLUE, "\nTearing down '%s'\n", test->name);
>         ret = test->teardown();
>         if (ret != EFI_ST_SUCCESS) {
>                 efi_st_error("Tearing down '%s' failed\n", test->name);
>                 ++*failures;
>         } else {
> -               efi_st_printf("Tearing down '%s' succeeded\n", test->name);
> +               efi_st_printc(EFI_LIGHTGREEN,
> +                             "Tearing down '%s' succeeded\n", test->name);
>         }
>         return ret;
>  }
> @@ -262,12 +265,12 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>                 }
>         }
>
> -       efi_st_printf("\nTesting EFI API implementation\n");
> +       efi_st_printc(EFI_WHITE, "\nTesting EFI API implementation\n");
>
>         if (testname)
> -               efi_st_printf("\nSelected test: '%ps'\n", testname);
> +               efi_st_printc(EFI_WHITE, "\nSelected test: '%ps'\n", testname);
>         else
> -               efi_st_printf("\nNumber of tests to execute: %u\n",
> +               efi_st_printc(EFI_WHITE, "\nNumber of tests to execute: %u\n",
>                               ll_entry_count(struct efi_unit_test,
>                                              efi_unit_test));
>
> @@ -291,7 +294,7 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>                         &failures);
>
>         /* Give feedback */
> -       efi_st_printf("\nSummary: %u failures\n\n", failures);
> +       efi_st_printc(EFI_WHITE, "\nSummary: %u failures\n\n", failures);
>
>         /* Reset system */
>         efi_st_printf("Preparing for reset. Press any key.\n");
> diff --git a/lib/efi_selftest/efi_selftest_console.c b/lib/efi_selftest/efi_selftest_console.c
> index 6a7fd20da5..7920c961ba 100644
> --- a/lib/efi_selftest/efi_selftest_console.c
> +++ b/lib/efi_selftest/efi_selftest_console.c
> @@ -130,12 +130,13 @@ static void int2dec(s32 value, u16 **buf)
>  }
>
>  /*
> - * Print a formatted string to the EFI console
> + * Print a colored formatted string to the EFI console
>   *
> - * @fmt: format string
> - * @...: optional arguments
> + * @color      color

same here

> + * @fmt                format string
> + * @...                optional arguments
>   */
> -void efi_st_printf(const char *fmt, ...)
> +void efi_st_printc(int color, const char *fmt, ...)
>  {
>         va_list args;
>         u16 buf[160];
> @@ -215,7 +216,11 @@ void efi_st_printf(const char *fmt, ...)
>         }
>         va_end(args);
>         *pos = 0;
> +       if (color >= 0)
> +               con_out->set_attribute(con_out, (unsigned long)color);
>         con_out->output_string(con_out, buf);
> +       if (color >= 0)
> +               con_out->set_attribute(con_out, EFI_LIGHTGRAY);
>  }
>
>  /*
> --
> 2.14.2
>

Regards,
Simon

[U-Boot] [PATCH 02/16] efi_loader: simplify efi_remove_all_protocols

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Replace list_for_each_safe() and list_entry() by
> list_for_each_entry_safe().
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 03/16] efi_selftest: do not try to close device path protocol

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> CloseProtocol cannot be called without agent handle.
>
> There is no need to close the device path protocol if
> it has been opened without agent handle.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_selftest/efi_selftest_devicepath.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 04/16] efi_loader: list of open protocol infos

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Add a list of open protocol infos to each protocol of a handle.
>
> Provide helper functions to access the list items.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_loader.h          | 15 ++++++++++++++-
>  lib/efi_loader/efi_boottime.c | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 05/16] efi_loader: open_info in OpenProtocol

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> efi_open_protocol has to keep track of opened protocols.
>
> OpenProtocol enters the agent and controller handle
> information into this list.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 107 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 103 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Would be good to mention that the test for this code comes later.

[U-Boot] [PATCH 06/16] efi_loader: open_info in CloseProtocol

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> efi_open_protocol and efi_close_protocol have to keep track of
> opened protocols.
>
> Check if the protocol was opened for the same agent and
> controller.
>
> Remove all open protocol information for this pair.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 07/16] efi_loader: implement OpenProtocolInformation

[Simon Glass]

On 17 December 2017 at 08:43, 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.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)


Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 08/16] efi_loader: debug output installed device path

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> When a device path protocol is installed write the device
> path to the console in debug mode.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 09/16] efi_loader: implement ConnectController

[Simon Glass]

Hi Heinrick,

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Implement the ConnectController boot service.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_api.h             |  22 ++++++
>  include/efi_loader.h          |   2 +
>  lib/efi_loader/efi_boottime.c | 178 ++++++++++++++++++++++++++++++++++++------
>  3 files changed, 178 insertions(+), 24 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Please see below. Also please mention when tests come for these.

>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 46963f2891..81e580dbbc 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -805,4 +805,26 @@ struct efi_file_info {
>         s16 file_name[0];
>  };
>
> +#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,
> +                       efi_uintn_t 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 637e6e166d..9e1ae8866b 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -88,6 +88,8 @@ uint16_t *efi_dp_str(struct efi_device_path *dp);
>  extern const efi_guid_t efi_global_variable_guid;
>  extern const efi_guid_t efi_guid_console_control;
>  extern const efi_guid_t efi_guid_device_path;
> +/* GUID of the EFI_DRIVER_BINDING_PROTOCOL */
> +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;
>  extern const efi_guid_t efi_simple_file_system_protocol_guid;
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index b5d6808bf7..6cc0659eb9 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -56,6 +56,9 @@ static volatile void *efi_gd, *app_gd;
>
>  static int entry_count;
>  static int nesting_level;
> +/* GUID of the EFI_DRIVER_BINDING_PROTOCOL */
> +const efi_guid_t efi_guid_driver_binding_protocol =
> +                       EFI_DRIVER_BINDING_PROTOCOL_GUID;
>
>  /* Called on every callback entry */
>  int __efi_entry_check(void)
> @@ -1619,30 +1622,6 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
>         return EFI_EXIT(efi_set_watchdog(timeout));
>  }
>
> -/*
> - * Connect a controller to a driver.
> - *
> - * This function implements the ConnectController service.
> - * See the Unified Extensible Firmware Interface (UEFI) specification
> - * for details.
> - *
> - * @controller_handle  handle of the controller
> - * @driver_image_handle        handle of the driver
> - * @remain_device_path device path of a child controller
> - * @recursive          true to connect all child controllers
> - * @return             status code
> - */
> -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_ENTRY("%p, %p, %p, %d", controller_handle, driver_image_handle,
> -                 remain_device_path, recursive);
> -       return EFI_EXIT(EFI_NOT_FOUND);
> -}
> -
>  /*
>   * Disconnect a controller from a driver.
>   *
> @@ -2352,6 +2331,157 @@ static efi_status_t EFIAPI efi_handle_protocol(void *handle,
>                                  NULL, EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL);
>  }
>
> +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;
> +       size_t 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;

What happens to any error here?

> +                               }
> +                       }
> +               }
> +       }
> +
> +       /*
> +        * TODO: Some overrides are not yet implemented:
> +        * - Platform Driver Override
> +        * - Driver Family Override Search
> +        * - Bus Specific Driver Override

Bus-specific ?

> +        */
> +
> +       /* 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;
> +}
> +
> +/*
> + * Connect a controller to a driver.
> + *
> + * This function implements the ConnectController service.
> + * See the Unified Extensible Firmware Interface (UEFI) specification
> + * for details.

Well I think it would be good to explain briefly what it does.

> + *
> + * @controller_handle  handle of the controller
> + * @driver_image_handle        handle of the driver
> + * @remain_device_path device path of a child controller
> + * @recursive          true to connect all child controllers
> + * @return             status code
> + */
> +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_status_t ret = EFI_NOT_FOUND;
> +       struct efi_object *efiobj;
> +
> +       EFI_ENTRY("%p, %p, %p, %d", controller_handle, driver_image_handle,
> +                 remain_device_path, recursive);
> +
> +       efiobj = efi_search_obj(controller_handle);
> +       if (!efiobj) {
> +               ret = EFI_INVALID_PARAMETER;
> +               goto out;
> +       }
> +
> +       r = efi_connect_single_controller(controller_handle,
> +                                         driver_image_handle,
> +                                         remain_device_path);
> +       if (r == EFI_SUCCESS)
> +               ret = EFI_SUCCESS;
> +       if (recursive) {
> +               struct efi_handler *handler;
> +               struct efi_open_protocol_info_item *item;
> +
> +               list_for_each_entry(handler, &efiobj->protocols, link) {
> +                       list_for_each_entry(item, &handler->open_infos, link) {
> +                               if (item->info.attributes &
> +                                   EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) {
> +                                       r = EFI_CALL(efi_connect_controller(
> +                                               item->info.controller_handle,
> +                                               driver_image_handle,
> +                                               remain_device_path,
> +                                               recursive));
> +                                       if (r == EFI_SUCCESS)
> +                                               ret = EFI_SUCCESS;
> +                               }
> +                       }
> +               }
> +       }
> +       /*  Check for child controller specified by end node */
> +       if (ret != EFI_SUCCESS && remain_device_path &&
> +           remain_device_path->type == DEVICE_PATH_TYPE_END)
> +               ret = EFI_SUCCESS;
> +out:
> +       return EFI_EXIT(ret);
> +}
> +
>  static const struct efi_boot_services efi_boot_services = {
>         .hdr = {
>                 .headersize = sizeof(struct efi_table_hdr),
> --
> 2.14.2
>

Regards,
Simon

[U-Boot] [PATCH 10/16] efi_loader: fix signature of efi_disconnect_controller

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Handles should be passed as efi_handle_t and not as void *.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_api.h             | 6 ++++--
>  lib/efi_loader/efi_boottime.c | 7 ++++---
>  2 files changed, 8 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 11/16] efi_loader: implement DisconnectController

[Simon Glass]

Hi Heinrich,

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Unfortunately we need a forward declaration because both
> OpenProtocol and CloseProtocol have to call DisconnectController.
> And DisconnectController calls both OpenProtcol and CloseProtocol.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 283 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 261 insertions(+), 22 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

I think it would be good to reduce the length of some of the identifies.

e.g. numbers_of_children -> child_count or num_children

It's just too verbose for U-Boot IMO.

- Simon

[U-Boot] [PATCH 12/16] efi_loader: disconnect controllers in UninstallProtocol

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The UninstallProtocol boot service should first try to
> disconnect controllers that have been connected with
> EFI_OPEN_PROTOCOL_BY_DRIVER.
>
> If the protocol is still opened by an agent, it should be
> closed.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 13/16] efi_selftest: remove todo in manage protocols

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The installation of UninstallProtocols is functional now.
> So we do not expect errors when calling it.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_selftest/efi_selftest_manageprotocols.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 14/16] efi_selftest: remove todo in device path test

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The installation of UninstallProtocol is functional now.
> So we do not expect errors when calling it.
>
> Call UninstallProtocol with correct level of indirection
> for parameter handle.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_selftest/efi_selftest_devicepath.c | 40 +++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 15 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 15/16] efi_selftest: test for (Dis)ConnectController

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> This unit test checks the following protocol services:
> ConnectController, DisconnectController,
> InstallProtocol, UninstallProtocol,
> OpenProtocol, CloseProtcol, OpenProtocolInformation
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_selftest/Makefile                   |   1 +
>  lib/efi_selftest/efi_selftest_controllers.c | 385 ++++++++++++++++++++++++++++
>  2 files changed, 386 insertions(+)
>  create mode 100644 lib/efi_selftest/efi_selftest_controllers.c

Reviewed-by: Simon Glass <sjg@chromium.org>

Again I think some identifiers should be shorter.

The EFI_SUCCESS thing shows up again, i.e. instead of

return 0;
if (ret)

we do:

return EFI_SUCCESS;
if (ret != EFI_SUCCESS)

which to me is an obfuscation. The error code indicates an error, and
is 0 if there is no error, which is the normal case. This is how
U-Boot works in general and I would prefer that the EFI code followed
that.

Regards,
Simon

[U-Boot] [PATCH 16/16] efi_loader: consistently use efi_handle_t for handles

[Simon Glass]

On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> We should consistently use the efi_handle_t typedef when
> referring to handles.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  cmd/bootefi.c                 | 10 ++++-----
>  include/efi_api.h             | 20 ++++++++++--------
>  include/efi_loader.h          | 14 +++++++------
>  lib/efi_loader/efi_boottime.c | 49 +++++++++++++++++++++++--------------------
>  lib/efi_loader/efi_console.c  |  6 +++---
>  5 files changed, 53 insertions(+), 46 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[...]

> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 98497db612..56b079cee8 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -503,21 +503,21 @@ int efi_console_register(void)
>         struct efi_object *efi_console_input_obj;
>
>         /* Create handles */
> -       r = efi_create_handle((void **)&efi_console_control_obj);
> +       r = efi_create_handle((efi_handle_t *)&efi_console_control_obj);

How come we need this cast?

Regards,
Simon

[U-Boot] [PATCH 11/16] efi_loader: implement DisconnectController

[Heinrich Schuchardt]

On 01/08/2018 04:35 AM, Simon Glass wrote:
> Hi Heinrich,
> 
> On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> Unfortunately we need a forward declaration because both
>> OpenProtocol and CloseProtocol have to call DisconnectController.
>> And DisconnectController calls both OpenProtcol and CloseProtocol.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   lib/efi_loader/efi_boottime.c | 283 ++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 261 insertions(+), 22 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I think it would be good to reduce the length of some of the identifies.
> 
> e.g. numbers_of_children -> child_count or num_children

number_of_children is what we used in the function definition in 
efi_api.h and is the name of the parameter in the UEFI spec.

I understand that you do not like bloat. But I tend to get confused when 
parameter names differ from the spec.

Regards

Heinrich

> 
> It's just too verbose for U-Boot IMO.
> 
> - Simon
> 

[U-Boot] [PATCH 16/16] efi_loader: consistently use efi_handle_t for handles

[Heinrich Schuchardt]

On 01/08/2018 04:37 AM, Simon Glass wrote:
> On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> We should consistently use the efi_handle_t typedef when
>> referring to handles.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   cmd/bootefi.c                 | 10 ++++-----
>>   include/efi_api.h             | 20 ++++++++++--------
>>   include/efi_loader.h          | 14 +++++++------
>>   lib/efi_loader/efi_boottime.c | 49 +++++++++++++++++++++++--------------------
>>   lib/efi_loader/efi_console.c  |  6 +++---
>>   5 files changed, 53 insertions(+), 46 deletions(-)
>>
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> [...]
> 
>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>> index 98497db612..56b079cee8 100644
>> --- a/lib/efi_loader/efi_console.c
>> +++ b/lib/efi_loader/efi_console.c
>> @@ -503,21 +503,21 @@ int efi_console_register(void)
>>          struct efi_object *efi_console_input_obj;
>>
>>          /* Create handles */
>> -       r = efi_create_handle((void **)&efi_console_control_obj);
>> +       r = efi_create_handle((efi_handle_t *)&efi_console_control_obj);
> 
> How come we need this cast?

Without any cast we get
"warning: passing argument 1 of ‘efi_create_handle’ from incompatible 
pointer type"

In most cases efi_handle_t is a (void *) pointer to a struct efi_object. 
But there is at least an exception for loaded images.

I would prefer if we could eliminate this insane duality. But that will 
need some careful cleanup.

Best regards

Heinrich

> 
> Regards,
> Simon
> 

[U-Boot] [PATCH 11/16] efi_loader: implement DisconnectController

[Simon Glass]

Hi Heinrich,

On 8 January 2018 at 16:11, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 01/08/2018 04:35 AM, Simon Glass wrote:
>>
>> Hi Heinrich,
>>
>> On 17 December 2017 at 08:43, Heinrich Schuchardt <xypron.glpk@gmx.de>
>> wrote:
>>>
>>> Unfortunately we need a forward declaration because both
>>> OpenProtocol and CloseProtocol have to call DisconnectController.
>>> And DisconnectController calls both OpenProtcol and CloseProtocol.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>>   lib/efi_loader/efi_boottime.c | 283
>>> ++++++++++++++++++++++++++++++++++++++----
>>>   1 file changed, 261 insertions(+), 22 deletions(-)
>>
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> I think it would be good to reduce the length of some of the identifies.
>>
>> e.g. numbers_of_children -> child_count or num_children
>
>
> number_of_children is what we used in the function definition in efi_api.h
> and is the name of the parameter in the UEFI spec.
>
> I understand that you do not like bloat. But I tend to get confused when
> parameter names differ from the spec.

Well please do your best. I don't think num_children is confusing.
Without getting into a discussion about the merits of the spec itself
I think we should try to avoid pulling its bloat into U-Boot.

>
> Regards
>
> Heinrich
>
>
>>
>> It's just too verbose for U-Boot IMO.
>>
>> - Simon
>>
>

Regards,
Simon