* [Qemu-devel] [PATCH 1/2] uhci: Add a uhci_handle_td_error() helper function
2012-10-31 11:54 [Qemu-devel] [PATCH 0/2] uhci: Don't crash on device disconnect Hans de Goede
@ 2012-10-31 11:54 ` Hans de Goede
2012-10-31 11:54 ` [Qemu-devel] [PATCH 2/2] uhci: Don't crash on device disconnect Hans de Goede
1 sibling, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2012-10-31 11:54 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Hans de Goede, qemu-devel
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
hw/usb/hcd-uhci.c | 90 +++++++++++++++++++++++++++++--------------------------
1 file changed, 47 insertions(+), 43 deletions(-)
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index b6b972f..99ed063 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -718,9 +718,52 @@ static void uhci_read_td(UHCIState *s, UHCI_TD *td, uint32_t link)
le32_to_cpus(&td->buffer);
}
+static int uhci_handle_td_error(UHCIState *s, UHCI_TD *td, uint32_t td_addr,
+ int status, uint32_t *int_mask)
+{
+ uint32_t queue_token = uhci_queue_token(td);
+ int ret;
+
+ switch (status) {
+ case USB_RET_NAK:
+ td->ctrl |= TD_CTRL_NAK;
+ return TD_RESULT_NEXT_QH;
+
+ case USB_RET_STALL:
+ td->ctrl |= TD_CTRL_STALL;
+ trace_usb_uhci_packet_complete_stall(queue_token, td_addr);
+ ret = TD_RESULT_NEXT_QH;
+ break;
+
+ case USB_RET_BABBLE:
+ td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
+ /* frame interrupted */
+ trace_usb_uhci_packet_complete_babble(queue_token, td_addr);
+ ret = TD_RESULT_STOP_FRAME;
+ break;
+
+ case USB_RET_IOERROR:
+ case USB_RET_NODEV:
+ default:
+ td->ctrl |= TD_CTRL_TIMEOUT;
+ td->ctrl &= ~(3 << TD_CTRL_ERROR_SHIFT);
+ trace_usb_uhci_packet_complete_error(queue_token, td_addr);
+ ret = TD_RESULT_NEXT_QH;
+ break;
+ }
+
+ td->ctrl &= ~TD_CTRL_ACTIVE;
+ s->status |= UHCI_STS_USBERR;
+ if (td->ctrl & TD_CTRL_IOC) {
+ *int_mask |= 0x01;
+ }
+ uhci_update_irq(s);
+ return ret;
+}
+
static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_t *int_mask)
{
- int len = 0, max_len, err, ret;
+ int len = 0, max_len, ret;
uint8_t pid;
max_len = ((td->token >> 21) + 1) & 0x7ff;
@@ -731,8 +774,9 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_
if (td->ctrl & TD_CTRL_IOS)
td->ctrl &= ~TD_CTRL_ACTIVE;
- if (ret < 0)
- goto out;
+ if (ret < 0) {
+ return uhci_handle_td_error(s, td, async->td_addr, ret, int_mask);
+ }
len = async->packet.result;
td->ctrl = (td->ctrl & ~0x7ff) | ((len - 1) & 0x7ff);
@@ -758,46 +802,6 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_
trace_usb_uhci_packet_complete_success(async->queue->token,
async->td_addr);
return TD_RESULT_COMPLETE;
-
-out:
- switch(ret) {
- case USB_RET_NAK:
- td->ctrl |= TD_CTRL_NAK;
- return TD_RESULT_NEXT_QH;
-
- case USB_RET_STALL:
- td->ctrl |= TD_CTRL_STALL;
- trace_usb_uhci_packet_complete_stall(async->queue->token,
- async->td_addr);
- err = TD_RESULT_NEXT_QH;
- break;
-
- case USB_RET_BABBLE:
- td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
- /* frame interrupted */
- trace_usb_uhci_packet_complete_babble(async->queue->token,
- async->td_addr);
- err = TD_RESULT_STOP_FRAME;
- break;
-
- case USB_RET_IOERROR:
- case USB_RET_NODEV:
- default:
- td->ctrl |= TD_CTRL_TIMEOUT;
- td->ctrl &= ~(3 << TD_CTRL_ERROR_SHIFT);
- trace_usb_uhci_packet_complete_error(async->queue->token,
- async->td_addr);
- err = TD_RESULT_NEXT_QH;
- break;
- }
-
- td->ctrl &= ~TD_CTRL_ACTIVE;
- s->status |= UHCI_STS_USBERR;
- if (td->ctrl & TD_CTRL_IOC) {
- *int_mask |= 0x01;
- }
- uhci_update_irq(s);
- return err;
}
static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
--
1.7.12.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [Qemu-devel] [PATCH 2/2] uhci: Don't crash on device disconnect
2012-10-31 11:54 [Qemu-devel] [PATCH 0/2] uhci: Don't crash on device disconnect Hans de Goede
2012-10-31 11:54 ` [Qemu-devel] [PATCH 1/2] uhci: Add a uhci_handle_td_error() helper function Hans de Goede
@ 2012-10-31 11:54 ` Hans de Goede
1 sibling, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2012-10-31 11:54 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Hans de Goede, qemu-devel
My recent uhci cleanup series has introduced a regression, where
qemu sometimes crashes on a device disconnect. The problem is that
the uhci code never checked for a device not / no longer existing, instead
it was relying on usb_handle_packet accepting a NULL device.
But since we now pass usb_handle_packet q->ep->dev, rather then just
a local dev variable, we crash as q->ep == NULL due to the device no longer
existing.
This patch fixes this. Note that this patch also improves over
the old behavior were we would:
1) create a queue for the device
2) create an async for the packet
3) have usb_handle_packet fail
4) destroy the async
5) wait for the queue to be idle for 32 frames
6) destroy the queue
Which was rather sub-optimal.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
hw/usb/hcd-uhci.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 99ed063..1ef1db9 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -879,6 +879,11 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
if (q == NULL) {
USBDevice *dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
USBEndpoint *ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
+
+ if (ep == NULL) {
+ return uhci_handle_td_error(s, td, td_addr, USB_RET_NODEV,
+ int_mask);
+ }
q = uhci_queue_new(s, qh_addr, td, ep);
}
async = uhci_async_alloc(q, td_addr);
--
1.7.12.1
^ permalink raw reply related [flat|nested] 3+ messages in thread