On Thu, 1 Oct 2026 at 20:47, Peter Maydell <[email protected]> wrote: > > The OHCI USB controller requires software to set up an Endpoint > Descriptor data structure. This specifies (among other things) both > an EndpointNumber telling the controller which USB endpoint to talk > to, and a Format (F) bit which is 1 for Isochronous transfers and 0 > for General transfers. > > The specification requires that software matches the F bit with the > endpoint it's setting up the descriptor for, so that Control, Bulk > and Interrupt endpoints use F=0, and Isochronous endpoints use F=1. > It doesn't require the controller to check whether software gets this > right. > > The spec isn't totally clear about what happens if software creates > an ED with a mismatch, but I believe that the controller just trusts > the F bit, and tries to talk to the endpoint with the wrong sequence > of USB tokens (notably, isochronous transactions have no handshaking > and the others do). This probably results in the transfer failing. > > In hcd-ehci.c we check for the "iso transfer to non-iso endpoint" and > make it fail, but we don't do this in hcd-ohci.c. The result is that > a guest that deliberately sets up this situation can cause us to hit > the assertion added in commit 7b1eba96aeca, where the endpoint is not > isochronous and returns USB_RET_ASYNC to an iso TD handling function > that isn't prepared to deal with that.
Side note -- compare 45684127cd which fixes an assert we hit in hcd-xhci when the guest configures an isoch transfer on a non-isoch endpoint, and the code in the hcd-ehci model which catches a similar case. An alternative approach here would be to add an argument to usb_handle_packet() which was a bool for "is isochronous transfer" (or some enum, for better ergonomics), and have usb_handle_packet() complete the packet with an error response in the case of an endpoint mismatch. That would allow us to deal with this in one place and make it harder for the controller models to forget about it, but it would also be more invasive as a change, which is a bit unappealing given USB's current unmaintained status and a lack of tests for it. thanks -- PMM
