* [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
@ 2008-08-21 19:30 Anthony Liguori
2008-08-21 22:32 ` Aurelien Jarno
0 siblings, 1 reply; 10+ messages in thread
From: Anthony Liguori @ 2008-08-21 19:30 UTC (permalink / raw)
To: qemu-devel
Revision: 5050
http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5050
Author: aliguori
Date: 2008-08-21 19:30:31 +0000 (Thu, 21 Aug 2008)
Log Message:
-----------
uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions (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>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Modified Paths:
--------------
trunk/hw/usb-uhci.c
Modified: trunk/hw/usb-uhci.c
===================================================================
--- trunk/hw/usb-uhci.c 2008-08-21 19:29:38 UTC (rev 5049)
+++ trunk/hw/usb-uhci.c 2008-08-21 19:30:31 UTC (rev 5050)
@@ -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 @@
/* 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 @@
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 @@
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 @@
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 @@
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 @@
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,418 +629,407 @@
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_IOS)
+ td->ctrl &= ~TD_CTRL_ACTIVE;
+
+ if (ret < 0)
+ goto out;
+
+ 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;
+ ret = USB_RET_BABBLE;
+ goto out;
+ }
+
+ 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;
+ }
}
- if (!(td->ctrl & TD_CTRL_ACTIVE))
+ /* success */
+ return 0;
+
+out:
+ switch(ret) {
+ case USB_RET_STALL:
+ td->ctrl |= TD_CTRL_STALL;
+ td->ctrl &= ~TD_CTRL_ACTIVE;
return 1;
- /* TD is active */
- max_len = ((td->token >> 21) + 1) & 0x7ff;
- pid = td->token & 0xff;
+ case USB_RET_BABBLE:
+ td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
+ td->ctrl &= ~TD_CTRL_ACTIVE;
+ /* frame interrupted */
+ return -1;
- 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 = max_len;
+ case USB_RET_NAK:
+ td->ctrl |= TD_CTRL_NAK;
+ if (pid == USB_TOKEN_SETUP)
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;
+ 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;
+ s->status |= UHCI_STS_USBERR;
uhci_update_irq(s);
- return -1;
}
}
+ td->ctrl = (td->ctrl & ~(3 << TD_CTRL_ERROR_SHIFT)) |
+ (err << TD_CTRL_ERROR_SHIFT);
+ return 1;
+}
+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;
}
- 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) {
- *int_mask |= 0x02;
- /* short packet: do not update QH */
+
+ 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;
+ 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;
- } 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;
- td->ctrl &= ~TD_CTRL_ACTIVE;
- /* frame interrupted */
- return -1;
- }
- }
+
+ if (db->count >= UHCI_MAX_QUEUES)
+ return 1;
+
+ db->addr[db->count++] = addr;
+ return 0;
}
-static void uhci_async_complete_packet(USBPacket * packet, void *opaque)
+static void uhci_process_frame(UHCIState *s)
{
- UHCIState *s = opaque;
+ uint32_t frame_addr, link, old_td_ctrl, val, int_mask;
+ uint32_t curr_qh;
+ int cnt, ret;
+ UHCI_TD td;
UHCI_QH qh;
- UHCI_TD td;
- uint32_t link;
- uint32_t old_td_ctrl;
- uint32_t val;
- uint32_t frame_addr;
- int ret;
+ 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);
- cpu_physical_memory_read(link & ~0xf, (uint8_t *)&td, sizeof(td));
+ 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);
+
+ 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);
+
+ 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, &td, &s->pending_int_mask, 1);
-
- /* update the status bits of the TD */
+ 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);
+ /* Start new frame */
+ s->frnum = (s->frnum + 1) & 0x7ff;
- /* 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);
+ dprintf("uhci: new frame #%u\n" , s->frnum);
- /* 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;
- }
+ 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) +
(ticks_per_sec / FRAME_TIMER_FREQ);
@@ -950,4 +1117,6 @@
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);
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-21 19:30 [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky) Anthony Liguori
@ 2008-08-21 22:32 ` Aurelien Jarno
2008-08-21 22:38 ` Anthony Liguori
2008-08-21 22:41 ` Anthony Liguori
0 siblings, 2 replies; 10+ messages in thread
From: Aurelien Jarno @ 2008-08-21 22:32 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: Anthony Liguori, qemu-devel
On Thu, Aug 21, 2008 at 07:30:32PM +0000, Anthony Liguori wrote:
> Revision: 5050
> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5050
> Author: aliguori
> Date: 2008-08-21 19:30:31 +0000 (Thu, 21 Aug 2008)
>
> Log Message:
> -----------
> uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions (Max Krasnyansky)
Please note that this commit has broken the -usbdevice option (at least
disk, tablet and mouse) for a GNU/Linux guest running a 2.6.18 kernel.
> 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>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-21 22:32 ` Aurelien Jarno
@ 2008-08-21 22:38 ` Anthony Liguori
2008-08-21 22:43 ` Aurelien Jarno
2008-08-21 22:41 ` Anthony Liguori
1 sibling, 1 reply; 10+ messages in thread
From: Anthony Liguori @ 2008-08-21 22:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Aurelien Jarno, Max Krasnyansky
Aurelien Jarno wrote:
> On Thu, Aug 21, 2008 at 07:30:32PM +0000, Anthony Liguori wrote:
>
>> Revision: 5050
>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5050
>> Author: aliguori
>> Date: 2008-08-21 19:30:31 +0000 (Thu, 21 Aug 2008)
>>
>> Log Message:
>> -----------
>> uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions (Max Krasnyansky)
>>
>
> Please note that this commit has broken the -usbdevice option (at least
> disk, tablet and mouse) for a GNU/Linux guest running a 2.6.18 kernel.
>
>
I'll revert it. Max, can you take a look at it?
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-21 22:32 ` Aurelien Jarno
2008-08-21 22:38 ` Anthony Liguori
@ 2008-08-21 22:41 ` Anthony Liguori
2008-08-22 0:26 ` Max Krasnyansky
1 sibling, 1 reply; 10+ messages in thread
From: Anthony Liguori @ 2008-08-21 22:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Aurelien Jarno, Max Krasnyansky
Aurelien Jarno wrote:
> On Thu, Aug 21, 2008 at 07:30:32PM +0000, Anthony Liguori wrote:
>
>> Revision: 5050
>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5050
>> Author: aliguori
>> Date: 2008-08-21 19:30:31 +0000 (Thu, 21 Aug 2008)
>>
>> Log Message:
>> -----------
>> uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions (Max Krasnyansky)
>>
>
> Please note that this commit has broken the -usbdevice option (at least
> disk, tablet and mouse) for a GNU/Linux guest running a 2.6.18 kernel.
>
Well, reverting is going to be painful since this is the 4th patch in a
series of 8, so I'll try to fix this tonight, otherwise, I'll revert.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-21 22:38 ` Anthony Liguori
@ 2008-08-21 22:43 ` Aurelien Jarno
0 siblings, 0 replies; 10+ messages in thread
From: Aurelien Jarno @ 2008-08-21 22:43 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Max Krasnyansky
On Thu, Aug 21, 2008 at 05:38:41PM -0500, Anthony Liguori wrote:
> Aurelien Jarno wrote:
>> On Thu, Aug 21, 2008 at 07:30:32PM +0000, Anthony Liguori wrote:
>>
>>> Revision: 5050
>>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5050
>>> Author: aliguori
>>> Date: 2008-08-21 19:30:31 +0000 (Thu, 21 Aug 2008)
>>>
>>> Log Message:
>>> -----------
>>> uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions (Max Krasnyansky)
>>>
>>
>> Please note that this commit has broken the -usbdevice option (at least
>> disk, tablet and mouse) for a GNU/Linux guest running a 2.6.18 kernel.
>>
>>
>
> I'll revert it. Max, can you take a look at it?
>
Well if we have a fix quickly, maybe there is no need to revert it.
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-21 22:41 ` Anthony Liguori
@ 2008-08-22 0:26 ` Max Krasnyansky
2008-08-22 1:46 ` Aurelien Jarno
0 siblings, 1 reply; 10+ messages in thread
From: Max Krasnyansky @ 2008-08-22 0:26 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Aurelien Jarno
Anthony Liguori wrote:
> Aurelien Jarno wrote:
>> On Thu, Aug 21, 2008 at 07:30:32PM +0000, Anthony Liguori wrote:
>>
>>> Revision: 5050
>>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5050
>>> Author: aliguori
>>> Date: 2008-08-21 19:30:31 +0000 (Thu, 21 Aug 2008)
>>>
>>> Log Message:
>>> -----------
>>> uhci: rewrite UHCI emulator, fully async operation with multiple
>>> outstanding transactions (Max Krasnyansky)
>>>
>>
>> Please note that this commit has broken the -usbdevice option (at least
>> disk, tablet and mouse) for a GNU/Linux guest running a 2.6.18 kernel.
>>
> Well, reverting is going to be painful since this is the 4th patch in a
> series of 8, so I'll try to fix this tonight, otherwise, I'll revert.
I've done all my testing so far with XP as a guest, and the -usbdevice
tablet is working just fine. In fact nothing should have changed in that
area since builtin devices are synchronous.
I'll setup Linux guest right now and check things out.
Aurelien, can you provide a bit more details on your setup. Do use KQEMU
or KVM ? Since you mentioned table & mouse I'm assuming that you're
using a graphic console. Is it VNC or SDL ?. In fact why don't you just
send me your qemu command like. So that I can replicated the problem better.
Thanx
Max
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-22 0:26 ` Max Krasnyansky
@ 2008-08-22 1:46 ` Aurelien Jarno
2008-08-22 3:34 ` Max Krasnyansky
2008-08-22 6:07 ` Max Krasnyansky
0 siblings, 2 replies; 10+ messages in thread
From: Aurelien Jarno @ 2008-08-22 1:46 UTC (permalink / raw)
To: Max Krasnyansky; +Cc: qemu-devel
On Thu, Aug 21, 2008 at 05:26:49PM -0700, Max Krasnyansky wrote:
> Anthony Liguori wrote:
>> Aurelien Jarno wrote:
>>> On Thu, Aug 21, 2008 at 07:30:32PM +0000, Anthony Liguori wrote:
>>>
>>>> Revision: 5050
>>>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5050
>>>> Author: aliguori
>>>> Date: 2008-08-21 19:30:31 +0000 (Thu, 21 Aug 2008)
>>>>
>>>> Log Message:
>>>> -----------
>>>> uhci: rewrite UHCI emulator, fully async operation with multiple
>>>> outstanding transactions (Max Krasnyansky)
>>>>
>>>
>>> Please note that this commit has broken the -usbdevice option (at least
>>> disk, tablet and mouse) for a GNU/Linux guest running a 2.6.18 kernel.
>>>
>> Well, reverting is going to be painful since this is the 4th patch in a
>> series of 8, so I'll try to fix this tonight, otherwise, I'll revert.
>
> I've done all my testing so far with XP as a guest, and the -usbdevice
> tablet is working just fine. In fact nothing should have changed in that
> area since builtin devices are synchronous.
>
> I'll setup Linux guest right now and check things out.
>
> Aurelien, can you provide a bit more details on your setup. Do use KQEMU
> or KVM ? Since you mentioned table & mouse I'm assuming that you're
> using a graphic console. Is it VNC or SDL ?. In fact why don't you just
> send me your qemu command like. So that I can replicated the problem
> better.
>
I am using QEMU, from the SVN, without KQEMU. The problem does not
depend of the console used (SDL, VNC or curses). The guest kernel
outputs the following errors:
usb 1-2: device not accepting address 2, error -71
usb 1-2: new full speed USB device using uhci_hcd and address 3
usb 1-2: device not accepting address 3, error -71
usb 1-2: new full speed USB device using uhci_hcd and address 4
usb 1-2: device not accepting address 4, error -71
usb 1-2: new full speed USB device using uhci_hcd and address 5
usb 1-2: device not accepting address 5, error -71
You can download the image from:
http://people.debian.org/~aurel32/qemu/amd64/
It is reproducible with the following command line:
qemu-system-x86_64 -hda debian_etch_amd64_small.qcow -usbdevice tablet
The error can also be reproduced with -usbdevice mouse or -usbdevice
disk:image.
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-22 1:46 ` Aurelien Jarno
@ 2008-08-22 3:34 ` Max Krasnyansky
2008-08-22 6:07 ` Max Krasnyansky
1 sibling, 0 replies; 10+ messages in thread
From: Max Krasnyansky @ 2008-08-22 3:34 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
Aurelien Jarno wrote:
> On Thu, Aug 21, 2008 at 05:26:49PM -0700, Max Krasnyansky wrote:
>> Aurelien, can you provide a bit more details on your setup. Do use KQEMU
>> or KVM ? Since you mentioned table & mouse I'm assuming that you're
>> using a graphic console. Is it VNC or SDL ?. In fact why don't you just
>> send me your qemu command like. So that I can replicated the problem
>> better.
>>
>
> I am using QEMU, from the SVN, without KQEMU. The problem does not
> depend of the console used (SDL, VNC or curses). The guest kernel
> outputs the following errors:
> usb 1-2: device not accepting address 2, error -71
> usb 1-2: new full speed USB device using uhci_hcd and address 3
> usb 1-2: device not accepting address 3, error -71
> usb 1-2: new full speed USB device using uhci_hcd and address 4
> usb 1-2: device not accepting address 4, error -71
> usb 1-2: new full speed USB device using uhci_hcd and address 5
> usb 1-2: device not accepting address 5, error -71
>
> You can download the image from:
> http://people.debian.org/~aurel32/qemu/amd64/
>
> It is reproducible with the following command line:
> qemu-system-x86_64 -hda debian_etch_amd64_small.qcow -usbdevice tablet
>
> The error can also be reproduced with -usbdevice mouse or -usbdevice
> disk:image.
Ok. I'm looking at it right now. I can reproduce the error with your image.
Interestingly enough Fedora9 32bit is working just fine under latest KVM plus
the same USB patches.
I'll report back as soon as I find something.
Max
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-22 1:46 ` Aurelien Jarno
2008-08-22 3:34 ` Max Krasnyansky
@ 2008-08-22 6:07 ` Max Krasnyansky
2008-08-22 8:59 ` Aurelien Jarno
1 sibling, 1 reply; 10+ messages in thread
From: Max Krasnyansky @ 2008-08-22 6:07 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
Aurelien Jarno wrote:
> I am using QEMU, from the SVN, without KQEMU. The problem does not
> depend of the console used (SDL, VNC or curses). The guest kernel
> outputs the following errors:
> usb 1-2: device not accepting address 2, error -71
> usb 1-2: new full speed USB device using uhci_hcd and address 3
> usb 1-2: device not accepting address 3, error -71
> usb 1-2: new full speed USB device using uhci_hcd and address 4
> usb 1-2: device not accepting address 4, error -71
> usb 1-2: new full speed USB device using uhci_hcd and address 5
> usb 1-2: device not accepting address 5, error -71
>
> You can download the image from:
> http://people.debian.org/~aurel32/qemu/amd64/
>
> It is reproducible with the following command line:
> qemu-system-x86_64 -hda debian_etch_amd64_small.qcow -usbdevice tablet
>
> The error can also be reproduced with -usbdevice mouse or -usbdevice
> disk:image.
Turns out I managed to screw up transaction length handling for control
transfers in certain scenarious. Looks like XP and newer kernels are not
sensitive to that.
I just sent out a patch that fixes the regression.
[PATCH] uhci: Fixed length handling for SETUP and OUT tokens
Please confirm that it fixes your setup. I tested XP, your Debian image and
Fedora 9.
Max
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky)
2008-08-22 6:07 ` Max Krasnyansky
@ 2008-08-22 8:59 ` Aurelien Jarno
0 siblings, 0 replies; 10+ messages in thread
From: Aurelien Jarno @ 2008-08-22 8:59 UTC (permalink / raw)
To: qemu-devel
On Thu, Aug 21, 2008 at 11:07:03PM -0700, Max Krasnyansky wrote:
> Aurelien Jarno wrote:
> > I am using QEMU, from the SVN, without KQEMU. The problem does not
> > depend of the console used (SDL, VNC or curses). The guest kernel
> > outputs the following errors:
> > usb 1-2: device not accepting address 2, error -71
> > usb 1-2: new full speed USB device using uhci_hcd and address 3
> > usb 1-2: device not accepting address 3, error -71
> > usb 1-2: new full speed USB device using uhci_hcd and address 4
> > usb 1-2: device not accepting address 4, error -71
> > usb 1-2: new full speed USB device using uhci_hcd and address 5
> > usb 1-2: device not accepting address 5, error -71
> >
> > You can download the image from:
> > http://people.debian.org/~aurel32/qemu/amd64/
> >
> > It is reproducible with the following command line:
> > qemu-system-x86_64 -hda debian_etch_amd64_small.qcow -usbdevice tablet
> >
> > The error can also be reproduced with -usbdevice mouse or -usbdevice
> > disk:image.
> Turns out I managed to screw up transaction length handling for control
> transfers in certain scenarious. Looks like XP and newer kernels are not
> sensitive to that.
>
> I just sent out a patch that fixes the regression.
>
> [PATCH] uhci: Fixed length handling for SETUP and OUT tokens
>
> Please confirm that it fixes your setup. I tested XP, your Debian image and
> Fedora 9.
Yes, I confirm it works with my setup. Thanks for your reactivity.
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-08-22 8:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-21 19:30 [Qemu-devel] [5050] uhci: rewrite UHCI emulator, fully async operation with multiple outstanding transactions ( Max Krasnyansky) Anthony Liguori
2008-08-21 22:32 ` Aurelien Jarno
2008-08-21 22:38 ` Anthony Liguori
2008-08-21 22:43 ` Aurelien Jarno
2008-08-21 22:41 ` Anthony Liguori
2008-08-22 0:26 ` Max Krasnyansky
2008-08-22 1:46 ` Aurelien Jarno
2008-08-22 3:34 ` Max Krasnyansky
2008-08-22 6:07 ` Max Krasnyansky
2008-08-22 8:59 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).