Hi Lorand,

Thanks for the fix. I reproduced the hang on our v6 series: with a stock
Raspberry Pi OS Trixie guest, a 400 MB upload through GENET dies after
roughly 47 MB (about 65k TX descriptors), and the guest stops responding.
With this patch applied, the same 400 MB upload completes in about 13
seconds, and the guest stays healthy afterwards. The loop was inherited
from the original code and our earlier, lighter transfer tests never
reached the 16-bit wrap, so thank you for catching it.

The change itself looks correct to me: masking the consumer index with
R_GENET_DMA_CONS_INDEX_INDEX_MASK keeps it consistent with the 16-bit
producer index.

Reviewed-by: Marcelo Manzo <[email protected]>
Tested-by: Marcelo Manzo <[email protected]>

Marcelo

On Mon, Nov 02, 2026 05:47 AM, Lorand Marton <[email protected]>
wrote:

> The producer and consumer indexes of a GENET DMA ring are 16-bit
> counters (Linux bcmgenet masks them with DMA_P_INDEX_MASK and
> DMA_C_INDEX_MASK). bcm2838_genet_tx() counted the consumer index up in
> a uint32_t and compared it with the 16-bit producer index, so the first
> transmit that crossed the wrap (consumer 0xfffe, producer 0x0001) never
> ended the loop: the device sent the ring's descriptors forever with the
> BQL held, and the guest and the monitor hung. A guest sending a
> sustained stream hung after about 65k descriptors on one ring.
>
> Wrap the index, and test a transmit across the wrap.
>
> Signed-off-by: Lorand Marton <[email protected]>
> ---
>  hw/net/bcm2838_genet.c           |   4 +-
>  tests/qtest/bcm2838-genet-test.c | 101 +++++++++++++++++++++++++++++++
>  tests/qtest/meson.build          |   2 +
>  3 files changed, 106 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qtest/bcm2838-genet-test.c
>
> diff --git a/hw/net/bcm2838_genet.c b/hw/net/bcm2838_genet.c
> index 53bba99b38..557a8a9636 100644
> --- a/hw/net/bcm2838_genet.c
> +++ b/hw/net/bcm2838_genet.c
> @@ -521,7 +521,9 @@ static uint64_t bcm2838_genet_tx(BCM2838GenetState
> *s, unsigned int ring_index,
>          }
>
>          num_descs++;
> -        cons_index_fld++;
> +        /* The indexes count modulo 2^16 */
> +        cons_index_fld = (cons_index_fld + 1)
> +                         & R_GENET_DMA_CONS_INDEX_INDEX_MASK;
>          s->regs.tdma.descs[desc_index].length_status =
>              FIELD_DP32(s->regs.tdma.descs[desc_index].length_status,
>                         GENET_RDMA_LENGTH_STATUS, OWN, 1);
> diff --git a/tests/qtest/bcm2838-genet-test.c
> b/tests/qtest/bcm2838-genet-test.c
> new file mode 100644
> index 0000000000..393abd7da2
> --- /dev/null
> +++ b/tests/qtest/bcm2838-genet-test.c
> @@ -0,0 +1,101 @@
> +/*
> + * 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.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/bitops.h"
> +#include "qemu/bswap.h"
> +#include "libqtest.h"
> +
> +#define GENET_BASE          0xfd580000
> +
> +#define RING_START_ADDR     0x14
> +#define RING_END_ADDR       0x1c
> +
> +#define TDMA_DESC(i)        (0x4000 + (i) * 12)
> +#define TDMA_RING(r)        (0x4c00 + (r) * 0x40)
> +#define TRING_READ_PTR      0x00
> +#define TRING_CONS_INDEX    0x08
> +#define TRING_PROD_INDEX    0x0c
> +#define TDMA_RING_CFG       0x5040
> +#define TDMA_CTRL           0x5044
> +#define TSB_LEN             64      /* transmit status block */
> +
> +#define DESCS_PER_RING      4
> +#define BUF_SIZE            2048
> +#define BUF_BASE            0x100000
> +#define FRAME_LEN           60
> +
> +static void genet_writel(QTestState *qts, uint32_t off, uint32_t val)
> +{
> +    qtest_writel(qts, GENET_BASE + off, val);
> +}
> +
> +static uint32_t genet_readl(QTestState *qts, uint32_t off)
> +{
> +    return qtest_readl(qts, GENET_BASE + off);
> +}
> +
> +/* Transmit across the wrap of the 16-bit consumer index */
> +static void test_tx_index_wrap(void)
> +{
> +    const unsigned r = 0, n = 3;
> +    const uint16_t first = 0xfffe;
> +    uint8_t frame[FRAME_LEN] = { 0 };
> +    uint8_t pkt[4 + FRAME_LEN];
> +    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]);
> +
> +    memset(frame, 0xff, 6);
> +    stw_be_p(frame + 12, 0x88b5);
> +    for (unsigned i = 0; i < n; i++) {
> +        frame[14] = i;
> +        qtest_memset(qts, BUF_BASE + i * BUF_SIZE, 0, TSB_LEN);
> +        qtest_memwrite(qts, BUF_BASE + i * BUF_SIZE + TSB_LEN, frame,
> +                       sizeof(frame));
> +        /* SOP, EOP, buffer length */
> +        genet_writel(qts, TDMA_DESC(i), BIT(13) | BIT(14) |
> +                     (TSB_LEN + FRAME_LEN) << 16);
> +        genet_writel(qts, TDMA_DESC(i) + 4, BUF_BASE + i * BUF_SIZE);
> +        genet_writel(qts, TDMA_DESC(i) + 8, 0);
> +    }
> +    genet_writel(qts, TDMA_RING(r) + RING_START_ADDR, 0);
> +    genet_writel(qts, TDMA_RING(r) + RING_END_ADDR, DESCS_PER_RING * 3 -
> 1);
> +    genet_writel(qts, TDMA_RING(r) + TRING_READ_PTR, 0);
> +    genet_writel(qts, TDMA_RING(r) + TRING_CONS_INDEX, first);
> +    genet_writel(qts, TDMA_RING(r) + TRING_PROD_INDEX, first);
> +    genet_writel(qts, TDMA_RING_CFG, BIT(r));
> +    genet_writel(qts, TDMA_CTRL, 1 | BIT(1 + r));
> +
> +    genet_writel(qts, TDMA_RING(r) + TRING_PROD_INDEX,
> +                 (uint16_t)(first + n));
> +    g_assert_cmphex(genet_readl(qts, TDMA_RING(r) + TRING_CONS_INDEX) &
> +                    0xffff, ==, (uint16_t)(first + n));
> +
> +    for (unsigned i = 0; i < n; i++) {
> +        g_assert_cmpint(recv(sv[0], pkt, sizeof(pkt), MSG_WAITALL), ==,
> +                        sizeof(pkt));
> +        g_assert_cmpuint(ldl_be_p(pkt), ==, FRAME_LEN);
> +        g_assert_cmpuint(pkt[4 + 14], ==, i);
> +    }
> +
> +    qtest_quit(qts);
> +    close(sv[0]);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +    qtest_add_func("/bcm2838-genet/tx-index-wrap", test_tx_index_wrap);
> +    return g_test_run();
> +}
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 46d4fc1d2b..36af0b267a 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -284,6 +284,8 @@ qtests_aarch64 = \
>    (config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ?
> ['xlnx-can-test', 'fuzz-xlnx-dp-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_XLNX_VERSAL') ?
> ['xlnx-canfd-test', 'xlnx-versal-trng-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test',
> 'bcm2835-i2c-test', 'bcm2835-ic-test'] : []) +  \
> +  (config_all_devices.has_key('CONFIG_RASPI') and host_os != 'windows' ?
> +    ['bcm2838-genet-test'] : []) + \
>    (config_all_accel.has_key('CONFIG_TCG') and
>                 \
>     config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ?
> ['tpm-tis-i2c-test', 'tpm-tis-i2c-swtpm-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 :
> []) + \
> --
> 2.56.0
>
>

Reply via email to