Hi Thinh,
On 11/23/22 7:36 AM, Thinh Nguyen wrote:
On Tue, Nov 22, 2022, Udipto Goswami wrote:
Hi Thinh,
On 11/22/22 7:00 AM, Thinh Nguyen wrote:
Hi Udipto,
On Fri, Nov 18, 2022, Udipto Goswami wrote:
Hi Thinh
On 11/18/22 7:31 AM, Thinh Nguyen wrote:
On Thu, Nov 17, 2022, Udipto Goswami wrote:
A dequeue for ep0 need to adjust the handling based on the
data stage and status stage. Currently if ep0 is in data/status
stage the handling isn't that different, driver will try giveback
as part of dequeue process which might potentially lead to the
controller accessing invalid trbs.
Also for ep0 the requests aren't moved into the started_list,
which might potentially lead to the un-mapping of the request
buffers without sending endxfer.
Maybe we need to track started_list for control endpoint? If the request
isn't prepared yet or that the transfer had completed, then there's no
need to issue End Tranfer command.
But I believe sending End Transfer for inactive endpoint should be fine
also. Then we maybe able to get away without checking the started list.
If you're planning to do that, please test and note it somewhere.
thanks for the suggestion, sure i'll do some more experiments and confirm
it.
Just curious, how do you hit/test this scenario?
For other endpoint types, I can see possible scenarios where a dequeue
may be needed, but I don't see one for control transfer.
The host can cancel the control transfer, and the controller will see
"setup_packet_pending" and handle accordingly. If there's a disconnect,
that's also handled separately by the controller driver also. So, where
does ep0_dequeue come into play here?
adding the reference to other thread [1]
[1]: https://urldefense.com/v3/__https://www.spinics.net/lists/linux-usb/msg233862.html__;!!A4F2R9G_pg!Z6hsArtLfeqmaf08IqxTov5VyXdvLJuncb8wIXYHC5PW7Zk7WO6u_r8Ap1gR-TlrmzwmQEQ-cJElQ2ED_0deM8t49emcjw$
was trying to address a race condition in the ffs driver where ep_dequeue
was suggested, before freeing the request dequeue it.
as per the current code, since ep0 req isn't moved to started list
therefore it will exit from this in ep_dequeue:
list_for_each_entry(r, &dep->pending_list, list) {
if (r == req) {
dwc3_gadget_giveback(dep, req, -ECONNRESET);
goto out;
}
}
but if the ep0 is in data/status phase technically it is still active.
We will unmap the buffer and giveback then the ep0 is in data/status stage.
This could potentially happen right?
The intent of a separate dequeue was to address that scenario when the
data/status phase isn't completed.
Hope this give some clarity.
I'm still unclear how it can lead to this condition. Can you list the
sequence of events that can lead to this scenario.
For functionfs_unbind() to occur, shouldn't there be a disconnect call,
triggering soft-disconnect? When that happens, there are checks in dwc3
to ensure that any pending control transfer will be STALLed and given
back. Any new control request will not be queued, and no active control
transfer should happen at this point.
Thanks for pointing this out. Yah i see the soft_disconnect already
addresses what we are trying to do with a separate ep0_dequeue. I
believe we don't need this change separately.
Thanks again for a clarifications.
-Udipto
