When a driver is unloaded, its disconnect function will be called, which
may free USB 3.0 bulk endpoint streams. disconnect() cannot fail, so the
xHCI command to free streams should not fail either.
When streams are first allocated, make the xHCI driver pre-allocate an
xhci_command and reserve a space on the command ring for the configure
endpoint command to free the streams. Fail that streams allocation if the
pre-allocation fails.
Make sure there's room on the command ring reserved for a configure
endpoint command for each endpoint that has streams enabled. The driver
may allocate streams with one call to xhci_alloc_streams(), but may call
xhci_free_streams() individually for each endpoint.
The caveat is that the xHC hardware can fail the configure endpoint
command. It is valid (by the xHCI 0.96 spec) to return a "Bandwidth
Error" or a "Resource Error" for a configure endpoint command. We should
never see a Bandwidth Error, since bulk endpoints do not effect the
reserved bandwidth. The host controller can still return a Resource
Error, but it's improbable since the xHC would be going from a more
resource-intensive configuration (streams) to a less resource-intensive
configuration (no streams).
If the xHC returns a Resource Error, the endpoint will be stuck with
streams and will be unusable for drivers. It's an unavoidable consequence
of broken host controller hardware.
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---
drivers/usb/host/xhci-hcd.c | 85 ++++++++++++++++++++++++++++++------------
drivers/usb/host/xhci-mem.c | 19 +++++++++-
drivers/usb/host/xhci-ring.c | 5 --
drivers/usb/host/xhci.h | 3 +
4 files changed, 82 insertions(+), 30 deletions(-)
diff --git a/drivers/usb/host/xhci-hcd.c b/drivers/usb/host/xhci-hcd.c
index e49934c..d3dcd51 100644
--- a/drivers/usb/host/xhci-hcd.c
+++ b/drivers/usb/host/xhci-hcd.c
@@ -728,6 +728,12 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
xhci_warn(xhci, "WARN: Can't enqueue URB while bulk ep "
"is transitioning to using streams.\n");
ret = -EINVAL;
+ } else if (xhci->devs[slot_id]->eps[ep_index].ep_state &
+ EP_GETTING_NO_STREAMS) {
+ xhci_warn(xhci, "WARN: Can't enqueue URB while bulk ep "
+ "is transitioning to "
+ "not having streams.\n");
+ ret = -EINVAL;
} else {
ret = xhci_queue_bulk_tx(xhci, GFP_ATOMIC, urb,
slot_id, ep_index);
@@ -1525,7 +1531,17 @@ static u32 xhci_calculate_no_streams_bitmask(struct xhci_hcd *xhci,
for (i = 0; i < num_eps; i++) {
ep_index = xhci_get_endpoint_index(&eps[i]->desc);
ep_state = xhci->devs[slot_id]->eps[ep_index].ep_state;
- if (!(ep_state & EP_HAS_STREAMS)) {
+ /* Are streams already being freed for the endpoint? */
+ if (ep_state & EP_GETTING_NO_STREAMS) {
+ xhci_warn(xhci, "WARN Can't disable streams for "
+ "endpoint 0x%x\n, "
+ "streams are being disabled already.",
+ eps[i]->desc.bEndpointAddress);
+ return 0;
+ }
+ /* Are there actually any streams to free? */
+ if (!(ep_state & EP_HAS_STREAMS) &&
+ !(ep_state & EP_GETTING_STREAMS)) {
xhci_warn(xhci, "WARN Can't disable streams for "
"endpoint 0x%x\n, "
"streams are already disabled!",
@@ -1562,6 +1578,7 @@ int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
int i, ret;
struct xhci_hcd *xhci;
struct xhci_virt_device *vdev;
+ struct xhci_command *config_cmd;
unsigned int ep_index;
unsigned int num_stream_ctxs;
unsigned long flags;
@@ -1578,6 +1595,12 @@ int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
xhci_dbg(xhci, "Driver wants %u stream IDs (including stream 0).\n",
num_streams);
+ config_cmd = xhci_alloc_command(xhci, true, mem_flags);
+ if (!config_cmd) {
+ xhci_dbg(xhci, "Could not allocate xHCI command structure.\n");
+ return -ENOMEM;
+ }
+
/* Check to make sure all endpoints are not already configured for
* streams. While we're at it, find the maximum number of streams that
* all the endpoints will support and check for duplicate endpoints.
@@ -1586,18 +1609,20 @@ int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
ret = xhci_calculate_streams_and_bitmask(xhci, udev, eps,
num_eps, &num_streams, &changed_ep_bitmask);
if (ret < 0) {
+ xhci_free_command(xhci, config_cmd);
spin_unlock_irqrestore(&xhci->lock, flags);
return ret;
}
if (num_streams <= 1) {
xhci_warn(xhci, "WARN: endpoints can't handle "
"more than one stream.\n");
+ xhci_free_command(xhci, config_cmd);
spin_unlock_irqrestore(&xhci->lock, flags);
return -EINVAL;
}
vdev = xhci->devs[udev->slot_id];
/* Mark each endpoint as being in transistion, so
- * xhci_enqueue_urb() will reject all URBs.
+ * xhci_urb_enqueue() will reject all URBs.
*/
for (i = 0; i < num_eps; i++) {
ep_index = xhci_get_endpoint_index(&eps[i]->desc);
@@ -1625,34 +1650,28 @@ int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
*/
}
- /* XXX No locking against something else using the input context. It's
- * possible another endpoint on the device stalled, and the input
- * context is being used. Alt settings could also change. For now,
- * don't think about it. If a race conditions occurs later, make this
- * command allocate memory (stall condition should use the pre-allocated
- * input context so that it doesn't have to block on memory allocation).
- */
/* Set up the input context for a configure endpoint command. */
for (i = 0; i < num_eps; i++) {
struct xhci_ep_ctx *ep_ctx;
ep_index = xhci_get_endpoint_index(&eps[i]->desc);
- ep_ctx = xhci_get_ep_ctx(xhci, vdev->in_ctx, ep_index);
+ ep_ctx = xhci_get_ep_ctx(xhci, config_cmd->in_ctx, ep_index);
- xhci_endpoint_copy(xhci, vdev, ep_index);
+ xhci_endpoint_copy(xhci, config_cmd->in_ctx,
+ vdev->out_ctx, ep_index);
xhci_setup_streams_ep_input_ctx(xhci, ep_ctx,
vdev->eps[ep_index].stream_info);
}
/* Tell the HW to drop its old copy of the endpoint context info
* and add the updated copy from the input context.
*/
- xhci_setup_input_ctx_for_config_ep(xhci, udev->slot_id,
- changed_ep_bitmask, changed_ep_bitmask);
+ xhci_setup_input_ctx_for_config_ep(xhci, config_cmd->in_ctx,
+ vdev->out_ctx, changed_ep_bitmask, changed_ep_bitmask);
/* Issue and wait for the configure endpoint command */
- ret = xhci_configure_endpoint(xhci, udev, vdev, false);
+ ret = xhci_configure_endpoint(xhci, udev, config_cmd,
+ false, false);
- xhci_zero_in_ctx(xhci, vdev);
/* xHC rejected the configure endpoint command for some reason, so we
* leave the old ring intact and free our internal streams data
* structure.
@@ -1670,6 +1689,7 @@ int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS;
}
}
+ xhci_free_command(xhci, config_cmd);
spin_unlock_irqrestore(&xhci->lock, flags);
/* Subtract 1 for stream 0, which drivers can't use */
@@ -1687,6 +1707,7 @@ cleanup:
vdev->eps[ep_index].ep_state &= ~EP_HAS_STREAMS;
xhci_endpoint_zero(xhci, vdev, eps[i]);
}
+ xhci_free_command(xhci, config_cmd);
return -ENOMEM;
}
@@ -1703,6 +1724,7 @@ int xhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
int i, ret;
struct xhci_hcd *xhci;
struct xhci_virt_device *vdev;
+ struct xhci_command *command;
unsigned int ep_index;
unsigned long flags;
u32 changed_ep_bitmask;
@@ -1711,28 +1733,43 @@ int xhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
vdev = xhci->devs[udev->slot_id];
/* Set up a configure endpoint command to remove the streams rings */
+ spin_lock_irqsave(&xhci->lock, flags);
changed_ep_bitmask = xhci_calculate_no_streams_bitmask(xhci,
udev, eps, num_eps);
- if (changed_ep_bitmask == 0)
+ if (changed_ep_bitmask == 0) {
+ spin_unlock_irqrestore(&xhci->lock, flags);
return -EINVAL;
+ }
+ /* Use the xhci_command structure from the first endpoint. We may have
+ * allocated too many, but the driver may call xhci_free_streams() for
+ * each endpoint it grouped into one call to xhci_alloc_streams().
+ */
+ ep_index = xhci_get_endpoint_index(&eps[0]->desc);
+ command = vdev->eps[ep_index].stream_info->free_streams_command;
for (i = 0; i < num_eps; i++) {
struct xhci_ep_ctx *ep_ctx;
ep_index = xhci_get_endpoint_index(&eps[i]->desc);
- ep_ctx = xhci_get_ep_ctx(xhci, vdev->in_ctx, ep_index);
+ ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index);
+ xhci->devs[udev->slot_id]->eps[ep_index].ep_state |=
+ EP_GETTING_NO_STREAMS;
- xhci_endpoint_copy(xhci, vdev, ep_index);
+ xhci_endpoint_copy(xhci, command->in_ctx,
+ vdev->out_ctx, ep_index);
xhci_setup_no_streams_ep_input_ctx(xhci, ep_ctx,
&vdev->eps[ep_index]);
}
- xhci_setup_input_ctx_for_config_ep(xhci, udev->slot_id,
- changed_ep_bitmask, changed_ep_bitmask);
+ xhci_setup_input_ctx_for_config_ep(xhci, command->in_ctx,
+ vdev->out_ctx, changed_ep_bitmask, changed_ep_bitmask);
+ spin_unlock_irqrestore(&xhci->lock, flags);
- /* Issue and wait for the configure endpoint command */
- ret = xhci_configure_endpoint(xhci, udev, vdev, false);
+ /* Issue and wait for the configure endpoint command,
+ * which must succeed.
+ */
+ ret = xhci_configure_endpoint(xhci, udev, command,
+ false, true);
- xhci_zero_in_ctx(xhci, vdev);
/* xHC rejected the configure endpoint command for some reason, so we
* leave the streams rings intact.
*/
@@ -1746,7 +1783,7 @@ int xhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
/* FIXME Unset maxPstreams in endpoint context and
* update deq ptr to point to normal string ring.
*/
- vdev->eps[ep_index].ep_state &= ~EP_GETTING_STREAMS;
+ vdev->eps[ep_index].ep_state &= ~EP_GETTING_NO_STREAMS;
vdev->eps[ep_index].ep_state &= ~EP_HAS_STREAMS;
}
spin_unlock_irqrestore(&xhci->lock, flags);
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index e7753ad..b09650e 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -473,9 +473,15 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
xhci_dbg(xhci, "Allocating %u streams and %u "
"stream context array entries.\n",
num_streams, num_stream_ctxs);
+ if (xhci->cmd_ring_reserved_trbs == MAX_RSVD_CMD_TRBS) {
+ xhci_dbg(xhci, "Command ring has no reserved TRBs available\n");
+ return NULL;
+ }
+ xhci->cmd_ring_reserved_trbs++;
+
stream_info = kzalloc(sizeof(struct xhci_stream_info), mem_flags);
if (!stream_info)
- return NULL;
+ goto cleanup_trbs;
stream_info->num_streams = num_streams;
stream_info->num_stream_ctxs = num_stream_ctxs;
@@ -496,6 +502,12 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
memset(stream_info->stream_ctx_array, 0,
sizeof(struct xhci_stream_ctx)*num_stream_ctxs);
+ /* Allocate everything needed to free the stream rings later */
+ stream_info->free_streams_command =
+ xhci_alloc_command(xhci, true, mem_flags);
+ if (!stream_info->free_streams_command)
+ goto cleanup_ctx;
+
INIT_RADIX_TREE(&stream_info->trb_address_map, GFP_ATOMIC);
/* Allocate rings for all the streams that the driver will use,
@@ -553,10 +565,13 @@ cleanup_rings:
stream_info->stream_rings[cur_stream] = NULL;
}
}
+ xhci_free_command(xhci, stream_info->free_streams_command);
cleanup_ctx:
kfree(stream_info->stream_rings);
cleanup_info:
kfree(stream_info);
+cleanup_trbs:
+ xhci->cmd_ring_reserved_trbs--;
return NULL;
}
/*
@@ -622,6 +637,8 @@ void xhci_free_stream_info(struct xhci_hcd *xhci,
stream_info->stream_rings[cur_stream] = NULL;
}
}
+ xhci_free_command(xhci, stream_info->free_streams_command);
+ xhci->cmd_ring_reserved_trbs--;
if (stream_info->stream_ctx_array)
xhci_free_stream_ctx(xhci,
stream_info->num_stream_ctxs,
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index db676e3..bdc088a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -843,11 +843,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci->devs[slot_id]->eps[ep_index].ep_state &=
~EP_HALTED;
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
- } else {
- /* Must have been for streams configuration. */
- xhci->devs[slot_id]->cmd_status =
- GET_COMP_CODE(event->status);
- complete(&xhci->devs[slot_id]->cmd_completion);
}
break;
case TRB_TYPE(TRB_EVAL_CONTEXT):
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index c0236c4..d86ecc3 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -679,6 +679,7 @@ struct xhci_stream_info {
dma_addr_t ctx_array_dma;
/* For mapping physical TRB addresses to segments in stream rings */
struct radix_tree_root trb_address_map;
+ struct xhci_command *free_streams_command;
};
#define SMALL_STREAM_ARRAY_SIZE 256
@@ -698,6 +699,8 @@ struct xhci_virt_ep {
/* Transitioning the endpoint to using streams, don't enqueue URBs */
#define EP_GETTING_STREAMS (1 << 2)
#define EP_HAS_STREAMS (1 << 3)
+/* Transitioning the endpoint to not using streams, don't enqueue URBs */
+#define EP_GETTING_NO_STREAMS (1 << 4)
/* ---- Related to URB cancellation ---- */
struct list_head cancelled_td_list;
unsigned int cancels_pending;
--
1.6.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
