* [Qemu-devel] [PATCH v4] hcd-ohci: add dma error handling
@ 2013-07-26 10:37 Alexey Kardashevskiy
2013-07-26 10:43 ` Gerd Hoffmann
0 siblings, 1 reply; 2+ messages in thread
From: Alexey Kardashevskiy @ 2013-07-26 10:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Anthony Liguori, Gerd Hoffmann
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_stop() call to ohci_bus_start() to handle possible
failure of qemu_new_timer_ns().
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
2013/07/26:
* ohci_stop renamed to ohci_die to describe the action better
2013/07/25:
* added ohci_stop to ohci_bus_start
* added ohci_bus_stop to ohci_stop
* added return to ohci_frame_boundary() if UnrecoverableError happened to
prevent ohci_sof from failing as the timer is freed at that point
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/usb/hcd-ohci.c | 173 ++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 124 insertions(+), 49 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 2bab8ff..a87991b 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) &&
+ 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) &&
+ 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.8.3.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH v4] hcd-ohci: add dma error handling
2013-07-26 10:37 [Qemu-devel] [PATCH v4] hcd-ohci: add dma error handling Alexey Kardashevskiy
@ 2013-07-26 10:43 ` Gerd Hoffmann
0 siblings, 0 replies; 2+ messages in thread
From: Gerd Hoffmann @ 2013-07-26 10:43 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Anthony Liguori, qemu-devel
Hi,
> This also adds ohci_stop() call to ohci_bus_start() to handle possible
> failure of qemu_new_timer_ns().
> * ohci_stop renamed to ohci_die to describe the action better
Except in the commit message ...
Also scripts/checkpatch.pl complains:
ERROR: return is not a function, parentheses are not required
#90: FILE: hw/usb/hcd-ohci.c:590:
+ return (get_dwords(ohci, addr, (uint32_t *)td, 4) ||
ERROR: return is not a function, parentheses are not required
#109: FILE: hw/usb/hcd-ohci.c:622:
+ return (put_dwords(ohci, addr, (uint32_t *)td, 4) ||
We are almost there now ;)
cheers,
Gerd
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-07-26 10:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-26 10:37 [Qemu-devel] [PATCH v4] hcd-ohci: add dma error handling Alexey Kardashevskiy
2013-07-26 10:43 ` Gerd Hoffmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).