linux-kernel - Re: [PATCH v2] usb: dwc3: gadget: check drained isoc ep

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zie5sN473m2rgNTK@pengutronix.de>
Date: Tue, 23 Apr 2024 15:37:52 +0200
From: Michael Grzeschik <mgr@...gutronix.de>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] usb: dwc3: gadget: check drained isoc ep

Hi Thinh,

On Thu, Apr 04, 2024 at 12:29:14AM +0000, Thinh Nguyen wrote:
>On Tue, Apr 02, 2024, Thinh Nguyen wrote:
>> On Tue, Apr 02, 2024, Thinh Nguyen wrote:
>> > My concern here is for the case where transfer_in_flight == true and
>>
>> I mean transfer_in_flight == false
>>
>> > list_empty(started_list) == false. That means that the requests in the
>> > started_list are completed but are not given back to the gadget driver.
>> >
>> > Since they remained in the started_list, they will be resubmitted again
>> > on the next usb_ep_queue. We may send duplicate transfers right?
>
>Actually, since the requests are completed, the HWO bits are cleared,
>nothing is submitted and no duplicate. But since the requests are not
>given back yet from the started_list, then the next Start_Transfer
>command will begin with the TRB address of the completed request
>(HWO=0), the controller may not process the next TRBs. Have you tested
>this scenario?
>
>> >
>> > You can try to cleanup requests in the started_list, but you need to be
>> > careful to make sure you're not out of sync with the transfer completion
>> > events and new requests from gadget driver.
>> >
>
>Was the problem you encounter due to no_interrupt settings where the
>it was set to the last request of the uvc data pump?
>
>if that's the case, can UVC function driver make sure to not set
>no_interrupt to the last request of the data pump from the UVC?

Actually no. What I want to do is to ensure that the dwc3 stream
is stopped when the hardware was drained. Which is a valid point
in my case. Since we are actually potentially enqueueing new request
in the complete handler, be it zero length or real transfers.

Calling kick_transfer on an drained hw will absolutely run into
missed isocs if the irq thread was called late. We saw this on real hardware,
where another irq_thread was scheduled with the same priority as the
dwc3 irq_thread but was running so long that the HW was running dry in
between the hw irq and the actual dwc3_irq_thread run.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)