On 2025/05/02 12:16, Nicholas Piggin wrote:
> The spec explicitly permits xITR register interval field to have a value
> of zero to disable throttling. The e1000e model already allows for this
> in the throttling logic, so remove the minimum value for the register.
>
> The spec appears to say there is a maximum observable interrupt rate
> when throttling is enabled, regardless of ITR value, so throttle timer
> calculation is clamped to that minimum value.
>
> EITR registers default to 0, as specified in spec 7.4.4.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> hw/net/e1000e_core.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index 24138587905..96f74f1ea14 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -51,8 +51,17 @@
>
> #include "trace.h"
>
> -/* No more then 7813 interrupts per second according to spec 10.2.4.2 */
> -#define E1000E_MIN_XITR (500)
> +/*
> + * A suggested range for ITR is 651-5580, according to spec 10.2.4.2, but
> + * QEMU has traditionally set 500 here.
> + */
> +#define E1000E_DEFAULT_ITR (500)
The cover letter says this version changes "initial ITR as well as EITR"
but the ITR value is unchanged here. Forgot to commit the change?
> +
> +/*
> + * spec 7.4.4 ITR rules says the maximum observable interrupt rate from the
> + * adapter should not exceed 7813/s (corresponding to 500).
> + */
> +#define E1000E_EFFECTIVE_MIN_XITR (500)
>
> #define E1000E_MAX_TX_FRAGS (64)
>
> @@ -105,8 +114,9 @@ e1000e_lower_legacy_irq(E1000ECore *core)
> static inline void
> e1000e_intrmgr_rearm_timer(E1000IntrDelayTimer *timer)
> {
> - int64_t delay_ns = (int64_t) timer->core->mac[timer->delay_reg] *
> - timer->delay_resolution_ns;
> + uint32_t delay = MAX(timer->core->mac[timer->delay_reg],
> + E1000E_EFFECTIVE_MIN_XITR);
> + int64_t delay_ns = (int64_t)delay * timer->delay_resolution_ns;
>
> trace_e1000e_irq_rearm_timer(timer->delay_reg << 2, delay_ns);
>
> @@ -2783,7 +2793,7 @@ e1000e_set_itr(E1000ECore *core, int index, uint32_t val)
> trace_e1000e_irq_itr_set(val);
>
> core->itr_guest_value = interval;
> - core->mac[index] = MAX(interval, E1000E_MIN_XITR);
> + core->mac[index] = interval;
> }
>
> static void
> @@ -2795,7 +2805,7 @@ e1000e_set_eitr(E1000ECore *core, int index, uint32_t val)
> trace_e1000e_irq_eitr_set(eitr_num, val);
>
> core->eitr_guest_value[eitr_num] = interval;
> - core->mac[index] = MAX(interval, E1000E_MIN_XITR);
> + core->mac[index] = interval;
> }
>
> static void
> @@ -3444,8 +3454,7 @@ static const uint32_t e1000e_mac_reg_init[] = {
> [FACTPS] = E1000_FACTPS_LAN0_ON | 0x20000000,
> [SWSM] = 1,
> [RXCSUM] = E1000_RXCSUM_IPOFLD | E1000_RXCSUM_TUOFLD,
> - [ITR] = E1000E_MIN_XITR,
> - [EITR...EITR + E1000E_MSIX_VEC_NUM - 1] = E1000E_MIN_XITR,
> + [ITR] = E1000E_DEFAULT_ITR,
> };
>
> static void e1000e_reset(E1000ECore *core, bool sw)