From eeb0cf9abf5992f35eca18c4cc63300df30521a4 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 22 Aug 2011 09:09:51 +0200 Subject: [PATCH 01/21] usb/vmstate: add parent dev path ... to make vmstate id string truely unique with multiple host controllers, i.e. move from "1/usb-ptr" to "0000:00:01.3/1/usb-ptr" (usb tabled connected to piix3 uhci). This obviously breaks migration. To handle this the usb bus property "full-path" is added. When setting this to false old behavior is maintained. This way current qemu will be compatible with old versions when started using '-M pc-$oldversion'. Signed-off-by: Gerd Hoffmann --- hw/pc_piix.c | 4 ++++ hw/usb.h | 5 +++++ hw/usb/bus.c | 17 ++++++++++++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 907d723728..6a75718fbb 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -377,6 +377,10 @@ static QEMUMachine pc_machine_v1_1 = { .driver = "apic",\ .property = "vapic",\ .value = "off",\ + },{\ + .driver = "USB",\ + .property = "full-path",\ + .value = "no",\ } static QEMUMachine pc_machine_v1_0 = { diff --git a/hw/usb.h b/hw/usb.h index e95085f0b3..ae7ccda18c 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -182,12 +182,17 @@ struct USBEndpoint { QTAILQ_HEAD(, USBPacket) queue; }; +enum USBDeviceFlags { + USB_DEV_FLAG_FULL_PATH, +}; + /* definition of a USB device */ struct USBDevice { DeviceState qdev; USBPort *port; char *port_path; void *opaque; + uint32_t flags; /* Actual connected speed */ int speed; diff --git a/hw/usb/bus.c b/hw/usb/bus.c index d3f835895d..2068640a58 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -19,6 +19,8 @@ static struct BusInfo usb_bus_info = { .get_fw_dev_path = usb_get_fw_dev_path, .props = (Property[]) { DEFINE_PROP_STRING("port", USBDevice, port_path), + DEFINE_PROP_BIT("full-path", USBDevice, flags, + USB_DEV_FLAG_FULL_PATH, true), DEFINE_PROP_END_OF_LIST() }, }; @@ -460,7 +462,20 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent) static char *usb_get_dev_path(DeviceState *qdev) { USBDevice *dev = USB_DEVICE(qdev); - return g_strdup(dev->port->path); + DeviceState *hcd = qdev->parent_bus->parent; + char *id = NULL; + + if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) && + hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) { + id = hcd->parent_bus->info->get_dev_path(hcd); + } + if (id) { + char *ret = g_strdup_printf("%s/%s", id, dev->port->path); + g_free(id); + return ret; + } else { + return g_strdup(dev->port->path); + } } static char *usb_get_fw_dev_path(DeviceState *qdev) From 52b0fecdba217e02d7e7eef975d942b153950b2f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 21 Mar 2012 18:25:25 +0100 Subject: [PATCH 02/21] usb-uhci: stop queue filling when we find a in-flight td Not only QHs can form rings, but TDs too. With the new queuing/pipelining support we are following TD chains and can actually walk in circles. An assert() prevents us from entering an endless loop then. Fix is easy: Just stop queuing when we figure the TD we are about to queue up is in flight already. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index e55dad914c..2be564b675 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -965,6 +965,9 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) } trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token); ret = uhci_handle_td(s, plink, &ptd, &int_mask); + if (ret == TD_RESULT_ASYNC_CONT) { + break; + } assert(ret == TD_RESULT_ASYNC_START); assert(int_mask == 0); plink = ptd.link; From ee008ba62673943d5cd40c5761955efdbca444cd Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 29 Mar 2012 16:02:20 +0200 Subject: [PATCH 03/21] usb-uhci: queuing fix When we queue up usb packets we may happen to find a already queued packet, which also might be finished at that point already. We don't want continue processing the packet at this point though, so lets just signal back we've found a in-flight packet when in queuing mode. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-uhci.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 2be564b675..266d550b9c 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -795,7 +795,8 @@ out: return TD_RESULT_NEXT_QH; } -static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, uint32_t *int_mask) +static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, + uint32_t *int_mask, bool queuing) { UHCIAsync *async; int len = 0, max_len; @@ -814,6 +815,12 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, uint32_t *in if (!async->done) return TD_RESULT_ASYNC_CONT; + if (queuing) { + /* we are busy filling the queue, we are not prepared + to consume completed packages then, just leave them + in async state */ + return TD_RESULT_ASYNC_CONT; + } uhci_async_unlink(async); goto done; @@ -964,7 +971,7 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) break; } trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token); - ret = uhci_handle_td(s, plink, &ptd, &int_mask); + ret = uhci_handle_td(s, plink, &ptd, &int_mask, true); if (ret == TD_RESULT_ASYNC_CONT) { break; } @@ -1048,7 +1055,7 @@ static void uhci_process_frame(UHCIState *s) trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token); old_td_ctrl = td.ctrl; - ret = uhci_handle_td(s, link, &td, &int_mask); + ret = uhci_handle_td(s, link, &td, &int_mask, false); if (old_td_ctrl != td.ctrl) { /* update the status bits of the TD */ val = cpu_to_le32(td.ctrl); From 65bb3a5c11b00671c1067ee27ea364b6d7e6e2ac Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 22 Mar 2012 10:48:03 +0100 Subject: [PATCH 04/21] Add bootindex support to usb-host and usb-redir When passing through a usb pendrive seabios will present it in the F12 boot menu and will happily boot from it. This patch adds bootorder support so you can even make it the default boot device. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 3 +++ hw/usb/redirect.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 90919c242a..5eb69160b0 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -115,6 +115,7 @@ typedef struct USBHostDevice { int addr; char port[MAX_PORTLEN]; struct USBAutoFilter match; + int32_t bootindex; int seen, errcount; QTAILQ_ENTRY(USBHostDevice) next; @@ -1403,6 +1404,7 @@ static int usb_host_initfn(USBDevice *dev) if (s->match.bus_num != 0 && s->match.port != NULL) { usb_host_claim_port(s); } + add_boot_device_path(s->bootindex, &dev->qdev, NULL); return 0; } @@ -1418,6 +1420,7 @@ static Property usb_host_dev_properties[] = { DEFINE_PROP_HEX32("vendorid", USBHostDevice, match.vendor_id, 0), DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0), DEFINE_PROP_UINT32("isobufs", USBHostDevice, iso_urb_count, 4), + DEFINE_PROP_INT32("bootindex", USBHostDevice, bootindex, -1), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 8e9f175dbb..4288324110 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -74,6 +74,7 @@ struct USBRedirDevice { CharDriverState *cs; uint8_t debug; char *filter_str; + int32_t bootindex; /* Data passed from chardev the fd_read cb to the usbredirparser read cb */ const uint8_t *read_buf; int read_buf_size; @@ -923,6 +924,7 @@ static int usbredir_initfn(USBDevice *udev) qemu_chr_add_handlers(dev->cs, usbredir_chardev_can_read, usbredir_chardev_read, usbredir_chardev_event, dev); + add_boot_device_path(dev->bootindex, &udev->qdev, NULL); return 0; } @@ -1452,6 +1454,7 @@ static Property usbredir_properties[] = { DEFINE_PROP_CHR("chardev", USBRedirDevice, cs), DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, 0), DEFINE_PROP_STRING("filter", USBRedirDevice, filter_str), + DEFINE_PROP_INT32("bootindex", USBRedirDevice, bootindex, -1), DEFINE_PROP_END_OF_LIST(), }; From e382e751ef7848b237be2d569815dcc4f8097306 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 22 Mar 2012 15:10:55 +0100 Subject: [PATCH 05/21] usb-host: trace emulated requests Add tracepoint to track completion of emulated control requests. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 12 +++++++++--- trace-events | 1 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 5eb69160b0..912ce23928 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -1035,13 +1035,19 @@ static int usb_host_handle_control(USBDevice *dev, USBPacket *p, switch (request) { case DeviceOutRequest | USB_REQ_SET_ADDRESS: - return usb_host_set_address(s, value); + ret = usb_host_set_address(s, value); + trace_usb_host_req_emulated(s->bus_num, s->addr, ret); + return ret; case DeviceOutRequest | USB_REQ_SET_CONFIGURATION: - return usb_host_set_config(s, value & 0xff); + ret = usb_host_set_config(s, value & 0xff); + trace_usb_host_req_emulated(s->bus_num, s->addr, ret); + return ret; case InterfaceOutRequest | USB_REQ_SET_INTERFACE: - return usb_host_set_interface(s, index, value); + ret = usb_host_set_interface(s, index, value); + trace_usb_host_req_emulated(s->bus_num, s->addr, ret); + return ret; } /* The rest are asynchronous */ diff --git a/trace-events b/trace-events index a5f276d020..8ac1b62128 100644 --- a/trace-events +++ b/trace-events @@ -315,6 +315,7 @@ usb_host_release_interfaces(int bus, int addr) "dev %d:%d" usb_host_req_control(int bus, int addr, int req, int value, int index) "dev %d:%d, req 0x%x, value %d, index %d" usb_host_req_data(int bus, int addr, int in, int ep, int size) "dev %d:%d, in %d, ep %d, size %d" usb_host_req_complete(int bus, int addr, int status) "dev %d:%d, status %d" +usb_host_req_emulated(int bus, int addr, int status) "dev %d:%d, status %d" usb_host_urb_submit(int bus, int addr, void *aurb, int length, int more) "dev %d:%d, aurb %p, length %d, more %d" usb_host_urb_complete(int bus, int addr, void *aurb, int status, int length, int more) "dev %d:%d, aurb %p, status %d, length %d, more %d" usb_host_ep_set_halt(int bus, int addr, int ep) "dev %d:%d, ep %d" From 6aebe407968a0c7e0be2647391753471ed674c65 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 23 Mar 2012 12:26:59 +0100 Subject: [PATCH 06/21] usb-host: trace canceled requests Add tracepoints to track canceled requests. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 4 +++- trace-events | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 912ce23928..6ea59a7af5 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -392,12 +392,14 @@ static void usb_host_async_cancel(USBDevice *dev, USBPacket *p) USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev); AsyncURB *aurb; + trace_usb_host_req_canceled(s->bus_num, s->addr); + QLIST_FOREACH(aurb, &s->aurbs, next) { if (p != aurb->packet) { continue; } - DPRINTF("husb: async cancel: packet %p, aurb %p\n", p, aurb); + trace_usb_host_urb_canceled(s->bus_num, s->addr, aurb); /* Mark it as dead (see async_complete above) */ aurb->packet = NULL; diff --git a/trace-events b/trace-events index 8ac1b62128..3e8da19fd8 100644 --- a/trace-events +++ b/trace-events @@ -316,8 +316,10 @@ usb_host_req_control(int bus, int addr, int req, int value, int index) "dev %d:% usb_host_req_data(int bus, int addr, int in, int ep, int size) "dev %d:%d, in %d, ep %d, size %d" usb_host_req_complete(int bus, int addr, int status) "dev %d:%d, status %d" usb_host_req_emulated(int bus, int addr, int status) "dev %d:%d, status %d" +usb_host_req_canceled(int bus, int addr) "dev %d:%d" usb_host_urb_submit(int bus, int addr, void *aurb, int length, int more) "dev %d:%d, aurb %p, length %d, more %d" usb_host_urb_complete(int bus, int addr, void *aurb, int status, int length, int more) "dev %d:%d, aurb %p, status %d, length %d, more %d" +usb_host_urb_canceled(int bus, int addr, void *aurb) "dev %d:%d, aurb %p" usb_host_ep_set_halt(int bus, int addr, int ep) "dev %d:%d, ep %d" usb_host_ep_clear_halt(int bus, int addr, int ep) "dev %d:%d, ep %d" usb_host_ep_start_iso(int bus, int addr, int ep) "dev %d:%d, ep %d" From 19b89252a32e53aa8bd90a8d4c3e3dcc5f8fe46d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 23 Mar 2012 12:35:55 +0100 Subject: [PATCH 07/21] usb-host: add usb packet to request tracepoints Add pointer to USBPacket to all tracepoints tracking requests to make it easier to identify them when multiple requests are in flight. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 26 ++++++++++++++------------ trace-events | 10 +++++----- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 6ea59a7af5..8084fd941a 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -375,10 +375,10 @@ static void async_complete(void *opaque) } if (aurb->urb.type == USBDEVFS_URB_TYPE_CONTROL) { - trace_usb_host_req_complete(s->bus_num, s->addr, p->result); + trace_usb_host_req_complete(s->bus_num, s->addr, p, p->result); usb_generic_async_ctrl_complete(&s->dev, p); } else if (!aurb->more) { - trace_usb_host_req_complete(s->bus_num, s->addr, p->result); + trace_usb_host_req_complete(s->bus_num, s->addr, p, p->result); usb_packet_complete(&s->dev, p); } } @@ -392,7 +392,7 @@ static void usb_host_async_cancel(USBDevice *dev, USBPacket *p) USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev); AsyncURB *aurb; - trace_usb_host_req_canceled(s->bus_num, s->addr); + trace_usb_host_req_canceled(s->bus_num, s->addr, p); QLIST_FOREACH(aurb, &s->aurbs, next) { if (p != aurb->packet) { @@ -847,12 +847,12 @@ static int usb_host_handle_data(USBDevice *dev, USBPacket *p) uint8_t *pbuf; uint8_t ep; - trace_usb_host_req_data(s->bus_num, s->addr, + trace_usb_host_req_data(s->bus_num, s->addr, p, p->pid == USB_TOKEN_IN, p->ep->nr, p->iov.size); if (!is_valid(s, p->pid, p->ep->nr)) { - trace_usb_host_req_complete(s->bus_num, s->addr, USB_RET_NAK); + trace_usb_host_req_complete(s->bus_num, s->addr, p, USB_RET_NAK); return USB_RET_NAK; } @@ -867,7 +867,7 @@ static int usb_host_handle_data(USBDevice *dev, USBPacket *p) ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &arg); if (ret < 0) { perror("USBDEVFS_CLEAR_HALT"); - trace_usb_host_req_complete(s->bus_num, s->addr, USB_RET_NAK); + trace_usb_host_req_complete(s->bus_num, s->addr, p, USB_RET_NAK); return USB_RET_NAK; } clear_halt(s, p->pid, p->ep->nr); @@ -922,11 +922,13 @@ static int usb_host_handle_data(USBDevice *dev, USBPacket *p) switch(errno) { case ETIMEDOUT: - trace_usb_host_req_complete(s->bus_num, s->addr, USB_RET_NAK); + trace_usb_host_req_complete(s->bus_num, s->addr, p, + USB_RET_NAK); return USB_RET_NAK; case EPIPE: default: - trace_usb_host_req_complete(s->bus_num, s->addr, USB_RET_STALL); + trace_usb_host_req_complete(s->bus_num, s->addr, p, + USB_RET_STALL); return USB_RET_STALL; } } @@ -1033,22 +1035,22 @@ static int usb_host_handle_control(USBDevice *dev, USBPacket *p, */ /* Note request is (bRequestType << 8) | bRequest */ - trace_usb_host_req_control(s->bus_num, s->addr, request, value, index); + trace_usb_host_req_control(s->bus_num, s->addr, p, request, value, index); switch (request) { case DeviceOutRequest | USB_REQ_SET_ADDRESS: ret = usb_host_set_address(s, value); - trace_usb_host_req_emulated(s->bus_num, s->addr, ret); + trace_usb_host_req_emulated(s->bus_num, s->addr, p, ret); return ret; case DeviceOutRequest | USB_REQ_SET_CONFIGURATION: ret = usb_host_set_config(s, value & 0xff); - trace_usb_host_req_emulated(s->bus_num, s->addr, ret); + trace_usb_host_req_emulated(s->bus_num, s->addr, p, ret); return ret; case InterfaceOutRequest | USB_REQ_SET_INTERFACE: ret = usb_host_set_interface(s, index, value); - trace_usb_host_req_emulated(s->bus_num, s->addr, ret); + trace_usb_host_req_emulated(s->bus_num, s->addr, p, ret); return ret; } diff --git a/trace-events b/trace-events index 3e8da19fd8..e3b178767c 100644 --- a/trace-events +++ b/trace-events @@ -312,11 +312,11 @@ usb_host_set_config(int bus, int addr, int config) "dev %d:%d, config %d" usb_host_set_interface(int bus, int addr, int interface, int alt) "dev %d:%d, interface %d, alt %d" usb_host_claim_interfaces(int bus, int addr, int config, int nif) "dev %d:%d, config %d, nif %d" usb_host_release_interfaces(int bus, int addr) "dev %d:%d" -usb_host_req_control(int bus, int addr, int req, int value, int index) "dev %d:%d, req 0x%x, value %d, index %d" -usb_host_req_data(int bus, int addr, int in, int ep, int size) "dev %d:%d, in %d, ep %d, size %d" -usb_host_req_complete(int bus, int addr, int status) "dev %d:%d, status %d" -usb_host_req_emulated(int bus, int addr, int status) "dev %d:%d, status %d" -usb_host_req_canceled(int bus, int addr) "dev %d:%d" +usb_host_req_control(int bus, int addr, void *p, int req, int value, int index) "dev %d:%d, packet %p, req 0x%x, value %d, index %d" +usb_host_req_data(int bus, int addr, void *p, int in, int ep, int size) "dev %d:%d, packet %p, in %d, ep %d, size %d" +usb_host_req_complete(int bus, int addr, void *p, int status) "dev %d:%d, packet %p, status %d" +usb_host_req_emulated(int bus, int addr, void *p, int status) "dev %d:%d, packet %p, status %d" +usb_host_req_canceled(int bus, int addr, void *p) "dev %d:%d, packet %p" usb_host_urb_submit(int bus, int addr, void *aurb, int length, int more) "dev %d:%d, aurb %p, length %d, more %d" usb_host_urb_complete(int bus, int addr, void *aurb, int status, int length, int more) "dev %d:%d, aurb %p, status %d, length %d, more %d" usb_host_urb_canceled(int bus, int addr, void *aurb) "dev %d:%d, aurb %p" From 39c20577009731d5e059db10ef269807b57e498d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 22 Mar 2012 15:28:45 +0100 Subject: [PATCH 08/21] usb-host: add property to turn off pipelining Add a property to usb-host to disable the bulk endpoint pipelining. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 8084fd941a..a382f0ad98 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -94,6 +94,10 @@ struct USBAutoFilter { uint32_t product_id; }; +enum USBHostDeviceOptions { + USB_HOST_OPT_PIPELINE, +}; + typedef struct USBHostDevice { USBDevice dev; int fd; @@ -104,6 +108,7 @@ typedef struct USBHostDevice { int descr_len; int closing; uint32_t iso_urb_count; + uint32_t options; Notifier exit; struct endp_data ep_in[USB_MAX_ENDPOINTS]; @@ -1203,7 +1208,8 @@ static int usb_linux_update_endp_table(USBHostDevice *s) USB_ENDPOINT_XFER_INVALID); usb_ep_set_type(&s->dev, pid, ep, type); usb_ep_set_ifnum(&s->dev, pid, ep, interface); - if (type == USB_ENDPOINT_XFER_BULK) { + if ((s->options & (1 << USB_HOST_OPT_PIPELINE)) && + (type == USB_ENDPOINT_XFER_BULK)) { usb_ep_set_pipeline(&s->dev, pid, ep, true); } @@ -1431,6 +1437,8 @@ static Property usb_host_dev_properties[] = { DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0), DEFINE_PROP_UINT32("isobufs", USBHostDevice, iso_urb_count, 4), DEFINE_PROP_INT32("bootindex", USBHostDevice, bootindex, -1), + DEFINE_PROP_BIT("pipeline", USBHostDevice, options, + USB_HOST_OPT_PIPELINE, true), DEFINE_PROP_END_OF_LIST(), }; From f5bf14bf39ec1ca2ad70ca1ec0e38a3e1e3f252d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 23 Mar 2012 13:34:50 +0100 Subject: [PATCH 09/21] usb_packet_set_state: handle p->ep == NULL usb_packet_set_state can be called with p->ep = NULL. The tracepoint there tries to log endpoint information, which leads to a segfault. This patch makes usb_packet_set_state handle the NULL pointer properly. Signed-off-by: Gerd Hoffmann --- hw/usb/core.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/hw/usb/core.c b/hw/usb/core.c index a4048fe3e0..9a14a53852 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -484,12 +484,17 @@ void usb_packet_check_state(USBPacket *p, USBPacketState expected) void usb_packet_set_state(USBPacket *p, USBPacketState state) { - USBDevice *dev = p->ep->dev; - USBBus *bus = usb_bus_from_device(dev); - - trace_usb_packet_state_change(bus->busnr, dev->port->path, p->ep->nr, p, - usb_packet_state_name(p->state), - usb_packet_state_name(state)); + if (p->ep) { + USBDevice *dev = p->ep->dev; + USBBus *bus = usb_bus_from_device(dev); + trace_usb_packet_state_change(bus->busnr, dev->port->path, p->ep->nr, p, + usb_packet_state_name(p->state), + usb_packet_state_name(state)); + } else { + trace_usb_packet_state_change(-1, "", -1, p, + usb_packet_state_name(p->state), + usb_packet_state_name(state)); + } p->state = state; } From 529f8f9fa939b8db13bf5cd1cd549d993cf25cb8 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 23 Mar 2012 15:42:58 +0100 Subject: [PATCH 10/21] usb-hub: add tracepoints Add tracepoints to the usb hub emulation. Signed-off-by: Gerd Hoffmann --- hw/usb/dev-hub.c | 43 +++++++++++++++++++++++++++++++++++++++++-- trace-events | 11 ++++++++++- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c index eb4e711207..9c9166551e 100644 --- a/hw/usb/dev-hub.c +++ b/hw/usb/dev-hub.c @@ -22,11 +22,10 @@ * THE SOFTWARE. */ #include "qemu-common.h" +#include "trace.h" #include "hw/usb.h" #include "hw/usb/desc.h" -//#define DEBUG - #define NUM_PORTS 8 typedef struct USBHubPort { @@ -157,6 +156,7 @@ static void usb_hub_attach(USBPort *port1) USBHubState *s = port1->opaque; USBHubPort *port = &s->ports[port1->index]; + trace_usb_hub_attach(s->dev.addr, port1->index + 1); port->wPortStatus |= PORT_STAT_CONNECTION; port->wPortChange |= PORT_STAT_C_CONNECTION; if (port->port.dev->speed == USB_SPEED_LOW) { @@ -172,6 +172,7 @@ static void usb_hub_detach(USBPort *port1) USBHubState *s = port1->opaque; USBHubPort *port = &s->ports[port1->index]; + trace_usb_hub_detach(s->dev.addr, port1->index + 1); usb_wakeup(s->intr); /* Let upstream know the device on this port is gone */ @@ -247,6 +248,7 @@ static void usb_hub_handle_reset(USBDevice *dev) USBHubPort *port; int i; + trace_usb_hub_reset(s->dev.addr); for (i = 0; i < NUM_PORTS; i++) { port = s->ports + i; port->wPortStatus = PORT_STAT_POWER; @@ -261,12 +263,39 @@ static void usb_hub_handle_reset(USBDevice *dev) } } +static const char *feature_name(int feature) +{ + static const char *name[] = { + [PORT_CONNECTION] = "connection", + [PORT_ENABLE] = "enable", + [PORT_SUSPEND] = "suspend", + [PORT_OVERCURRENT] = "overcurrent", + [PORT_RESET] = "reset", + [PORT_POWER] = "power", + [PORT_LOWSPEED] = "lowspeed", + [PORT_HIGHSPEED] = "highspeed", + [PORT_C_CONNECTION] = "change connection", + [PORT_C_ENABLE] = "change enable", + [PORT_C_SUSPEND] = "change suspend", + [PORT_C_OVERCURRENT] = "change overcurrent", + [PORT_C_RESET] = "change reset", + [PORT_TEST] = "test", + [PORT_INDICATOR] = "indicator", + }; + if (feature < 0 || feature >= ARRAY_SIZE(name)) { + return "?"; + } + return name[feature] ?: "?"; +} + static int usb_hub_handle_control(USBDevice *dev, USBPacket *p, int request, int value, int index, int length, uint8_t *data) { USBHubState *s = (USBHubState *)dev; int ret; + trace_usb_hub_control(s->dev.addr, request, value, index, length); + ret = usb_desc_handle_control(dev, p, request, value, index, length, data); if (ret >= 0) { return ret; @@ -295,6 +324,9 @@ static int usb_hub_handle_control(USBDevice *dev, USBPacket *p, goto fail; } port = &s->ports[n]; + trace_usb_hub_get_port_status(s->dev.addr, index, + port->wPortStatus, + port->wPortChange); data[0] = port->wPortStatus; data[1] = port->wPortStatus >> 8; data[2] = port->wPortChange; @@ -315,6 +347,10 @@ static int usb_hub_handle_control(USBDevice *dev, USBPacket *p, unsigned int n = index - 1; USBHubPort *port; USBDevice *dev; + + trace_usb_hub_set_port_feature(s->dev.addr, index, + feature_name(value)); + if (n >= NUM_PORTS) { goto fail; } @@ -345,6 +381,9 @@ static int usb_hub_handle_control(USBDevice *dev, USBPacket *p, unsigned int n = index - 1; USBHubPort *port; + trace_usb_hub_clear_port_feature(s->dev.addr, index, + feature_name(value)); + if (n >= NUM_PORTS) { goto fail; } diff --git a/trace-events b/trace-events index e3b178767c..e7ce5b2cb3 100644 --- a/trace-events +++ b/trace-events @@ -289,7 +289,7 @@ usb_uhci_td_nextqh(uint32_t qh, uint32_t td) "qh 0x%x, td 0x%x" usb_uhci_td_async(uint32_t qh, uint32_t td) "qh 0x%x, td 0x%x" usb_uhci_td_complete(uint32_t qh, uint32_t td) "qh 0x%x, td 0x%x" -# hw/usb-desc.c +# hw/usb/desc.c usb_desc_device(int addr, int len, int ret) "dev %d query device, len %d, ret %d" usb_desc_device_qualifier(int addr, int len, int ret) "dev %d query device qualifier, len %d, ret %d" usb_desc_config(int addr, int index, int len, int ret) "dev %d query config %d, len %d, ret %d" @@ -301,6 +301,15 @@ usb_set_interface(int addr, int iface, int alt, int ret) "dev %d, interface %d, usb_clear_device_feature(int addr, int feature, int ret) "dev %d, feature %d, ret %d" usb_set_device_feature(int addr, int feature, int ret) "dev %d, feature %d, ret %d" +# hw/usb/dev-hub.c +usb_hub_reset(int addr) "dev %d" +usb_hub_control(int addr, int request, int value, int index, int length) "dev %d, req 0x%x, value %d, index %d, langth %d" +usb_hub_get_port_status(int addr, int nr, int status, int changed) "dev %d, port %d, status 0x%x, changed 0x%x" +usb_hub_set_port_feature(int addr, int nr, const char *f) "dev %d, port %d, feature %s" +usb_hub_clear_port_feature(int addr, int nr, const char *f) "dev %d, port %d, feature %s" +usb_hub_attach(int addr, int nr) "dev %d, port %d" +usb_hub_detach(int addr, int nr) "dev %d, port %d" + # hw/usb/host-linux.c usb_host_open_started(int bus, int addr) "dev %d:%d" usb_host_open_success(int bus, int addr) "dev %d:%d" From 088351a7e50b7f14cda9520bcb8a7896e226aa36 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 23 Mar 2012 15:43:45 +0100 Subject: [PATCH 11/21] usb-ehci: fix ehci_child_detach Looks like a cut+paste bug from ehci_detach. When the device itself is detached from a ehci port (ehci_detach op) we have to clear the device pointer for the companion port too. When a device gets removed from a downstream port of a usb hub (ehci_child_detach op) the ehci port where the usb hub is plugged in is not affected. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 60f9f5bdb5..34f7538066 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -797,7 +797,6 @@ static void ehci_child_detach(USBPort *port, USBDevice *child) if (portsc & PORTSC_POWNER) { USBPort *companion = s->companion_ports[port->index]; companion->ops->child_detach(companion, child); - companion->dev = NULL; return; } From 58ea88d87a834923b31271750b8eab6403a797be Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 28 Mar 2012 20:47:51 +0200 Subject: [PATCH 12/21] usb-ehci: frindex always is a 14 bits counter frindex always is a 14 bits counter, and not a 13 bits one as we were emulating. There are some subtle hints to this in the spec, first of all "Table 2-12. FRINDEX - Frame Index Register" says: "Bit 13:0 Frame Index. The value in this register increments at the end of each time frame (e.g. micro-frame). Bits [N:3] are used for the Frame List current index. This means that each location of the frame list is accessed 8 times (frames or micro-frames) before moving to the next index. The following illustrates values of N based on the value of the Frame List Size field in the USBCMD register. USBCMD[Frame List Size] Number Elements N 00b 1024 12 01b 512 11 10b 256 10 11b Reserved" Notice how the text talks about "Bits [N:3]" are used ..., it does NOT say that when N == 12 (our case) the counter will wrap from 8191 to 0, or in otherwords that it is a 13 bits counter (bits 0 - 12). The other hint is in "Table 2-10. USBSTS USB Status Register Bit Definitions": "Bit 3 Frame List Rollover - R/WC. The Host Controller sets this bit to a one when the Frame List Index (see Section 2.3.4) rolls over from its maximum value to zero. The exact value at which the rollover occurs depends on the frame list size. For example, if the frame list size (as programmed in the Frame List Size field of the USBCMD register) is 1024, the Frame Index Register rolls over every time FRINDEX[13] toggles. Similarly, if the size is 512, the Host Controller sets this bit to a one every time FRINDEX[12] toggles." Notice how this text talks about setting bit 3 when bit 13 of frindex toggles (when there are 1024 entries, so our case), so this indicates that frindex has a bit 13 making it a 14 bit counter. Besides these clear hints the real proof is in the pudding. Before this patch I could not stream data from a USB2 webcam under Windows XP, after this cam using a USB2 webcam under Windows XP works fine, and no regressions with other operating systems were seen. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 34f7538066..be02308242 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2149,11 +2149,15 @@ static void ehci_frame_timer(void *opaque) if ( !(ehci->usbsts & USBSTS_HALT)) { ehci->frindex += 8; - if (ehci->frindex > 0x00001fff) { - ehci->frindex = 0; + if (ehci->frindex == 0x00002000) { ehci_set_interrupt(ehci, USBSTS_FLR); } + if (ehci->frindex == 0x00004000) { + ehci_set_interrupt(ehci, USBSTS_FLR); + ehci->frindex = 0; + } + ehci->sofv = (ehci->frindex - 1) >> 3; ehci->sofv &= 0x000003ff; } From d3f904ea7b3027f58037e359cc199c2490dc5c3d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 29 Mar 2012 12:04:54 +0200 Subject: [PATCH 13/21] usb: add USBDescriptor, use for device descriptors. This patch adds a new type for the binary representation of usb descriptors. It is put into use for the descriptor generator code where the struct replaces the hard-coded offsets. Signed-off-by: Gerd Hoffmann --- hw/usb/desc.c | 37 +++++++++++++++++++------------------ hw/usb/desc.h | 26 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/hw/usb/desc.c b/hw/usb/desc.c index 9847a75b83..de7e2043b5 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -18,32 +18,33 @@ int usb_desc_device(const USBDescID *id, const USBDescDevice *dev, uint8_t *dest, size_t len) { uint8_t bLength = 0x12; + USBDescriptor *d = (void *)dest; if (len < bLength) { return -1; } - dest[0x00] = bLength; - dest[0x01] = USB_DT_DEVICE; + d->bLength = bLength; + d->bDescriptorType = USB_DT_DEVICE; - dest[0x02] = usb_lo(dev->bcdUSB); - dest[0x03] = usb_hi(dev->bcdUSB); - dest[0x04] = dev->bDeviceClass; - dest[0x05] = dev->bDeviceSubClass; - dest[0x06] = dev->bDeviceProtocol; - dest[0x07] = dev->bMaxPacketSize0; + d->u.device.bcdUSB_lo = usb_lo(dev->bcdUSB); + d->u.device.bcdUSB_hi = usb_hi(dev->bcdUSB); + d->u.device.bDeviceClass = dev->bDeviceClass; + d->u.device.bDeviceSubClass = dev->bDeviceSubClass; + d->u.device.bDeviceProtocol = dev->bDeviceProtocol; + d->u.device.bMaxPacketSize0 = dev->bMaxPacketSize0; - dest[0x08] = usb_lo(id->idVendor); - dest[0x09] = usb_hi(id->idVendor); - dest[0x0a] = usb_lo(id->idProduct); - dest[0x0b] = usb_hi(id->idProduct); - dest[0x0c] = usb_lo(id->bcdDevice); - dest[0x0d] = usb_hi(id->bcdDevice); - dest[0x0e] = id->iManufacturer; - dest[0x0f] = id->iProduct; - dest[0x10] = id->iSerialNumber; + d->u.device.idVendor_lo = usb_lo(id->idVendor); + d->u.device.idVendor_hi = usb_hi(id->idVendor); + d->u.device.idProduct_lo = usb_lo(id->idProduct); + d->u.device.idProduct_hi = usb_hi(id->idProduct); + d->u.device.bcdDevice_lo = usb_lo(id->bcdDevice); + d->u.device.bcdDevice_hi = usb_hi(id->bcdDevice); + d->u.device.iManufacturer = id->iManufacturer; + d->u.device.iProduct = id->iProduct; + d->u.device.iSerialNumber = id->iSerialNumber; - dest[0x11] = dev->bNumConfigurations; + d->u.device.bNumConfigurations = dev->bNumConfigurations; return bLength; } diff --git a/hw/usb/desc.h b/hw/usb/desc.h index d6e07ea5d2..c5a242e3c1 100644 --- a/hw/usb/desc.h +++ b/hw/usb/desc.h @@ -3,6 +3,32 @@ #include +/* binary representation */ +typedef struct USBDescriptor { + uint8_t bLength; + uint8_t bDescriptorType; + union { + struct { + uint8_t bcdUSB_lo; + uint8_t bcdUSB_hi; + uint8_t bDeviceClass; + uint8_t bDeviceSubClass; + uint8_t bDeviceProtocol; + uint8_t bMaxPacketSize0; + uint8_t idVendor_lo; + uint8_t idVendor_hi; + uint8_t idProduct_lo; + uint8_t idProduct_hi; + uint8_t bcdDevice_lo; + uint8_t bcdDevice_hi; + uint8_t iManufacturer; + uint8_t iProduct; + uint8_t iSerialNumber; + uint8_t bNumConfigurations; + } device; + } u; +} QEMU_PACKED USBDescriptor; + struct USBDescID { uint16_t idVendor; uint16_t idProduct; From 3cfeee6177bb7c86db8e1fa01cd6f5d438e4c463 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 29 Mar 2012 12:15:01 +0200 Subject: [PATCH 14/21] usb: use USBDescriptor for device qualifier descriptors. Add device qualifier substruct to USBDescriptor, use it in the descriptor generator code. Signed-off-by: Gerd Hoffmann --- hw/usb/desc.c | 21 +++++++++++---------- hw/usb/desc.h | 10 ++++++++++ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/hw/usb/desc.c b/hw/usb/desc.c index de7e2043b5..5cecb25f2f 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -53,22 +53,23 @@ int usb_desc_device_qualifier(const USBDescDevice *dev, uint8_t *dest, size_t len) { uint8_t bLength = 0x0a; + USBDescriptor *d = (void *)dest; if (len < bLength) { return -1; } - dest[0x00] = bLength; - dest[0x01] = USB_DT_DEVICE_QUALIFIER; + d->bLength = bLength; + d->bDescriptorType = USB_DT_DEVICE_QUALIFIER; - dest[0x02] = usb_lo(dev->bcdUSB); - dest[0x03] = usb_hi(dev->bcdUSB); - dest[0x04] = dev->bDeviceClass; - dest[0x05] = dev->bDeviceSubClass; - dest[0x06] = dev->bDeviceProtocol; - dest[0x07] = dev->bMaxPacketSize0; - dest[0x08] = dev->bNumConfigurations; - dest[0x09] = 0; /* reserved */ + d->u.device_qualifier.bcdUSB_lo = usb_lo(dev->bcdUSB); + d->u.device_qualifier.bcdUSB_hi = usb_hi(dev->bcdUSB); + d->u.device_qualifier.bDeviceClass = dev->bDeviceClass; + d->u.device_qualifier.bDeviceSubClass = dev->bDeviceSubClass; + d->u.device_qualifier.bDeviceProtocol = dev->bDeviceProtocol; + d->u.device_qualifier.bMaxPacketSize0 = dev->bMaxPacketSize0; + d->u.device_qualifier.bNumConfigurations = dev->bNumConfigurations; + d->u.device_qualifier.bReserved = 0; return bLength; } diff --git a/hw/usb/desc.h b/hw/usb/desc.h index c5a242e3c1..15d0780ace 100644 --- a/hw/usb/desc.h +++ b/hw/usb/desc.h @@ -26,6 +26,16 @@ typedef struct USBDescriptor { uint8_t iSerialNumber; uint8_t bNumConfigurations; } device; + struct { + uint8_t bcdUSB_lo; + uint8_t bcdUSB_hi; + uint8_t bDeviceClass; + uint8_t bDeviceSubClass; + uint8_t bDeviceProtocol; + uint8_t bMaxPacketSize0; + uint8_t bNumConfigurations; + uint8_t bReserved; + } device_qualifier; } u; } QEMU_PACKED USBDescriptor; From 0a263db17a59a04b8f22540b284919956751e4f8 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 29 Mar 2012 12:24:08 +0200 Subject: [PATCH 15/21] usb: use USBDescriptor for config descriptors. Add config descriptor substruct to USBDescriptor, use it in the descriptor generator code. Signed-off-by: Gerd Hoffmann --- hw/usb/desc.c | 22 ++++++++++++---------- hw/usb/desc.h | 9 +++++++++ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/hw/usb/desc.c b/hw/usb/desc.c index 5cecb25f2f..3466968f6d 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -78,22 +78,24 @@ int usb_desc_config(const USBDescConfig *conf, uint8_t *dest, size_t len) { uint8_t bLength = 0x09; uint16_t wTotalLength = 0; + USBDescriptor *d = (void *)dest; int i, rc; if (len < bLength) { return -1; } - dest[0x00] = bLength; - dest[0x01] = USB_DT_CONFIG; - dest[0x04] = conf->bNumInterfaces; - dest[0x05] = conf->bConfigurationValue; - dest[0x06] = conf->iConfiguration; - dest[0x07] = conf->bmAttributes; - dest[0x08] = conf->bMaxPower; + d->bLength = bLength; + d->bDescriptorType = USB_DT_CONFIG; + + d->u.config.bNumInterfaces = conf->bNumInterfaces; + d->u.config.bConfigurationValue = conf->bConfigurationValue; + d->u.config.iConfiguration = conf->iConfiguration; + d->u.config.bmAttributes = conf->bmAttributes; + d->u.config.bMaxPower = conf->bMaxPower; wTotalLength += bLength; - /* handle grouped interfaces if any*/ + /* handle grouped interfaces if any */ for (i = 0; i < conf->nif_groups; i++) { rc = usb_desc_iface_group(&(conf->if_groups[i]), dest + wTotalLength, @@ -113,8 +115,8 @@ int usb_desc_config(const USBDescConfig *conf, uint8_t *dest, size_t len) wTotalLength += rc; } - dest[0x02] = usb_lo(wTotalLength); - dest[0x03] = usb_hi(wTotalLength); + d->u.config.wTotalLength_lo = usb_lo(wTotalLength); + d->u.config.wTotalLength_hi = usb_hi(wTotalLength); return wTotalLength; } diff --git a/hw/usb/desc.h b/hw/usb/desc.h index 15d0780ace..298e050553 100644 --- a/hw/usb/desc.h +++ b/hw/usb/desc.h @@ -36,6 +36,15 @@ typedef struct USBDescriptor { uint8_t bNumConfigurations; uint8_t bReserved; } device_qualifier; + struct { + uint8_t wTotalLength_lo; + uint8_t wTotalLength_hi; + uint8_t bNumInterfaces; + uint8_t bConfigurationValue; + uint8_t iConfiguration; + uint8_t bmAttributes; + uint8_t bMaxPower; + } config; } u; } QEMU_PACKED USBDescriptor; From feafd797ee5b12d6831aaafccd41c192ad9cc8bd Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 29 Mar 2012 12:30:33 +0200 Subject: [PATCH 16/21] usb: use USBDescriptor for interface descriptors. Add interface descriptor substruct to USBDescriptor, use it in the descriptor generator code. Signed-off-by: Gerd Hoffmann --- hw/usb/desc.c | 20 +++++++++++--------- hw/usb/desc.h | 9 +++++++++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/hw/usb/desc.c b/hw/usb/desc.c index 3466968f6d..2b8febb020 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -159,20 +159,22 @@ int usb_desc_iface(const USBDescIface *iface, uint8_t *dest, size_t len) { uint8_t bLength = 0x09; int i, rc, pos = 0; + USBDescriptor *d = (void *)dest; if (len < bLength) { return -1; } - dest[0x00] = bLength; - dest[0x01] = USB_DT_INTERFACE; - dest[0x02] = iface->bInterfaceNumber; - dest[0x03] = iface->bAlternateSetting; - dest[0x04] = iface->bNumEndpoints; - dest[0x05] = iface->bInterfaceClass; - dest[0x06] = iface->bInterfaceSubClass; - dest[0x07] = iface->bInterfaceProtocol; - dest[0x08] = iface->iInterface; + d->bLength = bLength; + d->bDescriptorType = USB_DT_INTERFACE; + + d->u.interface.bInterfaceNumber = iface->bInterfaceNumber; + d->u.interface.bAlternateSetting = iface->bAlternateSetting; + d->u.interface.bNumEndpoints = iface->bNumEndpoints; + d->u.interface.bInterfaceClass = iface->bInterfaceClass; + d->u.interface.bInterfaceSubClass = iface->bInterfaceSubClass; + d->u.interface.bInterfaceProtocol = iface->bInterfaceProtocol; + d->u.interface.iInterface = iface->iInterface; pos += bLength; for (i = 0; i < iface->ndesc; i++) { diff --git a/hw/usb/desc.h b/hw/usb/desc.h index 298e050553..6f42eae8b9 100644 --- a/hw/usb/desc.h +++ b/hw/usb/desc.h @@ -45,6 +45,15 @@ typedef struct USBDescriptor { uint8_t bmAttributes; uint8_t bMaxPower; } config; + struct { + uint8_t bInterfaceNumber; + uint8_t bAlternateSetting; + uint8_t bNumEndpoints; + uint8_t bInterfaceClass; + uint8_t bInterfaceSubClass; + uint8_t bInterfaceProtocol; + uint8_t iInterface; + } interface; } u; } QEMU_PACKED USBDescriptor; From e36a20d329b4cf52ce3b269345527e7ebcc00885 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 29 Mar 2012 16:01:21 +0200 Subject: [PATCH 17/21] usb: use USBDescriptor for endpoint descriptors. Add endpoint descriptor substruct to USBDescriptor, use it in the descriptor generator code. Signed-off-by: Gerd Hoffmann --- hw/usb/desc.c | 20 +++++++++++--------- hw/usb/desc.h | 9 +++++++++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/hw/usb/desc.c b/hw/usb/desc.c index 2b8febb020..3c77368cb0 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -200,21 +200,23 @@ int usb_desc_endpoint(const USBDescEndpoint *ep, uint8_t *dest, size_t len) { uint8_t bLength = ep->is_audio ? 0x09 : 0x07; uint8_t extralen = ep->extra ? ep->extra[0] : 0; + USBDescriptor *d = (void *)dest; if (len < bLength + extralen) { return -1; } - dest[0x00] = bLength; - dest[0x01] = USB_DT_ENDPOINT; - dest[0x02] = ep->bEndpointAddress; - dest[0x03] = ep->bmAttributes; - dest[0x04] = usb_lo(ep->wMaxPacketSize); - dest[0x05] = usb_hi(ep->wMaxPacketSize); - dest[0x06] = ep->bInterval; + d->bLength = bLength; + d->bDescriptorType = USB_DT_ENDPOINT; + + d->u.endpoint.bEndpointAddress = ep->bEndpointAddress; + d->u.endpoint.bmAttributes = ep->bmAttributes; + d->u.endpoint.wMaxPacketSize_lo = usb_lo(ep->wMaxPacketSize); + d->u.endpoint.wMaxPacketSize_hi = usb_hi(ep->wMaxPacketSize); + d->u.endpoint.bInterval = ep->bInterval; if (ep->is_audio) { - dest[0x07] = ep->bRefresh; - dest[0x08] = ep->bSynchAddress; + d->u.endpoint.bRefresh = ep->bRefresh; + d->u.endpoint.bSynchAddress = ep->bSynchAddress; } if (ep->extra) { memcpy(dest + bLength, ep->extra, extralen); diff --git a/hw/usb/desc.h b/hw/usb/desc.h index 6f42eae8b9..d164e8f891 100644 --- a/hw/usb/desc.h +++ b/hw/usb/desc.h @@ -54,6 +54,15 @@ typedef struct USBDescriptor { uint8_t bInterfaceProtocol; uint8_t iInterface; } interface; + struct { + uint8_t bEndpointAddress; + uint8_t bmAttributes; + uint8_t wMaxPacketSize_lo; + uint8_t wMaxPacketSize_hi; + uint8_t bInterval; + uint8_t bRefresh; /* only audio ep */ + uint8_t bSynchAddress; /* only audio ep */ + } endpoint; } u; } QEMU_PACKED USBDescriptor; From 96dd9aac37d30f3425088f81523942e67b2d03ac Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 29 Mar 2012 16:06:28 +0200 Subject: [PATCH 18/21] usb-host: rewrite usb_linux_update_endp_table This patch carries a complete rewrite of the usb descriptor parser. Changes / improvements: * We are using the USBDescriptor struct instead of hard-coded offsets now to access descriptor data. * (debug) printfs are all gone, tracepoints have been added instead. * We don't try (and fail) to skip over unneeded descriptors. We parse them all one by one. We keep track of which configuration, interface and altsetting we are looking at and use this information to figure which desciptors are in use and which we can ignore. * On parse errors we clear all endpoint information, which will disallow any communication with the device, except control endpoint messages. This makes sure we don't end up with a silly device state where half of the endpoints got enabled and the other half was left disabled. * Some sanity checks have been added. The new parser is more robust and also leaves complete device information in the trace log if you enable the ush_host_parse_* tracepoints. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 200 +++++++++++++++++++++++--------------------- trace-events | 6 ++ 2 files changed, 110 insertions(+), 96 deletions(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index a382f0ad98..061a1b7825 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -42,6 +42,7 @@ #include #include #include "hw/usb.h" +#include "hw/usb/desc.h" /* We redefine it to avoid version problems */ struct usb_ctrltransfer { @@ -1108,121 +1109,128 @@ static int usb_host_handle_control(USBDevice *dev, USBPacket *p, return USB_RET_ASYNC; } -static uint8_t usb_linux_get_alt_setting(USBHostDevice *s, - uint8_t configuration, uint8_t interface) -{ - char device_name[64], line[1024]; - int alt_setting; - - sprintf(device_name, "%d-%s:%d.%d", s->bus_num, s->port, - (int)configuration, (int)interface); - - if (!usb_host_read_file(line, sizeof(line), "bAlternateSetting", - device_name)) { - /* Assume alt 0 on error */ - return 0; - } - if (sscanf(line, "%d", &alt_setting) != 1) { - /* Assume alt 0 on error */ - return 0; - } - return alt_setting; -} - /* returns 1 on problem encountered or 0 for success */ static int usb_linux_update_endp_table(USBHostDevice *s) { - uint8_t *descriptors; - uint8_t devep, type, alt_interface; - uint16_t raw; - int interface, length, i, ep, pid; + static const char *tname[] = { + [USB_ENDPOINT_XFER_CONTROL] = "control", + [USB_ENDPOINT_XFER_ISOC] = "isoc", + [USB_ENDPOINT_XFER_BULK] = "bulk", + [USB_ENDPOINT_XFER_INT] = "int", + }; + uint8_t devep, type; + uint16_t mps, v, p; + int ep, pid; + unsigned int i, configuration = -1, interface = -1, altsetting = -1; struct endp_data *epd; + USBDescriptor *d; + bool active = false; usb_ep_init(&s->dev); - if (s->dev.configuration == 0) { - /* not configured yet -- leave all endpoints disabled */ - return 0; - } - - /* get the desired configuration, interface, and endpoint descriptors - * from device description */ - descriptors = &s->descr[18]; - length = s->descr_len - 18; - i = 0; - - while (i < length) { - if (descriptors[i + 1] != USB_DT_CONFIG) { - fprintf(stderr, "invalid descriptor data\n"); - return 1; - } else if (descriptors[i + 5] != s->dev.configuration) { - DPRINTF("not requested configuration %d\n", s->dev.configuration); - i += (descriptors[i + 3] << 8) + descriptors[i + 2]; - continue; - } - i += descriptors[i]; - - if (descriptors[i + 1] != USB_DT_INTERFACE || - (descriptors[i + 1] == USB_DT_INTERFACE && - descriptors[i + 4] == 0)) { - i += descriptors[i]; - continue; - } - - interface = descriptors[i + 2]; - alt_interface = usb_linux_get_alt_setting(s, s->dev.configuration, - interface); - - /* the current interface descriptor is the active interface - * and has endpoints */ - if (descriptors[i + 3] != alt_interface) { - i += descriptors[i]; - continue; - } - - /* advance to the endpoints */ - while (i < length && descriptors[i +1] != USB_DT_ENDPOINT) { - i += descriptors[i]; - } - - if (i >= length) + for (i = 0;; i += d->bLength) { + if (i+2 >= s->descr_len) { break; - - while (i < length) { - if (descriptors[i + 1] != USB_DT_ENDPOINT) { - break; + } + d = (void *)(s->descr + i); + if (d->bLength < 2) { + trace_usb_host_parse_error(s->bus_num, s->addr, + "descriptor too short"); + goto error; + } + if (i + d->bLength > s->descr_len) { + trace_usb_host_parse_error(s->bus_num, s->addr, + "descriptor too long"); + goto error; + } + switch (d->bDescriptorType) { + case 0: + trace_usb_host_parse_error(s->bus_num, s->addr, + "invalid descriptor type"); + goto error; + case USB_DT_DEVICE: + if (d->bLength < 0x12) { + trace_usb_host_parse_error(s->bus_num, s->addr, + "device descriptor too short"); + goto error; } - - devep = descriptors[i + 2]; + v = (d->u.device.idVendor_hi << 8) | d->u.device.idVendor_lo; + p = (d->u.device.idProduct_hi << 8) | d->u.device.idProduct_lo; + trace_usb_host_parse_device(s->bus_num, s->addr, v, p); + break; + case USB_DT_CONFIG: + if (d->bLength < 0x09) { + trace_usb_host_parse_error(s->bus_num, s->addr, + "config descriptor too short"); + goto error; + } + configuration = d->u.config.bConfigurationValue; + active = (configuration == s->dev.configuration); + trace_usb_host_parse_config(s->bus_num, s->addr, + configuration, active); + break; + case USB_DT_INTERFACE: + if (d->bLength < 0x09) { + trace_usb_host_parse_error(s->bus_num, s->addr, + "interface descriptor too short"); + goto error; + } + interface = d->u.interface.bInterfaceNumber; + altsetting = d->u.interface.bAlternateSetting; + active = (configuration == s->dev.configuration) && + (altsetting == s->dev.altsetting[interface]); + trace_usb_host_parse_interface(s->bus_num, s->addr, + interface, altsetting, active); + break; + case USB_DT_ENDPOINT: + if (d->bLength < 0x07) { + trace_usb_host_parse_error(s->bus_num, s->addr, + "endpoint descriptor too short"); + goto error; + } + devep = d->u.endpoint.bEndpointAddress; pid = (devep & USB_DIR_IN) ? USB_TOKEN_IN : USB_TOKEN_OUT; ep = devep & 0xf; if (ep == 0) { - fprintf(stderr, "usb-linux: invalid ep descriptor, ep == 0\n"); - return 1; + trace_usb_host_parse_error(s->bus_num, s->addr, + "invalid endpoint address"); + goto error; } - type = descriptors[i + 3] & 0x3; - raw = descriptors[i + 4] + (descriptors[i + 5] << 8); - usb_ep_set_max_packet_size(&s->dev, pid, ep, raw); - assert(usb_ep_get_type(&s->dev, pid, ep) == - USB_ENDPOINT_XFER_INVALID); - usb_ep_set_type(&s->dev, pid, ep, type); - usb_ep_set_ifnum(&s->dev, pid, ep, interface); - if ((s->options & (1 << USB_HOST_OPT_PIPELINE)) && - (type == USB_ENDPOINT_XFER_BULK)) { - usb_ep_set_pipeline(&s->dev, pid, ep, true); + type = d->u.endpoint.bmAttributes & 0x3; + mps = d->u.endpoint.wMaxPacketSize_lo | + (d->u.endpoint.wMaxPacketSize_hi << 8); + trace_usb_host_parse_endpoint(s->bus_num, s->addr, ep, + (devep & USB_DIR_IN) ? "in" : "out", + tname[type], active); + + if (active) { + usb_ep_set_max_packet_size(&s->dev, pid, ep, mps); + assert(usb_ep_get_type(&s->dev, pid, ep) == + USB_ENDPOINT_XFER_INVALID); + usb_ep_set_type(&s->dev, pid, ep, type); + usb_ep_set_ifnum(&s->dev, pid, ep, interface); + if ((s->options & (1 << USB_HOST_OPT_PIPELINE)) && + (type == USB_ENDPOINT_XFER_BULK)) { + usb_ep_set_pipeline(&s->dev, pid, ep, true); + } + + epd = get_endp(s, pid, ep); + epd->halted = 0; } - epd = get_endp(s, pid, ep); - epd->halted = 0; - - i += descriptors[i]; + break; + default: + trace_usb_host_parse_unknown(s->bus_num, s->addr, + d->bLength, d->bDescriptorType); + break; } } -#ifdef DEBUG - usb_ep_dump(&s->dev); -#endif return 0; + +error: + usb_ep_init(&s->dev); + return 1; } /* diff --git a/trace-events b/trace-events index e7ce5b2cb3..d2e787962f 100644 --- a/trace-events +++ b/trace-events @@ -337,6 +337,12 @@ usb_host_reset(int bus, int addr) "dev %d:%d" usb_host_auto_scan_enabled(void) usb_host_auto_scan_disabled(void) usb_host_claim_port(int bus, int hub, int port) "bus %d, hub addr %d, port %d" +usb_host_parse_device(int bus, int addr, int vendor, int product) "dev %d:%d, id %04x:%04x" +usb_host_parse_config(int bus, int addr, int value, int active) "dev %d:%d, value %d, active %d" +usb_host_parse_interface(int bus, int addr, int num, int alt, int active) "dev %d:%d, num %d, alt %d, active %d" +usb_host_parse_endpoint(int bus, int addr, int ep, const char *dir, const char *type, int active) "dev %d:%d, ep %d, %s, %s, active %d" +usb_host_parse_unknown(int bus, int addr, int len, int type) "dev %d:%d, len %d, type %d" +usb_host_parse_error(int bus, int addr, const char *errmsg) "dev %d:%d, msg %s" # hw/scsi-bus.c scsi_req_alloc(int target, int lun, int tag) "target %d lun %d tag %d" From 8e24283b2687e1d58d5f6a4872198c29e8a45d00 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 30 Mar 2012 09:53:53 +0200 Subject: [PATCH 19/21] usb-ehci: Drop unused sofv value The sofv value only ever gets a value assigned and is never used (read) anywhere, so we can just drop it. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index be02308242..e12f09870c 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -403,7 +403,6 @@ struct EHCIState { /* * Internal states, shadow registers, etc */ - uint32_t sofv; QEMUTimer *frame_timer; int attach_poll_counter; int astate; // Current state in asynchronous schedule @@ -1102,10 +1101,6 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val) val &= USBINTR_MASK; break; - case FRINDEX: - s->sofv = val >> 3; - break; - case CONFIGFLAG: val &= 0x1; if (val) { @@ -2157,9 +2152,6 @@ static void ehci_frame_timer(void *opaque) ehci_set_interrupt(ehci, USBSTS_FLR); ehci->frindex = 0; } - - ehci->sofv = (ehci->frindex - 1) >> 3; - ehci->sofv &= 0x000003ff; } if (frames - i > ehci->maxframes) { From 714f9db06c209fd42d67e6dffd4f7fd932b51b65 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 30 Mar 2012 09:53:54 +0200 Subject: [PATCH 20/21] usb-redir: Notify our peer when we reject a device due to a speed mismatch Also cleanup (reset) our device state when we reject a device due to a speed mismatch. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/redirect.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 4288324110..94ab4632ca 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -836,7 +836,13 @@ static void usbredir_do_attach(void *opaque) { USBRedirDevice *dev = opaque; - usb_device_attach(&dev->dev); + if (usb_device_attach(&dev->dev) != 0) { + usbredir_device_disconnect(dev); + if (usbredirparser_peer_has_cap(dev->parser, usb_redir_cap_filter)) { + usbredirparser_send_filter_reject(dev->parser); + usbredirparser_do_write(dev->parser); + } + } } /* From c7020c974073ba9c0110d45361720a29ff6b2f59 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 30 Mar 2012 13:20:21 +0200 Subject: [PATCH 21/21] usb-ehci: drop assert() Not sure what the purpose of the assert() was, in any case it is bogous. We can arrive there if transfer descriptors passed to us from the guest failed to pass sanity checks, i.e. it is guest-triggerable. We deal with that case by resetting the host controller. Everything is ok, no need to throw a core dump here. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index e12f09870c..23631a47c9 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2009,7 +2009,6 @@ static void ehci_advance_state(EHCIState *ehci, fprintf(stderr, "processing error - resetting ehci HC\n"); ehci_reset(ehci); again = 0; - assert(0); } } while (again);