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

[Nicholas Piggin]

On Sun Jan 19, 2025 at 7:22 PM AEST, Yan Vugenfirer wrote:
> On Fri, Jan 17, 2025 at 7:05 PM Nicholas Piggin <npiggin@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 <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);
>>  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.

This is just for qtest where assertions are used to indicate
failure.

Thanks,
Nick