The xhci driver rely on a couple helper variables, addr_dev and slot_id
in struct xhci_hcd which are not thread safe. They used to be protected
by the usb_address0_mutex, but this was changed in
commit 638139eb95d2 ("usb: hub: allow to process more usb hub events in
parallel")
As xhci controls two busses, addr_dev and slot_id can now race, which
occurred as an intermittent failure to initialise a 10-port hub (with
three internal VL812 4-port hub controllers) on boot, with a failure rate
of around 8%
Fix this by removing addr_dev and slot_id helpers from struct xhci_hcd,
and use the per-command xhci_command structure introduced in 3.16 for
storning completion and slot ID.
Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
---
drivers/usb/host/xhci-mem.c | 1 -
drivers/usb/host/xhci-ring.c | 11 +----------
drivers/usb/host/xhci.c | 22 ++++++++++++++--------
drivers/usb/host/xhci.h | 5 +----
4 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index f833640..626346b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2513,7 +2513,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
* something other than the default (~1ms minimum between interrupts).
* See section 5.5.1.2.
*/
- init_completion(&xhci->addr_dev);
for (i = 0; i < MAX_HC_SLOTS; ++i)
xhci->devs[i] = NULL;
for (i = 0; i < USB_MAXCHILDREN; ++i) {
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f5397a5..49cc702 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1070,15 +1070,6 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id,
}
}
-static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, int slot_id,
- u32 cmd_comp_code)
-{
- if (cmd_comp_code == COMP_SUCCESS)
- xhci->slot_id = slot_id;
- else
- xhci->slot_id = 0;
-}
-
static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id)
{
struct xhci_virt_device *virt_dev;
@@ -1337,7 +1328,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb->generic.field[3]));
switch (cmd_type) {
case TRB_ENABLE_SLOT:
- xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
+ cmd->slot_id = slot_id;
break;
case TRB_DISABLE_SLOT:
xhci_handle_cmd_disable_slot(xhci, slot_id);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..52acd1d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3685,16 +3685,16 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
int ret;
struct xhci_command *command;
- command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+ command = xhci_alloc_command(xhci, false, true, GFP_KERNEL);
if (!command)
return 0;
spin_lock_irqsave(&xhci->lock, flags);
- command->completion = &xhci->addr_dev;
ret = xhci_queue_slot_control(xhci, command, TRB_ENABLE_SLOT, 0);
if (ret) {
spin_unlock_irqrestore(&xhci->lock, flags);
xhci_dbg(xhci, "FIXME: allocate a command ring segment\n");
+ kfree(command->completion);
kfree(command);
return 0;
}
@@ -3703,11 +3703,12 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
wait_for_completion(command->completion);
- if (!xhci->slot_id || command->status != COMP_SUCCESS) {
+ if (!command->slot_id || command->status != COMP_SUCCESS) {
xhci_err(xhci, "Error while assigning device slot ID\n");
xhci_err(xhci, "Max number of devices this xHCI host supports is %u.\n",
HCS_MAX_SLOTS(
readl(&xhci->cap_regs->hcs_params1)));
+ kfree(command->completion);
kfree(command);
return 0;
}
@@ -3728,11 +3729,11 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
* xhci_discover_or_reset_device(), which may be called as part of
* mass storage driver error handling.
*/
- if (!xhci_alloc_virt_device(xhci, xhci->slot_id, udev, GFP_NOIO)) {
+ if (!xhci_alloc_virt_device(xhci, command->slot_id, udev, GFP_NOIO)) {
xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
goto disable_slot;
}
- udev->slot_id = xhci->slot_id;
+ udev->slot_id = command->slot_id;
#ifndef CONFIG_USB_DEFAULT_PERSIST
/*
@@ -3744,6 +3745,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
#endif
+ kfree(command->completion);
kfree(command);
/* Is this a LS or FS device under a HS hub? */
/* Hub or peripherial? */
@@ -3752,10 +3754,11 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
disable_slot:
/* Disable slot, if we can do it without mem alloc */
spin_lock_irqsave(&xhci->lock, flags);
+ kfree(command->completion);
command->completion = NULL;
command->status = 0;
if (!xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
- udev->slot_id))
+ command->slot_id))
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
return 0;
@@ -3808,18 +3811,18 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
}
}
- command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+ command = xhci_alloc_command(xhci, false, true, GFP_KERNEL);
if (!command)
return -ENOMEM;
command->in_ctx = virt_dev->in_ctx;
- command->completion = &xhci->addr_dev;
slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx);
if (!ctrl_ctx) {
xhci_warn(xhci, "%s: Could not get input context, bad type.\n",
__func__);
+ kfree(command->completion);
kfree(command);
return -EINVAL;
}
@@ -3848,6 +3851,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
spin_unlock_irqrestore(&xhci->lock, flags);
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
"FIXME: allocate a command ring segment");
+ kfree(command->completion);
kfree(command);
return ret;
}
@@ -3897,6 +3901,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
break;
}
if (ret) {
+ kfree(command->completion);
kfree(command);
return ret;
}
@@ -3932,6 +3937,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
"Internal device address = %d",
le32_to_cpu(slot_ctx->dev_state) & DEV_ADDR_MASK);
+ kfree(command->completion);
kfree(command);
return 0;
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8e421b8..9e5f8b1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -734,6 +734,7 @@ struct xhci_command {
/* Input context for changing device state */
struct xhci_container_ctx *in_ctx;
u32 status;
+ u32 slot_id;
/* If completion is null, no one is waiting on this command
* and the structure can be freed after the command completes.
*/
@@ -1495,10 +1496,6 @@ struct xhci_hcd {
struct xhci_scratchpad *scratchpad;
/* Store LPM test failed devices' information */
struct list_head lpm_failed_devs;
-
- /* slot enabling and address device helpers */
- struct completion addr_dev;
- int slot_id;
/* For USB 3.0 LPM enable/disable. */
struct xhci_command *lpm_command;
/* Internal mirror of the HW's dcbaa */
--
1.8.3.2
--
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
