[6/9] net/e1000e|igb: Fix interrupt throttling logic

Commit Message

Nicholas Piggin Jan. 17, 2025, 5:03 p.m. UTC

Interrupt throttling is broken in several ways:
- Timer expiry sends an interrupt even if there is no cause.
- Timer expiry that results in an interrupt does not re-arm
  the timer so an interrupt can appear immediately after the
  timer expiry interrupt.
- Interrupt auto-clear should not clear cause if an interrupt
  is delayed by throttling.

Fix these by skipping the auto-clear logic if an interrupt is
delayed, and when the throttle timer expires check the cause
bits corresponding to the msix vector before sending an irq,
and send it using the functions that run the throttling state
machine and perform auto-clear.

Signed-off-by: Nicholas Piggin <[e-mail]>
---
 hw/net/e1000e_core.c | 60 +++++++++++++++++++++++++++++++++++++++-----
 hw/net/igb_core.c    | 28 +++++++++++++--------
 2 files changed, 72 insertions(+), 16 deletions(-)

Patch

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index e32955d244b..c5be20bcbbe 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -168,16 +168,63 @@  e1000e_intrmgr_on_throttling_timer(void *opaque)
     }
 }
 
+/* Find causes from IVAR vectors and only interrupt if causes are set */
+static uint32_t find_msix_causes(E1000ECore *core, int vec)
+{
+    uint32_t causes = 0;
+    uint32_t int_cfg;
+
+    int_cfg = E1000_IVAR_RXQ0(core->mac[IVAR]);
+    if (E1000_IVAR_ENTRY_VALID(int_cfg) &&
+        E1000_IVAR_ENTRY_VEC(int_cfg) == vec) {
+        causes |= E1000_ICR_RXQ0;
+    }
+
+    int_cfg = E1000_IVAR_RXQ1(core->mac[IVAR]);
+    if (E1000_IVAR_ENTRY_VALID(int_cfg) &&
+        E1000_IVAR_ENTRY_VEC(int_cfg) == vec) {
+        causes |= E1000_ICR_RXQ1;
+    }
+
+    int_cfg = E1000_IVAR_TXQ0(core->mac[IVAR]);
+    if (E1000_IVAR_ENTRY_VALID(int_cfg) &&
+        E1000_IVAR_ENTRY_VEC(int_cfg) == vec) {
+        causes |= E1000_ICR_TXQ0;
+    }
+
+    int_cfg = E1000_IVAR_TXQ1(core->mac[IVAR]);
+    if (E1000_IVAR_ENTRY_VALID(int_cfg) &&
+        E1000_IVAR_ENTRY_VEC(int_cfg) == vec) {
+        causes |= E1000_ICR_TXQ1;
+    }
+
+    int_cfg = E1000_IVAR_OTHER(core->mac[IVAR]);
+    if (E1000_IVAR_ENTRY_VALID(int_cfg) &&
+        E1000_IVAR_ENTRY_VEC(int_cfg) == vec) {
+        causes |= E1000_ICR_OTHER;
+    }
+
+    return causes;
+}
+
+static void
+e1000e_msix_notify(E1000ECore *core, uint32_t causes);
+
 static void
 e1000e_intrmgr_on_msix_throttling_timer(void *opaque)
 {
     E1000IntrDelayTimer *timer = opaque;
-    int idx = timer - &timer->core->eitr[0];
+    E1000ECore *core = timer->core;
+    int idx = timer - &core->eitr[0];
+    uint32_t causes;
 
     timer->running = false;
 
-    trace_e1000e_irq_msix_notify_postponed_vec(idx);
-    msix_notify(timer->core->owner, idx);
+    causes = find_msix_causes(core, idx);
+    if (core->mac[IMS] & core->mac[ICR] & causes) {
+        trace_e1000e_irq_msix_notify_postponed_vec(idx);
+        e1000e_msix_notify(core, causes);
+    }
 }
 
 static void
@@ -1982,10 +2029,11 @@  e1000e_msix_notify_one(E1000ECore *core, uint32_t cause, uint32_t int_cfg)
     if (E1000_IVAR_ENTRY_VALID(int_cfg)) {
         uint32_t vec = E1000_IVAR_ENTRY_VEC(int_cfg);
         if (vec < E1000E_MSIX_VEC_NUM) {
-            if (!e1000e_eitr_should_postpone(core, vec)) {
-                trace_e1000e_irq_msix_notify_vec(vec);
-                msix_notify(core->owner, vec);
+            if (e1000e_eitr_should_postpone(core, vec)) {
+                return;
             }
+            trace_e1000e_irq_msix_notify_vec(vec);
+            msix_notify(core->owner, vec);
         } else {
             trace_e1000e_wrn_msix_vec_wrong(cause, int_cfg);
         }
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index cdebc917d2e..dad32be54fd 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -168,16 +168,7 @@  igb_intmgr_timer_resume(IGBIntrDelayTimer *timer)
 }
 
 static void
-igb_intrmgr_on_msix_throttling_timer(void *opaque)
-{
-    IGBIntrDelayTimer *timer = opaque;
-    int idx = timer - &timer->core->eitr[0];
-
-    timer->running = false;
-
-    trace_e1000e_irq_msix_notify_postponed_vec(idx);
-    igb_msix_notify(timer->core, idx);
-}
+igb_intrmgr_on_msix_throttling_timer(void *opaque);
 
 static void
 igb_intrmgr_initialize_all_timers(IGBCore *core, bool create)
@@ -2279,6 +2270,23 @@  static void igb_send_msix(IGBCore *core, uint32_t causes)
     }
 }
 
+static void
+igb_intrmgr_on_msix_throttling_timer(void *opaque)
+{
+    IGBIntrDelayTimer *timer = opaque;
+    IGBCore *core = timer->core;
+    int vector = timer - &core->eitr[0];
+    uint32_t causes;
+
+    timer->running = false;
+
+    causes = core->mac[EICR] & core->mac[EIMS];
+    if ((causes & BIT(vector)) && !igb_eitr_should_postpone(core, vector)) {
+        trace_e1000e_irq_msix_notify_postponed_vec(vector);
+        igb_msix_notify(core, vector);
+    }
+}
+
 static inline void
 igb_fix_icr_asserted(IGBCore *core)
 {