From 84d04e216279b5388c66c00fb00ed591da894357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Sun, 20 Dec 2015 00:23:49 +0100 Subject: [PATCH 1/7] ohci: split reset method in 3 parts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three parts are: - root hub reset (ohci_roothub_reset) - host controller soft reset (ohci_soft_reset) - host controller hard reset (ohci_hard_reset) Signed-off-by: Hervé Poussineau Tested-by: Mark Cave-Ayland Message-id: 1450567431-31795-2-git-send-email-hpoussin@reactos.org Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 64 +++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 7d65818064..0661804abb 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -439,15 +439,37 @@ static void ohci_stop_endpoints(OHCIState *ohci) } } -/* Reset the controller */ -static void ohci_reset(void *opaque) +static void ohci_roothub_reset(OHCIState *ohci) { - OHCIState *ohci = opaque; OHCIPort *port; int i; ohci_bus_stop(ohci); - ohci->ctl = 0; + ohci->rhdesc_a = OHCI_RHA_NPS | ohci->num_ports; + ohci->rhdesc_b = 0x0; /* Impl. specific */ + ohci->rhstatus = 0; + + for (i = 0; i < ohci->num_ports; i++) { + port = &ohci->rhport[i]; + port->ctrl = 0; + if (port->port.dev && port->port.dev->attached) { + usb_port_reset(&port->port); + } + } + if (ohci->async_td) { + usb_cancel_packet(&ohci->usb_packet); + ohci->async_td = 0; + } + ohci_stop_endpoints(ohci); +} + +/* Reset the controller */ +static void ohci_soft_reset(OHCIState *ohci) +{ + trace_usb_ohci_reset(ohci->name); + + ohci_bus_stop(ohci); + ohci->ctl = (ohci->ctl & OHCI_CTL_IR) | OHCI_USB_SUSPEND; ohci->old_ctl = 0; ohci->status = 0; ohci->intr_status = 0; @@ -470,25 +492,13 @@ static void ohci_reset(void *opaque) ohci->frame_number = 0; ohci->pstart = 0; ohci->lst = OHCI_LS_THRESH; +} - ohci->rhdesc_a = OHCI_RHA_NPS | ohci->num_ports; - ohci->rhdesc_b = 0x0; /* Impl. specific */ - ohci->rhstatus = 0; - - for (i = 0; i < ohci->num_ports; i++) - { - port = &ohci->rhport[i]; - port->ctrl = 0; - if (port->port.dev && port->port.dev->attached) { - usb_port_reset(&port->port); - } - } - if (ohci->async_td) { - usb_cancel_packet(&ohci->usb_packet); - ohci->async_td = 0; - } - ohci_stop_endpoints(ohci); - trace_usb_ohci_reset(ohci->name); +static void ohci_hard_reset(OHCIState *ohci) +{ + ohci_soft_reset(ohci); + ohci->ctl = 0; + ohci_roothub_reset(ohci); } /* Get an array of dwords from main memory */ @@ -1441,7 +1451,7 @@ static void ohci_set_ctl(OHCIState *ohci, uint32_t val) trace_usb_ohci_resume(ohci->name); break; case OHCI_USB_RESET: - ohci_reset(ohci); + ohci_hard_reset(ohci); break; } } @@ -1704,7 +1714,7 @@ static void ohci_mem_write(void *opaque, ohci->status |= val; if (ohci->status & OHCI_STATUS_HCR) - ohci_reset(ohci); + ohci_hard_reset(ohci); break; case 3: /* HcInterruptStatus */ @@ -1783,7 +1793,7 @@ static void ohci_mem_write(void *opaque, case 25: /* HcHReset */ ohci->hreset = val & ~OHCI_HRESET_FSBIR; if (val & OHCI_HRESET_FSBIR) - ohci_reset(ohci); + ohci_hard_reset(ohci); break; case 26: /* HcHInterruptEnable */ @@ -1960,7 +1970,7 @@ static void usb_ohci_reset_pci(DeviceState *d) OHCIPCIState *ohci = PCI_OHCI(dev); OHCIState *s = &ohci->state; - ohci_reset(s); + ohci_hard_reset(s); } #define TYPE_SYSBUS_OHCI "sysbus-ohci" @@ -1993,7 +2003,7 @@ static void usb_ohci_reset_sysbus(DeviceState *dev) OHCISysBusState *s = SYSBUS_OHCI(dev); OHCIState *ohci = &s->ohci; - ohci_reset(ohci); + ohci_hard_reset(ohci); } static Property ohci_pci_properties[] = { From 7d938fd14b87e1d8db2de5ea1359d6d11e366a56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Sun, 20 Dec 2015 00:23:50 +0100 Subject: [PATCH 2/7] ohci: fix Host Controller USBRESET MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Specification says that, when entering this state, "the contents of the registers (except Root Hub registers) are preserved by the HC. [...] The Root Hub is being reset, which causes the Root Hub's downstream ports to be reset and possibly powered off." Signed-off-by: Hervé Poussineau Tested-by: Mark Cave-Ayland Message-id: 1450567431-31795-3-git-send-email-hpoussin@reactos.org Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 0661804abb..24c62b4258 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1451,7 +1451,7 @@ static void ohci_set_ctl(OHCIState *ohci, uint32_t val) trace_usb_ohci_resume(ohci->name); break; case OHCI_USB_RESET: - ohci_hard_reset(ohci); + ohci_roothub_reset(ohci); break; } } From 0922c3f6064ebe4c65fd1190b014a742083b5906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Sun, 20 Dec 2015 00:23:51 +0100 Subject: [PATCH 3/7] ohci: fix command HostControllerReset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Specification says that: "This bit is set by HCD to initiate a software reset of HC." Signed-off-by: Hervé Poussineau Tested-by: Mark Cave-Ayland Message-id: 1450567431-31795-4-git-send-email-hpoussin@reactos.org Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 24c62b4258..d225ebbbe3 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1714,7 +1714,7 @@ static void ohci_mem_write(void *opaque, ohci->status |= val; if (ohci->status & OHCI_STATUS_HCR) - ohci_hard_reset(ohci); + ohci_soft_reset(ohci); break; case 3: /* HcInterruptStatus */ From c22d5dcd7a8ee8316dfbad66d13abb6d661c86b9 Mon Sep 17 00:00:00 2001 From: Bandan Das Date: Wed, 23 Dec 2015 14:39:46 +0530 Subject: [PATCH 4/7] usb-mtp: use safe variant when cleaning events list usb_mtp_inotify_cleanup uses QLIST_FOREACH to pick events from a list and free them which is incorrect. Use QLIST_FOREACH_SAFE instead. Signed-off-by: Bandan Das Message-id: 1450861787-16213-2-git-send-email-bsd@redhat.com Signed-off-by: Gerd Hoffmann --- hw/usb/dev-mtp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c index af056c7df9..db1fd59ccf 100644 --- a/hw/usb/dev-mtp.c +++ b/hw/usb/dev-mtp.c @@ -556,7 +556,7 @@ static int usb_mtp_inotify_init(MTPState *s) static void usb_mtp_inotify_cleanup(MTPState *s) { - MTPMonEntry *e; + MTPMonEntry *e, *p; if (!s->inotifyfd) { return; @@ -565,7 +565,7 @@ static void usb_mtp_inotify_cleanup(MTPState *s) qemu_set_fd_handler(s->inotifyfd, NULL, NULL, s); close(s->inotifyfd); - QTAILQ_FOREACH(e, &s->events, next) { + QTAILQ_FOREACH_SAFE(e, &s->events, next, p) { QTAILQ_REMOVE(&s->events, e, next); g_free(e); } From ec93e158b1930d6c6db22e3c0a655337ae221034 Mon Sep 17 00:00:00 2001 From: Bandan Das Date: Wed, 23 Dec 2015 14:39:47 +0530 Subject: [PATCH 5/7] usb-mtp: fix call to trace function trace_usb_mtp_inotify_event() was being called after the object was being freed. Signed-off-by: Bandan Das Message-id: 1450861787-16213-3-git-send-email-bsd@redhat.com Signed-off-by: Gerd Hoffmann --- hw/usb/dev-mtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c index db1fd59ccf..4177a87ea2 100644 --- a/hw/usb/dev-mtp.c +++ b/hw/usb/dev-mtp.c @@ -502,9 +502,9 @@ static void inotify_watchfn(void *arg) entry = g_new0(MTPMonEntry, 1); entry->handle = o->handle; entry->event = EVT_OBJ_REMOVED; - usb_mtp_object_free(s, o); trace_usb_mtp_inotify_event(s->dev.addr, o->path, event->mask, "Obj Deleted"); + usb_mtp_object_free(s, o); break; case IN_MODIFY: From fd0a10cd20a1c5ae829be32f3364dae88f435c4e Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Wed, 6 Jan 2016 20:45:24 +0100 Subject: [PATCH 6/7] ohci: delay first SOF interrupt On overcommitted CPU, kernel can be so slow that an interrupt can be triggered by the device whereas the driver is not ready to receive it. This drives us into an infinite loop. This does not happen on real hardware because real hardware never send interrupt immediately after the controller has been moved to OPERATION state. This patch tries to delay the first SOF interrupt to let driver exits from the critical section (which is not protected against interrupts...) Some details: - ohci_irq(): the OHCI interrupt handler, acknowledges the SOF IRQ only if the state of the driver (rh_state) is OHCI_STATE_RUNNING. So if this interrupt happens and the driver is not in this state, the function is called again and again, moving the system to a CPU starvation. - ohci_rh_resume(): the driver re-enables operation with OHCI_USB_OPER. In QEMU this start the SOF timer and QEMU starts to send IRQs. As the driver is not in OHCI_STATE_RUNNING and not protected against IRQ, the ohci_irq() can be called and the driver never moved to OHCI_STATE_RUNNING. Suggested-by: Gerd Hoffmann Signed-off-by: Laurent Vivier Reviewed-by: Thomas Huth Message-id: 1452109525-32150-2-git-send-email-lvivier@redhat.com Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index d225ebbbe3..ff5658e25e 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1241,11 +1241,16 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion) return active; } -/* Generate a SOF event, and set a timer for EOF */ -static void ohci_sof(OHCIState *ohci) +/* set a timer for EOF */ +static void ohci_eof_timer(OHCIState *ohci) { ohci->sof_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); timer_mod(ohci->eof_timer, ohci->sof_time + usb_frame_time); +} +/* Set a timer for EOF and generate a SOF event */ +static void ohci_sof(OHCIState *ohci) +{ + ohci_eof_timer(ohci); ohci_set_interrupt(ohci, OHCI_INTR_SF); } @@ -1353,7 +1358,12 @@ static int ohci_bus_start(OHCIState *ohci) trace_usb_ohci_start(ohci->name); - ohci_sof(ohci); + /* Delay the first SOF event by one frame time as + * linux driver is not ready to receive it and + * can meet some race conditions + */ + + ohci_eof_timer(ohci); return 1; } From 087462c7739869e9b888c06c06c8f1bbfd99779c Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Wed, 6 Jan 2016 20:45:25 +0100 Subject: [PATCH 7/7] ohci: clear pending SOF on suspend On overcommitted CPU, kernel can be so slow that an interrupt can be triggered by the device whereas the driver is not ready to receive it. This drives us into an infinite loop. On suspend, if a SOF interrupt is raised between the stop of the device processing and the change of the device internal state to OHCI_USB_SUSPEND (QEMU stops SOF timer on this state change), this interrupt is never acknowledged. This patch clears pending SOF interrupt on OHCI_USB_SUSPEND setting. Some details: - ohci_irq(): the OHCI interrupt handler, acknowledges the SOF IRQ only if the state of the driver (rh_state) is OHCI_STATE_RUNNING. So if this interrupt happens and the driver is not in this state, the function is called again and again, moving the system to a CPU starvation. - ohci_rh_suspend(): the function stop the operation and acknowledge pending interrupts (but doesn't disable it). Later in the function, the device is moved to OHCI_SUSPEND_STATE, and the driver to OHCI_RH_SUSPENDED. If between the moment when the interrupt is acknowledged and the moment when the device is suspended a new interrupt is raised, it will be never acknowledged because the driver is now not in OHCI_RH_RUNNING state. Signed-off-by: Laurent Vivier Reviewed-by: Thomas Huth Message-id: 1452109525-32150-3-git-send-email-lvivier@redhat.com Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ohci.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index ff5658e25e..efeaf7371f 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1456,6 +1456,9 @@ static void ohci_set_ctl(OHCIState *ohci, uint32_t val) break; case OHCI_USB_SUSPEND: ohci_bus_stop(ohci); + /* clear pending SF otherwise linux driver loops in ohci_irq() */ + ohci->intr_status &= ~OHCI_INTR_SF; + ohci_intr_update(ohci); break; case OHCI_USB_RESUME: trace_usb_ohci_resume(ohci->name);