[Qemu-devel] [PULL for-1.6 0/3] usb patch queu

[Qemu-devel] [PULL for-1.6 0/3] usb patch queu

[Gerd Hoffmann]

  Hi,

Here comes the usb patch queue, with some bugfixes, for 1.6.

please pull,
  Gerd

The following changes since commit dbe49aea76a7805f05b33d548b2e543b51fa8bb1:

  Update version for 1.6.0-rc0 (2013-07-29 20:30:29 -0500)

are available in the git repository at:

  git://git.kraxel.org/qemu usb.85

for you to fetch changes up to cf66ee8e207d5c1831c7d4d8c7f2544314fc676c:

  hcd-ohci: add dma error handling (2013-07-30 10:26:19 +0200)

----------------------------------------------------------------
Alexey Kardashevskiy (1):
      hcd-ohci: add dma error handling

Gerd Hoffmann (2):
      xhci: handle USB_RET_IOERROR
      uhci: egsm fix

 hw/usb/hcd-ohci.c |  177 ++++++++++++++++++++++++++++++++++++++---------------
 hw/usb/hcd-uhci.c |    7 +++
 hw/usb/hcd-xhci.c |    1 +
 3 files changed, 134 insertions(+), 51 deletions(-)

[Qemu-devel] [PATCH 1/3] xhci: handle USB_RET_IOERROR

[Gerd Hoffmann]

https://bugzilla.redhat.com/show_bug.cgi?id=980377

Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 58f311d..7cbf813 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1741,6 +1741,7 @@ static int xhci_complete_packet(XHCITransfer *xfer)
     trace_usb_xhci_xfer_error(xfer, xfer->packet.status);
     switch (xfer->packet.status) {
     case USB_RET_NODEV:
+    case USB_RET_IOERROR:
         xfer->status = CC_USB_TRANSACTION_ERROR;
         xhci_xfer_report(xfer);
         xhci_stall_ep(xfer);
-- 
1.7.9.7

[Qemu-devel] [PATCH 2/3] uhci: egsm fix

[Gerd Hoffmann]

When the guest goes suspend the uhci controller while there are
pending resume requests on the ports go signal global resume
instantly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-uhci.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index cb44abc..ac82833 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -189,6 +189,7 @@ typedef struct UHCI_QH {
 
 static void uhci_async_cancel(UHCIAsync *async);
 static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td);
+static void uhci_resume(void *opaque);
 
 static inline int32_t uhci_queue_token(UHCI_TD *td)
 {
@@ -498,6 +499,12 @@ static void uhci_port_write(void *opaque, hwaddr addr,
             return;
         }
         s->cmd = val;
+        if (val & UHCI_CMD_EGSM) {
+            if ((s->ports[0].ctrl & UHCI_PORT_RD) ||
+                (s->ports[1].ctrl & UHCI_PORT_RD)) {
+                uhci_resume(s);
+            }
+        }
         break;
     case 0x02:
         s->status &= ~val;
-- 
1.7.9.7

[Qemu-devel] [PATCH 3/3] hcd-ohci: add dma error handling

[Gerd Hoffmann]

From: Alexey Kardashevskiy <aik@ozlabs.ru>

Current hcd-ohci does not handle DMA errors. However they may happen
so here we introduce simple error handling.

On such errors, a typical OHCI will stop operating, signal the guest
about the error by sending "UnrecoverableError Event", set itself into
error state and set "Detected Parity Error" in its PCI config space
to signal that it got an error and so does the patch.

This also adds ohci_die() call to ohci_bus_start() to handle possible
failure of qemu_new_timer_ns().

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ohci.c |  177 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 126 insertions(+), 51 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index d438d64..d7836d6 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -22,7 +22,6 @@
  *  o Allocate bandwidth in frames properly
  *  o Disable timers when nothing needs to be done, or remove timer usage
  *    all together.
- *  o Handle unrecoverable errors properly
  *  o BIOS work to boot from USB storage
 */
 
@@ -308,6 +307,8 @@ struct ohci_iso_td {
 
 #define OHCI_HRESET_FSBIR       (1 << 0)
 
+static void ohci_die(OHCIState *ohci);
+
 /* Update IRQ levels */
 static inline void ohci_intr_update(OHCIState *ohci)
 {
@@ -508,11 +509,13 @@ static inline int get_dwords(OHCIState *ohci,
     addr += ohci->localmem_base;
 
     for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
-        dma_memory_read(ohci->as, addr, buf, sizeof(*buf));
+        if (dma_memory_read(ohci->as, addr, buf, sizeof(*buf))) {
+            return -1;
+        }
         *buf = le32_to_cpu(*buf);
     }
 
-    return 1;
+    return 0;
 }
 
 /* Put an array of dwords in to main memory */
@@ -525,10 +528,12 @@ static inline int put_dwords(OHCIState *ohci,
 
     for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
         uint32_t tmp = cpu_to_le32(*buf);
-        dma_memory_write(ohci->as, addr, &tmp, sizeof(tmp));
+        if (dma_memory_write(ohci->as, addr, &tmp, sizeof(tmp))) {
+            return -1;
+        }
     }
 
-    return 1;
+    return 0;
 }
 
 /* Get an array of words from main memory */
@@ -540,11 +545,13 @@ static inline int get_words(OHCIState *ohci,
     addr += ohci->localmem_base;
 
     for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
-        dma_memory_read(ohci->as, addr, buf, sizeof(*buf));
+        if (dma_memory_read(ohci->as, addr, buf, sizeof(*buf))) {
+            return -1;
+        }
         *buf = le16_to_cpu(*buf);
     }
 
-    return 1;
+    return 0;
 }
 
 /* Put an array of words in to main memory */
@@ -557,10 +564,12 @@ static inline int put_words(OHCIState *ohci,
 
     for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
         uint16_t tmp = cpu_to_le16(*buf);
-        dma_memory_write(ohci->as, addr, &tmp, sizeof(tmp));
+        if (dma_memory_write(ohci->as, addr, &tmp, sizeof(tmp))) {
+            return -1;
+        }
     }
 
-    return 1;
+    return 0;
 }
 
 static inline int ohci_read_ed(OHCIState *ohci,
@@ -578,15 +587,15 @@ static inline int ohci_read_td(OHCIState *ohci,
 static inline int ohci_read_iso_td(OHCIState *ohci,
                                    dma_addr_t addr, struct ohci_iso_td *td)
 {
-    return (get_dwords(ohci, addr, (uint32_t *)td, 4) &&
-            get_words(ohci, addr + 16, td->offset, 8));
+    return get_dwords(ohci, addr, (uint32_t *)td, 4) ||
+           get_words(ohci, addr + 16, td->offset, 8);
 }
 
 static inline int ohci_read_hcca(OHCIState *ohci,
                                  dma_addr_t addr, struct ohci_hcca *hcca)
 {
-    dma_memory_read(ohci->as, addr + ohci->localmem_base, hcca, sizeof(*hcca));
-    return 1;
+    return dma_memory_read(ohci->as, addr + ohci->localmem_base,
+                           hcca, sizeof(*hcca));
 }
 
 static inline int ohci_put_ed(OHCIState *ohci,
@@ -610,23 +619,22 @@ static inline int ohci_put_td(OHCIState *ohci,
 static inline int ohci_put_iso_td(OHCIState *ohci,
                                   dma_addr_t addr, struct ohci_iso_td *td)
 {
-    return (put_dwords(ohci, addr, (uint32_t *)td, 4) &&
-            put_words(ohci, addr + 16, td->offset, 8));
+    return put_dwords(ohci, addr, (uint32_t *)td, 4 ||
+           put_words(ohci, addr + 16, td->offset, 8));
 }
 
 static inline int ohci_put_hcca(OHCIState *ohci,
                                 dma_addr_t addr, struct ohci_hcca *hcca)
 {
-    dma_memory_write(ohci->as,
-                     addr + ohci->localmem_base + HCCA_WRITEBACK_OFFSET,
-                     (char *)hcca + HCCA_WRITEBACK_OFFSET,
-                     HCCA_WRITEBACK_SIZE);
-    return 1;
+    return dma_memory_write(ohci->as,
+                            addr + ohci->localmem_base + HCCA_WRITEBACK_OFFSET,
+                            (char *)hcca + HCCA_WRITEBACK_OFFSET,
+                            HCCA_WRITEBACK_SIZE);
 }
 
 /* Read/Write the contents of a TD from/to main memory.  */
-static void ohci_copy_td(OHCIState *ohci, struct ohci_td *td,
-                         uint8_t *buf, int len, DMADirection dir)
+static int ohci_copy_td(OHCIState *ohci, struct ohci_td *td,
+                        uint8_t *buf, int len, DMADirection dir)
 {
     dma_addr_t ptr, n;
 
@@ -634,18 +642,26 @@ static void ohci_copy_td(OHCIState *ohci, struct ohci_td *td,
     n = 0x1000 - (ptr & 0xfff);
     if (n > len)
         n = len;
-    dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, n, dir);
-    if (n == len)
-        return;
+
+    if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, n, dir)) {
+        return -1;
+    }
+    if (n == len) {
+        return 0;
+    }
     ptr = td->be & ~0xfffu;
     buf += n;
-    dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, len - n, dir);
+    if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf,
+                      len - n, dir)) {
+        return -1;
+    }
+    return 0;
 }
 
 /* Read/Write the contents of an ISO TD from/to main memory.  */
-static void ohci_copy_iso_td(OHCIState *ohci,
-                             uint32_t start_addr, uint32_t end_addr,
-                             uint8_t *buf, int len, DMADirection dir)
+static int ohci_copy_iso_td(OHCIState *ohci,
+                            uint32_t start_addr, uint32_t end_addr,
+                            uint8_t *buf, int len, DMADirection dir)
 {
     dma_addr_t ptr, n;
 
@@ -653,12 +669,20 @@ static void ohci_copy_iso_td(OHCIState *ohci,
     n = 0x1000 - (ptr & 0xfff);
     if (n > len)
         n = len;
-    dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, n, dir);
-    if (n == len)
-        return;
+
+    if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, n, dir)) {
+        return -1;
+    }
+    if (n == len) {
+        return 0;
+    }
     ptr = end_addr & ~0xfffu;
     buf += n;
-    dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf, len - n, dir);
+    if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf,
+                      len - n, dir)) {
+        return -1;
+    }
+    return 0;
 }
 
 static void ohci_process_lists(OHCIState *ohci, int completion);
@@ -698,8 +722,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
 
     addr = ed->head & OHCI_DPTR_MASK;
 
-    if (!ohci_read_iso_td(ohci, addr, &iso_td)) {
+    if (ohci_read_iso_td(ohci, addr, &iso_td)) {
         printf("usb-ohci: ISO_TD read error at %x\n", addr);
+        ohci_die(ohci);
         return 0;
     }
 
@@ -740,7 +765,10 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
         i = OHCI_BM(iso_td.flags, TD_DI);
         if (i < ohci->done_count)
             ohci->done_count = i;
-        ohci_put_iso_td(ohci, addr, &iso_td);
+        if (ohci_put_iso_td(ohci, addr, &iso_td)) {
+            ohci_die(ohci);
+            return 1;
+        }
         return 0;
     }
 
@@ -821,8 +849,11 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
     }
 
     if (len && dir != OHCI_TD_DIR_IN) {
-        ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, len,
-                         DMA_DIRECTION_TO_DEVICE);
+        if (ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, len,
+                             DMA_DIRECTION_TO_DEVICE)) {
+            ohci_die(ohci);
+            return 1;
+        }
     }
 
     if (!completion) {
@@ -852,8 +883,11 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
     /* Writeback */
     if (dir == OHCI_TD_DIR_IN && ret >= 0 && ret <= len) {
         /* IN transfer succeeded */
-        ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, ret,
-                         DMA_DIRECTION_FROM_DEVICE);
+        if (ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, ret,
+                             DMA_DIRECTION_FROM_DEVICE)) {
+            ohci_die(ohci);
+            return 1;
+        }
         OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_CC,
                     OHCI_CC_NOERROR);
         OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_SIZE, ret);
@@ -910,7 +944,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
         if (i < ohci->done_count)
             ohci->done_count = i;
     }
-    ohci_put_iso_td(ohci, addr, &iso_td);
+    if (ohci_put_iso_td(ohci, addr, &iso_td)) {
+        ohci_die(ohci);
+    }
     return 1;
 }
 
@@ -943,8 +979,9 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
 #endif
         return 1;
     }
-    if (!ohci_read_td(ohci, addr, &td)) {
+    if (ohci_read_td(ohci, addr, &td)) {
         fprintf(stderr, "usb-ohci: TD read error at %x\n", addr);
+        ohci_die(ohci);
         return 0;
     }
 
@@ -997,8 +1034,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
                 pktlen = len;
             }
             if (!completion) {
-                ohci_copy_td(ohci, &td, ohci->usb_buf, pktlen,
-                             DMA_DIRECTION_TO_DEVICE);
+                if (ohci_copy_td(ohci, &td, ohci->usb_buf, pktlen,
+                                 DMA_DIRECTION_TO_DEVICE)) {
+                    ohci_die(ohci);
+                }
             }
         }
     }
@@ -1055,8 +1094,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
 
     if (ret >= 0) {
         if (dir == OHCI_TD_DIR_IN) {
-            ohci_copy_td(ohci, &td, ohci->usb_buf, ret,
-                         DMA_DIRECTION_FROM_DEVICE);
+            if (ohci_copy_td(ohci, &td, ohci->usb_buf, ret,
+                             DMA_DIRECTION_FROM_DEVICE)) {
+                ohci_die(ohci);
+            }
 #ifdef DEBUG_PACKET
             DPRINTF("  data:");
             for (i = 0; i < ret; i++)
@@ -1133,7 +1174,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
     if (i < ohci->done_count)
         ohci->done_count = i;
 exit_no_retire:
-    ohci_put_td(ohci, addr, &td);
+    if (ohci_put_td(ohci, addr, &td)) {
+        ohci_die(ohci);
+        return 1;
+    }
     return OHCI_BM(td.flags, TD_CC) != OHCI_CC_NOERROR;
 }
 
@@ -1151,8 +1195,9 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion)
         return 0;
 
     for (cur = head; cur; cur = next_ed) {
-        if (!ohci_read_ed(ohci, cur, &ed)) {
+        if (ohci_read_ed(ohci, cur, &ed)) {
             fprintf(stderr, "usb-ohci: ED read error at %x\n", cur);
+            ohci_die(ohci);
             return 0;
         }
 
@@ -1194,7 +1239,10 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head, int completion)
             }
         }
 
-        ohci_put_ed(ohci, cur, &ed);
+        if (ohci_put_ed(ohci, cur, &ed)) {
+            ohci_die(ohci);
+            return 0;
+        }
     }
 
     return active;
@@ -1236,7 +1284,11 @@ static void ohci_frame_boundary(void *opaque)
     OHCIState *ohci = opaque;
     struct ohci_hcca hcca;
 
-    ohci_read_hcca(ohci, ohci->hcca, &hcca);
+    if (ohci_read_hcca(ohci, ohci->hcca, &hcca)) {
+        fprintf(stderr, "usb-ohci: HCCA read error at %x\n", ohci->hcca);
+        ohci_die(ohci);
+        return;
+    }
 
     /* Process all the lists at the end of the frame */
     if (ohci->ctl & OHCI_CTL_PLE) {
@@ -1257,6 +1309,11 @@ static void ohci_frame_boundary(void *opaque)
     ohci->old_ctl = ohci->ctl;
     ohci_process_lists(ohci, 0);
 
+    /* Stop if UnrecoverableError happened or ohci_sof will crash */
+    if (ohci->intr_status & OHCI_INTR_UE) {
+        return;
+    }
+
     /* Frame boundary, so do EOF stuf here */
     ohci->frt = ohci->fit;
 
@@ -1282,7 +1339,9 @@ static void ohci_frame_boundary(void *opaque)
     ohci_sof(ohci);
 
     /* Writeback HCCA */
-    ohci_put_hcca(ohci, ohci->hcca, &hcca);
+    if (ohci_put_hcca(ohci, ohci->hcca, &hcca)) {
+        ohci_die(ohci);
+    }
 }
 
 /* Start sending SOF tokens across the USB bus, lists are processed in
@@ -1296,7 +1355,7 @@ static int ohci_bus_start(OHCIState *ohci)
 
     if (ohci->eof_timer == NULL) {
         fprintf(stderr, "usb-ohci: %s: qemu_new_timer_ns failed\n", ohci->name);
-        /* TODO: Signal unrecoverable error */
+        ohci_die(ohci);
         return 0;
     }
 
@@ -1857,6 +1916,22 @@ typedef struct {
     uint32_t firstport;
 } OHCIPCIState;
 
+/** A typical O/EHCI will stop operating, set itself into error state
+ * (which can be queried by MMIO) and will set PERR in its config
+ * space to signal that it got an error
+ */
+static void ohci_die(OHCIState *ohci)
+{
+    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
+
+    fprintf(stderr, "%s: DMA error\n", __func__);
+
+    ohci_set_interrupt(ohci, OHCI_INTR_UE);
+    ohci_bus_stop(ohci);
+    pci_set_word(dev->parent_obj.config + PCI_STATUS,
+                 PCI_STATUS_DETECTED_PARITY);
+}
+
 static int usb_ohci_initfn_pci(PCIDevice *dev)
 {
     OHCIPCIState *ohci = PCI_OHCI(dev);
-- 
1.7.9.7