All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services
@ 2017-10-22 12:44 Heinrich Schuchardt
  2017-10-22 12:45 ` [U-Boot] [PATCH 1/9] efi_loader: capitalize EFI_LOCATE_SEARCH_TYPE values Heinrich Schuchardt
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:44 UTC (permalink / raw)
  To: u-boot

This patch series supplies a test case for boottime protocol
services that are used to manage protocols on handles.

The other patches do not introduce any new functionality
but clean up the code.

This series should be merged after
"efi_loader: implement SetWatchdogTimer".

Next think I am preparing is the conversion of the protocols array
to a linked list.

Heinrich Schuchardt (9):
  efi_loader: capitalize EFI_LOCATE_SEARCH_TYPE values
  efi_selftest: test protocol management
  efi_loader: eliminate efi_install_protocol_interface_ext
  efi_loader: eliminate efi_uninstall_protocol_interface_ext
  efi_loader: remove unused typedef for INTN
  efi_loader: replace UINTN by efi_uintn_t
  efi_loader: consistently use efi_uintn_t in boot services
  efi_loader: rework efi_locate_handle
  efi_loader: rework efi_search_obj

 include/efi.h                                   |   6 +-
 include/efi_api.h                               |  38 +--
 include/efi_loader.h                            |  18 +-
 include/efi_selftest.h                          |   9 +
 lib/efi_loader/efi_boottime.c                   | 203 ++++++-------
 lib/efi_loader/efi_memory.c                     |  20 +-
 lib/efi_selftest/Makefile                       |   3 +
 lib/efi_selftest/efi_selftest.c                 |   6 +-
 lib/efi_selftest/efi_selftest_events.c          |   2 +-
 lib/efi_selftest/efi_selftest_manageprotocols.c | 381 ++++++++++++++++++++++++
 lib/efi_selftest/efi_selftest_snp.c             |   2 +-
 lib/efi_selftest/efi_selftest_tpl.c             |   4 +-
 12 files changed, 533 insertions(+), 159 deletions(-)
 create mode 100644 lib/efi_selftest/efi_selftest_manageprotocols.c

-- 
2.14.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/9] efi_loader: capitalize EFI_LOCATE_SEARCH_TYPE values
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-06 16:58   ` Simon Glass
  2017-10-22 12:45 ` [U-Boot] [PATCH 2/9] efi_selftest: test protocol management Heinrich Schuchardt
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

Constants should be capitalized.
So rename the values of enum efi_locate_search_type.

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

diff --git a/include/efi.h b/include/efi.h
index dc8edc8743..2f0be9c86c 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -227,9 +227,9 @@ struct efi_time_cap {
 };
 
 enum efi_locate_search_type {
-	all_handles,
-	by_register_notify,
-	by_protocol
+	ALL_HANDLES,
+	BY_REGISTER_NOTIFY,
+	BY_PROTOCOL
 };
 
 struct efi_open_protocol_info_entry {
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index f70d8658ab..cdf895fb2d 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -934,11 +934,11 @@ static int efi_search(enum efi_locate_search_type search_type,
 	int i;
 
 	switch (search_type) {
-	case all_handles:
+	case ALL_HANDLES:
 		return 0;
-	case by_register_notify:
+	case BY_REGISTER_NOTIFY:
 		return -1;
-	case by_protocol:
+	case BY_PROTOCOL:
 		for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
 			const efi_guid_t *guid = efiobj->protocols[i].guid;
 			if (guid && !guidcmp(guid, protocol))
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 2/9] efi_selftest: test protocol management
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
  2017-10-22 12:45 ` [U-Boot] [PATCH 1/9] efi_loader: capitalize EFI_LOCATE_SEARCH_TYPE values Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-06 16:58   ` Simon Glass
  2017-10-22 12:45 ` [U-Boot] [PATCH 3/9] efi_loader: eliminate efi_install_protocol_interface_ext Heinrich Schuchardt
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

This unit test checks the following protocol services:
InstallProtocolInterface, UninstallProtocolInterface,
InstallMultipleProtocolsInterfaces,
UninstallMultipleProtocolsInterfaces,
HandleProtocol, ProtocolsPerHandle,
LocateHandle, LocateHandleBuffer.

As UninstallProtocolInterface and UninstallMultipleProtocolsInterfaces
are not completely implemented a TODO message will shown for
their failure.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_selftest.h                          |   9 +
 lib/efi_selftest/Makefile                       |   3 +
 lib/efi_selftest/efi_selftest_manageprotocols.c | 381 ++++++++++++++++++++++++
 3 files changed, 393 insertions(+)
 create mode 100644 lib/efi_selftest/efi_selftest_manageprotocols.c

diff --git a/include/efi_selftest.h b/include/efi_selftest.h
index 5cc8d4f600..be5ba4bfa9 100644
--- a/include/efi_selftest.h
+++ b/include/efi_selftest.h
@@ -27,6 +27,15 @@
 	(efi_st_printf("%s(%u):\nERROR: ", __FILE__, __LINE__), \
 	efi_st_printf(__VA_ARGS__)) \
 
+/*
+ * Prints a TODO message.
+ *
+ * @...	format string followed by fields to print
+ */
+#define efi_st_todo(...) \
+	(efi_st_printf("%s(%u):\nTODO: ", __FILE__, __LINE__), \
+	efi_st_printf(__VA_ARGS__)) \
+
 /*
  * A test may be setup and executed at boottime,
  * it may be setup at boottime and executed at runtime,
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 88b998bd4c..acd184db4b 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -15,6 +15,8 @@ CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)
 CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)
 CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI)
 CFLAGS_REMOVE_efi_selftest_exitbootservices.o := $(CFLAGS_NON_EFI)
+CFLAGS_efi_selftest_manageprotocols.o := $(CFLAGS_EFI)
+CFLAGS_REMOVE_efi_selftest_manageprotocols.o := $(CFLAGS_NON_EFI)
 CFLAGS_efi_selftest_snp.o := $(CFLAGS_EFI)
 CFLAGS_REMOVE_efi_selftest_snp.o := $(CFLAGS_NON_EFI)
 CFLAGS_efi_selftest_textoutput.o := $(CFLAGS_EFI)
@@ -31,6 +33,7 @@ efi_selftest.o \
 efi_selftest_console.o \
 efi_selftest_events.o \
 efi_selftest_exitbootservices.o \
+efi_selftest_manageprotocols.o \
 efi_selftest_snp.o \
 efi_selftest_textoutput.o \
 efi_selftest_tpl.o \
diff --git a/lib/efi_selftest/efi_selftest_manageprotocols.c b/lib/efi_selftest/efi_selftest_manageprotocols.c
new file mode 100644
index 0000000000..0a297f79ba
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_manageprotocols.c
@@ -0,0 +1,381 @@
+/*
+ * efi_selftest_manageprotocols
+ *
+ * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ *
+ * This unit test checks the following protocol services:
+ * InstallProtocolInterface, UninstallProtocolInterface,
+ * InstallMultipleProtocolsInterfaces, UninstallMultipleProtocolsInterfaces,
+ * HandleProtocol, ProtocolsPerHandle,
+ * LocateHandle, LocateHandleBuffer.
+ */
+
+#include <efi_selftest.h>
+
+static struct efi_boot_services *boottime;
+
+static efi_handle_t handle1;
+static efi_handle_t handle2;
+
+struct interface {
+	void (EFIAPI * inc)(void);
+};
+
+static efi_guid_t guid1 =
+	EFI_GUID(0x2e7ca819, 0x21d3, 0x0a3a,
+		 0xf7, 0x91, 0x82, 0x1f, 0x7a, 0x83, 0x67, 0xaf);
+static unsigned int counter1;
+void EFIAPI inc1(void)
+{
+	++counter1;
+}
+
+static struct interface interface1 = {
+	inc1,
+};
+
+static struct interface interface4 = {
+	inc1,
+};
+
+static efi_guid_t guid2 =
+	EFI_GUID(0xf909f2bb, 0x90a8, 0x0d77,
+		 0x94, 0x0c, 0x3e, 0xa8, 0xea, 0x38, 0xd6, 0x6f);
+static unsigned int counter2;
+void EFIAPI inc2(void)
+{
+	++counter2;
+}
+
+static struct interface interface2 = {
+	inc2,
+};
+
+static efi_guid_t guid3 =
+	EFI_GUID(0x06d641a3, 0xf4e7, 0xe0c9,
+		 0xe7, 0x8d, 0x41, 0x2d, 0x72, 0xa6, 0xb1, 0x24);
+static unsigned int counter3;
+void EFIAPI inc3(void)
+{
+	++counter3;
+}
+
+static struct interface interface3 = {
+	inc3,
+};
+
+/*
+ * Find a handle in an array.
+ *
+ * @handle:	handle to find
+ * @count:	number of entries in the array
+ * @buffer:	array to search
+ */
+efi_status_t find_in_buffer(efi_handle_t handle, size_t count,
+			    efi_handle_t *buffer)
+{
+	size_t i;
+
+	for (i = 0; i < count; ++i) {
+		if (buffer[i] == handle)
+			return EFI_SUCCESS;
+	}
+	return EFI_NOT_FOUND;
+}
+
+/*
+ * Setup unit test.
+ *
+ * Create two handles and install two out of three protocol interfaces on each
+ * of them:
+ *
+ * handle1
+ *   guid1 interface1
+ *   guid3 interface3
+ * handle2
+ *   guid1 interface4
+ *   guid2 interface2
+ *
+ * @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;
+	efi_handle_t handle;
+
+	boottime = systable->boottime;
+
+	ret = boottime->install_protocol_interface(&handle1, &guid3,
+						   EFI_NATIVE_INTERFACE,
+						   &interface3);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (!handle1) {
+		efi_st_error("InstallProtocolInterface failed to create handle\n");
+		return EFI_ST_FAILURE;
+	}
+	handle = handle1;
+	ret = boottime->install_protocol_interface(&handle1, &guid1,
+						   EFI_NATIVE_INTERFACE,
+						   &interface1);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (handle != handle1) {
+		efi_st_error("InstallProtocolInterface failed to use handle\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->install_multiple_protocol_interfaces(&handle2,
+			&guid1, &interface4, &guid2, &interface2, NULL);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallMultipleProtocolInterfaces failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (!handle2 || handle1 == handle2) {
+		efi_st_error("InstallMultipleProtocolInterfaces failed to create handle\n");
+		return EFI_ST_FAILURE;
+	}
+
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * Tear down unit test.
+ *
+ */
+static int teardown(void)
+{
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * Execute unit test.
+ *
+ */
+static int execute(void)
+{
+	struct interface *interface;
+	efi_status_t ret;
+	efi_handle_t *buffer;
+	size_t buffer_size;
+	unsigned long int count = 0;
+	efi_guid_t **prot_buffer;
+	unsigned long int prot_count;
+
+	/*
+	 * Test HandleProtocol
+	 */
+	ret = boottime->handle_protocol(handle1, &guid3, (void **)&interface);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("HandleProtocol failed to retrieve interface\n");
+		return EFI_ST_FAILURE;
+	}
+	if (interface != &interface3) {
+		efi_st_error("HandleProtocol returned wrong interface\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->handle_protocol(handle1, &guid2, (void **)&interface);
+	if (ret == EFI_SUCCESS) {
+		efi_st_error("HandleProtocol returned not installed interface\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/*
+	 * Test LocateHandleBuffer with AllHandles
+	 */
+	ret = boottime->locate_handle_buffer(ALL_HANDLES, NULL, NULL,
+					     &count, &buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandleBuffer with AllHandles failed\n");
+		return EFI_ST_FAILURE;
+	}
+	buffer_size = count;
+	ret = find_in_buffer(handle1, count, buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandleBuffer failed to locate new handle\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = find_in_buffer(handle2, count, buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandleBuffer failed to locate new handle\n");
+		return EFI_ST_FAILURE;
+	}
+	boottime->set_mem(buffer, sizeof(efi_handle_t) * buffer_size, 0);
+
+	/*
+	 * Test error handling in UninstallMultipleProtocols
+	 *
+	 * Try to uninstall more protocols than there are installed.
+	 */
+	ret = boottime->uninstall_multiple_protocol_interfaces(
+						handle2,
+						&guid1, &interface4,
+						&guid2, &interface2,
+						&guid3, &interface3,
+						NULL);
+	if (ret == EFI_SUCCESS) {
+		efi_st_todo("UninstallMultipleProtocolInterfaces did not catch error\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/*
+	 * Test LocateHandleBuffer with ByProtocol
+	 */
+	count = buffer_size;
+	ret = boottime->locate_handle_buffer(BY_PROTOCOL, &guid1, NULL,
+					     &count, &buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandleBuffer failed to locate new handles\n");
+		return EFI_ST_FAILURE;
+	}
+	if (count != 2) {
+		efi_st_error("LocateHandleBuffer failed to locate new handles\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = find_in_buffer(handle1, count, buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandleBuffer failed to locate new handle\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = find_in_buffer(handle2, count, buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandleBuffer failed to locate new handle\n");
+		return EFI_ST_FAILURE;
+	}
+	boottime->set_mem(buffer, sizeof(efi_handle_t) * buffer_size, 0);
+
+	/*
+	 * Test LocateHandle with ByProtocol
+	 */
+	count = buffer_size * sizeof(efi_handle_t);
+	ret = boottime->locate_handle(BY_PROTOCOL, &guid1, NULL,
+				      &count, buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandle with ByProtocol failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (count / sizeof(efi_handle_t) != 2) {
+		efi_st_error("LocateHandle failed to locate new handles\n");
+		return EFI_ST_FAILURE;
+	}
+	buffer_size = count;
+	ret = find_in_buffer(handle1, count, buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandle failed to locate new handles\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = find_in_buffer(handle2, count, buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandle failed to locate new handles\n");
+		return EFI_ST_FAILURE;
+	}
+	boottime->set_mem(buffer, sizeof(efi_handle_t) * buffer_size, 0);
+
+	/*
+	 * Test LocateProtocol
+	 */
+	ret = boottime->locate_protocol(&guid1, NULL, (void **)&interface);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateProtocol failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (interface != &interface1 && interface != &interface4) {
+		efi_st_error("LocateProtocol failed to locate protocol\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/*
+	 * Test UninstallMultipleProtocols
+	 */
+	ret = boottime->uninstall_multiple_protocol_interfaces(
+						handle2,
+						&guid1, &interface4,
+						&guid2, &interface2,
+						NULL);
+	if (ret != EFI_SUCCESS) {
+		efi_st_todo("UninstallMultipleProtocolInterfaces failed\n");
+		/* This test is known to fail due to missing implementation */
+	}
+	/*
+	 * Check that the protocols are really uninstalled.
+	 */
+	count = buffer_size;
+	ret = boottime->locate_handle_buffer(BY_PROTOCOL, &guid1, NULL,
+					     &count, &buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateHandleBuffer failed\n");
+		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 */
+	}
+	ret = find_in_buffer(handle1, count, buffer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to locate new handle\n");
+		return EFI_ST_FAILURE;
+	}
+	boottime->set_mem(buffer, sizeof(efi_handle_t) * buffer_size, 0);
+
+	/*
+	 * Test ProtocolsPerHandle
+	 */
+	ret = boottime->protocols_per_handle(handle1,
+					     &prot_buffer, &prot_count);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to get protocols per handle\n");
+		return EFI_ST_FAILURE;
+	}
+	if (prot_count != 2) {
+		efi_st_error("Failed to get protocols per handle\n");
+		return EFI_ST_FAILURE;
+	}
+	if (efi_st_memcmp(prot_buffer[0], &guid1, 16) &&
+	    efi_st_memcmp(prot_buffer[1], &guid1, 16)) {
+		efi_st_error("Failed to get protocols per handle\n");
+		return EFI_ST_FAILURE;
+	}
+	if (efi_st_memcmp(prot_buffer[0], &guid3, 16) &&
+	    efi_st_memcmp(prot_buffer[1], &guid3, 16)) {
+		efi_st_error("Failed to get protocols per handle\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/*
+	 * Uninstall remaining protocols
+	 */
+	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 */
+	}
+	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 */
+	}
+	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 */
+	}
+
+	return EFI_ST_SUCCESS;
+}
+
+EFI_UNIT_TEST(protserv) = {
+	.name = "manage protocols",
+	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+	.setup = setup,
+	.execute = execute,
+	.teardown = teardown,
+};
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 3/9] efi_loader: eliminate efi_install_protocol_interface_ext
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
  2017-10-22 12:45 ` [U-Boot] [PATCH 1/9] efi_loader: capitalize EFI_LOCATE_SEARCH_TYPE values Heinrich Schuchardt
  2017-10-22 12:45 ` [U-Boot] [PATCH 2/9] efi_selftest: test protocol management Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-06 16:58   ` Simon Glass
  2017-10-22 12:45 ` [U-Boot] [PATCH 4/9] efi_loader: eliminate efi_uninstall_protocol_interface_ext Heinrich Schuchardt
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

As we now have EFI_CALL there is no need for separate
functions efi_install_protocol_interface_ext and
efi_install_protocol_interface.

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index cdf895fb2d..c078f056db 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -702,9 +702,9 @@ static struct efi_object *efi_search_obj(void *handle)
 /*
  * Install protocol interface.
  *
- * This is the function for internal calls. For the API implementation of the
- * InstallProtocolInterface service see function
- * efi_install_protocol_interface_ext.
+ * This function implements the InstallProtocolInterface service.
+ * See the Unified Extensible Firmware Interface (UEFI) specification
+ * for details.
  *
  * @handle			handle on which the protocol shall be installed
  * @protocol			GUID of the protocol to be installed
@@ -713,14 +713,17 @@ static struct efi_object *efi_search_obj(void *handle)
  * @protocol_interface		interface of the protocol implementation
  * @return			status code
  */
-static efi_status_t EFIAPI efi_install_protocol_interface(void **handle,
-			const efi_guid_t *protocol, int protocol_interface_type,
-			void *protocol_interface)
+static efi_status_t EFIAPI efi_install_protocol_interface(
+			void **handle, const efi_guid_t *protocol,
+			int protocol_interface_type, void *protocol_interface)
 {
 	struct list_head *lhandle;
 	int i;
 	efi_status_t r;
 
+	EFI_ENTRY("%p, %pUl, %d, %p", handle, protocol, protocol_interface_type,
+		  protocol_interface);
+
 	if (!handle || !protocol ||
 	    protocol_interface_type != EFI_NATIVE_INTERFACE) {
 		r = EFI_INVALID_PARAMETER;
@@ -768,33 +771,7 @@ static efi_status_t EFIAPI efi_install_protocol_interface(void **handle,
 	}
 	r = EFI_INVALID_PARAMETER;
 out:
-	return r;
-}
-
-/*
- * Install protocol interface.
- *
- * This function implements the InstallProtocolInterface service.
- * See the Unified Extensible Firmware Interface (UEFI) specification
- * for details.
- *
- * @handle			handle on which the protocol shall be installed
- * @protocol			GUID of the protocol to be installed
- * @protocol_interface_type	type of the interface to be installed,
- *				always EFI_NATIVE_INTERFACE
- * @protocol_interface		interface of the protocol implementation
- * @return			status code
- */
-static efi_status_t EFIAPI efi_install_protocol_interface_ext(void **handle,
-			const efi_guid_t *protocol, int protocol_interface_type,
-			void *protocol_interface)
-{
-	EFI_ENTRY("%p, %pUl, %d, %p", handle, protocol, protocol_interface_type,
-		  protocol_interface);
-
-	return EFI_EXIT(efi_install_protocol_interface(handle, protocol,
-						       protocol_interface_type,
-						       protocol_interface));
+	return EFI_EXIT(r);
 }
 
 /*
@@ -1790,9 +1767,10 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces(
 		if (!protocol)
 			break;
 		protocol_interface = va_arg(argptr, void*);
-		r = efi_install_protocol_interface(handle, protocol,
-						   EFI_NATIVE_INTERFACE,
-						   protocol_interface);
+		r = EFI_CALL(efi_install_protocol_interface(
+						handle, protocol,
+						EFI_NATIVE_INTERFACE,
+						protocol_interface));
 		if (r != EFI_SUCCESS)
 			break;
 		i++;
@@ -2015,7 +1993,7 @@ static const struct efi_boot_services efi_boot_services = {
 	.signal_event = efi_signal_event_ext,
 	.close_event = efi_close_event,
 	.check_event = efi_check_event,
-	.install_protocol_interface = efi_install_protocol_interface_ext,
+	.install_protocol_interface = efi_install_protocol_interface,
 	.reinstall_protocol_interface = efi_reinstall_protocol_interface,
 	.uninstall_protocol_interface = efi_uninstall_protocol_interface_ext,
 	.handle_protocol = efi_handle_protocol,
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/9] efi_loader: eliminate efi_uninstall_protocol_interface_ext
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
                   ` (2 preceding siblings ...)
  2017-10-22 12:45 ` [U-Boot] [PATCH 3/9] efi_loader: eliminate efi_install_protocol_interface_ext Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-06 16:59   ` Simon Glass
  2017-10-22 12:45 ` [U-Boot] [PATCH 5/9] efi_loader: remove unused typedef for INTN Heinrich Schuchardt
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

As we now have EFI_CALL there is no need for separate
functions efi_uninstall_protocol_interface_ext and
efi_uninstall_protocol_interface.

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index c078f056db..4f86ddccea 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -800,22 +800,25 @@ static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle,
 /*
  * Uninstall protocol interface.
  *
- * This is the function for internal calls. For the API implementation of the
- * UninstallProtocolInterface service see function
- * efi_uninstall_protocol_interface_ext.
+ * This function implements the UninstallProtocolInterface service.
+ * See the Unified Extensible Firmware Interface (UEFI) specification
+ * for details.
  *
  * @handle			handle from which the protocol shall be removed
  * @protocol			GUID of the protocol to be removed
  * @protocol_interface		interface to be removed
  * @return			status code
  */
-static efi_status_t EFIAPI efi_uninstall_protocol_interface(void *handle,
-			const efi_guid_t *protocol, void *protocol_interface)
+static efi_status_t EFIAPI efi_uninstall_protocol_interface(
+				void *handle, const efi_guid_t *protocol,
+				void *protocol_interface)
 {
 	struct list_head *lhandle;
 	int i;
 	efi_status_t r = EFI_NOT_FOUND;
 
+	EFI_ENTRY("%p, %pUl, %p", handle, protocol, protocol_interface);
+
 	if (!handle || !protocol) {
 		r = EFI_INVALID_PARAMETER;
 		goto out;
@@ -847,28 +850,7 @@ static efi_status_t EFIAPI efi_uninstall_protocol_interface(void *handle,
 	}
 
 out:
-	return r;
-}
-
-/*
- * Uninstall protocol interface.
- *
- * This function implements the UninstallProtocolInterface service.
- * See the Unified Extensible Firmware Interface (UEFI) specification
- * for details.
- *
- * @handle			handle from which the protocol shall be removed
- * @protocol			GUID of the protocol to be removed
- * @protocol_interface		interface to be removed
- * @return			status code
- */
-static efi_status_t EFIAPI efi_uninstall_protocol_interface_ext(void *handle,
-			const efi_guid_t *protocol, void *protocol_interface)
-{
-	EFI_ENTRY("%p, %pUl, %p", handle, protocol, protocol_interface);
-
-	return EFI_EXIT(efi_uninstall_protocol_interface(handle, protocol,
-							 protocol_interface));
+	return EFI_EXIT(r);
 }
 
 /*
@@ -1784,8 +1766,8 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces(
 	for (; i; --i) {
 		protocol = va_arg(argptr, efi_guid_t*);
 		protocol_interface = va_arg(argptr, void*);
-		efi_uninstall_protocol_interface(handle, protocol,
-						 protocol_interface);
+		EFI_CALL(efi_uninstall_protocol_interface(handle, protocol,
+							  protocol_interface));
 	}
 	va_end(argptr);
 
@@ -1995,7 +1977,7 @@ static const struct efi_boot_services efi_boot_services = {
 	.check_event = efi_check_event,
 	.install_protocol_interface = efi_install_protocol_interface,
 	.reinstall_protocol_interface = efi_reinstall_protocol_interface,
-	.uninstall_protocol_interface = efi_uninstall_protocol_interface_ext,
+	.uninstall_protocol_interface = efi_uninstall_protocol_interface,
 	.handle_protocol = efi_handle_protocol,
 	.reserved = NULL,
 	.register_protocol_notify = efi_register_protocol_notify,
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 5/9] efi_loader: remove unused typedef for INTN
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
                   ` (3 preceding siblings ...)
  2017-10-22 12:45 ` [U-Boot] [PATCH 4/9] efi_loader: eliminate efi_uninstall_protocol_interface_ext Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-06 17:02   ` Simon Glass
  2017-10-22 12:45 ` [U-Boot] [PATCH 6/9] efi_loader: replace UINTN by efi_uintn_t Heinrich Schuchardt
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

INTN is not used in the coding.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_api.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index fcd7483ab2..8ea44b1a7a 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -29,7 +29,6 @@ enum efi_timer_delay {
 };
 
 #define UINTN size_t
-typedef long INTN;
 typedef uint16_t *efi_string_t;
 
 #define EVT_TIMER				0x80000000
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 6/9] efi_loader: replace UINTN by efi_uintn_t
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
                   ` (4 preceding siblings ...)
  2017-10-22 12:45 ` [U-Boot] [PATCH 5/9] efi_loader: remove unused typedef for INTN Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-06 17:02   ` Simon Glass
  2017-10-22 12:45 ` [U-Boot] [PATCH 7/9] efi_loader: consistently use efi_uintn_t in boot services Heinrich Schuchardt
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

UINTN is used in the UEFI specification for unsigned integers
matching the bitness of the CPU.

Types in U-Boot should be lower case. The patch replaces it
by efi_uintn_t.

Suggested-by: Simon Glass <sjg@chromium.org>
Suggested-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_api.h                   |  8 ++++----
 include/efi_loader.h                |  4 ++--
 lib/efi_loader/efi_boottime.c       | 12 ++++++------
 lib/efi_selftest/efi_selftest_tpl.c |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 8ea44b1a7a..5ab78baeea 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -28,7 +28,7 @@ enum efi_timer_delay {
 	EFI_TIMER_RELATIVE = 2
 };
 
-#define UINTN size_t
+#define efi_uintn_t size_t
 typedef uint16_t *efi_string_t;
 
 #define EVT_TIMER				0x80000000
@@ -48,8 +48,8 @@ struct efi_event;
 /* EFI Boot Services table */
 struct efi_boot_services {
 	struct efi_table_hdr hdr;
-	efi_status_t (EFIAPI *raise_tpl)(UINTN new_tpl);
-	void (EFIAPI *restore_tpl)(UINTN old_tpl);
+	efi_status_t (EFIAPI *raise_tpl)(efi_uintn_t new_tpl);
+	void (EFIAPI *restore_tpl)(efi_uintn_t old_tpl);
 
 	efi_status_t (EFIAPI *allocate_pages)(int, int, unsigned long,
 					      efi_physical_addr_t *);
@@ -61,7 +61,7 @@ struct efi_boot_services {
 	efi_status_t (EFIAPI *free_pool)(void *);
 
 	efi_status_t (EFIAPI *create_event)(uint32_t type,
-			UINTN notify_tpl,
+			efi_uintn_t notify_tpl,
 			void (EFIAPI *notify_function) (
 					struct efi_event *event,
 					void *context),
diff --git a/include/efi_loader.h b/include/efi_loader.h
index e506eeec61..83b27d0449 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -144,7 +144,7 @@ struct efi_object {
  */
 struct efi_event {
 	uint32_t type;
-	UINTN notify_tpl;
+	efi_uintn_t notify_tpl;
 	void (EFIAPI *notify_function)(struct efi_event *event, void *context);
 	void *notify_context;
 	u64 trigger_next;
@@ -193,7 +193,7 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
 /* Call this to set the current device name */
 void efi_set_bootdev(const char *dev, const char *devnr, const char *path);
 /* Call this to create an event */
-efi_status_t efi_create_event(uint32_t type, UINTN notify_tpl,
+efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl,
 			      void (EFIAPI *notify_function) (
 					struct efi_event *event,
 					void *context),
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 4f86ddccea..dd2347e04d 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -21,7 +21,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 /* Task priority level */
-static UINTN efi_tpl = TPL_APPLICATION;
+static efi_uintn_t efi_tpl = TPL_APPLICATION;
 
 /* This list contains all the EFI objects our payload has access to */
 LIST_HEAD(efi_obj_list);
@@ -165,9 +165,9 @@ void efi_signal_event(struct efi_event *event)
  * @new_tpl	new value of the task priority level
  * @return	old value of the task priority level
  */
-static unsigned long EFIAPI efi_raise_tpl(UINTN new_tpl)
+static unsigned long EFIAPI efi_raise_tpl(efi_uintn_t new_tpl)
 {
-	UINTN old_tpl = efi_tpl;
+	efi_uintn_t old_tpl = efi_tpl;
 
 	EFI_ENTRY("0x%zx", new_tpl);
 
@@ -190,7 +190,7 @@ static unsigned long EFIAPI efi_raise_tpl(UINTN new_tpl)
  *
  * @old_tpl	value of the task priority level to be restored
  */
-static void EFIAPI efi_restore_tpl(UINTN old_tpl)
+static void EFIAPI efi_restore_tpl(efi_uintn_t old_tpl)
 {
 	EFI_ENTRY("0x%zx", old_tpl);
 
@@ -359,7 +359,7 @@ static struct efi_event efi_events[16];
  * @event		created event
  * @return		status code
  */
-efi_status_t efi_create_event(uint32_t type, UINTN notify_tpl,
+efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl,
 			      void (EFIAPI *notify_function) (
 					struct efi_event *event,
 					void *context),
@@ -409,7 +409,7 @@ efi_status_t efi_create_event(uint32_t type, UINTN notify_tpl,
  * @return		status code
  */
 static efi_status_t EFIAPI efi_create_event_ext(
-			uint32_t type, UINTN notify_tpl,
+			uint32_t type, efi_uintn_t notify_tpl,
 			void (EFIAPI *notify_function) (
 					struct efi_event *event,
 					void *context),
diff --git a/lib/efi_selftest/efi_selftest_tpl.c b/lib/efi_selftest/efi_selftest_tpl.c
index ddb67ed268..15d62903c0 100644
--- a/lib/efi_selftest/efi_selftest_tpl.c
+++ b/lib/efi_selftest/efi_selftest_tpl.c
@@ -113,7 +113,7 @@ static int execute(void)
 {
 	size_t index;
 	efi_status_t ret;
-	UINTN old_tpl;
+	efi_uintn_t old_tpl;
 
 	/* Set 10 ms timer */
 	notification_count = 0;
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 7/9] efi_loader: consistently use efi_uintn_t in boot services
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
                   ` (5 preceding siblings ...)
  2017-10-22 12:45 ` [U-Boot] [PATCH 6/9] efi_loader: replace UINTN by efi_uintn_t Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-07  5:21   ` Simon Glass
  2017-10-22 12:45 ` [U-Boot] [PATCH 8/9] efi_loader: rework efi_locate_handle Heinrich Schuchardt
  2017-10-22 12:45 ` [U-Boot] [PATCH 9/9] efi_loader: rework efi_search_obj Heinrich Schuchardt
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

Consistenly use efi_uintn_t wherever the UEFI spec uses
UINTN in boot services interfaces.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/efi_api.h                               | 29 ++++++++++---------
 include/efi_loader.h                            | 12 ++++----
 lib/efi_loader/efi_boottime.c                   | 38 ++++++++++++-------------
 lib/efi_loader/efi_memory.c                     | 20 ++++++-------
 lib/efi_selftest/efi_selftest.c                 |  6 ++--
 lib/efi_selftest/efi_selftest_events.c          |  2 +-
 lib/efi_selftest/efi_selftest_manageprotocols.c |  4 +--
 lib/efi_selftest/efi_selftest_snp.c             |  2 +-
 lib/efi_selftest/efi_selftest_tpl.c             |  2 +-
 9 files changed, 59 insertions(+), 56 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 5ab78baeea..e0991b6eca 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -51,13 +51,15 @@ struct efi_boot_services {
 	efi_status_t (EFIAPI *raise_tpl)(efi_uintn_t new_tpl);
 	void (EFIAPI *restore_tpl)(efi_uintn_t old_tpl);
 
-	efi_status_t (EFIAPI *allocate_pages)(int, int, unsigned long,
+	efi_status_t (EFIAPI *allocate_pages)(int, int, efi_uintn_t,
 					      efi_physical_addr_t *);
-	efi_status_t (EFIAPI *free_pages)(efi_physical_addr_t, unsigned long);
-	efi_status_t (EFIAPI *get_memory_map)(unsigned long *memory_map_size,
-			struct efi_mem_desc *desc, unsigned long *key,
-			unsigned long *desc_size, u32 *desc_version);
-	efi_status_t (EFIAPI *allocate_pool)(int, unsigned long, void **);
+	efi_status_t (EFIAPI *free_pages)(efi_physical_addr_t, efi_uintn_t);
+	efi_status_t (EFIAPI *get_memory_map)(efi_uintn_t *memory_map_size,
+					      struct efi_mem_desc *desc,
+					      efi_uintn_t *key,
+					      efi_uintn_t *desc_size,
+					      u32 *desc_version);
+	efi_status_t (EFIAPI *allocate_pool)(int, efi_uintn_t, void **);
 	efi_status_t (EFIAPI *free_pool)(void *);
 
 	efi_status_t (EFIAPI *create_event)(uint32_t type,
@@ -69,8 +71,9 @@ struct efi_boot_services {
 	efi_status_t (EFIAPI *set_timer)(struct efi_event *event,
 					 enum efi_timer_delay type,
 					 uint64_t trigger_time);
-	efi_status_t (EFIAPI *wait_for_event)(unsigned long number_of_events,
-			struct efi_event **event, size_t *index);
+	efi_status_t (EFIAPI *wait_for_event)(efi_uintn_t number_of_events,
+					      struct efi_event **event,
+					      efi_uintn_t *index);
 	efi_status_t (EFIAPI *signal_event)(struct efi_event *event);
 	efi_status_t (EFIAPI *close_event)(struct efi_event *event);
 	efi_status_t (EFIAPI *check_event)(struct efi_event *event);
@@ -93,7 +96,7 @@ struct efi_boot_services {
 	efi_status_t (EFIAPI *locate_handle)(
 			enum efi_locate_search_type search_type,
 			const efi_guid_t *protocol, void *search_key,
-			unsigned long *buffer_size, efi_handle_t *buffer);
+			efi_uintn_t *buffer_size, efi_handle_t *buffer);
 	efi_status_t (EFIAPI *locate_device_path)(const efi_guid_t *protocol,
 			struct efi_device_path **device_path,
 			efi_handle_t *device);
@@ -140,14 +143,14 @@ struct efi_boot_services {
 	efi_status_t(EFIAPI *open_protocol_information)(efi_handle_t handle,
 			const efi_guid_t *protocol,
 			struct efi_open_protocol_info_entry **entry_buffer,
-			unsigned long *entry_count);
+			efi_uintn_t *entry_count);
 	efi_status_t (EFIAPI *protocols_per_handle)(efi_handle_t handle,
 			efi_guid_t ***protocol_buffer,
-			unsigned long *protocols_buffer_count);
+			efi_uintn_t *protocols_buffer_count);
 	efi_status_t (EFIAPI *locate_handle_buffer) (
 			enum efi_locate_search_type search_type,
 			const efi_guid_t *protocol, void *search_key,
-			unsigned long *no_handles, efi_handle_t **buffer);
+			efi_uintn_t *no_handles, efi_handle_t **buffer);
 	efi_status_t (EFIAPI *locate_protocol)(const efi_guid_t *protocol,
 			void *registration, void **protocol_interface);
 	efi_status_t (EFIAPI *install_multiple_protocol_interfaces)(
@@ -248,7 +251,7 @@ struct efi_system_table {
 	struct efi_simple_text_output_protocol *std_err;
 	struct efi_runtime_services *runtime;
 	struct efi_boot_services *boottime;
-	unsigned long nr_tables;
+	efi_uintn_t nr_tables;
 	struct efi_configuration_table *tables;
 };
 
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 83b27d0449..e3d1c35930 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -215,20 +215,20 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp);
 /* Generic EFI memory allocator, call this to get memory */
 void *efi_alloc(uint64_t len, int memory_type);
 /* More specific EFI memory allocator, called by EFI payloads */
-efi_status_t efi_allocate_pages(int type, int memory_type, unsigned long pages,
+efi_status_t efi_allocate_pages(int type, int memory_type, efi_uintn_t pages,
 				uint64_t *memory);
 /* EFI memory free function. */
-efi_status_t efi_free_pages(uint64_t memory, unsigned long pages);
+efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages);
 /* EFI memory allocator for small allocations */
-efi_status_t efi_allocate_pool(int pool_type, unsigned long size,
+efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size,
 			       void **buffer);
 /* EFI pool memory free function. */
 efi_status_t efi_free_pool(void *buffer);
 /* Returns the EFI memory map */
-efi_status_t efi_get_memory_map(unsigned long *memory_map_size,
+efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size,
 				struct efi_mem_desc *memory_map,
-				unsigned long *map_key,
-				unsigned long *descriptor_size,
+				efi_uintn_t *map_key,
+				efi_uintn_t *descriptor_size,
 				uint32_t *descriptor_version);
 /* Adds a range into the EFI memory map */
 uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type,
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index dd2347e04d..818d54abd1 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -217,12 +217,12 @@ static void EFIAPI efi_restore_tpl(efi_uintn_t old_tpl)
  * @return		status code
  */
 static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int memory_type,
-						  unsigned long pages,
+						  efi_uintn_t pages,
 						  uint64_t *memory)
 {
 	efi_status_t r;
 
-	EFI_ENTRY("%d, %d, 0x%lx, %p", type, memory_type, pages, memory);
+	EFI_ENTRY("%d, %d, 0x%zx, %p", type, memory_type, pages, memory);
 	r = efi_allocate_pages(type, memory_type, pages, memory);
 	return EFI_EXIT(r);
 }
@@ -239,11 +239,11 @@ static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int memory_type,
  * @return	status code
  */
 static efi_status_t EFIAPI efi_free_pages_ext(uint64_t memory,
-					      unsigned long pages)
+					      efi_uintn_t pages)
 {
 	efi_status_t r;
 
-	EFI_ENTRY("%"PRIx64", 0x%lx", memory, pages);
+	EFI_ENTRY("%"PRIx64", 0x%zx", memory, pages);
 	r = efi_free_pages(memory, pages);
 	return EFI_EXIT(r);
 }
@@ -264,10 +264,10 @@ static efi_status_t EFIAPI efi_free_pages_ext(uint64_t memory,
  * @return		status code
  */
 static efi_status_t EFIAPI efi_get_memory_map_ext(
-					unsigned long *memory_map_size,
+					efi_uintn_t *memory_map_size,
 					struct efi_mem_desc *memory_map,
-					unsigned long *map_key,
-					unsigned long *descriptor_size,
+					efi_uintn_t *map_key,
+					efi_uintn_t *descriptor_size,
 					uint32_t *descriptor_version)
 {
 	efi_status_t r;
@@ -292,12 +292,12 @@ static efi_status_t EFIAPI efi_get_memory_map_ext(
  * @return	status code
  */
 static efi_status_t EFIAPI efi_allocate_pool_ext(int pool_type,
-						 unsigned long size,
+						 efi_uintn_t size,
 						 void **buffer)
 {
 	efi_status_t r;
 
-	EFI_ENTRY("%d, %ld, %p", pool_type, size, buffer);
+	EFI_ENTRY("%d, %zd, %p", pool_type, size, buffer);
 	r = efi_allocate_pool(pool_type, size, buffer);
 	return EFI_EXIT(r);
 }
@@ -539,13 +539,13 @@ static efi_status_t EFIAPI efi_set_timer_ext(struct efi_event *event,
  * @index	index of the event that was signaled
  * @return	status code
  */
-static efi_status_t EFIAPI efi_wait_for_event(unsigned long num_events,
+static efi_status_t EFIAPI efi_wait_for_event(efi_uintn_t num_events,
 					      struct efi_event **event,
-					      size_t *index)
+					      efi_uintn_t *index)
 {
 	int i, j;
 
-	EFI_ENTRY("%ld, %p, %p", num_events, event, index);
+	EFI_ENTRY("%zd, %p, %p", num_events, event, index);
 
 	/* Check parameters */
 	if (!num_events || !event)
@@ -925,10 +925,10 @@ static int efi_search(enum efi_locate_search_type search_type,
 static efi_status_t efi_locate_handle(
 			enum efi_locate_search_type search_type,
 			const efi_guid_t *protocol, void *search_key,
-			unsigned long *buffer_size, efi_handle_t *buffer)
+			efi_uintn_t *buffer_size, efi_handle_t *buffer)
 {
 	struct list_head *lhandle;
-	unsigned long size = 0;
+	efi_uintn_t size = 0;
 
 	/* Count how much space we need */
 	list_for_each(lhandle, &efi_obj_list) {
@@ -977,7 +977,7 @@ static efi_status_t efi_locate_handle(
 static efi_status_t EFIAPI efi_locate_handle_ext(
 			enum efi_locate_search_type search_type,
 			const efi_guid_t *protocol, void *search_key,
-			unsigned long *buffer_size, efi_handle_t *buffer)
+			efi_uintn_t *buffer_size, efi_handle_t *buffer)
 {
 	EFI_ENTRY("%d, %pUl, %p, %p, %p", search_type, protocol, search_key,
 		  buffer_size, buffer);
@@ -1551,7 +1551,7 @@ static efi_status_t EFIAPI efi_close_protocol(void *handle,
 static efi_status_t EFIAPI efi_open_protocol_information(efi_handle_t handle,
 			const efi_guid_t *protocol,
 			struct efi_open_protocol_info_entry **entry_buffer,
-			unsigned long *entry_count)
+			efi_uintn_t *entry_count)
 {
 	EFI_ENTRY("%p, %pUl, %p, %p", handle, protocol, entry_buffer,
 		  entry_count);
@@ -1572,7 +1572,7 @@ static efi_status_t EFIAPI efi_open_protocol_information(efi_handle_t handle,
  */
 static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,
 			efi_guid_t ***protocol_buffer,
-			unsigned long *protocol_buffer_count)
+			efi_uintn_t *protocol_buffer_count)
 {
 	unsigned long buffer_size;
 	struct efi_object *efiobj;
@@ -1640,10 +1640,10 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,
 static efi_status_t EFIAPI efi_locate_handle_buffer(
 			enum efi_locate_search_type search_type,
 			const efi_guid_t *protocol, void *search_key,
-			unsigned long *no_handles, efi_handle_t **buffer)
+			efi_uintn_t *no_handles, efi_handle_t **buffer)
 {
 	efi_status_t r;
-	unsigned long buffer_size = 0;
+	efi_uintn_t buffer_size = 0;
 
 	EFI_ENTRY("%d, %pUl, %p, %p, %p", search_type, protocol, search_key,
 		  no_handles, buffer);
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index d47759e08e..0aa3e0881d 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -276,7 +276,7 @@ static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr)
 }
 
 efi_status_t efi_allocate_pages(int type, int memory_type,
-				unsigned long pages, uint64_t *memory)
+				efi_uintn_t pages, uint64_t *memory)
 {
 	u64 len = pages << EFI_PAGE_SHIFT;
 	efi_status_t r = EFI_SUCCESS;
@@ -338,7 +338,7 @@ void *efi_alloc(uint64_t len, int memory_type)
 	return NULL;
 }
 
-efi_status_t efi_free_pages(uint64_t memory, unsigned long pages)
+efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
 {
 	uint64_t r = 0;
 
@@ -351,7 +351,7 @@ efi_status_t efi_free_pages(uint64_t memory, unsigned long pages)
 	return EFI_NOT_FOUND;
 }
 
-efi_status_t efi_allocate_pool(int pool_type, unsigned long size,
+efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size,
 			       void **buffer)
 {
 	efi_status_t r;
@@ -392,16 +392,16 @@ efi_status_t efi_free_pool(void *buffer)
 	return r;
 }
 
-efi_status_t efi_get_memory_map(unsigned long *memory_map_size,
-			       struct efi_mem_desc *memory_map,
-			       unsigned long *map_key,
-			       unsigned long *descriptor_size,
-			       uint32_t *descriptor_version)
+efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size,
+				struct efi_mem_desc *memory_map,
+				efi_uintn_t *map_key,
+				efi_uintn_t *descriptor_size,
+				uint32_t *descriptor_version)
 {
-	ulong map_size = 0;
+	efi_uintn_t map_size = 0;
 	int map_entries = 0;
 	struct list_head *lhandle;
-	unsigned long provided_map_size = *memory_map_size;
+	efi_uintn_t provided_map_size = *memory_map_size;
 
 	list_for_each(lhandle, &efi_mem)
 		map_entries++;
diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c
index b6bc4dd5c0..263b53f672 100644
--- a/lib/efi_selftest/efi_selftest.c
+++ b/lib/efi_selftest/efi_selftest.c
@@ -32,9 +32,9 @@ static u16 reset_message[] = L"Selftest completed";
  */
 void efi_st_exit_boot_services(void)
 {
-	unsigned long map_size = 0;
-	unsigned long map_key;
-	unsigned long desc_size;
+	efi_uintn_t map_size = 0;
+	efi_uintn_t map_key;
+	efi_uintn_t desc_size;
 	u32 desc_version;
 	efi_status_t ret;
 	struct efi_mem_desc *memory_map;
diff --git a/lib/efi_selftest/efi_selftest_events.c b/lib/efi_selftest/efi_selftest_events.c
index 081f31257f..ad9490bd25 100644
--- a/lib/efi_selftest/efi_selftest_events.c
+++ b/lib/efi_selftest/efi_selftest_events.c
@@ -108,7 +108,7 @@ static int teardown(void)
  */
 static int execute(void)
 {
-	size_t index;
+	efi_uintn_t index;
 	efi_status_t ret;
 
 	/* Set 10 ms timer */
diff --git a/lib/efi_selftest/efi_selftest_manageprotocols.c b/lib/efi_selftest/efi_selftest_manageprotocols.c
index 0a297f79ba..a701090ce5 100644
--- a/lib/efi_selftest/efi_selftest_manageprotocols.c
+++ b/lib/efi_selftest/efi_selftest_manageprotocols.c
@@ -165,9 +165,9 @@ static int execute(void)
 	efi_status_t ret;
 	efi_handle_t *buffer;
 	size_t buffer_size;
-	unsigned long int count = 0;
+	efi_uintn_t count = 0;
 	efi_guid_t **prot_buffer;
-	unsigned long int prot_count;
+	efi_uintn_t prot_count;
 
 	/*
 	 * Test HandleProtocol
diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c
index bdd6ce20da..cc0705fb59 100644
--- a/lib/efi_selftest/efi_selftest_snp.c
+++ b/lib/efi_selftest/efi_selftest_snp.c
@@ -260,7 +260,7 @@ static int execute(void)
 {
 	efi_status_t ret;
 	struct efi_event *events[2];
-	size_t index;
+	efi_uintn_t index;
 	union {
 		struct dhcp p;
 		u8 b[PKTSIZE];
diff --git a/lib/efi_selftest/efi_selftest_tpl.c b/lib/efi_selftest/efi_selftest_tpl.c
index 15d62903c0..6ea0bb7177 100644
--- a/lib/efi_selftest/efi_selftest_tpl.c
+++ b/lib/efi_selftest/efi_selftest_tpl.c
@@ -111,7 +111,7 @@ static int teardown(void)
  */
 static int execute(void)
 {
-	size_t index;
+	efi_uintn_t index;
 	efi_status_t ret;
 	efi_uintn_t old_tpl;
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 8/9] efi_loader: rework efi_locate_handle
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
                   ` (6 preceding siblings ...)
  2017-10-22 12:45 ` [U-Boot] [PATCH 7/9] efi_loader: consistently use efi_uintn_t in boot services Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-07  5:21   ` Simon Glass
  2017-10-22 12:45 ` [U-Boot] [PATCH 9/9] efi_loader: rework efi_search_obj Heinrich Schuchardt
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

Check the parameters in efi_locate_handle.

Use list_for_each_entry instead of list_for_each.

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 818d54abd1..b6eebe3b14 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -896,6 +896,7 @@ static int efi_search(enum efi_locate_search_type search_type,
 	case ALL_HANDLES:
 		return 0;
 	case BY_REGISTER_NOTIFY:
+		/* RegisterProtocolNotify is not implemented yet */
 		return -1;
 	case BY_PROTOCOL:
 		for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
@@ -927,16 +928,38 @@ static efi_status_t efi_locate_handle(
 			const efi_guid_t *protocol, void *search_key,
 			efi_uintn_t *buffer_size, efi_handle_t *buffer)
 {
-	struct list_head *lhandle;
+	struct efi_object *efiobj;
 	efi_uintn_t size = 0;
 
+	/* Check parameters */
+	switch (search_type) {
+	case ALL_HANDLES:
+		break;
+	case BY_REGISTER_NOTIFY:
+		if (!search_key)
+			return EFI_INVALID_PARAMETER;
+		/* RegisterProtocolNotify is not implemented yet */
+		return EFI_UNSUPPORTED;
+	case BY_PROTOCOL:
+		if (!protocol)
+			return EFI_INVALID_PARAMETER;
+		break;
+	default:
+		return EFI_INVALID_PARAMETER;
+	}
+
+	/*
+	 * efi_locate_handle_buffer uses this function for
+	 * the calculation of the necessary buffer size.
+	 * So do not require a buffer for buffersize == 0.
+	 */
+	if (!buffer_size || (*buffer_size && !buffer))
+		return EFI_INVALID_PARAMETER;
+
 	/* Count how much space we need */
-	list_for_each(lhandle, &efi_obj_list) {
-		struct efi_object *efiobj;
-		efiobj = list_entry(lhandle, struct efi_object, link);
-		if (!efi_search(search_type, protocol, search_key, efiobj)) {
+	list_for_each_entry(efiobj, &efi_obj_list, link) {
+		if (!efi_search(search_type, protocol, search_key, efiobj))
 			size += sizeof(void*);
-		}
 	}
 
 	if (*buffer_size < size) {
@@ -949,12 +972,9 @@ static efi_status_t efi_locate_handle(
 		return EFI_NOT_FOUND;
 
 	/* Then fill the array */
-	list_for_each(lhandle, &efi_obj_list) {
-		struct efi_object *efiobj;
-		efiobj = list_entry(lhandle, struct efi_object, link);
-		if (!efi_search(search_type, protocol, search_key, efiobj)) {
-			*(buffer++) = efiobj->handle;
-		}
+	list_for_each_entry(efiobj, &efi_obj_list, link) {
+		if (!efi_search(search_type, protocol, search_key, efiobj))
+			*buffer++ = efiobj->handle;
 	}
 
 	return EFI_SUCCESS;
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 9/9] efi_loader: rework efi_search_obj
  2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
                   ` (7 preceding siblings ...)
  2017-10-22 12:45 ` [U-Boot] [PATCH 8/9] efi_loader: rework efi_locate_handle Heinrich Schuchardt
@ 2017-10-22 12:45 ` Heinrich Schuchardt
  2017-11-07  5:21   ` Simon Glass
  8 siblings, 1 reply; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-10-22 12:45 UTC (permalink / raw)
  To: u-boot

EFI_HANDLEs are used both in boottime and in runtime services.
efi_search_obj is a function that can be used to validate
handles. So let's make it accessible via efi_loader.h.

We can simplify the coding using list_for_each_entry.

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

diff --git a/include/efi_loader.h b/include/efi_loader.h
index e3d1c35930..2bcca3dfd8 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -192,6 +192,8 @@ void efi_restore_gd(void);
 void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
 /* Call this to set the current device name */
 void efi_set_bootdev(const char *dev, const char *devnr, const char *path);
+/* Call this to validate a handle and find the EFI object for it */
+struct efi_object *efi_search_obj(void *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 b6eebe3b14..bdbed32656 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -684,14 +684,11 @@ static efi_status_t EFIAPI efi_check_event(struct efi_event *event)
  * @handle	handle to find
  * @return	EFI object
  */
-static struct efi_object *efi_search_obj(void *handle)
+struct efi_object *efi_search_obj(void *handle)
 {
-	struct list_head *lhandle;
-
-	list_for_each(lhandle, &efi_obj_list) {
-		struct efi_object *efiobj;
+	struct efi_object *efiobj;
 
-		efiobj = list_entry(lhandle, struct efi_object, link);
+	list_for_each_entry(efiobj, &efi_obj_list, link) {
 		if (efiobj->handle == handle)
 			return efiobj;
 	}
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 1/9] efi_loader: capitalize EFI_LOCATE_SEARCH_TYPE values
  2017-10-22 12:45 ` [U-Boot] [PATCH 1/9] efi_loader: capitalize EFI_LOCATE_SEARCH_TYPE values Heinrich Schuchardt
@ 2017-11-06 16:58   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2017-11-06 16:58 UTC (permalink / raw)
  To: u-boot

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Constants should be capitalized.
> So rename the values of enum efi_locate_search_type.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi.h                 | 6 +++---
>  lib/efi_loader/efi_boottime.c | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 2/9] efi_selftest: test protocol management
  2017-10-22 12:45 ` [U-Boot] [PATCH 2/9] efi_selftest: test protocol management Heinrich Schuchardt
@ 2017-11-06 16:58   ` Simon Glass
  2017-11-06 19:49     ` Heinrich Schuchardt
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2017-11-06 16:58 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> This unit test checks the following protocol services:
> InstallProtocolInterface, UninstallProtocolInterface,
> InstallMultipleProtocolsInterfaces,
> UninstallMultipleProtocolsInterfaces,
> HandleProtocol, ProtocolsPerHandle,
> LocateHandle, LocateHandleBuffer.
>
> As UninstallProtocolInterface and UninstallMultipleProtocolsInterfaces
> are not completely implemented a TODO message will shown for
> their failure.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_selftest.h                          |   9 +
>  lib/efi_selftest/Makefile                       |   3 +
>  lib/efi_selftest/efi_selftest_manageprotocols.c | 381 ++++++++++++++++++++++++
>  3 files changed, 393 insertions(+)
>  create mode 100644 lib/efi_selftest/efi_selftest_manageprotocols.c
>

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

nits below

> diff --git a/include/efi_selftest.h b/include/efi_selftest.h
> index 5cc8d4f600..be5ba4bfa9 100644
> --- a/include/efi_selftest.h
> +++ b/include/efi_selftest.h
> @@ -27,6 +27,15 @@
>         (efi_st_printf("%s(%u):\nERROR: ", __FILE__, __LINE__), \
>         efi_st_printf(__VA_ARGS__)) \
>
> +/*
> + * Prints a TODO message.

Why is this called a TODO message rather than an error?

> + *
> + * @...        format string followed by fields to print
> + */
> +#define efi_st_todo(...) \
> +       (efi_st_printf("%s(%u):\nTODO: ", __FILE__, __LINE__), \
> +       efi_st_printf(__VA_ARGS__)) \
> +
>  /*
>   * A test may be setup and executed at boottime,
>   * it may be setup at boottime and executed at runtime,
> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> index 88b998bd4c..acd184db4b 100644
> --- a/lib/efi_selftest/Makefile
> +++ b/lib/efi_selftest/Makefile
> @@ -15,6 +15,8 @@ CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)
>  CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)
>  CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI)
>  CFLAGS_REMOVE_efi_selftest_exitbootservices.o := $(CFLAGS_NON_EFI)
> +CFLAGS_efi_selftest_manageprotocols.o := $(CFLAGS_EFI)
> +CFLAGS_REMOVE_efi_selftest_manageprotocols.o := $(CFLAGS_NON_EFI)
>  CFLAGS_efi_selftest_snp.o := $(CFLAGS_EFI)
>  CFLAGS_REMOVE_efi_selftest_snp.o := $(CFLAGS_NON_EFI)
>  CFLAGS_efi_selftest_textoutput.o := $(CFLAGS_EFI)
> @@ -31,6 +33,7 @@ efi_selftest.o \
>  efi_selftest_console.o \
>  efi_selftest_events.o \
>  efi_selftest_exitbootservices.o \
> +efi_selftest_manageprotocols.o \
>  efi_selftest_snp.o \
>  efi_selftest_textoutput.o \
>  efi_selftest_tpl.o \
> diff --git a/lib/efi_selftest/efi_selftest_manageprotocols.c b/lib/efi_selftest/efi_selftest_manageprotocols.c
> new file mode 100644
> index 0000000000..0a297f79ba
> --- /dev/null
> +++ b/lib/efi_selftest/efi_selftest_manageprotocols.c
> @@ -0,0 +1,381 @@
> +/*
> + * efi_selftest_manageprotocols
> + *
> + * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + *
> + * This unit test checks the following protocol services:
> + * InstallProtocolInterface, UninstallProtocolInterface,
> + * InstallMultipleProtocolsInterfaces, UninstallMultipleProtocolsInterfaces,
> + * HandleProtocol, ProtocolsPerHandle,
> + * LocateHandle, LocateHandleBuffer.
> + */
> +
> +#include <efi_selftest.h>
> +
> +static struct efi_boot_services *boottime;
> +
> +static efi_handle_t handle1;
> +static efi_handle_t handle2;
> +
> +struct interface {
> +       void (EFIAPI * inc)(void);
> +};
> +
> +static efi_guid_t guid1 =
> +       EFI_GUID(0x2e7ca819, 0x21d3, 0x0a3a,
> +                0xf7, 0x91, 0x82, 0x1f, 0x7a, 0x83, 0x67, 0xaf);
> +static unsigned int counter1;

blank line before function

> +void EFIAPI inc1(void)
> +{
> +       ++counter1;

This is just checking that the function is called, right? I suggest
adding a comment explaining that you are incrementing so the test can
detect success.

> +}
> +
> +static struct interface interface1 = {
> +       inc1,
> +};
> +
> +static struct interface interface4 = {
> +       inc1,
> +};
> +
> +static efi_guid_t guid2 =
> +       EFI_GUID(0xf909f2bb, 0x90a8, 0x0d77,
> +                0x94, 0x0c, 0x3e, 0xa8, 0xea, 0x38, 0xd6, 0x6f);

blank line here

> +static unsigned int counter2;
> +void EFIAPI inc2(void)
> +{
> +       ++counter2;
> +}
> +
> +static struct interface interface2 = {
> +       inc2,
> +};
> +
> +static efi_guid_t guid3 =
> +       EFI_GUID(0x06d641a3, 0xf4e7, 0xe0c9,
> +                0xe7, 0x8d, 0x41, 0x2d, 0x72, 0xa6, 0xb1, 0x24);
> +static unsigned int counter3;

blank line here

> +void EFIAPI inc3(void)
> +{
> +       ++counter3;
> +}
> +
> +static struct interface interface3 = {
> +       inc3,
> +};
> +
> +/*
> + * Find a handle in an array.
> + *
> + * @handle:    handle to find
> + * @count:     number of entries in the array
> + * @buffer:    array to search

@return

Please check in rest of file too.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 3/9] efi_loader: eliminate efi_install_protocol_interface_ext
  2017-10-22 12:45 ` [U-Boot] [PATCH 3/9] efi_loader: eliminate efi_install_protocol_interface_ext Heinrich Schuchardt
@ 2017-11-06 16:58   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2017-11-06 16:58 UTC (permalink / raw)
  To: u-boot

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> As we now have EFI_CALL there is no need for separate
> functions efi_install_protocol_interface_ext and
> efi_install_protocol_interface.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 52 +++++++++++++------------------------------
>  1 file changed, 15 insertions(+), 37 deletions(-)

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 4/9] efi_loader: eliminate efi_uninstall_protocol_interface_ext
  2017-10-22 12:45 ` [U-Boot] [PATCH 4/9] efi_loader: eliminate efi_uninstall_protocol_interface_ext Heinrich Schuchardt
@ 2017-11-06 16:59   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2017-11-06 16:59 UTC (permalink / raw)
  To: u-boot

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> As we now have EFI_CALL there is no need for separate
> functions efi_uninstall_protocol_interface_ext and
> efi_uninstall_protocol_interface.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 42 ++++++++++++------------------------------
>  1 file changed, 12 insertions(+), 30 deletions(-)

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 5/9] efi_loader: remove unused typedef for INTN
  2017-10-22 12:45 ` [U-Boot] [PATCH 5/9] efi_loader: remove unused typedef for INTN Heinrich Schuchardt
@ 2017-11-06 17:02   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2017-11-06 17:02 UTC (permalink / raw)
  To: u-boot

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> INTN is not used in the coding.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_api.h | 1 -
>  1 file changed, 1 deletion(-)

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 6/9] efi_loader: replace UINTN by efi_uintn_t
  2017-10-22 12:45 ` [U-Boot] [PATCH 6/9] efi_loader: replace UINTN by efi_uintn_t Heinrich Schuchardt
@ 2017-11-06 17:02   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2017-11-06 17:02 UTC (permalink / raw)
  To: u-boot

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> UINTN is used in the UEFI specification for unsigned integers
> matching the bitness of the CPU.
>
> Types in U-Boot should be lower case. The patch replaces it
> by efi_uintn_t.
>
> Suggested-by: Simon Glass <sjg@chromium.org>
> Suggested-by: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_api.h                   |  8 ++++----
>  include/efi_loader.h                |  4 ++--
>  lib/efi_loader/efi_boottime.c       | 12 ++++++------
>  lib/efi_selftest/efi_selftest_tpl.c |  2 +-
>  4 files changed, 13 insertions(+), 13 deletions(-)

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 2/9] efi_selftest: test protocol management
  2017-11-06 16:58   ` Simon Glass
@ 2017-11-06 19:49     ` Heinrich Schuchardt
  0 siblings, 0 replies; 20+ messages in thread
From: Heinrich Schuchardt @ 2017-11-06 19:49 UTC (permalink / raw)
  To: u-boot



On 11/06/2017 05:58 PM, Simon Glass wrote:
> Hi Heinrich,
> 
> On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> This unit test checks the following protocol services:
>> InstallProtocolInterface, UninstallProtocolInterface,
>> InstallMultipleProtocolsInterfaces,
>> UninstallMultipleProtocolsInterfaces,
>> HandleProtocol, ProtocolsPerHandle,
>> LocateHandle, LocateHandleBuffer.
>>
>> As UninstallProtocolInterface and UninstallMultipleProtocolsInterfaces
>> are not completely implemented a TODO message will shown for
>> their failure.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   include/efi_selftest.h                          |   9 +
>>   lib/efi_selftest/Makefile                       |   3 +
>>   lib/efi_selftest/efi_selftest_manageprotocols.c | 381 ++++++++++++++++++++++++
>>   3 files changed, 393 insertions(+)
>>   create mode 100644 lib/efi_selftest/efi_selftest_manageprotocols.c
>>
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> nits below
> 
>> diff --git a/include/efi_selftest.h b/include/efi_selftest.h
>> index 5cc8d4f600..be5ba4bfa9 100644
>> --- a/include/efi_selftest.h
>> +++ b/include/efi_selftest.h
>> @@ -27,6 +27,15 @@
>>          (efi_st_printf("%s(%u):\nERROR: ", __FILE__, __LINE__), \
>>          efi_st_printf(__VA_ARGS__)) \
>>
>> +/*
>> + * Prints a TODO message.
> 
> Why is this called a TODO message rather than an error?

If some part of the EFI spec is not yet inspected this is not an error 
but a TODO. Furthermore In this case I will not cause the Travis CI test 
to fail.

Best regards

Heinrich

> 
>> + *
>> + * @...        format string followed by fields to print
>> + */
>> +#define efi_st_todo(...) \
>> +       (efi_st_printf("%s(%u):\nTODO: ", __FILE__, __LINE__), \
>> +       efi_st_printf(__VA_ARGS__)) \
>> +
>>   /*
>>    * A test may be setup and executed at boottime,
>>    * it may be setup at boottime and executed at runtime,
>> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
>> index 88b998bd4c..acd184db4b 100644
>> --- a/lib/efi_selftest/Makefile
>> +++ b/lib/efi_selftest/Makefile
>> @@ -15,6 +15,8 @@ CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)
>>   CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)
>>   CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI)
>>   CFLAGS_REMOVE_efi_selftest_exitbootservices.o := $(CFLAGS_NON_EFI)
>> +CFLAGS_efi_selftest_manageprotocols.o := $(CFLAGS_EFI)
>> +CFLAGS_REMOVE_efi_selftest_manageprotocols.o := $(CFLAGS_NON_EFI)
>>   CFLAGS_efi_selftest_snp.o := $(CFLAGS_EFI)
>>   CFLAGS_REMOVE_efi_selftest_snp.o := $(CFLAGS_NON_EFI)
>>   CFLAGS_efi_selftest_textoutput.o := $(CFLAGS_EFI)
>> @@ -31,6 +33,7 @@ efi_selftest.o \
>>   efi_selftest_console.o \
>>   efi_selftest_events.o \
>>   efi_selftest_exitbootservices.o \
>> +efi_selftest_manageprotocols.o \
>>   efi_selftest_snp.o \
>>   efi_selftest_textoutput.o \
>>   efi_selftest_tpl.o \
>> diff --git a/lib/efi_selftest/efi_selftest_manageprotocols.c b/lib/efi_selftest/efi_selftest_manageprotocols.c
>> new file mode 100644
>> index 0000000000..0a297f79ba
>> --- /dev/null
>> +++ b/lib/efi_selftest/efi_selftest_manageprotocols.c
>> @@ -0,0 +1,381 @@
>> +/*
>> + * efi_selftest_manageprotocols
>> + *
>> + * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + *
>> + * This unit test checks the following protocol services:
>> + * InstallProtocolInterface, UninstallProtocolInterface,
>> + * InstallMultipleProtocolsInterfaces, UninstallMultipleProtocolsInterfaces,
>> + * HandleProtocol, ProtocolsPerHandle,
>> + * LocateHandle, LocateHandleBuffer.
>> + */
>> +
>> +#include <efi_selftest.h>
>> +
>> +static struct efi_boot_services *boottime;
>> +
>> +static efi_handle_t handle1;
>> +static efi_handle_t handle2;
>> +
>> +struct interface {
>> +       void (EFIAPI * inc)(void);
>> +};
>> +
>> +static efi_guid_t guid1 =
>> +       EFI_GUID(0x2e7ca819, 0x21d3, 0x0a3a,
>> +                0xf7, 0x91, 0x82, 0x1f, 0x7a, 0x83, 0x67, 0xaf);
>> +static unsigned int counter1;
> 
> blank line before function
> 
>> +void EFIAPI inc1(void)
>> +{
>> +       ++counter1;
> 
> This is just checking that the function is called, right? I suggest
> adding a comment explaining that you are incrementing so the test can
> detect success.
> 
>> +}
>> +
>> +static struct interface interface1 = {
>> +       inc1,
>> +};
>> +
>> +static struct interface interface4 = {
>> +       inc1,
>> +};
>> +
>> +static efi_guid_t guid2 =
>> +       EFI_GUID(0xf909f2bb, 0x90a8, 0x0d77,
>> +                0x94, 0x0c, 0x3e, 0xa8, 0xea, 0x38, 0xd6, 0x6f);
> 
> blank line here
> 
>> +static unsigned int counter2;
>> +void EFIAPI inc2(void)
>> +{
>> +       ++counter2;
>> +}
>> +
>> +static struct interface interface2 = {
>> +       inc2,
>> +};
>> +
>> +static efi_guid_t guid3 =
>> +       EFI_GUID(0x06d641a3, 0xf4e7, 0xe0c9,
>> +                0xe7, 0x8d, 0x41, 0x2d, 0x72, 0xa6, 0xb1, 0x24);
>> +static unsigned int counter3;
> 
> blank line here
> 
>> +void EFIAPI inc3(void)
>> +{
>> +       ++counter3;
>> +}
>> +
>> +static struct interface interface3 = {
>> +       inc3,
>> +};
>> +
>> +/*
>> + * Find a handle in an array.
>> + *
>> + * @handle:    handle to find
>> + * @count:     number of entries in the array
>> + * @buffer:    array to search
> 
> @return
> 
> Please check in rest of file too.
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 7/9] efi_loader: consistently use efi_uintn_t in boot services
  2017-10-22 12:45 ` [U-Boot] [PATCH 7/9] efi_loader: consistently use efi_uintn_t in boot services Heinrich Schuchardt
@ 2017-11-07  5:21   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2017-11-07  5:21 UTC (permalink / raw)
  To: u-boot

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Consistenly use efi_uintn_t wherever the UEFI spec uses
> UINTN in boot services interfaces.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_api.h                               | 29 ++++++++++---------
>  include/efi_loader.h                            | 12 ++++----
>  lib/efi_loader/efi_boottime.c                   | 38 ++++++++++++-------------
>  lib/efi_loader/efi_memory.c                     | 20 ++++++-------
>  lib/efi_selftest/efi_selftest.c                 |  6 ++--
>  lib/efi_selftest/efi_selftest_events.c          |  2 +-
>  lib/efi_selftest/efi_selftest_manageprotocols.c |  4 +--
>  lib/efi_selftest/efi_selftest_snp.c             |  2 +-
>  lib/efi_selftest/efi_selftest_tpl.c             |  2 +-
>  9 files changed, 59 insertions(+), 56 deletions(-)
>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 8/9] efi_loader: rework efi_locate_handle
  2017-10-22 12:45 ` [U-Boot] [PATCH 8/9] efi_loader: rework efi_locate_handle Heinrich Schuchardt
@ 2017-11-07  5:21   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2017-11-07  5:21 UTC (permalink / raw)
  To: u-boot

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Check the parameters in efi_locate_handle.
>
> Use list_for_each_entry instead of list_for_each.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 44 +++++++++++++++++++++++++++++++------------
>  1 file changed, 32 insertions(+), 12 deletions(-)

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH 9/9] efi_loader: rework efi_search_obj
  2017-10-22 12:45 ` [U-Boot] [PATCH 9/9] efi_loader: rework efi_search_obj Heinrich Schuchardt
@ 2017-11-07  5:21   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2017-11-07  5:21 UTC (permalink / raw)
  To: u-boot

On 22 October 2017 at 06:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> EFI_HANDLEs are used both in boottime and in runtime services.
> efi_search_obj is a function that can be used to validate
> handles. So let's make it accessible via efi_loader.h.
>
> We can simplify the coding using list_for_each_entry.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  include/efi_loader.h          | 2 ++
>  lib/efi_loader/efi_boottime.c | 9 +++------
>  2 files changed, 5 insertions(+), 6 deletions(-)

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-11-07  5:21 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-22 12:44 [U-Boot] [PATCH 0/9] efi_loader: clean up protocol services Heinrich Schuchardt
2017-10-22 12:45 ` [U-Boot] [PATCH 1/9] efi_loader: capitalize EFI_LOCATE_SEARCH_TYPE values Heinrich Schuchardt
2017-11-06 16:58   ` Simon Glass
2017-10-22 12:45 ` [U-Boot] [PATCH 2/9] efi_selftest: test protocol management Heinrich Schuchardt
2017-11-06 16:58   ` Simon Glass
2017-11-06 19:49     ` Heinrich Schuchardt
2017-10-22 12:45 ` [U-Boot] [PATCH 3/9] efi_loader: eliminate efi_install_protocol_interface_ext Heinrich Schuchardt
2017-11-06 16:58   ` Simon Glass
2017-10-22 12:45 ` [U-Boot] [PATCH 4/9] efi_loader: eliminate efi_uninstall_protocol_interface_ext Heinrich Schuchardt
2017-11-06 16:59   ` Simon Glass
2017-10-22 12:45 ` [U-Boot] [PATCH 5/9] efi_loader: remove unused typedef for INTN Heinrich Schuchardt
2017-11-06 17:02   ` Simon Glass
2017-10-22 12:45 ` [U-Boot] [PATCH 6/9] efi_loader: replace UINTN by efi_uintn_t Heinrich Schuchardt
2017-11-06 17:02   ` Simon Glass
2017-10-22 12:45 ` [U-Boot] [PATCH 7/9] efi_loader: consistently use efi_uintn_t in boot services Heinrich Schuchardt
2017-11-07  5:21   ` Simon Glass
2017-10-22 12:45 ` [U-Boot] [PATCH 8/9] efi_loader: rework efi_locate_handle Heinrich Schuchardt
2017-11-07  5:21   ` Simon Glass
2017-10-22 12:45 ` [U-Boot] [PATCH 9/9] efi_loader: rework efi_search_obj Heinrich Schuchardt
2017-11-07  5:21   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.