[U-Boot] [PATCH v2 00/17] efi_loader: Simple Network Protocol

[U-Boot] [PATCH v2 00/17] efi_loader: Simple Network Protocol

[Heinrich Schuchardt]

This patch series focuses on
* correction of parameter types for boot services
* fixes for the SetMem and CopyMem boot services
* fixes for the simple network protocol implementation (SNP)
* a unit test for SetMem, CopyMem and the simple network protocol

The unit test broadcasts a DHCPDISCOVER messager over the network and
receives the reply.

This patch series is based on
* efi-next tree
  https://github.com/agraf/u-boot/tree/efi-next
* [PATCH 1/1] efi_selftest: enable CONFIG_CMD_BOOTEFI_SELFTEST
  https://patchwork.ozlabs.org/patch/816412/
  This patch enables the unit test on qemu-x86_defconfig
* [PATCH 1/1] efi_loader: provide function comments for boot services
  https://patchwork.ozlabs.org/patch/817010/

---
v2
	Move efi_st_memcmp to a new module. We can reuse it in future
	tests.
	Use constants as return values.
	Reflect renaming of signaled to is_signaled for events.
---
Heinrich Schuchardt (17):
  efi_loader: call EFI_EXIT in efi_copy_mem, efi_set_mem
  efi_loader: parameters of CopyMem and SetMem
  efi_loader: pass GUIDs as const efi_guid_t *
  efi_loader: wrong type in wait_for_event
  efi_loader: incorrect definition of EFI_SIMPLE_NETWORK_PROTOCOL
  efi_loader: correct bits of receive_filters bit mask
  efi_loader: fill simple network protocol revision
  efi_loader: efi_net: hwaddr_size = 6
  efi_net: return EFI_UNSUPPORTED where appropriate
  efi_loader: use events for efi_net_receive
  efi_loader: implement WaitForPacket event
  efi_loader: fix efi_net_get_status
  efi_loader: size fields in SimpleNetworkProtocol
  efi_loader: fill return values in SimpleNetworkProtocol
  efi_selftest: correct definition of efi_st_error
  efi_selftest: allow printing MAC addresses
  efi_loader: supply EFI network test

 include/efi_api.h                       |  62 +++--
 include/efi_selftest.h                  |  15 +-
 lib/efi_loader/efi_boottime.c           |  54 ++--
 lib/efi_loader/efi_net.c                | 144 +++++++++--
 lib/efi_selftest/Makefile               |   8 +-
 lib/efi_selftest/efi_selftest_console.c |  41 ++-
 lib/efi_selftest/efi_selftest_events.c  |   2 +-
 lib/efi_selftest/efi_selftest_snp.c     | 424 ++++++++++++++++++++++++++++++++
 lib/efi_selftest/efi_selftest_tpl.c     |   2 +-
 lib/efi_selftest/efi_selftest_util.c    |  25 ++
 10 files changed, 702 insertions(+), 75 deletions(-)
 create mode 100644 lib/efi_selftest/efi_selftest_snp.c
 create mode 100644 lib/efi_selftest/efi_selftest_util.c

-- 
2.14.1

[U-Boot] [PATCH v2 01/17] efi_loader: call EFI_EXIT in efi_copy_mem, efi_set_mem

[Heinrich Schuchardt]

EFI_ENTRY and EFI_EXIT calls must match.

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

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 66ce92f654..b8b98f2c4a 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1868,6 +1868,7 @@ static void EFIAPI efi_copy_mem(void *destination, void *source,
 {
 	EFI_ENTRY("%p, %p, %ld", destination, source, length);
 	memcpy(destination, source, length);
+	EFI_EXIT(EFI_SUCCESS);
 }
 
 /*
@@ -1885,6 +1886,7 @@ static void EFIAPI efi_set_mem(void *buffer, unsigned long size, uint8_t value)
 {
 	EFI_ENTRY("%p, %ld, 0x%x", buffer, size, value);
 	memset(buffer, value, size);
+	EFI_EXIT(EFI_SUCCESS);
 }
 
 /*
-- 
2.14.1

[U-Boot] [PATCH v2 02/17] efi_loader: parameters of CopyMem and SetMem

[Heinrich Schuchardt]

The UEFI spec defines the length parameters of CopyMem and SetMem
as UINTN. We should size_t here.

The source buffer of CopyMem should be marked as const.

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

diff --git a/include/efi_api.h b/include/efi_api.h
index c3b9032a48..0b1a383e61 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -156,10 +156,9 @@ struct efi_boot_services {
 			void *handle, ...);
 	efi_status_t (EFIAPI *calculate_crc32)(void *data,
 			unsigned long data_size, uint32_t *crc32);
-	void (EFIAPI *copy_mem)(void *destination, void *source,
-			unsigned long length);
-	void (EFIAPI *set_mem)(void *buffer, unsigned long size,
-			uint8_t value);
+	void (EFIAPI *copy_mem)(void *destination, const void *source,
+			size_t length);
+	void (EFIAPI *set_mem)(void *buffer, size_t size, uint8_t value);
 	void *create_event_ex;
 };
 
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index b8b98f2c4a..c48ff2cd2a 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1863,10 +1863,10 @@ static efi_status_t EFIAPI efi_calculate_crc32(void *data,
  * @source		source of the copy operation
  * @length		number of bytes to copy
  */
-static void EFIAPI efi_copy_mem(void *destination, void *source,
-				unsigned long length)
+static void EFIAPI efi_copy_mem(void *destination, const void *source,
+				size_t length)
 {
-	EFI_ENTRY("%p, %p, %ld", destination, source, length);
+	EFI_ENTRY("%p, %p, %ld", destination, source, (unsigned long)length);
 	memcpy(destination, source, length);
 	EFI_EXIT(EFI_SUCCESS);
 }
@@ -1882,9 +1882,9 @@ static void EFIAPI efi_copy_mem(void *destination, void *source,
  * @size		size of buffer in bytes
  * @value		byte to copy to the buffer
  */
-static void EFIAPI efi_set_mem(void *buffer, unsigned long size, uint8_t value)
+static void EFIAPI efi_set_mem(void *buffer, size_t size, uint8_t value)
 {
-	EFI_ENTRY("%p, %ld, 0x%x", buffer, size, value);
+	EFI_ENTRY("%p, %ld, 0x%x", buffer, (unsigned long)size, value);
 	memset(buffer, value, size);
 	EFI_EXIT(EFI_SUCCESS);
 }
-- 
2.14.1

[U-Boot] [PATCH v2 03/17] efi_loader: pass GUIDs as const efi_guid_t *

[Heinrich Schuchardt]

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

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

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 include/efi_api.h             | 27 ++++++++++++++-------------
 lib/efi_loader/efi_boottime.c | 40 +++++++++++++++++++++-------------------
 2 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 0b1a383e61..aa4306aac9 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -77,24 +77,25 @@ struct efi_boot_services {
 	efi_status_t (EFIAPI *check_event)(struct efi_event *event);
 #define EFI_NATIVE_INTERFACE	0x00000000
 	efi_status_t (EFIAPI *install_protocol_interface)(
-			void **handle, efi_guid_t *protocol,
+			void **handle, const efi_guid_t *protocol,
 			int protocol_interface_type, void *protocol_interface);
 	efi_status_t (EFIAPI *reinstall_protocol_interface)(
-			void *handle, efi_guid_t *protocol,
+			void *handle, const efi_guid_t *protocol,
 			void *old_interface, void *new_interface);
 	efi_status_t (EFIAPI *uninstall_protocol_interface)(void *handle,
-			efi_guid_t *protocol, void *protocol_interface);
-	efi_status_t (EFIAPI *handle_protocol)(efi_handle_t, efi_guid_t *,
-					       void **);
+			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);
 	void *reserved;
 	efi_status_t (EFIAPI *register_protocol_notify)(
-			efi_guid_t *protocol, struct efi_event *event,
+			const efi_guid_t *protocol, struct efi_event *event,
 			void **registration);
 	efi_status_t (EFIAPI *locate_handle)(
 			enum efi_locate_search_type search_type,
-			efi_guid_t *protocol, void *search_key,
+			const efi_guid_t *protocol, void *search_key,
 			unsigned long *buffer_size, efi_handle_t *buffer);
-	efi_status_t (EFIAPI *locate_device_path)(efi_guid_t *protocol,
+	efi_status_t (EFIAPI *locate_device_path)(const efi_guid_t *protocol,
 			struct efi_device_path **device_path,
 			efi_handle_t *device);
 	efi_status_t (EFIAPI *install_configuration_table)(
@@ -131,14 +132,14 @@ struct efi_boot_services {
 #define EFI_OPEN_PROTOCOL_BY_DRIVER           0x00000010
 #define EFI_OPEN_PROTOCOL_EXCLUSIVE           0x00000020
 	efi_status_t (EFIAPI *open_protocol)(efi_handle_t handle,
-			efi_guid_t *protocol, void **interface,
+			const efi_guid_t *protocol, void **interface,
 			efi_handle_t agent_handle,
 			efi_handle_t controller_handle, u32 attributes);
 	efi_status_t (EFIAPI *close_protocol)(void *handle,
-			efi_guid_t *protocol, void *agent_handle,
+			const efi_guid_t *protocol, void *agent_handle,
 			void *controller_handle);
 	efi_status_t(EFIAPI *open_protocol_information)(efi_handle_t handle,
-			efi_guid_t *protocol,
+			const efi_guid_t *protocol,
 			struct efi_open_protocol_info_entry **entry_buffer,
 			unsigned long *entry_count);
 	efi_status_t (EFIAPI *protocols_per_handle)(efi_handle_t handle,
@@ -146,9 +147,9 @@ struct efi_boot_services {
 			unsigned long *protocols_buffer_count);
 	efi_status_t (EFIAPI *locate_handle_buffer) (
 			enum efi_locate_search_type search_type,
-			efi_guid_t *protocol, void *search_key,
+			const efi_guid_t *protocol, void *search_key,
 			unsigned long *no_handles, efi_handle_t **buffer);
-	efi_status_t (EFIAPI *locate_protocol)(efi_guid_t *protocol,
+	efi_status_t (EFIAPI *locate_protocol)(const efi_guid_t *protocol,
 			void *registration, void **protocol_interface);
 	efi_status_t (EFIAPI *install_multiple_protocol_interfaces)(
 			void **handle, ...);
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index c48ff2cd2a..e5adc17fab 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -704,7 +704,7 @@ static efi_status_t EFIAPI efi_check_event(struct efi_event *event)
  * @return			status code
  */
 static efi_status_t EFIAPI efi_install_protocol_interface(void **handle,
-			efi_guid_t *protocol, int protocol_interface_type,
+			const efi_guid_t *protocol, int protocol_interface_type,
 			void *protocol_interface)
 {
 	struct list_head *lhandle;
@@ -776,7 +776,7 @@ out:
  * @return			status code
  */
 static efi_status_t EFIAPI efi_install_protocol_interface_ext(void **handle,
-			efi_guid_t *protocol, int protocol_interface_type,
+			const efi_guid_t *protocol, int protocol_interface_type,
 			void *protocol_interface)
 {
 	EFI_ENTRY("%p, %pUl, %d, %p", handle, protocol, protocol_interface_type,
@@ -802,7 +802,7 @@ static efi_status_t EFIAPI efi_install_protocol_interface_ext(void **handle,
  * @return			status code
  */
 static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle,
-			efi_guid_t *protocol, void *old_interface,
+			const efi_guid_t *protocol, void *old_interface,
 			void *new_interface)
 {
 	EFI_ENTRY("%p, %pUl, %p, %p", handle, protocol, old_interface,
@@ -823,7 +823,7 @@ static efi_status_t EFIAPI efi_reinstall_protocol_interface(void *handle,
  * @return			status code
  */
 static efi_status_t EFIAPI efi_uninstall_protocol_interface(void *handle,
-			efi_guid_t *protocol, void *protocol_interface)
+			const efi_guid_t *protocol, void *protocol_interface)
 {
 	struct list_head *lhandle;
 	int i;
@@ -876,7 +876,7 @@ out:
  * @return			status code
  */
 static efi_status_t EFIAPI efi_uninstall_protocol_interface_ext(void *handle,
-			efi_guid_t *protocol, void *protocol_interface)
+			const efi_guid_t *protocol, void *protocol_interface)
 {
 	EFI_ENTRY("%p, %pUl, %p", handle, protocol, protocol_interface);
 
@@ -897,9 +897,10 @@ static efi_status_t EFIAPI efi_uninstall_protocol_interface_ext(void *handle,
  * @registration	key for retrieving the registration information
  * @return		status code
  */
-static efi_status_t EFIAPI efi_register_protocol_notify(efi_guid_t *protocol,
-							struct efi_event *event,
-							void **registration)
+static efi_status_t EFIAPI efi_register_protocol_notify(
+						const efi_guid_t *protocol,
+						struct efi_event *event,
+						void **registration)
 {
 	EFI_ENTRY("%pUl, %p, %p", protocol, event, registration);
 	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
@@ -917,7 +918,7 @@ static efi_status_t EFIAPI efi_register_protocol_notify(efi_guid_t *protocol,
  * @return		0 if the handle implements the protocol
  */
 static int efi_search(enum efi_locate_search_type search_type,
-		      efi_guid_t *protocol, void *search_key,
+		      const efi_guid_t *protocol, void *search_key,
 		      struct efi_object *efiobj)
 {
 	int i;
@@ -954,7 +955,7 @@ static int efi_search(enum efi_locate_search_type search_type,
  */
 static efi_status_t efi_locate_handle(
 			enum efi_locate_search_type search_type,
-			efi_guid_t *protocol, void *search_key,
+			const efi_guid_t *protocol, void *search_key,
 			unsigned long *buffer_size, efi_handle_t *buffer)
 {
 	struct list_head *lhandle;
@@ -1006,7 +1007,7 @@ static efi_status_t efi_locate_handle(
  */
 static efi_status_t EFIAPI efi_locate_handle_ext(
 			enum efi_locate_search_type search_type,
-			efi_guid_t *protocol, void *search_key,
+			const efi_guid_t *protocol, void *search_key,
 			unsigned long *buffer_size, efi_handle_t *buffer)
 {
 	EFI_ENTRY("%d, %pUl, %p, %p, %p", search_type, protocol, search_key,
@@ -1028,7 +1029,8 @@ static efi_status_t EFIAPI efi_locate_handle_ext(
  * @device		handle of the device
  * @return		status code
  */
-static efi_status_t EFIAPI efi_locate_device_path(efi_guid_t *protocol,
+static efi_status_t EFIAPI efi_locate_device_path(
+			const efi_guid_t *protocol,
 			struct efi_device_path **device_path,
 			efi_handle_t *device)
 {
@@ -1567,7 +1569,7 @@ static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle,
  * @return		status code
  */
 static efi_status_t EFIAPI efi_close_protocol(void *handle,
-					      efi_guid_t *protocol,
+					      const efi_guid_t *protocol,
 					      void *agent_handle,
 					      void *controller_handle)
 {
@@ -1590,7 +1592,7 @@ static efi_status_t EFIAPI efi_close_protocol(void *handle,
  * @return		status code
  */
 static efi_status_t EFIAPI efi_open_protocol_information(efi_handle_t handle,
-			efi_guid_t *protocol,
+			const efi_guid_t *protocol,
 			struct efi_open_protocol_info_entry **entry_buffer,
 			unsigned long *entry_count)
 {
@@ -1679,7 +1681,7 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,
  */
 static efi_status_t EFIAPI efi_locate_handle_buffer(
 			enum efi_locate_search_type search_type,
-			efi_guid_t *protocol, void *search_key,
+			const efi_guid_t *protocol, void *search_key,
 			unsigned long *no_handles, efi_handle_t **buffer)
 {
 	efi_status_t r;
@@ -1721,7 +1723,7 @@ out:
  * @registration	registration key passed to the notification function
  * @protocol_interface	interface implementing the protocol
  */
-static efi_status_t EFIAPI efi_locate_protocol(efi_guid_t *protocol,
+static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol,
 					       void *registration,
 					       void **protocol_interface)
 {
@@ -1774,7 +1776,7 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces(
 	EFI_ENTRY("%p", handle);
 
 	va_list argptr;
-	efi_guid_t *protocol;
+	const efi_guid_t *protocol;
 	void *protocol_interface;
 	efi_status_t r = EFI_SUCCESS;
 	int i = 0;
@@ -1905,7 +1907,7 @@ static void EFIAPI efi_set_mem(void *buffer, size_t size, uint8_t value)
  * @return		status code
  */
 static efi_status_t EFIAPI efi_open_protocol(
-			void *handle, efi_guid_t *protocol,
+			void *handle, const efi_guid_t *protocol,
 			void **protocol_interface, void *agent_handle,
 			void *controller_handle, uint32_t attributes)
 {
@@ -1989,7 +1991,7 @@ out:
  * @return		status code
  */
 static efi_status_t EFIAPI efi_handle_protocol(void *handle,
-					       efi_guid_t *protocol,
+					       const efi_guid_t *protocol,
 					       void **protocol_interface)
 {
 	return efi_open_protocol(handle, protocol, protocol_interface, NULL,
-- 
2.14.1

[U-Boot] [PATCH v2 04/17] efi_loader: wrong type in wait_for_event

[Heinrich Schuchardt]

The UEFI spec defines parameter index of WaitForEvent as UINTN*.
So we should use size_t here.

I deliberately do not use UINTN because I hold a following patch
that will eliminate UINTN because uppercase types to not match
the U-Boot coding style.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 include/efi_api.h                      | 2 +-
 lib/efi_loader/efi_boottime.c          | 2 +-
 lib/efi_selftest/efi_selftest_events.c | 2 +-
 lib/efi_selftest/efi_selftest_tpl.c    | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index aa4306aac9..c44dc9d0cb 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -71,7 +71,7 @@ struct efi_boot_services {
 					 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, unsigned long *index);
+			struct efi_event **event, size_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);
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index e5adc17fab..976d5822f7 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -553,7 +553,7 @@ static efi_status_t EFIAPI efi_set_timer_ext(struct efi_event *event,
  */
 static efi_status_t EFIAPI efi_wait_for_event(unsigned long num_events,
 					      struct efi_event **event,
-					      unsigned long *index)
+					      size_t *index)
 {
 	int i, j;
 
diff --git a/lib/efi_selftest/efi_selftest_events.c b/lib/efi_selftest/efi_selftest_events.c
index 532f165d43..b2cdc150da 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)
 {
-	unsigned long index;
+	size_t index;
 	efi_status_t ret;
 
 	/* Set 10 ms timer */
diff --git a/lib/efi_selftest/efi_selftest_tpl.c b/lib/efi_selftest/efi_selftest_tpl.c
index 5d13f3b52d..0b78ee7595 100644
--- a/lib/efi_selftest/efi_selftest_tpl.c
+++ b/lib/efi_selftest/efi_selftest_tpl.c
@@ -110,7 +110,7 @@ static int teardown(void)
  */
 static int execute(void)
 {
-	unsigned long index;
+	size_t index;
 	efi_status_t ret;
 	UINTN old_tpl;
 
-- 
2.14.1

[U-Boot] [PATCH v2 05/17] efi_loader: incorrect definition of EFI_SIMPLE_NETWORK_PROTOCOL

[Heinrich Schuchardt]

WaitForPacket is an event and not a function pointer.

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

diff --git a/include/efi_api.h b/include/efi_api.h
index c44dc9d0cb..308baeec49 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -633,7 +633,7 @@ struct efi_simple_network
 			ulong *header_size, ulong *buffer_size, void *buffer,
 			struct efi_mac_address *src_addr,
 			struct efi_mac_address *dest_addr, u16 *protocol);
-	void (EFIAPI *waitforpacket)(void);
+	struct efi_event *wait_for_packet;
 	struct efi_simple_network_mode *mode;
 };
 
-- 
2.14.1

[U-Boot] [PATCH v2 06/17] efi_loader: correct bits of receive_filters bit mask

[Heinrich Schuchardt]

Remove extraneous commas.
Add comment.

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

diff --git a/include/efi_api.h b/include/efi_api.h
index 308baeec49..8c227ce703 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -593,11 +593,12 @@ struct efi_simple_network_mode {
 	u8 media_present;
 };
 
-#define EFI_SIMPLE_NETWORK_RECEIVE_UNICAST               0x01,
-#define EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST             0x02,
-#define EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST             0x04,
-#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS           0x08,
-#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST 0x10,
+/* receive_filters bit mask */
+#define EFI_SIMPLE_NETWORK_RECEIVE_UNICAST               0x01
+#define EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST             0x02
+#define EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST             0x04
+#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS           0x08
+#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST 0x10
 
 struct efi_simple_network
 {
-- 
2.14.1

[U-Boot] [PATCH v2 07/17] efi_loader: fill simple network protocol revision

[Heinrich Schuchardt]

Provide the simple network protocol revision.
This revision number could be used to identify backwards compatible
enhancements of the protocol.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 include/efi_api.h        | 3 +++
 lib/efi_loader/efi_net.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/include/efi_api.h b/include/efi_api.h
index 8c227ce703..2f31464cb3 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -600,6 +600,9 @@ struct efi_simple_network_mode {
 #define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS           0x08
 #define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST 0x10
 
+/* revision of the simple network protocol */
+#define EFI_SIMPLE_NETWORK_PROTOCOL_REVISION	0x00010000
+
 struct efi_simple_network
 {
 	u64 revision;
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 91f1e4a69e..fb23bcf633 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -228,6 +228,7 @@ int efi_net_register(void)
 	netobj->parent.protocols[2].guid = &efi_pxe_guid;
 	netobj->parent.protocols[2].protocol_interface = &netobj->pxe;
 	netobj->parent.handle = &netobj->net;
+	netobj->net.revision = EFI_SIMPLE_NETWORK_PROTOCOL_REVISION;
 	netobj->net.start = efi_net_start;
 	netobj->net.stop = efi_net_stop;
 	netobj->net.initialize = efi_net_initialize;
-- 
2.14.1

[U-Boot] [PATCH v2 08/17] efi_loader: efi_net: hwaddr_size = 6

[Heinrich Schuchardt]

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

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

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

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index fb23bcf633..9def34cf93 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -245,6 +245,7 @@ int efi_net_register(void)
 	netobj->net.mode = &netobj->net_mode;
 	netobj->net_mode.state = EFI_NETWORK_STARTED;
 	memcpy(netobj->net_mode.current_address.mac_addr, eth_get_ethaddr(), 6);
+	netobj->net_mode.hwaddr_size = ARP_HLEN;
 	netobj->net_mode.max_packet_size = PKTSIZE;
 
 	netobj->pxe.mode = &netobj->pxe_mode;
-- 
2.14.1

[U-Boot] [PATCH v2 09/17] efi_net: return EFI_UNSUPPORTED where appropriate

[Heinrich Schuchardt]

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

The UEFI spec foresees to return EFI_UNSUPPORTED in these cases.

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

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 9def34cf93..569bf367a8 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -78,9 +78,7 @@ static efi_status_t EFIAPI efi_net_receive_filters(
 	EFI_ENTRY("%p, %x, %x, %x, %lx, %p", this, enable, disable,
 		  reset_mcast_filter, mcast_filter_count, mcast_filter);
 
-	/* XXX Do we care? */
-
-	return EFI_EXIT(EFI_SUCCESS);
+	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
 static efi_status_t EFIAPI efi_net_station_address(
@@ -89,7 +87,7 @@ static efi_status_t EFIAPI efi_net_station_address(
 {
 	EFI_ENTRY("%p, %x, %p", this, reset, new_mac);
 
-	return EFI_EXIT(EFI_INVALID_PARAMETER);
+	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
 static efi_status_t EFIAPI efi_net_statistics(struct efi_simple_network *this,
@@ -98,7 +96,7 @@ static efi_status_t EFIAPI efi_net_statistics(struct efi_simple_network *this,
 {
 	EFI_ENTRY("%p, %x, %p, %p", this, reset, stat_size, stat_table);
 
-	return EFI_EXIT(EFI_INVALID_PARAMETER);
+	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
 static efi_status_t EFIAPI efi_net_mcastiptomac(struct efi_simple_network *this,
@@ -118,7 +116,7 @@ static efi_status_t EFIAPI efi_net_nvdata(struct efi_simple_network *this,
 	EFI_ENTRY("%p, %x, %lx, %lx, %p", this, read_write, offset, buffer_size,
 		  buffer);
 
-	return EFI_EXIT(EFI_INVALID_PARAMETER);
+	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
 static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this,
-- 
2.14.1

[U-Boot] [PATCH v2 10/17] efi_loader: use events for efi_net_receive

[Heinrich Schuchardt]

A timer event is defined. The timer handler cares for receiving new
packets.

efi_timer_check is called both in efi_net_transmit and efi_net_receive
to enable events during network communication.

Calling efi_timer_check in efi_net_get_status is implemented in a
separate patch.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_net.c | 53 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 569bf367a8..3498b96f7a 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -19,6 +19,11 @@ static const efi_guid_t efi_pxe_guid = EFI_PXE_GUID;
 static struct efi_pxe_packet *dhcp_ack;
 static bool new_rx_packet;
 static void *new_tx_packet;
+/*
+ * The notification function of this event is called in every timer cycle
+ * to check if a new network packet has been received.
+ */
+static struct efi_event *network_timer_event;
 
 struct efi_net_obj {
 	/* Generic EFI object parent class data */
@@ -143,6 +148,8 @@ static efi_status_t EFIAPI efi_net_transmit(struct efi_simple_network *this,
 	EFI_ENTRY("%p, %lx, %lx, %p, %p, %p, %p", this, header_size,
 		  buffer_size, buffer, src_addr, dest_addr, protocol);
 
+	efi_timer_check();
+
 	if (header_size) {
 		/* We would need to create the header if header_size != 0 */
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
@@ -174,9 +181,7 @@ static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this,
 	EFI_ENTRY("%p, %p, %p, %p, %p, %p, %p", this, header_size,
 		  buffer_size, buffer, src_addr, dest_addr, protocol);
 
-	push_packet = efi_net_push;
-	eth_rx();
-	push_packet = NULL;
+	efi_timer_check();
 
 	if (!new_rx_packet)
 		return EFI_EXIT(EFI_NOT_READY);
@@ -204,10 +209,32 @@ void efi_net_set_dhcp_ack(void *pkt, int len)
 	memcpy(dhcp_ack, pkt, min(len, maxsize));
 }
 
+/*
+ * Check if a new network packet has been received.
+ *
+ * This notification function is called in every timer cycle.
+ *
+ * @event	the event for which this notification function is registered
+ * @context	event context - not used in this function
+ */
+static void EFIAPI efi_network_timer_notify(struct efi_event *event,
+					    void *context)
+{
+	EFI_ENTRY("%p, %p", event, context);
+
+	if (!new_rx_packet) {
+		push_packet = efi_net_push;
+		eth_rx();
+		push_packet = NULL;
+	}
+	EFI_EXIT(EFI_SUCCESS);
+}
+
 /* This gets called from do_bootefi_exec(). */
 int efi_net_register(void)
 {
 	struct efi_net_obj *netobj;
+	efi_status_t r;
 
 	if (!eth_get_dev()) {
 		/* No eth device active, don't expose any */
@@ -253,5 +280,25 @@ int efi_net_register(void)
 	/* Hook net up to the device list */
 	list_add_tail(&netobj->parent.link, &efi_obj_list);
 
+	/*
+	 * Create a timer event.
+	 *
+	 * The notification function is used to check if an new networkd packet
+	 * has been received.
+	 */
+	r = efi_create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+			     efi_network_timer_notify, NULL,
+			     &network_timer_event);
+	if (r != EFI_SUCCESS) {
+		printf("ERROR: Failed to register network event\n");
+		return r;
+	}
+	/* Network is time critical, create event in every timer cyle */
+	r = efi_set_timer(network_timer_event, EFI_TIMER_PERIODIC, 0);
+	if (r != EFI_SUCCESS) {
+		printf("ERROR: Failed to set network timer\n");
+		return r;
+	}
+
 	return 0;
 }
-- 
2.14.1

[U-Boot] [PATCH v2 11/17] efi_loader: implement WaitForPacket event

[Heinrich Schuchardt]

The WaitForPacket event informs that a network package has been
received by the SimpleNetworkProtocol.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	signaled has been renamed to is_signaled and is bool
---
 lib/efi_loader/efi_net.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 3498b96f7a..f63db686af 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -24,6 +24,10 @@ static void *new_tx_packet;
  * to check if a new network packet has been received.
  */
 static struct efi_event *network_timer_event;
+/*
+ * This event is signaled when a packet has been received.
+ */
+static struct efi_event *wait_for_packet;
 
 struct efi_net_obj {
 	/* Generic EFI object parent class data */
@@ -227,6 +231,8 @@ static void EFIAPI efi_network_timer_notify(struct efi_event *event,
 		eth_rx();
 		push_packet = NULL;
 	}
+	if (!new_rx_packet)
+		wait_for_packet->is_signaled = true;
 	EFI_EXIT(EFI_SUCCESS);
 }
 
@@ -280,6 +286,17 @@ int efi_net_register(void)
 	/* Hook net up to the device list */
 	list_add_tail(&netobj->parent.link, &efi_obj_list);
 
+	/*
+	 * Create WaitForPacket event.
+	 */
+	r = efi_create_event(EVT_NOTIFY_WAIT, TPL_CALLBACK,
+			     efi_network_timer_notify, NULL,
+			     &wait_for_packet);
+	if (r != EFI_SUCCESS) {
+		printf("ERROR: Failed to register network event\n");
+		return r;
+	}
+	netobj->net.wait_for_packet = wait_for_packet;
 	/*
 	 * Create a timer event.
 	 *
-- 
2.14.1

[U-Boot] [PATCH v2 12/17] efi_loader: fix efi_net_get_status

[Heinrich Schuchardt]

The returned interrupt status was wrong.

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

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

Furthermore we should call efi_timer_check() to handle events.

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

diff --git a/include/efi_api.h b/include/efi_api.h
index 2f31464cb3..1f349db246 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -600,6 +600,12 @@ struct efi_simple_network_mode {
 #define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS           0x08
 #define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST 0x10
 
+/* interrupt status bit mask */
+#define EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT	0x01
+#define EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT	0x02
+#define EFI_SIMPLE_NETWORK_COMMAND_INTERRUPT	0x04
+#define EFI_SIMPLE_NETWORK_SOFTWARE_INTERRUPT	0x08
+
 /* revision of the simple network protocol */
 #define EFI_SIMPLE_NETWORK_PROTOCOL_REVISION	0x00010000
 
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index f63db686af..5866119817 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -133,9 +133,14 @@ static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this,
 {
 	EFI_ENTRY("%p, %p, %p", this, int_status, txbuf);
 
-	/* We send packets synchronously, so nothing is outstanding */
-	if (int_status)
-		*int_status = 0;
+	efi_timer_check();
+
+	if (int_status) {
+		/* We send packets synchronously, so nothing is outstanding */
+		*int_status = EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
+		if (new_rx_packet)
+			*int_status |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
+	}
 	if (txbuf)
 		*txbuf = new_tx_packet;
 
-- 
2.14.1

[U-Boot] [PATCH v2 13/17] efi_loader: size fields in SimpleNetworkProtocol

[Heinrich Schuchardt]

The size fields in the Simple Network Protocol are all
UINTN in the UEFI spec. So use size_t.

Provide a function description of the receive function.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 include/efi_api.h        |  4 ++--
 lib/efi_loader/efi_net.c | 24 ++++++++++++++++++++----
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 1f349db246..a9a6494afe 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -636,11 +636,11 @@ struct efi_simple_network
 	efi_status_t (EFIAPI *get_status)(struct efi_simple_network *this,
 			u32 *int_status, void **txbuf);
 	efi_status_t (EFIAPI *transmit)(struct efi_simple_network *this,
-			ulong header_size, ulong buffer_size, void *buffer,
+			size_t header_size, size_t buffer_size, void *buffer,
 			struct efi_mac_address *src_addr,
 			struct efi_mac_address *dest_addr, u16 *protocol);
 	efi_status_t (EFIAPI *receive)(struct efi_simple_network *this,
-			ulong *header_size, ulong *buffer_size, void *buffer,
+			size_t *header_size, size_t *buffer_size, void *buffer,
 			struct efi_mac_address *src_addr,
 			struct efi_mac_address *dest_addr, u16 *protocol);
 	struct efi_event *wait_for_packet;
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 5866119817..f8ae5578ba 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -150,12 +150,13 @@ static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this,
 }
 
 static efi_status_t EFIAPI efi_net_transmit(struct efi_simple_network *this,
-		ulong header_size, ulong buffer_size, void *buffer,
+		size_t header_size, size_t buffer_size, void *buffer,
 		struct efi_mac_address *src_addr,
 		struct efi_mac_address *dest_addr, u16 *protocol)
 {
-	EFI_ENTRY("%p, %lx, %lx, %p, %p, %p, %p", this, header_size,
-		  buffer_size, buffer, src_addr, dest_addr, protocol);
+	EFI_ENTRY("%p, %lu, %lu, %p, %p, %p, %p", this,
+		  (unsigned long)header_size, (unsigned long)buffer_size,
+		  buffer, src_addr, dest_addr, protocol);
 
 	efi_timer_check();
 
@@ -182,8 +183,23 @@ static void efi_net_push(void *pkt, int len)
 	new_rx_packet = true;
 }
 
+/*
+ * Receive a packet from a network interface.
+ *
+ * This function implements the Receive service of the Simple Network Protocol.
+ * See the UEFI spec for details.
+ *
+ * @this	the instance of the Simple Network Protocol
+ * @header_size	size of the media header
+ * @buffer_size	size of the buffer to receive the packet
+ * @buffer	buffer to receive the packet
+ * @src_addr	source MAC address
+ * @dest_addr	destination MAC address
+ * @protocol	protocol
+ * @return	status code
+ */
 static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this,
-		ulong *header_size, ulong *buffer_size, void *buffer,
+		size_t *header_size, size_t *buffer_size, void *buffer,
 		struct efi_mac_address *src_addr,
 		struct efi_mac_address *dest_addr, u16 *protocol)
 {
-- 
2.14.1

[U-Boot] [PATCH v2 14/17] efi_loader: fill return values in SimpleNetworkProtocol

[Heinrich Schuchardt]

In the receive function all return values should be filled.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_net.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index f8ae5578ba..9c85e1b72f 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -203,6 +203,10 @@ static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this,
 		struct efi_mac_address *src_addr,
 		struct efi_mac_address *dest_addr, u16 *protocol)
 {
+	struct ethernet_hdr *eth_hdr;
+	size_t hdr_size = sizeof(struct ethernet_hdr);
+	u16 protlen;
+
 	EFI_ENTRY("%p, %p, %p, %p, %p, %p, %p", this, header_size,
 		  buffer_size, buffer, src_addr, dest_addr, protocol);
 
@@ -210,13 +214,32 @@ static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this,
 
 	if (!new_rx_packet)
 		return EFI_EXIT(EFI_NOT_READY);
-
+	/* Check that we at least received an Ethernet header */
+	if (net_rx_packet_len < sizeof(struct ethernet_hdr)) {
+		new_rx_packet = false;
+		return EFI_EXIT(EFI_NOT_READY);
+	}
+	/* Fill export parameters */
+	eth_hdr = (struct ethernet_hdr *)net_rx_packet;
+	protlen = ntohs(eth_hdr->et_protlen);
+	if (protlen == 0x8100) {
+		hdr_size += 4;
+		protlen = ntohs(*(u16 *)&net_rx_packet[hdr_size - 2]);
+	}
+	if (header_size)
+		*header_size = hdr_size;
+	if (dest_addr)
+		memcpy(dest_addr, eth_hdr->et_dest, ARP_HLEN);
+	if (src_addr)
+		memcpy(src_addr, eth_hdr->et_src, ARP_HLEN);
+	if (protocol)
+		*protocol = protlen;
 	if (*buffer_size < net_rx_packet_len) {
 		/* Packet doesn't fit, try again with bigger buf */
 		*buffer_size = net_rx_packet_len;
 		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
 	}
-
+	/* Copy packet */
 	memcpy(buffer, net_rx_packet, net_rx_packet_len);
 	*buffer_size = net_rx_packet_len;
 	new_rx_packet = false;
-- 
2.14.1

[U-Boot] [PATCH v2 15/17] efi_selftest: correct definition of efi_st_error

[Heinrich Schuchardt]

Enclose definition in parantheses to allow using efi_st_error
like a void function.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 include/efi_selftest.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/efi_selftest.h b/include/efi_selftest.h
index beb662d4e1..2f0992f06e 100644
--- a/include/efi_selftest.h
+++ b/include/efi_selftest.h
@@ -23,8 +23,8 @@
  * @...	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_printf("%s(%u):\nERROR: ", __FILE__, __LINE__), \
+	efi_st_printf(__VA_ARGS__)) \
 
 /*
  * A test may be setup and executed at boottime,
-- 
2.14.1

[U-Boot] [PATCH v2 16/17] efi_selftest: allow printing MAC addresses

[Heinrich Schuchardt]

Add %pm as format string to print a MAC address.
This is helpful when analyzing network problems.

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

diff --git a/lib/efi_selftest/efi_selftest_console.c b/lib/efi_selftest/efi_selftest_console.c
index 7b5b724a61..840e2290c6 100644
--- a/lib/efi_selftest/efi_selftest_console.c
+++ b/lib/efi_selftest/efi_selftest_console.c
@@ -12,6 +12,37 @@
 struct efi_simple_text_output_protocol *con_out;
 struct efi_simple_input_interface *con_in;
 
+/*
+ * Print a MAC address to an u16 string
+ *
+ * @pointer: mac address
+ * @buf: pointer to buffer address
+ * on return position of terminating zero word
+ */
+static void mac(void *pointer, u16 **buf)
+{
+	int i, j;
+	u16 c;
+	u8 *p = (u8 *)pointer;
+	u8 byte;
+	u16 *pos = *buf;
+
+	for (i = 0; i < ARP_HLEN; ++i) {
+		if (i)
+			*pos++ = ':';
+		byte = p[i];
+		for (j = 4; j >= 0; j -= 4) {
+			c = (byte >> j) & 0x0f;
+			c += '0';
+			if (c > '9')
+				c += 'a' - '9' - 1;
+			*pos++ = c;
+		}
+	}
+	*pos = 0;
+	*buf = pos;
+}
+
 /*
  * Print a pointer to an u16 string
  *
@@ -146,7 +177,15 @@ void efi_st_printf(const char *fmt, ...)
 				int2dec(va_arg(args, s32), &pos);
 				break;
 			case 'p':
-				pointer(va_arg(args, void*), &pos);
+				++c;
+				switch (*c) {
+				case 'm':
+					mac(va_arg(args, void*), &pos);
+					break;
+				default:
+					--c;
+					pointer(va_arg(args, void*), &pos);
+				}
 				break;
 			case 's':
 				s = va_arg(args, const char *);
-- 
2.14.1

[U-Boot] [PATCH v2 17/17] efi_loader: supply EFI network test

[Heinrich Schuchardt]

This patch provices an EFI application to check the correct function
of the Simple Network Protocol implementation.

It sends a DHCP request and analyzes the DHCP offer.

Different error conditions including a 10s timeout are checked.

A successful execution will look like this:

=> bootefi nettest
Scanning disk ide.blk#0...
Found 1 disks
WARNING: Invalid device tree, expect boot to fail
Network test
DHCP Discover
DHCP reply received from 192.168.76.2 (52:55:c0:a8:4c:02)
as broadcast message.
OK. The test was completed successfully.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	move efi_st_memcmp to efi_selftest_util.c
	The function can be reused for other tests.

	Use constants for return values.
---
 include/efi_selftest.h               |  11 +
 lib/efi_selftest/Makefile            |   8 +-
 lib/efi_selftest/efi_selftest_snp.c  | 424 +++++++++++++++++++++++++++++++++++
 lib/efi_selftest/efi_selftest_util.c |  25 +++
 4 files changed, 467 insertions(+), 1 deletion(-)
 create mode 100644 lib/efi_selftest/efi_selftest_snp.c
 create mode 100644 lib/efi_selftest/efi_selftest_util.c

diff --git a/include/efi_selftest.h b/include/efi_selftest.h
index 2f0992f06e..7ec42a0406 100644
--- a/include/efi_selftest.h
+++ b/include/efi_selftest.h
@@ -60,6 +60,17 @@ void efi_st_exit_boot_services(void);
 void efi_st_printf(const char *fmt, ...)
 		 __attribute__ ((format (__printf__, 1, 2)));
 
+/*
+ * Compare memory.
+ * We cannot use lib/string.c due to different CFLAGS values.
+ *
+ * @buf1:	first buffer
+ * @buf2:	second buffer
+ * @length:	number of bytes to compare
+ * @return:	0 if both buffers contain the same bytes
+ */
+int efi_st_memcmp(const void *buf1, const void *buf2, size_t length);
+
 /*
  * Reads an Unicode character from the input device.
  *
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 30f1960933..e446046e02 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -15,12 +15,18 @@ 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_snp.o := $(CFLAGS_EFI)
+CFLAGS_REMOVE_efi_selftest_snp.o := $(CFLAGS_NON_EFI)
 CFLAGS_efi_selftest_tpl.o := $(CFLAGS_EFI)
 CFLAGS_REMOVE_efi_selftest_tpl.o := $(CFLAGS_NON_EFI)
+CFLAGS_efi_selftest_util.o := $(CFLAGS_EFI)
+CFLAGS_REMOVE_efi_selftest_util.o := $(CFLAGS_NON_EFI)
 
 obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \
 efi_selftest.o \
 efi_selftest_console.o \
 efi_selftest_events.o \
 efi_selftest_exitbootservices.o \
-efi_selftest_tpl.o
+efi_selftest_snp.o \
+efi_selftest_tpl.o \
+efi_selftest_util.o
diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c
new file mode 100644
index 0000000000..638be0147d
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_snp.c
@@ -0,0 +1,424 @@
+/*
+ * efi_selftest_snp
+ *
+ * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ *
+ * This unit test covers the Simple Network Protocol as well as
+ * the CopyMem and SetMem boottime services.
+ *
+ * A DHCP discover message is sent. The test is successful if a
+ * DHCP reply is received.
+ *
+ * TODO: Once ConnectController and DisconnectController are implemented
+ *	 we should connect our code as controller.
+ */
+
+#include <efi_selftest.h>
+
+/*
+ * MAC address for broadcasts
+ */
+static const u8 BROADCAST_MAC[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+
+struct dhcp_hdr {
+	u8 op;
+#define BOOTREQUEST 1
+#define BOOTREPLY 2
+	u8 htype;
+# define HWT_ETHER 1
+	u8 hlen;
+# define HWL_ETHER 6
+	u8 hops;
+	u32 xid;
+	u16 secs;
+	u16 flags;
+#define DHCP_FLAGS_UNICAST	0x0000
+#define DHCP_FLAGS_BROADCAST	0x0080
+	u32 ciaddr;
+	u32 yiaddr;
+	u32 siaddr;
+	u32 giaddr;
+	u8 chaddr[16];
+	u8 sname[64];
+	u8 file[128];
+};
+
+/*
+ * Message type option.
+ */
+#define DHCP_MESSAGE_TYPE	0x35
+#define DHCPDISCOVER		1
+#define DHCPOFFER		2
+#define DHCPREQUEST		3
+#define DHCPDECLINE		4
+#define DHCPACK			5
+#define DHCPNAK			6
+#define DHCPRELEASE		7
+
+struct dhcp {
+	struct ethernet_hdr eth_hdr;
+	struct ip_udp_hdr ip_udp;
+	struct dhcp_hdr dhcp_hdr;
+	u8 opt[128];
+} __packed;
+
+static struct efi_boot_services *boottime;
+static struct efi_simple_network *net;
+static struct efi_event *timer;
+static const efi_guid_t efi_net_guid = EFI_SIMPLE_NETWORK_GUID;
+/* IP packet ID */
+static unsigned int net_ip_id;
+
+/*
+ * Compute the checksum of the IP header. We cover even values of length only.
+ * We cannot use net/checksum.c due to different CFLAGS values.
+ *
+ * @buf:	IP header
+ * @len:	length of header in bytes
+ * @return:	checksum
+ */
+static unsigned int efi_ip_checksum(const void *buf, size_t len)
+{
+	size_t i;
+	u32 sum = 0;
+	const u16 *pos = buf;
+
+	for (i = 0; i < len; i += 2)
+		sum += *pos++;
+
+	sum = (sum >> 16) + (sum & 0xffff);
+	sum += sum >> 16;
+	sum = ~sum & 0xffff;
+
+	return sum;
+}
+
+/*
+ * Transmit a DHCPDISCOVER message.
+ */
+static efi_status_t send_dhcp_discover(void)
+{
+	efi_status_t ret;
+	struct dhcp p = {};
+
+	/*
+	 * Fill ethernet header
+	 */
+	boottime->copy_mem(p.eth_hdr.et_dest, (void *)BROADCAST_MAC, ARP_HLEN);
+	boottime->copy_mem(p.eth_hdr.et_src, &net->mode->current_address,
+			   ARP_HLEN);
+	p.eth_hdr.et_protlen = htons(PROT_IP);
+	/*
+	 * Fill IP header
+	 */
+	p.ip_udp.ip_hl_v	= 0x45;
+	p.ip_udp.ip_len		= htons(sizeof(struct dhcp) -
+					sizeof(struct ethernet_hdr));
+	p.ip_udp.ip_id		= htons(++net_ip_id);
+	p.ip_udp.ip_off		= htons(IP_FLAGS_DFRAG);
+	p.ip_udp.ip_ttl		= 0xff; /* time to live */
+	p.ip_udp.ip_p		= IPPROTO_UDP;
+	boottime->set_mem(&p.ip_udp.ip_dst, 4, 0xff);
+	p.ip_udp.ip_sum		= efi_ip_checksum(&p.ip_udp, IP_HDR_SIZE);
+
+	/*
+	 * Fill UDP header
+	 */
+	p.ip_udp.udp_src	= htons(68);
+	p.ip_udp.udp_dst	= htons(67);
+	p.ip_udp.udp_len	= htons(sizeof(struct dhcp) -
+					sizeof(struct ethernet_hdr) -
+					sizeof(struct ip_hdr));
+	/*
+	 * Fill DHCP header
+	 */
+	p.dhcp_hdr.op		= BOOTREQUEST;
+	p.dhcp_hdr.htype	= HWT_ETHER;
+	p.dhcp_hdr.hlen		= HWL_ETHER;
+	p.dhcp_hdr.flags	= htons(DHCP_FLAGS_UNICAST);
+	boottime->copy_mem(&p.dhcp_hdr.chaddr,
+			   &net->mode->current_address, ARP_HLEN);
+	/*
+	 * Fill options
+	 */
+	p.opt[0]	= 0x63; /* DHCP magic cookie */
+	p.opt[1]	= 0x82;
+	p.opt[2]	= 0x53;
+	p.opt[3]	= 0x63;
+	p.opt[4]	= DHCP_MESSAGE_TYPE;
+	p.opt[5]	= 0x01; /* length */
+	p.opt[6]	= DHCPDISCOVER;
+	p.opt[7]	= 0x39; /* maximum message size */
+	p.opt[8]	= 0x02; /* length */
+	p.opt[9]	= 0x02; /* 576 bytes */
+	p.opt[10]	= 0x40;
+	p.opt[11]	= 0xff; /* end of options */
+
+	/*
+	 * Transmit DHCPDISCOVER message.
+	 */
+	ret = net->transmit(net, 0, sizeof(struct dhcp), &p, NULL, NULL, 0);
+	if (ret != EFI_SUCCESS)
+		efi_st_error("Sending a DHCP request failed\n");
+	else
+		efi_st_printf("DHCP Discover\n");
+	return ret;
+}
+
+/*
+ * Setup unit test.
+ *
+ * Create a 1 s periodic timer.
+ * Start the network driver.
+ *
+ * @handle:	handle of the loaded image
+ * @systable:	system table
+ * @return:	EFI_ST_SUCCESS for success
+ */
+static int setup(const efi_handle_t handle,
+		 const struct efi_system_table *systable)
+{
+	efi_status_t ret;
+
+	boottime = systable->boottime;
+
+	/*
+	 * Create a timer event.
+	 */
+	ret = boottime->create_event(EVT_TIMER, TPL_CALLBACK, NULL, NULL,
+				     &timer);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to create event\n");
+		return EFI_ST_FAILURE;
+	}
+	/*
+	 * Set timer period to 1s.
+	 */
+	ret = boottime->set_timer(timer, EFI_TIMER_PERIODIC, 10000000);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to locate simple network protocol\n");
+		return EFI_ST_FAILURE;
+	}
+	/*
+	 * Find an interface implementing the SNP protocol.
+	 */
+	ret = boottime->locate_protocol(&efi_net_guid, NULL, (void **)&net);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to locate simple network protocol\n");
+		return EFI_ST_FAILURE;
+	}
+	/*
+	 * Check hardware address size.
+	 */
+	if (!net->mode) {
+		efi_st_error("Mode not provided\n");
+		return EFI_ST_FAILURE;
+	}
+	if (net->mode->hwaddr_size != ARP_HLEN) {
+		efi_st_error("HwAddressSize = %u, expected %u\n",
+			     net->mode->hwaddr_size, ARP_HLEN);
+		return EFI_ST_FAILURE;
+	}
+	/*
+	 * Check that WaitForPacket event exists.
+	 */
+	if (!net->wait_for_packet) {
+		efi_st_error("WaitForPacket event missing\n");
+		return EFI_ST_FAILURE;
+	}
+	/*
+	 * Initialize network adapter.
+	 */
+	ret = net->initialize(net, 0, 0);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to initialize network adapter\n");
+		return EFI_ST_FAILURE;
+	}
+	/*
+	 * Start network adapter.
+	 */
+	ret = net->start(net);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to start network adapter\n");
+		return EFI_ST_FAILURE;
+	}
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * Execute unit test.
+ *
+ * A DHCP discover message is sent. The test is successful if a
+ * DHCP reply is received within 10 seconds.
+ *
+ * @return:	EFI_ST_SUCCESS for success
+ */
+static int execute(void)
+{
+	efi_status_t ret;
+	struct efi_event *events[2];
+	size_t index;
+	union {
+		struct dhcp p;
+		u8 b[PKTSIZE];
+	} buffer;
+	struct efi_mac_address srcaddr;
+	struct efi_mac_address destaddr;
+	size_t buffer_size;
+	u8 *addr;
+	/*
+	 * The timeout is to occur after 10 s.
+	 */
+	unsigned int timeout = 10;
+
+	/*
+	 * Send DHCP discover message
+	 */
+	ret = send_dhcp_discover();
+	if (ret != EFI_SUCCESS)
+		return EFI_ST_FAILURE;
+
+	/*
+	 * If we would call WaitForEvent only with the WaitForPacket event,
+	 * our code would block until a packet is received which might never
+	 * occur. By calling WaitFor event with both a timer event and the
+	 * WaitForPacket event we can escape this blocking situation.
+	 *
+	 * If the timer event occurs before we have received a DHCP reply
+	 * a further DHCP discover message is sent.
+	 */
+	events[0] = timer;
+	events[1] = net->wait_for_packet;
+	for (;;) {
+		/*
+		 * Wait for packet to be received or timer event.
+		 */
+		boottime->wait_for_event(2, events, &index);
+		if (index == 0) {
+			/*
+			 * The timer event occurred. Check for timeout.
+			 */
+			--timeout;
+			if (!timeout) {
+				efi_st_error("Timeout occurred\n");
+				return EFI_ST_FAILURE;
+			}
+			/*
+			 * Send further DHCP discover message
+			 */
+			ret = send_dhcp_discover();
+			if (ret != EFI_SUCCESS)
+				return EFI_ST_FAILURE;
+			continue;
+		}
+		/*
+		 * Receive packet
+		 */
+		buffer_size = sizeof(buffer);
+		net->receive(net, NULL, &buffer_size, &buffer,
+			     &srcaddr, &destaddr, NULL);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("Failed to receive packet");
+			return EFI_ST_FAILURE;
+		}
+		/*
+		 * Check the packet is meant for this system.
+		 * Unfortunately QEMU ignores the broadcast flag.
+		 * So we have to check for broadcasts too.
+		 */
+		if (efi_st_memcmp(&destaddr, &net->mode->current_address,
+				  ARP_HLEN) &&
+		    efi_st_memcmp(&destaddr, BROADCAST_MAC, ARP_HLEN))
+			continue;
+		/*
+		 * Check this is a DHCP reply
+		 */
+		if (buffer.p.eth_hdr.et_protlen != ntohs(PROT_IP) ||
+		    buffer.p.ip_udp.ip_hl_v != 0x45 ||
+		    buffer.p.ip_udp.ip_p != IPPROTO_UDP ||
+		    buffer.p.ip_udp.udp_src != ntohs(67) ||
+		    buffer.p.ip_udp.udp_dst != ntohs(68) ||
+		    buffer.p.dhcp_hdr.op != BOOTREPLY)
+			continue;
+		/*
+		 * We successfully received a DHCP reply.
+		 */
+		break;
+	}
+
+	/*
+	 * Write a log message.
+	 */
+	addr = (u8 *)&buffer.p.ip_udp.ip_src;
+	efi_st_printf("DHCP reply received from %u.%u.%u.%u (%pm) ",
+		      addr[0], addr[1], addr[2], addr[3], &srcaddr);
+	if (!efi_st_memcmp(&destaddr, BROADCAST_MAC, ARP_HLEN))
+		efi_st_printf("as broadcast message.\n");
+	else
+		efi_st_printf("as unicast message.\n");
+
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * Tear down unit test.
+ *
+ * Close the timer event created in setup.
+ * Shut down the network adapter.
+ *
+ * @return:	EFI_ST_SUCCESS for success
+ */
+static int teardown(void)
+{
+	efi_status_t ret;
+	int exit_status = EFI_ST_SUCCESS;
+
+	if (timer) {
+		/*
+		 * Stop timer.
+		 */
+		ret = boottime->set_timer(timer, EFI_TIMER_STOP, 0);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("Failed to stop timer");
+			exit_status = EFI_ST_FAILURE;
+		}
+		/*
+		 * Close timer event.
+		 */
+		ret = boottime->close_event(timer);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("Failed to close event");
+			exit_status = EFI_ST_FAILURE;
+		}
+	}
+	if (net) {
+		/*
+		 * Stop network adapter.
+		 */
+		ret = net->stop(net);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("Failed to stop network adapter\n");
+			exit_status = EFI_ST_FAILURE;
+		}
+		/*
+		 * Shut down network adapter.
+		 */
+		ret = net->shutdown(net);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("Failed to shut down network adapter\n");
+			exit_status = EFI_ST_FAILURE;
+		}
+	}
+
+	return exit_status;
+}
+
+EFI_UNIT_TEST(snp) = {
+	.name = "simple network protocol",
+	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+	.setup = setup,
+	.execute = execute,
+	.teardown = teardown,
+};
diff --git a/lib/efi_selftest/efi_selftest_util.c b/lib/efi_selftest/efi_selftest_util.c
new file mode 100644
index 0000000000..c9c295e2fb
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_util.c
@@ -0,0 +1,25 @@
+/*
+ * efi_selftest_util
+ *
+ * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ *
+ * Utility functions
+ */
+
+#include <efi_selftest.h>
+
+int efi_st_memcmp(const void *buf1, const void *buf2, size_t length)
+{
+	const u8 *pos1 = buf1;
+	const u8 *pos2 = buf2;
+
+	for (; length; --length) {
+		if (*pos1 != *pos2)
+			return pos1 - pos2;
+		++pos1;
+		++pos2;
+	}
+	return EFI_ST_SUCCESS;
+}
-- 
2.14.1

[U-Boot] [PATCH v2 11/17] efi_loader: implement WaitForPacket event

[Alexander Graf]

On 05.10.17 16:36, Heinrich Schuchardt wrote:
> The WaitForPacket event informs that a network package has been
> received by the SimpleNetworkProtocol.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
> 	signaled has been renamed to is_signaled and is bool
> ---
>  lib/efi_loader/efi_net.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
> index 3498b96f7a..f63db686af 100644
> --- a/lib/efi_loader/efi_net.c
> +++ b/lib/efi_loader/efi_net.c
> @@ -24,6 +24,10 @@ static void *new_tx_packet;
>   * to check if a new network packet has been received.
>   */
>  static struct efi_event *network_timer_event;
> +/*
> + * This event is signaled when a packet has been received.
> + */
> +static struct efi_event *wait_for_packet;
>  
>  struct efi_net_obj {
>  	/* Generic EFI object parent class data */
> @@ -227,6 +231,8 @@ static void EFIAPI efi_network_timer_notify(struct efi_event *event,
>  		eth_rx();
>  		push_packet = NULL;
>  	}
> +	if (!new_rx_packet)
> +		wait_for_packet->is_signaled = true;

Shouldn't that be if (new_rx_packet)? You want to know whether eth_rx()
received something, no?

In fact, why don't you just move this whole line into efi_net_push()?


Alex

>  	EFI_EXIT(EFI_SUCCESS);
>  }
>  
> @@ -280,6 +286,17 @@ int efi_net_register(void)
>  	/* Hook net up to the device list */
>  	list_add_tail(&netobj->parent.link, &efi_obj_list);
>  
> +	/*
> +	 * Create WaitForPacket event.
> +	 */
> +	r = efi_create_event(EVT_NOTIFY_WAIT, TPL_CALLBACK,
> +			     efi_network_timer_notify, NULL,
> +			     &wait_for_packet);
> +	if (r != EFI_SUCCESS) {
> +		printf("ERROR: Failed to register network event\n");
> +		return r;
> +	}
> +	netobj->net.wait_for_packet = wait_for_packet;
>  	/*
>  	 * Create a timer event.
>  	 *
> 

[U-Boot] [PATCH v2 00/17] efi_loader: Simple Network Protocol

[Alexander Graf]

On 05.10.17 16:35, Heinrich Schuchardt wrote:
> This patch series focuses on
> * correction of parameter types for boot services
> * fixes for the SetMem and CopyMem boot services
> * fixes for the simple network protocol implementation (SNP)
> * a unit test for SetMem, CopyMem and the simple network protocol
> 
> The unit test broadcasts a DHCPDISCOVER messager over the network and
> receives the reply.
> 
> This patch series is based on
> * efi-next tree
>   https://github.com/agraf/u-boot/tree/efi-next
> * [PATCH 1/1] efi_selftest: enable CONFIG_CMD_BOOTEFI_SELFTEST
>   https://patchwork.ozlabs.org/patch/816412/
>   This patch enables the unit test on qemu-x86_defconfig
> * [PATCH 1/1] efi_loader: provide function comments for boot services
>   https://patchwork.ozlabs.org/patch/817010/

Looks quite good to me. I've applied them (with the modification I
suggested), running through tests and will push them to master if nobody
(human or machine) objects within the next 1-2 days.


Alex

[U-Boot] [PATCH v2 00/17] efi_loader: Simple Network Protocol

[Heinrich Schuchardt]

On 10/06/2017 02:08 PM, Alexander Graf wrote:
> 
> 
> On 05.10.17 16:35, Heinrich Schuchardt wrote:
>> This patch series focuses on
>> * correction of parameter types for boot services
>> * fixes for the SetMem and CopyMem boot services
>> * fixes for the simple network protocol implementation (SNP)
>> * a unit test for SetMem, CopyMem and the simple network protocol
>>
>> The unit test broadcasts a DHCPDISCOVER messager over the network and
>> receives the reply.
>>
>> This patch series is based on
>> * efi-next tree
>>   https://github.com/agraf/u-boot/tree/efi-next
>> * [PATCH 1/1] efi_selftest: enable CONFIG_CMD_BOOTEFI_SELFTEST
>>   https://patchwork.ozlabs.org/patch/816412/
>>   This patch enables the unit test on qemu-x86_defconfig
>> * [PATCH 1/1] efi_loader: provide function comments for boot services
>>   https://patchwork.ozlabs.org/patch/817010/
> 
> Looks quite good to me. I've applied them (with the modification I
> suggested), running through tests and will push them to master if nobody
> (human or machine) objects within the next 1-2 days.
> 
> 
> Alex
> 

You moved setting is_signaled to efi_net_push.

181 static void efi_net_push(void *pkt, int len)
182 {
183         new_rx_packet = true;
184         wait_for_packet->is_signaled = true;
185 }

The code looks fine to me.

The selftest for the Simple Network Protocol correctly receives a DHCP
packet.

Thank you.

I just sent you a fix for efi_st_memcmp:
[PATCH 1/1] efi_selftest: efi_st_memcmp return difference of bytes

Best regards

Heinrich

[U-Boot] [PATCH v2 01/17] efi_loader: call EFI_EXIT in efi_copy_mem, efi_set_mem

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> EFI_ENTRY and EFI_EXIT calls must match.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_boottime.c | 2 ++
>  1 file changed, 2 insertions(+)

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

[U-Boot] [PATCH v2 02/17] efi_loader: parameters of CopyMem and SetMem

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The UEFI spec defines the length parameters of CopyMem and SetMem
> as UINTN. We should size_t here.
>
> The source buffer of CopyMem should be marked as const.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_api.h             |  7 +++----
>  lib/efi_loader/efi_boottime.c | 10 +++++-----
>  2 files changed, 8 insertions(+), 9 deletions(-)

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

[U-Boot] [PATCH v2 03/17] efi_loader: pass GUIDs as const efi_guid_t *

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> We need to call some boottime services internally.
> Our GUIDs are stored as const efi_guid_t *.
>
> The boottime services never change GUIDs.
> So we can define the parameters as const efi_guid_t *.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_api.h             | 27 ++++++++++++++-------------
>  lib/efi_loader/efi_boottime.c | 40 +++++++++++++++++++++-------------------
>  2 files changed, 35 insertions(+), 32 deletions(-)

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

[U-Boot] [PATCH v2 04/17] efi_loader: wrong type in wait_for_event

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The UEFI spec defines parameter index of WaitForEvent as UINTN*.
> So we should use size_t here.
>
> I deliberately do not use UINTN because I hold a following patch
> that will eliminate UINTN because uppercase types to not match
> the U-Boot coding style.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_api.h                      | 2 +-
>  lib/efi_loader/efi_boottime.c          | 2 +-
>  lib/efi_selftest/efi_selftest_events.c | 2 +-
>  lib/efi_selftest/efi_selftest_tpl.c    | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

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

[U-Boot] [PATCH v2 05/17] efi_loader: incorrect definition of EFI_SIMPLE_NETWORK_PROTOCOL

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> WaitForPacket is an event and not a function pointer.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_api.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

[U-Boot] [PATCH v2 06/17] efi_loader: correct bits of receive_filters bit mask

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Remove extraneous commas.
> Add comment.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_api.h | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

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

[U-Boot] [PATCH v2 07/17] efi_loader: fill simple network protocol revision

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Provide the simple network protocol revision.
> This revision number could be used to identify backwards compatible
> enhancements of the protocol.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_api.h        | 3 +++
>  lib/efi_loader/efi_net.c | 1 +
>  2 files changed, 4 insertions(+)

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

[U-Boot] [PATCH v2 08/17] efi_loader: efi_net: hwaddr_size = 6

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The length of a MAC address is 6.
> We have to set this length in the EFI_SIMPLE_NETWORK_MODE
> structure of the EFI_SIMPLE_NETWORK_PROTOCOL.
>
> Without this patch iPXE fails to initialize the network with
> error message
> SNP MAC(001e0633bcbf,0x0) has invalid hardware address length 0
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_net.c | 1 +
>  1 file changed, 1 insertion(+)

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

[U-Boot] [PATCH v2 09/17] efi_net: return EFI_UNSUPPORTED where appropriate

[Simon Glass]

On 5 October 2017 at 08:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> U-Boot does not implement all functions of the simple network
> protocol. The unimplemented functions return either of
> EFI_SUCCESS and EFI_INVALID_PARAMETER.
>
> The UEFI spec foresees to return EFI_UNSUPPORTED in these cases.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_net.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)

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

[U-Boot] [PATCH v2 10/17] efi_loader: use events for efi_net_receive

[Simon Glass]

On 5 October 2017 at 08:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> A timer event is defined. The timer handler cares for receiving new
> packets.
>
> efi_timer_check is called both in efi_net_transmit and efi_net_receive
> to enable events during network communication.
>
> Calling efi_timer_check in efi_net_get_status is implemented in a
> separate patch.

Can you explain in the commit message why this patch is needed?

>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_net.c | 53 +++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 50 insertions(+), 3 deletions(-)

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

[U-Boot] [PATCH v2 12/17] efi_loader: fix efi_net_get_status

[Simon Glass]

On 5 October 2017 at 08:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The returned interrupt status was wrong.
>
> As out transmit buffer is empty we need to always set
> EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT.
>
> When we have received a packet we need to set
> EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT.
>
> Furthermore we should call efi_timer_check() to handle events.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_api.h        |  6 ++++++
>  lib/efi_loader/efi_net.c | 11 ++++++++---
>  2 files changed, 14 insertions(+), 3 deletions(-)

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

[U-Boot] [PATCH v2 13/17] efi_loader: size fields in SimpleNetworkProtocol

[Simon Glass]

On 5 October 2017 at 08:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The size fields in the Simple Network Protocol are all
> UINTN in the UEFI spec. So use size_t.
>
> Provide a function description of the receive function.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_api.h        |  4 ++--
>  lib/efi_loader/efi_net.c | 24 ++++++++++++++++++++----
>  2 files changed, 22 insertions(+), 6 deletions(-)

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

[U-Boot] [PATCH v2 14/17] efi_loader: fill return values in SimpleNetworkProtocol

[Simon Glass]

On 5 October 2017 at 08:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> In the receive function all return values should be filled.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_net.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)

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

(in so far as I understand it)

[U-Boot] [PATCH v2 15/17] efi_selftest: correct definition of efi_st_error

[Simon Glass]

On 5 October 2017 at 08:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Enclose definition in parantheses to allow using efi_st_error
> like a void function.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_selftest.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

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

[U-Boot] [PATCH v2 16/17] efi_selftest: allow printing MAC addresses

[Simon Glass]

On 5 October 2017 at 08:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Add %pm as format string to print a MAC address.
> This is helpful when analyzing network problems.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_selftest/efi_selftest_console.c | 41 ++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)

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

I wonder if we could use U-Boot's existing printf() code?

[U-Boot] [PATCH v2 17/17] efi_loader: supply EFI network test

[Simon Glass]

On 5 October 2017 at 08:36, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> This patch provices an EFI application to check the correct function
> of the Simple Network Protocol implementation.
>
> It sends a DHCP request and analyzes the DHCP offer.
>
> Different error conditions including a 10s timeout are checked.
>
> A successful execution will look like this:
>
> => bootefi nettest
> Scanning disk ide.blk#0...
> Found 1 disks
> WARNING: Invalid device tree, expect boot to fail
> Network test
> DHCP Discover
> DHCP reply received from 192.168.76.2 (52:55:c0:a8:4c:02)
> as broadcast message.
> OK. The test was completed successfully.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         move efi_st_memcmp to efi_selftest_util.c
>         The function can be reused for other tests.
>
>         Use constants for return values.
> ---
>  include/efi_selftest.h               |  11 +
>  lib/efi_selftest/Makefile            |   8 +-
>  lib/efi_selftest/efi_selftest_snp.c  | 424 +++++++++++++++++++++++++++++++++++
>  lib/efi_selftest/efi_selftest_util.c |  25 +++
>  4 files changed, 467 insertions(+), 1 deletion(-)
>  create mode 100644 lib/efi_selftest/efi_selftest_snp.c
>  create mode 100644 lib/efi_selftest/efi_selftest_util.c

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