| 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: <20150529150549.GC2026@saruman.tx.rr.com>
Date: Fri, 29 May 2015 10:05:49 -0500
From: Felipe Balbi <balbi@...com>
To: sundeep subbaraya <sundeep.lkml@...il.com>
CC: "balbi@...com" <balbi@...com>,
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
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 ?
--
balbi
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists