| Message ID | 20240604071540.18138-2-fea.wang@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/dma: Add error handling for loading descriptions failing | expand |
On Tue, Jun 4, 2024 at 9:10 AM Fea.Wang <fea.wang@sifive.com> wrote: > Loading a description from memory may cause a bus-error. In this > case, the DMA should stop working, set the error flag, and return > the failure value. > > When calling the loading a description function, it should be noticed > that the function may return a failure value. Breaking the loop in this > case is one of the possible ways to handle it. > > Signed-off-by: Fea.Wang <fea.wang@sifive.com> > Reviewed-by: Frank Chang <frank.chang@sifive.com> > Thanks! Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com> --- > hw/dma/xilinx_axidma.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c > index 0ae056ed06..ad307994c2 100644 > --- a/hw/dma/xilinx_axidma.c > +++ b/hw/dma/xilinx_axidma.c > @@ -71,8 +71,11 @@ enum { > enum { > DMASR_HALTED = 1, > DMASR_IDLE = 2, > + DMASR_SLVERR = 1 << 5, > + DMASR_DECERR = 1 << 6, > DMASR_IOC_IRQ = 1 << 12, > DMASR_DLY_IRQ = 1 << 13, > + DMASR_ERR_IRQ = 1 << 14, > > DMASR_IRQ_MASK = 7 << 12 > }; > @@ -190,17 +193,32 @@ static inline int streamid_from_addr(hwaddr addr) > return sid; > } > > -static void stream_desc_load(struct Stream *s, hwaddr addr) > +static MemTxResult stream_desc_load(struct Stream *s, hwaddr addr) > { > struct SDesc *d = &s->desc; > > - address_space_read(&s->dma->as, addr, MEMTXATTRS_UNSPECIFIED, d, > sizeof *d); > + MemTxResult result = address_space_read(&s->dma->as, > + addr, MEMTXATTRS_UNSPECIFIED, > + d, sizeof *d); > + if (result != MEMTX_OK) { > + if (result == MEMTX_DECODE_ERROR) { > + s->regs[R_DMASR] |= DMASR_DECERR; > + } else { > + s->regs[R_DMASR] |= DMASR_SLVERR; > + } > + > + s->regs[R_DMACR] &= ~DMACR_RUNSTOP; > + s->regs[R_DMASR] |= DMASR_HALTED; > + s->regs[R_DMASR] |= DMASR_ERR_IRQ; > + return result; > + } > > /* Convert from LE into host endianness. */ > d->buffer_address = le64_to_cpu(d->buffer_address); > d->nxtdesc = le64_to_cpu(d->nxtdesc); > d->control = le32_to_cpu(d->control); > d->status = le32_to_cpu(d->status); > + return result; > } > > static void stream_desc_store(struct Stream *s, hwaddr addr) > @@ -279,7 +297,9 @@ static void stream_process_mem2s(struct Stream *s, > StreamSink *tx_data_dev, > } > > while (1) { > - stream_desc_load(s, s->regs[R_CURDESC]); > + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { > + break; > + } > > if (s->desc.status & SDESC_STATUS_COMPLETE) { > s->regs[R_DMASR] |= DMASR_HALTED; > @@ -336,7 +356,9 @@ static size_t stream_process_s2mem(struct Stream *s, > unsigned char *buf, > } > > while (len) { > - stream_desc_load(s, s->regs[R_CURDESC]); > + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { > + break; > + } > > if (s->desc.status & SDESC_STATUS_COMPLETE) { > s->regs[R_DMASR] |= DMASR_HALTED; > -- > 2.34.1 > >
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c index 0ae056ed06..ad307994c2 100644 --- a/hw/dma/xilinx_axidma.c +++ b/hw/dma/xilinx_axidma.c @@ -71,8 +71,11 @@ enum { enum { DMASR_HALTED = 1, DMASR_IDLE = 2, + DMASR_SLVERR = 1 << 5, + DMASR_DECERR = 1 << 6, DMASR_IOC_IRQ = 1 << 12, DMASR_DLY_IRQ = 1 << 13, + DMASR_ERR_IRQ = 1 << 14, DMASR_IRQ_MASK = 7 << 12 }; @@ -190,17 +193,32 @@ static inline int streamid_from_addr(hwaddr addr) return sid; } -static void stream_desc_load(struct Stream *s, hwaddr addr) +static MemTxResult stream_desc_load(struct Stream *s, hwaddr addr) { struct SDesc *d = &s->desc; - address_space_read(&s->dma->as, addr, MEMTXATTRS_UNSPECIFIED, d, sizeof *d); + MemTxResult result = address_space_read(&s->dma->as, + addr, MEMTXATTRS_UNSPECIFIED, + d, sizeof *d); + if (result != MEMTX_OK) { + if (result == MEMTX_DECODE_ERROR) { + s->regs[R_DMASR] |= DMASR_DECERR; + } else { + s->regs[R_DMASR] |= DMASR_SLVERR; + } + + s->regs[R_DMACR] &= ~DMACR_RUNSTOP; + s->regs[R_DMASR] |= DMASR_HALTED; + s->regs[R_DMASR] |= DMASR_ERR_IRQ; + return result; + } /* Convert from LE into host endianness. */ d->buffer_address = le64_to_cpu(d->buffer_address); d->nxtdesc = le64_to_cpu(d->nxtdesc); d->control = le32_to_cpu(d->control); d->status = le32_to_cpu(d->status); + return result; } static void stream_desc_store(struct Stream *s, hwaddr addr) @@ -279,7 +297,9 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, } while (1) { - stream_desc_load(s, s->regs[R_CURDESC]); + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { + break; + } if (s->desc.status & SDESC_STATUS_COMPLETE) { s->regs[R_DMASR] |= DMASR_HALTED; @@ -336,7 +356,9 @@ static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf, } while (len) { - stream_desc_load(s, s->regs[R_CURDESC]); + if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) { + break; + } if (s->desc.status & SDESC_STATUS_COMPLETE) { s->regs[R_DMASR] |= DMASR_HALTED;