| Message ID | 20241218074232.1784427-5-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | qtest: pci and e1000e/igb msix fixes | expand |
On 2024/12/18 16:42, Nicholas Piggin wrote: > The e1000e and igb tests do not clear the ICR/EICR cause bits (or > set auto-clear) on seeing queue interrupts, which inhibits the > triggering of a new interrupt. > > Fix this by clearing the cause bits, and verify that the expected > cause bit was set. > > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com> > Cc: Akihiko Odaki <akihiko.odaki@daynix.com> > Cc: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > tests/qtest/e1000e-test.c | 8 ++++++-- > tests/qtest/igb-test.c | 8 ++++++-- > 2 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c > index de9738fdb74..a69759da70e 100644 > --- a/tests/qtest/e1000e-test.c > +++ b/tests/qtest/e1000e-test.c > @@ -64,8 +64,10 @@ static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a > /* Put descriptor to the ring */ > e1000e_tx_ring_push(d, &descr); > > - /* Wait for TX WB interrupt */ > + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ It doesn't clear the MSI-X PBA unless the next patch is applied. This comment change should be moved to that patch. > e1000e_wait_isr(d, E1000E_TX0_MSG_ID); > + /* Read ICR which clears it ready for next interrupt, assert TXQ0 cause */ I suggest the following to make this comment clearer: Read ICR to make it ready for next interrupt, assert TXQ0 cause > + g_assert(e1000e_macreg_read(d, E1000_ICR) & E1000_ICR_TXQ0); > > /* Check DD bit */ > g_assert_cmphex(le32_to_cpu(descr.upper.data) & E1000_TXD_STAT_DD, ==, > @@ -115,8 +117,10 @@ static void e1000e_receive_verify(QE1000E *d, int *test_sockets, QGuestAllocator > /* Put descriptor to the ring */ > e1000e_rx_ring_push(d, &descr); > > - /* Wait for TX WB interrupt */ > + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ > e1000e_wait_isr(d, E1000E_RX0_MSG_ID); > + /* Read ICR which clears it ready for next interrupt, assert RXQ0 cause */ > + g_assert(e1000e_macreg_read(d, E1000_ICR) & E1000_ICR_RXQ0); > > /* Check DD bit */ > g_assert_cmphex(le32_to_cpu(descr.wb.upper.status_error) & > diff --git a/tests/qtest/igb-test.c b/tests/qtest/igb-test.c > index 3d397ea6973..2f22c4fb208 100644 > --- a/tests/qtest/igb-test.c > +++ b/tests/qtest/igb-test.c > @@ -67,8 +67,10 @@ static void igb_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *allo > /* Put descriptor to the ring */ > e1000e_tx_ring_push(d, &descr); > > - /* Wait for TX WB interrupt */ > + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ > e1000e_wait_isr(d, E1000E_TX0_MSG_ID); > + /* Read EICR which clears it ready for next interrupt, assert TXQ0 cause */ > + g_assert(e1000e_macreg_read(d, E1000_EICR) & (1 << E1000E_TX0_MSG_ID)); > > /* Check DD bit */ > g_assert_cmphex(le32_to_cpu(descr.wb.status) & E1000_TXD_STAT_DD, ==, > @@ -118,8 +120,10 @@ static void igb_receive_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a > /* Put descriptor to the ring */ > e1000e_rx_ring_push(d, &descr); > > - /* Wait for TX WB interrupt */ > + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ > e1000e_wait_isr(d, E1000E_RX0_MSG_ID); > + /* Read EICR which clears it ready for next interrupt, assert RXQ0 cause */ > + g_assert(e1000e_macreg_read(d, E1000_EICR) & (1 << E1000E_RX0_MSG_ID)); > > /* Check DD bit */ > g_assert_cmphex(le32_to_cpu(descr.wb.upper.status_error) &
On Thu Dec 19, 2024 at 7:00 PM AEST, Akihiko Odaki wrote: > On 2024/12/18 16:42, Nicholas Piggin wrote: > > The e1000e and igb tests do not clear the ICR/EICR cause bits (or > > set auto-clear) on seeing queue interrupts, which inhibits the > > triggering of a new interrupt. > > > > Fix this by clearing the cause bits, and verify that the expected > > cause bit was set. > > > > Cc: Michael S. Tsirkin <mst@redhat.com> > > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com> > > Cc: Akihiko Odaki <akihiko.odaki@daynix.com> > > Cc: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > tests/qtest/e1000e-test.c | 8 ++++++-- > > tests/qtest/igb-test.c | 8 ++++++-- > > 2 files changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c > > index de9738fdb74..a69759da70e 100644 > > --- a/tests/qtest/e1000e-test.c > > +++ b/tests/qtest/e1000e-test.c > > @@ -64,8 +64,10 @@ static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a > > /* Put descriptor to the ring */ > > e1000e_tx_ring_push(d, &descr); > > > > - /* Wait for TX WB interrupt */ > > + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ > > It doesn't clear the MSI-X PBA unless the next patch is applied. This > comment change should be moved to that patch. Good catch. That was leftover from my improper PBA write patch. > > e1000e_wait_isr(d, E1000E_TX0_MSG_ID); > > + /* Read ICR which clears it ready for next interrupt, assert TXQ0 cause */ > > I suggest the following to make this comment clearer: > Read ICR to make it ready for next interrupt, assert TXQ0 cause Sure. Thanks, Nick
diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c index de9738fdb74..a69759da70e 100644 --- a/tests/qtest/e1000e-test.c +++ b/tests/qtest/e1000e-test.c @@ -64,8 +64,10 @@ static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a /* Put descriptor to the ring */ e1000e_tx_ring_push(d, &descr); - /* Wait for TX WB interrupt */ + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ e1000e_wait_isr(d, E1000E_TX0_MSG_ID); + /* Read ICR which clears it ready for next interrupt, assert TXQ0 cause */ + g_assert(e1000e_macreg_read(d, E1000_ICR) & E1000_ICR_TXQ0); /* Check DD bit */ g_assert_cmphex(le32_to_cpu(descr.upper.data) & E1000_TXD_STAT_DD, ==, @@ -115,8 +117,10 @@ static void e1000e_receive_verify(QE1000E *d, int *test_sockets, QGuestAllocator /* Put descriptor to the ring */ e1000e_rx_ring_push(d, &descr); - /* Wait for TX WB interrupt */ + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ e1000e_wait_isr(d, E1000E_RX0_MSG_ID); + /* Read ICR which clears it ready for next interrupt, assert RXQ0 cause */ + g_assert(e1000e_macreg_read(d, E1000_ICR) & E1000_ICR_RXQ0); /* Check DD bit */ g_assert_cmphex(le32_to_cpu(descr.wb.upper.status_error) & diff --git a/tests/qtest/igb-test.c b/tests/qtest/igb-test.c index 3d397ea6973..2f22c4fb208 100644 --- a/tests/qtest/igb-test.c +++ b/tests/qtest/igb-test.c @@ -67,8 +67,10 @@ static void igb_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator *allo /* Put descriptor to the ring */ e1000e_tx_ring_push(d, &descr); - /* Wait for TX WB interrupt */ + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ e1000e_wait_isr(d, E1000E_TX0_MSG_ID); + /* Read EICR which clears it ready for next interrupt, assert TXQ0 cause */ + g_assert(e1000e_macreg_read(d, E1000_EICR) & (1 << E1000E_TX0_MSG_ID)); /* Check DD bit */ g_assert_cmphex(le32_to_cpu(descr.wb.status) & E1000_TXD_STAT_DD, ==, @@ -118,8 +120,10 @@ static void igb_receive_verify(QE1000E *d, int *test_sockets, QGuestAllocator *a /* Put descriptor to the ring */ e1000e_rx_ring_push(d, &descr); - /* Wait for TX WB interrupt */ + /* Wait for TX WB interrupt (this clears the MSIX PBA) */ e1000e_wait_isr(d, E1000E_RX0_MSG_ID); + /* Read EICR which clears it ready for next interrupt, assert RXQ0 cause */ + g_assert(e1000e_macreg_read(d, E1000_EICR) & (1 << E1000E_RX0_MSG_ID)); /* Check DD bit */ g_assert_cmphex(le32_to_cpu(descr.wb.upper.status_error) &
The e1000e and igb tests do not clear the ICR/EICR cause bits (or set auto-clear) on seeing queue interrupts, which inhibits the triggering of a new interrupt. Fix this by clearing the cause bits, and verify that the expected cause bit was set. Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com> Cc: Akihiko Odaki <akihiko.odaki@daynix.com> Cc: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- tests/qtest/e1000e-test.c | 8 ++++++-- tests/qtest/igb-test.c | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)