summaryrefslogtreecommitdiffstats
path: root/drivers/usb/host/xhci-ring.c
Commit message (Collapse)AuthorAgeFilesLines
...
* xhci: Find out where an endpoint or stream stopped from its context.Mathias Nyman2017-06-031-5/+12
| | | | | | | | | | | | | | | | | | When xHC is asked to stop an endpoint it will save the position it stopped on in the endpoint or stream context. xhci driver needs to know if the controller stopped on the exact same TRB that the driver was asked to cancel as it then needs to move past the TD instead of turning the TD to no-op TRBs. xhci driver used to get the stopped position from a "stopped" transfer event before the stop endpoint command completed, but if the ring is already stopped, or in a halted or error state this event is missing. Get the stopped position from the endpoint or stream context instead Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: remove unused stopped_td pointerMathias Nyman2017-06-031-4/+0Star
| | | | | | | | We no longer keep track of where we stopped in a stopped_td pointer. We get the ring dequeue pointer from the endpoint or stream context Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: Fix command ring stop regression in 4.11Mathias Nyman2017-05-181-4/+4
| | | | | | | | | | | | | | In 4.11 TRB completion codes were renamed to match spec. Completion codes for command ring stopped and endpoint stopped were mixed, leading to failures while handling a stopped command ring. Use the correct completion code for command ring stopped events. Fixes: 0b7c105a04ca ("usb: host: xhci: rename completion codes to match spec") Cc: <stable@vger.kernel.org> # 4.11 Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* USB: xhci: fix lock-inversion problemAlan Stern2017-05-181-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With threaded interrupts, bottom-half handlers are called with interrupts enabled. Therefore they can't safely use spin_lock(); they have to use spin_lock_irqsave(). Lockdep warns about a violation occurring in xhci_irq(): ========================================================= [ INFO: possible irq lock inversion dependency detected ] 4.11.0-rc8-dbg+ #1 Not tainted --------------------------------------------------------- swapper/7/0 just changed the state of lock: (&(&ehci->lock)->rlock){-.-...}, at: [<ffffffffa0130a69>] ehci_hrtimer_func+0x29/0xc0 [ehci_hcd] but this lock took another, HARDIRQ-unsafe lock in the past: (hcd_urb_list_lock){+.....} and interrupts could create inverse lock ordering between them. other info that might help us debug this: Possible interrupt unsafe locking scenario: CPU0 CPU1 ---- ---- lock(hcd_urb_list_lock); local_irq_disable(); lock(&(&ehci->lock)->rlock); lock(hcd_urb_list_lock); <Interrupt> lock(&(&ehci->lock)->rlock); *** DEADLOCK *** no locks held by swapper/7/0. the shortest dependencies between 2nd lock and 1st lock: -> (hcd_urb_list_lock){+.....} ops: 252 { HARDIRQ-ON-W at: __lock_acquire+0x602/0x1280 lock_acquire+0xd5/0x1c0 _raw_spin_lock+0x2f/0x40 usb_hcd_unlink_urb_from_ep+0x1b/0x60 [usbcore] xhci_giveback_urb_in_irq.isra.45+0x70/0x1b0 [xhci_hcd] finish_td.constprop.60+0x1d8/0x2e0 [xhci_hcd] xhci_irq+0xdd6/0x1fa0 [xhci_hcd] usb_hcd_irq+0x26/0x40 [usbcore] irq_forced_thread_fn+0x2f/0x70 irq_thread+0x149/0x1d0 kthread+0x113/0x150 ret_from_fork+0x2e/0x40 This patch fixes the problem. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Reported-and-tested-by: Bart Van Assche <bart.vanassche@sandisk.com> CC: <stable@vger.kernel.org> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci-ring: don't need to clear interrupt pending for MSI enabled hcdPeter Chen2017-05-181-4/+1Star
| | | | | | | | | | | | | | | | | | | According to xHCI spec Figure 30: Interrupt Throttle Flow Diagram If PCI Message Signaled Interrupts (MSI or MSI-X) are enabled, then the assertion of the Interrupt Pending (IP) flag in Figure 30 generates a PCI Dword write. The IP flag is automatically cleared by the completion of the PCI write. the MSI enabled HCs don't need to clear interrupt pending bit, but hcd->irq = 0 doesn't equal to MSI enabled HCD. At some Dual-role controller software designs, it sets hcd->irq as 0 to avoid HCD requesting interrupt, and they want to decide when to call usb_hcd_irq by software. Signed-off-by: Peter Chen <peter.chen@nxp.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: trace URB before giving it back instead of afterMathias Nyman2017-05-181-1/+1
| | | | | | | | Don't access any members of a URB after giving it back. URB might be freed by then already. Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: remove xhci_debug_trb()Lu Baolu2017-04-081-4/+0Star
| | | | | | | | | | Every XHCI TRB has already been traced by the trb trace events. It is unnecessary to put the same message in kernel log. This patch removes xhci_debug_trb(). Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: remove ring debugging codeLu Baolu2017-04-081-4/+0Star
| | | | | | | | | | XHCI ring changes have already been traced by the ring trace events. It's unnecessary to put the same messages in kernel log. This patch removes the debugging code for a ring. Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: remove enq_updates and deq_updates from ringLu Baolu2017-04-081-3/+0Star
| | | | | | | | | | | | enq_updates and deq_updates were introduced in the first place to check whether an xhci hardware is able to respond to trbs enqueued in the ring. We now have trb tracers to trace every single enqueue/dequeue trb. It's time to remove them and the associated debugging code. Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: remove error messages for failed memory allocationLu Baolu2017-04-081-6/+4Star
| | | | | | | | Omit extra messages for memory allocation failure. Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: add xhci_log_ring trace eventsLu Baolu2017-04-081-0/+5
| | | | | | | | | | | | | | | | | | This patch creates a new event class called xhci_log_ring, and defines the events used for tracing the change of all kinds of rings used by an xhci host. An xHCI ring is basically a memory block shared between software and hardware. By tracing changes of rings, it makes the life easier for debugging hardware or software problems. This info can be used, later, to print, in a human readable way, the life cycle of an xHCI ring using the trace-cmd tool and the appropriate plugin. Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Reviewed-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: Rework how we handle unresponsive or hoptlug removed hostsMathias Nyman2017-04-081-61/+57Star
| | | | | | | | | | | | | | | | | | | | Introduce a new xhci_hc_died() function that takes care of handling pending commands and URBs if a host controller becomes unresponsive. This addresses issues on hotpluggable xhci controllers that disappear from the bus suddenly, often while the bus (PCI) remove function is still being processed. xhci_hc_died() sets a XHCI_STATUS_DYING flag to prevent new URBs and commands or to be queued. The flag also ensures xhci_hc_died() will give back pending commands and URBs once. Host is considered dead if register read returns 0xffffffff, or host fails to abort the command ring, or fails stopping an endpoint after trying for 5 seconds. Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: add slot and endpoint numbers to debug messages in handle_tx_eventZhengjun Xing2017-04-081-25/+55
| | | | | | | | | | | There's one annoyance in how xhci prints debug messages, we often get logs with messages but it's hard to say from which device and endpoint the message originates. Add slot_id, ep_index messages in handle_tx_event. Signed-off-by: Zhengjun Xing <zhengjun.xing@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: add Slot and EP Context tracersFelipe Balbi2017-04-081-0/+42
| | | | | | | | | | | With these, we can track what's happening with the HW while executing each and every command. It will give us visibility into how the different contexts are being modified by xHC which can bring insight into problems while debugging. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: Set URB actual length for stopped control transfersMathias Nyman2017-03-291-0/+3
| | | | | | | | | | | | | | | | A control transfer that stopped at the status stage incorrectly warned about a "unexpected TRB Type 4", and did not set the transferred actual_length for the URB. The URB actual_length for control transfers should contain the bytes transferred in the data stage. Bytes of a partially sent setup stage and missing bytes from status stage should be left out. Cc: <stable@vger.kernel.org> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: simplify how we store TDs in urb private dataMathias Nyman2017-01-251-10/+10
| | | | | | | | | | | | | | | | | | | | | Instead of storing a zero length array of td pointers, and then allocate memory both for the td pointer array and the td's, just use a zero length array of actual td's in urb private data. old: struct urb_priv { struct xhci_td *td[0] } new: struct urb_priv { struct xhci_td td[0] } Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: Rename variables related to transfer descritporsMathias Nyman2017-01-251-5/+5
| | | | | | | | | | | | | | urb_priv structure has a count on how many TDs the URB contains, and how many of those TD's we have handled. rename: length -> num_tds td_cnt -> num_tds_done No functional changes Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: add urb_enqueue/dequeue/giveback tracersFelipe Balbi2017-01-251-0/+1
| | | | | | | | | These three new tracers will help us tie TRBs into URBs by *also* looking into URB lifetime. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: make a generic TRB tracerFelipe Balbi2017-01-251-2/+12
| | | | | | | | | | | | instead of having a tracer that can only trace command completions, let's promote this tracer so it can trace and decode any TRB. With that, it will be easier to extrapolate the lifetime of any TRB which might help debugging certain issues. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: convert to list_for_each_entry_safe()Felipe Balbi2017-01-251-7/+8
| | | | | | | | | instead of using while(!list_empty()) followed by list_first_entry(), we can actually use list_for_each_entry_safe(). Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: use the trb_to_noop() helper for command trbsMathias Nyman2017-01-251-10/+2Star
| | | | | | | | | | Remove duplicate code by using trb_to_noop() when handling Aborted commads Based on earlier code by Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: Introduce helper to turn one TRB into a no-opMathias Nyman2017-01-251-12/+17
| | | | | | | | | | Useful for turning both transfer and command trbs into no-ops. Based on earlier code by Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: unconditionally call xhci_unmap_td_bounce_buffer()Felipe Balbi2017-01-251-6/+3Star
| | | | | | | | | | xhci_unmap_td_bounce_buffer() already checks for a valid td->bounce_seg and bails out early if that's invalid. There's no need to check for this twice. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: check for a valid ring when unmapping bounce bufferFelipe Balbi2017-01-251-1/+1
| | | | | | | | | This way we can remove checks for valid ring from call sites of xhci_unmap_td_bounce_buffer() Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: remove bogus __releases()/__acquires() annotationFelipe Balbi2017-01-251-2/+0Star
| | | | | | | | | handle_tx_event() is not releasing xhci->lock nor reacquiring it, remove the bogus annotation. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: introduce xhci_td_cleanup()Felipe Balbi2017-01-251-42/+50
| | | | | | | | | | | By extracting xhci_td_cleanup() from finish_td(), code before clearer and easier to follow. There are no functional changes with this patch. It's merely a cleanup. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: reorder variable definitionsFelipe Balbi2017-01-251-8/+4Star
| | | | | | | | | | no functional changes. Simple cleanup to make sure variables are ordered in a 'reverse christmas tree' fashion. While at that, also remove an obsolete comment which doesn't apply anymore. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: use slightly better list helpersFelipe Balbi2017-01-251-9/+10
| | | | | | | | | Replace list_entry() with list_first_entry() and list_for_each() with list_for_each_entry(). This makes the code slightly more readable. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: simplify irq handler returnFelipe Balbi2017-01-251-15/+17
| | | | | | | | | | Instead of having several return points, let's use a local variable and a single place to return. This makes the code slightly easier to read. [set ret = IRQ_HANDLED in default working case -Mathias] Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: rename completion codes to match specFelipe Balbi2017-01-251-62/+62
| | | | | | | | | | | Cleanup only. This patch is a mechaninal rename to make sure our macros for TRB completion codes match what the specification uses to refer to such errors. The idea behind this is that it makes it far easier to grep the specification and match it with implementation. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: use list_is_singular for cmd_listLu Baolu2017-01-251-3/+4
| | | | | | | | | Use list_is_singular() to check if cmd_list has only one entry. [use list_empty() in queue command instead -Mathias] Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: avoid unnecessary calculationLu Baolu2017-01-251-10/+10
| | | | | | | | | No need to calculate remainder and length_field, if there is no data phase of a control transfer. Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: remove unnecessary assignmentLu Baolu2017-01-251-2/+0Star
| | | | | | | | Drop an unnecessary assignment in prepare_transfer(). Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: remove unnecessary second abort tryLu Baolu2017-01-251-13/+5Star
| | | | | | | | | | | | The second try was a workaround for (what we thought was) command ring failing to stop in the first place. But this turns out to be due to the race that we have fixed(see "xhci: Fix race related to abort operation"). With that fix, it is time to remove the second try. Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: remove unnecessary check for pending timerMathias Nyman2017-01-251-2/+1Star
| | | | | | | | | Checking if the command timeout timer is pending when queueing the first command to the command ring is not really useful, remove it. Suggested-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: detect stop endpoint race using pending timer instead of counter.Mathias Nyman2017-01-251-16/+11Star
| | | | | | | | | | | | | | | | | A counter was used to find out if the stop endpoint completion raced with the stop endpoint timeout timer. This was needed in case the stop ep completion failed to delete the timer as it was running on anoter cpu. The EP_STOP_CMD_PENDING flag was not enough as a new stop endpoint command may be queued between the command completion and timeout function, which would set the flag back. Instead of the separate counter that was used we can detect the race by checking both the STOP_EP_PENDING flag and timer_pending in the timeout function. Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: rename EP_HALT_PENDING to EP_STOP_CMD_PENDINGMathias Nyman2017-01-251-3/+3
| | | | | | | | | We don't want to confuse halted and stalled endpoint states with a flag indicating we are waiting for a stop endpoint command to finish or timeout Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: simplify if statement to make it more readableMathias Nyman2017-01-251-1/+2
| | | | | | | No functional change, De Morgan !(A && B) = (!A || !B) Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: fix deadlock at host remove by running watchdog correctlyMathias Nyman2017-01-111-11/+0Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | If a URB is killed while the host is removed we can end up in a situation where the hub thread takes the roothub device lock, and waits for the URB to be given back by xhci-hcd, blocking the host remove code. xhci-hcd tries to stop the endpoint and give back the urb, but can't as the host is removed from PCI bus at the same time, preventing the normal way of giving back urb. Instead we need to rely on the stop command timeout function to give back the urb. This xhci_stop_endpoint_command_watchdog() timeout function used a XHCI_STATE_DYING flag to indicate if the timeout function is already running, but later this flag has been taking into use in other places to mark that xhci is dying. Remove checks for XHCI_STATE_DYING in xhci_urb_dequeue. We are still checking that reading from pci state does not return 0xffffffff or that host is not halted before trying to stop the endpoint. This whole area of stopping endpoints, giving back URBs, and the wathdog timeout need rework, this fix focuses on solving a specific deadlock issue that we can then send to stable before any major rework. Cc: <stable@vger.kernel.org> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: handle COMP_STOP from SETUP phase tooFelipe Balbi2017-01-031-12/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stop Endpoint command can come at any point and we have no control of that. We should make sure to handle COMP_STOP on SETUP phase as well, otherwise urb->actual_length might be set to negative values in some occasions such as below: urb->length = 4; build_control_transfer_td_for(urb, ep); stop_endpoint(ep); COMP_STOP: [...] urb->actual_length = urb->length - trb->length; trb->length is 8 for SETUP stage (8 control request bytes), so actual_length would be set to -4 in this case. While doing that, also make sure to use TRB_TYPE field of the actual TRB instead of matching pointers to figure out in which stage of the control transfer we got our completion event. Cc: <stable@vger.kernel.org> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: Fix race related to abort operationOGAWA Hirofumi2017-01-031-80/+88
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Current abort operation has race. xhci_handle_command_timeout() xhci_abort_cmd_ring() xhci_write_64(CMD_RING_ABORT) xhci_handshake(5s) do { check CMD_RING_RUNNING udelay(1) ... COMP_CMD_ABORT event COMP_CMD_STOP event xhci_handle_stopped_cmd_ring() restart cmd_ring CMD_RING_RUNNING become 1 again } while () return -ETIMEDOUT xhci_write_64(CMD_RING_ABORT) /* can abort random command */ To do abort operation correctly, we have to wait both of COMP_CMD_STOP event and negation of CMD_RING_RUNNING. But like above, while timeout handler is waiting negation of CMD_RING_RUNNING, event handler can restart cmd_ring. So timeout handler never be notice negation of CMD_RING_RUNNING, and retry of CMD_RING_ABORT can abort random command (BTW, I guess retry of CMD_RING_ABORT was workaround of this race). To fix this race, this moves xhci_handle_stopped_cmd_ring() to xhci_abort_cmd_ring(). And timeout handler waits COMP_CMD_STOP event. At this point, timeout handler is owner of cmd_ring, and safely restart cmd_ring by using xhci_handle_stopped_cmd_ring(). [FWIW, as bonus, this way would be easily extend to add CMD_RING_PAUSE operation] [locks edited as patch is rebased on other locking fixes -Mathias] Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: Use delayed_work instead of timer for command timeoutOGAWA Hirofumi2017-01-031-10/+16
| | | | | | | | | | | This is preparation to fix abort operation race (See "xhci: Fix race related to abort operation"). To make timeout sleepable, use delayed_work instead of timer. [change a newly added pending timer fix to pending work -Mathias] Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: hold lock over xhci_abort_cmd_ring()Lu Baolu2017-01-031-4/+9
| | | | | | | | | | | | | | | | | | | | | In command timer function, xhci_handle_command_timeout(), xhci->lock is unlocked before call into xhci_abort_cmd_ring(). This might cause race between the timer function and the event handler. The xhci_abort_cmd_ring() function sets the CMD_RING_ABORT bit in the command register and polling it until the setting takes effect. A stop command ring event might be handled between writing the abort bit and polling for it. The event handler will restart the command ring, which causes the failure of polling, and we ever believed that we failed to stop it. As a bonus, this also fixes some issues of calling functions without locking in xhci_handle_command_timeout(). Cc: <stable@vger.kernel.org> # 3.7+ Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* xhci: Handle command completion and timeout raceMathias Nyman2017-01-031-1/+5
| | | | | | | | | | | | | | | | | | | | | | | If we get a command completion event at the same time as the command timeout work starts on another cpu we might end up aborting the wrong command. If the command completion takes the xhci lock before the timeout work, it will handle the command, pick the next command, mark it as current_cmd, and re-queue the timeout work. When the timeout work finally gets the lock It will start aborting the wrong command. This case can be resolved by checking if the timeout work is pending inside the timeout function itself. A new timeout work can only be pending if the command completed and a new command was queued. If there are no more commands pending then command completion will set the current_cmd to NULL, which is already handled in the timeout work. Cc: <stable@vger.kernel.org> Reported-by: Baolin Wang <baolin.wang@linaro.org> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: host: xhci: Fix possible wild pointer when handling abort commandBaolin Wang2017-01-031-1/+4
| | | | | | | | | | | When current command was supposed to be aborted, host will free the command in handle_cmd_completion() function. But it might be still referenced by xhci->current_cmd, which need to set NULL. Cc: <stable@vger.kernel.org> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* usb: xhci: fix possible wild pointerLu Baolu2017-01-031-5/+11
| | | | | | | | | | | | | | | | | | handle_cmd_completion() frees a command structure which might be still referenced by xhci->current_cmd. This might cause problem when xhci->current_cmd is accessed after that. A real-life case could be like this. The host takes a very long time to respond to a command, and the command timer is fired at the same time when the command completion event arrives. The command completion handler frees xhci->current_cmd before the timer function can grab xhci->lock. Afterward, timer function grabs the lock and go ahead with checking and setting members of xhci->current_cmd. Cc: <stable@vger.kernel.org> # v3.16+ Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* Merge tag 'usb-for-v4.10' of ↵Greg Kroah-Hartman2016-11-181-3/+3
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-next Felipe writes: usb: patches for v4.10 merge window One big merge this time with a total of 166 non-merge commits. Most of the work, by far, is on dwc2 this time (68.2%) with dwc3 a far second (22.5%). The remaining 9.3% are scattered on gadget drivers. The most important changes for dwc2 are the peripheral side DMA support implemented by Synopsys folks and support for the new IOT dwc2 compatible core from Synopsys. In dwc3 land we have support for high-bandwidth, high-speed isochronous endpoints and some non-critical fixes for large scatter lists. Apart from these, we have our usual set of cleanups, non-critical fixes, etc.
| * usb: host: xhci: purge GET_MAX_PACKET()Felipe Balbi2016-11-031-3/+3
| | | | | | | | | | | | | | | | usb_endpoint_maxp() is now returning maxpacket correctly - iow only bits 10:0. We can finaly remove XHCI's private GET_MAX_PACKET macro. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
* | usb: xhci: move slot_id from xhci_hcd to xhci_command structureLu Baolu2016-11-141-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | xhci->slot_id is used for providing a way to pass slot id from the command completion handler to the function waiting for completion. It's shared by enumerations of all USB devices connected to an xhci host. Hence, it's a source for possible races. Since we've introduced command structure and the command queue to xhci driver. It's better to move slot_id from xhci_hcd structure to xhci_command structure. Hence the race source is removed. Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* | xhci: add helper to get the endpoint state of a endpoint contextMathias Nyman2016-11-141-12/+6Star
| | | | | | | | | | | | | | | | | | | | define GET_EP_CTX_STATE() macro to get the endpoint state from a pointer to a le32 enpoint context structure No functional changes Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>