[prev in list] [next in list] [prev in thread] [next in thread]
List: openbsd-tech
Subject: Looking for testers for em(4) quirks patch
From: Stefan Fritsch <sf () sfritsch ! de>
Date: 2018-04-02 9:52:08
Message-ID: alpine.DEB.2.20.1804021146490.31799 () manul ! sfritsch ! de
[Download RAW message or body]
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)
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic