In such a scenario, device-A with slot_id equal to 1 is disconnecting while device-B is enumerating, device-B will fail to enumerate in the follow sequence.
1.[device-A] send disable slot command 2.[device-B] send enable slot command 3.[device-A] disable slot command completed and wakeup waiting thread 4.[device-B] enable slot command completed with slot_id equal to 1 and wakeup waiting thread 5.[device-B] driver check this slot_id was used by someone(device-A) in xhci_alloc_virt_device, this device fails to enumerate as this conflict 6.[device-A] xhci->devs[slot_id] set to NULL in xhci_free_virt_device
To fix driver's slot_id resources conflict, let the xhci_free_virt_device functionm call in the interrupt handler when disable slot command success.
Cc: stable@vger.kernel.org Fixes: 7faac1953ed1 ("xhci: avoid race between disable slot command and host runtime suspend") Signed-off-by: Weitao Wang WeitaoWang-oc@zhaoxin.com --- v1->v2 - Adjust the lock position in the function xhci_free_dev.
drivers/usb/host/xhci-hub.c | 5 +++-- drivers/usb/host/xhci-ring.c | 7 +++++-- drivers/usb/host/xhci.c | 35 +++++++++++++++++++++++++---------- 3 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 92bb84f8132a..fd8a64aa5779 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -705,10 +705,11 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci, continue;
retval = xhci_disable_slot(xhci, i); - xhci_free_virt_device(xhci, i); - if (retval) + if (retval) { xhci_err(xhci, "Failed to disable slot %d, %d. Enter test mode anyway\n", i, retval); + xhci_free_virt_device(xhci, i); + } } spin_lock_irqsave(&xhci->lock, *flags); /* Put all ports to the Disable state by clear PP */ diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 94c9c9271658..93dc28399c3c 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1589,7 +1589,8 @@ static void xhci_handle_cmd_enable_slot(int slot_id, struct xhci_command *comman command->slot_id = 0; }
-static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id) +static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id, + u32 cmd_comp_code) { struct xhci_virt_device *virt_dev; struct xhci_slot_ctx *slot_ctx; @@ -1604,6 +1605,8 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id) if (xhci->quirks & XHCI_EP_LIMIT_QUIRK) /* Delete default control endpoint resources */ xhci_free_device_endpoint_resources(xhci, virt_dev, true); + if (cmd_comp_code == COMP_SUCCESS) + xhci_free_virt_device(xhci, slot_id); }
static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id) @@ -1853,7 +1856,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, xhci_handle_cmd_enable_slot(slot_id, cmd, cmd_comp_code); break; case TRB_DISABLE_SLOT: - xhci_handle_cmd_disable_slot(xhci, slot_id); + xhci_handle_cmd_disable_slot(xhci, slot_id, cmd_comp_code); break; case TRB_CONFIG_EP: if (!cmd->completion) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 8a819e853288..6c6f6ebb8953 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3931,13 +3931,14 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd, * the USB device has been reset. */ ret = xhci_disable_slot(xhci, udev->slot_id); - xhci_free_virt_device(xhci, udev->slot_id); if (!ret) { ret = xhci_alloc_dev(hcd, udev); if (ret == 1) ret = 0; else ret = -EINVAL; + } else { + xhci_free_virt_device(xhci, udev->slot_id); } return ret; } @@ -4085,11 +4086,12 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev) for (i = 0; i < 31; i++) virt_dev->eps[i].ep_state &= ~EP_STOP_CMD_PENDING; virt_dev->udev = NULL; - xhci_disable_slot(xhci, udev->slot_id); - - spin_lock_irqsave(&xhci->lock, flags); - xhci_free_virt_device(xhci, udev->slot_id); - spin_unlock_irqrestore(&xhci->lock, flags); + ret = xhci_disable_slot(xhci, udev->slot_id); + if (ret) { + spin_lock_irqsave(&xhci->lock, flags); + xhci_free_virt_device(xhci, udev->slot_id); + spin_unlock_irqrestore(&xhci->lock, flags); + }
}
@@ -4128,9 +4130,20 @@ int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
wait_for_completion(command->completion);
- if (command->status != COMP_SUCCESS) + if (command->status != COMP_SUCCESS) { xhci_warn(xhci, "Unsuccessful disable slot %u command, status %d\n", slot_id, command->status); + switch (command->status) { + case COMP_COMMAND_ABORTED: + case COMP_COMMAND_RING_STOPPED: + xhci_warn(xhci, "Timeout while waiting for disable slot command\n"); + ret = -ETIME; + break; + default: + ret = -EINVAL; + break; + } + }
xhci_free_command(xhci, command);
@@ -4243,8 +4256,9 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) return 1;
disable_slot: - xhci_disable_slot(xhci, udev->slot_id); - xhci_free_virt_device(xhci, udev->slot_id); + ret = xhci_disable_slot(xhci, udev->slot_id); + if (ret) + xhci_free_virt_device(xhci, udev->slot_id);
return 0; } @@ -4381,10 +4395,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
mutex_unlock(&xhci->mutex); ret = xhci_disable_slot(xhci, udev->slot_id); - xhci_free_virt_device(xhci, udev->slot_id); if (!ret) { if (xhci_alloc_dev(hcd, udev) == 1) xhci_setup_addressable_virt_dev(xhci, udev); + } else { + xhci_free_virt_device(xhci, udev->slot_id); } kfree(command->completion); kfree(command);