usb_ep_find_packet_by_id mistakenly only checks the first packet and if that
is not a match, keeps trying the first packet! This patch fixes this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
It is tempting to use USB_RET_ASYNC for interrupt packets, rather then the
current NAK + polling approach, but this causes issues for migration, as
an async completed packet will not getting written back to guest memory until
the next poll time, and if a migration happens in between it will get lost!
Make an exception for host devices, because:
1) host-linux actually uses async completion for interrupt endpoints
2) host devices don't migrate anyways
Ideally we would convert host-linux.c to handle (input) interrupt endpoints in
a buffered manner like it does for isoc endpoints, keeping multiple urbs
submitted to ensure the devices timing requirements are met, as well as making
its interrupt ep handling the same as other usb-devices.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Since with the ehci and xhci controllers a single packet can be larger
then maxpacketsize, it is possible for the result of a single packet
to be both having transferred some data as well as the transfer to have
an error.
An example would be an input transfer from a bulk endpoint successfully
receiving 1 or more maxpacketsize packets from the device, followed
by a packet signalling halt.
While already touching all the devices and controllers handle_packet /
handle_data / handle_control code, also change the return type of
these functions to void, solely storing the status in the packet. To
make the code paths for regular versus async packet handling more
uniform.
This patch unfortunately is somewhat invasive, since makeing the qemu
usb core deal with this requires changes everywhere. This patch only
prepares the usb core for this, all the hcd / device changes are done
in such a way that there are no functional changes.
This patch has been tested with uhci and ehci hcds, together with usb-audio,
usb-hid and usb-storage devices, as well as with usb-redir redirection
with a wide variety of real devices.
Note that there is usually no need to directly set packet->actual_length
form devices handle_data callback, as that is done by usb_packet_copy()
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Currently we only do pipelining for output endpoints, since to properly
support short-not-ok semantics we can only have one outstanding input
packet. Since the ehci and uhci controllers have a limited per td packet
size guests will split large input transfers to into multiple packets,
and since we don't pipeline these, this comes with a serious performance
penalty.
This patch adds helper functions to (re-)combine packets which belong to 1
transfer at the guest device-driver level into 1 large transger. This can be
used by (redirection) usb-devices to enable pipelining for input endpoints.
This patch will combine packets together until a transfer terminating packet
is encountered. A terminating packet is a packet which meets one or more of
the following conditions:
1) The packet size is *not* a multiple of the endpoint max packet size
2) The packet does *not* have its short-not-ok flag set
3) The packet has its interrupt-on-complete flag set
The short-not-ok flag of the combined packet is that of the terminating packet.
Multiple combined packets may be submitted to the device, if the combined
packets do not have their short-not-ok flag set, enabling true pipelining.
If a combined packet does have its short-not-ok flag set the queue will
wait with submitting further packets to the device until that packet has
completed.
Once enabled in the usb-redir and ehci code, this improves the speed (MB/s)
of a Linux guest reading from a USB mass storage device by a factor of
1.2 - 1.5.
And the main reason why I started working on this, when reading from a pl2303
USB<->serial converter, it combines the previous 4 packets submitted per
device-driver level read into 1 big read, reducing the number of packets / sec
by a factor 4, and it allows to have multiple reads outstanding. This allows
for much better latency tolerance without the pl2303's internal buffer
overflowing (which was happening at 115200 bps, without serial flow control).
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
ehci was already testing for this, and we depend in various places
on no devices doing this, so lets move the check for this to the
usb core.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
After a short-not-ok packet ending short, we should not advance the queue.
Move enforcing this to the core, rather then handling it in the hcd code.
This may result in the queue now actually containing multiple input packets
(which would not happen before), and this requires special handling in
combination with pipelining, so disable pipleining for input endpoints
(for now).
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
hcds which queue up more then one packet at once (uhci, ehci and xhci),
must clear the queue after an error which has caused the queue to halt.
Currently this is handled as a special case inside the hcd code, this
patch instead adds an USB_RET_REMOVE_FROM_QUEUE packet result code, teaches
the 3 hcds about this and moves the clearing of the queue on a halt into
the USB core.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This can be used by usb-device code which wishes to process an entire endpoint
queue at once, to do this the usb-device code returns USB_RET_ADD_TO_QUEUE
from its handle_data class method and defines a flush_ep_queue class method
to call when the hcd is done queuing up packets.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This can happen with usb-redir live-migration when the packet gets re-queued
after the migration and the original queuing from the migration source side
has already finished.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This way the hcd can re-use the same packet to retry without needing
to re-init it.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This patch adds IDs to usb packets. Those IDs are (a) supposed to be
unique for the lifecycle of a packet (from packet setup until the packet
is either completed or canceled) and (b) stable across migration.
uhci, ohci, ehci and xhci use the guest physical address of the transfer
descriptor for this.
musb needs a different approach because there is no transfer descriptor.
But musb also doesn't support pipelining, so we have never more than one
packet per endpoint in flight. So we go create an ID based on endpoint
and device address.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
For controllers which queue up more then 1 packet at a time, we must halt the
ep queue, and inside the controller code cancel all pending packets on an
error.
There are multiple reasons for this:
1) Guests expect the controllers to halt ep queues on error, so that they
get the opportunity to cancel transfers which the scheduled after the failing
one, before processing continues
2) Not cancelling queued up packets after a failed transfer also messes up
the controller state machine, in the case of EHCI causing the following
assert to trigger: "assert(p->qtdaddr == q->qtdaddr)" at hcd-ehci.c:2075
3) For bulk endpoints with pipelining enabled (redirection to a real USB
device), we must cancel all the transfers after this a failed one so that:
a) If they've completed already, they are not processed further causing more
stalls to be reported, originating from the same failed transfer
b) If still in flight, they are cancelled before the guest does
a clear stall, otherwise the guest and device can loose sync!
Note this patch only touches the ehci and uhci controller changes, since AFAIK
no other controllers actually queue up multiple transfer. If I'm wrong on this
other controllers need to be updated too!
Also note that this patch was heavily tested with the ehci code, where I had
a reproducer for a device causing a transfer to fail. The uhci code is not
tested with actually failing transfers and could do with a thorough review!
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Need to clear p->result after copying setup data using usb_packet_copy()
because we'll reuse the USBPacket for the data transfer.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
* mjt/mjt-iov2:
rewrite iov_send_recv() and move it to iov.c
cleanup qemu_co_sendv(), qemu_co_recvv() and friends
export iov_send_recv() and use it in iov_send() and iov_recv()
rename qemu_sendv to iov_send, change proto and move declarations to iov.h
change qemu_iovec_to_buf() to match other to,from_buf functions
consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent
allow qemu_iovec_from_buffer() to specify offset from which to start copying
consolidate qemu_iovec_memset{,_skip}() into single function and use existing iov_memset()
rewrite iov_* functions
change iov_* function prototypes to be more appropriate
virtio-serial-bus: use correct lengths in control_out() message
Conflicts:
tests/Makefile
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Create a new usb_ep_reset() function to reset endpoint state, without
re-initialiting the queues, so we don't unlink in-flight packets just
because usb-host has to re-parse the descriptor tables.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reorder arguments to be more natural, readable and
consistent with other iov_* functions, and change
argument names, from:
iov_from_buf(iov, iov_cnt, buf, iov_off, size)
to
iov_from_buf(iov, iov_cnt, offset, buf, bytes)
The result becomes natural English:
copy data to this `iov' vector with `iov_cnt'
elements starting at byte offset `offset'
from memory buffer `buf', processing `bytes'
bytes max.
(Try to read the original prototype this way).
Also change iov_clear() to more general iov_memset()
(it uses memset() internally anyway).
While at it, add comments to the header file
describing what the routines actually does.
The patch only renames argumens in the header, but
keeps old names in the implementation. The next
patch will touch actual code to match.
Now, it might look wrong to pay so much attention
to so small things. But we've so many badly designed
interfaces already so the whole thing becomes rather
confusing or error prone. One example of this is
previous commit and small discussion which emerged
from it, with an outcome that the utility functions
like these aren't well-understdandable, leading to
strange usage cases. That's why I paid quite some
attention to this set of functions and a few
others in subsequent patches.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
usb_packet_set_state can be called with p->ep = NULL. The tracepoint
there tries to log endpoint information, which leads to a segfault.
This patch makes usb_packet_set_state handle the NULL pointer properly.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Add a new function to check whenever the packet state is as expected,
log more informations in case it isn't.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reorganize usb source files. Create a new hw/usb/ directory and move
all usb source code to that place. Also make filenames a bit more
descriptive. Host adapters are prefixed with "hch-" now, usb device
emulations are prefixed with "dev-". Fixup paths Makefile and include
paths to make it compile. No code changes.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Hans de Goede