git: a77257d60058 - main - igbv: Sanitize retained VF queue state
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 30 Jul 2026 14:06:08 UTC
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);