| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20170427102033.5500-1-matwey@sai.msu.ru>
Date: Thu, 27 Apr 2017 13:20:33 +0300
From: "Matwey V. Kornilov" <matwey@....msu.ru>
To: b-liu@...com, gregkh@...uxfoundation.org
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
"Matwey V. Kornilov" <matwey@....msu.ru>
Subject: [PATCH] usb: musb: musb_host: Introduce postponed URB giveback
This commit changes the order of actions undertaken in
musb_advance_schedule() in order to overcome issue with broken
isochronous transfer [1].
There is no harm to split musb_giveback into two pieces. The first
unlinks finished urb, the second givebacks it. The issue here that
givebacking may be quite time-consuming due to urb->complete() call.
As it happens in case of pwc-driven web cameras. It may take about 0.5
ms to call __musb_giveback() that calls urb->callback() internally.
Under specific circumstances setting MUSB_RXCSR_H_REQPKT in subsequent
musb_start_urb() for the next urb will be too late to produce physical
IN packet. Since auto req is not used by this module the exchange
would be as the following:
[ ] 7.220456 d= 0.000997 [182 + 3.667] [ 3] IN : 4.5
[ T ] 7.220459 d= 0.000003 [182 + 7.000] [800] DATA0: [skipped]
[ ] 7.222456 d= 0.001997 [184 + 3.667] [ 3] IN : 4.5
[ ] 7.222459 d= 0.000003 [184 + 7.000] [ 3] DATA0: 00 00
It is known that missed IN in isochronous mode makes some
perepherial broken. For instance, pwc-driven or uvc-driven
web cameras.
In order to workaround this issue we postpone calling
urb->callback() after setting MUSB_RXCSR_H_REQPKT for the
next urb if there is the next urb pending in queue.
[1] https://www.spinics.net/lists/linux-usb/msg145747.html
Fixes: f551e1352983 ("Revert "usb: musb: musb_host: Enable HCD_BH flag to handle urb return in bottom half"")
Signed-off-by: Matwey V. Kornilov <matwey@....msu.ru>
---
drivers/usb/musb/musb_host.c | 54 +++++++++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index ac3a4952abb4..b590c2555dab 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -299,19 +299,24 @@ musb_start_urb(struct musb *musb, int is_in, struct musb_qh *qh)
}
}
-/* Context: caller owns controller lock, IRQs are blocked */
-static void musb_giveback(struct musb *musb, struct urb *urb, int status)
+static void __musb_giveback(struct musb *musb, struct urb *urb, int status)
__releases(musb->lock)
__acquires(musb->lock)
{
- trace_musb_urb_gb(musb, urb);
-
- usb_hcd_unlink_urb_from_ep(musb->hcd, urb);
spin_unlock(&musb->lock);
usb_hcd_giveback_urb(musb->hcd, urb, status);
spin_lock(&musb->lock);
}
+/* Context: caller owns controller lock, IRQs are blocked */
+static void musb_giveback(struct musb *musb, struct urb *urb, int status)
+{
+ trace_musb_urb_gb(musb, urb);
+
+ usb_hcd_unlink_urb_from_ep(musb->hcd, urb);
+ __musb_giveback(musb, urb, status);
+}
+
/* For bulk/interrupt endpoints only */
static inline void musb_save_toggle(struct musb_qh *qh, int is_in,
struct urb *urb)
@@ -346,6 +351,7 @@ static void musb_advance_schedule(struct musb *musb, struct urb *urb,
struct musb_hw_ep *ep = qh->hw_ep;
int ready = qh->is_ready;
int status;
+ int postponed_giveback = 0;
status = (urb->status == -EINPROGRESS) ? 0 : urb->status;
@@ -361,9 +367,35 @@ static void musb_advance_schedule(struct musb *musb, struct urb *urb,
break;
}
- qh->is_ready = 0;
- musb_giveback(musb, urb, status);
- qh->is_ready = ready;
+ usb_hcd_unlink_urb_from_ep(musb->hcd, urb);
+
+ /* It may take about 0.5 ms to call __musb_giveback() that
+ * calls urb->callback() internally. Under specific circumstances
+ * setting MUSB_RXCSR_H_REQPKT in subsequent musb_start_urb() for the
+ * next urb will be too late to produce physical IN packet. Since
+ * auto req is not used by this module the exchange would be as the
+ * following:
+ *
+ * [ ] 7.220456 d= 0.000997 [182 + 3.667] [ 3] IN : 4.5
+ * [ T ] 7.220459 d= 0.000003 [182 + 7.000] [800] DATA0: [skipped]
+ * [ ] 7.222456 d= 0.001997 [184 + 3.667] [ 3] IN : 4.5
+ * [ ] 7.222459 d= 0.000003 [184 + 7.000] [ 3] DATA0: 00 00
+ *
+ * It is known that missed IN in isochronous mode makes some
+ * perepherial broken. For instance, pwc-driven or uvc-driven
+ * web cameras.
+ * In order to workaround this issue we postpone calling
+ * urb->callback() after setting MUSB_RXCSR_H_REQPKT for the
+ * next urb if there is the next urb pending in queue.
+ */
+ if (is_in && qh->type == USB_ENDPOINT_XFER_ISOC
+ && !list_empty(&qh->hep->urb_list)) {
+ postponed_giveback = 1;
+ } else {
+ qh->is_ready = 0;
+ __musb_giveback(musb, urb, status);
+ qh->is_ready = ready;
+ }
/* reclaim resources (and bandwidth) ASAP; deschedule it, and
* invalidate qh as soon as list_empty(&hep->urb_list)
@@ -428,6 +460,12 @@ static void musb_advance_schedule(struct musb *musb, struct urb *urb,
hw_ep->epnum, is_in ? 'R' : 'T', next_urb(qh));
musb_start_urb(musb, is_in, qh);
}
+
+ if (postponed_giveback) {
+ qh->is_ready = 0;
+ __musb_giveback(musb, urb, status);
+ qh->is_ready = ready;
+ }
}
static u16 musb_h_flush_rxfifo(struct musb_hw_ep *hw_ep, u16 csr)
--
2.12.0
Powered by blists - more mailing lists