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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sat, 30 May 2015 07:02:21 +0530
From:	sundeep subbaraya <sundeep.lkml@...il.com>
To:	"balbi@...com" <balbi@...com>
Cc:	Paul Zimmerman <Paul.Zimmerman@...opsys.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: DWC3 linux driver query

Hi,

On Fri, May 29, 2015 at 8:35 PM, Felipe Balbi <balbi@...com> wrote:
> On Fri, May 29, 2015 at 07:01:16PM +0530, sundeep subbaraya wrote:
>> Hi Felipe,
>>
>> On Thu, May 28, 2015 at 7:41 PM, Felipe Balbi <balbi@...com> wrote:
>> > Hi,
>> >
>> > On Thu, May 28, 2015 at 04:53:09PM +0530, sundeep subbaraya wrote:
>> >> Hi Felipe and Paul,
>> >
>> > btw, Paul has left Synopys :-)
>>
>> ahh I see..
>> >
>> >> I am seeing an issue while testing iperf for USB ethernet gadget with
>> >> dwc3 controller in 2.0 mode. After debugging I figured out that:
>> >>
>> >> 1. Network gadget queues say 3 requests. (for IN endpoint)
>> >> 2. It turns out with req.no_interrupt flag,
>> >>     DWC3 driver issues START_TRANSFER with req0:IOC, req1, req2:LST
>> >> 3. As per driver state machine, we get XFERNOTREADY then prepare these
>> >> TRBs and issue start transfer. Make Endpoint state as Busy.
>> >> 4. Endpoint state is set to free in XFERINPROGRESS or XFERCOMPLETE event.
>> >> 5. The issue I see here is there are NAKs going to host (seen in
>> >> analyzer) in between req0 and req2 hence XFERNOTREADY(Transfer Active)
>> >> events in between XFERINPROGRESS and          XFERCOMPLETE  events.
>> >> 6. As a result, EP is set as free in XFERINPROGRESS, since EP is free
>> >> start transfer command is issued in XFERNOTREADY handler.The command
>> >> fails since controller did not release the xfer resource yet.
>> >>
>> >> I feel controller behaviour is fine since it sends NAK and writes that
>> >> event. Driver may have to be modified to make EP as free only in
>> >> XFERCOMPLETE event handler (ofcourse not for Isoc).
>> >
>> > this sounds like the correct solution.
>> >
>> >> Note I am testing on a platform which is very slow (the interface
>> >> between DDR and core runs at 4Mhz).
>> >
>> > sweet :-)
>>
>> No it is not :) :). I struggled a lot while debugging :(
>>
>> >
>> >> Where as on Zynq (DWC3 in PL), a faster system compared to above one I
>> >
>> > hey, when will we see a glue layer for Zynq ? :-)
>>
>> we don't have hardware with 2.0 and 3.0 PHY together currently. I will
>> write and test
>> once the hardware is ready :)
>>
>> >
>> >> do not see any NAKs in between Start transfer command and XFERCOMPLETE
>> >> event.
>> >>
>> >> What do you guys say? Do you agree linux driver has to be modified or
>> >> Core should never issue NAKs in between Start transfer and
>> >> XFERCOMPLETE?
>> >
>> > well, if we queued enough transfers, it shouldn't NAK. OTOH, we
>> > shouldn't allow for a new StartTransfer command until all pending
>> > requests have been transferred.
>>
>> Yes correct but the hardware design is very slow here so hitting this case
>> >
>> >> A patch correcting DEPCMD status macros has been applied. Thank you
>> >> Felipe for trace points in driver otherwise it would have taken very
>> >> long time to figure out the root cause :) .
>> >
>> > yeah, those are really helpful :-)
>> >
>> >> Below is the trace log:(enabled only for IN bulk endpoint)
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.713513: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Not Active
>> >>      irq/97-dwc3-1308  [001] d...   553.713768: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68580 dma 011c60a2 length 1558 IOC
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.714266: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a687c0 dma 011c10a2 length 1558
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.714753: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68700 dma 011c18a2 length 1558 last IOC
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.717768: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.718203: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.718412: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.718638: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.718837: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.719049: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.719248: dwc3_msg: ep1in-bulk
>> >> XFERINPROGRESS
>> >>      irq/97-dwc3-1308  [001] d...   553.719520: dwc3_msg: request
>> >> ffffffc039a68580 from ep1in-bulk completed 1558/1558 ===> 0
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.720225: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.720612: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.720826: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.721026: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.721243: dwc3_msg: ep1in-bulk EP BUSY
>> >>      irq/97-dwc3-1308  [001] d...   553.721446: dwc3_msg: ep1in-bulk
>> >> XFERCOMPLETE
>> >>      irq/97-dwc3-1308  [001] d...   553.721711: dwc3_msg: request
>> >> ffffffc039a687c0 from ep1in-bulk completed 1558/1558 ===> 0
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.722411: dwc3_msg: request
>> >> ffffffc039a68700 from ep1in-bulk completed 1558/1558 ===> 0
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.722910: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Not Active
>> >>      irq/97-dwc3-1308  [001] d...   553.723159: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68ac0 dma 398b18a2 length 1558
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.723649: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68c40 dma 3a1ce8a2 length 1558
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.724136: dwc3_msg: ep1in-bulk:
>> >> req ffffffc039a68580 dma 3cc258a2 length 1558 last
>> >>
>> >>      irq/97-dwc3-1308  [001] d...   553.724722: dwc3_msg: CMD Error:1 for ep 3
>> >>      irq/97-dwc3-1308  [001] d...   553.727245: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.727767: dwc3_msg: CMD Error:1 for ep 3
>> >>      irq/97-dwc3-1308  [001] d...   553.728049: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.728394: dwc3_msg: CMD Error:1 for ep 3
>> >>      irq/97-dwc3-1308  [001] d...   553.731226: dwc3_msg: ep1in-bulk
>> >> XFERNOTREADY.Transfer Active
>> >>      irq/97-dwc3-1308  [001] d...   553.731658: dwc3_msg: CMD Error:1 for ep 3
>> >
>> > Can you try patch below ?
>> >
>> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> > index a03a485205c7..cad747865ce0 100644
>> > --- a/drivers/usb/dwc3/gadget.c
>> > +++ b/drivers/usb/dwc3/gadget.c
>> > @@ -1907,12 +1907,16 @@ static void dwc3_endpoint_transfer_complete(struct dwc3 *dwc,
>> >  {
>> >         unsigned                status = 0;
>> >         int                     clean_busy;
>> > +       u32                     is_xfer_complete;
>> > +
>> > +       is_xfer_complete = (event->endpoint_event == DWC3_DEPEVT_XFERCOMPLETE);
>> >
>> >         if (event->status & DEPEVT_STATUS_BUSERR)
>> >                 status = -ECONNRESET;
>>
>> >
>> >         clean_busy = dwc3_cleanup_done_reqs(dwc, dep, event, status);
>> > -       if (clean_busy)
>> > +       if (clean_busy & (is_xfer_complete ||
>> > +                               usb_endpoint_xfer_isoc(dep->endpoint.desc)))
>>
>> works perfect now and it should be &&.
>> Thanks for the immediate patch. What's next? Shall I send patch for
>> this or you applied this one already?
>
> I'll send this formally and apply for v4.2 merge window with a Cc stable
> tag. Can I add your Tested-by ?

Sure. Thanks.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ