* [Qemu-devel] [PATCH 1/5] husb: support for USB host device auto disconnect.
2008-08-14 4:22 [Qemu-devel] [PATCH 0/5] Various USB fixes and improvements Max Krasnyansky
@ 2008-08-14 4:22 ` Max Krasnyansky
2008-08-14 16:28 ` [Qemu-devel] " Anthony Liguori
2008-08-14 4:22 ` [Qemu-devel] [PATCH 2/5] husb: support for USB host device auto connect Max Krasnyansky
` (4 subsequent siblings)
5 siblings, 1 reply; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 4:22 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, Max Krasnyansky
I got really annoyed by the fact that you have to manually do
usb_del in the monitor when host device is unplugged and decided
to fix it :)
Basically we now automatically remove guest USB device
when the actual host device is disconnected.
At first I've extended set_fd_handlerX() stuff to support checking
for exceptions on fds. But unfortunately usbfs code does not wake up
user-space process when device is removed, which means we need a
timer to periodically check if device is still there. So I removed
fd exception stuff and implemented it with the timer.
Signed-off-by: Max Krasnyansky <maxk@kernel.org>
---
hw/usb.h | 1 +
usb-linux.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++---------
vl.c | 26 ++++++++++++++++++--------
3 files changed, 66 insertions(+), 17 deletions(-)
diff --git a/hw/usb.h b/hw/usb.h
index 8bdc68d..2edb982 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -197,6 +197,7 @@ static inline void usb_cancel_packet(USBPacket * p)
p->cancel_cb(p, p->cancel_opaque);
}
+int usb_device_del_addr(int bus_num, int addr);
void usb_attach(USBPort *port, USBDevice *dev);
int usb_generic_handle_packet(USBDevice *s, USBPacket *p);
int set_usb_string(uint8_t *buf, const char *str);
diff --git a/usb-linux.c b/usb-linux.c
index d3e4e2e..0023c1d 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include "qemu-common.h"
+#include "qemu-timer.h"
#include "hw/usb.h"
#include "console.h"
@@ -77,6 +78,7 @@ typedef struct USBHostDevice {
uint8_t descr[1024];
int descr_len;
int urbs_ready;
+ QEMUTimer *timer;
} USBHostDevice;
typedef struct PendingURB {
@@ -165,7 +167,11 @@ static int usb_host_update_interfaces(USBHostDevice *dev, int configuration)
}
config_descr_len = dev->descr[i];
- if (configuration == dev->descr[i + 5])
+#ifdef DEBUG
+ printf("config #%d need %d\n", dev->descr[i + 5], configuration);
+#endif
+
+ if (configuration < 0 || configuration == dev->descr[i + 5])
break;
i += config_descr_len;
@@ -230,8 +236,11 @@ static void usb_host_handle_destroy(USBDevice *dev)
{
USBHostDevice *s = (USBHostDevice *)dev;
+ qemu_del_timer(s->timer);
+
if (s->fd >= 0)
close(s->fd);
+
qemu_free(s);
}
@@ -594,6 +603,22 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
return 0;
}
+static void usb_host_device_check(void *priv)
+{
+ USBHostDevice *s = priv;
+ struct usbdevfs_connectinfo ci;
+ int err;
+
+ err = ioctl(s->fd, USBDEVFS_CONNECTINFO, &ci);
+ if (err < 0) {
+ printf("usb device %d.%d disconnected\n", 0, s->dev.addr);
+ usb_device_del_addr(0, s->dev.addr);
+ return;
+ }
+
+ qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 1000);
+}
+
/* XXX: exclude high speed devices or implement EHCI */
USBDevice *usb_host_device_open(const char *devname)
{
@@ -604,24 +629,30 @@ USBDevice *usb_host_device_open(const char *devname)
int bus_num, addr;
char product_name[PRODUCT_NAME_SZ];
+ if (usb_host_find_device(&bus_num, &addr,
+ product_name, sizeof(product_name),
+ devname) < 0)
+ return NULL;
+
+
dev = qemu_mallocz(sizeof(USBHostDevice));
if (!dev)
goto fail;
-#ifdef DEBUG_ISOCH
+ dev->timer = qemu_new_timer(rt_clock, usb_host_device_check, (void *) dev);
+ if (!dev->timer)
+ goto fail;
+
+#ifdef DEBUG
printf("usb_host_device_open %s\n", devname);
#endif
- if (usb_host_find_device(&bus_num, &addr,
- product_name, sizeof(product_name),
- devname) < 0)
- return NULL;
snprintf(buf, sizeof(buf), USBDEVFS_PATH "/%03d/%03d",
bus_num, addr);
fd = open(buf, O_RDWR | O_NONBLOCK);
if (fd < 0) {
perror(buf);
- return NULL;
+ goto fail;
}
/* read the device description */
@@ -645,7 +676,7 @@ USBDevice *usb_host_device_open(const char *devname)
dev->configuration = 1;
/* XXX - do something about initial configuration */
- if (!usb_host_update_interfaces(dev, 1))
+ if (!usb_host_update_interfaces(dev, -1))
goto fail;
ret = ioctl(fd, USBDEVFS_CONNECTINFO, &ci);
@@ -700,11 +731,18 @@ USBDevice *usb_host_device_open(const char *devname)
fcntl(dev->pipe_fds[1], F_SETFL, O_NONBLOCK);
qemu_set_fd_handler(dev->pipe_fds[0], urb_completion_pipe_read, NULL, dev);
#endif
+
+ /* Start the timer to detect disconnect */
+ qemu_mod_timer(dev->timer, qemu_get_clock(rt_clock) + 1000);
+
dev->urbs_ready = 0;
return (USBDevice *)dev;
fail:
- if (dev)
+ if (dev) {
+ if (dev->timer)
+ qemu_del_timer(dev->timer);
qemu_free(dev);
+ }
close(fd);
return NULL;
}
diff --git a/vl.c b/vl.c
index 6ade2ad..a278599 100644
--- a/vl.c
+++ b/vl.c
@@ -5805,22 +5805,15 @@ static int usb_device_add(const char *devname)
return 0;
}
-static int usb_device_del(const char *devname)
+int usb_device_del_addr(int bus_num, int addr)
{
USBPort *port;
USBPort **lastp;
USBDevice *dev;
- int bus_num, addr;
- const char *p;
if (!used_usb_ports)
return -1;
- p = strchr(devname, '.');
- if (!p)
- return -1;
- bus_num = strtoul(devname, NULL, 0);
- addr = strtoul(p + 1, NULL, 0);
if (bus_num != 0)
return -1;
@@ -5843,6 +5836,23 @@ static int usb_device_del(const char *devname)
return 0;
}
+static int usb_device_del(const char *devname)
+{
+ int bus_num, addr;
+ const char *p;
+
+ if (!used_usb_ports)
+ return -1;
+
+ p = strchr(devname, '.');
+ if (!p)
+ return -1;
+ bus_num = strtoul(devname, NULL, 0);
+ addr = strtoul(p + 1, NULL, 0);
+
+ return usb_device_del_addr(bus_num, addr);
+}
+
void do_usb_add(const char *devname)
{
int ret;
--
1.5.5.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 1/5] husb: support for USB host device auto disconnect.
2008-08-14 4:22 ` [Qemu-devel] [PATCH 1/5] husb: support for USB host device auto disconnect Max Krasnyansky
@ 2008-08-14 16:28 ` Anthony Liguori
2008-08-14 19:26 ` Max Krasnyansky
2008-08-14 21:41 ` Max Krasnyansky
0 siblings, 2 replies; 37+ messages in thread
From: Anthony Liguori @ 2008-08-14 16:28 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: qemu-devel, kvm
Max Krasnyansky wrote:
> I got really annoyed by the fact that you have to manually do
> usb_del in the monitor when host device is unplugged and decided
> to fix it :)
>
Yes, I've felt the same annoyance.
> Basically we now automatically remove guest USB device
> when the actual host device is disconnected.
>
> At first I've extended set_fd_handlerX() stuff to support checking
> for exceptions on fds. But unfortunately usbfs code does not wake up
> user-space process when device is removed, which means we need a
> timer to periodically check if device is still there. So I removed
> fd exception stuff and implemented it with the timer.
>
I'm surprised there isn't an EOF when the device is disconnected. There
has to be some sort of userspace notification that a device has been
unplugged. I don't really like the idea of polling but it wouldn't
surprise me if we had to.
> Signed-off-by: Max Krasnyansky <maxk@kernel.org>
> ---
> hw/usb.h | 1 +
> usb-linux.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++---------
> vl.c | 26 ++++++++++++++++++--------
> 3 files changed, 66 insertions(+), 17 deletions(-)
>
> diff --git a/hw/usb.h b/hw/usb.h
> index 8bdc68d..2edb982 100644
> --- a/hw/usb.h
> +++ b/hw/usb.h
> @@ -197,6 +197,7 @@ static inline void usb_cancel_packet(USBPacket * p)
> p->cancel_cb(p, p->cancel_opaque);
> }
>
> +int usb_device_del_addr(int bus_num, int addr);
> void usb_attach(USBPort *port, USBDevice *dev);
> int usb_generic_handle_packet(USBDevice *s, USBPacket *p);
> int set_usb_string(uint8_t *buf, const char *str);
> diff --git a/usb-linux.c b/usb-linux.c
> index d3e4e2e..0023c1d 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -22,6 +22,7 @@
> * THE SOFTWARE.
> */
> #include "qemu-common.h"
> +#include "qemu-timer.h"
> #include "hw/usb.h"
> #include "console.h"
>
> @@ -77,6 +78,7 @@ typedef struct USBHostDevice {
> uint8_t descr[1024];
> int descr_len;
> int urbs_ready;
> + QEMUTimer *timer;
> } USBHostDevice;
>
> typedef struct PendingURB {
> @@ -165,7 +167,11 @@ static int usb_host_update_interfaces(USBHostDevice *dev, int configuration)
> }
> config_descr_len = dev->descr[i];
>
> - if (configuration == dev->descr[i + 5])
> +#ifdef DEBUG
> + printf("config #%d need %d\n", dev->descr[i + 5], configuration);
> +#endif
> +
> + if (configuration < 0 || configuration == dev->descr[i + 5])
> break;
>
> i += config_descr_len;
> @@ -230,8 +236,11 @@ static void usb_host_handle_destroy(USBDevice *dev)
> {
> USBHostDevice *s = (USBHostDevice *)dev;
>
> + qemu_del_timer(s->timer);
> +
>
qemu_del_timer() only removes a pending timer. You need
qemu_free_timer() to actually free the memory associated with it.
> dev->urbs_ready = 0;
> return (USBDevice *)dev;
> fail:
> - if (dev)
> + if (dev) {
> + if (dev->timer)
> + qemu_del_timer(dev->timer);
>
Here too.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 1/5] husb: support for USB host device auto disconnect.
2008-08-14 16:28 ` [Qemu-devel] " Anthony Liguori
@ 2008-08-14 19:26 ` Max Krasnyansky
2008-08-14 21:41 ` Max Krasnyansky
1 sibling, 0 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 19:26 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, kvm
Anthony Liguori wrote:
> Max Krasnyansky wrote:
>> I got really annoyed by the fact that you have to manually do
>> usb_del in the monitor when host device is unplugged and decided
>> to fix it :)
>>
>
> Yes, I've felt the same annoyance.
>
>> Basically we now automatically remove guest USB device
>> when the actual host device is disconnected.
>>
>> At first I've extended set_fd_handlerX() stuff to support checking
>> for exceptions on fds. But unfortunately usbfs code does not wake up
>> user-space process when device is removed, which means we need a
>> timer to periodically check if device is still there. So I removed
>> fd exception stuff and implemented it with the timer.
>>
>
> I'm surprised there isn't an EOF when the device is disconnected. There
> has to be some sort of userspace notification that a device has been
> unplugged. I don't really like the idea of polling but it wouldn't
> surprise me if we had to.
I just wrote a simple test prog and it turns out that usbfs does wake user
process when device is removed. But previously I was select()ing for
'exception' and instead should be checking for 'read' or 'write'.
It turns out that async implementation detects the disconnect anyway because
we register 'write' handler to collect async completions.
Since you're liked the whole patchset I'll just send a delta patch that
removes the disconnect timer altogether.
>> @@ -230,8 +236,11 @@ static void usb_host_handle_destroy(USBDevice *dev)
>> {
>> USBHostDevice *s = (USBHostDevice *)dev;
>>
>> + qemu_del_timer(s->timer);
>> +
>>
>
> qemu_del_timer() only removes a pending timer. You need
> qemu_free_timer() to actually free the memory associated with it.
Makes sense. Since I'm nuking the timer this will go away.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 1/5] husb: support for USB host device auto disconnect.
2008-08-14 16:28 ` [Qemu-devel] " Anthony Liguori
2008-08-14 19:26 ` Max Krasnyansky
@ 2008-08-14 21:41 ` Max Krasnyansky
1 sibling, 0 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 21:41 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, kvm
Anthony Liguori wrote:
> I'm surprised there isn't an EOF when the device is disconnected. There
> has to be some sort of userspace notification that a device has been
> unplugged. I don't really like the idea of polling but it wouldn't
> surprise me if we had to.
I just sent you a patch that removes this polling mechanism. As I mentioned
async handler can actually detect that by itself.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-14 4:22 [Qemu-devel] [PATCH 0/5] Various USB fixes and improvements Max Krasnyansky
2008-08-14 4:22 ` [Qemu-devel] [PATCH 1/5] husb: support for USB host device auto disconnect Max Krasnyansky
@ 2008-08-14 4:22 ` Max Krasnyansky
2008-08-14 16:41 ` [Qemu-devel] " Anthony Liguori
2008-08-14 4:22 ` [Qemu-devel] [PATCH 3/5] usb: generic packet handler cleanup and documentation Max Krasnyansky
` (3 subsequent siblings)
5 siblings, 1 reply; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 4:22 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, Max Krasnyansky
QEMU can now automatically grab host USB devices that match the filter.
For now I just extended 'host:X.Y' and 'host:VID:PID' syntax to handle
wildcards. So for example if you do something like
usb_add host:5.*
QEMU will automatically grab any non-hub device with host address 5.*.
Same with the 'host:PID:*', we grab any device that matches PID.
Filtering itself is very generic so we can probably add more elaborate
syntax like 'host:BUS.ADDR:VID:PID'. So that we can do 'host:5.*:6000:*'.
Anyway, it's implemented using a periodic timer that scans host devices
and grabs those that match the filter. Timer is started when the first
filter is added.
We now keep the list of all host devices that we grabbed to make sure that
we do not grab the same device twice.
btw It's currently possible to grab the same host device more than once.
ie You can just do "usb_add host:1.1" more than once, which of course does
not work. So this patch fixes that issue too.
Along with auto disconnect patch that I send a minute ago the setup is very
seamless now. You can just allocate some usb ports to the VMs and plug/unplug
devices at any time.
Signed-off-by: Max Krasnyansky <maxk@kernel.org>
---
hw/usb.h | 1 +
usb-linux.c | 209 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
vl.c | 57 ++++++++--------
3 files changed, 225 insertions(+), 42 deletions(-)
diff --git a/hw/usb.h b/hw/usb.h
index 2edb982..4a009a5 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -197,6 +197,7 @@ static inline void usb_cancel_packet(USBPacket * p)
p->cancel_cb(p, p->cancel_opaque);
}
+int usb_device_add_dev(USBDevice *dev);
int usb_device_del_addr(int bus_num, int addr);
void usb_attach(USBPort *port, USBDevice *dev);
int usb_generic_handle_packet(USBDevice *s, USBPacket *p);
diff --git a/usb-linux.c b/usb-linux.c
index 0023c1d..622255c 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -3,6 +3,9 @@
*
* Copyright (c) 2005 Fabrice Bellard
*
+ * Support for host device auto connect & disconnect
+ * Copyright (c) 2008 Max Krasnyansky
+ *
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
@@ -67,6 +70,8 @@ struct endp_data {
uint8_t type;
};
+
+
/* FIXME: move USBPacket to PendingURB */
typedef struct USBHostDevice {
USBDevice dev;
@@ -78,9 +83,51 @@ typedef struct USBHostDevice {
uint8_t descr[1024];
int descr_len;
int urbs_ready;
+
QEMUTimer *timer;
+
+ /* Host side address */
+ int bus_num;
+ int addr;
+
+ struct USBHostDevice *next;
} USBHostDevice;
+static USBHostDevice *hostdev_list;
+
+static void hostdev_link(USBHostDevice *dev)
+{
+ dev->next = hostdev_list;
+ hostdev_list = dev;
+}
+
+static void hostdev_unlink(USBHostDevice *dev)
+{
+ USBHostDevice *pdev = hostdev_list;
+ USBHostDevice **prev = &hostdev_list;
+
+ while (pdev) {
+ if (pdev == dev) {
+ *prev = dev->next;
+ return;
+ }
+
+ prev = &pdev->next;
+ pdev = pdev->next;
+ }
+}
+
+static USBHostDevice *hostdev_find(int bus_num, int addr)
+{
+ USBHostDevice *s = hostdev_list;
+ while (s) {
+ if (s->bus_num == bus_num && s->addr == addr)
+ return s;
+ s = s->next;
+ }
+ return NULL;
+}
+
typedef struct PendingURB {
struct usbdevfs_urb *urb;
int status;
@@ -238,6 +285,8 @@ static void usb_host_handle_destroy(USBDevice *dev)
qemu_del_timer(s->timer);
+ hostdev_unlink(s);
+
if (s->fd >= 0)
close(s->fd);
@@ -619,32 +668,26 @@ static void usb_host_device_check(void *priv)
qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 1000);
}
-/* XXX: exclude high speed devices or implement EHCI */
-USBDevice *usb_host_device_open(const char *devname)
+static USBDevice *usb_host_device_open_addr(int bus_num, int addr, const char *prod_name)
{
int fd = -1, ret;
USBHostDevice *dev = NULL;
struct usbdevfs_connectinfo ci;
char buf[1024];
- int bus_num, addr;
- char product_name[PRODUCT_NAME_SZ];
-
- if (usb_host_find_device(&bus_num, &addr,
- product_name, sizeof(product_name),
- devname) < 0)
- return NULL;
-
dev = qemu_mallocz(sizeof(USBHostDevice));
if (!dev)
goto fail;
+ dev->bus_num = bus_num;
+ dev->addr = addr;
+
dev->timer = qemu_new_timer(rt_clock, usb_host_device_check, (void *) dev);
if (!dev->timer)
goto fail;
#ifdef DEBUG
- printf("usb_host_device_open %s\n", devname);
+ printf("usb_host_device_open %d.%d\n", bus_num, addr);
#endif
snprintf(buf, sizeof(buf), USBDEVFS_PATH "/%03d/%03d",
@@ -704,12 +747,12 @@ USBDevice *usb_host_device_open(const char *devname)
dev->dev.handle_data = usb_host_handle_data;
dev->dev.handle_destroy = usb_host_handle_destroy;
- if (product_name[0] == '\0')
+ if (!prod_name || prod_name[0] == '\0')
snprintf(dev->dev.devname, sizeof(dev->dev.devname),
- "host:%s", devname);
+ "host:%d.%d", bus_num, addr);
else
pstrcpy(dev->dev.devname, sizeof(dev->dev.devname),
- product_name);
+ prod_name);
#ifdef USE_ASYNCIO
/* set up the signal handlers */
@@ -735,8 +778,11 @@ USBDevice *usb_host_device_open(const char *devname)
/* Start the timer to detect disconnect */
qemu_mod_timer(dev->timer, qemu_get_clock(rt_clock) + 1000);
+ hostdev_link(dev);
+
dev->urbs_ready = 0;
return (USBDevice *)dev;
+
fail:
if (dev) {
if (dev->timer)
@@ -747,6 +793,24 @@ fail:
return NULL;
}
+USBDevice *usb_host_device_open(const char *devname)
+{
+ int bus_num, addr;
+ char product_name[PRODUCT_NAME_SZ];
+
+ if (usb_host_find_device(&bus_num, &addr,
+ product_name, sizeof(product_name),
+ devname) < 0)
+ return NULL;
+
+ if (hostdev_find(bus_num, addr)) {
+ printf("host usb device %d.%d is already open\n", bus_num, addr);
+ return NULL;
+ }
+
+ return usb_host_device_open_addr(bus_num, addr, product_name);
+}
+
static int get_tag_value(char *buf, int buf_size,
const char *str, const char *tag,
const char *stopchars)
@@ -846,6 +910,108 @@ static int usb_host_scan(void *opaque, USBScanFunc *func)
return ret;
}
+struct USBAutoFilter {
+ struct USBAutoFilter *next;
+ int bus_num;
+ int addr;
+ int vendor_id;
+ int product_id;
+};
+
+static QEMUTimer *usb_auto_timer;
+static struct USBAutoFilter *usb_auto_filter;
+
+static int usb_host_auto_scan(void *opaque, int bus_num, int addr,
+ int class_id, int vendor_id, int product_id,
+ const char *product_name, int speed)
+{
+ struct USBAutoFilter *f;
+ struct USBDevice *dev;
+
+ /* Ignore hubs */
+ if (class_id == 9)
+ return 0;
+
+ for (f = usb_auto_filter; f; f = f->next) {
+ // printf("Auto match: bus_num %d addr %d vid %d pid %d\n",
+ // bus_num, addr, vendor_id, product_id);
+
+ if (f->bus_num >= 0 && f->bus_num != bus_num)
+ continue;
+
+ if (f->addr >= 0 && f->addr != addr)
+ continue;
+
+ if (f->vendor_id >= 0 && f->vendor_id != vendor_id)
+ continue;
+
+ if (f->product_id >= 0 && f->product_id != product_id)
+ continue;
+
+ /* We got a match */
+
+ /* Allredy attached ? */
+ if (hostdev_find(bus_num, addr))
+ return 0;
+
+ printf("Auto open: bus_num %d addr %d\n", bus_num, addr);
+
+ dev = usb_host_device_open_addr(bus_num, addr, product_name);
+ if (dev)
+ usb_device_add_dev(dev);
+ }
+
+ return 0;
+}
+
+static void usb_host_auto_timer(void *unused)
+{
+ usb_host_scan(NULL, usb_host_auto_scan);
+ qemu_mod_timer(usb_auto_timer, qemu_get_clock(rt_clock) + 2000);
+}
+
+/*
+ * Add autoconnect filter
+ * -1 means 'any' (device, vendor, etc)
+ */
+static void usb_host_auto_add(int bus_num, int addr, int vendor_id, int product_id)
+{
+ struct USBAutoFilter *f = qemu_mallocz(sizeof(*f));
+ if (!f) {
+ printf("Failed to allocate auto filter\n");
+ return;
+ }
+
+ f->bus_num = bus_num;
+ f->addr = addr;
+ f->vendor_id = vendor_id;
+ f->product_id = product_id;
+
+ if (!usb_auto_filter) {
+ /*
+ * First entry. Init and start the monitor.
+ * Right now we're using timer to check for new devices.
+ * If this turns out to be too expensive we can move that into a
+ * separate thread.
+ */
+ usb_auto_timer = qemu_new_timer(rt_clock, usb_host_auto_timer, NULL);
+ if (!usb_auto_timer) {
+ printf("Failed to allocate timer\n");
+ qemu_free(f);
+ return;
+ }
+
+ /* Check for new devices every two seconds */
+ qemu_mod_timer(usb_auto_timer, qemu_get_clock(rt_clock) + 2000);
+ }
+
+ printf("Auto filter: bus_num %d addr %d vid %d pid %d\n",
+ bus_num, addr, vendor_id, product_id);
+
+ f->next = usb_auto_filter;
+ usb_auto_filter = f;
+}
+
typedef struct FindDeviceState {
int vendor_id;
int product_id;
@@ -887,6 +1053,12 @@ static int usb_host_find_device(int *pbus_num, int *paddr,
p = strchr(devname, '.');
if (p) {
*pbus_num = strtoul(devname, NULL, 0);
+
+ if (*(p + 1) == '*') {
+ usb_host_auto_add(*pbus_num, -1, -1, -1);
+ return -1;
+ }
+
*paddr = strtoul(p + 1, NULL, 0);
fs.bus_num = *pbus_num;
fs.addr = *paddr;
@@ -898,6 +1070,12 @@ static int usb_host_find_device(int *pbus_num, int *paddr,
p = strchr(devname, ':');
if (p) {
fs.vendor_id = strtoul(devname, NULL, 16);
+
+ if (*(p + 1) == '*') {
+ usb_host_auto_add(-1, -1, fs.vendor_id, -1);
+ return -1;
+ }
+
fs.product_id = strtoul(p + 1, NULL, 16);
ret = usb_host_scan(&fs, usb_host_find_device_scan);
if (ret) {
@@ -996,6 +1174,9 @@ void usb_host_info(void)
usb_host_scan(NULL, usb_host_info_device);
}
+
+
+
#else
void usb_host_info(void)
diff --git a/vl.c b/vl.c
index a278599..507dea7 100644
--- a/vl.c
+++ b/vl.c
@@ -5743,6 +5743,32 @@ void qemu_register_usb_port(USBPort *port, void *opaque, int index,
free_usb_ports = port;
}
+int usb_device_add_dev(USBDevice *dev)
+{
+ USBPort *port;
+
+ /* Find a USB port to add the device to. */
+ port = free_usb_ports;
+ if (!port->next) {
+ USBDevice *hub;
+
+ /* Create a new hub and chain it on. */
+ free_usb_ports = NULL;
+ port->next = used_usb_ports;
+ used_usb_ports = port;
+
+ hub = usb_hub_init(VM_USB_HUB_SIZE);
+ usb_attach(port, hub);
+ port = free_usb_ports;
+ }
+
+ free_usb_ports = port->next;
+ port->next = used_usb_ports;
+ used_usb_ports = port;
+ usb_attach(port, dev);
+ return 0;
+}
+
static int usb_device_add(const char *devname)
{
const char *p;
@@ -5783,26 +5809,7 @@ static int usb_device_add(const char *devname)
if (!dev)
return -1;
- /* Find a USB port to add the device to. */
- port = free_usb_ports;
- if (!port->next) {
- USBDevice *hub;
-
- /* Create a new hub and chain it on. */
- free_usb_ports = NULL;
- port->next = used_usb_ports;
- used_usb_ports = port;
-
- hub = usb_hub_init(VM_USB_HUB_SIZE);
- usb_attach(port, hub);
- port = free_usb_ports;
- }
-
- free_usb_ports = port->next;
- port->next = used_usb_ports;
- used_usb_ports = port;
- usb_attach(port, dev);
- return 0;
+ return usb_device_add_dev(dev);
}
int usb_device_del_addr(int bus_num, int addr)
@@ -5855,18 +5862,12 @@ static int usb_device_del(const char *devname)
void do_usb_add(const char *devname)
{
- int ret;
- ret = usb_device_add(devname);
- if (ret < 0)
- term_printf("Could not add USB device '%s'\n", devname);
+ usb_device_add(devname);
}
void do_usb_del(const char *devname)
{
- int ret;
- ret = usb_device_del(devname);
- if (ret < 0)
- term_printf("Could not remove USB device '%s'\n", devname);
+ usb_device_del(devname);
}
void usb_info(void)
--
1.5.5.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-14 4:22 ` [Qemu-devel] [PATCH 2/5] husb: support for USB host device auto connect Max Krasnyansky
@ 2008-08-14 16:41 ` Anthony Liguori
2008-08-14 19:38 ` Max Krasnyansky
0 siblings, 1 reply; 37+ messages in thread
From: Anthony Liguori @ 2008-08-14 16:41 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: qemu-devel, kvm
Max Krasnyansky wrote:
> Anyway, it's implemented using a periodic timer that scans host devices
> and grabs those that match the filter. Timer is started when the first
> filter is added.
>
Again, there has to be a way to get notified of usb device add/remove in
Linux.
> +static USBHostDevice *hostdev_find(int bus_num, int addr)
> +{
> + USBHostDevice *s = hostdev_list;
> + while (s) {
> + if (s->bus_num == bus_num && s->addr == addr)
> + return s;
> + s = s->next;
> + }
> + return NULL;
> +}
>
There are generic list macros in sys-queue.h. They aren't universally
used though so it's up to you whether you think it's appropriate.
>
> +struct USBAutoFilter {
> + struct USBAutoFilter *next;
> + int bus_num;
> + int addr;
> + int vendor_id;
> + int product_id;
> +};
> +
> +static QEMUTimer *usb_auto_timer;
> +static struct USBAutoFilter *usb_auto_filter;
> +
> +static int usb_host_auto_scan(void *opaque, int bus_num, int addr,
> + int class_id, int vendor_id, int product_id,
> + const char *product_name, int speed)
> +{
> + struct USBAutoFilter *f;
> + struct USBDevice *dev;
> +
> + /* Ignore hubs */
> + if (class_id == 9)
> + return 0;
> +
> + for (f = usb_auto_filter; f; f = f->next) {
> + // printf("Auto match: bus_num %d addr %d vid %d pid %d\n",
> + // bus_num, addr, vendor_id, product_id);
>
An #ifdef guard would be nicer.
> + if (f->bus_num >= 0 && f->bus_num != bus_num)
> + continue;
> +
> + if (f->addr >= 0 && f->addr != addr)
> + continue;
> +
> + if (f->vendor_id >= 0 && f->vendor_id != vendor_id)
> + continue;
> +
> + if (f->product_id >= 0 && f->product_id != product_id)
> + continue;
> +
> + /* We got a match */
> +
> + /* Allredy attached ? */
> + if (hostdev_find(bus_num, addr))
> + return 0;
> +
> + printf("Auto open: bus_num %d addr %d\n", bus_num, addr);
>
Please make this debug too.
> + dev = usb_host_device_open_addr(bus_num, addr, product_name);
> + if (dev)
> + usb_device_add_dev(dev);
> + }
> +
> + return 0;
> +}
> +
> +static void usb_host_auto_timer(void *unused)
> +{
> + usb_host_scan(NULL, usb_host_auto_scan);
> + qemu_mod_timer(usb_auto_timer, qemu_get_clock(rt_clock) + 2000);
> +}
> +
> +/*
> + * Add autoconnect filter
> + * -1 means 'any' (device, vendor, etc)
> + */
> +static void usb_host_auto_add(int bus_num, int addr, int vendor_id, int product_id)
> +{
> + struct USBAutoFilter *f = qemu_mallocz(sizeof(*f));
> + if (!f) {
> + printf("Failed to allocate auto filter\n");
> + return;
> + }
> +
> + f->bus_num = bus_num;
> + f->addr = addr;
> + f->vendor_id = vendor_id;
> + f->product_id = product_id;
> +
> + if (!usb_auto_filter) {
> + /*
> + * First entry. Init and start the monitor.
> + * Right now we're using timer to check for new devices.
> + * If this turns out to be too expensive we can move that into a
> + * separate thread.
> + */
> + usb_auto_timer = qemu_new_timer(rt_clock, usb_host_auto_timer, NULL);
> + if (!usb_auto_timer) {
> + printf("Failed to allocate timer\n");
>
Please print errors to stderr.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-14 16:41 ` [Qemu-devel] " Anthony Liguori
@ 2008-08-14 19:38 ` Max Krasnyansky
2008-08-14 20:21 ` Anthony Liguori
0 siblings, 1 reply; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 19:38 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, kvm
Anthony Liguori wrote:
> Max Krasnyansky wrote:
>> Anyway, it's implemented using a periodic timer that scans host devices
>> and grabs those that match the filter. Timer is started when the first
>> filter is added.
>>
>
> Again, there has to be a way to get notified of usb device add/remove in
> Linux.
Yes the best way is probably registering with HAL via dbus.
Do you mind if we do something like that incrementally ?
ie Merge the current patch then change polling to HAL registration.
That would simplify testing/patch stacking for me. Existing users are
not affected anyway since the timer is activated only when new filter is added.
>> +static USBHostDevice *hostdev_find(int bus_num, int addr)
>> +{
>> + USBHostDevice *s = hostdev_list;
>> + while (s) {
>> + if (s->bus_num == bus_num && s->addr == addr)
>> + return s;
>> + s = s->next;
>> + }
>> + return NULL;
>> +}
>>
>
> There are generic list macros in sys-queue.h. They aren't universally
> used though so it's up to you whether you think it's appropriate.
Oh, I looked around (mostly USB code) and did not see any generic lists.
I'd say ideally we should clean things up to use generic stuff as much as
possible. There seems to be a lot of duplication currently.
>> +struct USBAutoFilter {
>> + struct USBAutoFilter *next;
>> + int bus_num;
>> + int addr;
>> + int vendor_id;
>> + int product_id;
>> +};
>> +
>> +static QEMUTimer *usb_auto_timer;
>> +static struct USBAutoFilter *usb_auto_filter;
>> +
>> +static int usb_host_auto_scan(void *opaque, int bus_num, int addr,
>> + int class_id, int vendor_id, int product_id,
>> + const char *product_name, int speed)
>> +{
>> + struct USBAutoFilter *f;
>> + struct USBDevice *dev;
>> +
>> + /* Ignore hubs */
>> + if (class_id == 9)
>> + return 0;
>> +
>> + for (f = usb_auto_filter; f; f = f->next) {
>> + // printf("Auto match: bus_num %d addr %d vid %d pid %d\n",
>> + // bus_num, addr, vendor_id, product_id);
>>
>
> An #ifdef guard would be nicer.
Yep. Missed that one.
>> + if (f->bus_num >= 0 && f->bus_num != bus_num)
>> + continue;
>> +
>> + if (f->addr >= 0 && f->addr != addr)
>> + continue;
>> +
>> + if (f->vendor_id >= 0 && f->vendor_id != vendor_id)
>> + continue;
>> +
>> + if (f->product_id >= 0 && f->product_id != product_id)
>> + continue;
>> +
>> + /* We got a match */
>> +
>> + /* Allredy attached ? */
>> + if (hostdev_find(bus_num, addr))
>> + return 0;
>> +
>> + printf("Auto open: bus_num %d addr %d\n", bus_num, addr);
>>
>
> Please make this debug too.
Will do.
>
>> + dev = usb_host_device_open_addr(bus_num, addr, product_name);
>> + if (dev)
>> + usb_device_add_dev(dev);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void usb_host_auto_timer(void *unused)
>> +{
>> + usb_host_scan(NULL, usb_host_auto_scan);
>> + qemu_mod_timer(usb_auto_timer, qemu_get_clock(rt_clock) + 2000);
>> +}
>> +
>> +/*
>> + * Add autoconnect filter
>> + * -1 means 'any' (device, vendor, etc)
>> + */
>> +static void usb_host_auto_add(int bus_num, int addr, int vendor_id,
>> int product_id)
>> +{
>> + struct USBAutoFilter *f = qemu_mallocz(sizeof(*f));
>> + if (!f) {
>> + printf("Failed to allocate auto filter\n");
>> + return;
>> + }
>> +
>> + f->bus_num = bus_num;
>> + f->addr = addr;
>> + f->vendor_id = vendor_id;
>> + f->product_id = product_id;
>> +
>> + if (!usb_auto_filter) {
>> + /*
>> + * First entry. Init and start the monitor.
>> + * Right now we're using timer to check for new devices.
>> + * If this turns out to be too expensive we can move that
>> into a + * separate thread.
>> + */
>> + usb_auto_timer = qemu_new_timer(rt_clock, usb_host_auto_timer,
>> NULL);
>> + if (!usb_auto_timer) {
>> + printf("Failed to allocate timer\n");
>>
>
> Please print errors to stderr.
Will do.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-14 19:38 ` Max Krasnyansky
@ 2008-08-14 20:21 ` Anthony Liguori
2008-08-14 20:34 ` Max Krasnyansky
0 siblings, 1 reply; 37+ messages in thread
From: Anthony Liguori @ 2008-08-14 20:21 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: qemu-devel, kvm
Max Krasnyansky wrote:
> Anthony Liguori wrote:
>
>> Max Krasnyansky wrote:
>>
>>> Anyway, it's implemented using a periodic timer that scans host devices
>>> and grabs those that match the filter. Timer is started when the first
>>> filter is added.
>>>
>>>
>> Again, there has to be a way to get notified of usb device add/remove in
>> Linux.
>>
>
> Yes the best way is probably registering with HAL via dbus.
>
I was afraid you would say that :-)
> Do you mind if we do something like that incrementally ?
>
Yeah, I don't really want to have QEMU depend on dbus so in this case,
polling would be better.
> ie Merge the current patch then change polling to HAL registration.
> That would simplify testing/patch stacking for me. Existing users are
> not affected anyway since the timer is activated only when new filter is added.
>
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-14 20:21 ` Anthony Liguori
@ 2008-08-14 20:34 ` Max Krasnyansky
2008-08-14 20:41 ` Anthony Liguori
0 siblings, 1 reply; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 20:34 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, kvm
Anthony Liguori wrote:
> Max Krasnyansky wrote:
>> Anthony Liguori wrote:
>>
>>> Max Krasnyansky wrote:
>>>
>>>> Anyway, it's implemented using a periodic timer that scans host devices
>>>> and grabs those that match the filter. Timer is started when the first
>>>> filter is added.
>>>>
>>> Again, there has to be a way to get notified of usb device add/remove in
>>> Linux.
>>>
>>
>> Yes the best way is probably registering with HAL via dbus.
>>
> I was afraid you would say that :-)
:)
>> Do you mind if we do something like that incrementally ?
>>
>
> Yeah, I don't really want to have QEMU depend on dbus so in this case,
> polling would be better.
I'm thinking that maybe we could use 'inotify' on /proc/bus/usb.
Would you be ok with 'inotify' ?
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-14 20:34 ` Max Krasnyansky
@ 2008-08-14 20:41 ` Anthony Liguori
2008-08-14 21:14 ` François Revol
2008-08-15 7:46 ` Guido Günther
0 siblings, 2 replies; 37+ messages in thread
From: Anthony Liguori @ 2008-08-14 20:41 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: qemu-devel, kvm
Max Krasnyansky wrote:
>>> Do you mind if we do something like that incrementally ?
>>>
>>>
>> Yeah, I don't really want to have QEMU depend on dbus so in this case,
>> polling would be better.
>>
>
> I'm thinking that maybe we could use 'inotify' on /proc/bus/usb.
> Would you be ok with 'inotify' ?
>
Yeah, but I would be a little surprised if /proc/bus/usb supports inotify...
Regards,
Anthony Liguori
> Max
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-14 20:41 ` Anthony Liguori
@ 2008-08-14 21:14 ` François Revol
2008-08-15 7:46 ` Guido Günther
1 sibling, 0 replies; 37+ messages in thread
From: François Revol @ 2008-08-14 21:14 UTC (permalink / raw)
To: qemu-devel
> > I'm thinking that maybe we could use 'inotify' on /proc/bus/usb.
> > Would you be ok with 'inotify' ?
> >
>
> Yeah, but I would be a little surprised if /proc/bus/usb supports
> inotify...
Linux finally gets node monitoring, but it doesn't use it... oh well,
that's depressing :^)
Couldn't a select() on the fd be made to return exception ?
François.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-14 20:41 ` Anthony Liguori
2008-08-14 21:14 ` François Revol
@ 2008-08-15 7:46 ` Guido Günther
2008-08-15 18:24 ` Max Krasnyansky
1 sibling, 1 reply; 37+ messages in thread
From: Guido Günther @ 2008-08-15 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, Max Krasnyansky
On Thu, Aug 14, 2008 at 03:41:28PM -0500, Anthony Liguori wrote:
> Max Krasnyansky wrote:
>>>> Do you mind if we do something like that incrementally ?
>>>>
>>> Yeah, I don't really want to have QEMU depend on dbus so in this case,
>>> polling would be better.
>>>
>>
>> I'm thinking that maybe we could use 'inotify' on /proc/bus/usb.
>> Would you be ok with 'inotify' ?
>>
>
> Yeah, but I would be a little surprised if /proc/bus/usb supports inotify...
What about /dev/bus/usb - it supports inotify fine on udev?
-- Guido
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-15 7:46 ` Guido Günther
@ 2008-08-15 18:24 ` Max Krasnyansky
2008-08-15 18:31 ` Javier Guerra
` (2 more replies)
0 siblings, 3 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-15 18:24 UTC (permalink / raw)
To: Guido Günther; +Cc: qemu-devel, kvm
Guido Günther wrote:
> On Thu, Aug 14, 2008 at 03:41:28PM -0500, Anthony Liguori wrote:
>> Max Krasnyansky wrote:
>>>>> Do you mind if we do something like that incrementally ?
>>>>>
>>>> Yeah, I don't really want to have QEMU depend on dbus so in this case,
>>>> polling would be better.
>>>>
>>> I'm thinking that maybe we could use 'inotify' on /proc/bus/usb.
>>> Would you be ok with 'inotify' ?
>>>
>> Yeah, but I would be a little surprised if /proc/bus/usb supports inotify...
> What about /dev/bus/usb - it supports inotify fine on udev?
Yes it should since it's a regular filesystem.
Now inotify based solution probably won't be pretty because we'd have to
monitor each subdir. ie When new device get added top level /dev/bus/usb is
not modified, what does get modified is /dev/bus/usb/<bus_num>/ directory so
we'd have to monitor /dev/bus/usb and dynamically register/unregister monitors
for each /dev/bus/usb/<bus_num>/.
Maybe it won't be that bad. If I get a chance I'll give it a shot.
btw Interface to HAL might still be useful in general to monitor other device
classes that we may want to automatically assign to the VMs. So I'll play
around with that too (some day :)).
Max
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-15 18:24 ` Max Krasnyansky
@ 2008-08-15 18:31 ` Javier Guerra
2008-08-18 18:21 ` Max Krasnyansky
2008-08-17 7:52 ` Avi Kivity
2008-08-18 14:11 ` Anthony Liguori
2 siblings, 1 reply; 37+ messages in thread
From: Javier Guerra @ 2008-08-15 18:31 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: Guido Günther, qemu-devel, kvm
On Fri, Aug 15, 2008 at 1:24 PM, Max Krasnyansky <maxk@kernel.org> wrote:
> btw Interface to HAL might still be useful in general to monitor other device
> classes that we may want to automatically assign to the VMs. So I'll play
> around with that too (some day :)).
what about doing it the other way around? that is, setting udev
scripts that notify KVM of the hardware changes.
--
Javier
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-15 18:31 ` Javier Guerra
@ 2008-08-18 18:21 ` Max Krasnyansky
2008-08-18 18:52 ` Javier Guerra
0 siblings, 1 reply; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-18 18:21 UTC (permalink / raw)
To: Javier Guerra; +Cc: Guido Günther, qemu-devel, kvm
Javier Guerra wrote:
> On Fri, Aug 15, 2008 at 1:24 PM, Max Krasnyansky <maxk@kernel.org> wrote:
>
>> btw Interface to HAL might still be useful in general to monitor other device
>> classes that we may want to automatically assign to the VMs. So I'll play
>> around with that too (some day :)).
>
> what about doing it the other way around? that is, setting udev
> scripts that notify KVM of the hardware changes.
That seems a bit odd. What if you have more than one QEMU instance and
stuff.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-18 18:21 ` Max Krasnyansky
@ 2008-08-18 18:52 ` Javier Guerra
2008-08-18 18:56 ` Jamie Lokier
0 siblings, 1 reply; 37+ messages in thread
From: Javier Guerra @ 2008-08-18 18:52 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: Guido Günther, qemu-devel, kvm
On Mon, Aug 18, 2008 at 1:21 PM, Max Krasnyansky <maxk@kernel.org> wrote:
> Javier Guerra wrote:
>>
>> what about doing it the other way around? that is, setting udev
>> scripts that notify KVM of the hardware changes.
>
> That seems a bit odd. What if you have more than one QEMU instance and
> stuff.
there has to be some policy in place to redirect USB devices to each
QEMU instance, maybe at startup it could reserve a node in the USB
device tree (an USB controller, or maybe a port in a hub). when
invoked by udev, some script would consult those reservations and pick
the appropriate QEMU
yep, it's not trivial, but seems doable with scripts (and a small DB,
or maybe a clever filesystem arrangement).
--
Javier
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-18 18:52 ` Javier Guerra
@ 2008-08-18 18:56 ` Jamie Lokier
0 siblings, 0 replies; 37+ messages in thread
From: Jamie Lokier @ 2008-08-18 18:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Guido Günther, kvm, Max Krasnyansky
Javier Guerra wrote:
> there has to be some policy in place to redirect USB devices to each
> QEMU instance, maybe at startup it could reserve a node in the USB
> device tree (an USB controller, or maybe a port in a hub). when
> invoked by udev, some script would consult those reservations and pick
> the appropriate QEMU
>
> yep, it's not trivial, but seems doable with scripts (and a small DB,
> or maybe a clever filesystem arrangement).
I think that's more or less how udev is *supposed* to be used.
It's weird for a bit, then it makes sense.
Especially as it can integrate nicely with distro scripts, e.g.
for networking.
But HAL and PolicyKit etc. turn it all on its head and you soon get
lost in a twisty maze of configuration files.
-- Jamie
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-15 18:24 ` Max Krasnyansky
2008-08-15 18:31 ` Javier Guerra
@ 2008-08-17 7:52 ` Avi Kivity
2008-08-18 18:46 ` Max Krasnyansky
2008-08-18 14:11 ` Anthony Liguori
2 siblings, 1 reply; 37+ messages in thread
From: Avi Kivity @ 2008-08-17 7:52 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: Guido Günther, qemu-devel, kvm
Max Krasnyansky wrote:
> btw Interface to HAL might still be useful in general to monitor other device
> classes that we may want to automatically assign to the VMs. So I'll play
> around with that too (some day :)).
>
Assign to which VMs?
Interfacing with HAL is definitely the management layer's tasks, not
qemu, which only knows about one guest.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-17 7:52 ` Avi Kivity
@ 2008-08-18 18:46 ` Max Krasnyansky
0 siblings, 0 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-18 18:46 UTC (permalink / raw)
To: Avi Kivity; +Cc: Guido Günther, qemu-devel, kvm
Avi Kivity wrote:
> Max Krasnyansky wrote:
>> btw Interface to HAL might still be useful in general to monitor other
>> device
>> classes that we may want to automatically assign to the VMs. So I'll play
>> around with that too (some day :)).
>>
>
> Assign to which VMs?
>
> Interfacing with HAL is definitely the management layer's tasks, not
> qemu, which only knows about one guest.
With my USB patches you can now do
qemu -name VM1 ... -usb -usbdevice host:5.*
which will dynamically grab any USB device that is attached to port 5.
And you can start another instance with
qemu -name VM2 ... -usb -usbdevice host:6.*
and so on.
That's what I meant by assigning to the VM. ie In the example above
port5 is assigned to VM1 and port6 to VM2.
We might want to do this with other devices (disks maybe).
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-15 18:24 ` Max Krasnyansky
2008-08-15 18:31 ` Javier Guerra
2008-08-17 7:52 ` Avi Kivity
@ 2008-08-18 14:11 ` Anthony Liguori
2008-08-18 18:16 ` Max Krasnyansky
2 siblings, 1 reply; 37+ messages in thread
From: Anthony Liguori @ 2008-08-18 14:11 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: Guido Günther, qemu-devel, kvm
Max Krasnyansky wrote:
> Guido Günther wrote:
>
>> What about /dev/bus/usb - it supports inotify fine on udev?
>>
>
> Yes it should since it's a regular filesystem.
>
I was thinking of this too since /proc/bus/usb is deprecated in favor of
/dev/bus/usb. If someone was willing to port QEMU to /dev/bus/usb, it
would be greatly appreciated!
Regards,
Anthony Liguori
> Now inotify based solution probably won't be pretty because we'd have to
> monitor each subdir. ie When new device get added top level /dev/bus/usb is
> not modified, what does get modified is /dev/bus/usb/<bus_num>/ directory so
> we'd have to monitor /dev/bus/usb and dynamically register/unregister monitors
> for each /dev/bus/usb/<bus_num>/.
> Maybe it won't be that bad. If I get a chance I'll give it a shot.
>
> btw Interface to HAL might still be useful in general to monitor other device
> classes that we may want to automatically assign to the VMs. So I'll play
> around with that too (some day :)).
>
> Max
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 2/5] husb: support for USB host device auto connect.
2008-08-18 14:11 ` Anthony Liguori
@ 2008-08-18 18:16 ` Max Krasnyansky
0 siblings, 0 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-18 18:16 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Guido Günther, qemu-devel, kvm
Anthony Liguori wrote:
> Max Krasnyansky wrote:
>> Guido Günther wrote:
>>
>>> What about /dev/bus/usb - it supports inotify fine on udev?
>>>
>>
>> Yes it should since it's a regular filesystem.
>>
>
> I was thinking of this too since /proc/bus/usb is deprecated in favor of
> /dev/bus/usb. If someone was willing to port QEMU to /dev/bus/usb, it
> would be greatly appreciated!
Sure. It's a one liner. I have one more patch for UHCI and a couple for
usb-linux. I'll include this as well.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 3/5] usb: generic packet handler cleanup and documentation
2008-08-14 4:22 [Qemu-devel] [PATCH 0/5] Various USB fixes and improvements Max Krasnyansky
2008-08-14 4:22 ` [Qemu-devel] [PATCH 1/5] husb: support for USB host device auto disconnect Max Krasnyansky
2008-08-14 4:22 ` [Qemu-devel] [PATCH 2/5] husb: support for USB host device auto connect Max Krasnyansky
@ 2008-08-14 4:22 ` Max Krasnyansky
2008-08-14 4:22 ` [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions Max Krasnyansky
` (2 subsequent siblings)
5 siblings, 0 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 4:22 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, Max Krasnyansky
A bit better documentation of the USB device API, namely
return codes.
Rewrite of usb_generic_handle_packet() to make it more
reable and easier to follow.
Signed-off-by: Max Krasnyansky <maxk@kernel.org>
---
hw/usb.c | 265 ++++++++++++++++++++++++++++++++++----------------------------
hw/usb.h | 35 ++++++++-
2 files changed, 179 insertions(+), 121 deletions(-)
diff --git a/hw/usb.c b/hw/usb.c
index be4d66d..c17266d 100644
--- a/hw/usb.c
+++ b/hw/usb.c
@@ -3,6 +3,8 @@
*
* Copyright (c) 2005 Fabrice Bellard
*
+ * 2008 Generic packet handler rewrite by Max Krasnyansky
+ *
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
@@ -30,6 +32,7 @@ void usb_attach(USBPort *port, USBDevice *dev)
}
/**********************/
+
/* generic USB device helpers (you are not forced to use them when
writing your USB device driver, but they help handling the
protocol)
@@ -39,141 +42,164 @@ void usb_attach(USBPort *port, USBDevice *dev)
#define SETUP_STATE_DATA 1
#define SETUP_STATE_ACK 2
-int usb_generic_handle_packet(USBDevice *s, USBPacket *p)
+static int do_token_setup(USBDevice *s, USBPacket *p)
+{
+ int request, value, index;
+ int ret = 0;
+
+ if (p->len != 8)
+ return USB_RET_STALL;
+
+ memcpy(s->setup_buf, p->data, 8);
+ s->setup_len = (s->setup_buf[7] << 8) | s->setup_buf[6];
+ s->setup_index = 0;
+
+ request = (s->setup_buf[0] << 8) | s->setup_buf[1];
+ value = (s->setup_buf[3] << 8) | s->setup_buf[2];
+ index = (s->setup_buf[5] << 8) | s->setup_buf[4];
+
+ if (s->setup_buf[0] & USB_DIR_IN) {
+ ret = s->handle_control(s, request, value, index,
+ s->setup_len, s->data_buf);
+ if (ret < 0)
+ return ret;
+
+ if (ret < s->setup_len)
+ s->setup_len = ret;
+ s->setup_state = SETUP_STATE_DATA;
+ } else {
+ if (s->setup_len == 0)
+ s->setup_state = SETUP_STATE_ACK;
+ else
+ s->setup_state = SETUP_STATE_DATA;
+ }
+
+ return ret;
+}
+
+static int do_token_in(USBDevice *s, USBPacket *p)
{
- int l, ret = 0;
- int len = p->len;
- uint8_t *data = p->data;
+ int request, value, index;
+ int ret = 0;
+
+ if (p->devep != 0)
+ return s->handle_data(s, p);
+
+ request = (s->setup_buf[0] << 8) | s->setup_buf[1];
+ value = (s->setup_buf[3] << 8) | s->setup_buf[2];
+ index = (s->setup_buf[5] << 8) | s->setup_buf[4];
+
+ switch(s->setup_state) {
+ case SETUP_STATE_ACK:
+ if (!(s->setup_buf[0] & USB_DIR_IN)) {
+ s->setup_state = SETUP_STATE_IDLE;
+ ret = s->handle_control(s, request, value, index,
+ s->setup_len, s->data_buf);
+ if (ret > 0)
+ return 0;
+ return ret;
+ }
+
+ /* return 0 byte */
+ return 0;
+
+ case SETUP_STATE_DATA:
+ if (s->setup_buf[0] & USB_DIR_IN) {
+ int len = s->setup_len - s->setup_index;
+ if (len > p->len)
+ len = p->len;
+ memcpy(p->data, s->data_buf + s->setup_index, len);
+ s->setup_index += len;
+ if (s->setup_index >= s->setup_len)
+ s->setup_state = SETUP_STATE_ACK;
+ return len;
+ }
+
+ s->setup_state = SETUP_STATE_IDLE;
+ return USB_RET_STALL;
+
+ default:
+ return USB_RET_STALL;
+ }
+}
+
+static int do_token_out(USBDevice *s, USBPacket *p)
+{
+ if (p->devep != 0)
+ return s->handle_data(s, p);
+
+ switch(s->setup_state) {
+ case SETUP_STATE_ACK:
+ if (s->setup_buf[0] & USB_DIR_IN) {
+ s->setup_state = SETUP_STATE_IDLE;
+ /* transfer OK */
+ } else {
+ /* ignore additional output */
+ }
+ return 0;
+
+ case SETUP_STATE_DATA:
+ if (!(s->setup_buf[0] & USB_DIR_IN)) {
+ int len = s->setup_len - s->setup_index;
+ if (len > p->len)
+ len = p->len;
+ memcpy(s->data_buf + s->setup_index, p->data, len);
+ s->setup_index += len;
+ if (s->setup_index >= s->setup_len)
+ s->setup_state = SETUP_STATE_ACK;
+ return len;
+ }
+
+ s->setup_state = SETUP_STATE_IDLE;
+ return USB_RET_STALL;
+
+ default:
+ return USB_RET_STALL;
+ }
+}
+/*
+ * Generic packet handler.
+ * Called by the HC (host controller).
+ *
+ * Returns length of the transaction or one of the USB_RET_XXX codes.
+ */
+int usb_generic_handle_packet(USBDevice *s, USBPacket *p)
+{
switch(p->pid) {
case USB_MSG_ATTACH:
s->state = USB_STATE_ATTACHED;
- break;
+ return 0;
+
case USB_MSG_DETACH:
s->state = USB_STATE_NOTATTACHED;
- break;
+ return 0;
+
case USB_MSG_RESET:
s->remote_wakeup = 0;
s->addr = 0;
s->state = USB_STATE_DEFAULT;
s->handle_reset(s);
- break;
+ return 0;
+ }
+
+ /* Rest of the PIDs must match our address */
+ if (s->state < USB_STATE_DEFAULT || p->devaddr != s->addr)
+ return USB_RET_NODEV;
+
+ switch (p->pid) {
case USB_TOKEN_SETUP:
- if (s->state < USB_STATE_DEFAULT || p->devaddr != s->addr)
- return USB_RET_NODEV;
- if (len != 8)
- goto fail;
- memcpy(s->setup_buf, data, 8);
- s->setup_len = (s->setup_buf[7] << 8) | s->setup_buf[6];
- s->setup_index = 0;
- if (s->setup_buf[0] & USB_DIR_IN) {
- ret = s->handle_control(s,
- (s->setup_buf[0] << 8) | s->setup_buf[1],
- (s->setup_buf[3] << 8) | s->setup_buf[2],
- (s->setup_buf[5] << 8) | s->setup_buf[4],
- s->setup_len,
- s->data_buf);
- if (ret < 0)
- return ret;
- if (ret < s->setup_len)
- s->setup_len = ret;
- s->setup_state = SETUP_STATE_DATA;
- } else {
- if (s->setup_len == 0)
- s->setup_state = SETUP_STATE_ACK;
- else
- s->setup_state = SETUP_STATE_DATA;
- }
- break;
+ return do_token_setup(s, p);
+
case USB_TOKEN_IN:
- if (s->state < USB_STATE_DEFAULT || p->devaddr != s->addr)
- return USB_RET_NODEV;
- switch(p->devep) {
- case 0:
- switch(s->setup_state) {
- case SETUP_STATE_ACK:
- if (!(s->setup_buf[0] & USB_DIR_IN)) {
- s->setup_state = SETUP_STATE_IDLE;
- ret = s->handle_control(s,
- (s->setup_buf[0] << 8) | s->setup_buf[1],
- (s->setup_buf[3] << 8) | s->setup_buf[2],
- (s->setup_buf[5] << 8) | s->setup_buf[4],
- s->setup_len,
- s->data_buf);
- if (ret > 0)
- ret = 0;
- } else {
- /* return 0 byte */
- }
- break;
- case SETUP_STATE_DATA:
- if (s->setup_buf[0] & USB_DIR_IN) {
- l = s->setup_len - s->setup_index;
- if (l > len)
- l = len;
- memcpy(data, s->data_buf + s->setup_index, l);
- s->setup_index += l;
- if (s->setup_index >= s->setup_len)
- s->setup_state = SETUP_STATE_ACK;
- ret = l;
- } else {
- s->setup_state = SETUP_STATE_IDLE;
- goto fail;
- }
- break;
- default:
- goto fail;
- }
- break;
- default:
- ret = s->handle_data(s, p);
- break;
- }
- break;
+ return do_token_in(s, p);
+
case USB_TOKEN_OUT:
- if (s->state < USB_STATE_DEFAULT || p->devaddr != s->addr)
- return USB_RET_NODEV;
- switch(p->devep) {
- case 0:
- switch(s->setup_state) {
- case SETUP_STATE_ACK:
- if (s->setup_buf[0] & USB_DIR_IN) {
- s->setup_state = SETUP_STATE_IDLE;
- /* transfer OK */
- } else {
- /* ignore additional output */
- }
- break;
- case SETUP_STATE_DATA:
- if (!(s->setup_buf[0] & USB_DIR_IN)) {
- l = s->setup_len - s->setup_index;
- if (l > len)
- l = len;
- memcpy(s->data_buf + s->setup_index, data, l);
- s->setup_index += l;
- if (s->setup_index >= s->setup_len)
- s->setup_state = SETUP_STATE_ACK;
- ret = l;
- } else {
- s->setup_state = SETUP_STATE_IDLE;
- goto fail;
- }
- break;
- default:
- goto fail;
- }
- break;
- default:
- ret = s->handle_data(s, p);
- break;
- }
- break;
+ return do_token_out(s, p);
+
default:
- fail:
- ret = USB_RET_STALL;
- break;
+ return USB_RET_STALL;
}
- return ret;
}
/* XXX: fix overflow */
@@ -200,5 +226,6 @@ void usb_send_msg(USBDevice *dev, int msg)
memset(&p, 0, sizeof(p));
p.pid = msg;
dev->handle_packet(dev, &p);
-}
+ /* This _must_ be synchronous */
+}
diff --git a/hw/usb.h b/hw/usb.h
index 4a009a5..55b7e58 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -120,18 +120,49 @@ typedef struct USBPacket USBPacket;
/* definition of a USB device */
struct USBDevice {
void *opaque;
+
+ /*
+ * Process USB packet.
+ * Called by the HC (Host Controller).
+ *
+ * Returns length of the transaction
+ * or one of the USB_RET_XXX codes.
+ */
int (*handle_packet)(USBDevice *dev, USBPacket *p);
+
+ /*
+ * Called when device is destroyed.
+ */
void (*handle_destroy)(USBDevice *dev);
int speed;
/* The following fields are used by the generic USB device
- layer. They are here just to avoid creating a new structure for
- them. */
+ layer. They are here just to avoid creating a new structure
+ for them. */
+
+ /*
+ * Reset the device
+ */
void (*handle_reset)(USBDevice *dev);
+
+ /*
+ * Process control request.
+ * Called from handle_packet().
+ *
+ * Returns length or one of the USB_RET_ codes.
+ */
int (*handle_control)(USBDevice *dev, int request, int value,
int index, int length, uint8_t *data);
+
+ /*
+ * Process data transfers (both BULK and ISOC).
+ * Called from handle_packet().
+ *
+ * Returns length or one of the USB_RET_ codes.
+ */
int (*handle_data)(USBDevice *dev, USBPacket *p);
+
uint8_t addr;
char devname[32];
--
1.5.5.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions
2008-08-14 4:22 [Qemu-devel] [PATCH 0/5] Various USB fixes and improvements Max Krasnyansky
` (2 preceding siblings ...)
2008-08-14 4:22 ` [Qemu-devel] [PATCH 3/5] usb: generic packet handler cleanup and documentation Max Krasnyansky
@ 2008-08-14 4:22 ` Max Krasnyansky
2008-08-14 17:51 ` [Qemu-devel] " Anthony Liguori
2008-10-11 23:54 ` [Qemu-devel] " Juergen Lock
2008-08-14 4:22 ` [Qemu-devel] [PATCH 5/5] husb: rewrite Linux host USB layer, fully async operation Max Krasnyansky
2008-08-14 17:55 ` [Qemu-devel] Re: [PATCH 0/5] Various USB fixes and improvements Anthony Liguori
5 siblings, 2 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 4:22 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, Max Krasnyansky
This is esentially a re-write of the QEMU UHCI layer. My initial goal
was to support fully async operation with multiple outstanding async
transactions. Along the way I realized that I can greatly simplify
and cleanup the overall logic. There was a lot of duplicate and confusing
code in the UHCI data structure parsing and other places.
We were actually violating UHCI spec in handling async ISOC transaction
(host controller is not supposed to write into the frame pointer).
The reason I wanted to support fully async operation is because current
synchronous version is unusable with most devices exported from host
(via usb-linux.c). Transactions take a long time and the whole VM becomes
slow as hell.
Current async support is very rudimentory and for the most part
non-functional. Single transaction at a time is simply not enough. I have
a device for which XP driver submits both IN and OUT packets at the same
time. IN packet always times out unless OUT packet makes it to the device.
Hence we must be able to process both in order for that device to work.
The new code is backwards compatible and was first tested agains original
synchronous usb-linux.c and builtin usb devices like tablet which is also
synchronous. Rewrite of the usb-linux.c is coming up next.
Async support was tested against various XP versions (ie XP, SP2, SP3) and
a bunch of different USB devices: serial port controllers, mice, keyboard,
JTAG dongles (from Xilinx and Altera).
ISOC support was only lighly tested and needs more work. It's not any worse
than current code though.
UHCI parser changes are probably somewhat hard to review without the
understanding of the UHCI spec.
The async design should be fairly easy to follow. Basically we have a list
of async objects for each pending transfer. Async objects are tagged with
the original TD (transfer descriptor) address and token. We now support
unlimited number of outstanding isoc and one outstanding bulk/intr/ctrl
transfer per QH (queue head). UHCI spec does not have a clear protocol for
the cancelation of the trasfer requests. Driver can yank out TDs on any
frame boundary. In oder to handle that I added somewhat fancy TD validation
logic logic to avoid unnecessary cancelations.
Signed-off-by: Max Krasnyansky <maxk@kernel.org>
---
hw/usb-uhci.c | 897 ++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 533 insertions(+), 364 deletions(-)
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index b55fd84..c6e7751 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -3,6 +3,10 @@
*
* Copyright (c) 2005 Fabrice Bellard
*
+ * Copyright (c) 2008 Max Krasnyansky
+ * Magor rewrite of the UHCI data structures parser and frame processor
+ * Support for fully async operation and multiple outstanding transactions
+ *
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
@@ -27,8 +31,7 @@
#include "qemu-timer.h"
//#define DEBUG
-//#define DEBUG_PACKET
-//#define DEBUG_ISOCH
+//#define DEBUG_DUMP_DATA
#define UHCI_CMD_FGR (1 << 4)
#define UHCI_CMD_EGSM (1 << 3)
@@ -66,6 +69,52 @@
#define NB_PORTS 2
+#ifdef DEBUG
+#define dprintf printf
+
+const char *pid2str(int pid)
+{
+ switch (pid) {
+ case USB_TOKEN_SETUP: return "SETUP";
+ case USB_TOKEN_IN: return "IN";
+ case USB_TOKEN_OUT: return "OUT";
+ }
+ return "?";
+}
+
+#else
+#define dprintf(...)
+#endif
+
+#ifdef DEBUG_DUMP_DATA
+static void dump_data(const uint8_t *data, int len)
+{
+ int i;
+
+ printf("uhci: data: ");
+ for(i = 0; i < len; i++)
+ printf(" %02x", data[i]);
+ printf("\n");
+}
+#else
+static void dump_data(const uint8_t *data, int len) {}
+#endif
+
+/*
+ * Pending async transaction.
+ * 'packet' must be the first field because completion
+ * handler does "(UHCIAsync *) pkt" cast.
+ */
+typedef struct UHCIAsync {
+ USBPacket packet;
+ struct UHCIAsync *next;
+ uint32_t td;
+ uint32_t token;
+ int8_t valid;
+ uint8_t done;
+ uint8_t buffer[2048];
+} UHCIAsync;
+
typedef struct UHCIPort {
USBPort port;
uint16_t ctrl;
@@ -85,16 +134,10 @@ typedef struct UHCIState {
/* Interrupts that should be raised at the end of the current frame. */
uint32_t pending_int_mask;
- /* For simplicity of implementation we only allow a single pending USB
- request. This means all usb traffic on this controller is effectively
- suspended until that transfer completes. When the transfer completes
- the next transfer from that queue will be processed. However
- other queues will not be processed until the next frame. The solution
- is to allow multiple pending requests. */
- uint32_t async_qh;
- uint32_t async_frame_addr;
- USBPacket usb_packet;
- uint8_t usb_buf[2048];
+
+ /* Active packets */
+ UHCIAsync *async_pending;
+ UHCIAsync *async_pool;
} UHCIState;
typedef struct UHCI_TD {
@@ -109,6 +152,140 @@ typedef struct UHCI_QH {
uint32_t el_link;
} UHCI_QH;
+static UHCIAsync *uhci_async_alloc(UHCIState *s)
+{
+ UHCIAsync *async = qemu_malloc(sizeof(UHCIAsync));
+ if (async) {
+ memset(&async->packet, 0, sizeof(async->packet));
+ async->valid = 0;
+ async->td = 0;
+ async->token = 0;
+ async->done = 0;
+ async->next = NULL;
+ }
+
+ return async;
+}
+
+static void uhci_async_free(UHCIState *s, UHCIAsync *async)
+{
+ qemu_free(async);
+}
+
+static void uhci_async_link(UHCIState *s, UHCIAsync *async)
+{
+ async->next = s->async_pending;
+ s->async_pending = async;
+}
+
+static void uhci_async_unlink(UHCIState *s, UHCIAsync *async)
+{
+ UHCIAsync *curr = s->async_pending;
+ UHCIAsync **prev = &s->async_pending;
+
+ while (curr) {
+ if (curr == async) {
+ *prev = curr->next;
+ return;
+ }
+
+ prev = &curr->next;
+ curr = curr->next;
+ }
+}
+
+static void uhci_async_cancel(UHCIState *s, UHCIAsync *async)
+{
+ dprintf("uhci: cancel td 0x%x token 0x%x done %u\n",
+ async->td, async->token, async->done);
+
+ if (!async->done)
+ usb_cancel_packet(&async->packet);
+ uhci_async_free(s, async);
+}
+
+/*
+ * Mark all outstanding async packets as invalid.
+ * This is used for canceling them when TDs are removed by the HCD.
+ */
+static UHCIAsync *uhci_async_validate_begin(UHCIState *s)
+{
+ UHCIAsync *async = s->async_pending;
+
+ while (async) {
+ async->valid--;
+ async = async->next;
+ }
+ return NULL;
+}
+
+/*
+ * Cancel async packets that are no longer valid
+ */
+static void uhci_async_validate_end(UHCIState *s)
+{
+ UHCIAsync *curr = s->async_pending;
+ UHCIAsync **prev = &s->async_pending;
+ UHCIAsync *next;
+
+ while (curr) {
+ if (curr->valid > 0) {
+ prev = &curr->next;
+ curr = curr->next;
+ continue;
+ }
+
+ next = curr->next;
+
+ /* Unlink */
+ *prev = next;
+
+ uhci_async_cancel(s, curr);
+
+ curr = next;
+ }
+}
+
+static void uhci_async_cancel_all(UHCIState *s)
+{
+ UHCIAsync *curr = s->async_pending;
+ UHCIAsync *next;
+
+ while (curr) {
+ next = curr->next;
+
+ uhci_async_cancel(s, curr);
+
+ curr = next;
+ }
+
+ s->async_pending = NULL;
+}
+
+static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t addr, uint32_t token)
+{
+ UHCIAsync *async = s->async_pending;
+
+ while (async) {
+ if (async->td == addr) {
+ if (async->token == token)
+ return async;
+
+ /*
+ * TD was reused for a different transfer.
+ * Invalidate the original one asap.
+ */
+ if (async->valid > 0) {
+ async->valid = 0;
+ dprintf("husb: bad reuse. td 0x%x\n", async->td);
+ }
+ }
+
+ async = async->next;
+ }
+ return NULL;
+}
+
static void uhci_attach(USBPort *port1, USBDevice *dev);
static void uhci_update_irq(UHCIState *s)
@@ -143,15 +320,18 @@ static void uhci_reset(UHCIState *s)
s->intr = 0;
s->fl_base_addr = 0;
s->sof_timing = 64;
+
for(i = 0; i < NB_PORTS; i++) {
port = &s->ports[i];
port->ctrl = 0x0080;
if (port->port.dev)
uhci_attach(&port->port, port->port.dev);
}
+
+ uhci_async_cancel_all(s);
}
-#if 0
+#if 1
static void uhci_save(QEMUFile *f, void *opaque)
{
UHCIState *s = opaque;
@@ -239,9 +419,8 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
UHCIState *s = opaque;
addr &= 0x1f;
-#ifdef DEBUG
- printf("uhci writew port=0x%04x val=0x%04x\n", addr, val);
-#endif
+ dprintf("uhci: writew port=0x%04x val=0x%04x\n", addr, val);
+
switch(addr) {
case 0x00:
if ((val & UHCI_CMD_RS) && !(s->cmd & UHCI_CMD_RS)) {
@@ -350,9 +529,9 @@ static uint32_t uhci_ioport_readw(void *opaque, uint32_t addr)
val = 0xff7f; /* disabled port */
break;
}
-#ifdef DEBUG
- printf("uhci readw port=0x%04x val=0x%04x\n", addr, val);
-#endif
+
+ dprintf("uhci: readw port=0x%04x val=0x%04x\n", addr, val);
+
return val;
}
@@ -361,9 +540,8 @@ static void uhci_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
UHCIState *s = opaque;
addr &= 0x1f;
-#ifdef DEBUG
- printf("uhci writel port=0x%04x val=0x%08x\n", addr, val);
-#endif
+ dprintf("uhci: writel port=0x%04x val=0x%08x\n", addr, val);
+
switch(addr) {
case 0x08:
s->fl_base_addr = val & ~0xfff;
@@ -451,417 +629,406 @@ static void uhci_attach(USBPort *port1, USBDevice *dev)
static int uhci_broadcast_packet(UHCIState *s, USBPacket *p)
{
- UHCIPort *port;
- USBDevice *dev;
int i, ret;
-#ifdef DEBUG_PACKET
- {
- const char *pidstr;
- switch(p->pid) {
- case USB_TOKEN_SETUP: pidstr = "SETUP"; break;
- case USB_TOKEN_IN: pidstr = "IN"; break;
- case USB_TOKEN_OUT: pidstr = "OUT"; break;
- default: pidstr = "?"; break;
- }
- printf("frame %d: pid=%s addr=0x%02x ep=%d len=%d\n",
- s->frnum, pidstr, p->devaddr, p->devep, p->len);
- if (p->pid != USB_TOKEN_IN) {
- printf(" data_out=");
- for(i = 0; i < p->len; i++) {
- printf(" %02x", p->data[i]);
- }
- printf("\n");
- }
- }
-#endif
- for(i = 0; i < NB_PORTS; i++) {
- port = &s->ports[i];
- dev = port->port.dev;
- if (dev && (port->ctrl & UHCI_PORT_EN)) {
+ dprintf("uhci: packet enter. pid %s addr 0x%02x ep %d len %d\n",
+ pid2str(p->pid), p->devaddr, p->devep, p->len);
+ if (p->pid == USB_TOKEN_OUT)
+ dump_data(p->data, p->len);
+
+ ret = USB_RET_NODEV;
+ for (i = 0; i < NB_PORTS && ret == USB_RET_NODEV; i++) {
+ UHCIPort *port = &s->ports[i];
+ USBDevice *dev = port->port.dev;
+
+ if (dev && (port->ctrl & UHCI_PORT_EN))
ret = dev->handle_packet(dev, p);
- if (ret != USB_RET_NODEV) {
-#ifdef DEBUG_PACKET
- if (ret == USB_RET_ASYNC) {
- printf("usb-uhci: Async packet\n");
- } else {
- printf(" ret=%d ", ret);
- if (p->pid == USB_TOKEN_IN && ret > 0) {
- printf("data_in=");
- for(i = 0; i < ret; i++) {
- printf(" %02x", p->data[i]);
- }
- }
- printf("\n");
- }
-#endif
- return ret;
- }
- }
}
- return USB_RET_NODEV;
+
+ dprintf("uhci: packet exit. ret %d len %d\n", ret, p->len);
+ if (p->pid == USB_TOKEN_IN && ret > 0)
+ dump_data(p->data, ret);
+
+ return ret;
}
-static void uhci_async_complete_packet(USBPacket * packet, void *opaque);
+static void uhci_async_complete(USBPacket * packet, void *opaque);
+static void uhci_process_frame(UHCIState *s);
/* return -1 if fatal error (frame must be stopped)
0 if TD successful
1 if TD unsuccessful or inactive
*/
-static int uhci_handle_td(UHCIState *s, UHCI_TD *td, uint32_t *int_mask,
- int completion)
+static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_t *int_mask)
{
+ int len = 0, max_len, err, ret;
uint8_t pid;
- int len = 0, max_len, err, ret = 0;
- /* ??? This is wrong for async completion. */
- if (td->ctrl & TD_CTRL_IOC) {
+ max_len = ((td->token >> 21) + 1) & 0x7ff;
+ pid = td->token & 0xff;
+
+ ret = async->packet.len;
+
+ if (td->ctrl & TD_CTRL_IOC)
*int_mask |= 0x01;
- }
- if (!(td->ctrl & TD_CTRL_ACTIVE))
- return 1;
+ if (td->ctrl & TD_CTRL_IOS)
+ td->ctrl &= ~TD_CTRL_ACTIVE;
- /* TD is active */
- max_len = ((td->token >> 21) + 1) & 0x7ff;
- pid = td->token & 0xff;
+ if (ret < 0)
+ goto out;
- if (completion && (s->async_qh || s->async_frame_addr)) {
- ret = s->usb_packet.len;
- if (ret >= 0) {
- len = ret;
- if (len > max_len) {
- len = max_len;
- ret = USB_RET_BABBLE;
- }
- if (len > 0) {
- /* write the data back */
- cpu_physical_memory_write(td->buffer, s->usb_buf, len);
- }
- } else {
- len = 0;
- }
- s->async_qh = 0;
- s->async_frame_addr = 0;
- } else if (!completion) {
- s->usb_packet.pid = pid;
- s->usb_packet.devaddr = (td->token >> 8) & 0x7f;
- s->usb_packet.devep = (td->token >> 15) & 0xf;
- s->usb_packet.data = s->usb_buf;
- s->usb_packet.len = max_len;
- s->usb_packet.complete_cb = uhci_async_complete_packet;
- s->usb_packet.complete_opaque = s;
- switch(pid) {
- case USB_TOKEN_OUT:
- case USB_TOKEN_SETUP:
- cpu_physical_memory_read(td->buffer, s->usb_buf, max_len);
- ret = uhci_broadcast_packet(s, &s->usb_packet);
+ len = async->packet.len;
+ td->ctrl = (td->ctrl & ~0x7ff) | ((len - 1) & 0x7ff);
+
+ /* The NAK bit may have been set by a previous frame, so clear it
+ here. The docs are somewhat unclear, but win2k relies on this
+ behavior. */
+ td->ctrl &= ~(TD_CTRL_ACTIVE | TD_CTRL_NAK);
+
+ if (pid == USB_TOKEN_IN) {
+ if (len > max_len) {
len = max_len;
- break;
- case USB_TOKEN_IN:
- ret = uhci_broadcast_packet(s, &s->usb_packet);
- if (ret >= 0) {
- len = ret;
- if (len > max_len) {
- len = max_len;
- ret = USB_RET_BABBLE;
- }
- if (len > 0) {
- /* write the data back */
- cpu_physical_memory_write(td->buffer, s->usb_buf, len);
- }
- } else {
- len = 0;
- }
- break;
- default:
- /* invalid pid : frame interrupted */
- s->status |= UHCI_STS_HCPERR;
- uhci_update_irq(s);
- return -1;
+ ret = USB_RET_BABBLE;
+ goto out;
}
- }
- if (ret == USB_RET_ASYNC) {
- return 2;
- }
- if (td->ctrl & TD_CTRL_IOS)
- td->ctrl &= ~TD_CTRL_ACTIVE;
- if (ret >= 0) {
- td->ctrl = (td->ctrl & ~0x7ff) | ((len - 1) & 0x7ff);
- /* The NAK bit may have been set by a previous frame, so clear it
- here. The docs are somewhat unclear, but win2k relies on this
- behavior. */
- td->ctrl &= ~(TD_CTRL_ACTIVE | TD_CTRL_NAK);
- if (pid == USB_TOKEN_IN &&
- (td->ctrl & TD_CTRL_SPD) &&
- len < max_len) {
+ if (len > 0) {
+ /* write the data back */
+ cpu_physical_memory_write(td->buffer, async->buffer, len);
+ }
+
+ if ((td->ctrl & TD_CTRL_SPD) && len < max_len) {
*int_mask |= 0x02;
/* short packet: do not update QH */
+ dprintf("uhci: short packet. td 0x%x token 0x%x\n", async->td, async->token);
return 1;
- } else {
- /* success */
- return 0;
}
- } else {
- switch(ret) {
- default:
- case USB_RET_NODEV:
- do_timeout:
- td->ctrl |= TD_CTRL_TIMEOUT;
- err = (td->ctrl >> TD_CTRL_ERROR_SHIFT) & 3;
- if (err != 0) {
- err--;
- if (err == 0) {
- td->ctrl &= ~TD_CTRL_ACTIVE;
- s->status |= UHCI_STS_USBERR;
- uhci_update_irq(s);
- }
- }
- td->ctrl = (td->ctrl & ~(3 << TD_CTRL_ERROR_SHIFT)) |
- (err << TD_CTRL_ERROR_SHIFT);
- return 1;
- case USB_RET_NAK:
- td->ctrl |= TD_CTRL_NAK;
- if (pid == USB_TOKEN_SETUP)
- goto do_timeout;
- return 1;
- case USB_RET_STALL:
- td->ctrl |= TD_CTRL_STALL;
- td->ctrl &= ~TD_CTRL_ACTIVE;
- return 1;
- case USB_RET_BABBLE:
- td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
+ }
+
+ /* success */
+ return 0;
+
+out:
+ switch(ret) {
+ case USB_RET_STALL:
+ td->ctrl |= TD_CTRL_STALL;
+ td->ctrl &= ~TD_CTRL_ACTIVE;
+ return 1;
+
+ case USB_RET_BABBLE:
+ td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
+ td->ctrl &= ~TD_CTRL_ACTIVE;
+ /* frame interrupted */
+ return -1;
+
+ case USB_RET_NAK:
+ td->ctrl |= TD_CTRL_NAK;
+ if (pid == USB_TOKEN_SETUP)
+ break;
+ return 1;
+
+ case USB_RET_NODEV:
+ default:
+ break;
+ }
+
+ /* Retry the TD if error count is not zero */
+
+ td->ctrl |= TD_CTRL_TIMEOUT;
+ err = (td->ctrl >> TD_CTRL_ERROR_SHIFT) & 3;
+ if (err != 0) {
+ err--;
+ if (err == 0) {
td->ctrl &= ~TD_CTRL_ACTIVE;
- /* frame interrupted */
- return -1;
+ s->status |= UHCI_STS_USBERR;
+ uhci_update_irq(s);
}
}
+ td->ctrl = (td->ctrl & ~(3 << TD_CTRL_ERROR_SHIFT)) |
+ (err << TD_CTRL_ERROR_SHIFT);
+ return 1;
}
-static void uhci_async_complete_packet(USBPacket * packet, void *opaque)
+static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, uint32_t *int_mask)
+{
+ UHCIAsync *async;
+ int len = 0, max_len, ret = 0;
+ uint8_t pid;
+
+ /* Is active ? */
+ if (!(td->ctrl & TD_CTRL_ACTIVE))
+ return 1;
+
+ async = uhci_async_find_td(s, addr, td->token);
+ if (async) {
+ /* Already submitted */
+ async->valid = 10;
+
+ if (!async->done)
+ return 1;
+
+ uhci_async_unlink(s, async);
+ goto done;
+ }
+
+ /* Allocate new packet */
+ async = uhci_async_alloc(s);
+ if (!async)
+ return 1;
+
+ async->valid = 10;
+ async->td = addr;
+ async->token = td->token;
+
+ max_len = ((td->token >> 21) + 1) & 0x7ff;
+ pid = td->token & 0xff;
+
+ async->packet.pid = pid;
+ async->packet.devaddr = (td->token >> 8) & 0x7f;
+ async->packet.devep = (td->token >> 15) & 0xf;
+ async->packet.data = async->buffer;
+ async->packet.len = max_len;
+ async->packet.complete_cb = uhci_async_complete;
+ async->packet.complete_opaque = s;
+
+ switch(pid) {
+ case USB_TOKEN_OUT:
+ case USB_TOKEN_SETUP:
+ cpu_physical_memory_read(td->buffer, async->buffer, max_len);
+ ret = uhci_broadcast_packet(s, &async->packet);
+ len = max_len;
+ break;
+
+ case USB_TOKEN_IN:
+ ret = uhci_broadcast_packet(s, &async->packet);
+ break;
+
+ default:
+ /* invalid pid : frame interrupted */
+ uhci_async_free(s, async);
+ s->status |= UHCI_STS_HCPERR;
+ uhci_update_irq(s);
+ return -1;
+ }
+
+ if (ret == USB_RET_ASYNC) {
+ uhci_async_link(s, async);
+ return 2;
+ }
+
+ async->packet.len = ret;
+
+done:
+ ret = uhci_complete_td(s, td, async, int_mask);
+ uhci_async_free(s, async);
+ return ret;
+}
+
+static void uhci_async_complete(USBPacket *packet, void *opaque)
{
UHCIState *s = opaque;
- UHCI_QH qh;
+ UHCIAsync *async = (UHCIAsync *) packet;
+
+ dprintf("uhci: async complete. td 0x%x token 0x%x\n", async->td, async->token);
+
+ async->done = 1;
+
+ uhci_process_frame(s);
+}
+
+static int is_valid(uint32_t link)
+{
+ return (link & 1) == 0;
+}
+
+static int is_qh(uint32_t link)
+{
+ return (link & 2) != 0;
+}
+
+static int depth_first(uint32_t link)
+{
+ return (link & 4) != 0;
+}
+
+/* QH DB used for detecting QH loops */
+#define UHCI_MAX_QUEUES 128
+typedef struct {
+ uint32_t addr[UHCI_MAX_QUEUES];
+ int count;
+} QhDb;
+
+static void qhdb_reset(QhDb *db)
+{
+ db->count = 0;
+}
+
+/* Add QH to DB. Returns 1 if already present or DB is full. */
+static int qhdb_insert(QhDb *db, uint32_t addr)
+{
+ int i;
+ for (i = 0; i < db->count; i++)
+ if (db->addr[i] == addr)
+ return 1;
+
+ if (db->count >= UHCI_MAX_QUEUES)
+ return 1;
+
+ db->addr[db->count++] = addr;
+ return 0;
+}
+
+static void uhci_process_frame(UHCIState *s)
+{
+ uint32_t frame_addr, link, old_td_ctrl, val, int_mask;
+ uint32_t curr_qh;
+ int cnt, ret;
UHCI_TD td;
- uint32_t link;
- uint32_t old_td_ctrl;
- uint32_t val;
- uint32_t frame_addr;
- int ret;
+ UHCI_QH qh;
+ QhDb qhdb;
- /* Handle async isochronous packet completion */
- frame_addr = s->async_frame_addr;
- if (frame_addr) {
- cpu_physical_memory_read(frame_addr, (uint8_t *)&link, 4);
- le32_to_cpus(&link);
+ frame_addr = s->fl_base_addr + ((s->frnum & 0x3ff) << 2);
+
+ dprintf("uhci: processing frame %d addr 0x%x\n" , s->frnum, frame_addr);
+
+ cpu_physical_memory_read(frame_addr, (uint8_t *)&link, 4);
+ le32_to_cpus(&link);
- cpu_physical_memory_read(link & ~0xf, (uint8_t *)&td, sizeof(td));
+ int_mask = 0;
+ curr_qh = 0;
+
+ qhdb_reset(&qhdb);
+
+ for (cnt = FRAME_MAX_LOOPS; is_valid(link) && cnt; cnt--) {
+ if (is_qh(link)) {
+ /* QH */
+
+ if (qhdb_insert(&qhdb, link)) {
+ /*
+ * We're going in circles. Which is not a bug because
+ * HCD is allowed to do that as part of the BW management.
+ * In our case though it makes no sense to spin here. Sync transations
+ * are already done, and async completion handler will re-process
+ * the frame when something is ready.
+ */
+ dprintf("uhci: detected loop. qh 0x%x\n", link);
+ break;
+ }
+
+ cpu_physical_memory_read(link & ~0xf, (uint8_t *) &qh, sizeof(qh));
+ le32_to_cpus(&qh.link);
+ le32_to_cpus(&qh.el_link);
+
+ dprintf("uhci: QH 0x%x load. link 0x%x elink 0x%x\n",
+ link, qh.link, qh.el_link);
+
+ if (!is_valid(qh.el_link)) {
+ /* QH w/o elements */
+ curr_qh = 0;
+ link = qh.link;
+ } else {
+ /* QH with elements */
+ curr_qh = link;
+ link = qh.el_link;
+ }
+ continue;
+ }
+
+ /* TD */
+ cpu_physical_memory_read(link & ~0xf, (uint8_t *) &td, sizeof(td));
le32_to_cpus(&td.link);
le32_to_cpus(&td.ctrl);
le32_to_cpus(&td.token);
le32_to_cpus(&td.buffer);
- old_td_ctrl = td.ctrl;
- ret = uhci_handle_td(s, &td, &s->pending_int_mask, 1);
- /* update the status bits of the TD */
+ dprintf("uhci: TD 0x%x load. link 0x%x ctrl 0x%x token 0x%x qh 0x%x\n",
+ link, td.link, td.ctrl, td.token, curr_qh);
+
+ old_td_ctrl = td.ctrl;
+ ret = uhci_handle_td(s, link, &td, &int_mask);
if (old_td_ctrl != td.ctrl) {
+ /* update the status bits of the TD */
val = cpu_to_le32(td.ctrl);
cpu_physical_memory_write((link & ~0xf) + 4,
- (const uint8_t *)&val,
- sizeof(val));
+ (const uint8_t *)&val, sizeof(val));
}
- if (ret == 2) {
- s->async_frame_addr = frame_addr;
- } else if (ret == 0) {
- /* update qh element link */
- val = cpu_to_le32(td.link);
- cpu_physical_memory_write(frame_addr,
- (const uint8_t *)&val,
- sizeof(val));
+
+ if (ret < 0) {
+ /* interrupted frame */
+ break;
}
- return;
- }
- link = s->async_qh;
- if (!link) {
- /* This should never happen. It means a TD somehow got removed
- without cancelling the associated async IO request. */
- return;
- }
- cpu_physical_memory_read(link & ~0xf, (uint8_t *)&qh, sizeof(qh));
- le32_to_cpus(&qh.link);
- le32_to_cpus(&qh.el_link);
- /* Re-process the queue containing the async packet. */
- while (1) {
- cpu_physical_memory_read(qh.el_link & ~0xf,
- (uint8_t *)&td, sizeof(td));
- le32_to_cpus(&td.link);
- le32_to_cpus(&td.ctrl);
- le32_to_cpus(&td.token);
- le32_to_cpus(&td.buffer);
- old_td_ctrl = td.ctrl;
- ret = uhci_handle_td(s, &td, &s->pending_int_mask, 1);
+ if (ret == 2 || ret == 1) {
+ dprintf("uhci: TD 0x%x %s. link 0x%x ctrl 0x%x token 0x%x qh 0x%x\n",
+ link, ret == 2 ? "pend" : "skip",
+ td.link, td.ctrl, td.token, curr_qh);
- /* update the status bits of the TD */
- if (old_td_ctrl != td.ctrl) {
- val = cpu_to_le32(td.ctrl);
- cpu_physical_memory_write((qh.el_link & ~0xf) + 4,
- (const uint8_t *)&val,
- sizeof(val));
+ link = curr_qh ? qh.link : td.link;
+ continue;
}
- if (ret < 0)
- break; /* interrupted frame */
- if (ret == 2) {
- s->async_qh = link;
- break;
- } else if (ret == 0) {
- /* update qh element link */
- qh.el_link = td.link;
+
+ /* completed TD */
+
+ dprintf("uhci: TD 0x%x done. link 0x%x ctrl 0x%x token 0x%x qh 0x%x\n",
+ link, td.link, td.ctrl, td.token, curr_qh);
+
+ link = td.link;
+
+ if (curr_qh) {
+ /* update QH element link */
+ qh.el_link = link;
val = cpu_to_le32(qh.el_link);
- cpu_physical_memory_write((link & ~0xf) + 4,
- (const uint8_t *)&val,
- sizeof(val));
- if (!(qh.el_link & 4))
- break;
+ cpu_physical_memory_write((curr_qh & ~0xf) + 4,
+ (const uint8_t *)&val, sizeof(val));
+
+ if (!depth_first(link)) {
+ /* done with this QH */
+
+ dprintf("uhci: QH 0x%x done. link 0x%x elink 0x%x\n",
+ curr_qh, qh.link, qh.el_link);
+
+ curr_qh = 0;
+ link = qh.link;
+ }
}
- break;
+
+ /* go to the next entry */
}
+
+ s->pending_int_mask = int_mask;
}
static void uhci_frame_timer(void *opaque)
{
UHCIState *s = opaque;
int64_t expire_time;
- uint32_t frame_addr, link, old_td_ctrl, val, int_mask;
- int cnt, ret;
- UHCI_TD td;
- UHCI_QH qh;
- uint32_t old_async_qh;
if (!(s->cmd & UHCI_CMD_RS)) {
+ /* Full stop */
qemu_del_timer(s->frame_timer);
/* set hchalted bit in status - UHCI11D 2.1.2 */
s->status |= UHCI_STS_HCHALTED;
return;
}
- /* Complete the previous frame. */
- s->frnum = (s->frnum + 1) & 0x7ff;
+
+ /* Complete the previous frame */
if (s->pending_int_mask) {
s->status2 |= s->pending_int_mask;
- s->status |= UHCI_STS_USBINT;
+ s->status |= UHCI_STS_USBINT;
uhci_update_irq(s);
}
- old_async_qh = s->async_qh;
- frame_addr = s->fl_base_addr + ((s->frnum & 0x3ff) << 2);
- cpu_physical_memory_read(frame_addr, (uint8_t *)&link, 4);
- le32_to_cpus(&link);
- int_mask = 0;
- cnt = FRAME_MAX_LOOPS;
- while ((link & 1) == 0) {
- if (--cnt == 0)
- break;
- /* valid frame */
- if (link & 2) {
- /* QH */
- if (link == s->async_qh) {
- /* We've found a previously issues packet.
- Nothing else to do. */
- old_async_qh = 0;
- break;
- }
- cpu_physical_memory_read(link & ~0xf, (uint8_t *)&qh, sizeof(qh));
- le32_to_cpus(&qh.link);
- le32_to_cpus(&qh.el_link);
- depth_first:
- if (qh.el_link & 1) {
- /* no element : go to next entry */
- link = qh.link;
- } else if (qh.el_link & 2) {
- /* QH */
- link = qh.el_link;
- } else if (s->async_qh) {
- /* We can only cope with one pending packet. Keep looking
- for the previously issued packet. */
- link = qh.link;
- } else {
- /* TD */
- if (--cnt == 0)
- break;
- cpu_physical_memory_read(qh.el_link & ~0xf,
- (uint8_t *)&td, sizeof(td));
- le32_to_cpus(&td.link);
- le32_to_cpus(&td.ctrl);
- le32_to_cpus(&td.token);
- le32_to_cpus(&td.buffer);
- old_td_ctrl = td.ctrl;
- ret = uhci_handle_td(s, &td, &int_mask, 0);
-
- /* update the status bits of the TD */
- if (old_td_ctrl != td.ctrl) {
- val = cpu_to_le32(td.ctrl);
- cpu_physical_memory_write((qh.el_link & ~0xf) + 4,
- (const uint8_t *)&val,
- sizeof(val));
- }
- if (ret < 0)
- break; /* interrupted frame */
- if (ret == 2) {
- s->async_qh = link;
- } else if (ret == 0) {
- /* update qh element link */
- qh.el_link = td.link;
- val = cpu_to_le32(qh.el_link);
- cpu_physical_memory_write((link & ~0xf) + 4,
- (const uint8_t *)&val,
- sizeof(val));
- if (qh.el_link & 4) {
- /* depth first */
- goto depth_first;
- }
- }
- /* go to next entry */
- link = qh.link;
- }
- } else {
- /* TD */
- cpu_physical_memory_read(link & ~0xf, (uint8_t *)&td, sizeof(td));
- le32_to_cpus(&td.link);
- le32_to_cpus(&td.ctrl);
- le32_to_cpus(&td.token);
- le32_to_cpus(&td.buffer);
-
- /* Handle isochonous transfer. */
- /* FIXME: might be more than one isoc in frame */
- old_td_ctrl = td.ctrl;
- ret = uhci_handle_td(s, &td, &int_mask, 0);
- /* update the status bits of the TD */
- if (old_td_ctrl != td.ctrl) {
- val = cpu_to_le32(td.ctrl);
- cpu_physical_memory_write((link & ~0xf) + 4,
- (const uint8_t *)&val,
- sizeof(val));
- }
- if (ret < 0)
- break; /* interrupted frame */
- if (ret == 2) {
- s->async_frame_addr = frame_addr;
- }
- link = td.link;
- }
- }
- s->pending_int_mask = int_mask;
- if (old_async_qh) {
- /* A previously started transfer has disappeared from the transfer
- list. There's nothing useful we can do with it now, so just
- discard the packet and hope it wasn't too important. */
-#ifdef DEBUG
- printf("Discarding USB packet\n");
-#endif
- usb_cancel_packet(&s->usb_packet);
- s->async_qh = 0;
- }
+ /* Start new frame */
+ s->frnum = (s->frnum + 1) & 0x7ff;
+
+ dprintf("uhci: new frame #%u\n" , s->frnum);
+
+ uhci_async_validate_begin(s);
+
+ uhci_process_frame(s);
+
+ uhci_async_validate_end(s);
/* prepare the timer for the next frame */
expire_time = qemu_get_clock(vm_clock) +
@@ -950,4 +1117,6 @@ void usb_uhci_piix4_init(PCIBus *bus, int devfn)
to rely on this. */
pci_register_io_region(&s->dev, 4, 0x20,
PCI_ADDRESS_SPACE_IO, uhci_map);
+
+ register_savevm("uhci", 0, 1, uhci_save, uhci_load, s);
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions
2008-08-14 4:22 ` [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions Max Krasnyansky
@ 2008-08-14 17:51 ` Anthony Liguori
2008-08-14 19:49 ` Max Krasnyansky
2008-10-11 23:54 ` [Qemu-devel] " Juergen Lock
1 sibling, 1 reply; 37+ messages in thread
From: Anthony Liguori @ 2008-08-14 17:51 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: qemu-devel, kvm
Max Krasnyansky wrote:
>
> +static UHCIAsync *uhci_async_alloc(UHCIState *s)
> +{
> + UHCIAsync *async = qemu_malloc(sizeof(UHCIAsync));
> + if (async) {
> + memset(&async->packet, 0, sizeof(async->packet));
>
You can use qemu_mallocz() here.
> static void uhci_attach(USBPort *port1, USBDevice *dev);
>
> static void uhci_update_irq(UHCIState *s)
> @@ -143,15 +320,18 @@ static void uhci_reset(UHCIState *s)
> s->intr = 0;
> s->fl_base_addr = 0;
> s->sof_timing = 64;
> +
> for(i = 0; i < NB_PORTS; i++) {
> port = &s->ports[i];
> port->ctrl = 0x0080;
> if (port->port.dev)
> uhci_attach(&port->port, port->port.dev);
> }
> +
> + uhci_async_cancel_all(s);
> }
>
> -#if 0
> +#if 1
>
Just drop the #if
> static void uhci_save(QEMUFile *f, void *opaque)
> {
> UHCIState *s = opaque;
> @@ -239,9 +419,8 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
> UHCIState *s = opaque;
>
> addr &= 0x1f;
> -#ifdef DEBUG
> - printf("uhci writew port=0x%04x val=0x%04x\n", addr, val);
> -#endif
> + dprintf("uhci: writew port=0x%04x val=0x%04x\n", addr, val);
> +
> switch(addr) {
> case 0x00:
> if ((val & UHCI_CMD_RS) && !(s->cmd & UHCI_CMD_RS)) {
> @@ -350,9 +529,9 @@ static uint32_t uhci_ioport_readw(void *opaque, uint32_t addr)
> val = 0xff7f; /* disabled port */
> break;
> }
> -#ifdef DEBUG
> - printf("uhci readw port=0x%04x val=0x%04x\n", addr, val);
> -#endif
> +
> + dprintf("uhci: readw port=0x%04x val=0x%04x\n", addr, val);
> +
> return val;
> }
How do you handle the outstanding asynchronous requests in save/restore?
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions
2008-08-14 17:51 ` [Qemu-devel] " Anthony Liguori
@ 2008-08-14 19:49 ` Max Krasnyansky
0 siblings, 0 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 19:49 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, kvm
Anthony Liguori wrote:
> Max Krasnyansky wrote:
>>
>> +static UHCIAsync *uhci_async_alloc(UHCIState *s)
>> +{
>> + UHCIAsync *async = qemu_malloc(sizeof(UHCIAsync));
>> + if (async) {
>> + memset(&async->packet, 0, sizeof(async->packet));
>>
>
> You can use qemu_mallocz() here.
The reasons I decided not too is because there is 2K buffer in each UHCIAsync
struct. memset()ing 2K buffer for each transaction did not seem like a good
idea. ie qemu_mallocz() simply does memset(0)
>
>> static void uhci_attach(USBPort *port1, USBDevice *dev);
>>
>> static void uhci_update_irq(UHCIState *s)
>> @@ -143,15 +320,18 @@ static void uhci_reset(UHCIState *s)
>> s->intr = 0;
>> s->fl_base_addr = 0;
>> s->sof_timing = 64;
>> +
>> for(i = 0; i < NB_PORTS; i++) {
>> port = &s->ports[i];
>> port->ctrl = 0x0080;
>> if (port->port.dev)
>> uhci_attach(&port->port, port->port.dev);
>> }
>> +
>> + uhci_async_cancel_all(s);
>> }
>>
>> -#if 0
>> +#if 1
>>
>
> Just drop the #if
This is probably mismerge. Let me check.
>> static void uhci_save(QEMUFile *f, void *opaque)
>> {
>> UHCIState *s = opaque;
>> @@ -239,9 +419,8 @@ static void uhci_ioport_writew(void *opaque,
>> uint32_t addr, uint32_t val)
>> UHCIState *s = opaque;
>>
>> addr &= 0x1f;
>> -#ifdef DEBUG
>> - printf("uhci writew port=0x%04x val=0x%04x\n", addr, val);
>> -#endif
>> + dprintf("uhci: writew port=0x%04x val=0x%04x\n", addr, val);
>> +
>> switch(addr) {
>> case 0x00:
>> if ((val & UHCI_CMD_RS) && !(s->cmd & UHCI_CMD_RS)) {
>> @@ -350,9 +529,9 @@ static uint32_t uhci_ioport_readw(void *opaque,
>> uint32_t addr)
>> val = 0xff7f; /* disabled port */
>> break;
>> }
>> -#ifdef DEBUG
>> - printf("uhci readw port=0x%04x val=0x%04x\n", addr, val);
>> -#endif
>> +
>> + dprintf("uhci: readw port=0x%04x val=0x%04x\n", addr, val);
>> +
>> return val;
>> }
>
> How do you handle the outstanding asynchronous requests in save/restore?
I do not :).
I haven't really played with save/restore. I did not mean to intentionally
enable save/restore stuff, this probably came from KVM tree. Now that I'm
thinking about I did save/restore the VM once with KVM and USB worked fine.
That was before async patches. With async we can probably just cancel all
outstanding transactions. I'll try that out a bit later.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions
2008-08-14 4:22 ` [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions Max Krasnyansky
2008-08-14 17:51 ` [Qemu-devel] " Anthony Liguori
@ 2008-10-11 23:54 ` Juergen Lock
2008-10-15 19:54 ` Max Krasnyansky
1 sibling, 1 reply; 37+ messages in thread
From: Juergen Lock @ 2008-10-11 23:54 UTC (permalink / raw)
To: maxk; +Cc: qemu-devel
In article <1a8fc205ecb1a6dea2d6a2cc4019f359c90f39fb.1218685608.git.maxk@kernel.org> you write:
>This is esentially a re-write of the QEMU UHCI layer. My initial goal
>was to support fully async operation with multiple outstanding async
>transactions. Along the way I realized that I can greatly simplify
>and cleanup the overall logic. There was a lot of duplicate and confusing
>code in the UHCI data structure parsing and other places.
>We were actually violating UHCI spec in handling async ISOC transaction
>(host controller is not supposed to write into the frame pointer).
>
>The reason I wanted to support fully async operation is because current
>synchronous version is unusable with most devices exported from host
>(via usb-linux.c). Transactions take a long time and the whole VM becomes
>slow as hell.
>
>Current async support is very rudimentory and for the most part
>non-functional. Single transaction at a time is simply not enough. I have
>a device for which XP driver submits both IN and OUT packets at the same
>time. IN packet always times out unless OUT packet makes it to the device.
>Hence we must be able to process both in order for that device to work.
>
>The new code is backwards compatible and was first tested agains original
>synchronous usb-linux.c and builtin usb devices like tablet which is also
>synchronous. Rewrite of the usb-linux.c is coming up next.
>
>Async support was tested against various XP versions (ie XP, SP2, SP3) and
>a bunch of different USB devices: serial port controllers, mice, keyboard,
>JTAG dongles (from Xilinx and Altera).
>
>ISOC support was only lighly tested and needs more work. It's not any worse
>than current code though.
>
>UHCI parser changes are probably somewhat hard to review without the
>understanding of the UHCI spec.
>The async design should be fairly easy to follow. Basically we have a list
>of async objects for each pending transfer. Async objects are tagged with
>the original TD (transfer descriptor) address and token. We now support
>unlimited number of outstanding isoc and one outstanding bulk/intr/ctrl
>transfer per QH (queue head). UHCI spec does not have a clear protocol for
>the cancelation of the trasfer requests. Driver can yank out TDs on any
>frame boundary. In oder to handle that I added somewhat fancy TD validation
>logic logic to avoid unnecessary cancelations.
>
>Signed-off-by: Max Krasnyansky <maxk@kernel.org>
>---
This is whats got committed as r5050, right? It causes a large slowdown
with some linux guests, as posted here:
http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg01201.html
As mentioned the guest I tested was:
sidux-2008-03-ourea-pre1-kde-lite-i386-200809142136.iso
It uses a Linux 2.6.26 kernel, a FreeBSD guest is not affected by the
slowness, could this be related to dynticks?
Oh and on one box transfers over the emulated usb nic even sometimes
stall, I have to ping the guest's ip for them to continue, and the ping
then loses packets too.
Thanx, :)
Juergen
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions
2008-10-11 23:54 ` [Qemu-devel] " Juergen Lock
@ 2008-10-15 19:54 ` Max Krasnyansky
2008-10-15 22:05 ` andrzej zaborowski
0 siblings, 1 reply; 37+ messages in thread
From: Max Krasnyansky @ 2008-10-15 19:54 UTC (permalink / raw)
To: Juergen Lock; +Cc: qemu-devel
Hi Juergen,
Juergen Lock wrote:
> This is whats got committed as r5050, right? It causes a large slowdown
> with some linux guests, as posted here:
> http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg01201.html
> As mentioned the guest I tested was:
> sidux-2008-03-ourea-pre1-kde-lite-i386-200809142136.iso
> It uses a Linux 2.6.26 kernel, a FreeBSD guest is not affected by the
> slowness, could this be related to dynticks?
>
> Oh and on one box transfers over the emulated usb nic even sometimes
> stall, I have to ping the guest's ip for them to continue, and the ping
> then loses packets too.
Can you please clarify your setup. As I understood, from the thread you
mentioned above, you're running Linux 2.6.26 guest on QEMU under FreeBSD
host,correct ?
And what you're saying is that USB is slow in that configuration.
Assuming that I got that part correctly let me first say that this means
that usb-linux is not activated and you're using built-in QEMU USB
devices. What this in turn means is that your USB operation is
synchronous (all builtin devices are sync). New async logic with
multiple transactions, etc only applies to the real USB devices
virtualized via usb-linux & libusb.
After doing UHCI rewrite I made sure that new code generates exactly the
same UHCI structures as the old code. I may have missed something but in
general guests should not be affected, and as I explained new async
logic does not even kick in with built-in devices.
Is there any chance that there may be something else going on here ?
In other words are you sure that it's UHCI changes that cause the slow down.
If you are sure can you please recompile hw/usb-uhci.c with DEBUG
enabled. Save the trace it generates and send it to me. I can take a
look at it and see if there is something obvious. I do not have FreeBSD
hosts around to try and reproduce this myself.
Thanx
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions
2008-10-15 19:54 ` Max Krasnyansky
@ 2008-10-15 22:05 ` andrzej zaborowski
2008-10-16 21:25 ` Juergen Lock
0 siblings, 1 reply; 37+ messages in thread
From: andrzej zaborowski @ 2008-10-15 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Juergen Lock
2008/10/15 Max Krasnyansky <maxk@qualcomm.com>:
> Hi Juergen,
>
> Juergen Lock wrote:
>>
>> This is whats got committed as r5050, right? It causes a large slowdown
>> with some linux guests, as posted here:
>> http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg01201.html
>> As mentioned the guest I tested was:
>> sidux-2008-03-ourea-pre1-kde-lite-i386-200809142136.iso
>> It uses a Linux 2.6.26 kernel, a FreeBSD guest is not affected by the
>> slowness, could this be related to dynticks?
>>
>> Oh and on one box transfers over the emulated usb nic even sometimes
>> stall, I have to ping the guest's ip for them to continue, and the ping
>> then loses packets too.
>
> Can you please clarify your setup. As I understood, from the thread you
> mentioned above, you're running Linux 2.6.26 guest on QEMU under FreeBSD
> host,correct ?
For the record on Linux host I observed the same issues. Juergen
mentioned that in an earlier mail. The transfer rate is about 60kB/s
reading from an emulated mass storage device using a raw image. It
was before about 1MB/s according to Juergen. I wasn't using dynticks.
Cheers
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions
2008-10-15 22:05 ` andrzej zaborowski
@ 2008-10-16 21:25 ` Juergen Lock
0 siblings, 0 replies; 37+ messages in thread
From: Juergen Lock @ 2008-10-16 21:25 UTC (permalink / raw)
To: andrzej zaborowski; +Cc: qemu-devel, Max Krasnyansky
On Thu, Oct 16, 2008 at 12:05:43AM +0200, andrzej zaborowski wrote:
> 2008/10/15 Max Krasnyansky <maxk@qualcomm.com>:
> > Hi Juergen,
> >
> > Juergen Lock wrote:
> >>
> >> This is whats got committed as r5050, right? It causes a large slowdown
> >> with some linux guests, as posted here:
> >> http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg01201.html
> >> As mentioned the guest I tested was:
> >> sidux-2008-03-ourea-pre1-kde-lite-i386-200809142136.iso
> >> It uses a Linux 2.6.26 kernel, a FreeBSD guest is not affected by the
> >> slowness, could this be related to dynticks?
> >>
> >> Oh and on one box transfers over the emulated usb nic even sometimes
> >> stall, I have to ping the guest's ip for them to continue, and the ping
> >> then loses packets too.
> >
> > Can you please clarify your setup. As I understood, from the thread you
> > mentioned above, you're running Linux 2.6.26 guest on QEMU under FreeBSD
> > host,correct ?
Correct.
>
> For the record on Linux host I observed the same issues. Juergen
> mentioned that in an earlier mail. The transfer rate is about 60kB/s
> reading from an emulated mass storage device using a raw image. It
> was before about 1MB/s according to Juergen. I wasn't using dynticks.
>
Also no dynticks in your guest? The sidux iso we both tried does seem to
use dynticks at least...
Anyway I have enabled debug output like Max suggested, booted a sidux guest
to runlevel 3 (which took quite a while...) and when it finally was up ran:
$ wget 10.0.2.2:631/help/ref-cupsd-conf.html
(gave <40 kB/s)
$ su
# rmmod uhci-hcd
# modprobe uhci-hcd
# dhclient eth0
# ^D
$ wget 10.0.2.2:631/help/ref-cupsd-conf.html
(again gave <40 kB/s)
(the wget assumes cups running on the host, the file it gets is 85264
bytes here.)
The resulting log is 1.6G, tho running it thru bzip2 twice shrinks it
down to 10M, I put them here:
http://people.freebsd.org/~nox/qemu/usblog.bz2 (16M)
http://people.freebsd.org/~nox/qemu/usblog.bz2.bz2 (10M)
Oh and about whether I'm sure its the uhci changes that cause the slowdown,
all I can say is that I played with several svn revisions and r5049 and
earlier were way faster with this(!) guest than r5050 and later, as posted
here:
http://lists.gnu.org/archive/html/qemu-devel/2008-09/msg01201.html
And btw, I also since noticed that with this guest and a recent snapshot
(I haven't tried r5050) emulated scsi disks are slower (about 500 K/s here
with dd bs=64k) than in FreeBSD guests (more than 15 MB/s), and I remember
seeing the opposite, i.e. decent thruput with earlier linux guests and qemu
versions, and less (2 MB/s or so) with FreeBSD guests on that qemu version.
HTH,
Juergen
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 5/5] husb: rewrite Linux host USB layer, fully async operation
2008-08-14 4:22 [Qemu-devel] [PATCH 0/5] Various USB fixes and improvements Max Krasnyansky
` (3 preceding siblings ...)
2008-08-14 4:22 ` [Qemu-devel] [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions Max Krasnyansky
@ 2008-08-14 4:22 ` Max Krasnyansky
2008-08-15 14:24 ` Paul Brook
2008-08-14 17:55 ` [Qemu-devel] Re: [PATCH 0/5] Various USB fixes and improvements Anthony Liguori
5 siblings, 1 reply; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 4:22 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, Max Krasnyansky
This is a follow up to the async UHCI patch. Both BULK and ISOC transactions
are now fully asynchrounous. I left CONTROL synchronous for now, ideally
we want it to be async too and it should not be that hard to do now.
This patch obviously requires UHCI patch. The combo has been tested with
various devices. See the UHCI patch description for list of the devices.
Most of the testing was done with the KVM flavor of QEMU.
Signed-off-by: Max Krasnyansky <maxk@kernel.org>
---
usb-linux.c | 460 +++++++++++++++++++++++------------------------------------
1 files changed, 182 insertions(+), 278 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 622255c..786ef1b 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -3,8 +3,9 @@
*
* Copyright (c) 2005 Fabrice Bellard
*
- * Support for host device auto connect & disconnect
- * Copyright (c) 2008 Max Krasnyansky
+ * Copyright (c) 2008 Max Krasnyansky
+ * Support for host device auto connect & disconnect
+ * Magor rewrite to support fully async operation
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -55,12 +56,15 @@ static int usb_host_find_device(int *pbus_num, int *paddr,
const char *devname);
//#define DEBUG
-//#define DEBUG_ISOCH
-//#define USE_ASYNCIO
+
+#ifdef DEBUG
+#define dprintf printf
+#else
+#define dprintf(...)
+#endif
#define USBDEVFS_PATH "/proc/bus/usb"
#define PRODUCT_NAME_SZ 32
-#define SIG_ISOCOMPLETE (SIGRTMIN+7)
#define MAX_ENDPOINTS 16
struct sigaction sigact;
@@ -68,21 +72,18 @@ struct sigaction sigact;
/* endpoint association data */
struct endp_data {
uint8_t type;
+ uint8_t halted;
};
-
-
-/* FIXME: move USBPacket to PendingURB */
typedef struct USBHostDevice {
USBDevice dev;
- int fd;
- int pipe_fds[2];
- USBPacket *packet;
+ int fd;
+
+ uint8_t descr[1024];
+ int descr_len;
+ int configuration;
+
struct endp_data endp_table[MAX_ENDPOINTS];
- int configuration;
- uint8_t descr[1024];
- int descr_len;
- int urbs_ready;
QEMUTimer *timer;
@@ -93,6 +94,26 @@ typedef struct USBHostDevice {
struct USBHostDevice *next;
} USBHostDevice;
+static int is_isoc(USBHostDevice *s, int ep)
+{
+ return s->endp_table[ep - 1].type == USBDEVFS_URB_TYPE_ISO;
+}
+
+static int is_halted(USBHostDevice *s, int ep)
+{
+ return s->endp_table[ep - 1].halted;
+}
+
+static void clear_halt(USBHostDevice *s, int ep)
+{
+ s->endp_table[ep - 1].halted = 0;
+}
+
+static void set_halt(USBHostDevice *s, int ep)
+{
+ s->endp_table[ep - 1].halted = 1;
+}
+
static USBHostDevice *hostdev_list;
static void hostdev_link(USBHostDevice *dev)
@@ -128,64 +149,94 @@ static USBHostDevice *hostdev_find(int bus_num, int addr)
return NULL;
}
-typedef struct PendingURB {
- struct usbdevfs_urb *urb;
- int status;
- struct PendingURB *next;
-} PendingURB;
+/*
+ * Async URB state.
+ * We always allocate one isoc descriptor even for bulk transfers
+ * to simplify allocation and casts.
+ */
+typedef struct AsyncURB
+{
+ struct usbdevfs_urb urb;
+ struct usbdevfs_iso_packet_desc isocpd;
-static PendingURB *pending_urbs = NULL;
+ USBPacket *packet;
+ USBHostDevice *hdev;
+} AsyncURB;
-static int add_pending_urb(struct usbdevfs_urb *urb)
+static AsyncURB *async_alloc(void)
{
- PendingURB *purb = qemu_mallocz(sizeof(PendingURB));
- if (purb) {
- purb->urb = urb;
- purb->status = 0;
- purb->next = pending_urbs;
- pending_urbs = purb;
- return 1;
- }
- return 0;
+ return (AsyncURB *) qemu_mallocz(sizeof(AsyncURB));
}
-static int del_pending_urb(struct usbdevfs_urb *urb)
+static void async_free(AsyncURB *aurb)
{
- PendingURB *purb = pending_urbs;
- PendingURB *prev = NULL;
+ qemu_free(aurb);
+}
- while (purb && purb->urb != urb) {
- prev = purb;
- purb = purb->next;
- }
+static void async_complete(void *opaque)
+{
+ USBHostDevice *s = opaque;
+ AsyncURB *aurb;
+
+ while (1) {
+ USBPacket *p;
- if (purb && purb->urb == urb) {
- if (prev) {
- prev->next = purb->next;
- } else {
- pending_urbs = purb->next;
+ int r = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb);
+ if (r < 0) {
+ if (errno == EAGAIN)
+ return;
+
+ if (errno == ENODEV) {
+ printf("husb: device %d.%d disconnected\n", s->bus_num, s->addr);
+ usb_device_del_addr(0, s->dev.addr);
+ return;
+ }
+
+ dprintf("husb: async. reap urb failed errno %d\n", errno);
+ return;
}
- qemu_free(purb);
- return 1;
+
+ p = aurb->packet;
+
+ dprintf("husb: async completed. aurb %p status %d alen %d\n",
+ aurb, aurb->urb.status, aurb->urb.actual_length);
+
+ if (p) {
+ switch (aurb->urb.status) {
+ case 0:
+ p->len = aurb->urb.actual_length;
+ break;
+
+ case -EPIPE:
+ set_halt(s, p->devep);
+ /* fall through */
+ default:
+ p->len = USB_RET_NAK;
+ break;
+ }
+
+ usb_packet_complete(p);
+ }
+
+ async_free(aurb);
}
- return 0;
}
-#ifdef USE_ASYNCIO
-static PendingURB *get_pending_urb(struct usbdevfs_urb *urb)
+static void async_cancel(USBPacket *unused, void *opaque)
{
- PendingURB *purb = pending_urbs;
+ AsyncURB *aurb = opaque;
+ USBHostDevice *s = aurb->hdev;
- while (purb && purb->urb != urb) {
- purb = purb->next;
- }
+ dprintf("husb: async cancel. aurb %p\n", aurb);
+
+ /* Mark it as dead (see async_complete above) */
+ aurb->packet = NULL;
- if (purb && purb->urb == urb) {
- return purb;
+ int r = ioctl(s->fd, USBDEVFS_DISCARDURB, aurb);
+ if (r < 0) {
+ dprintf("husb: async. discard urb failed errno %d\n", errno);
}
- return NULL;
}
-#endif
static int usb_host_update_interfaces(USBHostDevice *dev, int configuration)
{
@@ -204,19 +255,16 @@ static int usb_host_update_interfaces(USBHostDevice *dev, int configuration)
i += dev_descr_len;
while (i < dev->descr_len) {
-#ifdef DEBUG
- printf("i is %d, descr_len is %d, dl %d, dt %d\n", i, dev->descr_len,
+ dprintf("husb: i is %d, descr_len is %d, dl %d, dt %d\n", i, dev->descr_len,
dev->descr[i], dev->descr[i+1]);
-#endif
+
if (dev->descr[i+1] != USB_DT_CONFIG) {
i += dev->descr[i];
continue;
}
config_descr_len = dev->descr[i];
-#ifdef DEBUG
- printf("config #%d need %d\n", dev->descr[i + 5], configuration);
-#endif
+ printf("husb: config #%d need %d\n", dev->descr[i + 5], configuration);
if (configuration < 0 || configuration == dev->descr[i + 5])
break;
@@ -225,7 +273,7 @@ static int usb_host_update_interfaces(USBHostDevice *dev, int configuration)
}
if (i >= dev->descr_len) {
- printf("usb_host: error - device has no matching configuration\n");
+ printf("husb: update iface failed. no matching configuration\n");
goto fail;
}
nb_interfaces = dev->descr[i + 4];
@@ -251,32 +299,29 @@ static int usb_host_update_interfaces(USBHostDevice *dev, int configuration)
ret = ioctl(dev->fd, USBDEVFS_CLAIMINTERFACE, &interface);
if (ret < 0) {
if (errno == EBUSY) {
- fprintf(stderr,
- "usb_host: warning - device already grabbed\n");
+ printf("husb: update iface. device already grabbed\n");
} else {
- perror("USBDEVFS_CLAIMINTERFACE");
+ perror("husb: failed to claim interface");
}
fail:
return 0;
}
}
-#ifdef DEBUG
- printf("usb_host: %d interfaces claimed for configuration %d\n",
+ printf("husb: %d interfaces claimed for configuration %d\n",
nb_interfaces, configuration);
-#endif
return 1;
}
static void usb_host_handle_reset(USBDevice *dev)
{
-#if 0
USBHostDevice *s = (USBHostDevice *)dev;
- /* USBDEVFS_RESET, but not the first time as it has already be
- done by the host OS */
+
+ dprintf("husb: reset device %u.%u\n", s->bus_num, s->addr);
+
ioctl(s->fd, USBDEVFS_RESET);
-#endif
+ usb_host_update_interfaces(s, s->configuration);
}
static void usb_host_handle_destroy(USBDevice *dev)
@@ -284,9 +329,12 @@ static void usb_host_handle_destroy(USBDevice *dev)
USBHostDevice *s = (USBHostDevice *)dev;
qemu_del_timer(s->timer);
+ qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
hostdev_unlink(s);
+ async_complete(s);
+
if (s->fd >= 0)
close(s->fd);
@@ -320,10 +368,7 @@ static int usb_host_handle_control(USBDevice *dev,
ret = ioctl(s->fd, USBDEVFS_SETINTERFACE, &si);
usb_linux_update_endp_table(s);
} else if (request == (DeviceOutRequest | USB_REQ_SET_CONFIGURATION)) {
-#ifdef DEBUG
- printf("usb_host_handle_control: SET_CONFIGURATION request - "
- "config %d\n", value & 0xff);
-#endif
+ dprintf("husb: ctrl set config %d\n", value & 0xff);
if (s->configuration != (value & 0xff)) {
s->configuration = (value & 0xff);
intf_update_required = 1;
@@ -339,6 +384,9 @@ static int usb_host_handle_control(USBDevice *dev,
ct.timeout = 50;
ct.data = data;
ret = ioctl(s->fd, USBDEVFS_CONTROL, &ct);
+
+ dprintf("husb: ctrl req 0x%x val 0x%x index %u len %u ret %d\n",
+ ct.bRequest, ct.wValue, ct.wIndex, ct.wLength, ret);
}
if (ret < 0) {
@@ -350,160 +398,69 @@ static int usb_host_handle_control(USBDevice *dev,
}
} else {
if (intf_update_required) {
-#ifdef DEBUG
- printf("usb_host_handle_control: updating interfaces\n");
-#endif
+ dprintf("husb: updating interfaces\n");
usb_host_update_interfaces(s, value & 0xff);
}
return ret;
}
}
-static int usb_host_handle_isoch(USBDevice *dev, USBPacket *p);
-
static int usb_host_handle_data(USBDevice *dev, USBPacket *p)
{
- USBHostDevice *s = (USBHostDevice *)dev;
- struct usbdevfs_bulktransfer bt;
+ USBHostDevice *s = (USBHostDevice *) dev;
+ AsyncURB *aurb;
+ struct usbdevfs_urb *urb;
int ret;
- uint8_t devep = p->devep;
- if (s->endp_table[p->devep - 1].type == USBDEVFS_URB_TYPE_ISO) {
- return usb_host_handle_isoch(dev, p);
+ aurb = async_alloc();
+ if (!aurb) {
+ dprintf("husb: async malloc failed\n");
+ return USB_RET_NAK;
}
+ aurb->hdev = s;
+ aurb->packet = p;
+
+ urb = &aurb->urb;
- /* XXX: optimize and handle all data types by looking at the
- config descriptor */
if (p->pid == USB_TOKEN_IN)
- devep |= 0x80;
- bt.ep = devep;
- bt.len = p->len;
- bt.timeout = 50;
- bt.data = p->data;
- ret = ioctl(s->fd, USBDEVFS_BULK, &bt);
- if (ret < 0) {
- switch(errno) {
- case ETIMEDOUT:
+ urb->endpoint = p->devep | 0x80;
+ else
+ urb->endpoint = p->devep;
+
+ if (is_halted(s, p->devep)) {
+ ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
+ if (ret < 0) {
+ dprintf("husb: failed to clear halt. ep 0x%x errno %d\n",
+ urb->endpoint, errno);
return USB_RET_NAK;
- case EPIPE:
- default:
-#ifdef DEBUG
- printf("handle_data: errno=%d\n", errno);
-#endif
- return USB_RET_STALL;
}
- } else {
- return ret;
- }
-}
-
-#ifdef USE_ASYNCIO
-static void urb_completion_pipe_read(void *opaque)
-{
- USBHostDevice *s = opaque;
- USBPacket *p = s->packet;
- PendingURB *pending_urb = NULL;
- struct usbdevfs_urb *purb = NULL;
- int len, ret;
-
- len = read(s->pipe_fds[0], &pending_urb, sizeof(pending_urb));
- if (len != sizeof(pending_urb)) {
- printf("urb_completion: error reading pending_urb, len=%d\n", len);
- return;
- }
-
- /* FIXME: handle pending_urb->status */
- del_pending_urb(pending_urb->urb);
-
- if (!p) {
- s->urbs_ready++;
- return;
+ clear_halt(s, p->devep);
}
- ret = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &purb);
- if (ret < 0) {
- printf("urb_completion: REAPURBNDELAY ioctl=%d errno=%d\n",
- ret, errno);
- return;
- }
-
-#ifdef DEBUG_ISOCH
- if (purb == pending_urb->urb) {
- printf("urb_completion: urb mismatch reaped=%p pending=%p\n",
- purb, urb);
- }
-#endif
-
- p->len = purb->actual_length;
- usb_packet_complete(p);
- qemu_free(purb);
- s->packet = NULL;
-}
-
-static void isoch_done(int signum, siginfo_t *info, void *context)
-{
- struct usbdevfs_urb *urb = (struct usbdevfs_urb *)info->si_addr;
- USBHostDevice *s = (USBHostDevice *)urb->usercontext;
- PendingURB *purb;
+ urb->buffer = p->data;
+ urb->buffer_length = p->len;
- if (info->si_code != SI_ASYNCIO ||
- info->si_signo != SIG_ISOCOMPLETE) {
- return;
+ 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;
}
- purb = get_pending_urb(urb);
- if (purb) {
- purb->status = info->si_errno;
- write(s->pipe_fds[1], &purb, sizeof(purb));
- }
-}
-#endif
+ urb->usercontext = s;
-static int usb_host_handle_isoch(USBDevice *dev, USBPacket *p)
-{
- USBHostDevice *s = (USBHostDevice *)dev;
- struct usbdevfs_urb *urb, *purb = NULL;
- int ret;
- uint8_t devep = p->devep;
+ ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
- if (p->pid == USB_TOKEN_IN)
- devep |= 0x80;
+ dprintf("husb: data submit. ep 0x%x len %u aurb %p\n", urb->endpoint, p->len, aurb);
- urb = qemu_mallocz(sizeof(struct usbdevfs_urb) +
- sizeof(struct usbdevfs_iso_packet_desc));
- if (!urb) {
- printf("usb_host_handle_isoch: malloc failed\n");
- return 0;
- }
+ if (ret < 0) {
+ dprintf("husb: submit failed. errno %d\n", errno);
+ async_free(aurb);
- urb->type = USBDEVFS_URB_TYPE_ISO;
- urb->endpoint = devep;
- urb->status = 0;
- urb->flags = USBDEVFS_URB_ISO_ASAP;
- urb->buffer = p->data;
- urb->buffer_length = p->len;
- urb->actual_length = 0;
- urb->start_frame = 0;
- urb->error_count = 0;
-#ifdef USE_ASYNCIO
- urb->signr = SIG_ISOCOMPLETE;
-#else
- urb->signr = 0;
-#endif
- urb->usercontext = s;
- urb->number_of_packets = 1;
- urb->iso_frame_desc[0].length = p->len;
- urb->iso_frame_desc[0].actual_length = 0;
- urb->iso_frame_desc[0].status = 0;
- ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
- if (ret == 0) {
- if (!add_pending_urb(urb)) {
- printf("usb_host_handle_isoch: add_pending_urb failed %p\n", urb);
- }
- } else {
- printf("usb_host_handle_isoch: SUBMITURB ioctl=%d errno=%d\n",
- ret, errno);
- qemu_free(urb);
switch(errno) {
case ETIMEDOUT:
return USB_RET_NAK;
@@ -512,37 +469,9 @@ static int usb_host_handle_isoch(USBDevice *dev, USBPacket *p)
return USB_RET_STALL;
}
}
-#ifdef USE_ASYNCIO
- /* FIXME: handle urbs_ready together with sync io
- * workaround for injecting the signaled urbs into current frame */
- if (s->urbs_ready > 0) {
- ret = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &purb);
- if (ret == 0) {
- ret = purb->actual_length;
- qemu_free(purb);
- s->urbs_ready--;
- }
- return ret;
- }
- s->packet = p;
+
+ usb_defer_packet(p, async_cancel, aurb);
return USB_RET_ASYNC;
-#else
- ret = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &purb);
- if (ret == 0) {
- if (del_pending_urb(purb)) {
- ret = purb->actual_length;
- qemu_free(purb);
- } else {
- printf("usb_host_handle_isoch: del_pending_urb failed %p\n", purb);
- }
- } else {
-#ifdef DEBUG_ISOCH
- printf("usb_host_handle_isoch: REAPURBNDELAY ioctl=%d errno=%d\n",
- ret, errno);
-#endif
- }
- return ret;
-#endif
}
/* returns 1 on problem encountered or 0 for success */
@@ -579,7 +508,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
if (descriptors[i + 1] != USB_DT_CONFIG ||
descriptors[i + 5] != configuration) {
- printf("invalid descriptor data - configuration\n");
+ dprintf("invalid descriptor data - configuration\n");
return 1;
}
i += descriptors[i];
@@ -641,10 +570,11 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
type = USBDEVFS_URB_TYPE_INTERRUPT;
break;
default:
- printf("usb_host: malformed endpoint type\n");
+ dprintf("usb_host: malformed endpoint type\n");
type = USBDEVFS_URB_TYPE_BULK;
}
s->endp_table[(devep & 0xf) - 1].type = type;
+ s->endp_table[(devep & 0xf) - 1].halted = 0;
i += descriptors[i];
}
@@ -660,7 +590,7 @@ static void usb_host_device_check(void *priv)
err = ioctl(s->fd, USBDEVFS_CONNECTINFO, &ci);
if (err < 0) {
- printf("usb device %d.%d disconnected\n", 0, s->dev.addr);
+ printf("husb: device %d.%d disconnected\n", s->bus_num, s->addr);
usb_device_del_addr(0, s->dev.addr);
return;
}
@@ -686,9 +616,7 @@ static USBDevice *usb_host_device_open_addr(int bus_num, int addr, const char *p
if (!dev->timer)
goto fail;
-#ifdef DEBUG
- printf("usb_host_device_open %d.%d\n", bus_num, addr);
-#endif
+ printf("husb: open device %d.%d\n", bus_num, addr);
snprintf(buf, sizeof(buf), USBDEVFS_PATH "/%03d/%03d",
bus_num, addr);
@@ -701,7 +629,7 @@ static USBDevice *usb_host_device_open_addr(int bus_num, int addr, const char *p
/* read the device description */
dev->descr_len = read(fd, dev->descr, sizeof(dev->descr));
if (dev->descr_len <= 0) {
- perror("usb_host_device_open: reading device data failed");
+ perror("husb: reading device data failed");
goto fail;
}
@@ -728,9 +656,7 @@ static USBDevice *usb_host_device_open_addr(int bus_num, int addr, const char *p
goto fail;
}
-#ifdef DEBUG
- printf("host USB device %d.%d grabbed\n", bus_num, addr);
-#endif
+ printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
ret = usb_linux_update_endp_table(dev);
if (ret)
@@ -754,34 +680,15 @@ static USBDevice *usb_host_device_open_addr(int bus_num, int addr, const char *p
pstrcpy(dev->dev.devname, sizeof(dev->dev.devname),
prod_name);
-#ifdef USE_ASYNCIO
- /* set up the signal handlers */
- sigemptyset(&sigact.sa_mask);
- sigact.sa_sigaction = isoch_done;
- sigact.sa_flags = SA_SIGINFO;
- sigact.sa_restorer = 0;
- ret = sigaction(SIG_ISOCOMPLETE, &sigact, NULL);
- if (ret < 0) {
- perror("usb_host_device_open: sigaction failed");
- goto fail;
- }
-
- if (pipe(dev->pipe_fds) < 0) {
- perror("usb_host_device_open: pipe creation failed");
- goto fail;
- }
- fcntl(dev->pipe_fds[0], F_SETFL, O_NONBLOCK | O_ASYNC);
- fcntl(dev->pipe_fds[1], F_SETFL, O_NONBLOCK);
- qemu_set_fd_handler(dev->pipe_fds[0], urb_completion_pipe_read, NULL, dev);
-#endif
+ /* USB devio uses 'write' flag to check for async completions */
+ qemu_set_fd_handler(dev->fd, NULL, async_complete, dev);
/* Start the timer to detect disconnect */
qemu_mod_timer(dev->timer, qemu_get_clock(rt_clock) + 1000);
hostdev_link(dev);
- dev->urbs_ready = 0;
- return (USBDevice *)dev;
+ return (USBDevice *) dev;
fail:
if (dev) {
@@ -804,7 +711,7 @@ USBDevice *usb_host_device_open(const char *devname)
return NULL;
if (hostdev_find(bus_num, addr)) {
- printf("host usb device %d.%d is already open\n", bus_num, addr);
+ term_printf("husb: host usb device %d.%d is already open\n", bus_num, addr);
return NULL;
}
@@ -844,7 +751,7 @@ static int usb_host_scan(void *opaque, USBScanFunc *func)
f = fopen(USBDEVFS_PATH "/devices", "r");
if (!f) {
- term_printf("Could not open %s\n", USBDEVFS_PATH "/devices");
+ term_printf("husb: could not open %s\n", USBDEVFS_PATH "/devices");
return 0;
}
device_count = 0;
@@ -954,7 +861,7 @@ static int usb_host_auto_scan(void *opaque, int bus_num, int addr,
if (hostdev_find(bus_num, addr))
return 0;
- printf("Auto open: bus_num %d addr %d\n", bus_num, addr);
+ dprintf("husb: auto open: bus_num %d addr %d\n", bus_num, addr);
dev = usb_host_device_open_addr(bus_num, addr, product_name);
if (dev)
@@ -978,7 +885,7 @@ static void usb_host_auto_add(int bus_num, int addr, int vendor_id, int product_
{
struct USBAutoFilter *f = qemu_mallocz(sizeof(*f));
if (!f) {
- printf("Failed to allocate auto filter\n");
+ printf("husb: failed to allocate auto filter\n");
return;
}
@@ -996,7 +903,7 @@ static void usb_host_auto_add(int bus_num, int addr, int vendor_id, int product_
*/
usb_auto_timer = qemu_new_timer(rt_clock, usb_host_auto_timer, NULL);
if (!usb_auto_timer) {
- printf("Failed to allocate timer\n");
+ printf("husb: failed to allocate timer\n");
qemu_free(f);
return;
}
@@ -1005,7 +912,7 @@ static void usb_host_auto_add(int bus_num, int addr, int vendor_id, int product_
qemu_mod_timer(usb_auto_timer, qemu_get_clock(rt_clock) + 2000);
}
- printf("Auto filter: bus_num %d addr %d vid %d pid %d\n",
+ dprintf("husb: auto filter: bus_num %d addr %d vid %d pid %d\n",
bus_num, addr, vendor_id, product_id);
f->next = usb_auto_filter;
@@ -1174,9 +1081,6 @@ void usb_host_info(void)
usb_host_scan(NULL, usb_host_info_device);
}
-
-
-
#else
void usb_host_info(void)
--
1.5.5.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 5/5] husb: rewrite Linux host USB layer, fully async operation
2008-08-14 4:22 ` [Qemu-devel] [PATCH 5/5] husb: rewrite Linux host USB layer, fully async operation Max Krasnyansky
@ 2008-08-15 14:24 ` Paul Brook
2008-08-15 19:04 ` Max Krasnyansky
0 siblings, 1 reply; 37+ messages in thread
From: Paul Brook @ 2008-08-15 14:24 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, Max Krasnyansky
> This patch obviously requires UHCI patch. The combo has been tested with
> various devices. See the UHCI patch description for list of the devices.
> Most of the testing was done with the KVM flavor of QEMU.
Have you tested OHCI hosts?
Paul
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] husb: rewrite Linux host USB layer, fully async operation
2008-08-15 14:24 ` Paul Brook
@ 2008-08-15 19:04 ` Max Krasnyansky
2008-08-15 19:53 ` Paul Brook
0 siblings, 1 reply; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-15 19:04 UTC (permalink / raw)
To: Paul Brook; +Cc: qemu-devel, kvm
Paul Brook wrote:
>> This patch obviously requires UHCI patch. The combo has been tested with
>> various devices. See the UHCI patch description for list of the devices.
>> Most of the testing was done with the KVM flavor of QEMU.
>
> Have you tested OHCI hosts?
>
Ah, OHCI. Good question. I have actually played with OHCI. I was pleasantly
surprised that if I just comment out uhci_pci_init() and replace it with
ohci_pci_init() XP happily detected it and internal devices and stuff worked fine.
After getting frustrated with UHCI's lack of "cancellation protocol" (UHCI
driver can yank out requests at any time, which is kind of a problem for async
since we have to cancel outstanding URBs) I started looking at and hacking on
OHCI. At first I thought OHCI is much better suited for virtualization because
of the well defined request cancellation rules and non-timer based control and
bulk queue processing. So I added all the async tracking infrastructure to the
OHCI first. While doing that I came up with TD validation logic for UHCI, in
other words I can detect true cancelation and retries fairly easily even with
UHCI. So I moved all the infrastructure to UHCI, to try out the idea, and it
worked great.
Given that OHCI is much more complex than UHCI (both the code and the spec) I
decided to give up on OHCI, at least for now. I noticed Codesourcery copyright
on OHCI. Did you have anything to do with the OHCI implementation ? If yes
maybe you can help me out with it ?
Now. What I meant by "This patch obviously requires UHCI patch." is that in
order to get all the benefits of the new async logic we need UHCI patches,
that does not however mean that usb-linux changes by itself break existing
stuff. New usb-linux does not really enforce any constraints or rules on how
requests are submitted. It's all driven by the UHCI or OHCI. Current async
handling in OHCI is very limited (just like it was in UHCI before my patches)
so while it's not completely broken it does not really work well because
everything is async now (besides control) and it used to be sync.
So we'd definitely need to work on OHCI if it's important for some platforms.
I won't have spare cycles though.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] husb: rewrite Linux host USB layer, fully async operation
2008-08-15 19:04 ` Max Krasnyansky
@ 2008-08-15 19:53 ` Paul Brook
2008-08-18 18:40 ` Max Krasnyansky
0 siblings, 1 reply; 37+ messages in thread
From: Paul Brook @ 2008-08-15 19:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, Max Krasnyansky
> Given that OHCI is much more complex than UHCI (both the code and the spec)
> I decided to give up on OHCI, at least for now. I noticed Codesourcery
> copyright on OHCI. Did you have anything to do with the OHCI implementation?
Yes, I wrote the current OHCI support, based on some initial patches by
Gianni.
I found OHCI to be much easier to deal with than than UHCI. The low-level bits
of the USB protocol are fairly nasty. UHCI is a cheap and nasty host
solution, which directly exposes (and requires faking of) all the nasty low
level details and timing.
OHCI is a higher level interface which I found makes it much easier to
actually implement things sanely in a virtual environment.
> So we'd definitely need to work on OHCI if it's important for some
> platforms.
I consider OHCI to be important. x86 is abut the only target foolish enough to
use UHCI.
Paul
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] husb: rewrite Linux host USB layer, fully async operation
2008-08-15 19:53 ` Paul Brook
@ 2008-08-18 18:40 ` Max Krasnyansky
0 siblings, 0 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-18 18:40 UTC (permalink / raw)
To: Paul Brook; +Cc: qemu-devel, kvm
Paul Brook wrote:
>> Given that OHCI is much more complex than UHCI (both the code and the spec)
>> I decided to give up on OHCI, at least for now. I noticed Codesourcery
>> copyright on OHCI. Did you have anything to do with the OHCI implementation?
>
> Yes, I wrote the current OHCI support, based on some initial patches by
> Gianni.
Great. So do you have spare cycles to add decent async support to it ?
> I found OHCI to be much easier to deal with than than UHCI. The low-level bits
> of the USB protocol are fairly nasty. UHCI is a cheap and nasty host
> solution, which directly exposes (and requires faking of) all the nasty low
> level details and timing.
>
> OHCI is a higher level interface which I found makes it much easier to
> actually implement things sanely in a virtual environment.
Four days ago :) I would've completely agreed with you. But once I
figured out a ridiculously simple way to match outstanding transactions
against current QHs and TDs (ED/TD in the OHCI terms) I changed my
opinion. I mean in general OHCI does seem better. But cheap does not
necessarily mean bad. In this case it means _simple_. Last Friday I made
asynchronous ISOC transactions work reliably and both UHCI and usb-linux
code is very generic and does not need any special logic for isoc which
is nice.
Unless I'm missing something OCHI also exposes USB frame timing when it
comes to ISOC and interrupt transactions, and is fairly similar to UHCI
in that regard.
btw With the new UHCI code there is really no need to fake any USB
timing in the UHCI. I simply match transactions by tokens (which encodes
addr/endpoit/size/etc) and use QH/TD structure just to determine how the
frame is composed. Yes there is per frame timer, but all it does it just
updates pending transactions and submits new ones. OHCI also needs a
timer to handle ISOC transactions.
So overall I think UHCI is totally usable and sane now. And what I like
the most is that it's very simple. It's almost 2:1 ratio, code size
wise, with OHCI vs UHCI once I added support for multiple async
transactions to both (like I did not finish OHCI).
>> So we'd definitely need to work on OHCI if it's important for some
>> platforms.
>
> I consider OHCI to be important. x86 is abut the only target foolish enough to
> use UHCI.
I think we need both for ultimate compatibility.
We also need EHCI otherwise XP keeps telling me "dude, you plugged
device XYZ into the wrong port" :).
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] Re: [PATCH 0/5] Various USB fixes and improvements
2008-08-14 4:22 [Qemu-devel] [PATCH 0/5] Various USB fixes and improvements Max Krasnyansky
` (4 preceding siblings ...)
2008-08-14 4:22 ` [Qemu-devel] [PATCH 5/5] husb: rewrite Linux host USB layer, fully async operation Max Krasnyansky
@ 2008-08-14 17:55 ` Anthony Liguori
2008-08-14 19:55 ` Max Krasnyansky
5 siblings, 1 reply; 37+ messages in thread
From: Anthony Liguori @ 2008-08-14 17:55 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: qemu-devel, kvm
Max Krasnyansky wrote:
> This patch series started when I tried to share USB ports between
> four instances of Windows XP running on the same Linux box (under KVM).
> I quickly realized that current USB support is not very flexible. We do
> not handle devices disconnects, there is not way to assign certain USB
> ports to VM instance, etc.
>
> Once I fixed that I discovered that USB devices that I absolutely need
> in the VMs (Xilinx and Altera USB dongles) do not really work with
> QEMU. VMs were getting stuck, applications unhappy, etc.
> So I endded up rewriting UHCI and Linux host USB layers to make them
> fully async and to support multiple outstanding transactions.
>
> The result is quite nice. We can now assign USB buses to VM instances
> and devices are automatically connected to the VMs. Just do
> usb_add host:N.*
> in the console or -usbdevice command line option (N is the bus number).
> Also when device is disconnected from the host it's automatically removed
> from the guest.
>
> Host USB devices operate in fully async mode (except the control transfers).
> All the stalls and jerkiness due to long synchronous transactions is gone.
> I can easily hook up four different USB devices (mouse, CF card reader,
> phone, Xilinx dongle) and everything is working perfectly. Mouse movements
> are silky smooth :).
>
> I did some profiling with OProfile and we seems to be doing ok while XP is
> pumping ~10 MBytes over USB (reported by one of the apps I'm using). UHCI
> stuff is well below VNC for example.
>
> There is more work to be done (async control transfers for example). But I
> think this is way better than what we have now and is ready for more testing
> by wider audience.
>
> Most of the testing so far was done with KVM flavor of QEMU. I did test
> generic i386-softmmu target a bit, it's too slow for any serious testing
> with XP. I did full compile (all targets) too and it went fine.
>
Altogether this patch series looks very promising. The current USB pass
through code is pretty fickle so some clean-up is definitely needed.
I don't understand USB well enough to do a thorough review. I hope
others can look through it and provide more detailed feedback. I'll do
some testing of it tonight.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 37+ messages in thread* [Qemu-devel] Re: [PATCH 0/5] Various USB fixes and improvements
2008-08-14 17:55 ` [Qemu-devel] Re: [PATCH 0/5] Various USB fixes and improvements Anthony Liguori
@ 2008-08-14 19:55 ` Max Krasnyansky
0 siblings, 0 replies; 37+ messages in thread
From: Max Krasnyansky @ 2008-08-14 19:55 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, kvm
Anthony Liguori wrote:
> Max Krasnyansky wrote:
>> This patch series started when I tried to share USB ports between four
>> instances of Windows XP running on the same Linux box (under KVM).
>> I quickly realized that current USB support is not very flexible. We do
>> not handle devices disconnects, there is not way to assign certain USB
>> ports to VM instance, etc.
>>
>> Once I fixed that I discovered that USB devices that I absolutely need
>> in the VMs (Xilinx and Altera USB dongles) do not really work with
>> QEMU. VMs were getting stuck, applications unhappy, etc.
>> So I endded up rewriting UHCI and Linux host USB layers to make them
>> fully async and to support multiple outstanding transactions.
>>
>> The result is quite nice. We can now assign USB buses to VM instances
>> and devices are automatically connected to the VMs. Just do
>> usb_add host:N.*
>> in the console or -usbdevice command line option (N is the bus number).
>> Also when device is disconnected from the host it's automatically removed
>> from the guest.
>>
>> Host USB devices operate in fully async mode (except the control
>> transfers).
>> All the stalls and jerkiness due to long synchronous transactions is
>> gone.
>> I can easily hook up four different USB devices (mouse, CF card
>> reader, phone, Xilinx dongle) and everything is working perfectly.
>> Mouse movements
>> are silky smooth :).
>>
>> I did some profiling with OProfile and we seems to be doing ok while
>> XP is pumping ~10 MBytes over USB (reported by one of the apps I'm
>> using). UHCI stuff is well below VNC for example.
>>
>> There is more work to be done (async control transfers for example).
>> But I think this is way better than what we have now and is ready for
>> more testing
>> by wider audience.
>> Most of the testing so far was done with KVM flavor of QEMU. I did
>> test generic i386-softmmu target a bit, it's too slow for any serious
>> testing with XP. I did full compile (all targets) too and it went fine.
>>
>
> Altogether this patch series looks very promising. The current USB pass
> through code is pretty fickle so some clean-up is definitely needed.
Great.
> I don't understand USB well enough to do a thorough review. I hope
> others can look through it and provide more detailed feedback.
That would be great. I bet not a lot of people understand UHCI specs though.
> I'll do some testing of it tonight.
Awesome. If something does not work as expected please enable DEBUG in both
hw/usb-uhci.c and usb-linux.c and send me the output.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread