[PATCH 2/2] hw/net/bcm2838_genet: fix per-ring DMA status and RX ring selection
Marcelo Manzo <[email protected]> Sat, 25 Jul 2026 19:11:42 -0400
| Newsgroups | org.nongnu.qemu-arm,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
Three related GENET DMA-ring bugs, all hit by newer guest drivers that actually use multiple RX/TX rings instead of relying on the single default ring: - BCM2838_GENET_RDMA_CTRL writes never updated the RDMA status register at all (only TDMA_CTRL updated a status field, and only a single overall DISABLED bit, not per-ring state). Track a proper per-ring enable bitmask (bits 0-17, one per ring plus the default ring) for both RDMA and TDMA, computed directly from the ctrl register's enable bits on every write. - Enabling a ring via RDMA_CTRL didn't flush any packets that had been queued while the ring was disabled, so traffic arriving in the window before a ring was enabled was silently dropped instead of delivered once it came up. Call qemu_flush_queued_packets() when the EN bit is set. - bcm2838_genet_receive()'s fallback path hardcoded ring BCM2838_GENET_DMA_RING_CNT - 1 (the last/default ring) whenever a packet didn't match a specific filter, with no check that this ring was actually active. Older guest kernels default to this ring, but newer ones (observed: Debian 13/Trixie's 6.18 kernel) actively use rings 0-4 and never enable the default ring at all, so every unfiltered packet was silently dropped. Fall back to scanning for the first actually-active ring instead of assuming the last one. Reset now initializes both status registers to "all rings enabled" to match real hardware defaults. Verified against real Raspberry Pi OS images under the patched raspi4b machine: Bullseye (5.15) and Bookworm (6.12) worked with the old code (they use the default ring); Trixie (6.18) did not get a working DHCP lease over GENET without this fix and does with it. Signed-off-by: Marcelo Manzo <[email protected]> --- hw/net/bcm2838_genet.c | 24 +++++++++++++++++++++--- include/hw/net/bcm2838_genet.h | 2 ++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/hw/net/bcm2838_genet.c b/hw/net/bcm2838_genet.c index 43583aba64..a9798073dc 100644 --- a/hw/net/bcm2838_genet.c +++ b/hw/net/bcm2838_genet.c @@ -132,6 +132,8 @@ FIELD(GENET_DMA_STATUS, DISABLED, 0, 1) FIELD(GENET_DMA_STATUS, DESC_RAM_INIT_BUSY, 1, 1) FIELD(GENET_DMA_STATUS, RSVD_2_31, 2, 30) +#define GENET_DMA_ENABLE_MASK ((1U << 18) - 1) + REG32(GENET_RDMA_LENGTH_STATUS, 0) FIELD(GENET_RDMA_LENGTH_STATUS, OVERRUN, 0, 1) FIELD(GENET_RDMA_LENGTH_STATUS, CRC_ERROR, 1, 1) @@ -622,10 +624,8 @@ static void bcm2838_genet_tdma(BCM2838GenetState *s, hwaddr offset, } break; case BCM2838_GENET_TDMA_CTRL: + s->regs.tdma.status = (~dma_ctrl) & GENET_DMA_ENABLE_MASK; if (exst_tdma_en != incm_tdma_en) { - s->regs.tdma.status = FIELD_DP32(s->regs.tdma.status, - GENET_DMA_STATUS, - DISABLED, !exst_tdma_en); trace_bcm2838_genet_tx_dma(incm_tdma_en == 1 ? "enabled" : "disabled"); @@ -727,6 +727,9 @@ static void bcm2838_genet_write(void *opaque, hwaddr offset, uint64_t value, s->regs.intrl0.stat = FIELD_DP32(s->regs.intrl0.stat, GENET_INTRL_0, MDIO_DONE, 1); break; + case BCM2838_GENET_RDMA_CTRL: + s->regs.rdma.status = (~value) & GENET_DMA_ENABLE_MASK; + break; case BCM2838_GENET_TDMA_REGS ... BCM2838_GENET_TDMA_REGS + sizeof(BCM2838GenetRegsTdma) - 1: bcm2838_genet_tdma(s, offset, value); @@ -736,6 +739,10 @@ static void bcm2838_genet_write(void *opaque, hwaddr offset, uint64_t value, } memcpy((uint8_t *)&s->regs + offset, &value, size); + if (offset == BCM2838_GENET_RDMA_CTRL && + FIELD_EX32(value, GENET_DMA_CTRL, EN)) { + qemu_flush_queued_packets(ncs); + } bcm2838_genet_set_irq_default(s); bcm2838_genet_set_irq_prio(s); } else { @@ -927,6 +934,15 @@ static ssize_t bcm2838_genet_receive(NetClientState *nc, const uint8_t *buf, ring_index = bcm2838_genet_filter2ring(s, filter_index); } else { ring_index = BCM2838_GENET_DMA_RING_CNT - 1; + if (!bcm2838_genet_rdma_ring_active(s, ring_index)) { + for (ring_index = 0; + ring_index < BCM2838_GENET_DMA_RING_CNT - 1; + ring_index++) { + if (bcm2838_genet_rdma_ring_active(s, ring_index)) { + break; + } + } + } } if (size <= MAX_PACKET_SIZE) { @@ -1063,6 +1079,8 @@ static void bcm2838_genet_reset(Object *obj, ResetType type) MAJOR_REV, BCM2838_GENET_REV_MAJOR); s->regs.sys.rev_ctrl = FIELD_DP32(s->regs.sys.rev_ctrl, GENET_SYS_REV_CTRL, MINOR_REV, BCM2838_GENET_REV_MINOR); + s->regs.rdma.status = GENET_DMA_ENABLE_MASK; + s->regs.tdma.status = GENET_DMA_ENABLE_MASK; trace_bcm2838_genet_reset("done"); diff --git a/include/hw/net/bcm2838_genet.h b/include/hw/net/bcm2838_genet.h index 5944e0cef9..960042f6f4 100644 --- a/include/hw/net/bcm2838_genet.h +++ b/include/hw/net/bcm2838_genet.h @@ -55,6 +55,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(BCM2838GenetState, BCM2838_GENET) #define BCM2838_GENET_TDMA_RINGS BCM2838_GENET_TDMA_REG(rings) #define BCM2838_GENET_TDMA_RING_CFG BCM2838_GENET_TDMA_REG(ring_cfg) #define BCM2838_GENET_TDMA_CTRL BCM2838_GENET_TDMA_REG(ctrl) +#define BCM2838_GENET_TDMA_STATUS BCM2838_GENET_TDMA_REG(status) #define BCM2838_GENET_RDMA_REGS offsetof(BCM2838GenetRegs, rdma) #define BCM2838_GENET_RDMA_REG(reg) (BCM2838_GENET_RDMA_REGS \ @@ -62,6 +63,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(BCM2838GenetState, BCM2838_GENET) #define BCM2838_GENET_RDMA_RINGS BCM2838_GENET_RDMA_REG(rings) #define BCM2838_GENET_RDMA_RING_CFG BCM2838_GENET_RDMA_REG(ring_cfg) #define BCM2838_GENET_RDMA_CTRL BCM2838_GENET_RDMA_REG(ctrl) +#define BCM2838_GENET_RDMA_STATUS BCM2838_GENET_RDMA_REG(status) #define BCM2838_GENET_TRING_REG(reg) offsetof(BCM2838GenetTdmaRing, reg) #define BCM2838_GENET_TRING_WRITE_PTR BCM2838_GENET_TRING_REG(write_ptr) -- 2.47.1