From ef72f76e58107bd4096018c3db2912d28249308e Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 28 Aug 2012 14:04:27 +0100 Subject: qed: refuse unaligned zero writes with a backing file Zero writes have cluster granularity in QED. Therefore they can only be used to zero entire clusters. If the zero write request leaves sectors untouched, zeroing the entire cluster would obscure the backing file. Instead return -ENOTSUP, which is handled by block.c:bdrv_co_do_write_zeroes() and falls back to a regular write. The qemu-iotests 034 test cases covers this scenario. Signed-off-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/qed.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/block/qed.c b/block/qed.c index a02dbfd72..21cb23987 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1363,10 +1363,21 @@ static int coroutine_fn bdrv_qed_co_write_zeroes(BlockDriverState *bs, int nb_sectors) { BlockDriverAIOCB *blockacb; + BDRVQEDState *s = bs->opaque; QEDWriteZeroesCB cb = { .done = false }; QEMUIOVector qiov; struct iovec iov; + /* Refuse if there are untouched backing file sectors */ + if (bs->backing_hd) { + if (qed_offset_into_cluster(s, sector_num * BDRV_SECTOR_SIZE) != 0) { + return -ENOTSUP; + } + if (qed_offset_into_cluster(s, nb_sectors * BDRV_SECTOR_SIZE) != 0) { + return -ENOTSUP; + } + } + /* Zero writes start without an I/O buffer. If a buffer becomes necessary * then it will be allocated during request processing. */ -- cgit v1.2.3 From 571cd9dcc7f2fee59e47913365ced7781f33c2d3 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 28 Aug 2012 15:26:48 +0100 Subject: stream: complete early if end of backing file is reached It is possible to create an image that is larger than its backing file. Reading beyond the end of the backing file produces zeroes if no writes have been made to those sectors in the image file. This patch finishes streaming early when the end of the backing file is reached. Without this patch the block job hangs and continually tries to stream the first sectors beyond the end of the backing file. To reproduce the hung block job bug: $ qemu-img create -f qcow2 backing.qcow2 128M $ qemu-img create -f qcow2 -o backing_file=backing.qcow2 image.qcow2 6G $ qemu -drive if=virtio,cache=none,file=image.qcow2 (qemu) block_stream virtio0 (qemu) info block-jobs The qemu-iotests 030 streaming test still passes. Signed-off-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/stream.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/block/stream.c b/block/stream.c index 37c46525d..c4f87dd5b 100644 --- a/block/stream.c +++ b/block/stream.c @@ -122,6 +122,12 @@ wait: * known-unallocated area [sector_num, sector_num+n). */ ret = bdrv_co_is_allocated_above(bs->backing_hd, base, sector_num, n, &n); + + /* Finish early if end of backing file has been reached */ + if (ret == 0 && n == 0) { + n = end - sector_num; + } + copy = (ret == 1); } trace_stream_one_iteration(s, sector_num, n, ret); -- cgit v1.2.3 From 774a8850d708aeb6dd6de493c28b374098c1a4c3 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 28 Aug 2012 15:26:49 +0100 Subject: qemu-iotests: add backing file smaller than image test case This new test case checks that streaming completes successfully when the backing file is smaller than the image file. Signed-off-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- tests/qemu-iotests/030 | 33 +++++++++++++++++++++++++++++++++ tests/qemu-iotests/030.out | 4 ++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index f71ab8dd1..55b16f81d 100755 --- a/tests/qemu-iotests/030 +++ b/tests/qemu-iotests/030 @@ -125,6 +125,39 @@ class TestSingleDrive(ImageStreamingTestCase): result = self.vm.qmp('block-stream', device='nonexistent') self.assert_qmp(result, 'error/class', 'DeviceNotFound') + +class TestSmallerBackingFile(ImageStreamingTestCase): + backing_len = 1 * 1024 * 1024 # MB + image_len = 2 * backing_len + + def setUp(self): + self.create_image(backing_img, self.backing_len) + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img, str(self.image_len)) + self.vm = iotests.VM().add_drive(test_img) + self.vm.launch() + + # If this hangs, then you are missing a fix to complete streaming when the + # end of the backing file is reached. + def test_stream(self): + self.assert_no_active_streams() + + result = self.vm.qmp('block-stream', device='drive0') + self.assert_qmp(result, 'return', {}) + + completed = False + while not completed: + for event in self.vm.get_qmp_events(wait=True): + if event['event'] == 'BLOCK_JOB_COMPLETED': + self.assert_qmp(event, 'data/type', 'stream') + self.assert_qmp(event, 'data/device', 'drive0') + self.assert_qmp(event, 'data/offset', self.image_len) + self.assert_qmp(event, 'data/len', self.image_len) + completed = True + + self.assert_no_active_streams() + self.vm.shutdown() + + class TestStreamStop(ImageStreamingTestCase): image_len = 8 * 1024 * 1024 * 1024 # GB diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out index 3f8a935a0..2f7d3902f 100644 --- a/tests/qemu-iotests/030.out +++ b/tests/qemu-iotests/030.out @@ -1,5 +1,5 @@ -...... +....... ---------------------------------------------------------------------- -Ran 6 tests +Ran 7 tests OK -- cgit v1.2.3 From da9fbe76a0639c529f9678aabcc052dfe4cd9cc4 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 11 Jul 2012 12:21:23 +0200 Subject: fix info qtree indention Without the patch bus properties are are not in line with the other properties: [ ... ] dev: fw_cfg, id "" ctl_iobase = 0x510 data_iobase = 0x511 irq 0 mmio ffffffffffffffff/0000000000000002 mmio ffffffffffffffff/0000000000000001 [ ... ] With the patch applied everything is lined up properly: [ ... ] dev: fw_cfg, id "" ctl_iobase = 0x510 data_iobase = 0x511 irq 0 mmio ffffffffffffffff/0000000000000002 mmio ffffffffffffffff/0000000000000001 [ ... ] Needed to make the autotest qtree parser happy. Signed-off-by: Gerd Hoffmann --- hw/qdev-monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c index 018b38678..33b7f79a9 100644 --- a/hw/qdev-monitor.c +++ b/hw/qdev-monitor.c @@ -543,7 +543,7 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent) qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent); class = object_class_get_parent(class); } while (class != object_class_by_name(TYPE_DEVICE)); - bus_print_dev(dev->parent_bus, mon, dev, indent + 2); + bus_print_dev(dev->parent_bus, mon, dev, indent); QLIST_FOREACH(child, &dev->child_bus, sibling) { qbus_print(mon, child, indent); } -- cgit v1.2.3 From 0132b4b6595423c92f54d7e0b172b5d73aaa8375 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 17 Aug 2012 15:24:49 +0200 Subject: usb: Halt ep queue en cancel pending packets on a packet error For controllers which queue up more then 1 packet at a time, we must halt the ep queue, and inside the controller code cancel all pending packets on an error. There are multiple reasons for this: 1) Guests expect the controllers to halt ep queues on error, so that they get the opportunity to cancel transfers which the scheduled after the failing one, before processing continues 2) Not cancelling queued up packets after a failed transfer also messes up the controller state machine, in the case of EHCI causing the following assert to trigger: "assert(p->qtdaddr == q->qtdaddr)" at hcd-ehci.c:2075 3) For bulk endpoints with pipelining enabled (redirection to a real USB device), we must cancel all the transfers after this a failed one so that: a) If they've completed already, they are not processed further causing more stalls to be reported, originating from the same failed transfer b) If still in flight, they are cancelled before the guest does a clear stall, otherwise the guest and device can loose sync! Note this patch only touches the ehci and uhci controller changes, since AFAIK no other controllers actually queue up multiple transfer. If I'm wrong on this other controllers need to be updated too! Also note that this patch was heavily tested with the ehci code, where I had a reproducer for a device causing a transfer to fail. The uhci code is not tested with actually failing transfers and could do with a thorough review! Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb.h | 1 + hw/usb/core.c | 35 ++++++++++++++++++++++++++++------- hw/usb/hcd-ehci.c | 13 +++++++++++++ hw/usb/hcd-uhci.c | 16 ++++++++++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/hw/usb.h b/hw/usb.h index 432ccae57..e5744773b 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -179,6 +179,7 @@ struct USBEndpoint { uint8_t ifnum; int max_packet_size; bool pipeline; + bool halted; USBDevice *dev; QTAILQ_HEAD(, USBPacket) queue; }; diff --git a/hw/usb/core.c b/hw/usb/core.c index c7e5bc047..28b840e52 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -382,12 +382,23 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p) usb_packet_check_state(p, USB_PACKET_SETUP); assert(p->ep != NULL); + /* Submitting a new packet clears halt */ + if (p->ep->halted) { + assert(QTAILQ_EMPTY(&p->ep->queue)); + p->ep->halted = false; + } + if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline) { ret = usb_process_one(p); if (ret == USB_RET_ASYNC) { usb_packet_set_state(p, USB_PACKET_ASYNC); QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue); } else { + /* + * When pipelining is enabled usb-devices must always return async, + * otherwise packets can complete out of order! + */ + assert(!p->ep->pipeline); p->result = ret; usb_packet_set_state(p, USB_PACKET_COMPLETE); } @@ -399,6 +410,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p) return ret; } +static void __usb_packet_complete(USBDevice *dev, USBPacket *p) +{ + USBEndpoint *ep = p->ep; + + assert(p->result != USB_RET_ASYNC && p->result != USB_RET_NAK); + + if (p->result < 0) { + ep->halted = true; + } + usb_packet_set_state(p, USB_PACKET_COMPLETE); + QTAILQ_REMOVE(&ep->queue, p, queue); + dev->port->ops->complete(dev->port, p); +} + /* Notify the controller that an async packet is complete. This should only be called for packets previously deferred by returning USB_RET_ASYNC from handle_packet. */ @@ -409,11 +434,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p) usb_packet_check_state(p, USB_PACKET_ASYNC); assert(QTAILQ_FIRST(&ep->queue) == p); - usb_packet_set_state(p, USB_PACKET_COMPLETE); - QTAILQ_REMOVE(&ep->queue, p, queue); - dev->port->ops->complete(dev->port, p); + __usb_packet_complete(dev, p); - while (!QTAILQ_EMPTY(&ep->queue)) { + while (!ep->halted && !QTAILQ_EMPTY(&ep->queue)) { p = QTAILQ_FIRST(&ep->queue); if (p->state == USB_PACKET_ASYNC) { break; @@ -425,9 +448,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p) break; } p->result = ret; - usb_packet_set_state(p, USB_PACKET_COMPLETE); - QTAILQ_REMOVE(&ep->queue, p, queue); - dev->port->ops->complete(dev->port, p); + __usb_packet_complete(ep->dev, p); } } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 8b94b1772..8504a6a6b 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2138,6 +2138,19 @@ static int ehci_state_writeback(EHCIQueue *q) * bit is clear. */ if (q->qh.token & QTD_TOKEN_HALT) { + /* + * We should not do any further processing on a halted queue! + * This is esp. important for bulk endpoints with pipelining enabled + * (redirection to a real USB device), where we must cancel all the + * transfers after this one so that: + * 1) If they've completed already, they are not processed further + * causing more stalls, originating from the same failed transfer + * 2) If still in flight, they are cancelled before the guest does + * a clear stall, otherwise the guest and device can loose sync! + */ + while ((p = QTAILQ_FIRST(&q->packets)) != NULL) { + ehci_free_packet(p); + } ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); again = 1; } else { diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 1ace2a41d..898773414 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -748,6 +748,22 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_ return TD_RESULT_COMPLETE; out: + /* + * We should not do any further processing on a queue with errors! + * This is esp. important for bulk endpoints with pipelining enabled + * (redirection to a real USB device), where we must cancel all the + * transfers after this one so that: + * 1) If they've completed already, they are not processed further + * causing more stalls, originating from the same failed transfer + * 2) If still in flight, they are cancelled before the guest does + * a clear stall, otherwise the guest and device can loose sync! + */ + while (!QTAILQ_EMPTY(&async->queue->asyncs)) { + UHCIAsync *as = QTAILQ_FIRST(&async->queue->asyncs); + uhci_async_unlink(as); + uhci_async_cancel(as); + } + switch(ret) { case USB_RET_STALL: td->ctrl |= TD_CTRL_STALL; -- cgit v1.2.3 From e983395d30d1d5bfa0ed3ae9c028c130f7c498cc Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 23 Aug 2012 13:30:13 +0200 Subject: usb: unique packet ids This patch adds IDs to usb packets. Those IDs are (a) supposed to be unique for the lifecycle of a packet (from packet setup until the packet is either completed or canceled) and (b) stable across migration. uhci, ohci, ehci and xhci use the guest physical address of the transfer descriptor for this. musb needs a different approach because there is no transfer descriptor. But musb also doesn't support pipelining, so we have never more than one packet per endpoint in flight. So we go create an ID based on endpoint and device address. Signed-off-by: Gerd Hoffmann --- hw/usb.h | 3 ++- hw/usb/core.c | 3 ++- hw/usb/hcd-ehci.c | 9 +++++---- hw/usb/hcd-musb.c | 3 ++- hw/usb/hcd-ohci.c | 4 ++-- hw/usb/hcd-uhci.c | 4 ++-- hw/usb/hcd-xhci.c | 2 +- 7 files changed, 16 insertions(+), 12 deletions(-) diff --git a/hw/usb.h b/hw/usb.h index e5744773b..b8fceec89 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -332,6 +332,7 @@ typedef enum USBPacketState { struct USBPacket { /* Data fields for use by the driver. */ int pid; + uint64_t id; USBEndpoint *ep; QEMUIOVector iov; uint64_t parameter; /* control transfers */ @@ -344,7 +345,7 @@ struct USBPacket { void usb_packet_init(USBPacket *p); void usb_packet_set_state(USBPacket *p, USBPacketState state); void usb_packet_check_state(USBPacket *p, USBPacketState expected); -void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep); +void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id); void usb_packet_addbuf(USBPacket *p, void *ptr, size_t len); int usb_packet_map(USBPacket *p, QEMUSGList *sgl); void usb_packet_unmap(USBPacket *p, QEMUSGList *sgl); diff --git a/hw/usb/core.c b/hw/usb/core.c index 28b840e52..2da38e7fd 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -520,10 +520,11 @@ void usb_packet_set_state(USBPacket *p, USBPacketState state) p->state = state; } -void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep) +void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id) { assert(!usb_packet_is_inflight(p)); assert(p->iov.iov != NULL); + p->id = id; p->pid = pid; p->ep = ep; p->result = 0; diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 8504a6a6b..f43d690eb 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1530,7 +1530,7 @@ static int ehci_execute(EHCIPacket *p, const char *action) endp = get_field(p->queue->qh.epchar, QH_EPCHAR_EP); ep = usb_ep_get(p->queue->dev, p->pid, endp); - usb_packet_setup(&p->packet, p->pid, ep); + usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr); usb_packet_map(&p->packet, &p->sgl); trace_usb_ehci_packet_action(p->queue, p, action); @@ -1552,7 +1552,8 @@ static int ehci_execute(EHCIPacket *p, const char *action) */ static int ehci_process_itd(EHCIState *ehci, - EHCIitd *itd) + EHCIitd *itd, + uint32_t addr) { USBDevice *dev; USBEndpoint *ep; @@ -1598,7 +1599,7 @@ static int ehci_process_itd(EHCIState *ehci, dev = ehci_find_device(ehci, devaddr); ep = usb_ep_get(dev, pid, endp); if (ep->type == USB_ENDPOINT_XFER_ISOC) { - usb_packet_setup(&ehci->ipacket, pid, ep); + usb_packet_setup(&ehci->ipacket, pid, ep, addr); usb_packet_map(&ehci->ipacket, &ehci->isgl); ret = usb_handle_packet(dev, &ehci->ipacket); assert(ret != USB_RET_ASYNC); @@ -1862,7 +1863,7 @@ static int ehci_state_fetchitd(EHCIState *ehci, int async) sizeof(EHCIitd) >> 2); ehci_trace_itd(ehci, entry, &itd); - if (ehci_process_itd(ehci, &itd) != 0) { + if (ehci_process_itd(ehci, &itd, entry) != 0) { return -1; } diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c index fa9385ee4..0bb5c7b19 100644 --- a/hw/usb/hcd-musb.c +++ b/hw/usb/hcd-musb.c @@ -626,7 +626,8 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep, /* A wild guess on the FADDR semantics... */ dev = usb_find_device(&s->port, ep->faddr[idx]); uep = usb_ep_get(dev, pid, ep->type[idx] & 0xf); - usb_packet_setup(&ep->packey[dir].p, pid, uep); + usb_packet_setup(&ep->packey[dir].p, pid, uep, + (dev->addr << 16) | (uep->nr << 8) | pid); usb_packet_addbuf(&ep->packey[dir].p, ep->buf[idx], len); ep->packey[dir].ep = ep; ep->packey[dir].dir = dir; diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 844e7ed16..c36184ae4 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -812,7 +812,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, } else { dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA)); ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN)); - usb_packet_setup(&ohci->usb_packet, pid, ep); + usb_packet_setup(&ohci->usb_packet, pid, ep, addr); usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len); ret = usb_handle_packet(dev, &ohci->usb_packet); if (ret == USB_RET_ASYNC) { @@ -1011,7 +1011,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed) } dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA)); ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN)); - usb_packet_setup(&ohci->usb_packet, pid, ep); + usb_packet_setup(&ohci->usb_packet, pid, ep, addr); usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, pktlen); ret = usb_handle_packet(dev, &ohci->usb_packet); #ifdef DEBUG_PACKET diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 898773414..b0db92145 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -859,14 +859,14 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td, * for initial isochronous requests */ async->queue->valid = 32; - async->isoc = td->ctrl & TD_CTRL_IOS; + async->isoc = td->ctrl & TD_CTRL_IOS; max_len = ((td->token >> 21) + 1) & 0x7ff; pid = td->token & 0xff; dev = uhci_find_device(s, (td->token >> 8) & 0x7f); ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf); - usb_packet_setup(&async->packet, pid, ep); + usb_packet_setup(&async->packet, pid, ep, addr); qemu_sglist_add(&async->sgl, td->buffer, max_len); usb_packet_map(&async->packet, &async->sgl); diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 6c2ff024e..3eb27fadb 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1392,7 +1392,7 @@ static int xhci_setup_packet(XHCITransfer *xfer, USBDevice *dev) dir = xfer->in_xfer ? USB_TOKEN_IN : USB_TOKEN_OUT; ep = usb_ep_get(dev, dir, xfer->epid >> 1); - usb_packet_setup(&xfer->packet, dir, ep); + usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr); usb_packet_addbuf(&xfer->packet, xfer->data, xfer->data_length); DPRINTF("xhci: setup packet pid 0x%x addr %d ep %d\n", xfer->packet.pid, dev->addr, ep->nr); -- cgit v1.2.3 From 7ce86aa1aafaa65e7d3e572873bdf37bdb896f49 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 28 Aug 2012 11:50:26 +0200 Subject: ehci: Fix NULL ptr deref when unplugging an USB dev with an iso stream active Signed-off-by: Hans de Goede --- hw/usb/hcd-ehci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index f43d690eb..a3aea6deb 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1598,7 +1598,7 @@ static int ehci_process_itd(EHCIState *ehci, dev = ehci_find_device(ehci, devaddr); ep = usb_ep_get(dev, pid, endp); - if (ep->type == USB_ENDPOINT_XFER_ISOC) { + if (ep && ep->type == USB_ENDPOINT_XFER_ISOC) { usb_packet_setup(&ehci->ipacket, pid, ep, addr); usb_packet_map(&ehci->ipacket, &ehci->isgl); ret = usb_handle_packet(dev, &ehci->ipacket); -- cgit v1.2.3 From a1c3e4b839f8e7ec7f1792b8a11c63ca845aa021 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 30 Aug 2012 09:55:19 +0200 Subject: ehci: Schedule async-bh when IAAD bit gets set After the "ehci: Print a warning when a queue unexpectedly contains packets on cancel" commit. Under certain reproducable conditions I was getting the following message: "EHCI: Warning queue not empty on queue reset". After aprox. 8 hours of debugging I've finally found the cause. The Linux EHCI driver has an IAAD watchdog, to work around certain EHCI hardware sometimes not acknowledging the doorbell at all. This watchdog has a timeout of 10 ms, which is less then the time between 2 runs through the async schedule when async_stepdown is at its highest value. Thus the watchdog can trigger, after which Linux clears the IAAD bit and re-uses the QH. IOW we were not properly detecting the unlink of the qh, due to us missing (ignoring for more then 10 ms) the IAAD command, which triggered the warning. Signed-off-by: Hans de Goede --- hw/usb/hcd-ehci.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index a3aea6deb..3e1097749 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1194,6 +1194,15 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val) val &= ~USBCMD_FLS; } + if (val & USBCMD_IAAD) { + /* + * Process IAAD immediately, otherwise the Linux IAAD watchdog may + * trigger and re-use a qh without us seeing the unlink. + */ + s->async_stepdown = 0; + qemu_bh_schedule(s->async_bh); + } + if (((USBCMD_RUNSTOP | USBCMD_PSE | USBCMD_ASE) & val) != ((USBCMD_RUNSTOP | USBCMD_PSE | USBCMD_ASE) & s->usbcmd)) { if (s->pstate == EST_INACTIVE) { -- cgit v1.2.3 From 53dd6f7032ec4898ca8f95356df795a92cd27e09 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 16 Aug 2012 15:47:29 +0200 Subject: ehci: Remove unnecessary ehci_flush_qh call ehci_qh_do_overlay() already calls ehci_flush_qh() before it returns, calling it twice is useless. Signed-off-by: Hans de Goede 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 3e1097749..923a949ea 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1966,7 +1966,6 @@ static int ehci_state_fetchqtd(EHCIQueue *q) } if (p != NULL) { ehci_qh_do_overlay(q); - ehci_flush_qh(q); if (p->async == EHCI_ASYNC_INFLIGHT) { ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); } else { -- cgit v1.2.3 From 574ef17191f5ec5a3cc4782c1f59dc5eb8279654 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 17 Aug 2012 11:39:17 +0200 Subject: ehci: simplify ehci_state_executing ehci_state_executing does not need to check for p->usb_status == USB_RET_ASYNC or USB_RET_PROCERR, since ehci_execute_complete already does a similar check and will trigger an assert if either value is encountered. USB_RET_ASYNC should never be the packet status when execute_complete runs for obvious reasons, and USB_RET_PROCERR is only used by ehci_state_execute / ehci_execute not by ehci_state_executing / ehci_execute_complete. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 923a949ea..b6169ce96 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2084,19 +2084,11 @@ out: static int ehci_state_executing(EHCIQueue *q) { EHCIPacket *p = QTAILQ_FIRST(&q->packets); - int again = 0; assert(p != NULL); assert(p->qtdaddr == q->qtdaddr); ehci_execute_complete(q); - if (p->usb_status == USB_RET_ASYNC) { - goto out; - } - if (p->usb_status == USB_RET_PROCERR) { - again = -1; - goto out; - } // 4.10.3 if (!q->async) { @@ -2114,11 +2106,8 @@ static int ehci_state_executing(EHCIQueue *q) ehci_set_state(q->ehci, q->async, EST_WRITEBACK); } - again = 1; - -out: ehci_flush_qh(q); - return again; + return 1; } -- cgit v1.2.3 From c7cdca3b853eed0dd521c43098b6d07bcce24fd1 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 21 Aug 2012 13:58:40 +0200 Subject: ehci: add ehci_cancel_queue() Factor out function to cancel all packets of a queue. No behavior change. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index b6169ce96..30e5e8fa9 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -766,15 +766,27 @@ static EHCIQueue *ehci_alloc_queue(EHCIState *ehci, uint32_t addr, int async) return q; } +static void ehci_cancel_queue(EHCIQueue *q) +{ + EHCIPacket *p; + + p = QTAILQ_FIRST(&q->packets); + if (p == NULL) { + return; + } + + trace_usb_ehci_queue_action(q, "cancel"); + do { + ehci_free_packet(p); + } while ((p = QTAILQ_FIRST(&q->packets)) != NULL); +} + static void ehci_free_queue(EHCIQueue *q) { EHCIQueueHead *head = q->async ? &q->ehci->aqueues : &q->ehci->pqueues; - EHCIPacket *p; trace_usb_ehci_queue_action(q, "free"); - while ((p = QTAILQ_FIRST(&q->packets)) != NULL) { - ehci_free_packet(p); - } + ehci_cancel_queue(q); QTAILQ_REMOVE(head, q, next); g_free(q); } @@ -1796,9 +1808,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async) if (q->dev != NULL && q->dev->addr != devaddr) { if (!QTAILQ_EMPTY(&q->packets)) { /* should not happen (guest bug) */ - while ((p = QTAILQ_FIRST(&q->packets)) != NULL) { - ehci_free_packet(p); - } + ehci_cancel_queue(q); } q->dev = NULL; } @@ -1959,10 +1969,10 @@ static int ehci_state_fetchqtd(EHCIQueue *q) ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), &qtd); p = QTAILQ_FIRST(&q->packets); - while (p != NULL && p->qtdaddr != q->qtdaddr) { + if (p != NULL && p->qtdaddr != q->qtdaddr) { /* should not happen (guest bug) */ - ehci_free_packet(p); - p = QTAILQ_FIRST(&q->packets); + ehci_cancel_queue(q); + p = NULL; } if (p != NULL) { ehci_qh_do_overlay(q); -- cgit v1.2.3 From 287fd3f1dd0b2abbd69e58b402e5364b334e95bd Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 21 Aug 2012 14:03:09 +0200 Subject: ehci: handle TD deactivation of inflight packets Check the TDs of inflight packets, cancel packets in case the guest clears the active bit. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 30e5e8fa9..eca1431bf 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1816,11 +1816,6 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async) q->dev = ehci_find_device(q->ehci, devaddr); } - if (p && p->async == EHCI_ASYNC_INFLIGHT) { - /* I/O still in progress -- skip queue */ - ehci_set_state(ehci, async, EST_HORIZONTALQH); - goto out; - } if (p && p->async == EHCI_ASYNC_FINISHED) { /* I/O finished -- continue processing queue */ trace_usb_ehci_packet_action(p->queue, p, "complete"); @@ -1969,28 +1964,41 @@ static int ehci_state_fetchqtd(EHCIQueue *q) ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), &qtd); p = QTAILQ_FIRST(&q->packets); - if (p != NULL && p->qtdaddr != q->qtdaddr) { - /* should not happen (guest bug) */ - ehci_cancel_queue(q); - p = NULL; - } if (p != NULL) { - ehci_qh_do_overlay(q); + if (p->qtdaddr != q->qtdaddr || + (!NLPTR_TBIT(p->qtd.next) && (p->qtd.next != qtd.next)) || + (!NLPTR_TBIT(p->qtd.altnext) && (p->qtd.altnext != qtd.altnext)) || + p->qtd.bufptr[0] != qtd.bufptr[0]) { + /* guest bug: guest updated active QH or qTD underneath us */ + ehci_cancel_queue(q); + p = NULL; + } else { + p->qtd = qtd; + ehci_qh_do_overlay(q); + } + } + + if (!(qtd.token & QTD_TOKEN_ACTIVE)) { + if (p != NULL) { + /* transfer canceled by guest (clear active) */ + ehci_cancel_queue(q); + p = NULL; + } + ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); + again = 1; + } else if (p != NULL) { if (p->async == EHCI_ASYNC_INFLIGHT) { ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); } else { ehci_set_state(q->ehci, q->async, EST_EXECUTING); } again = 1; - } else if (qtd.token & QTD_TOKEN_ACTIVE) { + } else { p = ehci_alloc_packet(q); p->qtdaddr = q->qtdaddr; p->qtd = qtd; ehci_set_state(q->ehci, q->async, EST_EXECUTE); again = 1; - } else { - ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); - again = 1; } return again; -- cgit v1.2.3 From adf478342b11cf9f540baf1f387b669210d3bea1 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 30 Aug 2012 11:20:51 +0200 Subject: ehci: Fix interrupt endpoints no longer working One of the recent changes (likely the addition of queuing support) has broken interrupt endpoints, this patch fixes this. Signed-off-by: Hans de Goede --- hw/usb/hcd-ehci.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index eca1431bf..017342b56 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1987,10 +1987,19 @@ static int ehci_state_fetchqtd(EHCIQueue *q) ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); again = 1; } else if (p != NULL) { - if (p->async == EHCI_ASYNC_INFLIGHT) { + switch (p->async) { + case EHCI_ASYNC_NONE: + /* Previously nacked packet (likely interrupt ep) */ + ehci_set_state(q->ehci, q->async, EST_EXECUTE); + break; + case EHCI_ASYNC_INFLIGHT: + /* Unfinyshed async handled packet, go horizontal */ ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); - } else { + break; + case EHCI_ASYNC_FINISHED: + /* Should never happen, as this case is caught by fetchqh */ ehci_set_state(q->ehci, q->async, EST_EXECUTING); + break; } again = 1; } else { -- cgit v1.2.3 From 347e40ffe61b7cc8d4565be476c20acd00611669 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 31 Aug 2012 14:34:19 +0200 Subject: uas: move transfer kickoff Kick next scsi transfer from request release callback instead of command completion callback, otherwise we might get stuck in case scsi_req_unref() doesn't release the request instantly due to someone else holding a reference too. Signed-off-by: Gerd Hoffmann --- hw/usb/dev-uas.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c index b13eeba56..5a0057a36 100644 --- a/hw/usb/dev-uas.c +++ b/hw/usb/dev-uas.c @@ -424,6 +424,7 @@ static void usb_uas_scsi_free_request(SCSIBus *bus, void *priv) } QTAILQ_REMOVE(&uas->requests, req, next); g_free(req); + usb_uas_start_next_transfer(uas); } static UASRequest *usb_uas_find_request(UASDevice *uas, uint16_t tag) @@ -456,7 +457,6 @@ static void usb_uas_scsi_command_complete(SCSIRequest *r, uint32_t status, size_t resid) { UASRequest *req = r->hba_private; - UASDevice *uas = req->uas; trace_usb_uas_scsi_complete(req->uas->dev.addr, req->tag, status, resid); req->complete = true; @@ -465,7 +465,6 @@ static void usb_uas_scsi_command_complete(SCSIRequest *r, } usb_uas_queue_sense(req, status); scsi_req_unref(req->req); - usb_uas_start_next_transfer(uas); } static void usb_uas_scsi_request_cancelled(SCSIRequest *r) -- cgit v1.2.3 From 8bd6b06d7b718b3e595aab279699ef3651ce2e48 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Fri, 17 Aug 2012 15:50:44 +0200 Subject: console: Fix warning from clang (and potential crash) ccc-analyzer reports this warning: console.c:1090:29: warning: Dereference of null pointer if (active_console->cursor_timer) { ^ Function console_select allows active_console to be NULL, but would crash when accessing cursor_timer. Fix this. Reviewed-by: Jan Kiszka Signed-off-by: Stefan Weil Signed-off-by: Anthony Liguori --- console.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/console.c b/console.c index 4525cc70b..f5e881479 100644 --- a/console.c +++ b/console.c @@ -1087,7 +1087,7 @@ void console_select(unsigned int index) if (s) { DisplayState *ds = s->ds; - if (active_console->cursor_timer) { + if (active_console && active_console->cursor_timer) { qemu_del_timer(active_console->cursor_timer); } active_console = s; -- cgit v1.2.3 From 0232cd355d37e1ef938c3636a0e934da87f3bcc8 Mon Sep 17 00:00:00 2001 From: Anthony Liguori Date: Fri, 31 Aug 2012 10:50:46 -0500 Subject: Update version to 1.2.0-rc3 Signed-off-by: Anthony Liguori --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 99f6a00b1..9a03ea637 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.92 +1.1.93 -- cgit v1.2.3 From e7eee62a90c671d22d50964b7de05e3f4fd96f5f Mon Sep 17 00:00:00 2001 From: Max Filippov Date: Wed, 22 Aug 2012 22:03:35 +0400 Subject: target-xtensa: return ENOSYS for unimplemented simcalls This prevents guest from proceeding with uninitialised garbage returned from unimplemented simcalls. Signed-off-by: Max Filippov Signed-off-by: Blue Swirl --- target-xtensa/xtensa-semi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c index 1c8a19ed5..6d001c2c1 100644 --- a/target-xtensa/xtensa-semi.c +++ b/target-xtensa/xtensa-semi.c @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) default: qemu_log("%s(%d): not implemented\n", __func__, regs[2]); + regs[2] = -1; + regs[3] = ENOSYS; break; } } -- cgit v1.2.3 From de188751da8db3c77a681bf903035a0e5218c463 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 Sep 2012 17:34:32 +0200 Subject: qemu-timer: properly arm alarm timer for timers set by device initialization QEMU will hang when fed the following command-line qemu-system-mips -kernel vmlinux-2.6.32-5-4kc-malta -append "console=ttyS0" -nographic -net none The -net none is important otherwise it seems some events are generated causing the things to work. When it doesn't work, the guest hangs when measuring the CPU frequency, after the following line: [ 0.000000] NR_IRQS:256 Pressing a key on the serial port unblocks it, hinting that the problem is due to the recent elimination of the 1 second timeout in the main loop. The problem is that because init_timer_alarm sets the timer's pending flag to true, the alarm timer is never armed until after the first time through the main loop. Thus the bug started when QEMU started testing the pending flag in qemu_mod_timer (commit 1828be3, more alarm timer cleanup, 2010-03-10). But actually, it isn't true at all that a timer is pending when the alarm timer is created, and the real bug has been latent forever: the fix is to remove the bogus setting of pending flag. Reported-by: Aurelien Jarno Signed-off-by: Paolo Bonzini Reviewed-by: Jan Kiszka Tested-by: Aurelien Jarno Tested-by: Michael Tokarev Signed-off-by: Aurelien Jarno --- qemu-timer.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/qemu-timer.c b/qemu-timer.c index 5aea94e8e..c7a1551a3 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -759,11 +759,8 @@ int init_timer_alarm(void) goto fail; } - /* first event is at time 0 */ atexit(quit_timers); - t->pending = true; alarm_timer = t; - return 0; fail: -- cgit v1.2.3 From 3eea5498ca501922520b3447ba94815bfc109743 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Tue, 4 Sep 2012 10:26:09 -0500 Subject: console: bounds check whenever changing the cursor due to an escape code This is XSA-17 / CVE-2012-3515 Signed-off-by: Ian Campbell Signed-off-by: Anthony Liguori --- console.c | 57 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/console.c b/console.c index f5e881479..3b5cabba3 100644 --- a/console.c +++ b/console.c @@ -850,6 +850,26 @@ static void console_clear_xy(TextConsole *s, int x, int y) update_xy(s, x, y); } +/* set cursor, checking bounds */ +static void set_cursor(TextConsole *s, int x, int y) +{ + if (x < 0) { + x = 0; + } + if (y < 0) { + y = 0; + } + if (y >= s->height) { + y = s->height - 1; + } + if (x >= s->width) { + x = s->width - 1; + } + + s->x = x; + s->y = y; +} + static void console_putchar(TextConsole *s, int ch) { TextCell *c; @@ -921,7 +941,8 @@ static void console_putchar(TextConsole *s, int ch) s->esc_params[s->nb_esc_params] * 10 + ch - '0'; } } else { - s->nb_esc_params++; + if (s->nb_esc_params < MAX_ESC_PARAMS) + s->nb_esc_params++; if (ch == ';') break; #ifdef DEBUG_CONSOLE @@ -935,59 +956,37 @@ static void console_putchar(TextConsole *s, int ch) if (s->esc_params[0] == 0) { s->esc_params[0] = 1; } - s->y -= s->esc_params[0]; - if (s->y < 0) { - s->y = 0; - } + set_cursor(s, s->x, s->y - s->esc_params[0]); break; case 'B': /* move cursor down */ if (s->esc_params[0] == 0) { s->esc_params[0] = 1; } - s->y += s->esc_params[0]; - if (s->y >= s->height) { - s->y = s->height - 1; - } + set_cursor(s, s->x, s->y + s->esc_params[0]); break; case 'C': /* move cursor right */ if (s->esc_params[0] == 0) { s->esc_params[0] = 1; } - s->x += s->esc_params[0]; - if (s->x >= s->width) { - s->x = s->width - 1; - } + set_cursor(s, s->x + s->esc_params[0], s->y); break; case 'D': /* move cursor left */ if (s->esc_params[0] == 0) { s->esc_params[0] = 1; } - s->x -= s->esc_params[0]; - if (s->x < 0) { - s->x = 0; - } + set_cursor(s, s->x - s->esc_params[0], s->y); break; case 'G': /* move cursor to column */ - s->x = s->esc_params[0] - 1; - if (s->x < 0) { - s->x = 0; - } + set_cursor(s, s->esc_params[0] - 1, s->y); break; case 'f': case 'H': /* move cursor to row, column */ - s->x = s->esc_params[1] - 1; - if (s->x < 0) { - s->x = 0; - } - s->y = s->esc_params[0] - 1; - if (s->y < 0) { - s->y = 0; - } + set_cursor(s, s->esc_params[1] - 1, s->esc_params[0] - 1); break; case 'J': switch (s->esc_params[0]) { -- cgit v1.2.3 From 8db972cfa469b4e4afd9c65e54e796b83b5ce3a2 Mon Sep 17 00:00:00 2001 From: Anthony Liguori Date: Wed, 5 Sep 2012 07:50:01 -0500 Subject: Update version for 1.2.0 Signed-off-by: Anthony Liguori --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 9a03ea637..26aaba0e8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.93 +1.2.0 -- cgit v1.2.3