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
