From 2596c65b168d68e3673d21f0ae3c484a48000bfe Mon Sep 17 00:00:00 2001 From: Yue Wang Date: Fri, 23 Aug 2019 15:19:27 +0800 Subject: [PATCH] usb: detect stop endpoint race using pending timer instead of counter. [1/1] PD#SWPL-13177 Problem: stop endpoint race Solution: 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. Verify: franklin Change-Id: Ie958ffd530a6bd176d0cf451894a5bd4dece38da Signed-off-by: Yue Wang --- drivers/usb/host/xhci-ring.c | 32 +++++++++++++++++++++++++++++++- drivers/usb/host/xhci.c | 12 ++++++++++++ drivers/usb/host/xhci.h | 10 ++++++++-- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 64d38cc..46804aa 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -393,8 +393,13 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, * pointer command pending because the device can choose to start any * stream once the endpoint is on the HW schedule. */ +#ifdef CONFIG_AMLOGIC_USB + if ((ep_state & EP_STOP_CMD_PENDING) || (ep_state & SET_DEQ_PENDING) + || (ep_state & EP_HALTED)) +#else if ((ep_state & EP_HALT_PENDING) || (ep_state & SET_DEQ_PENDING) || (ep_state & EP_HALTED)) +#endif return; writel(DB_VALUE(ep_index, stream_id), db_addr); /* The CPU has better things to do at this point than wait for a @@ -632,13 +637,19 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci, struct xhci_virt_ep *ep) { +#ifdef CONFIG_AMLOGIC_USB + ep->ep_state &= ~EP_STOP_CMD_PENDING; + /* Can't del_timer_sync in interrupt */ + del_timer(&ep->stop_cmd_timer); +#else ep->ep_state &= ~EP_HALT_PENDING; - /* Can't del_timer_sync in interrupt, so we attempt to cancel. If the + /* Can't del_timer_sync in interrupt, so we attempt to cancel. If the * timer is running on another CPU, we don't decrement stop_cmds_pending * (since we didn't successfully stop the watchdog timer). */ if (del_timer(&ep->stop_cmd_timer)) ep->stop_cmds_pending--; +#endif } /* Must be called with xhci->lock held in interrupt context */ @@ -902,10 +913,15 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, * simple flag to say whether there is a pending stop endpoint command for a * particular endpoint. * +#ifdef CONFIG_AMLOGIC_USB + * Instead we use a combination of that flag and checking if a new timer is + * pending. +#else * Instead we use a combination of that flag and a counter for the number of * pending stop endpoint commands. If the timer is the tail end of the last * stop endpoint command, and the endpoint's command is still pending, we assume * the host is dying. +#endif */ void xhci_stop_endpoint_command_watchdog(unsigned long arg) { @@ -919,12 +935,21 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) spin_lock_irqsave(&xhci->lock, flags); +#ifdef CONFIG_AMLOGIC_USB + /* bail out if cmd completed but raced with stop ep watchdog timer.*/ + if (!(ep->ep_state & EP_STOP_CMD_PENDING) || + timer_pending(&ep->stop_cmd_timer)) { +#else ep->stop_cmds_pending--; if (!(ep->stop_cmds_pending == 0 && (ep->ep_state & EP_HALT_PENDING))) { xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Stop EP timer ran, but no command pending, " "exiting."); +#endif spin_unlock_irqrestore(&xhci->lock, flags); +#ifdef CONFIG_AMLOGIC_USB + xhci_dbg(xhci, "Stop EP timer raced with cmd completion, exit"); +#endif return; } @@ -933,7 +958,12 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) /* Oops, HC is dead or dying or at least not responding to the stop * endpoint command. */ + xhci->xhc_state |= XHCI_STATE_DYING; +#ifdef CONFIG_AMLOGIC_USB + ep->ep_state &= ~EP_STOP_CMD_PENDING; +#endif + /* Disable interrupts from the host controller and start halting it */ xhci_quiesce(xhci); spin_unlock_irqrestore(&xhci->lock, flags); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1c9a49d..c1a30982 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1589,14 +1589,22 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) /* Queue a stop endpoint command, but only if this is * the first cancellation to be handled. */ +#ifdef CONFIG_AMLOGIC_USB + if (!(ep->ep_state & EP_STOP_CMD_PENDING)) { +#else if (!(ep->ep_state & EP_HALT_PENDING)) { +#endif command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC); if (!command) { ret = -ENOMEM; goto done; } +#ifdef CONFIG_AMLOGIC_USB + ep->ep_state |= EP_STOP_CMD_PENDING; +#else ep->ep_state |= EP_HALT_PENDING; ep->stop_cmds_pending++; +#endif ep->stop_cmd_timer.expires = jiffies + XHCI_STOP_EP_CMD_TIMEOUT * HZ; add_timer(&ep->stop_cmd_timer); @@ -3642,7 +3650,11 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev) /* Stop any wayward timer functions (which may grab the lock) */ for (i = 0; i < 31; ++i) { +#ifdef CONFIG_AMLOGIC_USB + virt_dev->eps[i].ep_state &= ~EP_STOP_CMD_PENDING; +#else virt_dev->eps[i].ep_state &= ~EP_HALT_PENDING; +#endif del_timer_sync(&virt_dev->eps[i].stop_cmd_timer); } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 8c81244..4899577 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -917,7 +917,11 @@ struct xhci_virt_ep { unsigned int ep_state; #define SET_DEQ_PENDING (1 << 0) #define EP_HALTED (1 << 1) /* For stall handling */ -#define EP_HALT_PENDING (1 << 2) /* For URB cancellation */ +#ifdef CONFIG_AMLOGIC_USB +#define EP_STOP_CMD_PENDING (1 << 2) /* For URB cancellation */ +#else +#define EP_HALT_PENDING (1 << 2) /* For URB cancellation */ +#endif /* Transitioning the endpoint to using streams, don't enqueue URBs */ #define EP_GETTING_STREAMS (1 << 3) #define EP_HAS_STREAMS (1 << 4) @@ -929,7 +933,9 @@ struct xhci_virt_ep { unsigned int stopped_stream; /* Watchdog timer for stop endpoint command to cancel URBs */ struct timer_list stop_cmd_timer; - int stop_cmds_pending; +#ifndef CONFIG_AMLOGIC_USB + int stop_cmds_pending; +#endif struct xhci_hcd *xhci; /* Dequeue pointer and dequeue segment for a submitted Set TR Dequeue * command. We'll need to update the ring's dequeue segment and dequeue -- 2.7.4