* [Qemu-devel] [PATCH 2/7] usb-linux: Get the alt. setting from sysfs rather then asking the dev
2010-11-26 18:13 [Qemu-devel] [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Hans de Goede
@ 2010-11-26 18:13 ` Hans de Goede
2010-11-26 18:13 ` [Qemu-devel] [PATCH 3/7] usb-linux: Add support for buffering iso usb packets Hans de Goede
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2010-11-26 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Gerd Hoffmann, Hans de Goede
At least one device I have lies when receiving a USB_REQ_GET_INTERFACE,
always returning 0 even if the alternate setting is different. This is
likely caused because in practice this control message is never used as
the operating system's usb stack knows which alternate setting it has
told the device to get into, and thus this ctrl message does not get
tested by device manufacturers.
When usb_fs_type == USB_FS_SYS, the active alt. setting can be read directly
from sysfs, which allows using this device through qemu's usb redirection.
More in general it seems a good idea to not send needless control msg's to
devices, esp. as the code in question is called every time a set_interface
is done. Which happens multiple times during virtual machine startup, and
when device drivers are activating the usb device.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
usb-linux.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index a93f0e5..ddd7c67 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -829,6 +829,24 @@ static uint8_t usb_linux_get_alt_setting(USBHostDevice *s,
struct usb_ctrltransfer ct;
int ret;
+ if (usb_fs_type == USB_FS_SYS) {
+ char device_name[64], line[1024];
+ int alt_setting;
+
+ sprintf(device_name, "%d-%d:%d.%d", s->bus_num, s->devpath,
+ (int)configuration, (int)interface);
+
+ if (!usb_host_read_file(line, sizeof(line), "bAlternateSetting",
+ device_name)) {
+ goto usbdevfs;
+ }
+ if (sscanf(line, "%d", &alt_setting) != 1) {
+ goto usbdevfs;
+ }
+ return alt_setting;
+ }
+
+usbdevfs:
ct.bRequestType = USB_DIR_IN | USB_RECIP_INTERFACE;
ct.bRequest = USB_REQ_GET_INTERFACE;
ct.wValue = 0;
--
1.7.3.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 3/7] usb-linux: Add support for buffering iso usb packets
2010-11-26 18:13 [Qemu-devel] [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Hans de Goede
2010-11-26 18:13 ` [Qemu-devel] [PATCH 2/7] usb-linux: Get the alt. setting from sysfs rather then asking the dev Hans de Goede
@ 2010-11-26 18:13 ` Hans de Goede
2010-11-26 18:13 ` [Qemu-devel] [PATCH 4/7] usb-linux: Refuse packets for endpoints which are not in the usb descriptor Hans de Goede
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2010-11-26 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Gerd Hoffmann, Hans de Goede
Currently we are submitting iso packets to the host one at a time, as we
receive them from the emulated host controller. This has 2 problems:
1) If we were fast enough to submit every packet in time for the next host host
controller usb frame, we would be generating 1000 hardware interrupts per
second on the host
2) We are not fast enough to submit every packet in time for the next host host
controller usb frame, causing us to not submit iso urbs in some usb frames
which causes devices with an endpoint with an interval of 1 ms (so every
frame) to loose data. This causes for example ubs-1.1 webcams to not work
properly (usb-2.0 is not supported at all atm).
This patch fixes both problems by changing the iso packet pass through handling
to buffer packets. This version only does so for iso input packets (webcams,
audio in) I'm working on a second patch extending this to iso output packets
(audio out).
This patch makes use of the linux batching of iso packets in one urb.
When an iso in packet gets received from the emulated host controller,
it immediately submits 3 urbs with 32 iso in packets each. This causes
the host to only get an hw interrupt every 32 packets dropping the
interrupt rate to 32 interrupts per second and gives it a queue of urbs
to work from once the first 32 iso in packets have been received to make sure
no packets are dropped.
Besides submitting a whole bunch or urbs as soon as the first urb is
received, effectively creating a buffer inside the kernel, this patch also
gets rid of the asynchroneous completion for iso in urbs. Instead they are
only marked as complete in the fd write callback (which usbfs uses to signal
complete urbs). These complete packets then get consumed by returning them
synchroneously to the emulated host controller when it submits an iso in
packet for the ep in question. When no complete packets are ready (which
happens when the stream is starting) a 0 length packet gets returned to
the emulated host controller.
With this patch I've several usb-1.1 webcams working well with usb pass
through, where as without this patch none of them work.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
usb-linux.c | 243 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 226 insertions(+), 17 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index ddd7c67..96aa1ed 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -92,9 +92,17 @@ static char *usb_host_device_path;
static int usb_fs_type;
/* endpoint association data */
+#define ISO_FRAME_DESC_PER_URB 32
+#define ISO_URB_COUNT 3
+
+typedef struct AsyncURB AsyncURB;
+
struct endp_data {
uint8_t type;
uint8_t halted;
+ AsyncURB *iso_urb;
+ int iso_urb_idx;
+ int max_packet_size;
};
enum {
@@ -175,19 +183,48 @@ static void set_halt(USBHostDevice *s, int ep)
s->endp_table[ep - 1].halted = 1;
}
+static void set_iso_urb(USBHostDevice *s, int ep, AsyncURB *iso_urb)
+{
+ s->endp_table[ep - 1].iso_urb = iso_urb;
+}
+
+static AsyncURB *get_iso_urb(USBHostDevice *s, int ep)
+{
+ return s->endp_table[ep - 1].iso_urb;
+}
+
+static void set_iso_urb_idx(USBHostDevice *s, int ep, int i)
+{
+ s->endp_table[ep - 1].iso_urb_idx = i;
+}
+
+static int get_iso_urb_idx(USBHostDevice *s, int ep)
+{
+ return s->endp_table[ep - 1].iso_urb_idx;
+}
+
+static int get_max_packet_size(USBHostDevice *s, int ep)
+{
+ return s->endp_table[ep - 1].max_packet_size;
+}
+
/*
* Async URB state.
- * We always allocate one isoc descriptor even for bulk transfers
+ * We always allocate iso packet descriptors even for bulk transfers
* to simplify allocation and casts.
*/
-typedef struct AsyncURB
+struct AsyncURB
{
struct usbdevfs_urb urb;
- struct usbdevfs_iso_packet_desc isocpd;
+ struct usbdevfs_iso_packet_desc isocpd[ISO_FRAME_DESC_PER_URB];
+ /* For regular async urbs */
USBPacket *packet;
USBHostDevice *hdev;
-} AsyncURB;
+
+ /* For buffered iso handling */
+ int iso_frame_idx; /* -1 means in flight */
+};
static AsyncURB *async_alloc(void)
{
@@ -244,11 +281,21 @@ static void async_complete(void *opaque)
return;
}
- p = aurb->packet;
-
DPRINTF("husb: async completed. aurb %p status %d alen %d\n",
aurb, aurb->urb.status, aurb->urb.actual_length);
+ /* If this is a buffered iso urb mark it as complete and don't do
+ anything else (it is handled further in usb_host_handle_iso_data) */
+ if (aurb->iso_frame_idx == -1) {
+ if (aurb->urb.status == -EPIPE) {
+ set_halt(s, aurb->urb.endpoint & 0xf);
+ }
+ aurb->iso_frame_idx = 0;
+ continue;
+ }
+
+ p = aurb->packet;
+
if (p) {
switch (aurb->urb.status) {
case 0:
@@ -414,34 +461,181 @@ static void usb_host_handle_destroy(USBDevice *dev)
static int usb_linux_update_endp_table(USBHostDevice *s);
+/* iso data is special, we need to keep enough urbs in flight to make sure
+ that the controller never runs out of them, otherwise the device will
+ likely suffer a buffer underrun / overrun. */
+static AsyncURB *usb_host_alloc_iso(USBHostDevice *s, uint8_t ep, int in)
+{
+ AsyncURB *aurb;
+ int i, j, len = get_max_packet_size(s, ep);
+
+ aurb = qemu_mallocz(ISO_URB_COUNT * sizeof(*aurb));
+ for (i = 0; i < ISO_URB_COUNT; i++) {
+ aurb[i].urb.endpoint = ep;
+ aurb[i].urb.buffer_length = ISO_FRAME_DESC_PER_URB * len;
+ aurb[i].urb.buffer = qemu_malloc(aurb[i].urb.buffer_length);
+ aurb[i].urb.type = USBDEVFS_URB_TYPE_ISO;
+ aurb[i].urb.flags = USBDEVFS_URB_ISO_ASAP;
+ aurb[i].urb.number_of_packets = ISO_FRAME_DESC_PER_URB;
+ for (j = 0 ; j < ISO_FRAME_DESC_PER_URB; j++)
+ aurb[i].urb.iso_frame_desc[j].length = len;
+ if (in) {
+ aurb[i].urb.endpoint |= 0x80;
+ /* Mark as fully consumed (idle) */
+ aurb[i].iso_frame_idx = ISO_FRAME_DESC_PER_URB;
+ }
+ }
+ set_iso_urb(s, ep, aurb);
+
+ return aurb;
+}
+
+static void usb_host_stop_n_free_iso(USBHostDevice *s, uint8_t ep)
+{
+ AsyncURB *aurb;
+ int i, ret, killed = 0, free = 1;
+
+ aurb = get_iso_urb(s, ep);
+ if (!aurb) {
+ return;
+ }
+
+ for (i = 0; i < ISO_URB_COUNT; i++) {
+ /* in flight? */
+ if (aurb[i].iso_frame_idx == -1) {
+ ret = ioctl(s->fd, USBDEVFS_DISCARDURB, &aurb[i]);
+ if (ret < 0) {
+ printf("husb: discard isoc in urb failed errno %d\n", errno);
+ free = 0;
+ continue;
+ }
+ killed++;
+ }
+ }
+
+ /* Make sure any urbs we've killed are reaped before we free them */
+ if (killed) {
+ async_complete(s);
+ }
+
+ for (i = 0; i < ISO_URB_COUNT; i++) {
+ qemu_free(aurb[i].urb.buffer);
+ }
+
+ if (free)
+ qemu_free(aurb);
+ else
+ printf("husb: leaking iso urbs because of discard failure\n");
+ set_iso_urb(s, ep, NULL);
+}
+
+static int urb_status_to_usb_ret(int status)
+{
+ switch (status) {
+ case -EPIPE:
+ return USB_RET_STALL;
+ default:
+ return USB_RET_NAK;
+ }
+}
+
+static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p)
+{
+ AsyncURB *aurb;
+ int i, j, ret, len = 0;
+
+ aurb = get_iso_urb(s, p->devep);
+ if (!aurb) {
+ aurb = usb_host_alloc_iso(s, p->devep, 1);
+ }
+
+ i = get_iso_urb_idx(s, p->devep);
+ j = aurb[i].iso_frame_idx;
+ if (j >= 0 && j < ISO_FRAME_DESC_PER_URB) {
+ /* Check urb status */
+ if (aurb[i].urb.status) {
+ len = urb_status_to_usb_ret(aurb[i].urb.status);
+ /* Move to the next urb */
+ aurb[i].iso_frame_idx = ISO_FRAME_DESC_PER_URB - 1;
+ /* Check frame status */
+ } else if (aurb[i].urb.iso_frame_desc[j].status) {
+ len = urb_status_to_usb_ret(aurb[i].urb.iso_frame_desc[j].status);
+ /* Check the frame fits */
+ } else if (aurb[i].urb.iso_frame_desc[j].actual_length > p->len) {
+ printf("husb: error received isoc data is larger then packet\n");
+ len = USB_RET_NAK;
+ /* All good copy data over */
+ } else {
+ len = aurb[i].urb.iso_frame_desc[j].actual_length;
+ memcpy(p->data,
+ aurb[i].urb.buffer +
+ j * aurb[i].urb.iso_frame_desc[0].length,
+ len);
+ }
+ aurb[i].iso_frame_idx++;
+ if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
+ i = (i + 1) % ISO_URB_COUNT;
+ set_iso_urb_idx(s, p->devep, i);
+ }
+ }
+
+ /* (Re)-submit all fully consumed urbs */
+ for (i = 0; i < ISO_URB_COUNT; i++) {
+ if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
+ ret = ioctl(s->fd, USBDEVFS_SUBMITURB, &aurb[i]);
+ if (ret < 0) {
+ printf("husb error submitting isoc urb %d: %d\n", i, errno);
+ if (len == 0) {
+ switch(errno) {
+ case ETIMEDOUT:
+ len = USB_RET_NAK;
+ case EPIPE:
+ default:
+ len = USB_RET_STALL;
+ }
+ }
+ break;
+ }
+ aurb[i].iso_frame_idx = -1;
+ }
+ }
+
+ return len;
+}
+
static int usb_host_handle_data(USBHostDevice *s, USBPacket *p)
{
struct usbdevfs_urb *urb;
AsyncURB *aurb;
int ret;
-
- aurb = async_alloc();
- aurb->hdev = s;
- aurb->packet = p;
-
- urb = &aurb->urb;
+ uint8_t ep;
if (p->pid == USB_TOKEN_IN) {
- urb->endpoint = p->devep | 0x80;
+ ep = p->devep | 0x80;
} else {
- urb->endpoint = p->devep;
+ ep = p->devep;
}
if (is_halted(s, p->devep)) {
- ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
+ ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &ep);
if (ret < 0) {
DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n",
- urb->endpoint, errno);
+ ep, errno);
return USB_RET_NAK;
}
clear_halt(s, p->devep);
}
+ if (is_isoc(s, p->devep) && p->pid == USB_TOKEN_IN)
+ return usb_host_handle_iso_data(s, p);
+
+ aurb = async_alloc();
+ aurb->hdev = s;
+ aurb->packet = p;
+
+ urb = &aurb->urb;
+
+ urb->endpoint = ep;
urb->buffer = p->data;
urb->buffer_length = p->len;
@@ -514,7 +708,13 @@ static int usb_host_set_config(USBHostDevice *s, int config)
static int usb_host_set_interface(USBHostDevice *s, int iface, int alt)
{
struct usbdevfs_setinterface si;
- int ret;
+ int i, ret;
+
+ for (i = 1; i < MAX_ENDPOINTS; i++) {
+ if (is_isoc(s, i)) {
+ usb_host_stop_n_free_iso(s, i);
+ }
+ }
si.interface = iface;
si.altsetting = alt;
@@ -926,6 +1126,8 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
break;
case 0x01:
type = USBDEVFS_URB_TYPE_ISO;
+ s->endp_table[(devep & 0xf) - 1].max_packet_size =
+ descriptors[i + 4] + (descriptors[i + 5] << 8);
break;
case 0x02:
type = USBDEVFS_URB_TYPE_BULK;
@@ -1048,12 +1250,19 @@ fail:
static int usb_host_close(USBHostDevice *dev)
{
+ int i;
+
if (dev->fd == -1) {
return -1;
}
qemu_set_fd_handler(dev->fd, NULL, NULL, NULL);
dev->closing = 1;
+ for (i = 1; i < MAX_ENDPOINTS; i++) {
+ if (is_isoc(dev, i)) {
+ usb_host_stop_n_free_iso(dev, i);
+ }
+ }
async_complete(dev);
dev->closing = 0;
usb_device_detach(&dev->dev);
--
1.7.3.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 4/7] usb-linux: Refuse packets for endpoints which are not in the usb descriptor
2010-11-26 18:13 [Qemu-devel] [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Hans de Goede
2010-11-26 18:13 ` [Qemu-devel] [PATCH 2/7] usb-linux: Get the alt. setting from sysfs rather then asking the dev Hans de Goede
2010-11-26 18:13 ` [Qemu-devel] [PATCH 3/7] usb-linux: Add support for buffering iso usb packets Hans de Goede
@ 2010-11-26 18:13 ` Hans de Goede
2010-11-26 18:13 ` [Qemu-devel] [PATCH 5/7] usb-linux: Refuse iso packets when max packet size is 0 (alt setting 0) Hans de Goede
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2010-11-26 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Gerd Hoffmann, Hans de Goede
If an endpoint is not in the usb descriptor we've no idea what kind of
endpoint it is and thus how to handle it, refuse packages in this case.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
usb-linux.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 96aa1ed..8513ace 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -94,6 +94,7 @@ static int usb_fs_type;
/* endpoint association data */
#define ISO_FRAME_DESC_PER_URB 32
#define ISO_URB_COUNT 3
+#define INVALID_EP_TYPE 255
typedef struct AsyncURB AsyncURB;
@@ -168,6 +169,11 @@ static int is_isoc(USBHostDevice *s, int ep)
return s->endp_table[ep - 1].type == USBDEVFS_URB_TYPE_ISO;
}
+static int is_valid(USBHostDevice *s, int ep)
+{
+ return s->endp_table[ep - 1].type != INVALID_EP_TYPE;
+}
+
static int is_halted(USBHostDevice *s, int ep)
{
return s->endp_table[ep - 1].halted;
@@ -610,6 +616,10 @@ static int usb_host_handle_data(USBHostDevice *s, USBPacket *p)
int ret;
uint8_t ep;
+ if (!is_valid(s, p->devep)) {
+ return USB_RET_NAK;
+ }
+
if (p->pid == USB_TOKEN_IN) {
ep = p->devep | 0x80;
} else {
@@ -1070,6 +1080,9 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
uint8_t devep, type, configuration, alt_interface;
int interface, length, i;
+ for (i = 0; i < MAX_ENDPOINTS; i++)
+ s->endp_table[i].type = INVALID_EP_TYPE;
+
i = usb_linux_get_configuration(s);
if (i < 0)
return 1;
--
1.7.3.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 5/7] usb-linux: Refuse iso packets when max packet size is 0 (alt setting 0)
2010-11-26 18:13 [Qemu-devel] [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Hans de Goede
` (2 preceding siblings ...)
2010-11-26 18:13 ` [Qemu-devel] [PATCH 4/7] usb-linux: Refuse packets for endpoints which are not in the usb descriptor Hans de Goede
@ 2010-11-26 18:13 ` Hans de Goede
2010-11-26 18:13 ` [Qemu-devel] [PATCH 6/7] usb-linux: We only need to keep track of 15 endpoints Hans de Goede
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2010-11-26 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Gerd Hoffmann, Hans de Goede
Refuse iso usb packets when then max packet size for the endpoint is 0,
this avoids an abort in usb_host_alloc_iso() caused by trying to qemu_malloc
a 0 bytes large buffer.
---
usb-linux.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 8513ace..5a979bd 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -548,7 +548,11 @@ static int urb_status_to_usb_ret(int status)
static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p)
{
AsyncURB *aurb;
- int i, j, ret, len = 0;
+ int i, j, ret, max_packet_size, len = 0;
+
+ max_packet_size = get_max_packet_size(s, p->devep);
+ if (max_packet_size == 0)
+ return USB_RET_NAK;
aurb = get_iso_urb(s, p->devep);
if (!aurb) {
--
1.7.3.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 6/7] usb-linux: We only need to keep track of 15 endpoints
2010-11-26 18:13 [Qemu-devel] [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Hans de Goede
` (3 preceding siblings ...)
2010-11-26 18:13 ` [Qemu-devel] [PATCH 5/7] usb-linux: Refuse iso packets when max packet size is 0 (alt setting 0) Hans de Goede
@ 2010-11-26 18:13 ` Hans de Goede
2010-11-26 18:13 ` [Qemu-devel] [PATCH 7/7] usb-linux: Add support for buffering iso out usb packets Hans de Goede
2010-12-09 13:16 ` [Qemu-devel] Re: [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Gerd Hoffmann
6 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2010-11-26 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Gerd Hoffmann, Hans de Goede
Currently we reserve room for endpoint data for 16 endpoints, but given
that we only use endpoint data for endpoints 1-15, and always index the
array with the endpoint-number - 1, 15 is enough.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
usb-linux.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 5a979bd..9301084 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -78,7 +78,7 @@ typedef int USBScanFunc(void *opaque, int bus_num, int addr, int devpath,
#define USBPROCBUS_PATH "/proc/bus/usb"
#define PRODUCT_NAME_SZ 32
-#define MAX_ENDPOINTS 16
+#define MAX_ENDPOINTS 15
#define USBDEVBUS_PATH "/dev/bus/usb"
#define USBSYSBUS_PATH "/sys/bus/usb"
@@ -724,7 +724,7 @@ static int usb_host_set_interface(USBHostDevice *s, int iface, int alt)
struct usbdevfs_setinterface si;
int i, ret;
- for (i = 1; i < MAX_ENDPOINTS; i++) {
+ for (i = 1; i <= MAX_ENDPOINTS; i++) {
if (is_isoc(s, i)) {
usb_host_stop_n_free_iso(s, i);
}
@@ -1275,7 +1275,7 @@ static int usb_host_close(USBHostDevice *dev)
qemu_set_fd_handler(dev->fd, NULL, NULL, NULL);
dev->closing = 1;
- for (i = 1; i < MAX_ENDPOINTS; i++) {
+ for (i = 1; i <= MAX_ENDPOINTS; i++) {
if (is_isoc(dev, i)) {
usb_host_stop_n_free_iso(dev, i);
}
--
1.7.3.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 7/7] usb-linux: Add support for buffering iso out usb packets
2010-11-26 18:13 [Qemu-devel] [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Hans de Goede
` (4 preceding siblings ...)
2010-11-26 18:13 ` [Qemu-devel] [PATCH 6/7] usb-linux: We only need to keep track of 15 endpoints Hans de Goede
@ 2010-11-26 18:13 ` Hans de Goede
2010-12-09 13:16 ` [Qemu-devel] Re: [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Gerd Hoffmann
6 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2010-11-26 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: spice-devel, Gerd Hoffmann, Hans de Goede
Extend the iso buffering code to also buffer iso out packets, this
fixes for example using usb speakers with usb redirection.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
usb-linux.c | 152 +++++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 101 insertions(+), 51 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 9301084..3ef8198 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -101,8 +101,10 @@ typedef struct AsyncURB AsyncURB;
struct endp_data {
uint8_t type;
uint8_t halted;
+ uint8_t iso_started;
AsyncURB *iso_urb;
int iso_urb_idx;
+ int iso_buffer_used;
int max_packet_size;
};
@@ -189,6 +191,21 @@ static void set_halt(USBHostDevice *s, int ep)
s->endp_table[ep - 1].halted = 1;
}
+static int is_iso_started(USBHostDevice *s, int ep)
+{
+ return s->endp_table[ep - 1].iso_started;
+}
+
+static void clear_iso_started(USBHostDevice *s, int ep)
+{
+ s->endp_table[ep - 1].iso_started = 0;
+}
+
+static void set_iso_started(USBHostDevice *s, int ep)
+{
+ s->endp_table[ep - 1].iso_started = 1;
+}
+
static void set_iso_urb(USBHostDevice *s, int ep, AsyncURB *iso_urb)
{
s->endp_table[ep - 1].iso_urb = iso_urb;
@@ -209,6 +226,16 @@ static int get_iso_urb_idx(USBHostDevice *s, int ep)
return s->endp_table[ep - 1].iso_urb_idx;
}
+static void set_iso_buffer_used(USBHostDevice *s, int ep, int i)
+{
+ s->endp_table[ep - 1].iso_buffer_used = i;
+}
+
+static int get_iso_buffer_used(USBHostDevice *s, int ep)
+{
+ return s->endp_table[ep - 1].iso_buffer_used;
+}
+
static int get_max_packet_size(USBHostDevice *s, int ep)
{
return s->endp_table[ep - 1].max_packet_size;
@@ -533,6 +560,8 @@ static void usb_host_stop_n_free_iso(USBHostDevice *s, uint8_t ep)
else
printf("husb: leaking iso urbs because of discard failure\n");
set_iso_urb(s, ep, NULL);
+ set_iso_urb_idx(s, ep, 0);
+ clear_iso_started(s, ep);
}
static int urb_status_to_usb_ret(int status)
@@ -545,10 +574,10 @@ static int urb_status_to_usb_ret(int status)
}
}
-static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p)
+static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p, int in)
{
AsyncURB *aurb;
- int i, j, ret, max_packet_size, len = 0;
+ int i, j, ret, max_packet_size, offset, len = 0;
max_packet_size = get_max_packet_size(s, p->devep);
if (max_packet_size == 0)
@@ -556,57 +585,88 @@ static int usb_host_handle_iso_data(USBHostDevice *s, USBPacket *p)
aurb = get_iso_urb(s, p->devep);
if (!aurb) {
- aurb = usb_host_alloc_iso(s, p->devep, 1);
+ aurb = usb_host_alloc_iso(s, p->devep, in);
}
i = get_iso_urb_idx(s, p->devep);
j = aurb[i].iso_frame_idx;
if (j >= 0 && j < ISO_FRAME_DESC_PER_URB) {
- /* Check urb status */
- if (aurb[i].urb.status) {
- len = urb_status_to_usb_ret(aurb[i].urb.status);
- /* Move to the next urb */
- aurb[i].iso_frame_idx = ISO_FRAME_DESC_PER_URB - 1;
- /* Check frame status */
- } else if (aurb[i].urb.iso_frame_desc[j].status) {
- len = urb_status_to_usb_ret(aurb[i].urb.iso_frame_desc[j].status);
- /* Check the frame fits */
- } else if (aurb[i].urb.iso_frame_desc[j].actual_length > p->len) {
- printf("husb: error received isoc data is larger then packet\n");
- len = USB_RET_NAK;
- /* All good copy data over */
+ if (in) {
+ /* Check urb status */
+ if (aurb[i].urb.status) {
+ len = urb_status_to_usb_ret(aurb[i].urb.status);
+ /* Move to the next urb */
+ aurb[i].iso_frame_idx = ISO_FRAME_DESC_PER_URB - 1;
+ /* Check frame status */
+ } else if (aurb[i].urb.iso_frame_desc[j].status) {
+ len = urb_status_to_usb_ret(
+ aurb[i].urb.iso_frame_desc[j].status);
+ /* Check the frame fits */
+ } else if (aurb[i].urb.iso_frame_desc[j].actual_length > p->len) {
+ printf("husb: received iso data is larger then packet\n");
+ len = USB_RET_NAK;
+ /* All good copy data over */
+ } else {
+ len = aurb[i].urb.iso_frame_desc[j].actual_length;
+ memcpy(p->data,
+ aurb[i].urb.buffer +
+ j * aurb[i].urb.iso_frame_desc[0].length,
+ len);
+ }
} else {
- len = aurb[i].urb.iso_frame_desc[j].actual_length;
- memcpy(p->data,
- aurb[i].urb.buffer +
- j * aurb[i].urb.iso_frame_desc[0].length,
- len);
+ len = p->len;
+ offset = (j == 0) ? 0 : get_iso_buffer_used(s, p->devep);
+
+ /* Check the frame fits */
+ if (len > max_packet_size) {
+ printf("husb: send iso data is larger then max packet size\n");
+ return USB_RET_NAK;
+ }
+
+ /* All good copy data over */
+ memcpy(aurb[i].urb.buffer + offset, p->data, len);
+ aurb[i].urb.iso_frame_desc[j].length = len;
+ offset += len;
+ set_iso_buffer_used(s, p->devep, offset);
+
+ /* Start the stream once we have buffered enough data */
+ if (!is_iso_started(s, p->devep) && i == 1 && j == 8) {
+ set_iso_started(s, p->devep);
+ }
}
aurb[i].iso_frame_idx++;
if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
i = (i + 1) % ISO_URB_COUNT;
set_iso_urb_idx(s, p->devep, i);
}
+ } else {
+ if (in) {
+ set_iso_started(s, p->devep);
+ } else {
+ DPRINTF("hubs: iso out error no free buffer, dropping packet\n");
+ }
}
- /* (Re)-submit all fully consumed urbs */
- for (i = 0; i < ISO_URB_COUNT; i++) {
- if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
- ret = ioctl(s->fd, USBDEVFS_SUBMITURB, &aurb[i]);
- if (ret < 0) {
- printf("husb error submitting isoc urb %d: %d\n", i, errno);
- if (len == 0) {
- switch(errno) {
- case ETIMEDOUT:
- len = USB_RET_NAK;
- case EPIPE:
- default:
- len = USB_RET_STALL;
+ if (is_iso_started(s, p->devep)) {
+ /* (Re)-submit all fully consumed / filled urbs */
+ for (i = 0; i < ISO_URB_COUNT; i++) {
+ if (aurb[i].iso_frame_idx == ISO_FRAME_DESC_PER_URB) {
+ ret = ioctl(s->fd, USBDEVFS_SUBMITURB, &aurb[i]);
+ if (ret < 0) {
+ printf("husb error submitting iso urb %d: %d\n", i, errno);
+ if (!in || len == 0) {
+ switch(errno) {
+ case ETIMEDOUT:
+ len = USB_RET_NAK;
+ case EPIPE:
+ default:
+ len = USB_RET_STALL;
+ }
}
+ break;
}
- break;
+ aurb[i].iso_frame_idx = -1;
}
- aurb[i].iso_frame_idx = -1;
}
}
@@ -640,8 +700,9 @@ static int usb_host_handle_data(USBHostDevice *s, USBPacket *p)
clear_halt(s, p->devep);
}
- if (is_isoc(s, p->devep) && p->pid == USB_TOKEN_IN)
- return usb_host_handle_iso_data(s, p);
+ if (is_isoc(s, p->devep)) {
+ return usb_host_handle_iso_data(s, p, p->pid == USB_TOKEN_IN);
+ }
aurb = async_alloc();
aurb->hdev = s;
@@ -652,19 +713,8 @@ static int usb_host_handle_data(USBHostDevice *s, USBPacket *p)
urb->endpoint = ep;
urb->buffer = p->data;
urb->buffer_length = p->len;
-
- if (is_isoc(s, p->devep)) {
- /* Setup ISOC transfer */
- urb->type = USBDEVFS_URB_TYPE_ISO;
- urb->flags = USBDEVFS_URB_ISO_ASAP;
- urb->number_of_packets = 1;
- urb->iso_frame_desc[0].length = p->len;
- } else {
- /* Setup bulk transfer */
- urb->type = USBDEVFS_URB_TYPE_BULK;
- }
-
- urb->usercontext = s;
+ urb->type = USBDEVFS_URB_TYPE_BULK;
+ urb->usercontext = s;
ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
--
1.7.3.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] Re: [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function
2010-11-26 18:13 [Qemu-devel] [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Hans de Goede
` (5 preceding siblings ...)
2010-11-26 18:13 ` [Qemu-devel] [PATCH 7/7] usb-linux: Add support for buffering iso out usb packets Hans de Goede
@ 2010-12-09 13:16 ` Gerd Hoffmann
2010-12-15 11:50 ` Gerd Hoffmann
6 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2010-12-09 13:16 UTC (permalink / raw)
To: Hans de Goede; +Cc: spice-devel, qemu-devel
On 11/26/10 19:13, Hans de Goede wrote:
> The next patch in this series introduces multiple ways to get the
> alt setting dependent upon usb_fs_type, it is cleaner to put this
> into its own function.
>
> Note that this patch also changes the assumed alt setting in case
> of an error getting the alt setting to be 0 (a sane default) rather
> then the interface numberwhich makes no sense.
Patch series looks good.
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
cheers,
Gerd
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] Re: [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function
2010-12-09 13:16 ` [Qemu-devel] Re: [PATCH 1/7] usb-linux: introduce a usb_linux_alt_setting function Gerd Hoffmann
@ 2010-12-15 11:50 ` Gerd Hoffmann
0 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2010-12-15 11:50 UTC (permalink / raw)
To: Hans de Goede; +Cc: spice-devel, qemu-devel
On 12/09/10 14:16, Gerd Hoffmann wrote:
> On 11/26/10 19:13, Hans de Goede wrote:
>> The next patch in this series introduces multiple ways to get the
>> alt setting dependent upon usb_fs_type, it is cleaner to put this
>> into its own function.
>>
>> Note that this patch also changes the assumed alt setting in case
>> of an error getting the alt setting to be 0 (a sane default) rather
>> then the interface numberwhich makes no sense.
>
> Patch series looks good.
>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Ping? Any chance this can go in for 0.14?
cheers,
Gerd
^ permalink raw reply [flat|nested] 9+ messages in thread