Date: Thu, 30 Jul 2026 14:06:08 +0000 From: Kevin Bowling <kbowling@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: a77257d60058 - main - igbv: Sanitize retained VF queue state Message-ID: <6a6b5a50.1c9e9.1bdeef7c@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by kbowling: URL: https://cgit.FreeBSD.org/src/commit/?id=a77257d6005839f2dd540e98f1e9ecd70879a38d commit a77257d6005839f2dd540e98f1e9ecd70879a38d Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2026-07-30 08:10:31 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2026-07-30 14:06:00 +0000 igbv: Sanitize retained VF queue state 82576 and I350 VFLR leave queue configuration unchanged. A previous VF owner can therefore leave a transmit head-writeback DMA destination and other queue policy for the next guest. After each reset attempt, disable all exposed VF queues and wait for their enable bits to clear before clearing SRRCTL, VFPSRTYPE, RXCTRL, TXCTRL, and TDWBAL/H. Spin briefly and then sleep until the bounded queue-disable deadline. iflib cannot report initialization failure and marks an interface running after its init callback returns. On sanitation failure, keep interrupts disabled and use the deferred admin task to clear RUNNING. Retry after 100 and 500 ms; after three total failures, leave the interface down until another administrative initialization starts a new bounded attempt set. igbv uses queue zero on both families, but 82576 exposes a second VF queue whose retained state must also be cleared. Extend the INVARIANTS register validator for only those queue-one CSRs and only on 82576. This implements the VF side of Software Clarification 3 from the 82576 and I350 specification updates. It also means an igbv guest does not depend on its PF to sanitize a previous VF owner's state. Sponsored by: BBOX.io --- sys/dev/e1000/e1000_osdep.h | 20 +++- sys/dev/e1000/e1000_regs.h | 1 + sys/dev/e1000/if_em.c | 21 ++++- sys/dev/e1000/if_em.h | 12 +++ sys/dev/e1000/if_igbv.c | 216 +++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 265 insertions(+), 5 deletions(-) diff --git a/sys/dev/e1000/e1000_osdep.h b/sys/dev/e1000/e1000_osdep.h index 0f6364f9a7b5..1226c0264333 100644 --- a/sys/dev/e1000/e1000_osdep.h +++ b/sys/dev/e1000/e1000_osdep.h @@ -171,8 +171,10 @@ struct e1000_osdep * the OS accessors so a PF-only register that leaks into the shared VF path * fails at its first access instead of returning reserved-register garbage. * - * The driver intentionally uses only queue pair zero. Expanding VF queue - * support must extend this predicate from the applicable device CSR map. + * The driver intentionally uses only queue pair zero. It also clears the + * retained configuration of the unused second 82576 queue pair after VFLR. + * Expanding VF data-path queue support must extend this predicate from the + * applicable device CSR map. */ static __inline bool e1000_vf_reg_valid(uint32_t reg, bool write, bool vf_82576) @@ -212,6 +214,20 @@ e1000_vf_reg_valid(uint32_t reg, bool write, bool vf_82576) return (true); } + /* Retained configuration from 82576 virtual queue one. */ + if (vf_82576) { + switch (reg) { + case 0x0290c: /* SRRCTL(1) */ + case 0x02914: /* RXCTL(1) */ + case 0x02928: /* RXDCTL(1) */ + case 0x03914: /* TXCTL(1) */ + case 0x03928: /* TXDCTL(1) */ + case 0x03938: /* TDWBAL(1) */ + case 0x0393c: /* TDWBAH(1) */ + return (true); + } + } + /* * 82576 exposes VFMPRC at 0xf3c. I350 erratum 31 makes * its corrected 0xf38 address inaccessible to a VF. diff --git a/sys/dev/e1000/e1000_regs.h b/sys/dev/e1000/e1000_regs.h index 5af1142a1ff0..138155cfc5bc 100644 --- a/sys/dev/e1000/e1000_regs.h +++ b/sys/dev/e1000/e1000_regs.h @@ -286,6 +286,7 @@ #define E1000_TXDMAC 0x03000 /* Tx DMA Control - RW */ #define E1000_KABGTXD 0x03004 /* AFE Band Gap Transmit Ref Data */ #define E1000_PSRTYPE(_i) (0x05480 + ((_i) * 4)) +#define E1000_VFPSRTYPE 0x00F0C #define E1000_RAL(_i) (((_i) <= 15) ? (0x05400 + ((_i) * 8)) : \ (0x054E0 + ((_i - 16) * 8))) #define E1000_RAH(_i) (((_i) <= 15) ? (0x05404 + ((_i) * 8)) : \ diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c index db84004643f2..43519dd72d25 100644 --- a/sys/dev/e1000/if_em.c +++ b/sys/dev/e1000/if_em.c @@ -1668,6 +1668,8 @@ em_if_detach(if_ctx_t ctx) INIT_DEBUGOUT("em_if_detach: begin"); igb_iov_detach(sc); + if (sc->vf_ifp) + igbv_queue_retry_detach(sc); if (!sc->vf_ifp) e1000_phy_hw_reset(&sc->hw); @@ -1700,6 +1702,8 @@ em_if_suspend(if_ctx_t ctx) { struct e1000_softc *sc = iflib_get_softc(ctx); + if (sc->vf_ifp) + igbv_queue_retry_stop(sc); em_release_manageability(sc); em_release_hw_control(sc); em_enable_wakeup(ctx); @@ -1791,8 +1795,10 @@ em_if_init(if_ctx_t ctx) INIT_DEBUGOUT("em_if_init: begin"); vf_mbx_ready = !sc->vf_ifp; - if (sc->vf_ifp) + if (sc->vf_ifp) { + igbv_queue_retry_prepare(sc); sc->vf_reset_pending = true; + } /* Get the latest mac address, User can use a LAA */ bcopy(if_getlladdr(ifp), sc->hw.mac.addr, ETHER_ADDR_LEN); @@ -1824,6 +1830,16 @@ em_if_init(if_ctx_t ctx) } else { em_reset(ctx); } + if (sc->vf_ifp && !sc->vf_queues_sanitized) { + /* + * Do not program or enable rings while retained queue state + * might still contain a previous VF owner's DMA address. A + * bounded callout retries initialization after iflib returns; + * the deferred admin task clears its optimistic RUNNING flag. + */ + igbv_queue_retry_failed(ctx); + return; + } if (sc->vf_ifp && vf_mbx_ready) igbv_reconcile_mac(sc, ifp); /* Re-arm a link-up transition deferred for this reset. */ @@ -2782,6 +2798,9 @@ em_if_stop(if_ctx_t ctx) INIT_DEBUGOUT("em_if_stop: begin"); + if (sc->vf_ifp) + igbv_queue_retry_stop(sc); + /* I219 needs special flushing to avoid hangs */ if (sc->hw.mac.type >= e1000_pch_spt && sc->hw.mac.type < igb_mac_min) em_flush_desc_rings(sc); diff --git a/sys/dev/e1000/if_em.h b/sys/dev/e1000/if_em.h index 617ecaa8e294..6e9d17c7b7a0 100644 --- a/sys/dev/e1000/if_em.h +++ b/sys/dev/e1000/if_em.h @@ -646,7 +646,15 @@ struct e1000_softc { struct e1000_vf_stats vf_stats; /* sc->vf_ifp */ } ustats; + struct callout vf_queue_retry; + struct timeval vf_last_queue_log; + u32 vf_queue_retry_new_epoch; + u32 vf_queue_retry_pending; u16 vf_ifp; + u8 vf_queue_failures; + bool vf_queue_gave_up; + bool vf_queue_retry_initialized; + bool vf_queues_sanitized; bool vf_reset_pending; /* A PF can retain auxiliary filters across a VF reset. */ bool vf_uc_filters_set; @@ -674,6 +682,10 @@ void igbv_if_intr_disable(if_ctx_t); void igbv_if_update_admin_status(if_ctx_t); void igbv_initialize_receive_unit(if_ctx_t); void igbv_initialize_transmit_unit(if_ctx_t); +void igbv_queue_retry_detach(struct e1000_softc *); +void igbv_queue_retry_failed(if_ctx_t); +void igbv_queue_retry_prepare(struct e1000_softc *); +void igbv_queue_retry_stop(struct e1000_softc *); void igbv_reconcile_mac(struct e1000_softc *, if_t); bool igbv_reset(if_ctx_t); void igbv_update_uc_addr_list(struct e1000_softc *, if_t); diff --git a/sys/dev/e1000/if_igbv.c b/sys/dev/e1000/if_igbv.c index 01c586fdb4b0..c8ecd334d862 100644 --- a/sys/dev/e1000/if_igbv.c +++ b/sys/dev/e1000/if_igbv.c @@ -30,10 +30,25 @@ #include <sys/sbuf.h> +#define IGBV_82576_QUEUES 2 +#define IGBV_I350_QUEUES 1 #define IGBV_MAX_MAC_FILTERS 3 +#define IGBV_QUEUE_DISABLE_BUSY_RETRIES 10 +#define IGBV_QUEUE_DISABLE_DELAY_US 10 +#define IGBV_QUEUE_DISABLE_PAUSE (100 * SBT_1US) +#define IGBV_QUEUE_DISABLE_RETRIES 20 +#define IGBV_QUEUE_SANITIZE_ATTEMPTS 3 #define IGBV_VLAN_RETRY_BATCH 4 #define IGBV_VLAN_RETRY_WINDOW (8 * SBT_1S) +static const struct timeval igbv_queue_log_interval = { 2, 0 }; +static const sbintime_t igbv_queue_retry_delay[] = { + 100 * SBT_1MS, + 500 * SBT_1MS, +}; +_Static_assert(nitems(igbv_queue_retry_delay) + 1 == + IGBV_QUEUE_SANITIZE_ATTEMPTS, "missing queue retry delay"); + struct igb_vf_uc_addr_list { struct e1000_softc *sc; u8 addrs[IGBV_MAX_MAC_FILTERS][ETHER_ADDR_LEN]; @@ -43,6 +58,101 @@ static bool igbv_tx_pending(struct e1000_softc *); static bool igbv_vlan_retry_pending(const struct e1000_softc *); static void igbv_vlan_retry_tick(struct e1000_softc *); +static void +igbv_queue_retry_callout(void *arg) +{ + struct e1000_softc *sc; + if_t ifp; + + sc = arg; + if (atomic_readandclear_32(&sc->vf_queue_retry_pending) == 0) + return; + ifp = iflib_get_ifp(sc->ctx); + if ((if_getflags(ifp) & IFF_UP) == 0) { + atomic_set_32(&sc->vf_queue_retry_new_epoch, 1); + return; + } + iflib_request_reset(sc->ctx); + iflib_admin_intr_deferred(sc->ctx); +} + +void +igbv_queue_retry_detach(struct e1000_softc *sc) +{ + + if (!sc->vf_queue_retry_initialized) + return; + atomic_readandclear_32(&sc->vf_queue_retry_pending); + callout_drain(&sc->vf_queue_retry); + sc->vf_queue_retry_initialized = false; +} + +void +igbv_queue_retry_stop(struct e1000_softc *sc) +{ + + if (!sc->vf_queue_retry_initialized) + return; + if (atomic_readandclear_32(&sc->vf_queue_retry_pending) != 0) + atomic_set_32(&sc->vf_queue_retry_new_epoch, 1); + callout_stop(&sc->vf_queue_retry); +} + +void +igbv_queue_retry_prepare(struct e1000_softc *sc) +{ + bool new_epoch; + + new_epoch = + atomic_readandclear_32(&sc->vf_queue_retry_new_epoch) != 0; + if (!sc->vf_queue_gave_up && !new_epoch) + return; + sc->vf_queue_failures = 0; + sc->vf_queue_gave_up = false; +} + +static void +igbv_queue_retry_succeeded(struct e1000_softc *sc) +{ + + atomic_readandclear_32(&sc->vf_queue_retry_pending); + atomic_readandclear_32(&sc->vf_queue_retry_new_epoch); + if (sc->vf_queue_retry_initialized) + callout_stop(&sc->vf_queue_retry); + sc->vf_queue_failures = 0; + sc->vf_queue_gave_up = false; +} + +void +igbv_queue_retry_failed(if_ctx_t ctx) +{ + struct e1000_softc *sc; + sbintime_t delay; + + sc = iflib_get_softc(ctx); + KASSERT(sc->vf_ifp, ("%s called for a PF", __func__)); + + if (sc->vf_queue_failures < IGBV_QUEUE_SANITIZE_ATTEMPTS) + sc->vf_queue_failures++; + if (sc->vf_queue_failures < IGBV_QUEUE_SANITIZE_ATTEMPTS) { + delay = igbv_queue_retry_delay[sc->vf_queue_failures - 1]; + atomic_set_32(&sc->vf_queue_retry_pending, 1); + callout_reset_sbt(&sc->vf_queue_retry, delay, 0, + igbv_queue_retry_callout, sc, C_PREL(1)); + } else if (!sc->vf_queue_gave_up) { + atomic_readandclear_32(&sc->vf_queue_retry_pending); + callout_stop(&sc->vf_queue_retry); + sc->vf_queue_gave_up = true; + device_printf(sc->dev, + "retained VF queues remained active after %u attempts; " + "interface left down; toggle it down/up to retry\n", + sc->vf_queue_failures); + } + + iflib_link_state_change(ctx, LINK_STATE_DOWN, 0); + iflib_admin_intr_deferred(ctx); +} + void igbv_vlan_retry_add(struct e1000_softc *sc, u16 vid) { @@ -125,6 +235,7 @@ igbv_vlan_retry_tick(struct e1000_softc *sc) int igbv_if_attach_pre(if_ctx_t ctx) { + struct e1000_softc *sc; device_t dev; int error; @@ -137,7 +248,11 @@ igbv_if_attach_pre(if_ctx_t ctx) if (error != 0) return (error); - KASSERT(((struct e1000_softc *)iflib_get_softc(ctx))->vf_ifp && + sc = iflib_get_softc(ctx); + callout_init(&sc->vf_queue_retry, 1); + sc->vf_queue_retry_initialized = true; + + KASSERT(sc->vf_ifp && (iflib_get_sctx(ctx)->isc_flags & IFLIB_IS_VF) != 0, ("%s: igbv attached without VF policy", __func__)); return (0); @@ -188,6 +303,18 @@ igbv_if_update_admin_status(if_ctx_t ctx) dev = iflib_get_dev(ctx); KASSERT(sc->vf_ifp, ("%s called for a PF", __func__)); + /* + * iflib's init callback cannot report failure and marks the interface + * running after it returns. Complete the failed-init transition from + * this deferred task, after iflib has set its driver flags. + */ + if (!sc->vf_queues_sanitized) { + igbv_if_intr_disable(ctx); + if_setdrvflagbits(iflib_get_ifp(ctx), IFF_DRV_OACTIVE, + IFF_DRV_RUNNING); + return; + } + if (!sc->vf_reset_pending && atomic_readandclear_32(&sc->promisc_pending) != 0) (void)em_if_set_promisc_impl(ctx, @@ -258,11 +385,87 @@ igbv_tx_pending(struct e1000_softc *sc) return (false); } +static bool +igbv_sanitize_queues(struct e1000_softc *sc) +{ + struct e1000_hw *hw; + u32 rxdctl, txdctl; + int i, nqueues, retry; + + hw = &sc->hw; + switch (hw->mac.type) { + case e1000_vfadapt: + nqueues = IGBV_82576_QUEUES; + break; + case e1000_vfadapt_i350: + nqueues = IGBV_I350_QUEUES; + break; + default: + return (true); + } + + /* + * The 82576 and I350 specification updates, Software Clarification 3, + * note that VFLR leaves this queue configuration intact. Clear it + * before programming the new rings so igbv does not depend on its PF + * to sanitize state left by a previous VF owner. igbv uses only queue + * zero, but must also clear the unused second 82576 queue. + * + * Disable every queue first and wait for outstanding DMA activity to + * stop before programming TDWBAL/H. Spin only for the normal fast + * transition, then sleep until the bounded deadline. + */ + for (i = 0; i < nqueues; i++) { + E1000_WRITE_REG(hw, E1000_RXDCTL(i), 0); + E1000_WRITE_REG(hw, E1000_TXDCTL(i), 0); + } + E1000_WRITE_FLUSH(hw); + for (retry = 0; retry < IGBV_QUEUE_DISABLE_RETRIES; retry++) { + for (i = 0; i < nqueues; i++) { + rxdctl = E1000_READ_REG(hw, E1000_RXDCTL(i)); + txdctl = E1000_READ_REG(hw, E1000_TXDCTL(i)); + if ((rxdctl & E1000_RXDCTL_QUEUE_ENABLE) != 0 || + (txdctl & E1000_TXDCTL_QUEUE_ENABLE) != 0) + break; + } + if (i == nqueues) + break; + if (retry + 1 < IGBV_QUEUE_DISABLE_RETRIES) { + if (retry < IGBV_QUEUE_DISABLE_BUSY_RETRIES) + DELAY(IGBV_QUEUE_DISABLE_DELAY_US); + else + pause_sbt("igbvqds", + IGBV_QUEUE_DISABLE_PAUSE, 0, + C_PREL(1)); + } + } + if (retry == IGBV_QUEUE_DISABLE_RETRIES) { + if (ratecheck(&sc->vf_last_queue_log, + &igbv_queue_log_interval)) + device_printf(sc->dev, + "could not disable retained VF queues; " + "reset deferred\n"); + return (false); + } + + for (i = 0; i < nqueues; i++) { + E1000_WRITE_REG(hw, E1000_SRRCTL(i), 0); + E1000_WRITE_REG(hw, E1000_DCA_RXCTRL(i), 0); + E1000_WRITE_REG(hw, E1000_TDWBAL(i), 0); + E1000_WRITE_REG(hw, E1000_TDWBAH(i), 0); + E1000_WRITE_REG(hw, E1000_DCA_TXCTRL(i), 0); + } + E1000_WRITE_REG(hw, E1000_VFPSRTYPE, 0); + E1000_WRITE_FLUSH(hw); + return (true); +} + bool igbv_reset(if_ctx_t ctx) { struct e1000_softc *sc; struct e1000_hw *hw; + s32 error; sc = iflib_get_softc(ctx); hw = &sc->hw; @@ -278,7 +481,14 @@ igbv_reset(if_ctx_t ctx) .requested_mode = e1000_fc_none, }; - if (e1000_reset_hw(hw) != E1000_SUCCESS) { + error = e1000_reset_hw(hw); + sc->vf_queues_sanitized = igbv_sanitize_queues(sc); + if (!sc->vf_queues_sanitized) { + e1000_check_for_link(hw); + return (false); + } + igbv_queue_retry_succeeded(sc); + if (error != E1000_SUCCESS) { e1000_check_for_link(hw); return (false); } @@ -322,6 +532,8 @@ igbv_if_intr_enable(if_ctx_t ctx) sc = iflib_get_softc(ctx); hw = &sc->hw; KASSERT(sc->vf_ifp, ("%s called for a PF", __func__)); + if (!sc->vf_queues_sanitized) + return; mask = sc->que_mask | sc->link_mask; E1000_WRITE_REG(hw, E1000_EIAC, mask);home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a6b5a50.1c9e9.1bdeef7c>