Hi, > One possible alternative to re-combining would be to pass the > short-not-ok flag along the chain all the way down into usbfs,
Yes, this is what I have in mind. > but > that won't work since the USBFS_URB_SHORT_NOT_OK flag does not > work with XHCI controllers! Oops. > So lets try to split this discussion into multiple questions: > > 1) Can we agree that re-combining input packets back into their original > transfers as done at the guest device driver level, is useful and given > the serial <-> usb converters case, even necessary to have ? Given that the USBFS_URB_SHORT_NOT_OK doesn't work there is no way around that I guess. > 2) If we agree on 1), what is the right place to do them ? > > To be able to properly combine packets into one larger transfer, an > uncombine them again later, some coordination is needed between the > entity doing the combining / uncombining and the hcd code: > > 2a) The combining entity needs to know when the hcd is done with > queuing up packets for an ep. Yes. We can add a USBDeviceClass method for that. > 2b) When uncombining it is possible that less packets are actually > used / filled with data, then were combined into one transfer. When > this happens the hcd code will stop processing the queue after the > short packet, and wait for the guest to restart it. When the guest > restarts the queue it will be modified and the un-used packets will > be gone from it. Since the hcd code keeps its own queue, when > when uncombining the hcd code needs to be told to forget about > the unused packets. Yes. We can add a new USBPortOps for that, or reuse USBPortOps->complete with USBPacket->result == USB_RET_NOT_USED. > Note that currently we have the same problem on a halt of the ep > queue already, and currently we expect the hcd to completely empty > the queue on a stall. I'm planning to change this code path over > to also let the core tell the hcd to forget about the packets it > had queued beyond the stall, as this seems something to do at the > core level rather then reproducing it in every hcd emulation > separately. The core can and should do that for packets it owns (USBPacketState == USB_PACKET_QUEUED) because they are not (yet) passed to the USBDevice. Packets owned by USBDevice (USBPacketState == USB_PACKET_ASYNC) must be handled by the USBDevice itself. > Given the necessary close interaction between the hcd code and > the combine / uncombine code + not wanting to reproduce the > same code in both host-linux.c and redirect.c I believe that > the usb core is the right place for this. I think the combining should happen just before submitting the transfer to usbfs, in usb-host. I'd like usb-host see what is really going on, and the usb core not hiding this by magically creating jumbo packets. Likewise I think usbredir should do the combining on the *server* side, not in the qemu redir code. >> Maybe add a callback to notify USBDevice >> when the host controller is done filling the queue, so USBDevices can >> process all queued packets in one go (bottom half would work too >> though). > > Changing the callback which I added for this to a bh is fine with me, > I think that both from a making clear what is going on when reading > the code, and from a having all the data hot in cache pov, the callback > is better though. Yes, callback is probably better, that is just a little minor detail though. cheers, Gerd
