[Qemu-devel] [PATCH] ehci: handle dma errors

* [Qemu-devel] [PATCH] ehci: handle dma errors
@ 2012-11-15 12:22 Gerd Hoffmann
  2012-11-15 13:13 ` Hans de Goede
  0 siblings, 1 reply; 2+ messages in thread
From: Gerd Hoffmann @ 2012-11-15 12:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Starting with commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d dma
transfers can actually fail.  This patch makes ehci keep track
of the busmaster bit in pci config space, by setting/clearing the
dma_context pointer.  Attempts to dma without context will result
in raising HSE (Host System Error) interrupt and stopping the host
controller.

This patch fixes WinXP not booting with a usb stick attached to ehci.
Root cause is seabios activating ehci so you can boot from the stick,
and WinXP clearing the busmaster bit before resetting the host
controller, leading to ehci actually trying dma while it is disabled.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci-pci.c |   17 +++++++++++++
 hw/usb/hcd-ehci.c     |   63 ++++++++++++++++++++++++++++++++++--------------
 trace-events          |    1 +
 3 files changed, 62 insertions(+), 19 deletions(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index fe45a1f..5887eab 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -17,6 +17,7 @@
 
 #include "hw/usb/hcd-ehci.h"
 #include "hw/pci.h"
+#include "range.h"
 
 typedef struct EHCIPCIState {
     PCIDevice pcidev;
@@ -79,6 +80,21 @@ static int usb_ehci_pci_initfn(PCIDevice *dev)
     return 0;
 }
 
+static void usb_ehci_pci_write_config(PCIDevice *dev, uint32_t addr,
+                                      uint32_t val, int l)
+{
+    EHCIPCIState *i = DO_UPCAST(EHCIPCIState, pcidev, dev);
+    bool busmaster;
+
+    pci_default_write_config(dev, addr, val, l);
+
+    if (!range_covers_byte(addr, l, PCI_COMMAND)) {
+        return;
+    }
+    busmaster = pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER;
+    i->ehci.dma = busmaster ? pci_dma_context(dev) : NULL;
+}
+
 static Property ehci_pci_properties[] = {
     DEFINE_PROP_UINT32("maxframes", EHCIPCIState, ehci.maxframes, 128),
     DEFINE_PROP_END_OF_LIST(),
@@ -106,6 +122,7 @@ static void ehci_class_init(ObjectClass *klass, void *data)
     k->device_id = i->device_id;
     k->revision = i->revision;
     k->class_id = PCI_CLASS_SERIAL_USB;
+    k->config_write = usb_ehci_pci_write_config;
     dc->vmsd = &vmstate_ehci_pci;
     dc->props = ehci_pci_properties;
 }
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 57da76c..df3aaed 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1000,21 +1000,25 @@ static void ehci_opreg_write(void *ptr, hwaddr addr,
                                 *mmio, old);
 }
 
-
-// TODO : Put in common header file, duplication from usb-ohci.c
-
 /* Get an array of dwords from main memory */
 static inline int get_dwords(EHCIState *ehci, uint32_t addr,
                              uint32_t *buf, int num)
 {
     int i;
 
+    if (!ehci->dma) {
+        ehci_raise_irq(ehci, USBSTS_HSE);
+        ehci->usbcmd &= ~USBCMD_RUNSTOP;
+        trace_usb_ehci_dma_error();
+        return -1;
+    }
+
     for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
         dma_memory_read(ehci->dma, addr, buf, sizeof(*buf));
         *buf = le32_to_cpu(*buf);
     }
 
-    return 1;
+    return num;
 }
 
 /* Put an array of dwords in to main memory */
@@ -1023,12 +1027,19 @@ static inline int put_dwords(EHCIState *ehci, uint32_t addr,
 {
     int i;
 
+    if (!ehci->dma) {
+        ehci_raise_irq(ehci, USBSTS_HSE);
+        ehci->usbcmd &= ~USBCMD_RUNSTOP;
+        trace_usb_ehci_dma_error();
+        return -1;
+    }
+
     for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
         uint32_t tmp = cpu_to_le32(*buf);
         dma_memory_write(ehci->dma, addr, &tmp, sizeof(tmp));
     }
 
-    return 1;
+    return num;
 }
 
 /*
@@ -1440,8 +1451,10 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int async)
 
     /*  Find the head of the list (4.9.1.1) */
     for(i = 0; i < MAX_QH; i++) {
-        get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh,
-                   sizeof(EHCIqh) >> 2);
+        if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh,
+                       sizeof(EHCIqh) >> 2) < 0) {
+            return 0;
+        }
         ehci_trace_qh(NULL, NLPTR_GET(entry), &qh);
 
         if (qh.epchar & QH_EPCHAR_H) {
@@ -1538,8 +1551,11 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
         goto out;
     }
 
-    get_dwords(ehci, NLPTR_GET(q->qhaddr),
-               (uint32_t *) &qh, sizeof(EHCIqh) >> 2);
+    if (get_dwords(ehci, NLPTR_GET(q->qhaddr),
+                   (uint32_t *) &qh, sizeof(EHCIqh) >> 2) < 0) {
+        q = NULL;
+        goto out;
+    }
     ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &qh);
 
     /*
@@ -1626,8 +1642,10 @@ static int ehci_state_fetchitd(EHCIState *ehci, int async)
     assert(!async);
     entry = ehci_get_fetch_addr(ehci, async);
 
-    get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd,
-               sizeof(EHCIitd) >> 2);
+    if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd,
+                   sizeof(EHCIitd) >> 2) < 0) {
+        return -1;
+    }
     ehci_trace_itd(ehci, entry, &itd);
 
     if (ehci_process_itd(ehci, &itd, entry) != 0) {
@@ -1650,8 +1668,10 @@ static int ehci_state_fetchsitd(EHCIState *ehci, int async)
     assert(!async);
     entry = ehci_get_fetch_addr(ehci, async);
 
-    get_dwords(ehci, NLPTR_GET(entry), (uint32_t *)&sitd,
-               sizeof(EHCIsitd) >> 2);
+    if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *)&sitd,
+                   sizeof(EHCIsitd) >> 2) < 0) {
+        return 0;
+    }
     ehci_trace_sitd(ehci, entry, &sitd);
 
     if (!(sitd.results & SITD_RESULTS_ACTIVE)) {
@@ -1712,8 +1732,10 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
     EHCIPacket *p;
     int again = 1;
 
-    get_dwords(q->ehci, NLPTR_GET(q->qtdaddr), (uint32_t *) &qtd,
-               sizeof(EHCIqtd) >> 2);
+    if (get_dwords(q->ehci, NLPTR_GET(q->qtdaddr), (uint32_t *) &qtd,
+                   sizeof(EHCIqtd) >> 2) < 0) {
+        return 0;
+    }
     ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), &qtd);
 
     p = QTAILQ_FIRST(&q->packets);
@@ -1806,8 +1828,10 @@ static int ehci_fill_queue(EHCIPacket *p)
                 goto leave;
             }
         }
-        get_dwords(q->ehci, NLPTR_GET(qtdaddr),
-                   (uint32_t *) &qtd, sizeof(EHCIqtd) >> 2);
+        if (get_dwords(q->ehci, NLPTR_GET(qtdaddr),
+                       (uint32_t *) &qtd, sizeof(EHCIqtd) >> 2) < 0) {
+            return -1;
+        }
         ehci_trace_qtd(q, NLPTR_GET(qtdaddr), &qtd);
         if (!(qtd.token & QTD_TOKEN_ACTIVE)) {
             break;
@@ -2106,8 +2130,9 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
         }
         list |= ((ehci->frindex & 0x1ff8) >> 1);
 
-        dma_memory_read(ehci->dma, list, &entry, sizeof entry);
-        entry = le32_to_cpu(entry);
+        if (get_dwords(ehci, list, &entry, 1) < 0) {
+            break;
+        }
 
         DPRINTF("PERIODIC state adv fr=%d.  [%08X] -> %08X\n",
                 ehci->frindex / 8, list, entry);
diff --git a/trace-events b/trace-events
index 10813d5..913e00b 100644
--- a/trace-events
+++ b/trace-events
@@ -286,6 +286,7 @@ usb_ehci_irq(uint32_t level, uint32_t frindex, uint32_t sts, uint32_t mask) "lev
 usb_ehci_guest_bug(const char *reason) "%s"
 usb_ehci_doorbell_ring(void) ""
 usb_ehci_doorbell_ack(void) ""
+usb_ehci_dma_error(void) ""
 
 # hw/usb/hcd-uhci.c
 usb_uhci_reset(void) "=== RESET ==="
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread