On Fri, Jan 17, 2025 at 7:05 PM Nicholas Piggin <npig...@gmail.com> wrote:
> Assert there is no existing irq raised that would lead to a false
> positive interrupt test.
>
> e1000e has to disable interrupt throttling for this test, because
> it can cause delayed superfluous interrupts which trip the assertions.
>
> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> ---
> tests/qtest/libqos/e1000e.h | 1 +
> tests/qtest/e1000e-test.c | 10 ++++++++++
> tests/qtest/igb-test.c | 6 ++++++
> tests/qtest/libqos/e1000e.c | 9 ++++++++-
> 4 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/libqos/e1000e.h b/tests/qtest/libqos/e1000e.h
> index 30643c80949..7154aec0339 100644
> --- a/tests/qtest/libqos/e1000e.h
> +++ b/tests/qtest/libqos/e1000e.h
> @@ -54,6 +54,7 @@ static inline uint32_t e1000e_macreg_read(QE1000E *d,
> uint32_t reg)
> return qpci_io_readl(&d_pci->pci_dev, d_pci->mac_regs, reg);
> }
>
> +bool e1000e_seen_isr(QE1000E *d, uint16_t msg_id);
> void e1000e_wait_isr(QE1000E *d, uint16_t msg_id);
> void e1000e_tx_ring_push(QE1000E *d, void *descr);
> void e1000e_rx_ring_push(QE1000E *d, void *descr);
> diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c
> index 746d26cfb67..9ab81ecff5b 100644
> --- a/tests/qtest/e1000e-test.c
> +++ b/tests/qtest/e1000e-test.c
> @@ -61,6 +61,9 @@ static void e1000e_send_verify(QE1000E *d, int
> *test_sockets, QGuestAllocator *a
> E1000_TXD_DTYP_D |
> sizeof(buffer));
>
> + /* Ensure the interrupt has not been taken already */
> + g_assert(!e1000e_seen_isr(d, E1000E_TX0_MSG_ID));
> +
> /* Put descriptor to the ring */
> e1000e_tx_ring_push(d, &descr);
>
> @@ -105,6 +108,9 @@ static void e1000e_receive_verify(QE1000E *d, int
> *test_sockets, QGuestAllocator
> char buffer[64];
> int ret;
>
> + /* Ensure the interrupt has not been taken already */
> + g_assert(!e1000e_seen_isr(d, E1000E_RX0_MSG_ID));
> +
>
I don't think potentially crashing the guest is the right solution.
I suggest maybe substituting with logging.
> /* Send a dummy packet to device's socket*/
> ret = iov_send(test_sockets[0], iov, 2, 0, sizeof(len) +
> sizeof(packet));
> g_assert_cmpint(ret, == , sizeof(packet) + sizeof(len));
> @@ -188,6 +194,10 @@ static void test_e1000e_multiple_transfers(void *obj,
> void *data,
> return;
> }
>
> + /* Clear EITR because buggy QEMU throttle timer causes superfluous
> irqs */
> + e1000e_macreg_write(d, E1000_EITR + E1000E_RX0_MSG_ID * 4, 0);
> + e1000e_macreg_write(d, E1000_EITR + E1000E_TX0_MSG_ID * 4, 0);
> +
> for (i = 0; i < iterations; i++) {
> e1000e_send_verify(d, data, alloc);
> e1000e_receive_verify(d, data, alloc);
> diff --git a/tests/qtest/igb-test.c b/tests/qtest/igb-test.c
> index cf8b4131cf2..1bbb4bea4c1 100644
> --- a/tests/qtest/igb-test.c
> +++ b/tests/qtest/igb-test.c
> @@ -64,6 +64,9 @@ static void igb_send_verify(QE1000E *d, int
> *test_sockets, QGuestAllocator *allo
> E1000_TXD_DTYP_D |
> sizeof(buffer));
>
> + /* Ensure the interrupt has not been taken already */
> + g_assert(!e1000e_seen_isr(d, E1000E_TX0_MSG_ID));
> +
> /* Put descriptor to the ring */
> e1000e_tx_ring_push(d, &descr);
>
> @@ -119,6 +122,9 @@ static void igb_receive_verify(QE1000E *d, int
> *test_sockets, QGuestAllocator *a
> memset(&descr, 0, sizeof(descr));
> descr.read.pkt_addr = cpu_to_le64(data);
>
> + /* Ensure the interrupt has not been taken already */
> + g_assert(!e1000e_seen_isr(d, E1000E_RX0_MSG_ID));
> +
> /* Put descriptor to the ring */
> e1000e_rx_ring_push(d, &descr);
>
> diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c
> index 925654c7fd4..4e4c387b0bf 100644
> --- a/tests/qtest/libqos/e1000e.c
> +++ b/tests/qtest/libqos/e1000e.c
> @@ -77,13 +77,20 @@ static void e1000e_foreach_callback(QPCIDevice *dev,
> int devfn, void *data)
> g_free(dev);
> }
>
> +bool e1000e_seen_isr(QE1000E *d, uint16_t msg_id)
> +{
> + QE1000E_PCI *d_pci = container_of(d, QE1000E_PCI, e1000e);
> +
> + return qpci_msix_pending(&d_pci->pci_dev, msg_id);
> +}
> +
> void e1000e_wait_isr(QE1000E *d, uint16_t msg_id)
> {
> QE1000E_PCI *d_pci = container_of(d, QE1000E_PCI, e1000e);
> guint64 end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
>
> do {
> - if (qpci_msix_pending(&d_pci->pci_dev, msg_id)) {
> + if (e1000e_seen_isr(d, msg_id)) {
> return;
> }
> qtest_clock_step(d_pci->pci_dev.bus->qts, 10000);
> --
> 2.45.2
>
>
>
