Re: [PATCH v3 10/12] net/e1000e|igb: Only send delayed msix interrupts that have a cause

[Akihiko Odaki]

On 2025/05/02 12:17, Nicholas Piggin wrote:

The msix interrupt throttling timer expiry sends an interrupt even if
there is no unmasked interrupt causes. This can be observed by seeing
two interrupts in response to a single event when throttling is active.

The e1000e non-msix paths seem to get this right by masking and testing
ICR and IMS. Add similar checks for the msix cases in e1000e and igb.

Signed-off-by: Nicholas Piggin <npig...@gmail.com>
---
  hw/net/e1000e_core.c | 10 ++++++----
  hw/net/igb_core.c    | 11 ++++++++---
  2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 022884a2ea0..d53f70065ef 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -230,10 +230,12 @@ e1000e_intrmgr_on_msix_throttling_timer(void *opaque)

timer->running = false; - causes = find_msix_causes(core, idx);

-    trace_e1000e_irq_msix_notify_postponed_vec(idx);
-    msix_notify(core->owner, idx);
-    e1000e_msix_auto_clear_mask(core, causes);
+    causes = find_msix_causes(core, idx) & core->mac[IMS] & core->mac[ICR];
+    if (causes) {
+        trace_e1000e_irq_msix_notify_postponed_vec(idx);
+        msix_notify(core->owner, causes);

I think this should be: msix_notify(core->owner, idx);

+        e1000e_msix_auto_clear_mask(core, causes);
+    }
  }

static void

diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 3ae3e53530b..035637f81f8 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -171,12 +171,17 @@ static void
  igb_intrmgr_on_msix_throttling_timer(void *opaque)
  {
      IGBIntrDelayTimer *timer = opaque;
-    int idx = timer - &timer->core->eitr[0];
+    IGBCore *core = timer->core;
+    int vector = timer - &core->eitr[0];
+    uint32_t causes;

timer->running = false; - trace_e1000e_irq_msix_notify_postponed_vec(idx);

-    igb_msix_notify(timer->core, idx);
+    causes = core->mac[EICR] & core->mac[EIMS];
+    if (causes & BIT(vector)) {
+        trace_e1000e_irq_msix_notify_postponed_vec(vector);
+        igb_msix_notify(core, vector);
+    }
  }

static void