On Thu, Aug 17, 2023, Alan Stern wrote: > On Fri, Aug 18, 2023 at 01:08:19AM +0000, Thinh Nguyen wrote: > > Hi, > > > > On Fri, Aug 18, 2023, Andrey Konovalov wrote: > > > Hi Alan and Thinh, > > > > > > I have been testing Raw Gadget with the dwc3 UDC driver and stumbled > > > upon an issue related to how dwc3 handles setup requests with wLength > > > == 0. > > > > > > When running a simple Raw Gadget-based keyboard emulator [1], > > > everything works as expected until the point when the host sends a > > > SET_CONFIGURATION request, which has wLength == 0. > > > > > > For setup requests with wLength != 0, just like the other UDC drivers > > > I tested, dwc3 calls the gadget driver's ->setup() callback and then > > > waits until the gadget driver queues an URB to EP0 as a response. > > > > For the lack of better term, can we use "request" or "usb_request" > > instead of URB for gadget side, I get confused with the host side > > whenever we mention URB. > > > > > > > > However, for a setup request with wLength == 0, dwc3 does not wait > > > until the gadget driver queues an URB to ack the transfer. It appears > > > that dwc3 just acks the request internally and then proceeds with > > > calling the ->setup() callback for the next request received from the > > > > It depends on the bRequest. It should not proceed to ->setup() unless > > the gadget driver already setups the request for it. > > Let's see if I understand what you're saying. Some control transfers > are handled directly by the UDC driver (things like SET_ADDRESS or > CLEAR_HALT). For these transfers, the ->setup() callback is not invoked > and the gadget driver is completely unaware of them. But for all other > control transfers, the ->setup() callback _is_ invoked. > > Is that what you meant? That's not what I meant. I was referring to the next request. It should not be processed until the first request is completed. Depending on the type of request, if there's a delayed_status, the dwc3 driver will not prepare for the Status stage and Setup stage (after status completion) to proceed to the _next_ ->setup callback. My understanding from the described problem is that somehow dwc3 processes the next request immediately without waiting for the raw gadget preparing the data stage. > > > > host. This confuses Raw Gadget, as it does not expect to get a new > > > ->setup() call before it explicitly acks the previous one by queuing > > > an URB. As a result, the emulation fails. > > > > If the host intent is to send a 3-stage control request with a 0-length > > data packet, the gadget driver needs to return USB_GADGET_DELAYED_STATUS > > to prepare a 0-length request. For SET_CONFIGURATION, we don't expect > > a data phase, why should the gadget driver queue a 0-length data? > > The USB-2 spec prohibits 3-stage control requests with wLength == 0 (see > sections 9.3.1 and 9.3.5). Therefore the host's intent can never be to > send a 3-stage control request with a 0-length Data-stage packet. Right. Forgot about that, but my point was about the sequential flow of the control transfer. > > > > I suspect this issue has not been observed with other gadget drivers, > > > as they queue an URB immediately after receiving a ->setup() call: > > > dwc3 appears to somehow correctly handle this internally even though > > > it acks the transfer by itself. But the timings with Raw Gadget are > > > different, as it requires userspace to ack the transfer. Sometimes > > > though, the Raw Gadget-based emulator also manages to queue an URB > > > before the next request is received from the host and the enumeration > > > continues properly (until the next request with wLength == 0). > > > > > > What do you think would be the best approach to deal with this? > > > > The communication should be clearly defined. That is, the dwc3 needs to > > know if this is a 3-stage or 2-stage control transfer. It knows about > > the standard requests, but not the vendor/non-standard ones. If the raw > > gadget defined some unknown OUT request, it needs to tell dwc3 whether > > it should expect the data stage or not. > > The communication _is_ clearly defined. Here's how it works: > > For control transfers that aren't handled directly by the UDC, the UDC > driver invokes the ->setup() callback and waits for the gadget driver to > queue a request. If the SETUP packet's wLength value is > 0 then the > gadget driver queues an IN or OUT request (depending on the transfer's > direction) and the UDC waits for the host to transfer the Data stage > packets, completing the request. After this happens, the UDC driver > automatically queues an internal 0-length request in the opposite > direction for the Status stage. Data-stage transfers are not allowed to > span more than one usb_request. I was talking in context of 0-length transfer (albeit I forgot about the special case of control OUT doesn't have 3-stage). If it's a vendor request 0-length transfer, without responding with USB_GADGET_DELAYED_STATUS, the dwc3 will proceed with preparing the status stage. > > (IMO that automatic action is a design flaw; the UDC driver should wait > for the gadget driver to explictly queue a 0-length request or a STALL > instead of doing it automatically.) Would every UDC has this capability? I recalled some aren't capable of delayed_status. > > But if the SETUP packet's wLength value is 0 then when the gadget driver > is ready, it queues a 0-length IN request which will act as the Status > stage. In this situation the UDC does not automatically create a > Status-stage request. > > Note that the gadget driver is allowed to queue its various requests > either while the ->setup() callback is running or after it has returned. > > (Another design flaw is that this design doesn't specify what should > happen if the UDC receives another SETUP packet from the host before the > Status stage completes. By sending another SETUP packet, the host is > indicating that the earlier control transfer has been aborted. > Presumably the UDC driver will complete all the outstanding requests > with an error status, but there's a potential race in the gadget driver > between queuing a request for the first transfer and executing the > ->setup() callback for the second transfer.) If there's another SETUP packet coming while there's a pending control transfer, for dwc3 UDC, the pending control TRB should be completed with a Setup_pending status indicating aborted control transfer for dwc3 driver to handle that. > > If the dwc3 UDC driver doesn't behave this way then it needs to be > fixed. > BR, Thinh
