[Qemu-devel] [PULL 0/7] usb patch queue

[Qemu-devel] [PULL 0/7] usb patch queue

[Gerd Hoffmann]

  Hi,

Here comes the usb patch queue, with a bunch of bugfixes for
mtp and ohci.

please pull,
  Gerd

The following changes since commit a7e00e2536941a6e570b45b7ab4afec4505ff67e:

  petalogix-ml605: Set the MicroBlaze CPU version to 8.10.a (2016-01-07 14:57:26 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/pull-usb-20160108-1

for you to fetch changes up to 087462c7739869e9b888c06c06c8f1bbfd99779c:

  ohci: clear pending SOF on suspend (2016-01-08 09:29:24 +0100)

----------------------------------------------------------------
usb: mtp and ohci fixes.

----------------------------------------------------------------
Bandan Das (2):
      usb-mtp: use safe variant when cleaning events list
      usb-mtp: fix call to trace function

Hervé Poussineau (3):
      ohci: split reset method in 3 parts
      ohci: fix Host Controller USBRESET
      ohci: fix command HostControllerReset

Laurent Vivier (2):
      ohci: delay first SOF interrupt
      ohci: clear pending SOF on suspend

 hw/usb/dev-mtp.c  |  6 ++--
 hw/usb/hcd-ohci.c | 85 +++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 57 insertions(+), 34 deletions(-)

[Qemu-devel] [PULL 1/7] ohci: split reset method in 3 parts

[Gerd Hoffmann]

From: Hervé Poussineau <hpoussin@reactos.org>

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 <hpoussin@reactos.org>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-id: 1450567431-31795-2-git-send-email-hpoussin@reactos.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ohci.c | 66 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 7d65818..0661804 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -439,15 +439,37 @@ static void ohci_stop_endpoints(OHCIState *ohci)
     }
 }
 
+static void ohci_roothub_reset(OHCIState *ohci)
+{
+    OHCIPort *port;
+    int i;
+
+    ohci_bus_stop(ohci);
+    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_reset(void *opaque)
+static void ohci_soft_reset(OHCIState *ohci)
 {
-    OHCIState *ohci = opaque;
-    OHCIPort *port;
-    int i;
+    trace_usb_ohci_reset(ohci->name);
 
     ohci_bus_stop(ohci);
-    ohci->ctl = 0;
+    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[] = {
-- 
1.8.3.1

[Qemu-devel] [PULL 2/7] ohci: fix Host Controller USBRESET

[Gerd Hoffmann]

From: Hervé Poussineau <hpoussin@reactos.org>

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 <hpoussin@reactos.org>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-id: 1450567431-31795-3-git-send-email-hpoussin@reactos.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 0661804..24c62b4 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;
     }
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 3/7] ohci: fix command HostControllerReset

[Gerd Hoffmann]

From: Hervé Poussineau <hpoussin@reactos.org>

Specification says that: "This bit is set by HCD to initiate a software reset of HC."

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-id: 1450567431-31795-4-git-send-email-hpoussin@reactos.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 24c62b4..d225ebb 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 */
-- 
1.8.3.1

[Qemu-devel] [PULL 4/7] usb-mtp: use safe variant when cleaning events list

[Gerd Hoffmann]

From: Bandan Das <bsd@redhat.com>

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 <bsd@redhat.com>
Message-id: 1450861787-16213-2-git-send-email-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 af056c7..db1fd59 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);
     }
-- 
1.8.3.1

[Qemu-devel] [PULL 5/7] usb-mtp: fix call to trace function

[Gerd Hoffmann]

From: Bandan Das <bsd@redhat.com>

trace_usb_mtp_inotify_event() was being called after the object was
being freed.

Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 1450861787-16213-3-git-send-email-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 db1fd59..4177a87 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:
-- 
1.8.3.1

[Qemu-devel] [PULL 6/7] ohci: delay first SOF interrupt

[Gerd Hoffmann]

From: Laurent Vivier <lvivier@redhat.com>

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 <kraxel@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 1452109525-32150-2-git-send-email-lvivier@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 d225ebb..ff5658e 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;
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 7/7] ohci: clear pending SOF on suspend

[Gerd Hoffmann]

From: Laurent Vivier <lvivier@redhat.com>

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 <lvivier@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 1452109525-32150-3-git-send-email-lvivier@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 ff5658e..efeaf73 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);
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 0/7] usb patch queue

[Peter Maydell]

On 8 January 2016 at 10:40, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here comes the usb patch queue, with a bunch of bugfixes for
> mtp and ohci.
>
> please pull,
>   Gerd
>
> The following changes since commit a7e00e2536941a6e570b45b7ab4afec4505ff67e:
>
>   petalogix-ml605: Set the MicroBlaze CPU version to 8.10.a (2016-01-07 14:57:26 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/pull-usb-20160108-1
>
> for you to fetch changes up to 087462c7739869e9b888c06c06c8f1bbfd99779c:
>
>   ohci: clear pending SOF on suspend (2016-01-08 09:29:24 +0100)
>
> ----------------------------------------------------------------
> usb: mtp and ohci fixes.
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM