Hi Lorand, I tested this patch together with 1/2 on our v6 series, running a stock Raspberry Pi OS Trixie guest. A 300 MB download through GENET completed at about 28 MB/s with no timeouts or checksum errors in the guest log, and the new tests/qtest/bcm2838-genet-test.c passes.
Tested-by: Marcelo Manzo <[email protected]> Marcelo On Mon, Nov 02, 2026 05:51 AM, Lorand Marton <[email protected]> wrote: > bcm2838_genet_rdma() never looked at the consumer index of the RX > ring: it wrote each frame to the next descriptor whatever the guest > had read, so a burst longer than the ring (Linux bcmgenet gives the > default ring 256 descriptors) overwrote frames the guest had not taken > yet, and the producer index ran more than a ring ahead of the consumer > index, which bcmgenet_desc_rx() does not expect. > > Count the free descriptors of the ring from its start and end > addresses and the two indexes. When there are none, return 0 so that > the net layer keeps the frame queued, and flush the queue when the > guest writes the consumer index of an RX ring, as e1000 does on a > write of RDT. If the ring fills inside a frame that takes several > descriptors, stop there instead of writing past the consumer index. > > Real GENET drops such frames and counts them in the DISCARD_CNT field > of the producer index; holding them costs no frame on a slow host and > matches the other QEMU NICs. > > Test that a ring of 4 descriptors keeps 6 frames apart until the > guest moves its consumer index. > > Signed-off-by: Lorand Marton <[email protected]> > --- > hw/net/bcm2838_genet.c | 38 +++++++++++ > tests/qtest/bcm2838-genet-test.c | 107 ++++++++++++++++++++++++++++++- > 2 files changed, 143 insertions(+), 2 deletions(-) > > diff --git a/hw/net/bcm2838_genet.c b/hw/net/bcm2838_genet.c > index 557a8a9636..92d550cb63 100644 > --- a/hw/net/bcm2838_genet.c > +++ b/hw/net/bcm2838_genet.c > @@ -779,6 +779,13 @@ static void bcm2838_genet_write(void *opaque, hwaddr > offset, uint64_t value, > FIELD_EX32(value, GENET_DMA_CTRL, EN)) { > qemu_flush_queued_packets(ncs); > } > + /* The guest gave descriptors back to an RX ring */ > + if (offset >= BCM2838_GENET_RDMA_RINGS && > + offset < BCM2838_GENET_RDMA_RINGS + sizeof(s->regs.rdma.rings) && > + (offset - BCM2838_GENET_RDMA_RINGS) % sizeof(BCM2838GenetRdmaRing) > + == BCM2838_GENET_RRING_CONS_INDEX) { > + qemu_flush_queued_packets(ncs); > + } > bcm2838_genet_set_irq_default(s); > bcm2838_genet_set_irq_prio(s); > } > @@ -823,6 +830,23 @@ static bool is_packet_multicast(const uint8_t *buf, > size_t size) > return !!(buf[0] & 0x01); > } > > +/* > + * Descriptors of an RX ring that the guest has handed back: the ring > + * holds END - START + 1 of them, and PROD - CONS (modulo 2^16) are full. > + */ > +static unsigned bcm2838_genet_rdma_free_descs(BCM2838GenetRdmaRing *ring, > + hwaddr start_index, > + hwaddr end_index) > +{ > + uint16_t used = FIELD_EX32(ring->prod_index, GENET_DMA_PROD_INDEX, > INDEX) > + - FIELD_EX32(ring->cons_index, GENET_DMA_CONS_INDEX, > INDEX); > + > + if (end_index < start_index || used > end_index - start_index) { > + return 0; > + } > + return end_index - start_index + 1 - used; > +} > + > static ssize_t bcm2838_genet_rdma(BCM2838GenetState *s, uint32_t > ring_idx, > const void *buf, size_t size) > { > @@ -855,6 +879,12 @@ static ssize_t bcm2838_genet_rdma(BCM2838GenetState > *s, uint32_t ring_idx, > BCM2838_GENET_DMA_DESC_CNT - 1); > return -1; > } > + > + /* A full ring: keep the frame queued until the guest frees some */ > + if (!bcm2838_genet_rdma_free_descs(ring, start_index, end_index)) { > + return 0; > + } > + > desc = &rdma->descs[write_index]; > > desc->length_status = FIELD_DP32(desc->length_status, > @@ -948,6 +978,14 @@ static ssize_t bcm2838_genet_rdma(BCM2838GenetState > *s, uint32_t ring_idx, > desc = &rdma->descs[write_index]; > ring->write_ptr = write_index * desc_word_size; > ring->write_ptr_hi = ((hwaddr)write_index * desc_word_size) >> 32; > + > + if (len < size && > + !bcm2838_genet_rdma_free_descs(ring, start_index, > end_index)) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: RX ring %u full inside a frame\n", > + __func__, ring_idx); > + break; > + } > } > > if (ring_idx == BCM2838_GENET_DMA_RING_DEFAULT) { > diff --git a/tests/qtest/bcm2838-genet-test.c > b/tests/qtest/bcm2838-genet-test.c > index 393abd7da2..35653773d1 100644 > --- a/tests/qtest/bcm2838-genet-test.c > +++ b/tests/qtest/bcm2838-genet-test.c > @@ -1,8 +1,8 @@ > /* > * QTest testcase for the BCM2838 GENET DMA rings > * > - * Frames go out over a socket netdev. The producer and consumer indexes > - * of a ring count modulo 2^16. > + * Frames come in and go out over a socket netdev. The producer and > + * consumer indexes of a ring count modulo 2^16. > * > * SPDX-License-Identifier: GPL-2.0-or-later > */ > @@ -14,8 +14,16 @@ > > #define GENET_BASE 0xfd580000 > > +#define RDMA_DESC(i) (0x2000 + (i) * 12) > +#define RDMA_RING(r) (0x2c00 + (r) * 0x40) > +#define RING_WRITE_PTR 0x00 > +#define RING_PROD_INDEX 0x08 > +#define RING_CONS_INDEX 0x0c > +#define RING_BUF_SIZE 0x10 > #define RING_START_ADDR 0x14 > #define RING_END_ADDR 0x1c > +#define RDMA_RING_CFG 0x3040 > +#define RDMA_CTRL 0x3044 > > #define TDMA_DESC(i) (0x4000 + (i) * 12) > #define TDMA_RING(r) (0x4c00 + (r) * 0x40) > @@ -26,9 +34,11 @@ > #define TDMA_CTRL 0x5044 > #define TSB_LEN 64 /* transmit status block */ > > +#define DEFAULT_RING 16 > #define DESCS_PER_RING 4 > #define BUF_SIZE 2048 > #define BUF_BASE 0x100000 > +#define STATUS_LEN 64 /* receive status block, then 2 bytes > */ > #define FRAME_LEN 60 > > static void genet_writel(QTestState *qts, uint32_t off, uint32_t val) > @@ -41,6 +51,98 @@ static uint32_t genet_readl(QTestState *qts, uint32_t > off) > return qtest_readl(qts, GENET_BASE + off); > } > > +/* The default ring uses the first DESCS_PER_RING descriptors */ > +static QTestState *genet_rx_init(int *fd) > +{ > + const unsigned r = DEFAULT_RING; > + int sv[2]; > + QTestState *qts; > + > + g_assert_cmpint(socketpair(PF_UNIX, SOCK_STREAM, 0, sv), ==, 0); > + qts = qtest_initf("-M raspi4b -nic socket,fd=%d,model=bcm2838-genet", > + sv[1]); > + close(sv[1]); > + *fd = sv[0]; > + > + for (unsigned i = 0; i < DESCS_PER_RING; i++) { > + genet_writel(qts, RDMA_DESC(i) + 4, BUF_BASE + i * BUF_SIZE); > + genet_writel(qts, RDMA_DESC(i) + 8, 0); > + } > + genet_writel(qts, RDMA_RING(r) + RING_START_ADDR, 0); > + genet_writel(qts, RDMA_RING(r) + RING_END_ADDR, (DESCS_PER_RING - 1) > * 3); > + genet_writel(qts, RDMA_RING(r) + RING_WRITE_PTR, 0); > + genet_writel(qts, RDMA_RING(r) + RING_BUF_SIZE, BUF_SIZE); > + genet_writel(qts, RDMA_RING_CFG, BIT(r)); > + genet_writel(qts, RDMA_CTRL, 1 | BIT(1 + r)); > + return qts; > +} > + > +static uint32_t prod_index(QTestState *qts, unsigned r) > +{ > + return genet_readl(qts, RDMA_RING(r) + RING_PROD_INDEX) & 0xffff; > +} > + > +/* Send a broadcast frame into the socket netdev */ > +static void send_frame(int fd, uint16_t ethertype, uint8_t byte14) > +{ > + uint8_t pkt[4 + FRAME_LEN] = { 0 }; > + > + stl_be_p(pkt, FRAME_LEN); > + memset(pkt + 4, 0xff, 6); > + stw_be_p(pkt + 4 + 12, ethertype); > + pkt[4 + 14] = byte14; > + g_assert_cmpint(send(fd, pkt, sizeof(pkt), 0), ==, sizeof(pkt)); > +} > + > +/* The byte at 14 of the frame that descriptor I holds */ > +static uint8_t rx_byte14(QTestState *qts, unsigned i) > +{ > + return qtest_readb(qts, BUF_BASE + i * BUF_SIZE + STATUS_LEN + 2 + > 14); > +} > + > +/* Wait up to MS milliseconds for the producer index of ring R to be N */ > +static bool wait_prod_index(QTestState *qts, unsigned r, uint32_t n, > + unsigned ms) > +{ > + gint64 end = g_get_monotonic_time() + ms * G_TIME_SPAN_MILLISECOND; > + > + do { > + qtest_clock_step(qts, 100); > + if (prod_index(qts, r) == n) { > + return true; > + } > + } while (g_get_monotonic_time() < end); > + return false; > +} > + > +/* A full RX ring keeps the next frames until the guest frees descriptors > */ > +static void test_rx_ring_full(void) > +{ > + const unsigned n = DESCS_PER_RING + 2; > + int fd; > + QTestState *qts = genet_rx_init(&fd); > + > + for (unsigned i = 0; i < n; i++) { > + send_frame(fd, 0x0800, i); > + } > + g_assert(wait_prod_index(qts, DEFAULT_RING, DESCS_PER_RING, 5000)); > + /* the rest must not overwrite descriptors the guest has not read */ > + g_assert(!wait_prod_index(qts, DEFAULT_RING, DESCS_PER_RING + 1, > 500)); > + for (unsigned i = 0; i < DESCS_PER_RING; i++) { > + g_assert_cmpuint(rx_byte14(qts, i), ==, i); > + } > + > + genet_writel(qts, RDMA_RING(DEFAULT_RING) + RING_CONS_INDEX, 2); > + g_assert(wait_prod_index(qts, DEFAULT_RING, n, 5000)); > + for (unsigned i = 0; i < n; i++) { > + g_assert_cmpuint(rx_byte14(qts, i % DESCS_PER_RING), ==, > + i < 2 ? i + DESCS_PER_RING : i); > + } > + > + qtest_quit(qts); > + close(fd); > +} > + > /* Transmit across the wrap of the 16-bit consumer index */ > static void test_tx_index_wrap(void) > { > @@ -96,6 +198,7 @@ static void test_tx_index_wrap(void) > int main(int argc, char **argv) > { > g_test_init(&argc, &argv, NULL); > + qtest_add_func("/bcm2838-genet/rx-ring-full", test_rx_ring_full); > qtest_add_func("/bcm2838-genet/tx-index-wrap", test_tx_index_wrap); > return g_test_run(); > } > -- > 2.56.0 > >
