Re: [PATCH 3/9] qtest/e1000e|igb: assert irqs are clear before triggering an irq

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2025/01/18 2:02, Nicholas Piggin 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 <npiggin@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);

I would rather name it e1000e_pending_isr.
We may have seen an interrupt in the past but it may no longer be pending.

>   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));
> +
>       /* 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);