On Mon, 2013-03-18 at 04:22 +0000, Ben Hutchings wrote: > 3.2-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > > commit feca7746d5d9e84b105a613b7f3b6ad00d327372 upstream. > > This patch (as1661) fixes a rather obscure bug in ehci-hcd. In a > couple of places, the driver compares the DMA address stored in a QH's > overlay region with the address of a particular qTD, in order to see > whether that qTD is the one currently being processed by the hardware. > (If it is then the status in the QH's overlay region is more > up-to-date than the status in the qTD, and if it isn't then the > overlay's value needs to be adjusted when the QH is added back to the > active schedule.) > > However, DMA address in the overlay region isn't always valid. It > sometimes will contain a stale value, which may happen by coincidence > to be equal to a qTD's DMA address. Instead of checking the DMA > address, we should check whether the overlay region is active and > valid. The patch tests the ACTIVE bit in the overlay, and clears this > bit when the overlay becomes invalid (which happens when the > currently-executing URB is unlinked). > > This is the second part of a fix for the regression reported at: > > https://bugs.launchpad.net/bugs/1088733 Alan, the first part (commit 6402c796d3b4 aka as1660) didn't apply and I couldn't see how to adapt it for 3.2. Does this second part have any value without the first? Or, if you could provide a backport of the first part, that would be very much appreciated. Ben. > Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Reported-by: Joseph Salisbury <joseph.salisbury@xxxxxxxxxxxxx> > Reported-and-tested-by: Stephen Thirlwall <sdt@xxxxxx> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx> > --- > drivers/usb/host/ehci-q.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > --- a/drivers/usb/host/ehci-q.c > +++ b/drivers/usb/host/ehci-q.c > @@ -135,7 +135,7 @@ qh_refresh (struct ehci_hcd *ehci, struc > * qtd is updated in qh_completions(). Update the QH > * overlay here. > */ > - if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current) { > + if (qh->hw->hw_token & ACTIVE_BIT(ehci)) { > qh->hw->hw_qtd_next = qtd->hw_next; > qtd = NULL; > } > @@ -444,11 +444,19 @@ qh_completions (struct ehci_hcd *ehci, s > else if (last_status == -EINPROGRESS && !urb->unlinked) > continue; > > - /* qh unlinked; token in overlay may be most current */ > - if (state == QH_STATE_IDLE > - && cpu_to_hc32(ehci, qtd->qtd_dma) > - == hw->hw_current) { > + /* > + * If this was the active qtd when the qh was unlinked > + * and the overlay's token is active, then the overlay > + * hasn't been written back to the qtd yet so use its > + * token instead of the qtd's. After the qtd is > + * processed and removed, the overlay won't be valid > + * any more. > + */ > + if (state == QH_STATE_IDLE && > + qh->qtd_list.next == &qtd->qtd_list && > + (hw->hw_token & ACTIVE_BIT(ehci))) { > token = hc32_to_cpu(ehci, hw->hw_token); > + hw->hw_token &= ~ACTIVE_BIT(ehci); > > /* An unlink may leave an incomplete > * async transaction in the TT buffer. > -- Ben Hutchings When you say `I wrote a program that crashed Windows', people just stare ... and say `Hey, I got those with the system, *for free*'. - Linus Torvalds
Attachment:
signature.asc
Description: This is a digitally signed message part