hw/dma,net: Fix Xilinx AXI DMA and Ethernet
cover hw/dma,net: Fix Xilinx AXI DMA and Ethernet
The Xilinx AXI DMA controller supports 64-bit descriptor addresses, but the QEMU model only handled the low 32 bits during stream processing. This causes descriptor accesses to use incorrect addresses when descriptors are located above 4 GiB.
This series updates the descriptor and MMIO handling and fixes related AXI DMA and AXI Ethernet behavior.
The series:
- Add 64-bit descriptor address handling to stream processing.
- Add separate 32-bit and 64-bit MMIO access helpers.
- Fix DMASR W1C and DMACR start processing behavior.
- Reset both DMA channels when DMACR_RESET is written.
- Do not start a transfer when TAILDESC is written while the channel is halted.
- Fix AXI Ethernet RX packet double-buffering.
Testing:
- Tested Xilinx AXI Ethernet networking with U-Boot.
- Tested DHCP and ping.
Jay Chang (6):
hw/dma/xilinx_axidma: Support 64-bit descriptor addressing in stream
processing
hw/dma/xilinx_axidma: Add 64-bit MMIO access support hw/dma/xilinx_axidma: Fix DMASR register handling hw/dma/xilinx_axidma: Reset both channels on DMACR_RESET hw/dma/xilinx_axidma: Don't start transfer on TAILDESC write when
halted
hw/net/xilinx_axienet: Fix packet double-buffering issue
hw/dma/xilinx_axidma.c | 249 +++++++++++++++++++++++++++++----------- hw/net/xilinx_axienet.c | 12 ++ 2 files changed, 193 insertions(+), 68 deletions(-)
Signature · 1 line
2.48.1
PATCH 1/6 hw/dma/xilinx_axidma: Support 64-bit descriptor addressing in stream processing
The Xilinx AXI DMA controller supports both 32-bit and 64-bit descriptor addressing through register pairs (e.g., CURDESC/CURDESC_MSB and TAILDESC/TAILDESC_MSB). However, the original implementation only handled 32-bit addresses correctly in the stream processing functions.
This resulted in MEMTX_DECODE_ERROR when the DMA engine attempted to access buffer descriptors at invalid addresses, breaking network operations like DHCP and ping.
--- hw/dma/xilinx_axidma.c | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-)
hw/dma/xilinx_axidma.c +30 -12
@@ -181,6 +181,11 @@ static void stream_reset(struct Stream *s) { s->regs[R_DMASR ] =DMASR_HALTED ;/* starts up halted. */ s->regs[R_DMACR ] =1 <<16 ;/* Starts with one in compl threshold. */ +/* Initialize descriptor registers to zero for both 32 and 64 bit */ + s->regs[R_CURDESC ] =0 ; + s->regs[R_CURDESC +1 ] =0 ;/* CURDESC_MSB */ + s->regs[R_TAILDESC ] =0 ; + s->regs[R_TAILDESC +1 ] =0 ;/* TAILDESC_MSB */ s->sof = true; }
@@ -194,6 +199,16 @@ static inline int streamid_from_addr(hwaddr addr)return sid; } +static inline hwaddr stream_desc_addr (struct Stream *s) +{ +return s->regs[R_CURDESC ] | ((uint64_t )s->regs[R_CURDESC +1 ] <<32 ); +} + +static inline hwaddr stream_tail_addr (struct Stream *s) +{ +return s->regs[R_TAILDESC ] | ((uint64_t )s->regs[R_TAILDESC +1 ] <<32 ); +} +static MemTxResult stream_desc_load (struct Stream *s,hwaddr addr) {struct SDesc *d = &s->desc;
@@ -290,9 +305,9 @@ static void stream_complete(struct Stream *s)static void stream_process_mem2s (struct Stream *s,StreamSink *tx_data_dev,StreamSink *tx_control_dev) { -uint32_t prev_d; -uint32_t txlen; +uint32_t txlen, origin_txlen;uint64_t addr; +hwaddr curdesc;bool eop;if (!stream_running (s) ||stream_idle (s) ||stream_halted (s)) {
@@ -300,7 +315,8 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, }while (1 ) { -if (MEMTX_OK !=stream_desc_load (s, s->regs[R_CURDESC ])) { + curdesc =stream_desc_addr (s); +if (MEMTX_OK !=stream_desc_load (s, curdesc)) {break ; }
@@ -314,6 +330,7 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, } txlen = s->desc.control &SDESC_CTRL_LEN_MASK ; + origin_txlen = txlen; eop =stream_desc_eof (&s->desc); addr = s->desc.buffer_address;
@@ -334,13 +351,13 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, }/* Update the descriptor. */ - s->desc.status = txlen |SDESC_STATUS_COMPLETE ; -stream_desc_store (s, s->regs[R_CURDESC ]); + s->desc.status = origin_txlen |SDESC_STATUS_COMPLETE ; +stream_desc_store (s, curdesc);/* Advance. */ - prev_d = s->regs[R_CURDESC ]; s->regs[R_CURDESC ] = s->desc.nxtdesc; -if (prev_d == s->regs[R_TAILDESC ]) { + s->regs[R_CURDESC +1 ] = s->desc.nxtdesc >>32 ; +if (curdesc ==stream_tail_addr (s)) { s->regs[R_DMASR ] |=DMASR_IDLE ;break ; }
@@ -350,16 +367,17 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev,static size_t stream_process_s2mem (struct Stream *s,unsigned char *buf,size_t len,bool eop) { -uint32_t prev_d;unsigned int rxlen;size_t pos =0 ; +hwaddr curdesc;if (!stream_running (s) ||stream_idle (s) ||stream_halted (s)) {return 0 ; }while (len) { -if (MEMTX_OK !=stream_desc_load (s, s->regs[R_CURDESC ])) { + curdesc =stream_desc_addr (s); +if (MEMTX_OK !=stream_desc_load (s, curdesc)) {break ; }
@@ -388,13 +406,13 @@ static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf, s->desc.status |= s->sof <<SDESC_STATUS_SOF_BIT ; s->desc.status |=SDESC_STATUS_COMPLETE ; -stream_desc_store (s, s->regs[R_CURDESC ]); +stream_desc_store (s, curdesc); s->sof = eop;/* Advance. */ - prev_d = s->regs[R_CURDESC ]; s->regs[R_CURDESC ] = s->desc.nxtdesc; -if (prev_d == s->regs[R_TAILDESC ]) { + s->regs[R_CURDESC +1 ] = s->desc.nxtdesc >>32 ; +if (curdesc ==stream_tail_addr (s)) { s->regs[R_DMASR ] |=DMASR_IDLE ;break ; }
Signature · 1 line
2.48.1
Re: [PATCH 1/6] hw/dma/xilinx_axidma: Support 64-bit descriptor addressing in stream processing
Quoted 7 lines · Jay Chang
The Xilinx AXI DMA controller supports both 32-bit and 64-bit descriptor
addressing through register pairs (e.g., CURDESC/CURDESC_MSB and TAILDESC/TAILDESC_MSB). However, the original implementation only handled
32-bit addresses correctly in the stream processing functions.
Quoted 8 lines
This resulted in
MEMTX_DECODE_ERRORwhen the DMA engine attempted to access buffer descriptors at invalid addresses, breaking network operations
like DHCP and ping.Signed-off-by Jay Chang <[email protected]>Reviewed-by Frank Chang <[email protected]>Reviewed-by Jim Shu <[email protected]>
Alistair
Quoted 135 lines
---
hw/dma/xilinx_axidma.c | 42 ++++++++++++++++++++++++++++++---------- --1 file changed, 30 insertions(+), 12 deletions(-)hw/dma/xilinx_axidma.c +0
-0@@ -181,6 +181,11 @@ static void stream_reset(struct Stream *s) {s->regs[R_DMASR] = DMASR_HALTED; /* starts up halted. */ s->regs[R_DMACR] = 1 << 16; /* Starts with one in compl threshold. */
+ /* Initialize descriptor registers to zero for both 32 and 64 bit */
+ s->regs[R_CURDESC] = 0;
+ s->regs[R_CURDESC + 1] = 0; /*CURDESC_MSB*/
+ s->regs[R_TAILDESC] = 0;
+ s->regs[R_TAILDESC + 1] = 0; /*TAILDESC_MSB*/s->sof= true;
}hw/dma/xilinx_axidma.c +0
-0@@ -194,6 +199,16 @@ static inline int streamid_from_addr(hwaddr addr) return sid; }hw/dma/xilinx_axidma.c +3
-0+static inline hwaddr stream_desc_addr (struct Stream *s) +{ + return s->regs[R_CURDESC] | ((uint64_t)s->regs[R_CURDESC + 1] <<32);
hw/dma/xilinx_axidma.c +5
-0+} + +static inline hwaddr stream_tail_addr (struct Stream *s) +{ + return s->regs[R_TAILDESC] | ((uint64_t)s->regs[R_TAILDESC + 1]<< 32);
hw/dma/xilinx_axidma.c +2
-0+} +static MemTxResult
stream_desc_load(struct Stream *s, hwaddr addr){
struct SDesc *d =&s->desc;hw/dma/xilinx_axidma.c +0
-0@@ -290,9 +305,9 @@ static void stream_complete(struct Stream *s)static void stream_process_mem2s(struct Stream *s, StreamSink
*tx_data_dev,
StreamSink*tx_control_dev) {
-uint32_tprev_d;
-uint32_ttxlen;
+uint32_ttxlen,origin_txlen;
uint64_taddr;
+ hwaddr curdesc;
bool eop;
if (!stream_running(s) ||stream_idle(s)||stream_halted(s)) {hw/dma/xilinx_axidma.c +0
-0@@ -300,7 +315,8 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, }
while (1) {
- if (MEMTX_OK!=stream_desc_load(s, s->regs[R_CURDESC])) { + curdesc =stream_desc_addr(s);
+ if (MEMTX_OK!=stream_desc_load(s, curdesc)) { break;
}hw/dma/xilinx_axidma.c +0
-0@@ -314,6 +330,7 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, }
txlen =s->desc.control&SDESC_CTRL_LEN_MASK; +origin_txlen= txlen;
eop =stream_desc_eof(&s->desc);
addr =s->desc.buffer_address;hw/dma/xilinx_axidma.c +0
-0@@ -334,13 +351,13 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev, }
/* Update the descriptor. */
-s->desc.status= txlen |SDESC_STATUS_COMPLETE; -stream_desc_store(s, s->regs[R_CURDESC]);
+s->desc.status=origin_txlen|SDESC_STATUS_COMPLETE; +stream_desc_store(s, curdesc);
/* Advance. */
-prev_d= s->regs[R_CURDESC]; s->regs[R_CURDESC] =s->desc.nxtdesc;
- if (prev_d== s->regs[R_TAILDESC]) {
+ s->regs[R_CURDESC + 1] =s->desc.nxtdesc>> 32; + if (curdesc ==stream_tail_addr(s)) { s->regs[R_DMASR] |=DMASR_IDLE;
break;
}hw/dma/xilinx_axidma.c +0
-0@@ -350,16 +367,17 @@ static void stream_process_mem2s(struct Stream *s, StreamSink *tx_data_dev,static
size_tstream_process_s2mem(struct Stream *s, unsigned char *buf,
size_tlen, bool eop) {
-uint32_tprev_d;
unsigned int rxlen;
size_tpos = 0;
+ hwaddr curdesc;
if (!stream_running(s) ||stream_idle(s)||stream_halted(s)) { return 0;
}
while (len) {
- if (MEMTX_OK!=stream_desc_load(s, s->regs[R_CURDESC])) { + curdesc =stream_desc_addr(s);
+ if (MEMTX_OK!=stream_desc_load(s, curdesc)) { break;
}hw/dma/xilinx_axidma.c +0
-0@@ -388,13 +406,13 @@ static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf,
s->desc.status|=s->sof<<SDESC_STATUS_SOF_BIT;s->desc.status|=SDESC_STATUS_COMPLETE; -stream_desc_store(s, s->regs[R_CURDESC]); +stream_desc_store(s, curdesc);
s->sof= eop;
/* Advance. */
-prev_d= s->regs[R_CURDESC]; s->regs[R_CURDESC] =s->desc.nxtdesc;
- if (prev_d== s->regs[R_TAILDESC]) {
+ s->regs[R_CURDESC + 1] =s->desc.nxtdesc>> 32; + if (curdesc ==stream_tail_addr(s)) { s->regs[R_DMASR] |=DMASR_IDLE; break;
}
PATCH 2/6 hw/dma/xilinx_axidma: Add 64-bit MMIO access support
In reply to Jay ChangImplement 32-bit and 64-bit descriptor register accesses through dedicated MMIO helpers.
--- hw/dma/xilinx_axidma.c | 199 +++++++++++++++++++++++++++++------------ 1 file changed, 143 insertions(+), 56 deletions(-)
hw/dma/xilinx_axidma.c +143 -56
@@ -56,7 +56,9 @@ DECLARE_INSTANCE_CHECKER(XilinxAXIDMAStreamSink, XILINX_AXI_DMA_CONTROL_STREAM,#define R_DMACR (0x00 / 4)#define R_DMASR (0x04 / 4)#define R_CURDESC (0x08 / 4) +#define R_CURDESC_MSB (0x0C / 4)#define R_TAILDESC (0x10 / 4) +#define R_TAILDESC_MSB (0x14 / 4)#define R_MAX (0x30 / 4)#define CONTROL_PAYLOAD_WORDS 5
@@ -477,12 +479,40 @@ xilinx_axidma_data_stream_push(StreamSink *obj, unsigned char *buf, size_t len,return ret; } -static uint64_t axidma_read (void *opaque,hwaddr addr, -unsigned size) +static uint32_t axidma_readl (XilinxAXIDMA *d,struct Stream *s, +hwaddr addr,int sid) +{ +switch (addr) { +case R_DMACR : +/* Simulate one cycle reset delay. */ + s->regs[R_DMACR ] &= ~DMACR_RESET ; +return s->regs[R_DMACR ]; +case R_DMASR : + s->regs[R_DMASR ] &=0xffff ; + s->regs[R_DMASR ] |= (s->complete_cnt &0xff ) <<16 ; + s->regs[R_DMASR ] |= (ptimer_get_count (s->ptimer) &0xff ) <<24 ; +return s->regs[R_DMASR ]; +default : +D (qemu_log ("%s ch=%d addr=" HWADDR_FMT_plx" v=%x\n" , + __func__, sid, addr *4 , s->regs[addr])); +return s->regs[addr]; + } +} + +static uint64_t axidma_readq (XilinxAXIDMA *d,struct Stream *s, +hwaddr addr,int sid) +{ +/* Combine two consecutive 32-bit slots into a 64-bit value. */ +if (addr +1 <R_MAX ) { +return s->regs[addr] | ((uint64_t )s->regs[addr +1 ] <<32 ); + } +return s->regs[addr]; +} + +static uint64_t axidma_read (void *opaque,hwaddr addr,unsigned size) {XilinxAXIDMA *d = opaque;struct Stream *s; -uint32_t r =0 ;int sid; sid =streamid_from_addr (addr);
@@ -490,26 +520,102 @@ static uint64_t axidma_read(void *opaque, hwaddr addr, addr = addr %0x30 ; addr >>=2 ; + +switch (size) { +case 8 : +return axidma_readq (d, s, addr, sid); +case 4 : +default : +return axidma_readl (d, s, addr, sid); + } +} + +static void axidma_kick (XilinxAXIDMA *d,struct Stream *s,int sid) +{ + s->regs[R_DMASR ] &= ~DMASR_IDLE ; +if (!sid) { +stream_process_mem2s (s, d->tx_data_dev, d->tx_control_dev); + } +} + +static void axidma_writel (XilinxAXIDMA *d,struct Stream *s,hwaddr addr, +uint32_t value,int sid) +{switch (addr) { -case R_DMACR : -/* Simulate one cycles reset delay. */ - s->regs[addr] &= ~DMACR_RESET ; - r = s->regs[addr]; -break ; -case R_DMASR : - s->regs[addr] &=0xffff ; - s->regs[addr] |= (s->complete_cnt &0xff ) <<16 ; - s->regs[addr] |= (ptimer_get_count (s->ptimer) &0xff ) <<24 ; - r = s->regs[addr]; -break ; -default : - r = s->regs[addr]; -D (qemu_log ("%s ch=%d addr=" HWADDR_FMT_plx" v=%x\n" , - __func__, sid, addr *4 , r)); -break ; +case R_DMACR : +/* Tailptr mode is always on. */ + value |=DMACR_TAILPTR_MODE ; +/* Remember our previous reset state. */ + value |= (s->regs[R_DMACR ] &DMACR_RESET ); + s->regs[R_DMACR ] = value; + +if (value &DMACR_RESET ) { +stream_reset (s); + } + +if ((value &1 ) && !stream_resetting (s)) { +/* Start processing. */ + s->regs[R_DMASR ] &= ~(DMASR_HALTED |DMASR_IDLE ); + } +stream_reload_complete_cnt (s); +break ; + +case R_DMASR : +/* Mask away write to clear irq lines. */ + value &= ~(value &DMASR_IRQ_MASK ); + s->regs[R_DMASR ] = value; +break ; + +case R_CURDESC : + s->regs[R_CURDESC ] = value; +break ; + +case R_CURDESC_MSB : + s->regs[R_CURDESC_MSB ] = value; +break ; + +case R_TAILDESC : + s->regs[R_TAILDESC ] = value; +axidma_kick (d, s, sid); +break ; + +case R_TAILDESC_MSB : + s->regs[R_TAILDESC_MSB ] = value; +axidma_kick (d, s, sid); +break ; + +default : +D (qemu_log ("%s: ch=%d addr=" HWADDR_FMT_plx" v=%x\n" , + __func__, sid, addr *4 , value)); + s->regs[addr] = value; +break ; } -return r; +} +static void axidma_writeq (XilinxAXIDMA *d,struct Stream *s,hwaddr addr, +uint64_t value,int sid) +{ +switch (addr) { +case R_CURDESC : + s->regs[R_CURDESC ] = (uint32_t )value; + s->regs[R_CURDESC_MSB ] = (uint32_t )(value >>32 ); +break ; + +case R_TAILDESC : + s->regs[R_TAILDESC ] = (uint32_t )value; + s->regs[R_TAILDESC_MSB ] = (uint32_t )(value >>32 ); +axidma_kick (d, s, sid); +break ; + +default : +D (qemu_log ("%s: ch=%d addr=" HWADDR_FMT_plx" v=%x\n" , + __func__, sid, addr *4 , (unsigned )value)); + s->regs[addr] = (uint32_t )value; +if (addr +1 <R_MAX ) { + s->regs[addr +1 ] = (uint32_t )(value >>32 ); + } +break ; + } } static void axidma_write(void *opaque, hwaddr addr,
@@ -524,44 +630,17 @@ static void axidma_write(void *opaque, hwaddr addr, addr = addr %0x30 ; addr >>=2 ; -switch (addr) { -case R_DMACR : -/* Tailptr mode is always on. */ - value |=DMACR_TAILPTR_MODE ; -/* Remember our previous reset state. */ - value |= (s->regs[addr] &DMACR_RESET ); - s->regs[addr] = value; - -if (value &DMACR_RESET ) { -stream_reset (s); - } - -if ((value &1 ) && !stream_resetting (s)) { -/* Start processing. */ - s->regs[R_DMASR ] &= ~(DMASR_HALTED |DMASR_IDLE ); - } -stream_reload_complete_cnt (s); -break ; - -case R_DMASR : -/* Mask away write to clear irq lines. */ - value &= ~(value &DMASR_IRQ_MASK ); - s->regs[addr] = value; -break ; -case R_TAILDESC : - s->regs[addr] = value; - s->regs[R_DMASR ] &= ~DMASR_IDLE ;/* Not idle. */ -if (!sid) { -stream_process_mem2s (s, d->tx_data_dev, d->tx_control_dev); - } -break ; -default : -D (qemu_log ("%s: ch=%d addr=" HWADDR_FMT_plx" v=%x\n" , - __func__, sid, addr *4 , (unsigned )value)); - s->regs[addr] = value; -break ; +switch (size) { +case 8 : +axidma_writeq (d, s, addr, value, sid); +break ; +case 4 : +default : +axidma_writel (d, s, addr, (uint32_t )value, sid); +break ; } +if (sid ==1 && d->notify) {StreamCanPushNotifyFn notifytmp = d->notify; d->notify =NULL ;
@@ -574,6 +653,14 @@ static const MemoryRegionOps axidma_ops = { .read = axidma_read, .write = axidma_write, .endianness = DEVICE_NATIVE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size =8 + }, + .impl = { + .min_access_size = 4, + .max_access_size = 8 + } };static void xilinx_axidma_realize (DeviceState *dev,Error **errp)
Signature · 1 line
2.48.1
Re: [PATCH 2/6] hw/dma/xilinx_axidma: Add 64-bit MMIO access support
Quoted 6 lines · Jay Chang
Implement 32-bit and 64-bit descriptor register accesses through dedicated MMIO helpers.
Signed-off-by Jay Chang <[email protected]>Reviewed-by Frank Chang <[email protected]>Reviewed-by Jim Shu <[email protected]>
Alistair
Quoted 262 lines
---
hw/dma/xilinx_axidma.c | 199 +++++++++++++++++++++++++++++---------- --1 file changed, 143 insertions(+), 56 deletions(-)hw/dma/xilinx_axidma.c +2
-0@@ -56,7 +56,9 @@ DECLARE_INSTANCE_CHECKER(XilinxAXIDMAStreamSink, XILINX_AXI_DMA_CONTROL_STREAM, #define R_DMACR (0x00 / 4) #define R_DMASR (0x04 / 4) #define R_CURDESC (0x08 / 4) +#define R_CURDESC_MSB (0x0C / 4) #define R_TAILDESC (0x10 / 4) +#define R_TAILDESC_MSB (0x14 / 4) #define R_MAX (0x30 / 4)
#defineCONTROL_PAYLOAD_WORDS5hw/dma/xilinx_axidma.c +0
-0@@ -477,12 +479,40 @@ xilinx_axidma_data_stream_push(StreamSink *obj, unsigned char *buf, size_t len, return ret; }hw/dma/xilinx_axidma.c +12
-2-static uint64_t axidma_read (void *opaque,hwaddr addr, - unsigned size) +static uint32_t axidma_readl (XilinxAXIDMA *d,struct Stream *s, + hwaddr addr, int sid) +{ + switch (addr) { + case R_DMACR: + /* Simulate one cycle reset delay. */ + s->regs[R_DMACR] &= ~DMACR_RESET; + return s->regs[R_DMACR]; + case R_DMASR: + s->regs[R_DMASR] &= 0xffff; + s->regs[R_DMASR] |= (s->complete_cnt & 0xff) << 16; + s->regs[R_DMASR] |= (ptimer_get_count(s->ptimer) & 0xff) <<24;
+ return s->regs[R_DMASR];
+ default:
+ D(qemu_log("%s ch=%d addr="HWADDR_FMT_plx" v=%x\n", +__func__, sid, addr * 4, s->regs[addr])); + return s->regs[addr];
+ }hw/dma/xilinx_axidma.c +13
-0+} + +static uint64_t axidma_readq (XilinxAXIDMA *d,struct Stream *s, + hwaddr addr, int sid) +{ + /* Combine two consecutive 32-bit slots into a 64-bit value. */ + if (addr + 1 < R_MAX) { + return s->regs[addr] | ((uint64_t)s->regs[addr + 1] << 32); + } + return s->regs[addr]; +} + +static uint64_t axidma_read(void *opaque, hwaddr addr, unsignedsize)
{
XilinxAXIDMA *d = opaque;
struct Stream *s;
-uint32_tr = 0;
int sid;
sid =streamid_from_addr(addr);hw/dma/xilinx_axidma.c +0
-0@@ -490,26 +520,102 @@ static uint64_t axidma_read(void *opaque, hwaddr addr,
addr = addr %0x30;
addr >>= 2;hw/dma/xilinx_axidma.c +19
-0+ + switch (size) { + case 8: + return axidma_readq(d, s, addr, sid); + case 4: + default: + return axidma_readl(d, s, addr, sid); + } +} + +static void axidma_kick (XilinxAXIDMA *d,struct Stream *s,int sid) +{ + s->regs[R_DMASR] &= ~DMASR_IDLE; + if (!sid) { + stream_process_mem2s(s, d->tx_data_dev, d->tx_control_dev); + } +} + +static void axidma_writel(XilinxAXIDMA *d, struct Stream *s, hwaddraddr,
+uint32_tvalue, int sid)
+{
switch (addr) {
- caseR_DMACR:
- /* Simulate one cycles reset delay. */
- s->regs[addr] &= ~DMACR_RESET;
- r = s->regs[addr];
- break;
- caseR_DMASR:
- s->regs[addr] &=0xffff;
- s->regs[addr] |= (s->complete_cnt&0xff) << 16; - s->regs[addr] |= (ptimer_get_count(s->ptimer)&0xff) << 24;
- r = s->regs[addr];
- break;
- default:
- r = s->regs[addr];
- D(qemu_log("%s ch=%d addr="HWADDR_FMT_plx" v=%x\n", -__func__, sid, addr * 4, r)); - break;
+ caseR_DMACR:
+ /* Tailptr mode is always on. */
+ value |=DMACR_TAILPTR_MODE;
+ /* Remember our previous reset state. */
+ value |= (s->regs[R_DMACR] &DMACR_RESET);
+ s->regs[R_DMACR] = value;hw/dma/xilinx_axidma.c +41
-0+ + if (value & DMACR_RESET) { + stream_reset(s); + } + + if ((value & 1) && !stream_resetting(s)) { + /* Start processing. */ + s->regs[R_DMASR] &= ~(DMASR_HALTED | DMASR_IDLE); + } + stream_reload_complete_cnt(s); + break; + + case R_DMASR: + /* Mask away write to clear irq lines. */ + value &= ~(value & DMASR_IRQ_MASK); + s->regs[R_DMASR] = value; + break; + + case R_CURDESC: + s->regs[R_CURDESC] = value; + break; + + case R_CURDESC_MSB: + s->regs[R_CURDESC_MSB] = value; + break; + + case R_TAILDESC: + s->regs[R_TAILDESC] = value; + axidma_kick(d, s, sid); + break; + + case R_TAILDESC_MSB: + s->regs[R_TAILDESC_MSB] = value; + axidma_kick(d, s, sid); + break; + + default: + D(qemu_log("%s: ch=%d addr=" HWADDR_FMT_plx " v=%x\n", + __func__, sid, addr * 4, value)); + s->regs[addr] = value; + break;}
- return r;
+}
+static void axidma_writeq(XilinxAXIDMA *d, struct Stream *s, hwaddr addr,
+uint64_tvalue, int sid)hw/dma/xilinx_axidma.c +22
-0+{ + switch (addr) { + case R_CURDESC: + s->regs[R_CURDESC] = (uint32_t)value; + s->regs[R_CURDESC_MSB] = (uint32_t)(value >> 32); + break; + + case R_TAILDESC: + s->regs[R_TAILDESC] = (uint32_t)value; + s->regs[R_TAILDESC_MSB] = (uint32_t)(value >> 32); + axidma_kick(d, s, sid); + break; + + default: + D(qemu_log("%s: ch=%d addr=" HWADDR_FMT_plx " v=%x\n", + __func__, sid, addr * 4, (unsigned)value)); + s->regs[addr] = (uint32_t)value; + if (addr + 1 < R_MAX) { + s->regs[addr + 1] = (uint32_t)(value >> 32); + } + break; + }}
static void axidma_write(void *opaque, hwaddr addr,hw/dma/xilinx_axidma.c +0
-0@@ -524,44 +630,17 @@ static void axidma_write(void *opaque, hwaddr addr,
addr = addr %0x30;
addr >>= 2;
- switch (addr) {
- caseR_DMACR:
- /* Tailptr mode is always on. */ - value |=DMACR_TAILPTR_MODE;
- /* Remember our previous reset state. */ - value |= (s->regs[addr] &DMACR_RESET); - s->regs[addr] = value;hw/dma/xilinx_axidma.c +0
-17- - if (value & DMACR_RESET) { - stream_reset(s); - } - - if ((value & 1) && !stream_resetting(s)) { - /* Start processing. */ - s->regs[R_DMASR] &= ~(DMASR_HALTED | DMASR_IDLE); - } - stream_reload_complete_cnt(s); - break; - - case R_DMASR: - /* Mask away write to clear irq lines. */ - value &= ~(value & DMASR_IRQ_MASK); - s->regs[addr] = value; - break;
- caseR_TAILDESC:
- s->regs[addr] = value;
- s->regs[R_DMASR] &= ~DMASR_IDLE; /* Not idle. */ - if (!sid) {
- stream_process_mem2s(s,d->tx_data_dev, d-Quoted 1 line
tx_control_dev);- }
- break;
- default:
- D(qemu_log("%s: ch=%d addr="HWADDR_FMT_plx" v=%x\n", -__func__, sid, addr * 4, (unsigned)value)); - s->regs[addr] = value;
- break;
+ switch (size) {
+ case 8:
+axidma_writeq(d, s, addr, value, sid);
+ break;
+ case 4:
+ default:
+axidma_writel(d, s, addr, (uint32_t)value, sid); + break;
}
+
if (sid == 1 &&d->notify) {
StreamCanPushNotifyFn notifytmp =d->notify;d->notify= NULL;hw/dma/xilinx_axidma.c +8
-0@@ -574,6 +653,14 @@ static const MemoryRegionOps axidma_ops = { .read = axidma_read, .write = axidma_write, .endianness = DEVICE_NATIVE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size = 8 + }, + .impl = { + .min_access_size = 4, + .max_access_size = 8 + } };
static voidxilinx_axidma_realize(DeviceState *dev, Error **errp)
PATCH 3/6 hw/dma/xilinx_axidma: Fix DMASR register handling
In reply to Jay ChangThis patch fixes two register handling issues in the Xilinx AXI DMA controller:
1. DMACR Start Processing Logic:
When the guest writes to R_DMACR to start DMA processing, the original code cleared both DMASR_HALTED and DMASR_IDLE flags. However, DMASR_IDLE should only be cleared when TAILDESC is written (indicating actual work is queued). Incorrectly clearing IDLE on DMACR write can cause the DMA state machine to behave incorrectly.
2. DMASR W1C (Write-1-to-Clear) IRQ Bits:
The original implementation had incorrect W1C logic:
value &= ~(value & DMASR_IRQ_MASK);
s->regs[addr] = value; // write R/O bitThis would clear the entire register instead of just the IRQ bits. The correct W1C implementation should clear only the bits that are both set in the register AND written as 1 by the guest:
s->regs[addr] &= ~(value & DMASR_IRQ_MASK);
3. Enhanced Error Logging:
Added qemu_log_mask(LOG_GUEST_ERROR) when the DMA engine encounters a descriptor that is already marked COMPLETE. This indicates a guest driver error (reusing descriptors without proper reset) and helps debugging network issues.
--- hw/dma/xilinx_axidma.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
hw/dma/xilinx_axidma.c +7 -4
@@ -385,6 +385,10 @@ static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf,if (s->desc.status &SDESC_STATUS_COMPLETE ) { s->regs[R_DMASR ] |=DMASR_HALTED ; +qemu_log_mask (LOG_GUEST_ERROR , +"xilinx_axidma: HALT: curdesc=0x%016lx already" +" COMPLETE status=0x%x\n" , + (unsigned long )curdesc, s->desc.status);break ; }
@@ -555,15 +559,14 @@ static void axidma_writel(XilinxAXIDMA *d, struct Stream *s, hwaddr addr,if ((value &1 ) && !stream_resetting (s)) {/* Start processing. */ - s->regs[R_DMASR ] &= ~(DMASR_HALTED |DMASR_IDLE ); + s->regs[R_DMASR ] &= ~DMASR_HALTED ; }stream_reload_complete_cnt (s);break ;case R_DMASR : -/* Mask away write to clear irq lines. */ - value &= ~(value &DMASR_IRQ_MASK ); - s->regs[R_DMASR ] = value; +/* Clear W1C IRQ bits. */ + s->regs[R_DMASR ] &= ~(value &DMASR_IRQ_MASK );break ;case R_CURDESC :
Signature · 1 line
2.48.1
Re: [PATCH 3/6] hw/dma/xilinx_axidma: Fix DMASR register handling
Quoted 29 lines · Jay Chang
This patch fixes two register handling issues in the Xilinx AXI DMA controller:
1. DMACR Start Processing Logic:
When the guest writes toR_DMACRto start DMA processing, the original code cleared bothDMASR_HALTEDandDMASR_IDLEflags. However,DMASR_IDLEshould only be cleared when TAILDESC is written
(indicating actual work is queued). Incorrectly clearing IDLE on DMACR write can cause the DMA state machine to behave incorrectly.2. DMASR W1C (Write-1-to-Clear) IRQ Bits:
The original implementation had incorrect W1C logic:
value &= ~(value &DMASR_IRQ_MASK); s->regs[addr] = value; // write R/O bitThis would clear the entire register instead of just the IRQ bits. The correct W1C implementation should clear only the bits that are both set in the register AND written as 1 by the guest:
s->regs[addr] &= ~(value &DMASR_IRQ_MASK);3. Enhanced Error Logging:
Addedqemu_log_mask(LOG_GUEST_ERROR)when the DMA engine encounters
a descriptor that is already marked COMPLETE. This indicates a guest
driver error (reusing descriptors without proper reset) and helps debugging network issues.
Quoted 3 lines
Signed-off-by Jay Chang <[email protected]>Reviewed-by Frank Chang <[email protected]>Reviewed-by Jim Shu <[email protected]>
Alistair
Quoted 41 lines
---
hw/dma/xilinx_axidma.c | 11 +++++++----1 file changed, 7 insertions(+), 4 deletions(-)hw/dma/xilinx_axidma.c +0
-0@@ -385,6 +385,10 @@ static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf,
if (s->desc.status&SDESC_STATUS_COMPLETE) { s->regs[R_DMASR] |=DMASR_HALTED;
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "xilinx_axidma: HALT: curdesc=0x%016lx already"
+ " COMPLETE status=0x%x\n",
+ (unsigned long)curdesc,s->desc.status); break;
}hw/dma/xilinx_axidma.c +0
-0@@ -555,15 +559,14 @@ static void axidma_writel(XilinxAXIDMA *d, struct Stream *s, hwaddr addr,
if ((value & 1) && !stream_resetting(s)) { /* Start processing. */
- s->regs[R_DMASR] &= ~(DMASR_HALTED |DMASR_IDLE); + s->regs[R_DMASR] &= ~DMASR_HALTED;
}
stream_reload_complete_cnt(s);
break;
caseR_DMASR:
- /* Mask away write to clear irq lines. */ - value &= ~(value &DMASR_IRQ_MASK);
- s->regs[R_DMASR] = value;
+ /* Clear W1C IRQ bits. */
+ s->regs[R_DMASR] &= ~(value &DMASR_IRQ_MASK); break;
caseR_CURDESC:
PATCH 4/6 hw/dma/xilinx_axidma: Reset both channels on DMACR_RESET
In reply to Jay ChangSpec says setting either MM2S_DMACR.Reset = 1 or S2MM_DMACR.Reset = 1 resets the entire AXI DMA engine.
Also replace magic number '1' with DMACR_RUNSTOP for clarity.
--- hw/dma/xilinx_axidma.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
hw/dma/xilinx_axidma.c +3 -2
@@ -554,10 +554,11 @@ static void axidma_writel(XilinxAXIDMA *d, struct Stream *s, hwaddr addr, s->regs[R_DMACR ] = value;if (value &DMACR_RESET ) { -stream_reset (s); +stream_reset (&d->streams[0 ]); +stream_reset (&d->streams[1 ]); } -if ((value &1 ) && !stream_resetting (s)) { +if ((value &DMACR_RUNSTOP ) && !stream_resetting (s)) {/* Start processing. */ s->regs[R_DMASR ] &= ~DMASR_HALTED ; }
Signature · 1 line
2.48.1
Re: [PATCH 4/6] hw/dma/xilinx_axidma: Reset both channels on DMACR_RESET
Quoted 8 lines · Jay Chang
Spec says setting either MM2S_DMACR.Reset = 1 or S2MM_DMACR.Reset = 1 resets the entire AXI DMA engine.
Also replace magic number '1' with
DMACR_RUNSTOPfor clarity.Signed-off-by Jay Chang <[email protected]>Reviewed-by Frank Chang <[email protected]>Reviewed-by Jim Shu <[email protected]>
Alistair
Quoted 23 lines
---
hw/dma/xilinx_axidma.c | 5 +++--1 file changed, 3 insertions(+), 2 deletions(-)hw/dma/xilinx_axidma.c +0
-0@@ -554,10 +554,11 @@ static void axidma_writel(XilinxAXIDMA *d, struct Stream *s, hwaddr addr, s->regs[R_DMACR] = value;
if (value &DMACR_RESET) {
-stream_reset(s);
+stream_reset(&d->streams[0]);
+stream_reset(&d->streams[1]);
}
- if ((value & 1) && !stream_resetting(s)) {
+ if ((value &DMACR_RUNSTOP) && !stream_resetting(s)) { /* Start processing. */ s->regs[R_DMASR] &= ~DMASR_HALTED;
}
PATCH 5/6 hw/dma/xilinx_axidma: Don't start transfer on TAILDESC write when halted
In reply to Jay ChangSpec: "If the AXI DMA Channel is halted (DMASR.Halted = 1 and DMACR.RS = 0), a write by the CPU to the TAILDESC_PTR register has no effect except to reposition the pause point."
Check DMASR.Halted and DMACR.RS in axidma_kick() so that a TAILDESC write while the channel is halted only updates the pointer and does not start a transfer.
--- hw/dma/xilinx_axidma.c | 4 ++++ 1 file changed, 4 insertions(+)
hw/dma/xilinx_axidma.c +4 -0
@@ -536,6 +536,10 @@ static uint64_t axidma_read(void *opaque, hwaddr addr, unsigned size)static void axidma_kick (XilinxAXIDMA *d,struct Stream *s,int sid) { +if (stream_halted (s) && !stream_running (s)) { +return ; + } + s->regs[R_DMASR ] &= ~DMASR_IDLE ;if (!sid) {stream_process_mem2s (s, d->tx_data_dev, d->tx_control_dev);
Signature · 1 line
2.48.1
Re: [PATCH 5/6] hw/dma/xilinx_axidma: Don't start transfer on TAILDESC write when halted
Quoted 11 lines · Jay Chang
Spec: "If the AXI DMA Channel is halted (DMASR.Halted = 1 and DMACR.RS = 0), a write by the CPU to the
TAILDESC_PTRregister has no effect except to reposition the pause point."Check DMASR.Halted and DMACR.RS in
axidma_kick()so that a TAILDESC write while the channel is halted only updates the pointer and does not start a transfer.Signed-off-by Jay Chang <[email protected]>Reviewed-by Frank Chang <[email protected]>Reviewed-by Jim Shu <[email protected]>
Alistair
Quoted 20 lines
---
hw/dma/xilinx_axidma.c | 4 ++++1 file changed, 4 insertions(+)hw/dma/xilinx_axidma.c +0
-0@@ -536,6 +536,10 @@ static uint64_t axidma_read(void *opaque, hwaddr addr, unsigned size)
static voidaxidma_kick(XilinxAXIDMA *d, struct Stream *s, int sid){
+ if (stream_halted(s)&& !stream_running(s)) {
+ return;
+ }
+
s->regs[R_DMASR] &= ~DMASR_IDLE;
if (!sid) {
stream_process_mem2s(s, d->tx_data_dev, d->tx_control_dev);
PATCH 6/6 hw/net/xilinx_axienet: Fix packet double-buffering issue
In reply to Jay ChangIn the Xilinx AXI Ethernet device model, U-Boot network commands (ping, dhcp, tftp) all fail due to a packet duplication bug.
The original implementation had a packet duplication issue where the same packet could exist in both the internal buffer and the incoming queue:
Problem Flow:
1. eth_rx() receives a packet from the network layer.
2. Packet is processed and stored in s->rxmem buffer.
3. axienet_eth_rx_notify() is called to push data to DMA.
4. If DMA cannot accept data immediately, the rxpos is 0 and wth_rx will return 0
5. Because eth_rx callback returns 0, the network layer queues this packet again in the incoming queue.
6. Now the same packet exists in TWO places:
- Already copied in s->rxmem buffer
- Queued in the incoming packet queue
7. This causes duplicate packets to be delivered to the guest
8. Network protocols fail due to receiving identical packets twice
Root Cause:eth_rx() did not check if the DMA stream could accept data before copying the packet into the internal buffer. This caused unnecessary buffering and potential double-processing.
Fix:
1. Check stream_can_push() at the beginning of eth_rx()
- If DMA cannot accept data, set need_flush and return 0 immediately
- This prevents packet buffering when DMA is not ready
- The packet will be redelivered by QEMU's network layer
2. Move need_flush check outside the while loop in axienet_eth_rx_notify()
- Ensures flush happens even when rxsize == 0
- Handles edge cases where the buffer is already empty but flush is needed
--- hw/net/xilinx_axienet.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
hw/net/xilinx_axienet.c +12 -0
@@ -699,6 +699,12 @@ static void axienet_eth_rx_notify(void *opaque) } } } + +/* Check need_flush outside the loop for an empty RX buffer. */ +if (!s->rxsize && s->need_flush) { + s->need_flush = false; +qemu_flush_queued_packets (qemu_get_queue (s->nic)); + }enet_update_irq (s); }
@@ -722,6 +728,12 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)return 0 ; } +/* Check if DMA can accept data before processing packet */ +if (!stream_can_push (s->tx_data_dev, axienet_eth_rx_notify, s)) { + s->need_flush = true; +return 0 ; + } + unicast = ~buf[0 ] &0x1 ; broadcast =memcmp (buf, sa_bcast,6 ) ==0 ; multicast = !unicast && !broadcast;
Signature · 1 line
2.48.1
Re: [PATCH 6/6] hw/net/xilinx_axienet: Fix packet double-buffering issue
Quoted 40 lines · Jay Chang
In the Xilinx AXI Ethernet device model, U-Boot network commands (ping, dhcp, tftp) all fail due to a packet duplication bug.
The original implementation had a packet duplication issue where the same
packet could exist in both the internal buffer and the incoming queue:Problem Flow:
1.eth_rx()receives a packet from the network layer.
2. Packet is processed and stored ins->rxmembuffer.
3.axienet_eth_rx_notify()is called to push data to DMA.
4. If DMA cannot accept data immediately, the rxpos is 0 andwth_rxwill return 0
5. Becauseeth_rxcallback returns 0, the network layer queues this packet again in the incoming queue.
6. Now the same packet exists in TWO places:
- Already copied ins->rxmembuffer
- Queued in the incoming packet queue
7. This causes duplicate packets to be delivered to the guest
8. Network protocols fail due to receiving identical packets twiceRoot Cause:
eth_rx()did not check if the DMA stream could accept data before copying the packet into the internal buffer. This caused unnecessary buffering and potential double-processing.Fix:
1. Checkstream_can_push()at the beginning ofeth_rx()
- If DMA cannot accept data, setneed_flushand return 0 immediately
- This prevents packet buffering when DMA is not ready
- The packet will be redelivered by QEMU's network layer2. Move
need_flushcheck outside the while loop inaxienet_eth_rx_notify()
- Ensures flush happens even when rxsize == 0
- Handles edge cases where the buffer is already empty but flush is needed
Quoted 3 lines
Signed-off-by Jay Chang <[email protected]>Reviewed-by Frank Chang <[email protected]>Reviewed-by Jim Shu <[email protected]>
Alistair
Quoted 36 lines
---
hw/net/xilinx_axienet.c | 12 ++++++++++++1 file changed, 12 insertions(+)hw/net/xilinx_axienet.c +6
-0@@ -699,6 +699,12 @@ static void axienet_eth_rx_notify(void *opaque) } } } + + /* Check need_flush outside the loop for an empty RX buffer. */ + if (!s->rxsize && s->need_flush) { + s->need_flush = false; + qemu_flush_queued_packets(qemu_get_queue(s->nic)); + } enet_update_irq(s); }hw/net/xilinx_axienet.c +0
-0@@ -722,6 +728,12 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size) return 0; }
+ /* Check if DMA can accept data before processing packet */ + if (!stream_can_push(s->tx_data_dev,axienet_eth_rx_notify, s)) {
+s->need_flush= true;
+ return 0;
+ }
+
unicast = ~buf[0] &0x1;
broadcast =memcmp(buf, sa_bcast, 6)== 0;
multicast = !unicast && !broadcast;
Alistair
AppliedQuoted 19 lines · Jay Chang
The Xilinx AXI DMA controller supports 64-bit descriptor addresses, but the QEMU model only handled the low 32 bits during stream processing. This causes descriptor accesses to use incorrect addresses when descriptors are located above 4 GiB.
This series updates the descriptor and MMIO handling and fixes related
AXI DMA and AXI Ethernet behavior.The series:
- Add 64-bit descriptor address handling to stream processing.
- Add separate 32-bit and 64-bit MMIO access helpers.
- Fix DMASR W1C and DMACR start processing behavior.
- Reset both DMA channels whenDMACR_RESETis written.
- Do not start a transfer when TAILDESC is written while the channel is halted.
- Fix AXI Ethernet RX packet double-buffering.
Quoted 3 lines
Testing:
- Tested Xilinx AXI Ethernet networking with U-Boot.
- Tested DHCP and ping.
Thanks!
Applied to riscv-to-apply.next
Alistair
Quoted 16 lines
Jay Chang (6):
hw/dma/xilinx_axidma: Support 64-bit descriptor addressing in stream
processing
hw/dma/xilinx_axidma: Add 64-bit MMIO access support hw/dma/xilinx_axidma: Fix DMASR register handling hw/dma/xilinx_axidma: Reset both channels onDMACR_RESEThw/dma/xilinx_axidma: Don't start transfer on TAILDESC write when halted
hw/net/xilinx_axienet: Fix packet double-buffering issuehw/dma/xilinx_axidma.c | 249 +++++++++++++++++++++++++++++--------- --
hw/net/xilinx_axienet.c | 12 ++2 files changed, 193 insertions(+), 68 deletions(-)