git: e7aa5a5a3f69 - main - e1000: Serialize 82579 CSR writes with the Management Engine

The branch main has been updated by kbowling:

URL: https://cgit.FreeBSD.org/src/commit/?id=e7aa5a5a3f690f49488f89e01444ba1bcebc427b

commit e7aa5a5a3f690f49488f89e01444ba1bcebc427b
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-16 07:10:09 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-08-24 09:51:38 +0000

    e1000: Serialize 82579 CSR writes with the Management Engine
    
    The 82579 PCIm2PCI arbiter can acknowledge a host MAC CSR write while
    the Management Engine is accessing another CSR.  The host write can be
    lost; subsequent target accesses may no longer be claimed by the MAC and
    can hang the system.
    
    For 82579 controllers with valid management firmware, wait for the ME
    CSR access indication before every MAC CSR write.  Keep the wait bounded
    and use DELAY because writes occur in interrupt and datapath contexts.
    Verify every transmit and receive tail write.  If a tail does not hold
    the requested value, disable its datapath direction and request a full
    iflib reset.
    
    Keep the ordinary register-write path as a direct MMIO write behind a
    predicted per-device gate.  Contain the wait and tail recovery in the
    82579 slow path rather than adding tail-specific accessors and state to
    the rest of the e1000 family.
    
    Documentation on the PCH NICs is scare so Intel's Linux e1000e fixes
    publicly document the hardware failure and required serialization as
    commits bdc125f73f3c and d601afcae2fe.  This implementation is a bit
    cleaner.
    
    Tested on a Thinkpad T430 (82579LM) with a test kernel to simulate ME
    contention without incident as well as lost tail writes causing a
    succesful recovery.
    
    MFC after:      2 weeks
    Sponsored by:   BBOX.io
---
 sys/dev/e1000/e1000_osdep.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 sys/dev/e1000/e1000_osdep.h | 20 +++++++++------
 sys/dev/e1000/if_em.c       |  9 +++++++
 3 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/sys/dev/e1000/e1000_osdep.c b/sys/dev/e1000/e1000_osdep.c
index 8b598f18cf12..c38e963f2ea3 100644
--- a/sys/dev/e1000/e1000_osdep.c
+++ b/sys/dev/e1000/e1000_osdep.c
@@ -36,6 +36,66 @@
 
 int e1000_use_pause_delay = 0;
 
+/*
+ * Wait while the 82579 Management Engine owns the PCIm2PCI arbiter.  DELAY
+ * is required because CSR writes also occur from interrupt and datapath
+ * contexts where sleeping is forbidden.
+ */
+static void
+e1000_pcim2pci_arbiter_wait(struct e1000_osdep *osdep)
+{
+	int i;
+
+	i = E1000_ICH_FWSM_PCIM2PCI_COUNT;
+	while ((bus_space_read_4(osdep->mem_bus_space_tag,
+	    osdep->mem_bus_space_handle, E1000_FWSM) &
+	    E1000_ICH_FWSM_PCIM2PCI) != 0 && --i != 0)
+		DELAY(50);
+}
+
+/*
+ * Serialize an 82579 MAC CSR write against the Management Engine.  The
+ * FreeBSD driver exposes one queue on this controller, so recognize its two
+ * tail registers here and verify them without imposing tail-specific APIs on
+ * the rest of the e1000 family.
+ */
+void
+e1000_pcim2pci_write(struct e1000_osdep *osdep, uint32_t reg, uint32_t value)
+{
+	uint32_t control, control_reg, enable;
+	const char *direction;
+
+	e1000_pcim2pci_arbiter_wait(osdep);
+	bus_space_write_4(osdep->mem_bus_space_tag,
+	    osdep->mem_bus_space_handle, reg, value);
+
+	if (reg == E1000_TDT(0)) {
+		control_reg = E1000_TCTL;
+		enable = E1000_TCTL_EN;
+		direction = "transmit";
+	} else if (reg == E1000_RDT(0)) {
+		control_reg = E1000_RCTL;
+		enable = E1000_RCTL_EN;
+		direction = "receive";
+	} else {
+		return;
+	}
+	if (bus_space_read_4(osdep->mem_bus_space_tag,
+	    osdep->mem_bus_space_handle, reg) == value)
+		return;
+
+	control = bus_space_read_4(osdep->mem_bus_space_tag,
+	    osdep->mem_bus_space_handle, control_reg);
+	e1000_pcim2pci_arbiter_wait(osdep);
+	bus_space_write_4(osdep->mem_bus_space_tag,
+	    osdep->mem_bus_space_handle, control_reg, control & ~enable);
+	device_printf(osdep->dev,
+	    "Management Engine caused an invalid %s tail write; "
+	    "requesting reset\n", direction);
+	iflib_request_reset(osdep->ctx);
+	iflib_admin_intr_deferred(osdep->ctx);
+}
+
 static void
 e1000_enable_pause_delay(void *use_pause_delay)
 {
diff --git a/sys/dev/e1000/e1000_osdep.h b/sys/dev/e1000/e1000_osdep.h
index 1226c0264333..b3b16b102edb 100644
--- a/sys/dev/e1000/e1000_osdep.h
+++ b/sys/dev/e1000/e1000_osdep.h
@@ -161,10 +161,13 @@ struct e1000_osdep
 	bus_space_handle_t flash_bus_space_handle;
 	device_t	   dev;
 	if_ctx_t	   ctx;
+	bool		   pcim2pci_arbiter_wa;
 	bool		   vf;
 	bool		   vf_82576;
 };
 
+void e1000_pcim2pci_write(struct e1000_osdep *, uint32_t, uint32_t);
+
 #ifdef INVARIANTS
 /*
  * 82576 and I350 VFs expose a sparse register file.  Keep this list local to
@@ -276,11 +279,6 @@ e1000_vf_reg_valid(uint32_t reg, bool write, bool vf_82576)
     bus_space_read_4(((struct e1000_osdep *)(hw)->back)->mem_bus_space_tag, \
     ((struct e1000_osdep *)(hw)->back)->mem_bus_space_handle, offset)
 
-/* Write to an absolute offset in the adapter's memory space */
-#define E1000_WRITE_OFFSET(hw, offset, value) \
-    bus_space_write_4(((struct e1000_osdep *)(hw)->back)->mem_bus_space_tag, \
-    ((struct e1000_osdep *)(hw)->back)->mem_bus_space_handle, offset, value)
-
 static __inline uint32_t
 e1000_rd32(struct e1000_osdep *osdep, uint32_t reg)
 {
@@ -312,10 +310,18 @@ e1000_wr32(struct e1000_osdep *osdep, uint32_t reg, uint32_t value)
 	    ("e1000: invalid VF register write at %#x", reg));
 #endif
 
-	bus_space_write_4(osdep->mem_bus_space_tag,
-	    osdep->mem_bus_space_handle, reg, value);
+	if (__predict_true(!osdep->pcim2pci_arbiter_wa)) {
+		bus_space_write_4(osdep->mem_bus_space_tag,
+		    osdep->mem_bus_space_handle, reg, value);
+		return;
+	}
+	e1000_pcim2pci_write(osdep, reg, value);
 }
 
+/* Write to an absolute offset in the adapter's memory space. */
+#define E1000_WRITE_OFFSET(hw, offset, value) \
+	e1000_wr32((hw)->back, (offset), (value))
+
 /* Register READ/WRITE macros */
 
 #define E1000_READ_REG(hw, reg)	\
diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c
index aeb38fd4fb10..082549767315 100644
--- a/sys/dev/e1000/if_em.c
+++ b/sys/dev/e1000/if_em.c
@@ -1413,6 +1413,15 @@ em_if_attach_pre(if_ctx_t ctx)
 		error = ENXIO;
 		goto err_pci;
 	}
+	/*
+	 * 82579 can lose a host CSR write while the Management Engine owns
+	 * the PCIm2PCI arbiter.  Enable the OS register write interlock before
+	 * shared code initialization performs any MAC writes.
+	 */
+	if (hw->mac.type == e1000_pch2lan &&
+	    (E1000_READ_REG(hw, E1000_FWSM) &
+	    E1000_ICH_FWSM_FW_VALID) != 0)
+		sc->osdep.pcim2pci_arbiter_wa = true;
 
 	/*
 	** For ICH8 and family we need to