[block] Replace gPXE block-device API with an iPXE asynchronous interface

The block device interface used in gPXE predates the invention of even
the old gPXE data-transfer interface, let alone the current iPXE
generic asynchronous interface mechanism.  Bring this old code up to
date, with the following benefits:

 o  Block device commands can be cancelled by the requestor.  The INT 13
    layer uses this to provide a global timeout on all INT 13 calls,
    with the result that an unexpected passive failure mode (such as
    an iSCSI target ACKing the request but never sending a response)
    will lead to a timeout that gets reported back to the INT 13 user,
    rather than simply freezing the system.

 o  INT 13,00 (reset drive) is now able to reset the underlying block
    device.  INT 13 users, such as DOS, that use INT 13,00 as a method
    for error recovery now have a chance of recovering.

 o  All block device commands are tagged, with a numerical tag that
    will show up in debugging output and in packet captures; this will
    allow easier interpretation of bug reports that include both
    sources of information.

 o  The extremely ugly hacks used to generate the boot firmware tables
    have been eradicated and replaced with a generic acpi_describe()
    method (exploiting the ability of iPXE interfaces to pass through
    methods to an underlying interface).  The ACPI tables are now
    built in a shared data block within .bss16, rather than each
    requiring dedicated space in .data16.

 o  The architecture-independent concept of a SAN device has been
    exposed to the iPXE core through the sanboot API, which provides
    calls to hook, unhook, boot, and describe SAN devices.  This
    allows for much more flexible usage patterns (such as hooking an
    empty SAN device and then running an OS installer via TFTP).

Signed-off-by: Michael Brown <mcb30@ipxe.org>

3 files changed, 1393 insertions, 531 deletions

diff --git a/src/net/aoe.c b/src/net/aoe.c
index 5e1a3b5bd..fec8d33f0 100644
--- a/src/net/aoe.c
+++ b/src/net/aoe.c
@@ -27,13 +27,15 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <byteswap.h>
 #include <ipxe/list.h>
 #include <ipxe/if_ether.h>
-#include <ipxe/ethernet.h>
 #include <ipxe/iobuf.h>
 #include <ipxe/uaccess.h>
-#include <ipxe/ata.h>
 #include <ipxe/netdevice.h>
-#include <ipxe/process.h>
 #include <ipxe/features.h>
+#include <ipxe/interface.h>
+#include <ipxe/xfer.h>
+#include <ipxe/uri.h>
+#include <ipxe/open.h>
+#include <ipxe/ata.h>
 #include <ipxe/aoe.h>
 
 /** @file
@@ -46,140 +48,302 @@ FEATURE ( FEATURE_PROTOCOL, "AoE", DHCP_EB_FEATURE_AOE, 1 );
 
 struct net_protocol aoe_protocol __net_protocol;
 
-/** List of all AoE sessions */
-static LIST_HEAD ( aoe_sessions );
+/******************************************************************************
+ *
+ * AoE devices and commands
+ *
+ ******************************************************************************
+ */
+
+/** List of all AoE devices */
+static LIST_HEAD ( aoe_devices );
+
+/** List of active AoE commands */
+static LIST_HEAD ( aoe_commands );
+
+/** An AoE device */
+struct aoe_device {
+	/** Reference counter */
+	struct refcnt refcnt;
+
+	/** Network device */
+	struct net_device *netdev;
+	/** ATA command issuing interface */
+	struct interface ata;
+
+	/** Major number */
+	uint16_t major;
+	/** Minor number */
+	uint8_t minor;
+	/** Target MAC address */
+	uint8_t target[MAX_LL_ADDR_LEN];
 
-static void aoe_free ( struct refcnt *refcnt ) {
-	struct aoe_session *aoe =
-		container_of ( refcnt, struct aoe_session, refcnt );
+	/** Saved timeout value */
+	unsigned long timeout;
+
+	/** Configuration command interface */
+	struct interface config;
+	/** Device is configued */
+	int configured;
+};
+
+/** An AoE command */
+struct aoe_command {
+	/** Reference count */
+	struct refcnt refcnt;
+	/** AOE device */
+	struct aoe_device *aoedev;
+	/** List of active commands */
+	struct list_head list;
+
+	/** ATA command interface */
+	struct interface ata;
+
+	/** ATA command */
+	struct ata_cmd command;
+	/** Command type */
+	struct aoe_command_type *type;
+	/** Command tag */
+	uint32_t tag;
+
+	/** Retransmission timer */
+	struct retry_timer timer;
+};
+
+/** An AoE command type */
+struct aoe_command_type {
+	/**
+	 * Calculate length of AoE command IU
+	 *
+	 * @v aoecmd		AoE command
+	 * @ret len		Length of command IU
+	 */
+	size_t ( * cmd_len ) ( struct aoe_command *aoecmd );
+	/**
+	 * Build AoE command IU
+	 *
+	 * @v aoecmd		AoE command
+	 * @v data		Command IU
+	 * @v len		Length of command IU
+	 */
+	void ( * cmd ) ( struct aoe_command *aoecmd, void *data, size_t len );
+	/**
+	 * Handle AoE response IU
+	 *
+	 * @v aoecmd		AoE command
+	 * @v data		Response IU
+	 * @v len		Length of response IU
+	 * @v ll_source		Link-layer source address
+	 * @ret rc		Return status code
+	 */
+	int ( * rsp ) ( struct aoe_command *aoecmd, const void *data,
+			size_t len, const void *ll_source );
+};
 
-	netdev_put ( aoe->netdev );
-	free ( aoe );
+/**
+ * Get reference to AoE device
+ *
+ * @v aoedev		AoE device
+ * @ret aoedev		AoE device
+ */
+static inline __attribute__ (( always_inline )) struct aoe_device *
+aoedev_get ( struct aoe_device *aoedev ) {
+	ref_get ( &aoedev->refcnt );
+	return aoedev;
 }
 
 /**
- * Mark current AoE command complete
+ * Drop reference to AoE device
  *
- * @v aoe		AoE session
- * @v rc		Return status code
+ * @v aoedev		AoE device
  */
-static void aoe_done ( struct aoe_session *aoe, int rc ) {
+static inline __attribute__ (( always_inline )) void
+aoedev_put ( struct aoe_device *aoedev ) {
+	ref_put ( &aoedev->refcnt );
+}
 
-	/* Record overall command status */
-	if ( aoe->command ) {
-		aoe->command->cb.cmd_stat = aoe->status;
-		aoe->command->rc = rc;
-		aoe->command = NULL;
-	}
+/**
+ * Get reference to AoE command
+ *
+ * @v aoecmd		AoE command
+ * @ret aoecmd		AoE command
+ */
+static inline __attribute__ (( always_inline )) struct aoe_command *
+aoecmd_get ( struct aoe_command *aoecmd ) {
+	ref_get ( &aoecmd->refcnt );
+	return aoecmd;
+}
 
-	/* Stop retransmission timer */
-	stop_timer ( &aoe->timer );
+/**
+ * Drop reference to AoE command
+ *
+ * @v aoecmd		AoE command
+ */
+static inline __attribute__ (( always_inline )) void
+aoecmd_put ( struct aoe_command *aoecmd ) {
+	ref_put ( &aoecmd->refcnt );
+}
 
-	/* Mark operation as complete */
-	aoe->rc = rc;
+/**
+ * Name AoE device
+ *
+ * @v aoedev		AoE device
+ * @ret name		AoE device name
+ */
+static const char * aoedev_name ( struct aoe_device *aoedev ) {
+	static char buf[16];
+
+	snprintf ( buf, sizeof ( buf ), "%s/e%d.%d", aoedev->netdev->name,
+		   aoedev->major, aoedev->minor );
+	return buf;
 }
 
 /**
- * Send AoE command
+ * Free AoE command
  *
- * @v aoe		AoE session
- * @ret rc		Return status code
+ * @v refcnt		Reference counter
+ */
+static void aoecmd_free ( struct refcnt *refcnt ) {
+	struct aoe_command *aoecmd =
+		container_of ( refcnt, struct aoe_command, refcnt );
+
+	assert ( ! timer_running ( &aoecmd->timer ) );
+	assert ( list_empty ( &aoecmd->list ) );
+
+	aoedev_put ( aoecmd->aoedev );
+	free ( aoecmd );
+}
+
+/**
+ * Close AoE command
  *
- * This transmits an AoE command packet.  It does not wait for a
- * response.
+ * @v aoecmd		AoE command
+ * @v rc		Reason for close
  */
-static int aoe_send_command ( struct aoe_session *aoe ) {
-	struct ata_command *command = aoe->command;
+static void aoecmd_close ( struct aoe_command *aoecmd, int rc ) {
+	struct aoe_device *aoedev = aoecmd->aoedev;
+
+	/* Stop timer */
+	stop_timer ( &aoecmd->timer );
+
+	/* Preserve the timeout value for subsequent commands */
+	aoedev->timeout = aoecmd->timer.timeout;
+
+	/* Remove from list of commands */
+	if ( ! list_empty ( &aoecmd->list ) ) {
+		list_del ( &aoecmd->list );
+		INIT_LIST_HEAD ( &aoecmd->list );
+		aoecmd_put ( aoecmd );
+	}
+
+	/* Shut down interfaces */
+	intf_shutdown ( &aoecmd->ata, rc );
+}
+
+/**
+ * Transmit AoE command request
+ *
+ * @v aoecmd		AoE command
+ * @ret rc		Return status code
+ */
+static int aoecmd_tx ( struct aoe_command *aoecmd ) {
+	struct aoe_device *aoedev = aoecmd->aoedev;
 	struct io_buffer *iobuf;
 	struct aoehdr *aoehdr;
-	union aoecmd *aoecmd;
-	struct aoeata *aoeata;
-	unsigned int count;
-	unsigned int data_out_len;
-	unsigned int aoecmdlen;
-
-	/* Fail immediately if we have no netdev to send on */
-	if ( ! aoe->netdev ) {
-		aoe_done ( aoe, -ENETUNREACH );
-		return -ENETUNREACH;
-	}
+	size_t cmd_len;
+	int rc;
+
+	/* Sanity check */
+	assert ( aoedev->netdev != NULL );
 
 	/* If we are transmitting anything that requires a response,
          * start the retransmission timer.  Do this before attempting
          * to allocate the I/O buffer, in case allocation itself
          * fails.
          */
-	start_timer ( &aoe->timer );
-
-	/* Calculate count and data_out_len for this subcommand */
-	switch ( aoe->aoe_cmd_type ) {
-	case AOE_CMD_ATA:
-		count = command->cb.count.native;
-		if ( count > AOE_MAX_COUNT )
-			count = AOE_MAX_COUNT;
-		data_out_len = ( command->data_out ?
-				 ( count * ATA_SECTOR_SIZE ) : 0 );
-		aoecmdlen = sizeof ( aoecmd->ata );
-		break;
-	case AOE_CMD_CONFIG:
-		count = 0;
-		data_out_len = 0;
-		aoecmdlen = sizeof ( aoecmd->cfg );
-		break;
-	default:
-		return -ENOTSUP;
-	}
+	start_timer ( &aoecmd->timer );
 
 	/* Create outgoing I/O buffer */
-	iobuf = alloc_iob ( ETH_HLEN + sizeof ( *aoehdr ) +
-			    aoecmdlen + data_out_len );
-
+	cmd_len = aoecmd->type->cmd_len ( aoecmd );
+	iobuf = alloc_iob ( MAX_LL_HEADER_LEN + cmd_len );
 	if ( ! iobuf )
 		return -ENOMEM;
-	iob_reserve ( iobuf, ETH_HLEN );
-	aoehdr = iob_put ( iobuf, sizeof ( *aoehdr ) );
-	aoecmd = iob_put ( iobuf, aoecmdlen );
-	memset ( aoehdr, 0, ( sizeof ( *aoehdr ) + aoecmdlen ) );
+	iob_reserve ( iobuf, MAX_LL_HEADER_LEN );
+	aoehdr = iob_put ( iobuf, cmd_len );
 
 	/* Fill AoE header */
+	memset ( aoehdr, 0, sizeof ( *aoehdr ) );
 	aoehdr->ver_flags = AOE_VERSION;
-	aoehdr->major = htons ( aoe->major );
-	aoehdr->minor = aoe->minor;
-	aoehdr->command = aoe->aoe_cmd_type;
-	aoehdr->tag = htonl ( ++aoe->tag );
-
-	/* Fill AoE payload */
-	switch ( aoe->aoe_cmd_type ) {
-	case AOE_CMD_ATA:
-		/* Fill AoE command */
-		aoeata = &aoecmd->ata;
-		linker_assert ( AOE_FL_DEV_HEAD	== ATA_DEV_SLAVE,
-				__fix_ata_h__ );
-		aoeata->aflags = ( ( command->cb.lba48 ? AOE_FL_EXTENDED : 0 )|
-				   ( command->cb.device & ATA_DEV_SLAVE ) |
-				   ( data_out_len ? AOE_FL_WRITE : 0 ) );
-		aoeata->err_feat = command->cb.err_feat.bytes.cur;
-		aoeata->count = count;
-		aoeata->cmd_stat = command->cb.cmd_stat;
-		aoeata->lba.u64 = cpu_to_le64 ( command->cb.lba.native );
-		if ( ! command->cb.lba48 )
-			aoeata->lba.bytes[3] |=
-				( command->cb.device & ATA_DEV_MASK );
-
-		/* Fill data payload */
-		copy_from_user ( iob_put ( iobuf, data_out_len ),
-				 command->data_out, aoe->command_offset,
-				 data_out_len );
-		break;
-	case AOE_CMD_CONFIG:
-		/* Nothing to do */
-		break;
-	default:
-		assert ( 0 );
-	}
+	aoehdr->major = htons ( aoedev->major );
+	aoehdr->minor = aoedev->minor;
+	aoehdr->tag = htonl ( aoecmd->tag );
+	aoecmd->type->cmd ( aoecmd, iobuf->data, iob_len ( iobuf ) );
 
 	/* Send packet */
-	return net_tx ( iobuf, aoe->netdev, &aoe_protocol, aoe->target );
+	if ( ( rc = net_tx ( iobuf, aoedev->netdev, &aoe_protocol,
+			     aoedev->target ) ) != 0 ) {
+		DBGC ( aoedev, "AoE %s/%08x could not transmit: %s\n",
+		       aoedev_name ( aoedev ), aoecmd->tag,
+		       strerror ( rc ) );
+		return rc;
+	}
+
+	return 0;
+}
+
+/**
+ * Receive AoE command response
+ *
+ * @v aoecmd		AoE command
+ * @v iobuf		I/O buffer
+ * @v ll_source		Link-layer source address
+ * @ret rc		Return status code
+ */
+static int aoecmd_rx ( struct aoe_command *aoecmd, struct io_buffer *iobuf,
+		       const void *ll_source ) {
+	struct aoe_device *aoedev = aoecmd->aoedev;
+	struct aoehdr *aoehdr = iobuf->data;
+	int rc;
+
+	/* Sanity check */
+	if ( iob_len ( iobuf ) < sizeof ( *aoehdr ) ) {
+		DBGC ( aoedev, "AoE %s/%08x received underlength response "
+		       "(%zd bytes)\n", aoedev_name ( aoedev ),
+		       aoecmd->tag, iob_len ( iobuf ) );
+		rc = -EINVAL;
+		goto done;
+	}
+	if ( ( ntohs ( aoehdr->major ) != aoedev->major ) ||
+	     ( aoehdr->minor != aoedev->minor ) ) {
+		DBGC ( aoedev, "AoE %s/%08x received response for incorrect "
+		       "device e%d.%d\n", aoedev_name ( aoedev ), aoecmd->tag,
+		       ntohs ( aoehdr->major ), aoehdr->minor );
+		rc = -EINVAL;
+		goto done;
+	}
+
+	/* Catch command failures */
+	if ( aoehdr->ver_flags & AOE_FL_ERROR ) {
+		DBGC ( aoedev, "AoE %s/%08x terminated in error\n",
+		       aoedev_name ( aoedev ), aoecmd->tag );
+		aoecmd_close ( aoecmd, -EIO );
+		rc = -EIO;
+		goto done;
+	}
+
+	/* Hand off to command completion handler */
+	if ( ( rc = aoecmd->type->rsp ( aoecmd, iobuf->data, iob_len ( iobuf ),
+					ll_source ) ) != 0 )
+		goto done;
+
+ done:
+	/* Free I/O buffer */
+	free_iob ( iobuf );
+
+	/* Terminate command */
+	aoecmd_close ( aoecmd, rc );
+
+	return rc;
 }
 
 /**
@@ -188,94 +352,531 @@ static int aoe_send_command ( struct aoe_session *aoe ) {
  * @v timer		AoE retry timer
  * @v fail		Failure indicator
  */
-static void aoe_timer_expired ( struct retry_timer *timer, int fail ) {
-	struct aoe_session *aoe =
-		container_of ( timer, struct aoe_session, timer );
+static void aoecmd_expired ( struct retry_timer *timer, int fail ) {
+	struct aoe_command *aoecmd =
+		container_of ( timer, struct aoe_command, timer );
 
 	if ( fail ) {
-		aoe_done ( aoe, -ETIMEDOUT );
+		aoecmd_close ( aoecmd, -ETIMEDOUT );
 	} else {
-		aoe_send_command ( aoe );
+		aoecmd_tx ( aoecmd );
 	}
 }
 
 /**
- * Handle AoE configuration command response
+ * Calculate length of AoE ATA command IU
+ *
+ * @v aoecmd		AoE command
+ * @ret len		Length of command IU
+ */
+static size_t aoecmd_ata_cmd_len ( struct aoe_command *aoecmd ) {
+	struct ata_cmd *command = &aoecmd->command;
+
+	return ( sizeof ( struct aoehdr ) + sizeof ( struct aoeata ) +
+		 command->data_out_len );
+}
+
+/**
+ * Build AoE ATA command IU
+ *
+ * @v aoecmd		AoE command
+ * @v data		Command IU
+ * @v len		Length of command IU
+ */
+static void aoecmd_ata_cmd ( struct aoe_command *aoecmd,
+			     void *data, size_t len ) {
+	struct aoe_device *aoedev = aoecmd->aoedev;
+	struct ata_cmd *command = &aoecmd->command;
+	struct aoehdr *aoehdr = data;
+	struct aoeata *aoeata = &aoehdr->payload[0].ata;
+
+	/* Sanity check */
+	linker_assert ( AOE_FL_DEV_HEAD	== ATA_DEV_SLAVE, __fix_ata_h__ );
+	assert ( len == ( sizeof ( *aoehdr ) + sizeof ( *aoeata ) +
+			  command->data_out_len ) );
+
+	/* Build IU */
+	aoehdr->command = AOE_CMD_ATA;
+	memset ( aoeata, 0, sizeof ( *aoeata ) );
+	aoeata->aflags = ( ( command->cb.lba48 ? AOE_FL_EXTENDED : 0 ) |
+			   ( command->cb.device & ATA_DEV_SLAVE ) |
+			   ( command->data_out_len ? AOE_FL_WRITE : 0 ) );
+	aoeata->err_feat = command->cb.err_feat.bytes.cur;
+	aoeata->count = command->cb.count.native;
+	aoeata->cmd_stat = command->cb.cmd_stat;
+	aoeata->lba.u64 = cpu_to_le64 ( command->cb.lba.native );
+	if ( ! command->cb.lba48 )
+		aoeata->lba.bytes[3] |=
+			( command->cb.device & ATA_DEV_MASK );
+	copy_from_user ( aoeata->data, command->data_out, 0,
+			 command->data_out_len );
+
+	DBGC2 ( aoedev, "AoE %s/%08x ATA cmd %02x:%02x:%02x:%02x:%08llx",
+		aoedev_name ( aoedev ), aoecmd->tag, aoeata->aflags,
+		aoeata->err_feat, aoeata->count, aoeata->cmd_stat,
+		aoeata->lba.u64 );
+	if ( command->data_out_len )
+		DBGC2 ( aoedev, " out %04zx", command->data_out_len );
+	if ( command->data_in_len )
+		DBGC2 ( aoedev, " in %04zx", command->data_in_len );
+	DBGC2 ( aoedev, "\n" );
+}
+
+/**
+ * Handle AoE ATA response IU
  *
- * @v aoe		AoE session
+ * @v aoecmd		AoE command
+ * @v data		Response IU
+ * @v len		Length of response IU
  * @v ll_source		Link-layer source address
  * @ret rc		Return status code
  */
-static int aoe_rx_cfg ( struct aoe_session *aoe, const void *ll_source ) {
+static int aoecmd_ata_rsp ( struct aoe_command *aoecmd, const void *data,
+			    size_t len, const void *ll_source __unused ) {
+	struct aoe_device *aoedev = aoecmd->aoedev;
+	struct ata_cmd *command = &aoecmd->command;
+	const struct aoehdr *aoehdr = data;
+	const struct aoeata *aoeata = &aoehdr->payload[0].ata;
+	size_t data_len;
 
-	/* Record target MAC address */
-	memcpy ( aoe->target, ll_source, sizeof ( aoe->target ) );
-	DBGC ( aoe, "AoE %p target MAC address %s\n",
-	       aoe, eth_ntoa ( aoe->target ) );
+	/* Sanity check */
+	if ( len < ( sizeof ( *aoehdr ) + sizeof ( *aoeata ) ) ) {
+		DBGC ( aoedev, "AoE %s/%08x received underlength ATA response "
+		       "(%zd bytes)\n", aoedev_name ( aoedev ),
+		       aoecmd->tag, len );
+		return -EINVAL;
+	}
+	data_len = ( len - ( sizeof ( *aoehdr ) + sizeof ( *aoeata ) ) );
+	DBGC2 ( aoedev, "AoE %s/%08x ATA rsp %02x in %04zx\n",
+		aoedev_name ( aoedev ), aoecmd->tag, aoeata->cmd_stat,
+		data_len );
+
+	/* Check for command failure */
+	if ( aoeata->cmd_stat & ATA_STAT_ERR ) {
+		DBGC ( aoedev, "AoE %s/%08x status %02x\n",
+		       aoedev_name ( aoedev ), aoecmd->tag, aoeata->cmd_stat );
+		return -EIO;
+	}
 
-	/* Mark config request as complete */
-	aoe_done ( aoe, 0 );
+	/* Check data-in length is sufficient.  (There may be trailing
+	 * garbage due to Ethernet minimum-frame-size padding.)
+	 */
+	if ( data_len < command->data_in_len ) {
+		DBGC ( aoedev, "AoE %s/%08x data-in underrun (received %zd, "
+		       "expected %zd)\n", aoedev_name ( aoedev ), aoecmd->tag,
+		       data_len, command->data_in_len );
+		return -ERANGE;
+	}
+
+	/* Copy out data payload */
+	copy_to_user ( command->data_in, 0, aoeata->data,
+		       command->data_in_len );
 
 	return 0;
 }
 
+/** AoE ATA command */
+static struct aoe_command_type aoecmd_ata = {
+	.cmd_len = aoecmd_ata_cmd_len,
+	.cmd = aoecmd_ata_cmd,
+	.rsp = aoecmd_ata_rsp,
+};
+
+/**
+ * Calculate length of AoE configuration command IU
+ *
+ * @v aoecmd		AoE command
+ * @ret len		Length of command IU
+ */
+static size_t aoecmd_cfg_cmd_len ( struct aoe_command *aoecmd __unused ) {
+	return ( sizeof ( struct aoehdr ) + sizeof ( struct aoecfg ) );
+}
+
+/**
+ * Build AoE configuration command IU
+ *
+ * @v aoecmd		AoE command
+ * @v data		Command IU
+ * @v len		Length of command IU
+ */
+static void aoecmd_cfg_cmd ( struct aoe_command *aoecmd,
+			     void *data, size_t len ) {
+	struct aoe_device *aoedev = aoecmd->aoedev;
+	struct aoehdr *aoehdr = data;
+	struct aoecfg *aoecfg = &aoehdr->payload[0].cfg;
+
+	/* Sanity check */
+	assert ( len == ( sizeof ( *aoehdr ) + sizeof ( *aoecfg ) ) );
+
+	/* Build IU */
+	aoehdr->command = AOE_CMD_CONFIG;
+	memset ( aoecfg, 0, sizeof ( *aoecfg ) );
+
+	DBGC ( aoedev, "AoE %s/%08x CONFIG cmd\n",
+	       aoedev_name ( aoedev ), aoecmd->tag );
+}
+
 /**
- * Handle AoE ATA command response
+ * Handle AoE configuration response IU
  *
- * @v aoe		AoE session
- * @v aoeata		AoE ATA command
- * @v len		Length of AoE ATA command
+ * @v aoecmd		AoE command
+ * @v data		Response IU
+ * @v len		Length of response IU
+ * @v ll_source		Link-layer source address
  * @ret rc		Return status code
  */
-static int aoe_rx_ata ( struct aoe_session *aoe, struct aoeata *aoeata,
-			size_t len ) {
-	struct ata_command *command = aoe->command;
-	unsigned int rx_data_len;
-	unsigned int count;
-	unsigned int data_len;
+static int aoecmd_cfg_rsp ( struct aoe_command *aoecmd, const void *data,
+			    size_t len, const void *ll_source ) {
+	struct aoe_device *aoedev = aoecmd->aoedev;
+	struct ll_protocol *ll_protocol = aoedev->netdev->ll_protocol;
+	const struct aoehdr *aoehdr = data;
+	const struct aoecfg *aoecfg = &aoehdr->payload[0].cfg;
 
 	/* Sanity check */
-	if ( len < sizeof ( *aoeata ) ) {
-		/* Ignore packet; allow timer to trigger retransmit */
+	if ( len < ( sizeof ( *aoehdr ) + sizeof ( *aoecfg ) ) ) {
+		DBGC ( aoedev, "AoE %s/%08x received underlength "
+		       "configuration response (%zd bytes)\n",
+		       aoedev_name ( aoedev ), aoecmd->tag, len );
 		return -EINVAL;
 	}
-	rx_data_len = ( len - sizeof ( *aoeata ) );
-
-	/* Calculate count and data_len for this subcommand */
-	count = command->cb.count.native;
-	if ( count > AOE_MAX_COUNT )
-		count = AOE_MAX_COUNT;
-	data_len = count * ATA_SECTOR_SIZE;
-
-	/* Merge into overall ATA status */
-	aoe->status |= aoeata->cmd_stat;
-
-	/* Copy data payload */
-	if ( command->data_in ) {
-		if ( rx_data_len > data_len )
-			rx_data_len = data_len;
-		copy_to_user ( command->data_in, aoe->command_offset,
-			       aoeata->data, rx_data_len );
+	DBGC ( aoedev, "AoE %s/%08x CONFIG rsp buf %04x fw %04x scnt %02x\n",
+	       aoedev_name ( aoedev ), aoecmd->tag, ntohs ( aoecfg->bufcnt ),
+	       aoecfg->fwver, aoecfg->scnt );
+
+	/* Record target MAC address */
+	memcpy ( aoedev->target, ll_source, ll_protocol->ll_addr_len );
+	DBGC ( aoedev, "AoE %s has MAC address %s\n",
+	       aoedev_name ( aoedev ), ll_protocol->ntoa ( aoedev->target ) );
+
+	return 0;
+}
+
+/** AoE configuration command */
+static struct aoe_command_type aoecmd_cfg = {
+	.cmd_len = aoecmd_cfg_cmd_len,
+	.cmd = aoecmd_cfg_cmd,
+	.rsp = aoecmd_cfg_rsp,
+};
+
+/** AoE command ATA interface operations */
+static struct interface_operation aoecmd_ata_op[] = {
+	INTF_OP ( intf_close, struct aoe_command *, aoecmd_close ),
+};
+
+/** AoE command ATA interface descriptor */
+static struct interface_descriptor aoecmd_ata_desc =
+	INTF_DESC ( struct aoe_command, ata, aoecmd_ata_op );
+
+/**
+ * Identify AoE command by tag
+ *
+ * @v tag		Command tag
+ * @ret aoecmd		AoE command, or NULL
+ */
+static struct aoe_command * aoecmd_find_tag ( uint32_t tag ) {
+	struct aoe_command *aoecmd;
+
+	list_for_each_entry ( aoecmd, &aoe_commands, list ) {
+		if ( aoecmd->tag == tag )
+			return aoecmd;
 	}
+	return NULL;
+}
+
+/**
+ * Choose an AoE command tag
+ *
+ * @ret tag		New tag, or negative error
+ */
+static int aoecmd_new_tag ( void ) {
+	static uint16_t tag_idx;
+	unsigned int i;
+
+	for ( i = 0 ; i < 65536 ; i++ ) {
+		tag_idx++;
+		if ( aoecmd_find_tag ( tag_idx ) == NULL )
+			return ( AOE_TAG_MAGIC | tag_idx );
+	}
+	return -EADDRINUSE;
+}
+
+/**
+ * Create AoE command
+ *
+ * @v aoedev		AoE device
+ * @v type		AoE command type
+ * @ret aoecmd		AoE command
+ */
+static struct aoe_command * aoecmd_create ( struct aoe_device *aoedev,
+					    struct aoe_command_type *type ) {
+	struct aoe_command *aoecmd;
+	int tag;
+
+	/* Allocate command tag */
+	tag = aoecmd_new_tag();
+	if ( tag < 0 )
+		return NULL;
+
+	/* Allocate and initialise structure */
+	aoecmd = zalloc ( sizeof ( *aoecmd ) );
+	if ( ! aoecmd )
+		return NULL;
+	ref_init ( &aoecmd->refcnt, aoecmd_free );
+	list_add ( &aoecmd->list, &aoe_commands );
+	intf_init ( &aoecmd->ata, &aoecmd_ata_desc, &aoecmd->refcnt );
+	timer_init ( &aoecmd->timer, aoecmd_expired, &aoecmd->refcnt );
+	aoecmd->aoedev = aoedev_get ( aoedev );
+	aoecmd->type = type;
+	aoecmd->tag = tag;
+
+	/* Preserve timeout from last completed command */
+	aoecmd->timer.timeout = aoedev->timeout;
+
+	/* Return already mortalised.  (Reference is held by command list.) */
+	return aoecmd;
+}
+
+/**
+ * Issue AoE ATA command
+ *
+ * @v aoedev		AoE device
+ * @v parent		Parent interface
+ * @v command		ATA command
+ * @ret tag		Command tag, or negative error
+ */
+static int aoedev_ata_command ( struct aoe_device *aoedev,
+				struct interface *parent,
+				struct ata_cmd *command ) {
+	struct aoe_command *aoecmd;
+
+	/* Create command */
+	aoecmd = aoecmd_create ( aoedev, &aoecmd_ata );
+	if ( ! aoecmd )
+		return -ENOMEM;
+	memcpy ( &aoecmd->command, command, sizeof ( aoecmd->command ) );
+
+	/* Attempt to send command.  Allow failures to be handled by
+	 * the retry timer.
+	 */
+	aoecmd_tx ( aoecmd );
+
+	/* Attach to parent interface, leave reference with command
+	 * list, and return.
+	 */
+	intf_plug_plug ( &aoecmd->ata, parent );
+	return aoecmd->tag;
+}
+
+/**
+ * Issue AoE configuration command
+ *
+ * @v aoedev		AoE device
+ * @v parent		Parent interface
+ * @ret tag		Command tag, or negative error
+ */
+static int aoedev_cfg_command ( struct aoe_device *aoedev,
+				struct interface *parent ) {
+	struct aoe_command *aoecmd;
+
+	/* Create command */
+	aoecmd = aoecmd_create ( aoedev, &aoecmd_cfg );
+	if ( ! aoecmd )
+		return -ENOMEM;
+
+	/* Attempt to send command.  Allow failures to be handled by
+	 * the retry timer.
+	 */
+	aoecmd_tx ( aoecmd );
+
+	/* Attach to parent interface, leave reference with command
+	 * list, and return.
+	 */
+	intf_plug_plug ( &aoecmd->ata, parent );
+	return aoecmd->tag;
+}
+
+/**
+ * Free AoE device
+ *
+ * @v refcnt		Reference count
+ */
+static void aoedev_free ( struct refcnt *refcnt ) {
+	struct aoe_device *aoedev =
+		container_of ( refcnt, struct aoe_device, refcnt );
+
+	netdev_put ( aoedev->netdev );
+	free ( aoedev );
+}
+
+/**
+ * Close AoE device
+ *
+ * @v aoedev		AoE device
+ * @v rc		Reason for close
+ */
+static void aoedev_close ( struct aoe_device *aoedev, int rc ) {
+	struct aoe_command *aoecmd;
+	struct aoe_command *tmp;
+
+	/* Shut down interfaces */
+	intf_shutdown ( &aoedev->ata, rc );
+	intf_shutdown ( &aoedev->config, rc );
+
+	/* Shut down any active commands */
+	list_for_each_entry_safe ( aoecmd, tmp, &aoe_commands, list ) {
+		if ( aoecmd->aoedev != aoedev )
+			continue;
+		aoecmd_get ( aoecmd );
+		aoecmd_close ( aoecmd, rc );
+		aoecmd_put ( aoecmd );
+	}
+}
+
+/**
+ * Check AoE device flow-control window
+ *
+ * @v aoedev		AoE device
+ * @ret len		Length of window
+ */
+static size_t aoedev_window ( struct aoe_device *aoedev ) {
+	return ( aoedev->configured ? ~( ( size_t ) 0 ) : 0 );
+}
+
+/**
+ * Handle AoE device configuration completion
+ *
+ * @v aoedev		AoE device
+ * @v rc		Reason for completion
+ */
+static void aoedev_config_done ( struct aoe_device *aoedev, int rc ) {
+
+	/* Shut down interface */
+	intf_shutdown ( &aoedev->config, rc );
+
+	/* Close device on failure */
+	if ( rc != 0 ) {
+		aoedev_close ( aoedev, rc );
+		return;
+	}
+
+	/* Mark device as configured */
+	aoedev->configured = 1;
+	xfer_window_changed ( &aoedev->ata );
+}
+
+/**
+ * Describe AoE device in an ACPI table
+ *
+ * @v aoedev		AoE device
+ * @v acpi		ACPI table
+ * @v len		Length of ACPI table
+ * @ret rc		Return status code
+ */
+static int aoedev_describe ( struct aoe_device *aoedev,
+			     struct acpi_description_header *acpi,
+			     size_t len ) {
+	struct abft_table *abft =
+		container_of ( acpi, struct abft_table, acpi );
+
+	/* Sanity check */
+	if ( len < sizeof ( *abft ) )
+		return -ENOBUFS;
 
-	/* Update ATA command and offset */
-	aoe->command_offset += data_len;
-	command->cb.lba.native += count;
-	command->cb.count.native -= count;
+	/* Populate table */
+	abft->acpi.signature = cpu_to_le32 ( ABFT_SIG );
+	abft->acpi.length = cpu_to_le32 ( sizeof ( *abft ) );
+	abft->acpi.revision = 1;
+	abft->shelf = cpu_to_le16 ( aoedev->major );
+	abft->slot = aoedev->minor;
+	memcpy ( abft->mac, aoedev->netdev->ll_addr, sizeof ( abft->mac ) );
 
-	/* Check for operation complete */
-	if ( ! command->cb.count.native ) {
-		aoe_done ( aoe, 0 );
-		return 0;
+	return 0;
+}
+
+/** AoE device ATA interface operations */
+static struct interface_operation aoedev_ata_op[] = {
+	INTF_OP ( ata_command, struct aoe_device *, aoedev_ata_command ),
+	INTF_OP ( xfer_window, struct aoe_device *, aoedev_window ),
+	INTF_OP ( intf_close, struct aoe_device *, aoedev_close ),
+	INTF_OP ( acpi_describe, struct aoe_device *, aoedev_describe ),
+};
+
+/** AoE device ATA interface descriptor */
+static struct interface_descriptor aoedev_ata_desc =
+	INTF_DESC ( struct aoe_device, ata, aoedev_ata_op );
+
+/** AoE device configuration interface operations */
+static struct interface_operation aoedev_config_op[] = {
+	INTF_OP ( intf_close, struct aoe_device *, aoedev_config_done ),
+};
+
+/** AoE device configuration interface descriptor */
+static struct interface_descriptor aoedev_config_desc =
+	INTF_DESC ( struct aoe_device, config, aoedev_config_op );
+
+/**
+ * Open AoE device
+ *
+ * @v parent		Parent interface
+ * @v netdev		Network device
+ * @v major		Device major number
+ * @v minor		Device minor number
+ * @ret rc		Return status code
+ */
+static int aoedev_open ( struct interface *parent, struct net_device *netdev,
+			 unsigned int major, unsigned int minor ) {
+	struct aoe_device *aoedev;
+	int rc;
+
+	/* Allocate and initialise structure */
+	aoedev = zalloc ( sizeof ( *aoedev ) );
+	if ( ! aoedev ) {
+		rc = -ENOMEM;
+		goto err_zalloc;
+	}
+	ref_init ( &aoedev->refcnt, aoedev_free );
+	intf_init ( &aoedev->ata, &aoedev_ata_desc, &aoedev->refcnt );
+	intf_init ( &aoedev->config, &aoedev_config_desc, &aoedev->refcnt );
+	aoedev->netdev = netdev_get ( netdev );
+	aoedev->major = major;
+	aoedev->minor = minor;
+	memcpy ( aoedev->target, netdev->ll_broadcast,
+		 netdev->ll_protocol->ll_addr_len );
+
+	/* Initiate configuration */
+	if ( ( rc = aoedev_cfg_command ( aoedev, &aoedev->config ) ) < 0 ) {
+		DBGC ( aoedev, "AoE %s could not initiate configuration: %s\n",
+		       aoedev_name ( aoedev ), strerror ( rc ) );
+		goto err_config;
 	}
 
-	/* Transmit next portion of request */
-	stop_timer ( &aoe->timer );
-	aoe_send_command ( aoe );
+	/* Attach ATA device to parent interface */
+	if ( ( rc = ata_open ( parent, &aoedev->ata, ATA_DEV_MASTER,
+			       AOE_MAX_COUNT ) ) != 0 ) {
+		DBGC ( aoedev, "AoE %s could not create ATA device: %s\n",
+		       aoedev_name ( aoedev ), strerror ( rc ) );
+		goto err_ata_open;
+	}
 
+	/* Mortalise self and return */
+	ref_put ( &aoedev->refcnt );
 	return 0;
+
+ err_ata_open:
+ err_config:
+	aoedev_close ( aoedev, rc );
+	ref_put ( &aoedev->refcnt );
+ err_zalloc:
+	return rc;
 }
 
+/******************************************************************************
+ *
+ * AoE network protocol
+ *
+ ******************************************************************************
+ */
+
 /**
  * Process incoming AoE packets
  *
@@ -289,52 +890,47 @@ static int aoe_rx ( struct io_buffer *iobuf,
 		    struct net_device *netdev __unused,
 		    const void *ll_source ) {
 	struct aoehdr *aoehdr = iobuf->data;
-	struct aoe_session *aoe;
-	int rc = 0;
+	struct aoe_command *aoecmd;
+	int rc;
 
-	/* Sanity checks */
+	/* Sanity check */
 	if ( iob_len ( iobuf ) < sizeof ( *aoehdr ) ) {
+		DBG ( "AoE received underlength packet (%zd bytes)\n",
+		      iob_len ( iobuf ) );
 		rc = -EINVAL;
-		goto done;
+		goto err_sanity;
 	}
 	if ( ( aoehdr->ver_flags & AOE_VERSION_MASK ) != AOE_VERSION ) {
+		DBG ( "AoE received packet for unsupported protocol version "
+		      "%02x\n", ( aoehdr->ver_flags & AOE_VERSION_MASK ) );
 		rc = -EPROTONOSUPPORT;
-		goto done;
+		goto err_sanity;
 	}
 	if ( ! ( aoehdr->ver_flags & AOE_FL_RESPONSE ) ) {
-		/* Ignore AoE requests that we happen to see */
-		goto done;
+		DBG ( "AoE received request packet\n" );
+		rc = -EOPNOTSUPP;
+		goto err_sanity;
 	}
-	iob_pull ( iobuf, sizeof ( *aoehdr ) );
 
-	/* Demultiplex amongst active AoE sessions */
-	list_for_each_entry ( aoe, &aoe_sessions, list ) {
-		if ( ntohs ( aoehdr->major ) != aoe->major )
-			continue;
-		if ( aoehdr->minor != aoe->minor )
-			continue;
-		if ( ntohl ( aoehdr->tag ) != aoe->tag )
-			continue;
-		if ( aoehdr->ver_flags & AOE_FL_ERROR ) {
-			aoe_done ( aoe, -EIO );
-			break;
-		}
-		switch ( aoehdr->command ) {
-		case AOE_CMD_ATA:
-			rc = aoe_rx_ata ( aoe, iobuf->data, iob_len ( iobuf ));
-			break;
-		case AOE_CMD_CONFIG:
-			rc = aoe_rx_cfg ( aoe, ll_source );
-			break;
-		default:
-			DBGC ( aoe, "AoE %p ignoring command %02x\n",
-			       aoe, aoehdr->command );
-			break;
-		}
-		break;
+	/* Demultiplex amongst active AoE commands */
+	aoecmd = aoecmd_find_tag ( ntohl ( aoehdr->tag ) );
+	if ( ! aoecmd ) {
+		DBG ( "AoE received packet for unused tag %08x\n",
+		      ntohl ( aoehdr->tag ) );
+		rc = -ENOENT;
+		goto err_demux;
 	}
 
- done:
+	/* Pass received frame to command */
+	aoecmd_get ( aoecmd );
+	if ( ( rc = aoecmd_rx ( aoecmd, iob_disown ( iobuf ),
+				ll_source ) ) != 0 )
+		goto err_rx;
+
+ err_rx:
+	aoecmd_put ( aoecmd );
+ err_demux:
+ err_sanity:
 	free_iob ( iobuf );
 	return rc;
 }
@@ -346,126 +942,90 @@ struct net_protocol aoe_protocol __net_protocol = {
 	.rx = aoe_rx,
 };
 
-/**
- * Issue ATA command via an open AoE session
+/******************************************************************************
  *
- * @v ata		ATA device
- * @v command		ATA command
- * @ret rc		Return status code
+ * AoE URIs
+ *
+ ******************************************************************************
  */
-static int aoe_command ( struct ata_device *ata,
-			 struct ata_command *command ) {
-	struct aoe_session *aoe =
-		container_of ( ata->backend, struct aoe_session, refcnt );
-
-	aoe->command = command;
-	aoe->status = 0;
-	aoe->command_offset = 0;
-	aoe->aoe_cmd_type = AOE_CMD_ATA;
-
-	aoe_send_command ( aoe );
-
-	return 0;
-}
 
 /**
- * Issue AoE config query for AoE target discovery
+ * Parse AoE URI
  *
- * @v aoe		AoE session
+ * @v uri		URI
+ * @ret major		Major device number
+ * @ret minor		Minor device number
  * @ret rc		Return status code
+ *
+ * An AoE URI has the form "aoe:e<major>.<minor>".
  */
-static int aoe_discover ( struct aoe_session *aoe ) {
-	int rc;
-
-	aoe->status = 0;
-	aoe->aoe_cmd_type = AOE_CMD_CONFIG;
-	aoe->command = NULL;
-
-	aoe_send_command ( aoe );
-
-	aoe->rc = -EINPROGRESS;
-	while ( aoe->rc == -EINPROGRESS )
-		step();
-	rc = aoe->rc;
-
-	return rc;
-}
-
-static int aoe_detached_command ( struct ata_device *ata __unused,
-				  struct ata_command *command __unused ) {
-	return -ENODEV;
-}
-
-void aoe_detach ( struct ata_device *ata ) {
-	struct aoe_session *aoe =
-		container_of ( ata->backend, struct aoe_session, refcnt );
+static int aoe_parse_uri ( struct uri *uri, unsigned int *major,
+			   unsigned int *minor ) {
+	const char *ptr;
+	char *end;
 
-	stop_timer ( &aoe->timer );
-	ata->command = aoe_detached_command;
-	list_del ( &aoe->list );
-	ref_put ( ata->backend );
-	ata->backend = NULL;
-}
-
-static int aoe_parse_root_path ( struct aoe_session *aoe,
-				 const char *root_path ) {
-	char *ptr;
-
-	if ( strncmp ( root_path, "aoe:", 4 ) != 0 )
+	/* Check for URI with opaque portion */
+	if ( ! uri->opaque )
 		return -EINVAL;
-	ptr = ( ( char * ) root_path + 4 );
+	ptr = uri->opaque;
 
-	if ( *ptr++ != 'e' )
+	/* Check for initial 'e' */
+	if ( *ptr != 'e' )
 		return -EINVAL;
+	ptr++;
 
-	aoe->major = strtoul ( ptr, &ptr, 10 );
-	if ( *ptr++ != '.' )
+	/* Parse major device number */
+	*major = strtoul ( ptr, &end, 10 );
+	if ( *end != '.' )
 		return -EINVAL;
+	ptr = ( end + 1 );
 
-	aoe->minor = strtoul ( ptr, &ptr, 10 );
-	if ( *ptr )
+	/* Parse minor device number */
+	*minor = strtoul ( ptr, &end, 10 );
+	if ( *end )
 		return -EINVAL;
 
 	return 0;
 }
 
-int aoe_attach ( struct ata_device *ata, struct net_device *netdev,
-		 const char *root_path ) {
-	struct aoe_session *aoe;
+/**
+ * Open AoE URI
+ *
+ * @v parent		Parent interface
+ * @v uri		URI
+ * @ret rc		Return status code
+ */
+static int aoe_open ( struct interface *parent, struct uri *uri ) {
+	struct net_device *netdev;
+	unsigned int major;
+	unsigned int minor;
 	int rc;
 
-	/* Allocate and initialise structure */
-	aoe = zalloc ( sizeof ( *aoe ) );
-	if ( ! aoe )
-		return -ENOMEM;
-	ref_init ( &aoe->refcnt, aoe_free );
-	timer_init ( &aoe->timer, aoe_timer_expired, &aoe->refcnt );
-	aoe->netdev = netdev_get ( netdev );
-	memcpy ( aoe->target, netdev->ll_broadcast, sizeof ( aoe->target ) );
-	aoe->tag = AOE_TAG_MAGIC;
-
-	/* Parse root path */
-	if ( ( rc = aoe_parse_root_path ( aoe, root_path ) ) != 0 )
-		goto err;
-
-	/* Attach parent interface, transfer reference to connection
-	 * list, and return
+	/* Identify network device.  This is something of a hack, but
+	 * the AoE URI scheme that has been in use for some time now
+	 * provides no way to specify a particular device.
 	 */
-	ata->backend = ref_get ( &aoe->refcnt );
-	ata->command = aoe_command;
-	list_add ( &aoe->list, &aoe_sessions );
-
-	/* Send discovery packet to find the target MAC address.
-	 * Ideally, this ought to be done asynchronously, but the
-	 * block device interface does not yet support asynchronous
-	 * operation.
-	 */
-	if ( ( rc = aoe_discover( aoe ) ) != 0 )
-	       goto err;
+	netdev = last_opened_netdev();
+	if ( ! netdev ) {
+		DBG ( "AoE cannot identify network device\n" );
+		return -ENODEV;
+	}
 
-	return 0;
+	/* Parse URI */
+	if ( ( rc = aoe_parse_uri ( uri, &major, &minor ) ) != 0 ) {
+		DBG ( "AoE cannot parse URI\n" );
+		return rc;
+	}
 
- err:
-	ref_put ( &aoe->refcnt );
-	return rc;
+	/* Open AoE device */
+	if ( ( rc = aoedev_open ( parent, netdev, major, minor ) ) != 0 )
+		return rc;
+
+	return 0;
 }
+
+/** AoE URI opener */
+struct uri_opener aoe_uri_opener __uri_opener = {
+	.scheme = "aoe",
+	.open = aoe_open,
+};
diff --git a/src/net/infiniband/ib_srp.c b/src/net/infiniband/ib_srp.c
index ef0078d43..41416a202 100644
--- a/src/net/infiniband/ib_srp.c
+++ b/src/net/infiniband/ib_srp.c
@@ -32,7 +32,11 @@ FILE_LICENCE ( BSD2 );
 
 #include <stdlib.h>
 #include <errno.h>
+#include <ipxe/interface.h>
+#include <ipxe/uri.h>
+#include <ipxe/open.h>
 #include <ipxe/base16.h>
+#include <ipxe/acpi.h>
 #include <ipxe/srp.h>
 #include <ipxe/infiniband.h>
 #include <ipxe/ib_cmrc.h>
@@ -56,6 +60,192 @@ FILE_LICENCE ( BSD2 );
 #define EINFO_EINVAL_RP_TOO_SHORT __einfo_uniqify \
 	( EINFO_EINVAL, 0x04, "Root path too short" )
 
+/******************************************************************************
+ *
+ * IB SRP devices
+ *
+ ******************************************************************************
+ */
+
+/** An Infiniband SRP device */
+struct ib_srp_device {
+	/** Reference count */
+	struct refcnt refcnt;
+
+	/** SRP transport interface */
+	struct interface srp;
+	/** CMRC interface */
+	struct interface cmrc;
+
+	/** Infiniband device */
+	struct ib_device *ibdev;
+
+	/** Destination GID (for boot firmware table) */
+	struct ib_gid dgid;
+	/** Service ID (for boot firmware table) */
+	struct ib_gid_half service_id;
+};
+
+/**
+ * Free IB SRP device
+ *
+ * @v refcnt		Reference count
+ */
+static void ib_srp_free ( struct refcnt *refcnt ) {
+	struct ib_srp_device *ib_srp =
+		container_of ( refcnt, struct ib_srp_device, refcnt );
+
+	ibdev_put ( ib_srp->ibdev );
+	free ( ib_srp );
+}
+
+/**
+ * Close IB SRP device
+ *
+ * @v ib_srp		IB SRP device
+ * @v rc		Reason for close
+ */
+static void ib_srp_close ( struct ib_srp_device *ib_srp, int rc ) {
+
+	/* Shut down interfaces */
+	intf_shutdown ( &ib_srp->cmrc, rc );
+	intf_shutdown ( &ib_srp->srp, rc );
+}
+
+/**
+ * Describe IB SRP device in an ACPI table
+ *
+ * @v srpdev		SRP device
+ * @v acpi		ACPI table
+ * @v len		Length of ACPI table
+ * @ret rc		Return status code
+ */
+static int ib_srp_describe ( struct ib_srp_device *ib_srp,
+			     struct acpi_description_header *acpi,
+			     size_t len ) {
+	struct ib_device *ibdev = ib_srp->ibdev;
+	struct sbft_table *sbft =
+		container_of ( acpi, struct sbft_table, acpi );
+	struct sbft_ib_subtable *ib_sbft;
+	size_t used;
+
+	/* Sanity check */
+	if ( acpi->signature != SBFT_SIG )
+		return -EINVAL;
+
+	/* Append IB subtable to existing table */
+	used = le32_to_cpu ( sbft->acpi.length );
+	sbft->ib_offset = cpu_to_le16 ( used );
+	ib_sbft = ( ( ( void * ) sbft ) + used );
+	used += sizeof ( *ib_sbft );
+	if ( used > len )
+		return -ENOBUFS;
+	sbft->acpi.length = cpu_to_le32 ( used );
+
+	/* Populate subtable */
+	memcpy ( &ib_sbft->sgid, &ibdev->gid, sizeof ( ib_sbft->sgid ) );
+	memcpy ( &ib_sbft->dgid, &ib_srp->dgid, sizeof ( ib_sbft->dgid ) );
+	memcpy ( &ib_sbft->service_id, &ib_srp->service_id,
+		 sizeof ( ib_sbft->service_id ) );
+	ib_sbft->pkey = cpu_to_le16 ( ibdev->pkey );
+
+	return 0;
+}
+
+/** IB SRP CMRC interface operations */
+static struct interface_operation ib_srp_cmrc_op[] = {
+	INTF_OP ( intf_close, struct ib_srp_device *, ib_srp_close ),
+};
+
+/** IB SRP CMRC interface descriptor */
+static struct interface_descriptor ib_srp_cmrc_desc =
+	INTF_DESC_PASSTHRU ( struct ib_srp_device, cmrc, ib_srp_cmrc_op, srp );
+
+/** IB SRP SRP interface operations */
+static struct interface_operation ib_srp_srp_op[] = {
+	INTF_OP ( acpi_describe, struct ib_srp_device *, ib_srp_describe ),
+	INTF_OP ( intf_close, struct ib_srp_device *, ib_srp_close ),
+};
+
+/** IB SRP SRP interface descriptor */
+static struct interface_descriptor ib_srp_srp_desc =
+	INTF_DESC_PASSTHRU ( struct ib_srp_device, srp, ib_srp_srp_op, cmrc );
+
+/**
+ * Open IB SRP device
+ *
+ * @v block		Block control interface
+ * @v ibdev		Infiniband device
+ * @v dgid		Destination GID
+ * @v service_id	Service ID
+ * @v initiator		Initiator port ID
+ * @v target		Target port ID
+ * @v lun		SCSI LUN
+ * @ret rc		Return status code
+ */
+static int ib_srp_open ( struct interface *block, struct ib_device *ibdev,
+			 struct ib_gid *dgid, struct ib_gid_half *service_id,
+			 union srp_port_id *initiator,
+			 union srp_port_id *target, struct scsi_lun *lun ) {
+	struct ib_srp_device *ib_srp;
+	int rc;
+
+	/* Allocate and initialise structure */
+	ib_srp = zalloc ( sizeof ( *ib_srp ) );
+	if ( ! ib_srp ) {
+		rc = -ENOMEM;
+		goto err_zalloc;
+	}
+	ref_init ( &ib_srp->refcnt, ib_srp_free );
+	intf_init ( &ib_srp->srp, &ib_srp_srp_desc, &ib_srp->refcnt );
+	intf_init ( &ib_srp->cmrc, &ib_srp_cmrc_desc, &ib_srp->refcnt );
+	ib_srp->ibdev = ibdev_get ( ibdev );
+	DBGC ( ib_srp, "IBSRP %p created for %08x%08x%08x%08x:%08x%08x\n",
+	       ib_srp, ntohl ( dgid->u.dwords[0] ),
+	       ntohl ( dgid->u.dwords[1] ), ntohl ( dgid->u.dwords[2] ),
+	       ntohl ( dgid->u.dwords[3] ), ntohl ( service_id->u.dwords[0] ),
+	       ntohl ( service_id->u.dwords[1] ) );
+
+	/* Preserve parameters required for boot firmware table */
+	memcpy ( &ib_srp->dgid, dgid, sizeof ( ib_srp->dgid ) );
+	memcpy ( &ib_srp->service_id, service_id,
+		 sizeof ( ib_srp->service_id ) );
+
+	/* Open CMRC socket */
+	if ( ( rc = ib_cmrc_open ( &ib_srp->cmrc, ibdev, dgid,
+				   service_id ) ) != 0 ) {
+		DBGC ( ib_srp, "IBSRP %p could not open CMRC socket: %s\n",
+		       ib_srp, strerror ( rc ) );
+		goto err_cmrc_open;
+	}
+
+	/* Attach SRP device to parent interface */
+	if ( ( rc = srp_open ( block, &ib_srp->srp, initiator, target,
+			       ibdev->rdma_key, lun ) ) != 0 ) {
+		DBGC ( ib_srp, "IBSRP %p could not create SRP device: %s\n",
+		       ib_srp, strerror ( rc ) );
+		goto err_srp_open;
+	}
+
+	/* Mortalise self and return */
+	ref_put ( &ib_srp->refcnt );
+	return 0;
+
+ err_srp_open:
+ err_cmrc_open:
+	ib_srp_close ( ib_srp, rc );
+	ref_put ( &ib_srp->refcnt );
+ err_zalloc:
+	return rc;
+}
+
+/******************************************************************************
+ *
+ * IB SRP URIs
+ *
+ ******************************************************************************
+ */
+
 /** IB SRP parse flags */
 enum ib_srp_parse_flags {
 	IB_SRP_PARSE_REQUIRED = 0x0000,
@@ -65,12 +255,20 @@ enum ib_srp_parse_flags {
 
 /** IB SRP root path parameters */
 struct ib_srp_root_path {
+	/** Source GID */
+	struct ib_gid sgid;
+	/** Initiator port ID */
+	union ib_srp_initiator_port_id initiator;
+	/** Destination GID */
+	struct ib_gid dgid;
+	/** Partition key */
+	uint16_t pkey;
+	/** Service ID */
+	struct ib_gid_half service_id;
 	/** SCSI LUN */
-	struct scsi_lun *lun;
-	/** SRP port IDs */
-	struct srp_port_ids *port_ids;
-	/** IB SRP parameters */
-	struct ib_srp_parameters *ib;
+	struct scsi_lun lun;
+	/** Target port ID */
+	union ib_srp_target_port_id target;
 };
 
 /**
@@ -99,7 +297,6 @@ static int ib_srp_parse_byte_string ( const char *rp_comp, uint8_t *bytes,
 	decoded_size = base16_decode ( rp_comp, bytes );
 	if ( decoded_size < 0 )
 		return decoded_size;
-	assert ( decoded_size == size );
 
 	return 0;
 }
@@ -126,19 +323,6 @@ static int ib_srp_parse_integer ( const char *rp_comp, int default_value ) {
 }
 
 /**
- * Parse IB SRP root path literal component
- *
- * @v rp_comp		Root path component string
- * @v rp		IB SRP root path
- * @ret rc		Return status code
- */
-static int ib_srp_parse_literal ( const char *rp_comp __unused,
-				  struct ib_srp_root_path *rp __unused ) {
-	/* Ignore */
-	return 0;
-}
-
-/**
  * Parse IB SRP root path source GID
  *
  * @v rp_comp		Root path component string
@@ -151,10 +335,10 @@ static int ib_srp_parse_sgid ( const char *rp_comp,
 
 	/* Default to the GID of the last opened Infiniband device */
 	if ( ( ibdev = last_opened_ibdev() ) != NULL )
-		memcpy ( &rp->ib->sgid, &ibdev->gid, sizeof ( rp->ib->sgid ) );
+		memcpy ( &rp->sgid, &ibdev->gid, sizeof ( rp->sgid ) );
 
-	return ib_srp_parse_byte_string ( rp_comp, rp->ib->sgid.u.bytes,
-					  ( sizeof ( rp->ib->sgid ) |
+	return ib_srp_parse_byte_string ( rp_comp, rp->sgid.u.bytes,
+					  ( sizeof ( rp->sgid ) |
 					    IB_SRP_PARSE_OPTIONAL ) );
 }
 
@@ -167,11 +351,10 @@ static int ib_srp_parse_sgid ( const char *rp_comp,
  */
 static int ib_srp_parse_initiator_id_ext ( const char *rp_comp,
 					   struct ib_srp_root_path *rp ) {
-	struct ib_srp_initiator_port_id *port_id =
-		ib_srp_initiator_port_id ( rp->port_ids );
+	union ib_srp_initiator_port_id *port_id = &rp->initiator;
 
-	return ib_srp_parse_byte_string ( rp_comp, port_id->id_ext.u.bytes,
-					  ( sizeof ( port_id->id_ext ) |
+	return ib_srp_parse_byte_string ( rp_comp, port_id->ib.id_ext.u.bytes,
+					  ( sizeof ( port_id->ib.id_ext ) |
 					    IB_SRP_PARSE_OPTIONAL ) );
 }
 
@@ -184,15 +367,14 @@ static int ib_srp_parse_initiator_id_ext ( const char *rp_comp,
  */
 static int ib_srp_parse_initiator_hca_guid ( const char *rp_comp,
 					     struct ib_srp_root_path *rp ) {
-	struct ib_srp_initiator_port_id *port_id =
-		ib_srp_initiator_port_id ( rp->port_ids );
+	union ib_srp_initiator_port_id *port_id = &rp->initiator;
 
 	/* Default to the GUID portion of the source GID */
-	memcpy ( &port_id->hca_guid, &rp->ib->sgid.u.half[1],
-		 sizeof ( port_id->hca_guid ) );
+	memcpy ( &port_id->ib.hca_guid, &rp->sgid.u.half[1],
+		 sizeof ( port_id->ib.hca_guid ) );
 
-	return ib_srp_parse_byte_string ( rp_comp, port_id->hca_guid.u.bytes,
-					  ( sizeof ( port_id->hca_guid ) |
+	return ib_srp_parse_byte_string ( rp_comp, port_id->ib.hca_guid.u.bytes,
+					  ( sizeof ( port_id->ib.hca_guid ) |
 					    IB_SRP_PARSE_OPTIONAL ) );
 }
 
@@ -205,8 +387,8 @@ static int ib_srp_parse_initiator_hca_guid ( const char *rp_comp,
  */
 static int ib_srp_parse_dgid ( const char *rp_comp,
 			       struct ib_srp_root_path *rp ) {
-	return ib_srp_parse_byte_string ( rp_comp, rp->ib->dgid.u.bytes,
-					  ( sizeof ( rp->ib->dgid ) |
+	return ib_srp_parse_byte_string ( rp_comp, rp->dgid.u.bytes,
+					  ( sizeof ( rp->dgid ) |
 					    IB_SRP_PARSE_REQUIRED ) );
 }
 
@@ -223,7 +405,7 @@ static int ib_srp_parse_pkey ( const char *rp_comp,
 
 	if ( ( pkey = ib_srp_parse_integer ( rp_comp, IB_PKEY_DEFAULT ) ) < 0 )
 		return pkey;
-	rp->ib->pkey = pkey;
+	rp->pkey = pkey;
 	return 0;
 }
 
@@ -236,8 +418,8 @@ static int ib_srp_parse_pkey ( const char *rp_comp,
  */
 static int ib_srp_parse_service_id ( const char *rp_comp,
 				     struct ib_srp_root_path *rp ) {
-	return ib_srp_parse_byte_string ( rp_comp, rp->ib->service_id.u.bytes,
-					  ( sizeof ( rp->ib->service_id ) |
+	return ib_srp_parse_byte_string ( rp_comp, rp->service_id.u.bytes,
+					  ( sizeof ( rp->service_id ) |
 					    IB_SRP_PARSE_REQUIRED ) );
 }
 
@@ -250,7 +432,7 @@ static int ib_srp_parse_service_id ( const char *rp_comp,
  */
 static int ib_srp_parse_lun ( const char *rp_comp,
 			      struct ib_srp_root_path *rp ) {
-	return scsi_parse_lun ( rp_comp, rp->lun );
+	return scsi_parse_lun ( rp_comp, &rp->lun );
 }
 
 /**
@@ -262,11 +444,10 @@ static int ib_srp_parse_lun ( const char *rp_comp,
  */
 static int ib_srp_parse_target_id_ext ( const char *rp_comp,
 					struct ib_srp_root_path *rp ) {
-	struct ib_srp_target_port_id *port_id =
-		ib_srp_target_port_id ( rp->port_ids );
+	union ib_srp_target_port_id *port_id = &rp->target;
 
-	return ib_srp_parse_byte_string ( rp_comp, port_id->id_ext.u.bytes,
-					  ( sizeof ( port_id->id_ext ) |
+	return ib_srp_parse_byte_string ( rp_comp, port_id->ib.id_ext.u.bytes,
+					  ( sizeof ( port_id->ib.id_ext ) |
 					    IB_SRP_PARSE_REQUIRED ) );
 }
 
@@ -279,11 +460,10 @@ static int ib_srp_parse_target_id_ext ( const char *rp_comp,
  */
 static int ib_srp_parse_target_ioc_guid ( const char *rp_comp,
 					  struct ib_srp_root_path *rp ) {
-	struct ib_srp_target_port_id *port_id =
-		ib_srp_target_port_id ( rp->port_ids );
+	union ib_srp_target_port_id *port_id = &rp->target;
 
-	return ib_srp_parse_byte_string ( rp_comp, port_id->ioc_guid.u.bytes,
-					  ( sizeof ( port_id->ioc_guid ) |
+	return ib_srp_parse_byte_string ( rp_comp, port_id->ib.ioc_guid.u.bytes,
+					  ( sizeof ( port_id->ib.ioc_guid ) |
 					    IB_SRP_PARSE_REQUIRED ) );
 }
 
@@ -301,7 +481,6 @@ struct ib_srp_root_path_parser {
 
 /** IB SRP root path components */
 static struct ib_srp_root_path_parser ib_srp_rp_parser[] = {
-	{ ib_srp_parse_literal },
 	{ ib_srp_parse_sgid },
 	{ ib_srp_parse_initiator_id_ext },
 	{ ib_srp_parse_initiator_hca_guid },
@@ -320,18 +499,13 @@ static struct ib_srp_root_path_parser ib_srp_rp_parser[] = {
 /**
  * Parse IB SRP root path
  *
- * @v srp		SRP device
- * @v rp_string		Root path
+ * @v rp_string		Root path string
+ * @v rp		IB SRP root path
  * @ret rc		Return status code
  */
-static int ib_srp_parse_root_path ( struct srp_device *srp,
-				    const char *rp_string ) {
-	struct ib_srp_parameters *ib_params = ib_srp_params ( srp );
-	struct ib_srp_root_path rp = {
-		.lun = &srp->lun,
-		.port_ids = &srp->port_ids,
-		.ib = ib_params,
-	};
+static int ib_srp_parse_root_path ( const char *rp_string,
+				    struct ib_srp_root_path *rp ) {
+	struct ib_srp_root_path_parser *parser;
 	char rp_string_copy[ strlen ( rp_string ) + 1 ];
 	char *rp_comp[IB_SRP_NUM_RP_COMPONENTS];
 	char *rp_string_tmp = rp_string_copy;
@@ -346,8 +520,8 @@ static int ib_srp_parse_root_path ( struct srp_device *srp,
 			break;
 		for ( ; *rp_string_tmp != ':' ; rp_string_tmp++ ) {
 			if ( ! *rp_string_tmp ) {
-				DBGC ( srp, "SRP %p root path \"%s\" too "
-				       "short\n", srp, rp_string );
+				DBG ( "IBSRP root path \"%s\" too short\n",
+				      rp_string );
 				return -EINVAL_RP_TOO_SHORT;
 			}
 		}
@@ -356,11 +530,11 @@ static int ib_srp_parse_root_path ( struct srp_device *srp,
 
 	/* Parse root path components */
 	for ( i = 0 ; i < IB_SRP_NUM_RP_COMPONENTS ; i++ ) {
-		if ( ( rc = ib_srp_rp_parser[i].parse ( rp_comp[i],
-							&rp ) ) != 0 ) {
-			DBGC ( srp, "SRP %p could not parse \"%s\" in root "
-			       "path \"%s\": %s\n", srp, rp_comp[i],
-			       rp_string, strerror ( rc ) );
+		parser = &ib_srp_rp_parser[i];
+		if ( ( rc = parser->parse ( rp_comp[i], rp ) ) != 0 ) {
+			DBG ( "IBSRP could not parse \"%s\" in root path "
+			      "\"%s\": %s\n", rp_comp[i], rp_string,
+			      strerror ( rc ) );
 			return rc;
 		}
 	}
@@ -369,41 +543,42 @@ static int ib_srp_parse_root_path ( struct srp_device *srp,
 }
 
 /**
- * Connect IB SRP session
+ * Open IB SRP URI
  *
- * @v srp		SRP device
+ * @v parent		Parent interface
+ * @v uri		URI
  * @ret rc		Return status code
  */
-static int ib_srp_connect ( struct srp_device *srp ) {
-	struct ib_srp_parameters *ib_params = ib_srp_params ( srp );
+static int ib_srp_open_uri ( struct interface *parent, struct uri *uri ) {
+	struct ib_srp_root_path rp;
 	struct ib_device *ibdev;
 	int rc;
 
+	/* Parse URI */
+	if ( ! uri->opaque )
+		return -EINVAL;
+	memset ( &rp, 0, sizeof ( rp ) );
+	if ( ( rc = ib_srp_parse_root_path ( uri->opaque, &rp ) ) != 0 )
+		return rc;
+
 	/* Identify Infiniband device */
-	ibdev = find_ibdev ( &ib_params->sgid );
+	ibdev = find_ibdev ( &rp.sgid );
 	if ( ! ibdev ) {
-		DBGC ( srp, "SRP %p could not identify Infiniband device\n",
-		       srp );
+		DBG ( "IBSRP could not identify Infiniband device\n" );
 		return -ENODEV;
 	}
 
-	/* Configure remaining SRP parameters */
-	srp->memory_handle = ibdev->rdma_key;
-
-	/* Open CMRC socket */
-	if ( ( rc = ib_cmrc_open ( &srp->socket, ibdev, &ib_params->dgid,
-				   &ib_params->service_id ) ) != 0 ) {
-		DBGC ( srp, "SRP %p could not open CMRC socket: %s\n",
-		       srp, strerror ( rc ) );
+	/* Open IB SRP device */
+	if ( ( rc = ib_srp_open ( parent, ibdev, &rp.dgid, &rp.service_id,
+				  &rp.initiator.srp, &rp.target.srp,
+				  &rp.lun ) ) != 0 )
 		return rc;
-	}
 
 	return 0;
 }
 
-/** IB SRP transport type */
-struct srp_transport_type ib_srp_transport = {
-	.priv_len = sizeof ( struct ib_srp_parameters ),
-	.parse_root_path = ib_srp_parse_root_path,
-	.connect = ib_srp_connect,
+/** IB SRP URI opener */
+struct uri_opener ib_srp_uri_opener __uri_opener = {
+	.scheme = "ib_srp",
+	.open = ib_srp_open_uri,
 };
diff --git a/src/net/tcp/iscsi.c b/src/net/tcp/iscsi.c
index 80c63a6b1..e19f4d827 100644
--- a/src/net/tcp/iscsi.c
+++ b/src/net/tcp/iscsi.c
@@ -28,6 +28,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/vsprintf.h>
 #include <ipxe/socket.h>
 #include <ipxe/iobuf.h>
+#include <ipxe/uri.h>
 #include <ipxe/xfer.h>
 #include <ipxe/open.h>
 #include <ipxe/scsi.h>
@@ -38,6 +39,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/features.h>
 #include <ipxe/base16.h>
 #include <ipxe/base64.h>
+#include <ipxe/ibft.h>
 #include <ipxe/iscsi.h>
 
 /** @file
@@ -128,6 +130,36 @@ static void iscsi_rx_buffered_data_done ( struct iscsi_session *iscsi ) {
 }
 
 /**
+ * Receive PDU data into buffer
+ *
+ * @v iscsi		iSCSI session
+ * @v data		Data to receive
+ * @v len		Length of data
+ * @ret rc		Return status code
+ *
+ * This can be used when the RX PDU type handler wishes to buffer up
+ * all received data and process the PDU as a single unit.  The caller
+ * is repsonsible for calling iscsi_rx_buffered_data_done() after
+ * processing the data.
+ */
+static int iscsi_rx_buffered_data ( struct iscsi_session *iscsi,
+				    const void *data, size_t len ) {
+
+	/* Allocate buffer on first call */
+	if ( ! iscsi->rx_buffer ) {
+		iscsi->rx_buffer = malloc ( iscsi->rx_len );
+		if ( ! iscsi->rx_buffer )
+			return -ENOMEM;
+	}
+
+	/* Copy data to buffer */
+	assert ( ( iscsi->rx_offset + len ) <= iscsi->rx_len );
+	memcpy ( ( iscsi->rx_buffer + iscsi->rx_offset ), data, len );
+
+	return 0;
+}
+
+/**
  * Free iSCSI session
  *
  * @v refcnt		Reference counter
@@ -144,10 +176,45 @@ static void iscsi_free ( struct refcnt *refcnt ) {
 	free ( iscsi->target_password );
 	chap_finish ( &iscsi->chap );
 	iscsi_rx_buffered_data_done ( iscsi );
+	free ( iscsi->command );
 	free ( iscsi );
 }
 
 /**
+ * Shut down iSCSI interface
+ *
+ * @v iscsi		iSCSI session
+ * @v rc		Reason for close
+ */
+static void iscsi_close ( struct iscsi_session *iscsi, int rc ) {
+
+	/* A TCP graceful close is still an error from our point of view */
+	if ( rc == 0 )
+		rc = -ECONNRESET;
+
+	DBGC ( iscsi, "iSCSI %p closed: %s\n", iscsi, strerror ( rc ) );
+
+	/* Stop transmission process */
+	process_del ( &iscsi->process );
+
+	/* Shut down interfaces */
+	intf_shutdown ( &iscsi->socket, rc );
+	intf_shutdown ( &iscsi->control, rc );
+	intf_shutdown ( &iscsi->data, rc );
+}
+
+/**
+ * Assign new iSCSI initiator task tag
+ *
+ * @v iscsi		iSCSI session
+ */
+static void iscsi_new_itt ( struct iscsi_session *iscsi ) {
+	static uint16_t itt_idx;
+
+	iscsi->itt = ( ISCSI_TAG_MAGIC | (++itt_idx) );
+}
+
+/**
  * Open iSCSI transport-layer connection
  *
  * @v iscsi		iSCSI session
@@ -180,7 +247,7 @@ static int iscsi_open_connection ( struct iscsi_session *iscsi ) {
 		iscsi->status |= ISCSI_STATUS_AUTH_REVERSE_REQUIRED;
 
 	/* Assign fresh initiator task tag */
-	iscsi->itt++;
+	iscsi_new_itt ( iscsi );
 
 	/* Initiate login */
 	iscsi_start_login ( iscsi );
@@ -220,21 +287,34 @@ static void iscsi_close_connection ( struct iscsi_session *iscsi, int rc ) {
  *
  * @v iscsi		iSCSI session
  * @v rc		Return status code
+ * @v rsp		SCSI response, if any
  *
  * Note that iscsi_scsi_done() will not close the connection, and must
  * therefore be called only when the internal state machines are in an
  * appropriate state, otherwise bad things may happen on the next call
- * to iscsi_issue().  The general rule is to call iscsi_scsi_done()
- * only at the end of receiving a PDU; at this point the TX and RX
- * engines should both be idle.
+ * to iscsi_scsi_command().  The general rule is to call
+ * iscsi_scsi_done() only at the end of receiving a PDU; at this point
+ * the TX and RX engines should both be idle.
  */
-static void iscsi_scsi_done ( struct iscsi_session *iscsi, int rc ) {
+static void iscsi_scsi_done ( struct iscsi_session *iscsi, int rc,
+			      struct scsi_rsp *rsp ) {
+	uint32_t itt = iscsi->itt;
 
 	assert ( iscsi->tx_state == ISCSI_TX_IDLE );
-	assert ( iscsi->command != NULL );
 
-	iscsi->command->rc = rc;
+	/* Clear command */
+	free ( iscsi->command );
 	iscsi->command = NULL;
+
+	/* Send SCSI response, if any */
+	scsi_response ( &iscsi->data, rsp );
+
+	/* Close SCSI command, if this is still the same command.  (It
+	 * is possible that the command interface has already been
+	 * closed as a result of the SCSI response we sent.)
+	 */
+	if ( iscsi->itt == itt )
+		intf_restart ( &iscsi->data, rc );
 }
 
 /****************************************************************************
@@ -268,8 +348,9 @@ static void iscsi_start_command ( struct iscsi_session *iscsi ) {
 	if ( iscsi->command->data_out )
 		command->flags |= ISCSI_COMMAND_FLAG_WRITE;
 	/* lengths left as zero */
-	command->lun = iscsi->lun;
-	command->itt = htonl ( ++iscsi->itt );
+	memcpy ( &command->lun, &iscsi->command->lun,
+		 sizeof ( command->lun ) );
+	command->itt = htonl ( iscsi->itt );
 	command->exp_len = htonl ( iscsi->command->data_in_len |
 				   iscsi->command->data_out_len );
 	command->cmdsn = htonl ( iscsi->cmdsn );
@@ -297,28 +378,39 @@ static int iscsi_rx_scsi_response ( struct iscsi_session *iscsi,
 				    size_t remaining ) {
 	struct iscsi_bhs_scsi_response *response
 		= &iscsi->rx_bhs.scsi_response;
-	int sense_offset;
+	struct scsi_rsp rsp;
+	uint32_t residual_count;
+	int rc;
 
-	/* Capture the sense response code as it floats past, if present */
-	sense_offset = ISCSI_SENSE_RESPONSE_CODE_OFFSET - iscsi->rx_offset;
-	if ( ( sense_offset >= 0 ) && len ) {
-		iscsi->command->sense_response =
-			* ( ( char * ) data + sense_offset );
+	/* Buffer up the PDU data */
+	if ( ( rc = iscsi_rx_buffered_data ( iscsi, data, len ) ) != 0 ) {
+		DBGC ( iscsi, "iSCSI %p could not buffer login response: %s\n",
+		       iscsi, strerror ( rc ) );
+		return rc;
 	}
-
-	/* Wait for whole SCSI response to arrive */
 	if ( remaining )
 		return 0;
-	
-	/* Record SCSI status code */
-	iscsi->command->status = response->status;
+
+	/* Parse SCSI response and discard buffer */
+	memset ( &rsp, 0, sizeof ( rsp ) );
+	rsp.status = response->status;
+	residual_count = ntohl ( response->residual_count );
+	if ( response->flags & ISCSI_DATA_FLAG_OVERFLOW ) {
+		rsp.overrun = residual_count;
+	} else if ( response->flags & ISCSI_DATA_FLAG_UNDERFLOW ) {
+		rsp.overrun = -(residual_count);
+	}
+	if ( ISCSI_DATA_LEN ( response->lengths ) )
+		memcpy ( &rsp.sense, ( iscsi->rx_buffer + 2 ),
+			 sizeof ( rsp.sense ) );
+	iscsi_rx_buffered_data_done ( iscsi );
 
 	/* Check for errors */
 	if ( response->response != ISCSI_RESPONSE_COMMAND_COMPLETE )
 		return -EIO;
 
 	/* Mark as completed */
-	iscsi_scsi_done ( iscsi, 0 );
+	iscsi_scsi_done ( iscsi, 0, &rsp );
 	return 0;
 }
 
@@ -352,9 +444,8 @@ static int iscsi_rx_data_in ( struct iscsi_session *iscsi,
 	if ( data_in->flags & ISCSI_DATA_FLAG_STATUS ) {
 		assert ( ( offset + len ) == iscsi->command->data_in_len );
 		assert ( data_in->flags & ISCSI_FLAG_FINAL );
-		iscsi->command->status = data_in->status;
 		/* iSCSI cannot return an error status via a data-in */
-		iscsi_scsi_done ( iscsi, 0 );
+		iscsi_scsi_done ( iscsi, 0, NULL );
 	}
 
 	return 0;
@@ -412,7 +503,7 @@ static void iscsi_start_data_out ( struct iscsi_session *iscsi,
 	if ( len == remaining )
 		data_out->flags = ( ISCSI_FLAG_FINAL );
 	ISCSI_SET_LENGTHS ( data_out->lengths, 0, len );
-	data_out->lun = iscsi->lun;
+	data_out->lun = iscsi->command->lun;
 	data_out->itt = htonl ( iscsi->itt );
 	data_out->ttt = htonl ( iscsi->ttt );
 	data_out->expstatsn = htonl ( iscsi->statsn + 1 );
@@ -592,6 +683,19 @@ static void iscsi_start_login ( struct iscsi_session *iscsi ) {
 	struct iscsi_bhs_login_request *request = &iscsi->tx_bhs.login_request;
 	int len;
 
+	switch ( iscsi->status & ISCSI_LOGIN_CSG_MASK ) {
+	case ISCSI_LOGIN_CSG_SECURITY_NEGOTIATION:
+		DBGC ( iscsi, "iSCSI %p entering security negotiation\n",
+		       iscsi );
+		break;
+	case ISCSI_LOGIN_CSG_OPERATIONAL_NEGOTIATION:
+		DBGC ( iscsi, "iSCSI %p entering operational negotiation\n",
+		       iscsi );
+		break;
+	default:
+		assert ( 0 );
+	}
+
 	/* Construct BHS and initiate transmission */
 	iscsi_start_tx ( iscsi );
 	request->opcode = ( ISCSI_OPCODE_LOGIN_REQUEST |
@@ -604,7 +708,7 @@ static void iscsi_start_login ( struct iscsi_session *iscsi ) {
 	request->isid_iana_en = htonl ( ISCSI_ISID_IANA |
 					IANA_EN_FEN_SYSTEMS );
 	/* isid_iana_qual left as zero */
-	request->tsih = htons ( iscsi->tsih );
+	/* tsih left as zero */
 	request->itt = htonl ( iscsi->itt );
 	/* cid left as zero */
 	request->cmdsn = htonl ( iscsi->cmdsn );
@@ -1012,36 +1116,6 @@ static int iscsi_handle_strings ( struct iscsi_session *iscsi,
 }
 
 /**
- * Receive PDU data into buffer
- *
- * @v iscsi		iSCSI session
- * @v data		Data to receive
- * @v len		Length of data
- * @ret rc		Return status code
- *
- * This can be used when the RX PDU type handler wishes to buffer up
- * all received data and process the PDU as a single unit.  The caller
- * is repsonsible for calling iscsi_rx_buffered_data_done() after
- * processing the data.
- */
-static int iscsi_rx_buffered_data ( struct iscsi_session *iscsi,
-				    const void *data, size_t len ) {
-
-	/* Allocate buffer on first call */
-	if ( ! iscsi->rx_buffer ) {
-		iscsi->rx_buffer = malloc ( iscsi->rx_len );
-		if ( ! iscsi->rx_buffer )
-			return -ENOMEM;
-	}
-
-	/* Copy data to buffer */
-	assert ( ( iscsi->rx_offset + len ) <= iscsi->rx_len );
-	memcpy ( ( iscsi->rx_buffer + iscsi->rx_offset ), data, len );
-
-	return 0;
-}
-
-/**
  * Convert iSCSI response status to return status code
  *
  * @v status_class	iSCSI status class
@@ -1119,7 +1193,6 @@ static int iscsi_rx_login_response ( struct iscsi_session *iscsi,
 		       response->status_class, response->status_detail );
 		rc = iscsi_status_to_rc ( response->status_class,
 					  response->status_detail );
-		iscsi->instant_rc = rc;
 		return rc;
 	}
 
@@ -1160,14 +1233,9 @@ static int iscsi_rx_login_response ( struct iscsi_session *iscsi,
 		return -EPROTO;
 	}
 
-	/* Reset retry count */
-	iscsi->retry_count = 0;
-
-	/* Record TSIH for future reference */
-	iscsi->tsih = ntohl ( response->tsih );
-	
-	/* Send the actual SCSI command */
-	iscsi_start_command ( iscsi );
+	/* Notify SCSI layer of window change */
+	DBGC ( iscsi, "iSCSI %p entering full feature phase\n", iscsi );
+	xfer_window_changed ( &iscsi->control );
 
 	return 0;
 }
@@ -1187,13 +1255,18 @@ static int iscsi_rx_login_response ( struct iscsi_session *iscsi,
  * be in transit at any one time.
  */
 static void iscsi_start_tx ( struct iscsi_session *iscsi ) {
+
 	assert ( iscsi->tx_state == ISCSI_TX_IDLE );
+	assert ( ! process_running ( &iscsi->process ) );
 	
 	/* Initialise TX BHS */
 	memset ( &iscsi->tx_bhs, 0, sizeof ( iscsi->tx_bhs ) );
 
 	/* Flag TX engine to start transmitting */
 	iscsi->tx_state = ISCSI_TX_BHS;
+
+	/* Start transmission process */
+	process_add ( &iscsi->process );
 }
 
 /**
@@ -1273,6 +1346,9 @@ static int iscsi_tx_data_padding ( struct iscsi_session *iscsi ) {
 static void iscsi_tx_done ( struct iscsi_session *iscsi ) {
 	struct iscsi_bhs_common *common = &iscsi->tx_bhs.common;
 
+	/* Stop transmission process */
+	process_del ( &iscsi->process );
+
 	switch ( common->opcode & ISCSI_OPCODE_MASK ) {
 	case ISCSI_OPCODE_DATA_OUT:
 		iscsi_data_out_done ( iscsi );
@@ -1305,9 +1381,6 @@ static void iscsi_tx_step ( struct process *process ) {
 	/* Select fragment to transmit */
 	while ( 1 ) {
 		switch ( iscsi->tx_state ) {
-		case ISCSI_TX_IDLE:
-			/* Stop processing */
-			return;
 		case ISCSI_TX_BHS:
 			tx = iscsi_tx_bhs;
 			tx_len = sizeof ( iscsi->tx_bhs );
@@ -1328,6 +1401,10 @@ static void iscsi_tx_step ( struct process *process ) {
 			tx_len = ISCSI_DATA_PAD_LEN ( common->lengths );
 			next_state = ISCSI_TX_IDLE;
 			break;
+		case ISCSI_TX_IDLE:
+			/* Stop processing */
+			iscsi_tx_done ( iscsi );
+			return;
 		default:
 			assert ( 0 );
 			return;
@@ -1343,13 +1420,13 @@ static void iscsi_tx_step ( struct process *process ) {
 		if ( ( rc = tx ( iscsi ) ) != 0 ) {
 			DBGC ( iscsi, "iSCSI %p could not transmit: %s\n",
 			       iscsi, strerror ( rc ) );
+			/* Transmission errors are fatal */
+			iscsi_close ( iscsi, rc );
 			return;
 		}
 
 		/* Move to next state */
 		iscsi->tx_state = next_state;
-		if ( next_state == ISCSI_TX_IDLE )
-			iscsi_tx_done ( iscsi );
 	}
 }
 
@@ -1495,8 +1572,6 @@ static int iscsi_socket_deliver ( struct iscsi_session *iscsi,
 				 remaining ) ) != 0 ) {
 			DBGC ( iscsi, "iSCSI %p could not process received "
 			       "data: %s\n", iscsi, strerror ( rc ) );
-			iscsi_close_connection ( iscsi, rc );
-			iscsi_scsi_done ( iscsi, rc );
 			goto done;
 		}
 
@@ -1518,39 +1593,12 @@ static int iscsi_socket_deliver ( struct iscsi_session *iscsi,
  done:
 	/* Free I/O buffer */
 	free_iob ( iobuf );
-	return rc;
-}
-
-/**
- * Handle stream connection closure
- *
- * @v iscsi		iSCSI session
- * @v rc		Reason for close
- *
- */
-static void iscsi_socket_close ( struct iscsi_session *iscsi, int rc ) {
-
-	/* Even a graceful close counts as an error for iSCSI */
-	if ( ! rc )
-		rc = -ECONNRESET;
 
-	/* Close session cleanly */
-	iscsi_close_connection ( iscsi, rc );
+	/* Destroy session on error */
+	if ( rc != 0 )
+		iscsi_close ( iscsi, rc );
 
-	/* Retry connection if within the retry limit, otherwise fail */
-	if ( ++iscsi->retry_count <= ISCSI_MAX_RETRIES ) {
-		DBGC ( iscsi, "iSCSI %p retrying connection (retry #%d)\n",
-		       iscsi, iscsi->retry_count );
-		if ( ( rc = iscsi_open_connection ( iscsi ) ) != 0 ) {
-			DBGC ( iscsi, "iSCSI %p could not reconnect: %s\n",
-			       iscsi, strerror ( rc ) );
-			iscsi_scsi_done ( iscsi, rc );
-		}
-	} else {
-		DBGC ( iscsi, "iSCSI %p retry count exceeded\n", iscsi );
-		iscsi->instant_rc = rc;
-		iscsi_scsi_done ( iscsi, rc );
-	}
+	return rc;
 }
 
 /**
@@ -1582,13 +1630,12 @@ static int iscsi_vredirect ( struct iscsi_session *iscsi, int type,
 
 	return xfer_vreopen ( &iscsi->socket, type, args );
 }
-			     
 
 /** iSCSI socket interface operations */
 static struct interface_operation iscsi_socket_operations[] = {
 	INTF_OP ( xfer_deliver, struct iscsi_session *, iscsi_socket_deliver ),
 	INTF_OP ( xfer_vredirect, struct iscsi_session *, iscsi_vredirect ),
-	INTF_OP ( intf_close, struct iscsi_session *, iscsi_socket_close ),
+	INTF_OP ( intf_close, struct iscsi_session *, iscsi_close ),
 };
 
 /** iSCSI socket interface descriptor */
@@ -1602,54 +1649,100 @@ static struct interface_descriptor iscsi_socket_desc =
  */
 
 /**
- * Issue SCSI command
+ * Check iSCSI flow-control window
  *
- * @v scsi		SCSI device
- * @v command		SCSI command
- * @ret rc		Return status code
+ * @v iscsi		iSCSI session
+ * @ret len		Length of window
  */
-static int iscsi_command ( struct scsi_device *scsi,
-			   struct scsi_command *command ) {
-	struct iscsi_session *iscsi =
-		container_of ( scsi->backend, struct iscsi_session, refcnt );
-	int rc;
+static size_t iscsi_scsi_window ( struct iscsi_session *iscsi ) {
 
-	/* Abort immediately if we have a recorded permanent failure */
-	if ( iscsi->instant_rc )
-		return iscsi->instant_rc;
+	if ( ( ( iscsi->status & ISCSI_STATUS_PHASE_MASK ) ==
+	       ISCSI_STATUS_FULL_FEATURE_PHASE ) &&
+	     ( iscsi->command == NULL ) ) {
+		/* We cannot handle concurrent commands */
+		return 1;
+	} else {
+		return 0;
+	}
+}
 
-	/* Record SCSI command */
-	iscsi->command = command;
+/**
+ * Issue iSCSI SCSI command
+ *
+ * @v iscsi		iSCSI session
+ * @v parent		Parent interface
+ * @v command		SCSI command
+ * @ret tag		Command tag, or negative error
+ */
+static int iscsi_scsi_command ( struct iscsi_session *iscsi,
+				struct interface *parent,
+				struct scsi_cmd *command ) {
 
-	/* Issue command or open connection as appropriate */
-	if ( iscsi->status ) {
-		iscsi_start_command ( iscsi );
-	} else {
-		if ( ( rc = iscsi_open_connection ( iscsi ) ) != 0 ) {
-			iscsi->command = NULL;
-			return rc;
-		}
+	/* This iSCSI implementation cannot handle multiple concurrent
+	 * commands or commands arriving before login is complete.
+	 */
+	if ( iscsi_scsi_window ( iscsi ) == 0 ) {
+		DBGC ( iscsi, "iSCSI %p cannot handle concurrent commands\n",
+		       iscsi );
+		return -EOPNOTSUPP;
 	}
 
-	return 0;
+	/* Store command */
+	iscsi->command = malloc ( sizeof ( *command ) );
+	if ( ! iscsi->command )
+		return -ENOMEM;
+	memcpy ( iscsi->command, command, sizeof ( *command ) );
+
+	/* Assign new ITT */
+	iscsi_new_itt ( iscsi );
+
+	/* Start sending command */
+	iscsi_start_command ( iscsi );
+
+	/* Attach to parent interface and return */
+	intf_plug_plug ( &iscsi->data, parent );
+	return iscsi->itt;
 }
 
+/** iSCSI SCSI command-issuing interface operations */
+static struct interface_operation iscsi_control_op[] = {
+	INTF_OP ( scsi_command, struct iscsi_session *, iscsi_scsi_command ),
+	INTF_OP ( xfer_window, struct iscsi_session *, iscsi_scsi_window ),
+	INTF_OP ( intf_close, struct iscsi_session *, iscsi_close ),
+	INTF_OP ( acpi_describe, struct iscsi_session *, ibft_describe ),
+};
+
+/** iSCSI SCSI command-issuing interface descriptor */
+static struct interface_descriptor iscsi_control_desc =
+	INTF_DESC ( struct iscsi_session, control, iscsi_control_op );
+
 /**
- * Shut down iSCSI interface
+ * Close iSCSI command
  *
- * @v scsi		SCSI device
+ * @v iscsi		iSCSI session
+ * @v rc		Reason for close
  */
-void iscsi_detach ( struct scsi_device *scsi ) {
-	struct iscsi_session *iscsi =
-		container_of ( scsi->backend, struct iscsi_session, refcnt );
+static void iscsi_command_close ( struct iscsi_session *iscsi, int rc ) {
 
-	iscsi_close_connection ( iscsi, 0 );
-	process_del ( &iscsi->process );
-	scsi->command = scsi_detached_command;
-	ref_put ( scsi->backend );
-	scsi->backend = NULL;
+	/* Restart interface */
+	intf_restart ( &iscsi->data, rc );
+
+	/* Treat unsolicited command closures mid-command as fatal,
+	 * because we have no code to handle partially-completed PDUs.
+	 */
+	if ( iscsi->command != NULL )
+		iscsi_close ( iscsi, ( ( rc == 0 ) ? -ECANCELED : rc ) );
 }
 
+/** iSCSI SCSI command interface operations */
+static struct interface_operation iscsi_data_op[] = {
+	INTF_OP ( intf_close, struct iscsi_session *, iscsi_command_close ),
+};
+
+/** iSCSI SCSI command interface descriptor */
+static struct interface_descriptor iscsi_data_desc =
+	INTF_DESC ( struct iscsi_session, data, iscsi_data_op );
+
 /****************************************************************************
  *
  * Instantiator
@@ -1658,8 +1751,7 @@ void iscsi_detach ( struct scsi_device *scsi ) {
 
 /** iSCSI root path components (as per RFC4173) */
 enum iscsi_root_path_component {
-	RP_LITERAL = 0,
-	RP_SERVERNAME,
+	RP_SERVERNAME = 0,
 	RP_PROTOCOL,
 	RP_PORT,
 	RP_LUN,
@@ -1779,60 +1871,95 @@ static int iscsi_set_auth ( struct iscsi_session *iscsi,
 }
 
 /**
- * Attach iSCSI interface
+ * Open iSCSI URI
  *
- * @v scsi		SCSI device
- * @v root_path		iSCSI root path (as per RFC4173)
+ * @v parent		Parent interface
+ * @v uri		URI
  * @ret rc		Return status code
  */
-int iscsi_attach ( struct scsi_device *scsi, const char *root_path ) {
+static int iscsi_open ( struct interface *parent, struct uri *uri ) {
 	struct iscsi_session *iscsi;
 	int rc;
 
+	/* Sanity check */
+	if ( ! uri->opaque ) {
+		rc = -EINVAL;
+		goto err_sanity_uri;
+	}
+
 	/* Allocate and initialise structure */
 	iscsi = zalloc ( sizeof ( *iscsi ) );
-	if ( ! iscsi )
-		return -ENOMEM;
+	if ( ! iscsi ) {
+		rc = -ENOMEM;
+		goto err_zalloc;
+	}
 	ref_init ( &iscsi->refcnt, iscsi_free );
+	intf_init ( &iscsi->control, &iscsi_control_desc, &iscsi->refcnt );
+	intf_init ( &iscsi->data, &iscsi_data_desc, &iscsi->refcnt );
 	intf_init ( &iscsi->socket, &iscsi_socket_desc, &iscsi->refcnt );
-	process_init ( &iscsi->process, iscsi_tx_step, &iscsi->refcnt );
+	process_init_stopped ( &iscsi->process, iscsi_tx_step,
+			       &iscsi->refcnt );
 
 	/* Parse root path */
-	if ( ( rc = iscsi_parse_root_path ( iscsi, root_path ) ) != 0 )
-		goto err;
+	if ( ( rc = iscsi_parse_root_path ( iscsi, uri->opaque ) ) != 0 )
+		goto err_parse_root_path;
 	/* Set fields not specified by root path */
 	if ( ( rc = iscsi_set_auth ( iscsi,
 				     iscsi_initiator_username,
 				     iscsi_initiator_password,
 				     iscsi_target_username,
 				     iscsi_target_password ) ) != 0 )
-		goto err;
+		goto err_set_auth;
 
 	/* Sanity checks */
 	if ( ! iscsi->target_address ) {
 		DBGC ( iscsi, "iSCSI %p does not yet support discovery\n",
 		       iscsi );
 		rc = -ENOTSUP_DISCOVERY;
-		goto err;
+		goto err_sanity_address;
 	}
 	if ( ! iscsi->target_iqn ) {
 		DBGC ( iscsi, "iSCSI %p no target address supplied in %s\n",
-		       iscsi, root_path );
+		       iscsi, uri->opaque );
 		rc = -EINVAL;
-		goto err;
+		goto err_sanity_iqn;
 	}
 
-	/* Attach parent interface, mortalise self, and return */
-	scsi->backend = ref_get ( &iscsi->refcnt );
-	scsi->command = iscsi_command;
+	/* Open socket */
+	if ( ( rc = iscsi_open_connection ( iscsi ) ) != 0 )
+		goto err_open_connection;
+
+	/* Attach SCSI device to parent interface */
+	if ( ( rc = scsi_open ( parent, &iscsi->control,
+				&iscsi->lun ) ) != 0 ) {
+		DBGC ( iscsi, "iSCSI %p could not create SCSI device: %s\n",
+		       iscsi, strerror ( rc ) );
+		goto err_scsi_open;
+	}
+
+	/* Mortalise self, and return */
 	ref_put ( &iscsi->refcnt );
 	return 0;
 	
- err:
+ err_scsi_open:
+ err_open_connection:
+ err_sanity_iqn:
+ err_sanity_address:
+ err_set_auth:
+ err_parse_root_path:
+	iscsi_close ( iscsi, rc );
 	ref_put ( &iscsi->refcnt );
+ err_zalloc:
+ err_sanity_uri:
 	return rc;
 }
 
+/** iSCSI URI opener */
+struct uri_opener iscsi_uri_opener __uri_opener = {
+	.scheme = "iscsi",
+	.open = iscsi_open,
+};
+
 /****************************************************************************
  *
  * Settings