Commit f96634eee4 for qemu.org
commit f96634eee4b5f145ed97e24d117e0a92a6a4b3a8
Author: Lorand Marton <lorandmarton.dev@gmail.com>
Date: Mon Nov 9 13:52:08 2026 +0000
hw/net/bcm2838_genet: hold RX frames while the ring is full
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 <lorandmarton.dev@gmail.com>
Tested-by: Marcelo Manzo <marcelomanzo@gmail.com>
Message-id: fdaf96319d81b2b6602822fd5881a5b5a72e5daa.1793613972.git.lorandmarton.dev@gmail.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
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();
}