Hi Edgar,
Thank you for recommending to me. I will make the change in the next
version of the patch series.
Sincerely,
Fea
On Mon, Jun 3, 2024 at 6:19 PM Edgar E. Iglesias <edgar.igles...@gmail.com>
wrote:
> On Mon, Jun 3, 2024 at 7:47 AM Fea.Wang <fea.w...@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 error value.
>>
>> Signed-off-by: Fea.Wang <fea.w...@sifive.com>
>>
>
>
> Hi Fea,
>
> I've got a couple of small comments:
>
>
> ---
>> hw/dma/xilinx_axidma.c | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
>> index 0ae056ed06..4b475e5484 100644
>> --- a/hw/dma/xilinx_axidma.c
>> +++ b/hw/dma/xilinx_axidma.c
>> @@ -71,8 +71,10 @@ enum {
>> enum {
>> DMASR_HALTED = 1,
>> DMASR_IDLE = 2,
>> + DMASR_SLVERR = 1 << 5,
>>
>
> We should also add 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 +192,27 @@ 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) {
>> + s->regs[R_DMACR] &= ~DMACR_RUNSTOP;
>> + s->regs[R_DMASR] |= DMASR_HALTED;
>> + s->regs[R_DMASR] |= DMASR_SLVERR;
>>
>
> ... and map MEMTX_DECODE_ERROR to DMASR_DECERR and everything else to
> SLVERR, for example:
> if (result == MEMTX_DECODE_ERROR) {
> s->regs[R_DMASR] |= DMASR_DECERR;
> } else {
> s->regs[R_DMASR] |= DMASR_SLVERR;
> }
>
>
>> + 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)
>> --
>> 2.34.1
>>
>>
