Hi, We have seen problems with em on i219V and i219LM. For example, "Hardware Initialization Failed" if no cable is plugged in during boot, or watchdog timeouts / hangs until next boot if the cable is removed while data is transmitted.
This patch adds a bunch of quirks and fixes from freebsd. It would be nice if people could give it a try on various em(4) hardware to check if there are any regressions. Comments on the patch are of course welcome, too. Cheers, Stefan * Some em chips have a semaphore ("software flag") to synchronize access to certain registers between OS and firmware (ME/AMT). Make the logic to get the flag match the logic in freebsd. This includes higher timeouts and waiting for a previous unlock to complete before trying a lock again. * Another problem was that openbsd em driver calls em_get_software_flag recursively, which causes the semaphore to be unlocked too early. Make em_get_software_flag/em_release_software_flag handle this correctly. Freebsd does not do this, but they have a mutex that probably allows them to detect recursive calls to e1000_acquire_swflag_ich8lan(). Reworking the openbsd driver to not recursively get the semaphore would be very invasive. * Port the logic from freebsd to em_check_phy_reset_block(). A single read does not seem to be reliable. * Also, increase delay after reset to 20ms, which is the value in freebsd for ich8lan. * Add another magic 1ms delay that seems to help with some remaining issues on an HP elitebook 820 G3. A printf() at the same place helps, too. * Port a silicon errata workaround from FreeBSD. https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/i218-i219-ethernet-connection-spec-update.pdf?asset=9561 * While there, print mac+phy type in em_attach(). This makes it easier to determine which quirks are hit/missing when comparing to freebsd. * Also print em_init_hw() error code if something goes wrong. diff --git sys/dev/pci/if_em.c sys/dev/pci/if_em.c index ec8e35245ef..f6a1f1c3894 100644 --- sys/dev/pci/if_em.c +++ sys/dev/pci/if_em.c @@ -557,6 +557,8 @@ em_attach(struct device *parent, struct device *self, void *aux) if (!defer) em_update_link_status(sc); + printf(", mac_type %#x phy_type %#x", sc->hw.mac_type, + sc->hw.phy_type); printf(", address %s\n", ether_sprintf(sc->sc_ac.ac_enaddr)); /* Indicate SOL/IDER usage */ @@ -1860,8 +1862,8 @@ em_hardware_init(struct em_softc *sc) INIT_DEBUGOUT("\nHardware Initialization Deferred "); return (EAGAIN); } - printf("\n%s: Hardware Initialization Failed\n", - DEVNAME(sc)); + printf("\n%s: Hardware Initialization Failed: %d\n", + DEVNAME(sc), ret_val); return (EIO); } @@ -2265,7 +2267,9 @@ em_initialize_transmit_unit(struct em_softc *sc) EM_WRITE_REG(&sc->hw, E1000_IOSFPC, reg_val); reg_val = E1000_READ_REG(&sc->hw, TARC0); - reg_val |= E1000_TARC0_CB_MULTIQ_3_REQ; + /* i218-i219 Specification Update 1.5.4.5 */ + reg_val &= ~E1000_TARC0_CB_MULTIQ_3_REQ; + reg_val |= E1000_TARC0_CB_MULTIQ_2_REQ; E1000_WRITE_REG(&sc->hw, TARC0, reg_val); } } diff --git sys/dev/pci/if_em_hw.c sys/dev/pci/if_em_hw.c index df0fa571736..d122e727875 100644 --- sys/dev/pci/if_em_hw.c +++ sys/dev/pci/if_em_hw.c @@ -945,7 +945,9 @@ em_reset_hw(struct em_hw *hw) } em_get_software_flag(hw); E1000_WRITE_REG(hw, CTRL, (ctrl | E1000_CTRL_RST)); - msec_delay(5); + /* HW reset releases software_flag */ + hw->sw_flag = 0; + msec_delay(20); /* Ungate automatic PHY configuration on non-managed 82579 */ if (hw->mac_type == em_pch2lan && !hw->phy_reset_disable && @@ -1491,6 +1493,8 @@ em_init_hw(struct em_hw *hw) /* Set the media type and TBI compatibility */ em_set_media_type(hw); + /* Magic delay that improves problems with i219LM on HP Elitebook */ + msec_delay(1); /* Must be called after em_set_media_type because media_type is used */ em_initialize_hardware_bits(hw); @@ -9539,9 +9543,18 @@ em_check_phy_reset_block(struct em_hw *hw) DEBUGFUNC("em_check_phy_reset_block\n"); if (IS_ICH8(hw->mac_type)) { - fwsm = E1000_READ_REG(hw, FWSM); - return (fwsm & E1000_FWSM_RSPCIPHY) ? E1000_SUCCESS : - E1000_BLK_PHY_RESET; + int i = 0; + int blocked = 0; + do { + fwsm = E1000_READ_REG(hw, FWSM); + if (!(fwsm & E1000_FWSM_RSPCIPHY)) { + blocked = 1; + msec_delay(10); + continue; + } + blocked = 0; + } while (blocked && (i++ < 30)); + return blocked ? E1000_BLK_PHY_RESET : E1000_SUCCESS; } if (hw->mac_type > em_82547_rev_2) manc = E1000_READ_REG(hw, MANC); @@ -9602,11 +9615,27 @@ em_get_software_flag(struct em_hw *hw) DEBUGFUNC("em_get_software_flag"); if (IS_ICH8(hw->mac_type)) { + if (hw->sw_flag) { + hw->sw_flag++; + return E1000_SUCCESS; + } while (timeout) { extcnf_ctrl = E1000_READ_REG(hw, EXTCNF_CTRL); - extcnf_ctrl |= E1000_EXTCNF_CTRL_SWFLAG; - E1000_WRITE_REG(hw, EXTCNF_CTRL, extcnf_ctrl); + if (!(extcnf_ctrl & E1000_EXTCNF_CTRL_SWFLAG)) + break; + msec_delay_irq(1); + timeout--; + } + if (!timeout) { + printf("%s: SW has already locked the resource?\n", + __func__); + return -E1000_ERR_CONFIG; + } + timeout = SW_FLAG_TIMEOUT; + extcnf_ctrl |= E1000_EXTCNF_CTRL_SWFLAG; + E1000_WRITE_REG(hw, EXTCNF_CTRL, extcnf_ctrl); + while (timeout) { extcnf_ctrl = E1000_READ_REG(hw, EXTCNF_CTRL); if (extcnf_ctrl & E1000_EXTCNF_CTRL_SWFLAG) break; @@ -9615,10 +9644,15 @@ em_get_software_flag(struct em_hw *hw) } if (!timeout) { - DEBUGOUT("FW or HW locks the resource too long.\n"); + printf("Failed to acquire the semaphore, FW or HW " + "has it: FWSM=0x%8.8x EXTCNF_CTRL=0x%8.8x)\n", + E1000_READ_REG(hw, FWSM), extcnf_ctrl); + extcnf_ctrl &= ~E1000_EXTCNF_CTRL_SWFLAG; + E1000_WRITE_REG(hw, EXTCNF_CTRL, extcnf_ctrl); return -E1000_ERR_CONFIG; } } + hw->sw_flag++; return E1000_SUCCESS; } @@ -9638,6 +9672,13 @@ em_release_software_flag(struct em_hw *hw) DEBUGFUNC("em_release_software_flag"); if (IS_ICH8(hw->mac_type)) { + if (hw->sw_flag <= 0) { + printf("%s: not locked!\n", __func__); + return; + } + hw->sw_flag--; + if (hw->sw_flag > 0) + return; extcnf_ctrl = E1000_READ_REG(hw, EXTCNF_CTRL); extcnf_ctrl &= ~E1000_EXTCNF_CTRL_SWFLAG; E1000_WRITE_REG(hw, EXTCNF_CTRL, extcnf_ctrl); diff --git sys/dev/pci/if_em_hw.h sys/dev/pci/if_em_hw.h index 5e415e60a34..9c2cfe97569 100644 --- sys/dev/pci/if_em_hw.h +++ sys/dev/pci/if_em_hw.h @@ -1634,6 +1634,7 @@ struct em_hw { uint8_t bus_func; uint16_t swfw; boolean_t eee_enable; + int sw_flag; }; #define E1000_EEPROM_SWDPIN0 0x0001 /* SWDPIN 0 EEPROM Value */ @@ -2295,6 +2296,7 @@ struct em_hw { #define E1000_WUS_FLX_FILTERS 0x000F0000 /* Mask for the 4 flexible filters */ /* TRAC0 bits */ +#define E1000_TARC0_CB_MULTIQ_2_REQ (1 << 29) #define E1000_TARC0_CB_MULTIQ_3_REQ (1 << 28 | 1 << 29) /* Management Control */ @@ -2755,6 +2757,8 @@ struct em_host_command_info { #define AUTO_READ_DONE_TIMEOUT 10 /* Number of milliseconds we wait for PHY configuration done after MAC reset */ #define PHY_CFG_TIMEOUT 100 +/* SW Semaphore flag timeout in ms */ +#define SW_FLAG_TIMEOUT 1000 #define E1000_TX_BUFFER_SIZE ((uint32_t)1514)