[PATCH v3 1/3] hw/usb/hcd-ehci: Check for DMA errors in get_dwords()/put_dwords()

Jamin Lin <[email protected]>
Newsgroups gmane.comp.emulators.qemu
Message-ID <[email protected]>
get_dwords() and put_dwords() return -1 when ehci->as is NULL, but that
can no longer happen. The sysbus variants set it in instance_init
(ehci_sysbus_init()), the PCI variant sets it in realize
(usb_ehci_pci_realize()), and usb_ehci_pci_write_config() only switches
between the bus master address space and address_space_memory.

Meanwhile the results of dma_memory_read() and dma_memory_write() are
ignored, so a failed guest memory access is silently treated as success.

Drop the dead NULL test and check the MemTxResult of each access
instead, so that the existing -1 error path reports real DMA errors:
raise USBSTS_HSE, clear USBCMD_RUNSTOP and stop processing the
descriptor, as the NULL path used to do. All 11 get_dwords() callers
already check the return value.

put_dwords() now returns void: none of its four callers (ehci_flush_qh(),
ehci_state_fetchitd() and twice in ehci_state_writeback()) looks at the
status.

Suggested-by: Peter Maydell <[email protected]>
Signed-off-by: Jamin Lin <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
---
 hw/usb/hcd-ehci.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 451a918e9f..ecf98c4e19 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -421,16 +421,14 @@ static inline int get_dwords(EHCIState *ehci, uint64_t addr,
 {
     int i;
 
-    if (!ehci->as) {
-        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->as, addr, buf, sizeof(*buf),
-                        MEMTXATTRS_UNSPECIFIED);
+        if (dma_memory_read(ehci->as, addr, buf, sizeof(*buf),
+                            MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
+            ehci_raise_irq(ehci, USBSTS_HSE);
+            ehci->usbcmd &= ~USBCMD_RUNSTOP;
+            trace_usb_ehci_dma_error();
+            return -1;
+        }
         *buf = le32_to_cpu(*buf);
     }
 
@@ -438,25 +436,21 @@ static inline int get_dwords(EHCIState *ehci, uint64_t addr,
 }
 
 /* Put an array of dwords in to main memory */
-static inline int put_dwords(EHCIState *ehci, uint64_t addr,
-                             uint32_t *buf, int num)
+static inline void put_dwords(EHCIState *ehci, uint64_t addr,
+                              uint32_t *buf, int num)
 {
     int i;
 
-    if (!ehci->as) {
-        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->as, addr, &tmp, sizeof(tmp),
-                         MEMTXATTRS_UNSPECIFIED);
+        if (dma_memory_write(ehci->as, addr, &tmp, sizeof(tmp),
+                             MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
+            ehci_raise_irq(ehci, USBSTS_HSE);
+            ehci->usbcmd &= ~USBCMD_RUNSTOP;
+            trace_usb_ehci_dma_error();
+            return;
+        }
     }
-
-    return num;
 }
 
 static int ehci_get_pid(EHCIqtd *qtd)
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.