From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752947AbbCJStV (ORCPT ); Tue, 10 Mar 2015 14:49:21 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:34015 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1752029AbbCJStT (ORCPT ); Tue, 10 Mar 2015 14:49:19 -0400 Date: Tue, 10 Mar 2015 14:49:18 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Mathias Nyman cc: =?ISO-8859-1?Q?J=F6rg_Otte?= , Linux Kernel Mailing List , Greg Kroah-Hartman , USB list , Linus Torvalds , Mauro Carvalho Chehab Subject: Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD In-Reply-To: <54FF3632.3080305@linux.intel.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 10 Mar 2015, Mathias Nyman wrote: > > Mathias: > > > > Your patch description says this: > > > >> The endpoint might already processesed some TRBs on the endpiont ring > >> before we soft reset the endpoint. > >> Make sure we set the dequeue pointer to where we were befere soft reset > > > > However, if a driver tries to issue an endpoint reset while there are > > still some URBs queued, it is a bug. Host controller drivers shouldn't > > have to worry about this -- xhci_endpoint_reset() should simply return > > an error if the endpoint ring isn't empty. > > > > I suppose we should check for this in the USB core. I'll write a patch > > and CC: you. > > > > Alan Stern > > > > It's possible that there's something in usb core as well, > but I think the following was what happened: > > 1. First a normal configure endpoint command is issued, it sets endpoint dequeue pointer > to xxx400 = start of ring segment > 2. two urbs get queued -> two TDs put on endpoint ring. > 3. xhci executes those, ring is in running (idle) state. sw dequeue at xxx430, No TDs queued. > Endpoint dequeue pointer is not written to the endpoint output context as the ring is still > in running state (even if idle, not advancing with no TDs queued) it still shows xxx400 > 4. -> something happends, xhci_endpoint_reset() is called, we do a new configure endpoint > to 'soft reset' the endpiont, but we copy the dequeue pointer from the old endpoint > output context to the configure endpoint input context, which re-initializes the old > dequeue xxx400 pointer to xhci hardware, and it starts executing the old TDs from the ring. Obviously that's bad. But don't you have to stop the endpoint ring in order to configure it? When you stop the ring, doesn't the controller store the correct current value of the dequeue pointer somewhere? > 5. xhci driver notices that we get events for old TRBs that do not belong to the TD the driver > thinks we should be handling Alan Stern