[U-Boot] [PATCH v3 3/3] efi_loader: fix simple network protocol

Heinrich Schuchardt xypron.glpk at gmx.de
Sat Oct 20 19:54:57 UTC 2018


We should not call eth_rx() before the network interface is initialized.
The services of the simple network protocol should check the state of
the network adapter.

Add and correct comments.

Without this patch i.mx6 system Wandboard Quad rev B1 fails to execute
bootefi selftest.

Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
v3:
	split the patch into a series
v2:
	match EFI_ENTRY() and EFI_EXIT() in efi_network_timer_notify()
	update comments
---
 lib/efi_loader/efi_net.c | 391 +++++++++++++++++++++++++++++++++------
 1 file changed, 332 insertions(+), 59 deletions(-)

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 9987aba1b9..c4b1e6e1d9 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -43,22 +43,68 @@ struct efi_net_obj {
 	struct efi_pxe_mode pxe_mode;
 };
 
+/*
+ * efi_net_start() - start the network interface
+ *
+ * This function implements the Start service of the
+ * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
+ * (UEFI) specification for details.
+ *
+ * @this:	pointer to the protocol instance
+ * Return:	status code
+ */
 static efi_status_t EFIAPI efi_net_start(struct efi_simple_network *this)
 {
+	efi_status_t ret = EFI_SUCCESS;
+
 	EFI_ENTRY("%p", this);
 
-	return EFI_EXIT(EFI_SUCCESS);
+	/* Check parameters */
+	if (!this) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	if (this->mode->state != EFI_NETWORK_STOPPED)
+		ret = EFI_ALREADY_STARTED;
+	else
+		this->mode->state = EFI_NETWORK_STARTED;
+out:
+	return EFI_EXIT(ret);
 }
 
+/*
+ * efi_net_stop() - stop the network interface
+ *
+ * This function implements the Stop service of the
+ * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
+ * (UEFI) specification for details.
+ *
+ * @this:	pointer to the protocol instance
+ * Return:	status code
+ */
 static efi_status_t EFIAPI efi_net_stop(struct efi_simple_network *this)
 {
+	efi_status_t ret = EFI_SUCCESS;
+
 	EFI_ENTRY("%p", this);
 
-	return EFI_EXIT(EFI_SUCCESS);
+	/* Check parameters */
+	if (!this) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	if (this->mode->state == EFI_NETWORK_STOPPED)
+		ret = EFI_NOT_STARTED;
+	else
+		this->mode->state = EFI_NETWORK_STOPPED;
+out:
+	return EFI_EXIT(ret);
 }
 
 /*
- * Initialize network adapter and allocate transmit and receive buffers.
+ * efi_net_initialize() - initialize the network interface
  *
  * This function implements the Initialize service of the
  * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
@@ -67,7 +113,7 @@ static efi_status_t EFIAPI efi_net_stop(struct efi_simple_network *this)
  * @this:	pointer to the protocol instance
  * @extra_rx:	extra receive buffer to be allocated
  * @extra_tx:	extra transmit buffer to be allocated
- * @return:	status code
+ * Return:	status code
  */
 static efi_status_t EFIAPI efi_net_initialize(struct efi_simple_network *this,
 					      ulong extra_rx, ulong extra_tx)
@@ -77,9 +123,10 @@ static efi_status_t EFIAPI efi_net_initialize(struct efi_simple_network *this,
 
 	EFI_ENTRY("%p, %lx, %lx", this, extra_rx, extra_tx);
 
+	/* Check parameters */
 	if (!this) {
 		r = EFI_INVALID_PARAMETER;
-		goto error;
+		goto out;
 	}
 
 	/* Setup packet buffers */
@@ -92,32 +139,83 @@ static efi_status_t EFIAPI efi_net_initialize(struct efi_simple_network *this,
 	ret = eth_init();
 	if (ret < 0) {
 		eth_halt();
+		this->mode->state = EFI_NETWORK_STOPPED;
 		r = EFI_DEVICE_ERROR;
+		goto out;
+	} else {
+		this->mode->state = EFI_NETWORK_INITIALIZED;
 	}
-
-error:
+out:
 	return EFI_EXIT(r);
 }
 
+/*
+ * efi_net_reset() - reinitialize the network interface
+ *
+ * This function implements the Reset service of the
+ * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
+ * (UEFI) specification for details.
+ *
+ * @this:			pointer to the protocol instance
+ * @extended_verification:	execute exhaustive verification
+ * Return:			status code
+ */
 static efi_status_t EFIAPI efi_net_reset(struct efi_simple_network *this,
 					 int extended_verification)
 {
 	EFI_ENTRY("%p, %x", this, extended_verification);
 
-	return EFI_EXIT(EFI_SUCCESS);
+	return EFI_EXIT(EFI_CALL(efi_net_initialize(this, 0, 0)));
 }
 
+/*
+ * efi_net_shutdown() - shut down the network interface
+ *
+ * This function implements the Shutdown service of the
+ * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
+ * (UEFI) specification for details.
+ *
+ * @this:	pointer to the protocol instance
+ * Return:	status code
+ */
 static efi_status_t EFIAPI efi_net_shutdown(struct efi_simple_network *this)
 {
+	efi_status_t ret = EFI_SUCCESS;
+
 	EFI_ENTRY("%p", this);
 
-	return EFI_EXIT(EFI_SUCCESS);
+	/* Check parameters */
+	if (!this) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	eth_halt();
+	this->mode->state = EFI_NETWORK_STOPPED;
+
+out:
+	return EFI_EXIT(ret);
 }
 
-static efi_status_t EFIAPI efi_net_receive_filters(
-		struct efi_simple_network *this, u32 enable, u32 disable,
-		int reset_mcast_filter, ulong mcast_filter_count,
-		struct efi_mac_address *mcast_filter)
+/*
+ * efi_net_receive_filters() - mange multicast receive filters
+ *
+ * This function implements the ReceiveFilters service of the
+ * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
+ * (UEFI) specification for details.
+ *
+ * @this:		pointer to the protocol instance
+ * @enable:		bit mask of receive filters to enable
+ * @disable:		bit mask of receive filters to disable
+ * @reset_mcast_filter:	true resets contents of the filters
+ * @mcast_filter_count:	number of hardware MAC addresses in the new filters list
+ * @mcast_filter:	list of new filters
+ * Return:		status code
+ */
+static efi_status_t EFIAPI efi_net_receive_filters
+		(struct efi_simple_network *this, u32 enable, u32 disable,
+		 int reset_mcast_filter, ulong mcast_filter_count,
+		 struct efi_mac_address *mcast_filter)
 {
 	EFI_ENTRY("%p, %x, %x, %x, %lx, %p", this, enable, disable,
 		  reset_mcast_filter, mcast_filter_count, mcast_filter);
@@ -125,15 +223,40 @@ static efi_status_t EFIAPI efi_net_receive_filters(
 	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
-static efi_status_t EFIAPI efi_net_station_address(
-		struct efi_simple_network *this, int reset,
-		struct efi_mac_address *new_mac)
+/*
+ * efi_net_station_address() - set the hardware MAC address
+ *
+ * This function implements the StationAddress service of the
+ * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
+ * (UEFI) specification for details.
+ *
+ * @this:	pointer to the protocol instance
+ * @reset:	if true reset the address to default
+ * @new_mac:	new MAC address
+ * Return:	status code
+ */
+static efi_status_t EFIAPI efi_net_station_address
+		(struct efi_simple_network *this, int reset,
+		 struct efi_mac_address *new_mac)
 {
 	EFI_ENTRY("%p, %x, %p", this, reset, new_mac);
 
 	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
+/*
+ * efi_net_statistics() - reset or collect statistics of the network interface
+ *
+ * This function implements the Statistics service of the
+ * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
+ * (UEFI) specification for details.
+ *
+ * @this:	pointer to the protocol instance
+ * @reset:	if true, the statistics are reset
+ * @stat_size:	size of the statistics table
+ * @stat_table:	table to receive the statistics
+ * Return:	status code
+ */
 static efi_status_t EFIAPI efi_net_statistics(struct efi_simple_network *this,
 					      int reset, ulong *stat_size,
 					      void *stat_table)
@@ -143,6 +266,19 @@ static efi_status_t EFIAPI efi_net_statistics(struct efi_simple_network *this,
 	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
+/*
+ * efi_net_mcastiptomac() - translate multicast IP address to MAC address
+ *
+ * This function implements the Statistics service of the
+ * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface
+ * (UEFI) specification for details.
+ *
+ * @this:	pointer to the protocol instance
+ * @ipv6:	true if the IP address is an IPv6 address
+ * @ip:		IP address
+ * @mac:	MAC address
+ * Return:	status code
+ */
 static efi_status_t EFIAPI efi_net_mcastiptomac(struct efi_simple_network *this,
 						int ipv6,
 						struct efi_ip_address *ip,
@@ -153,6 +289,19 @@ static efi_status_t EFIAPI efi_net_mcastiptomac(struct efi_simple_network *this,
 	return EFI_EXIT(EFI_INVALID_PARAMETER);
 }
 
+/**
+ * efi_net_nvdata() - read or write NVRAM
+ *
+ * This function implements the GetStatus service of the Simple Network
+ * Protocol. See the UEFI spec for details.
+ *
+ * @this:		the instance of the Simple Network Protocol
+ * @readwrite:		true for read, false for write
+ * @offset:		offset in NVRAM
+ * @buffer_size:	size of buffer
+ * @buffer:		buffer
+ * Return:		status code
+ */
 static efi_status_t EFIAPI efi_net_nvdata(struct efi_simple_network *this,
 					  int read_write, ulong offset,
 					  ulong buffer_size, char *buffer)
@@ -163,13 +312,42 @@ static efi_status_t EFIAPI efi_net_nvdata(struct efi_simple_network *this,
 	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
+/**
+ * efi_net_get_status() - get interrupt status
+ *
+ * This function implements the GetStatus service of the Simple Network
+ * Protocol. See the UEFI spec for details.
+ *
+ * @this:		the instance of the Simple Network Protocol
+ * @int_status:		interface status
+ * @txbuf:		transmission buffer
+ */
 static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this,
 					      u32 *int_status, void **txbuf)
 {
+	efi_status_t ret = EFI_SUCCESS;
+
 	EFI_ENTRY("%p, %p, %p", this, int_status, txbuf);
 
 	efi_timer_check();
 
+	/* Check parameters */
+	if (!this) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	switch (this->mode->state) {
+	case EFI_NETWORK_STOPPED:
+		ret = EFI_NOT_STARTED;
+		goto out;
+	case EFI_NETWORK_STARTED:
+		ret = EFI_DEVICE_ERROR;
+		goto out;
+	default:
+		break;
+	}
+
 	if (int_status) {
 		/* We send packets synchronously, so nothing is outstanding */
 		*int_status = EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
@@ -180,63 +358,103 @@ static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this,
 		*txbuf = new_tx_packet;
 
 	new_tx_packet = NULL;
-
-	return EFI_EXIT(EFI_SUCCESS);
+out:
+	return EFI_EXIT(ret);
 }
 
-static efi_status_t EFIAPI efi_net_transmit(struct efi_simple_network *this,
-		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_net_transmit() - transmit a packet
+ *
+ * This function implements the Transmit 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 hardware MAC address
+ * @dest_addr:		destination hardware MAC address
+ * @protocol:		type of header to build
+ * Return:		status code
+ */
+static efi_status_t EFIAPI efi_net_transmit
+		(struct efi_simple_network *this, 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 ret = EFI_SUCCESS;
+
 	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();
 
+	/* Check parameters */
+	if (!this) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	/* We do not support jumbo packets */
+	if (buffer_size > PKTSIZE_ALIGN) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
 	if (header_size) {
-		/* We would need to create the header if header_size != 0 */
-		return EFI_EXIT(EFI_INVALID_PARAMETER);
+		/*
+		 * TODO: We would need to create the header
+		 * if header_size != 0
+		 */
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	switch (this->mode->state) {
+	case EFI_NETWORK_STOPPED:
+		ret = EFI_NOT_STARTED;
+		goto out;
+	case EFI_NETWORK_STARTED:
+		ret = EFI_DEVICE_ERROR;
+		goto out;
+	default:
+		break;
 	}
 
-	/* Ensure that the packet fits into the buffer */
-	if (buffer_size > PKTSIZE_ALIGN)
-		return EFI_EXIT(EFI_INVALID_PARAMETER);
+	/* Ethernet packets always fit, just bounce */
 	memcpy(transmit_buffer, buffer, buffer_size);
 	net_send_packet(transmit_buffer, buffer_size);
 
 	new_tx_packet = buffer;
 
-	return EFI_EXIT(EFI_SUCCESS);
-}
-
-static void efi_net_push(void *pkt, int len)
-{
-	new_rx_packet = true;
-	wait_for_packet->is_signaled = true;
+out:
+	return EFI_EXIT(ret);
 }
 
-/*
- * Receive a packet from a network interface.
+/**
+ * efi_net_receive() - 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
+ * @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,
-		size_t *header_size, size_t *buffer_size, void *buffer,
-		struct efi_mac_address *src_addr,
-		struct efi_mac_address *dest_addr, u16 *protocol)
+static efi_status_t EFIAPI efi_net_receive
+		(struct efi_simple_network *this, 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 ret = EFI_SUCCESS;
 	struct ethernet_hdr *eth_hdr;
 	size_t hdr_size = sizeof(struct ethernet_hdr);
 	u16 protlen;
@@ -244,14 +462,35 @@ 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);
 
+	/* Execute events */
 	efi_timer_check();
 
-	if (!new_rx_packet)
-		return EFI_EXIT(EFI_NOT_READY);
+	/* Check parameters */
+	if (!this) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	switch (this->mode->state) {
+	case EFI_NETWORK_STOPPED:
+		ret = EFI_NOT_STARTED;
+		goto out;
+	case EFI_NETWORK_STARTED:
+		ret = EFI_DEVICE_ERROR;
+		goto out;
+	default:
+		break;
+	}
+
+	if (!new_rx_packet) {
+		ret = EFI_NOT_READY;
+		goto out;
+	}
 	/* 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);
+		ret = EFI_NOT_READY;
+		goto out;
 	}
 	/* Fill export parameters */
 	eth_hdr = (struct ethernet_hdr *)net_rx_packet;
@@ -271,16 +510,22 @@ static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this,
 	if (*buffer_size < net_rx_packet_len) {
 		/* Packet doesn't fit, try again with bigger buffer */
 		*buffer_size = net_rx_packet_len;
-		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+		ret = EFI_BUFFER_TOO_SMALL;
+		goto out;
 	}
 	/* Copy packet */
 	memcpy(buffer, net_rx_packet, net_rx_packet_len);
 	*buffer_size = net_rx_packet_len;
 	new_rx_packet = false;
-
-	return EFI_EXIT(EFI_SUCCESS);
+out:
+	return EFI_EXIT(ret);
 }
 
+/**
+ * efi_net_set_dhcp_ack() - take note of a selected DHCP IP address
+ *
+ * This function is called by dhcp_handler().
+ */
 void efi_net_set_dhcp_ack(void *pkt, int len)
 {
 	int maxsize = sizeof(*dhcp_ack);
@@ -291,8 +536,22 @@ 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.
+/**
+ * efi_net_push() - callback for received network packet
+ *
+ * This function is called when a network packet is received by eth_rx().
+ *
+ * @pkt:	network packet
+ * @len:	length
+ */
+static void efi_net_push(void *pkt, int len)
+{
+	new_rx_packet = true;
+	wait_for_packet->is_signaled = true;
+}
+
+/**
+ * efi_network_timer_notify() - check if a new network packet has been received
  *
  * This notification function is called in every timer cycle.
  *
@@ -302,20 +561,34 @@ void efi_net_set_dhcp_ack(void *pkt, int len)
 static void EFIAPI efi_network_timer_notify(struct efi_event *event,
 					    void *context)
 {
+	struct efi_simple_network *this = (struct efi_simple_network *)context;
+
 	EFI_ENTRY("%p, %p", event, context);
 
+	/*
+	 * Some network drivers do not support calling eth_rx() before
+	 * initialization.
+	 */
+	if (!this || this->mode->state != EFI_NETWORK_INITIALIZED)
+		goto out;
+
 	if (!new_rx_packet) {
 		push_packet = efi_net_push;
 		eth_rx();
 		push_packet = NULL;
 	}
+out:
 	EFI_EXIT(EFI_SUCCESS);
 }
 
-/* This gets called from do_bootefi_exec(). */
+/**
+ * efi_net_register() - register the simple network protocol
+ *
+ * This gets called from do_bootefi_exec().
+ */
 efi_status_t efi_net_register(void)
 {
-	struct efi_net_obj *netobj;
+	struct efi_net_obj *netobj = NULL;
 	efi_status_t r;
 
 	if (!eth_get_dev()) {
@@ -396,7 +669,7 @@ efi_status_t efi_net_register(void)
 	 * iPXE is running at TPL_CALLBACK most of the time. Use a higher TPL.
 	 */
 	r = efi_create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, TPL_NOTIFY,
-			     efi_network_timer_notify, NULL, NULL,
+			     efi_network_timer_notify, &netobj->net, NULL,
 			     &network_timer_event);
 	if (r != EFI_SUCCESS) {
 		printf("ERROR: Failed to register network event\n");
-- 
2.19.0



More information about the U-Boot mailing list