From: Nicholas Piggin <npiggin@gmail.com>
To: Akihiko Odaki <akihiko.odaki@daynix.com>
Cc: Nicholas Piggin <npiggin@gmail.com>,
qemu-devel@nongnu.org,
Dmitry Fleytman <dmitry.fleytman@gmail.com>,
Jason Wang <jasowang@redhat.com>,
Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>,
Fabiano Rosas <farosas@suse.de>,
Laurent Vivier <lvivier@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH v2 7/8] net/e1000e|igb: Fix interrupt throttling logic
Date: Fri, 11 Apr 2025 14:31:27 +1000 [thread overview]
Message-ID: <20250411043128.201289-8-npiggin@gmail.com> (raw)
In-Reply-To: <20250411043128.201289-1-npiggin@gmail.com>
Interrupt throttling is broken in several ways:
- Timer expiry sends an interrupt even if there is no cause.
- (e1000e) Mitigated interrupts still auto-clear cause bits.
- Timer expiry that results in an interrupt does not re-arm the timer so
an interrupt can appear immediately after the timer expiry interrupt.
To fix:
- When the throttle timer expires, check the cause bits corresponding to
the msix vector before sending an irq.
- (e1000e) Skip the auto-clear logic if an interrupt is delayed, and
send delayed irqs using e1000e_msix_notify() to perform auto-clear.
- Re-load the throttle timer when a delayed interrupt is signaled. e1000e
gets this by signaling them with e1000e_msix_notify(), igb calls
igb_intrmgr_rearm_timer() directly.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/net/e1000e_core.c | 59 +++++++++++++++++++++++++++++++++++++++-----
hw/net/igb_core.c | 50 ++++++++++++++++++++++++-------------
2 files changed, 86 insertions(+), 23 deletions(-)
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index f8e6522f810..6fb8da32e4d 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -178,16 +178,62 @@ e1000e_intrmgr_on_throttling_timer(void *opaque)
}
}
+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) & core->mac[IMS] & core->mac[ICR];
+ if (causes) {
+ trace_e1000e_irq_msix_notify_postponed_vec(idx);
+ e1000e_msix_notify(core, causes);
+ }
}
static void
@@ -1992,10 +2038,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 3ae3e53530b..cc25a1d5baa 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -152,11 +152,14 @@ igb_intrmgr_arm_timer(IGBIntrDelayTimer *timer, int64_t delay_ns)
static inline void
igb_intrmgr_rearm_timer(IGBIntrDelayTimer *timer)
{
- uint32_t interval = (timer->core->mac[timer->delay_reg] &
- E1000_EITR_INTERVAL) >> 2;
- int64_t delay_ns = (int64_t)interval * timer->delay_resolution_ns;
+ uint32_t eitr = timer->core->mac[timer->delay_reg];
- igb_intrmgr_arm_timer(timer, delay_ns);
+ if (eitr != 0) {
+ uint32_t interval = (eitr & E1000_EITR_INTERVAL) >> 2;
+ int64_t delay_ns = (int64_t)interval * timer->delay_resolution_ns;
+
+ igb_intrmgr_arm_timer(timer, delay_ns);
+ }
}
static void
@@ -168,16 +171,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)
@@ -2253,9 +2247,7 @@ igb_postpone_interrupt(IGBIntrDelayTimer *timer)
return true;
}
- if (timer->core->mac[timer->delay_reg] != 0) {
- igb_intrmgr_rearm_timer(timer);
- }
+ igb_intrmgr_rearm_timer(timer);
return false;
}
@@ -2279,6 +2271,30 @@ 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)) {
+ /*
+ * The moderation counter is loaded with interval value whenever the
+ * interrupt is signaled. This includes when the interrupt is signaled
+ * by the counter reaching 0.
+ */
+ igb_intrmgr_rearm_timer(timer);
+
+ trace_e1000e_irq_msix_notify_postponed_vec(vector);
+ igb_msix_notify(core, vector);
+ }
+}
+
static inline void
igb_fix_icr_asserted(IGBCore *core)
{
--
2.47.1
next prev parent reply other threads:[~2025-04-11 4:33 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-11 4:31 [PATCH v2 0/8] hw/e1000e|igb: interrupts and qtests fixes Nicholas Piggin
2025-04-11 4:31 ` [PATCH v2 1/8] qtest/e1000e|igb: Clear interrupt-cause and msix pending bits after irq Nicholas Piggin
2025-04-11 4:31 ` [PATCH v2 2/8] net/e1000e: Permit disabling interrupt throttling Nicholas Piggin
2025-04-19 7:15 ` Akihiko Odaki
2025-04-11 4:31 ` [PATCH v2 3/8] hw/net/e1000e|igb: Remove xitr_guest_value logic Nicholas Piggin
2025-04-19 7:12 ` Akihiko Odaki
2025-04-11 4:31 ` [PATCH v2 4/8] qtest/e1000e|igb: assert irqs are clear before triggering an irq Nicholas Piggin
2025-04-11 4:31 ` [PATCH v2 5/8] net/igb: Fix interrupt throttling interval calculation Nicholas Piggin
2025-04-19 7:22 ` Akihiko Odaki
2025-04-11 4:31 ` [PATCH v2 6/8] net/igb: Implement EITR Moderation Counter Nicholas Piggin
2025-04-19 7:40 ` Akihiko Odaki
2025-04-11 4:31 ` Nicholas Piggin [this message]
2025-04-19 7:55 ` [PATCH v2 7/8] net/e1000e|igb: Fix interrupt throttling logic Akihiko Odaki
2025-04-11 4:31 ` [PATCH v2 8/8] qtest/e1000e|igb: Test interrupt throttling in multiple_transfers test Nicholas Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250411043128.201289-8-npiggin@gmail.com \
--to=npiggin@gmail.com \
--cc=akihiko.odaki@daynix.com \
--cc=dmitry.fleytman@gmail.com \
--cc=farosas@suse.de \
--cc=jasowang@redhat.com \
--cc=lvivier@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=sriram.yagnaraman@ericsson.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.