[V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Jörg Otte]

If I plug in my USB DVB-T stick I get the following in dmesg:

dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state.
dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer.
DVB: registering new adapter (TerraTec/qanu USB2.0 Highspeed DVB-T Receiver)
usb 1-1: DVB: registering adapter 0 frontend 0 (TerraTec/qanu USB2.0
Highspeed DVB-T Receiver)...
input: IR-receiver inside an USB DVB receiver as
/devices/pci0000:00/0000:00:14.0/usb1/1-1/input/input17
dvb-usb: schedule remote query interval to 50 msecs.
xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of
current TD ep_index 1 comp_code 1
xhci_hcd 0000:00:14.0: Looking for event-dma 0000000207540400
trb-start 0000000207540420 trb-end 0000000207540420 seg-start
00000002075404
xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of
current TD ep_index 1 comp_code 1
xhci_hcd 0000:00:14.0: Looking for event-dma 0000000207540410
trb-start 0000000207540420 trb-end 0000000207540420 seg-start
00000002075404
dvb-usb: bulk message failed: -110 (2/0)

and DVB-T is not functional. The problem came in with:

1163d50 Merge tag 'usb-4.0-rc3' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb

I never had this xhci_hcd error before so this is a regression.


Thanks, Jörg

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Mathias Nyman]

On 10.03.2015 11:40, Jörg Otte wrote:
> If I plug in my USB DVB-T stick I get the following in dmesg:
> 
> dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state.
> dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer.
> DVB: registering new adapter (TerraTec/qanu USB2.0 Highspeed DVB-T Receiver)
> usb 1-1: DVB: registering adapter 0 frontend 0 (TerraTec/qanu USB2.0
> Highspeed DVB-T Receiver)...
> input: IR-receiver inside an USB DVB receiver as
> /devices/pci0000:00/0000:00:14.0/usb1/1-1/input/input17
> dvb-usb: schedule remote query interval to 50 msecs.
> xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of
> current TD ep_index 1 comp_code 1
> xhci_hcd 0000:00:14.0: Looking for event-dma 0000000207540400
> trb-start 0000000207540420 trb-end 0000000207540420 seg-start
> 00000002075404
> xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of
> current TD ep_index 1 comp_code 1
> xhci_hcd 0000:00:14.0: Looking for event-dma 0000000207540410
> trb-start 0000000207540420 trb-end 0000000207540420 seg-start
> 00000002075404
> dvb-usb: bulk message failed: -110 (2/0)
> 
> and DVB-T is not functional. The problem came in with:
> 
> 1163d50 Merge tag 'usb-4.0-rc3' of
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
> 
> I never had this xhci_hcd error before so this is a regression.
> 
> 
> Thanks, Jörg

Oh, thanks.

Looks like we get an event for a TRB we just moved past. 

Any chance you could take a log with xhci debugging enabled before attaching the DVB-T
stick?

echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control


I'd suspect one of these two patches:

commit 45ba2154d12fc43b70312198ec47085f10be801a
    xhci: fix reporting of 0-sized URBs in control endpoint

commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
    xhci: Clear the host side toggle manually when endpoint is 'soft reset'

-Mathias

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Jörg Otte]

[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]

2015-03-10 14:06 GMT+01:00 Mathias Nyman <mathias.nyman@linux.intel.com>:
> On 10.03.2015 11:40, Jörg Otte wrote:
>> If I plug in my USB DVB-T stick I get the following in dmesg:
>>
>> dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state.
>> dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer.
>> DVB: registering new adapter (TerraTec/qanu USB2.0 Highspeed DVB-T Receiver)
>> usb 1-1: DVB: registering adapter 0 frontend 0 (TerraTec/qanu USB2.0
>> Highspeed DVB-T Receiver)...
>> input: IR-receiver inside an USB DVB receiver as
>> /devices/pci0000:00/0000:00:14.0/usb1/1-1/input/input17
>> dvb-usb: schedule remote query interval to 50 msecs.
>> xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of
>> current TD ep_index 1 comp_code 1
>> xhci_hcd 0000:00:14.0: Looking for event-dma 0000000207540400
>> trb-start 0000000207540420 trb-end 0000000207540420 seg-start
>> 00000002075404
>> xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of
>> current TD ep_index 1 comp_code 1
>> xhci_hcd 0000:00:14.0: Looking for event-dma 0000000207540410
>> trb-start 0000000207540420 trb-end 0000000207540420 seg-start
>> 00000002075404
>> dvb-usb: bulk message failed: -110 (2/0)
>>
>> and DVB-T is not functional. The problem came in with:
>>
>> 1163d50 Merge tag 'usb-4.0-rc3' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
>>
>> I never had this xhci_hcd error before so this is a regression.
>>
>>
>> Thanks, Jörg
>
> Oh, thanks.
>
> Looks like we get an event for a TRB we just moved past.
>
> Any chance you could take a log with xhci debugging enabled before attaching the DVB-T
> stick?
>
> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control
>
>

here it comes attached.


> I'd suspect one of these two patches:
>
> commit 45ba2154d12fc43b70312198ec47085f10be801a
>     xhci: fix reporting of 0-sized URBs in control endpoint
>
> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
>     xhci: Clear the host side toggle manually when endpoint is 'soft reset'
>
> -Mathias
>

Thanks, Jörg

[-- Attachment #2: xhci-debug.gz --]
[-- Type: application/x-gzip, Size: 22035 bytes --]

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Jörg Otte]

2015-03-10 15:03 GMT+01:00 Jörg Otte <jrg.otte@gmail.com>:
> 2015-03-10 14:06 GMT+01:00 Mathias Nyman <mathias.nyman@linux.intel.com>:
>> On 10.03.2015 11:40, Jörg Otte wrote:
>>> If I plug in my USB DVB-T stick I get the following in dmesg:
>>>
>>> dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state.
>>> dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer.
>>> DVB: registering new adapter (TerraTec/qanu USB2.0 Highspeed DVB-T Receiver)
>>> usb 1-1: DVB: registering adapter 0 frontend 0 (TerraTec/qanu USB2.0
>>> Highspeed DVB-T Receiver)...
>>> input: IR-receiver inside an USB DVB receiver as
>>> /devices/pci0000:00/0000:00:14.0/usb1/1-1/input/input17
>>> dvb-usb: schedule remote query interval to 50 msecs.
>>> xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of
>>> current TD ep_index 1 comp_code 1
>>> xhci_hcd 0000:00:14.0: Looking for event-dma 0000000207540400
>>> trb-start 0000000207540420 trb-end 0000000207540420 seg-start
>>> 00000002075404
>>> xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of
>>> current TD ep_index 1 comp_code 1
>>> xhci_hcd 0000:00:14.0: Looking for event-dma 0000000207540410
>>> trb-start 0000000207540420 trb-end 0000000207540420 seg-start
>>> 00000002075404
>>> dvb-usb: bulk message failed: -110 (2/0)
>>>
>>> and DVB-T is not functional. The problem came in with:
>>>
>>> 1163d50 Merge tag 'usb-4.0-rc3' of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
>>>
>>> I never had this xhci_hcd error before so this is a regression.
>>>
>>>
>>> Thanks, Jörg
>>
>> Oh, thanks.
>>
>> Looks like we get an event for a TRB we just moved past.
>>
>> Any chance you could take a log with xhci debugging enabled before attaching the DVB-T
>> stick?
>>
>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control
>>
>>
>
> here it comes attached.
>
>
>> I'd suspect one of these two patches:
>>
>> commit 45ba2154d12fc43b70312198ec47085f10be801a
>>     xhci: fix reporting of 0-sized URBs in control endpoint
>>
>> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
>>     xhci: Clear the host side toggle manually when endpoint is 'soft reset'
>>

Revert the commits.
The second one  "xhci: Clear the host side..."  is it !

Thanks, Jörg

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Mathias Nyman]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On 10.03.2015 17:36, Jörg Otte wrote:

>>> Any chance you could take a log with xhci debugging enabled before attaching the DVB-T
>>> stick?
>>>
>>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control
>>>
>>>
>>
>> here it comes attached.
>>
>>
>>> I'd suspect one of these two patches:
>>>
>>> commit 45ba2154d12fc43b70312198ec47085f10be801a
>>>     xhci: fix reporting of 0-sized URBs in control endpoint
>>>
>>> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
>>>     xhci: Clear the host side toggle manually when endpoint is 'soft reset'
>>>
> 
> Revert the commits.
> The second one  "xhci: Clear the host side..."  is it !
> 

Yes, thank you

Seems that It wasn't mature enough, I'll revert it.

>From your logs I can see what went wrong, 

If you still have some time, could you try out a patch (attached) and see if it solves the
issue for you. (on top of clean 4.0-rc3). I can't reproduce it with my own USB DVB-T device

-Mathias 


[-- Attachment #2: 0001-xhci-set-correct-dequeue-status-in-endpoint-soft-res.patch --]
[-- Type: text/x-patch, Size: 1493 bytes --]

>From a895eb69a63dfef1943f0593da29167bea12100c Mon Sep 17 00:00:00 2001
From: Mathias Nyman <mathias.nyman@linux.intel.com>
Date: Tue, 10 Mar 2015 18:50:45 +0200
Subject: [PATCH] xhci: set correct dequeue status in endpoint soft reset

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

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b06d1a5..64527a4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2972,6 +2972,8 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
 	unsigned int ep_index, ep_state;
 	unsigned long flags;
 	u32 ep_flag;
+	struct xhci_ep_ctx *ep_ctx;
+	dma_addr_t addr;
 
 	xhci = hcd_to_xhci(hcd);
 	udev = (struct usb_device *) ep->hcpriv;
@@ -3046,6 +3048,9 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
 					   virt_dev->out_ctx, ctrl_ctx,
 					   ep_flag, ep_flag);
 	xhci_endpoint_copy(xhci, command->in_ctx, virt_dev->out_ctx, ep_index);
+	ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index);
+	addr = xhci_trb_virt_to_dma(virt_ep->ring->deq_seg, virt_ep->ring->dequeue);
+	ep_ctx->deq  = cpu_to_le64(addr | virt_ep->ring->cycle_state);
 
 	xhci_queue_configure_endpoint(xhci, command, command->in_ctx->dma,
 				     udev->slot_id, false);
-- 
1.8.3.2

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Alan Stern]

On Tue, 10 Mar 2015, Mathias Nyman wrote:

> Yes, thank you
> 
> Seems that It wasn't mature enough, I'll revert it.
> 
> From your logs I can see what went wrong, 
> 
> If you still have some time, could you try out a patch (attached) and see if it solves the
> issue for you. (on top of clean 4.0-rc3). I can't reproduce it with my own USB DVB-T device

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

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Mathias Nyman]

On 10.03.2015 19:29, Alan Stern wrote:
> On Tue, 10 Mar 2015, Mathias Nyman wrote:
> 
>> Yes, thank you
>>
>> Seems that It wasn't mature enough, I'll revert it.
>>
>> From your logs I can see what went wrong, 
>>
>> If you still have some time, could you try out a patch (attached) and see if it solves the
>> issue for you. (on top of clean 4.0-rc3). I can't reproduce it with my own USB DVB-T device
> 
> 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.
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

-Mathias

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Alan Stern]

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

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Lu, Baolu]

On 03/11/2015 02:49 AM, Alan Stern wrote:
> 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.

Is it possible to return an error message up to client driver? The 
client driver then decides
how to handle this kind of error. It, possibly, unlink all ongoing 
transfers and ask host driver
to soft reset this endpoint. When xhci_endpoint_reset is called, there 
should be no ongoing
transfers.

Thanks,
Baolu

> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Alan Stern]

On Wed, 11 Mar 2015, Lu, Baolu wrote:

> >> 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.
> 
> Is it possible to return an error message up to client driver? The 
> client driver then decides
> how to handle this kind of error. It, possibly, unlink all ongoing 
> transfers and ask host driver
> to soft reset this endpoint. When xhci_endpoint_reset is called, there 
> should be no ongoing
> transfers.

That doesn't seem to be the problem here.  Mathias is saying that all
the transfers have indeed completed, but when reconfiguring the
endpoint, the driver tells the controller that some transfers are still
active (because it stores a stale copy of the dequeue pointer).

But Mathias, what about the cycle bits in the TRBs?  Wouldn't they be
set to indicate that the OS now owns the TRBs?  This would cause the
endpoint to stop working, not cause the sort of error that Jörg saw.  
Or does the reconfigure command also store a stale copy of the Dequeue
Cycle State setting?

Alan Stern

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Mathias Nyman]

On 11.03.2015 16:03, Alan Stern wrote:
> On Wed, 11 Mar 2015, Lu, Baolu wrote:
> 
>>>> 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.
>>
>> Is it possible to return an error message up to client driver? The 
>> client driver then decides
>> how to handle this kind of error. It, possibly, unlink all ongoing 
>> transfers and ask host driver
>> to soft reset this endpoint. When xhci_endpoint_reset is called, there 
>> should be no ongoing
>> transfers.
> 
> That doesn't seem to be the problem here.  Mathias is saying that all
> the transfers have indeed completed, but when reconfiguring the
> endpoint, the driver tells the controller that some transfers are still
> active (because it stores a stale copy of the dequeue pointer).
> 
> But Mathias, what about the cycle bits in the TRBs?  Wouldn't they be
> set to indicate that the OS now owns the TRBs?  This would cause the
> endpoint to stop working, not cause the sort of error that Jörg saw.  
> Or does the reconfigure command also store a stale copy of the Dequeue
> Cycle State setting?

xhci keeps track of a producer cycle state and consumer cycle state.

These are only updated when the producer or consumer  (enqueue ptr=producer, dequeue ptr=consumer in this case)
pass the last link TRB of the last segment. The cycle bit in a TRB is only written once,
together when the producer writes the trb to the ring.

The TRB cycle bit at the dequeue pointer is compared to the consumer cycle state.  

So the cycle bit check would only mismatch if the actual sw dequeue pointer just passed the last TRB
of the last segment, and the stale dequeue pointer in the output context would roll it back past that
TRB again.

-Mathias
 

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Mathias Nyman]

On 10.03.2015 20:49, Alan Stern wrote:
> 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?
> 

Normally we stop the endpoint before configuring it, but in this case
the endpoint is already configured, and we don't really want to change the configuration,
we just want to reset the toggle so that it's in sync with the device.

As I understand the xhci specs allows us to issue a configure endpoint command 
for a running endpoint as long as it's empty. 

xhci 1.0  4.6.6 Configure Endpoint:  
"An endpoint shall be in the Stopped state or if in the Running state shall be “idle” (e.g. no USB
Transactions are in progress, the Transfer Ring is empty, and software has processed all
outstanding events for the Transfer Ring) if its Drop Context flag is set. If this condition is not met
undefined behavior may occur.

But the output context we copy is from last time the endpoint was stopped or configured.
So we need to update the dequeue pointer to the one we have in the driver, I need to check
if the other old fields in the output context can cause any issues as well.

-Mathias 

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Jörg Otte]

2015-03-10 18:04 GMT+01:00 Mathias Nyman <mathias.nyman@linux.intel.com>:
> On 10.03.2015 17:36, Jörg Otte wrote:
>
>>>> Any chance you could take a log with xhci debugging enabled before attaching the DVB-T
>>>> stick?
>>>>
>>>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control
>>>>
>>>>
>>>
>>> here it comes attached.
>>>
>>>
>>>> I'd suspect one of these two patches:
>>>>
>>>> commit 45ba2154d12fc43b70312198ec47085f10be801a
>>>>     xhci: fix reporting of 0-sized URBs in control endpoint
>>>>
>>>> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
>>>>     xhci: Clear the host side toggle manually when endpoint is 'soft reset'
>>>>
>>
>> Revert the commits.
>> The second one  "xhci: Clear the host side..."  is it !
>>
>
> Yes, thank you
>
> Seems that It wasn't mature enough, I'll revert it.
>
> From your logs I can see what went wrong,
>
> If you still have some time, could you try out a patch (attached) and see if it solves the
> issue for you. (on top of clean 4.0-rc3). I can't reproduce it with my own USB DVB-T device

Problems:
error: patch failed: drivers/usb/host/xhci.c:2972
error: drivers/usb/host/xhci.c: patch does not apply

For me the patch looks formally good.
No idea why.

Thanks, Jörg

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Jörg Otte]

2015-03-11 12:01 GMT+01:00 Jörg Otte <jrg.otte@gmail.com>:
> 2015-03-10 18:04 GMT+01:00 Mathias Nyman <mathias.nyman@linux.intel.com>:
>> On 10.03.2015 17:36, Jörg Otte wrote:
>>
>>>>> Any chance you could take a log with xhci debugging enabled before attaching the DVB-T
>>>>> stick?
>>>>>
>>>>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control
>>>>>
>>>>>
>>>>
>>>> here it comes attached.
>>>>
>>>>
>>>>> I'd suspect one of these two patches:
>>>>>
>>>>> commit 45ba2154d12fc43b70312198ec47085f10be801a
>>>>>     xhci: fix reporting of 0-sized URBs in control endpoint
>>>>>
>>>>> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
>>>>>     xhci: Clear the host side toggle manually when endpoint is 'soft reset'
>>>>>
>>>
>>> Revert the commits.
>>> The second one  "xhci: Clear the host side..."  is it !
>>>
>>
>> Yes, thank you
>>
>> Seems that It wasn't mature enough, I'll revert it.
>>
>> From your logs I can see what went wrong,
>>
>> If you still have some time, could you try out a patch (attached) and see if it solves the
>> issue for you. (on top of clean 4.0-rc3). I can't reproduce it with my own USB DVB-T device
>
> Problems:
> error: patch failed: drivers/usb/host/xhci.c:2972
> error: drivers/usb/host/xhci.c: patch does not apply
>
> For me the patch looks formally good.
> No idea why.

OK, finally I got it applied successfully.
I can confirm now it works for me.

Thanks, Jörg

Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD

[Mathias Nyman]

On 11.03.2015 18:16, Jörg Otte wrote:
> 2015-03-11 12:01 GMT+01:00 Jörg Otte <jrg.otte@gmail.com>:
>> 2015-03-10 18:04 GMT+01:00 Mathias Nyman <mathias.nyman@linux.intel.com>:
>>> On 10.03.2015 17:36, Jörg Otte wrote:
>>>
>>>>>> I'd suspect one of these two patches:
>>>>>>
>>>>>> commit 45ba2154d12fc43b70312198ec47085f10be801a
>>>>>>     xhci: fix reporting of 0-sized URBs in control endpoint
>>>>>>
>>>>>> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
>>>>>>     xhci: Clear the host side toggle manually when endpoint is 'soft reset'
>>>>>>
>>>>
>>>> Revert the commits.
>>>> The second one  "xhci: Clear the host side..."  is it !
>>>>
>>>
>>> Yes, thank you
>>>
>>> Seems that It wasn't mature enough, I'll revert it.
>>>
>>> From your logs I can see what went wrong,
>>>
>>> If you still have some time, could you try out a patch (attached) and see if it solves the
>>> issue for you. (on top of clean 4.0-rc3). I can't reproduce it with my own USB DVB-T device
>>
>> Problems:
>> error: patch failed: drivers/usb/host/xhci.c:2972
>> error: drivers/usb/host/xhci.c: patch does not apply
>>
>> For me the patch looks formally good.
>> No idea why.
> 
> OK, finally I got it applied successfully.
> I can confirm now it works for me.
> 

Great, Thanks

-Mathias