[usb] Include setup packet within I/O buffer for message transfers

The USB API currently assumes that host controllers will have
immediate data buffer space available in which to store the setup
packet.  This is true for xHCI, partially true for EHCI (which happens
to have 12 bytes of padding in each transfer descriptor due to
alignment requirements), and not true at all for UHCI.

Include the setup packet within the I/O buffer passed to the host
controller's message() method, thereby eliminating the requirement for
host controllers to provide immediate data buffers.

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

diff --git a/src/drivers/bus/usb.c b/src/drivers/bus/usb.c
index f53f746105d0e9c4c747dc400fcbd2187fe33c67..2191867ae27bb045c72aabb145f9617c8325fb89 100644 (file)
--- a/src/drivers/bus/usb.c
+++ b/src/drivers/bus/usb.c
@@ -460,16 +460,22 @@ static int usb_endpoint_mtu ( struct usb_endpoint *ep, size_t mtu ) {
  * @v index            Index parameter
  * @v iobuf            I/O buffer
  * @ret rc             Return status code
+ *
+ * The I/O buffer must have sufficient headroom to contain a setup
+ * packet.
  */
 int usb_message ( struct usb_endpoint *ep, unsigned int request,
                  unsigned int value, unsigned int index,
                  struct io_buffer *iobuf ) {
        struct usb_device *usb = ep->usb;
        struct usb_port *port = usb->port;
-       struct usb_setup_packet packet;
+       struct usb_setup_packet *packet;
        size_t len = iob_len ( iobuf );
        int rc;
 
+       /* Sanity check */
+       assert ( iob_headroom ( iobuf ) >= sizeof ( *packet ) );
+
        /* Fail immediately if device has been unplugged */
        if ( port->speed == USB_SPEED_NONE )
                return -ENODEV;
@@ -484,13 +490,14 @@ int usb_message ( struct usb_endpoint *ep, unsigned int request,
                memset ( iobuf->data, 0, len );
 
        /* Construct setup packet */
-       packet.request = cpu_to_le16 ( request );
-       packet.value = cpu_to_le16 ( value );
-       packet.index = cpu_to_le16 ( index );
-       packet.len = cpu_to_le16 ( len );
+       packet = iob_push ( iobuf, sizeof ( *packet ) );
+       packet->request = cpu_to_le16 ( request );
+       packet->value = cpu_to_le16 ( value );
+       packet->index = cpu_to_le16 ( index );
+       packet->len = cpu_to_le16 ( len );
 
        /* Enqueue message transfer */
-       if ( ( rc = ep->host->message ( ep, &packet, iobuf ) ) != 0 ) {
+       if ( ( rc = ep->host->message ( ep, iobuf ) ) != 0 ) {
                DBGC ( usb, "USB %s %s could not enqueue message transfer: "
                       "%s\n", usb->name, usb_endpoint_name ( ep->address ),
                       strerror ( rc ) );
@@ -734,19 +741,23 @@ int usb_control ( struct usb_device *usb, unsigned int request,
                  size_t len ) {
        struct usb_bus *bus = usb->port->hub->bus;
        struct usb_endpoint *ep = &usb->control;
-       struct usb_control_pseudo_header *pshdr;
        struct io_buffer *iobuf;
        struct io_buffer *cmplt;
+       union {
+               struct usb_setup_packet setup;
+               struct usb_control_pseudo_header pshdr;
+       } *headroom;
+       struct usb_control_pseudo_header *pshdr;
        unsigned int i;
        int rc;
 
        /* Allocate I/O buffer */
-       iobuf = alloc_iob ( sizeof ( *pshdr ) + len );
+       iobuf = alloc_iob ( sizeof ( *headroom ) + len );
        if ( ! iobuf ) {
                rc = -ENOMEM;
                goto err_alloc;
        }
-       iob_reserve ( iobuf, sizeof ( *pshdr ) );
+       iob_reserve ( iobuf, sizeof ( *headroom ) );
        iob_put ( iobuf, len );
        if ( request & USB_DIR_IN ) {
                memset ( data, 0, len );

diff --git a/src/drivers/usb/ehci.c b/src/drivers/usb/ehci.c
index 4bff3fef16be558e1e7df5bacc67e2dc40270143..e33b0e19f9cd973e59a391b8dc101cabd3fdc26d 100644 (file)
--- a/src/drivers/usb/ehci.c
+++ b/src/drivers/usb/ehci.c
@@ -528,8 +528,6 @@ static int ehci_enqueue ( struct ehci_device *ehci, struct ehci_ring *ring,
 
        /* Fail if any portion is unreachable */
        for ( i = 0 ; i < count ; i++ ) {
-               if ( xfer->flags & EHCI_FL_IMMEDIATE )
-                       continue;
                phys = ( virt_to_phys ( xfer[i].data ) + xfer[i].len - 1 );
                if ( ( phys > 0xffffffffUL ) && ( ! ehci->addr64 ) )
                        return -ENOTSUP;
@@ -547,16 +545,9 @@ static int ehci_enqueue ( struct ehci_device *ehci, struct ehci_ring *ring,
                desc->len = cpu_to_le16 ( xfer->len | toggle );
                desc->flags = ( xfer->flags | EHCI_FL_CERR_MAX );
 
-               /* Copy data to immediate data buffer (if requested) */
+               /* Populate buffer pointers */
                data = xfer->data;
                len = xfer->len;
-               if ( xfer->flags & EHCI_FL_IMMEDIATE ) {
-                       assert ( len <= sizeof ( desc->immediate ) );
-                       memcpy ( desc->immediate, data, len );
-                       data = desc->immediate;
-               }
-
-               /* Populate buffer pointers */
                for ( i = 0 ; len ; i++ ) {
 
                        /* Calculate length of this fragment */
@@ -1103,28 +1094,32 @@ static int ehci_endpoint_mtu ( struct usb_endpoint *ep ) {
  * Enqueue message transfer
  *
  * @v ep               USB endpoint
- * @v packet           Setup packet
  * @v iobuf            I/O buffer
  * @ret rc             Return status code
  */
 static int ehci_endpoint_message ( struct usb_endpoint *ep,
-                                  struct usb_setup_packet *packet,
                                   struct io_buffer *iobuf ) {
        struct ehci_endpoint *endpoint = usb_endpoint_get_hostdata ( ep );
        struct ehci_device *ehci = endpoint->ehci;
-       unsigned int input = ( packet->request & cpu_to_le16 ( USB_DIR_IN ) );
+       struct usb_setup_packet *packet;
+       unsigned int input;
        struct ehci_transfer xfers[3];
        struct ehci_transfer *xfer = xfers;
-       size_t len = iob_len ( iobuf );
+       size_t len;
        int rc;
 
        /* Construct setup stage */
+       assert ( iob_len ( iobuf ) >= sizeof ( *packet ) );
+       packet = iobuf->data;
+       iob_pull ( iobuf, sizeof ( *packet ) );
        xfer->data = packet;
        xfer->len = sizeof ( *packet );
-       xfer->flags = ( EHCI_FL_IMMEDIATE | EHCI_FL_PID_SETUP );
+       xfer->flags = EHCI_FL_PID_SETUP;
        xfer++;
 
        /* Construct data stage, if applicable */
+       len = iob_len ( iobuf );
+       input = ( packet->request & cpu_to_le16 ( USB_DIR_IN ) );
        if ( len ) {
                xfer->data = iobuf->data;
                xfer->len = len;

diff --git a/src/drivers/usb/ehci.h b/src/drivers/usb/ehci.h
index d575727d1c02693c6d1e2e4d4d2dff32be3a789e..7ad1e64991e1a5fff8a81e6846640ba82bf8ae56 100644 (file)
--- a/src/drivers/usb/ehci.h
+++ b/src/drivers/usb/ehci.h
@@ -241,21 +241,8 @@ struct ehci_transfer_descriptor {
        uint32_t low[5];
        /** Extended buffer pointers (high 32 bits) */
        uint32_t high[5];
-
-       /** Immediate data buffer
-        *
-        * This is not part of the hardware data structure.  Transfer
-        * descriptors must be aligned to a 32-byte boundary.  Create
-        * an array of descriptors therefore requires 12 bytes of
-        * padding at the end of each descriptor.
-        *
-        * We can use this padding as an immediate data buffer (for
-        * setup packets).  This avoids the need for separate
-        * allocations.  As a bonus, there is no need to check this
-        * buffer for reachability, since it is contained within a
-        * transfer descriptor which must already be reachable.
-        */
-       uint8_t immediate[12];
+       /** Reserved */
+       uint8_t reserved[12];
 } __attribute__ (( packed ));
 
 /** Transaction error */
@@ -483,9 +470,6 @@ struct ehci_transfer {
        unsigned int flags;
 };
 
-/** Copy data to immediate data buffer */
-#define EHCI_FL_IMMEDIATE 0x0100
-
 /** Set initial data toggle */
 #define EHCI_FL_TOGGLE 0x8000
 

diff --git a/src/drivers/usb/xhci.c b/src/drivers/usb/xhci.c
index 523a1c0b6acaa4f6e6255316e8ec7504d406d451..831e8e6ef44495937b9a4675e1a80e5f3d767658 100644 (file)
--- a/src/drivers/usb/xhci.c
+++ b/src/drivers/usb/xhci.c
@@ -2472,16 +2472,15 @@ static int xhci_endpoint_mtu ( struct usb_endpoint *ep ) {
  * Enqueue message transfer
  *
  * @v ep               USB endpoint
- * @v packet           Setup packet
  * @v iobuf            I/O buffer
  * @ret rc             Return status code
  */
 static int xhci_endpoint_message ( struct usb_endpoint *ep,
-                                  struct usb_setup_packet *packet,
                                   struct io_buffer *iobuf ) {
        struct xhci_endpoint *endpoint = usb_endpoint_get_hostdata ( ep );
-       unsigned int input = ( le16_to_cpu ( packet->request ) & USB_DIR_IN );
-       size_t len = iob_len ( iobuf );
+       struct usb_setup_packet *packet;
+       unsigned int input;
+       size_t len;
        union xhci_trb trbs[ 1 /* setup */ + 1 /* possible data */ +
                             1 /* status */ ];
        union xhci_trb *trb = trbs;
@@ -2495,11 +2494,16 @@ static int xhci_endpoint_message ( struct usb_endpoint *ep,
 
        /* Construct setup stage TRB */
        memset ( trbs, 0, sizeof ( trbs ) );
+       assert ( iob_len ( iobuf ) >= sizeof ( *packet ) );
+       packet = iobuf->data;
+       iob_pull ( iobuf, sizeof ( *packet ) );
        setup = &(trb++)->setup;
        memcpy ( &setup->packet, packet, sizeof ( setup->packet ) );
        setup->len = cpu_to_le32 ( sizeof ( *packet ) );
        setup->flags = XHCI_TRB_IDT;
        setup->type = XHCI_TRB_SETUP;
+       len = iob_len ( iobuf );
+       input = ( packet->request & cpu_to_le16 ( USB_DIR_IN ) );
        if ( len )
                setup->direction = ( input ? XHCI_SETUP_IN : XHCI_SETUP_OUT );
 

diff --git a/src/include/ipxe/usb.h b/src/include/ipxe/usb.h
index b3803cd18ed8db534147f7ddab56b1bd19a02645..991a6f44ebfb84b0d5a8866ca5463306d1b9df53 100644 (file)
--- a/src/include/ipxe/usb.h
+++ b/src/include/ipxe/usb.h
@@ -433,12 +433,10 @@ struct usb_endpoint_host_operations {
        /** Enqueue message transfer
         *
         * @v ep                USB endpoint
-        * @v packet            Setup packet
-        * @v iobuf             I/O buffer (if any)
+        * @v iobuf             I/O buffer
         * @ret rc              Return status code
         */
        int ( * message ) ( struct usb_endpoint *ep,
-                           struct usb_setup_packet *setup,
                            struct io_buffer *iobuf );
        /** Enqueue stream transfer
         *