From 030ec9e68239a8b7851701edc29de5745fe15024 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Mon, 26 Sep 2016 20:46:25 -0700 Subject: [PATCH 01/10] igb: Realign bad indentation Statements should start on tabstops. Use a single statement and test instead of multiple tests. Signed-off-by: Joe Perches Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/e1000_mac.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/e1000_mac.c b/drivers/net/ethernet/intel/igb/e1000_mac.c index 5010e2232c50..5eff82678f0b 100644 --- a/drivers/net/ethernet/intel/igb/e1000_mac.c +++ b/drivers/net/ethernet/intel/igb/e1000_mac.c @@ -792,15 +792,13 @@ static s32 igb_set_default_fc(struct e1000_hw *hw) * control setting, then the variable hw->fc will * be initialized based on a value in the EEPROM. */ - if (hw->mac.type == e1000_i350) { + if (hw->mac.type == e1000_i350) lan_offset = NVM_82580_LAN_FUNC_OFFSET(hw->bus.func); - ret_val = hw->nvm.ops.read(hw, NVM_INIT_CONTROL2_REG - + lan_offset, 1, &nvm_data); - } else { - ret_val = hw->nvm.ops.read(hw, NVM_INIT_CONTROL2_REG, - 1, &nvm_data); - } + else + lan_offset = 0; + ret_val = hw->nvm.ops.read(hw, NVM_INIT_CONTROL2_REG + lan_offset, + 1, &nvm_data); if (ret_val) { hw_dbg("NVM Read Error\n"); goto out; @@ -808,8 +806,7 @@ static s32 igb_set_default_fc(struct e1000_hw *hw) if ((nvm_data & NVM_WORD0F_PAUSE_MASK) == 0) hw->fc.requested_mode = e1000_fc_none; - else if ((nvm_data & NVM_WORD0F_PAUSE_MASK) == - NVM_WORD0F_ASM_DIR) + else if ((nvm_data & NVM_WORD0F_PAUSE_MASK) == NVM_WORD0F_ASM_DIR) hw->fc.requested_mode = e1000_fc_tx_pause; else hw->fc.requested_mode = e1000_fc_full; From 7e54d9d063fa239c95c21548c5267f0ef419ff56 Mon Sep 17 00:00:00 2001 From: khalidm Date: Mon, 17 Oct 2016 09:51:08 -0700 Subject: [PATCH 02/10] e1000e: driver trying to free already-free irq During systemd reboot sequence network driver interface is shutdown by e1000_close. The PCI driver interface is shut by e1000_shutdown. The e1000_shutdown checks for netif_running status, if still up it brings down driver. But it disables msi outside of this if statement, regardless of netif status. All this is OK when e1000_close happens after shutdown. However, by default, everything in systemd is done in parallel. This creates a conditions where e1000_shutdown is called after e1000_close, therefore hitting BUG_ON assert in free_msi_irqs. CC: xe-kernel@external.cisco.com Signed-off-by: khalidm Signed-off-by: David Singleton Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/e1000e/netdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index eccf1da9356b..af3960853a32 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -6276,8 +6276,8 @@ static int e1000e_pm_freeze(struct device *dev) /* Quiesce the device without resetting the hardware */ e1000e_down(adapter, false); e1000_free_irq(adapter); + e1000e_reset_interrupt_capability(adapter); } - e1000e_reset_interrupt_capability(adapter); /* Allow time for pending master requests to run */ e1000e_disable_pcie_master(&adapter->hw); From c4f9b6e5783c06b0b798091a4dd81c23c7ecfbe7 Mon Sep 17 00:00:00 2001 From: Cao jin Date: Wed, 2 Nov 2016 15:20:15 +0800 Subject: [PATCH 03/10] igb: correct register comments Signed-off-by: Cao jin Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/e1000_regs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/igb/e1000_regs.h b/drivers/net/ethernet/intel/igb/e1000_regs.h index d84afdd83e53..58adbf234e07 100644 --- a/drivers/net/ethernet/intel/igb/e1000_regs.h +++ b/drivers/net/ethernet/intel/igb/e1000_regs.h @@ -320,7 +320,7 @@ #define E1000_VT_CTL 0x0581C /* VMDq Control - RW */ #define E1000_WUC 0x05800 /* Wakeup Control - RW */ #define E1000_WUFC 0x05808 /* Wakeup Filter Control - RW */ -#define E1000_WUS 0x05810 /* Wakeup Status - RO */ +#define E1000_WUS 0x05810 /* Wakeup Status - R/W1C */ #define E1000_MANC 0x05820 /* Management Control - RW */ #define E1000_IPAV 0x05838 /* IP Address Valid - RW */ #define E1000_WUPL 0x05900 /* Wakeup Packet Length - RW */ From 4e684f59d760a2c7c716bb60190783546e2d08a1 Mon Sep 17 00:00:00 2001 From: Chris J Arges Date: Wed, 2 Nov 2016 09:13:42 -0500 Subject: [PATCH 04/10] igb: Workaround for igb i210 firmware issue Sometimes firmware may not properly initialize I347AT4_PAGE_SELECT causing the probe of an igb i210 NIC to fail. This patch adds an addition zeroing of this register during igb_get_phy_id to workaround this issue. Thanks for Jochen Henneberg for the idea and original patch. Signed-off-by: Chris J Arges Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/e1000_phy.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c index 5b54254aed4f..569ee25642b4 100644 --- a/drivers/net/ethernet/intel/igb/e1000_phy.c +++ b/drivers/net/ethernet/intel/igb/e1000_phy.c @@ -77,6 +77,10 @@ s32 igb_get_phy_id(struct e1000_hw *hw) s32 ret_val = 0; u16 phy_id; + /* ensure PHY page selection to fix misconfigured i210 */ + if (hw->mac.type == e1000_i210) + phy->ops.write_reg(hw, I347AT4_PAGE_SELECT, 0); + ret_val = phy->ops.read_reg(hw, PHY_ID1, &phy_id); if (ret_val) goto out; From 629823b872402451b42462414da08dddd0e2c93d Mon Sep 17 00:00:00 2001 From: Cao jin Date: Tue, 8 Nov 2016 15:06:20 +0800 Subject: [PATCH 05/10] igb: use igb_adapter->io_addr instead of e1000_hw->hw_addr When running as guest, under certain condition, it will oops as following. writel() in igb_configure_tx_ring() results in oops, because hw->hw_addr is NULL. While other register access won't oops kernel because they use wr32/rd32 which have a defense against NULL pointer. [ 141.225449] pcieport 0000:00:1c.0: AER: Multiple Uncorrected (Fatal) error received: id=0101 [ 141.225523] igb 0000:01:00.1: PCIe Bus Error: severity=Uncorrected (Fatal), type=Unaccessible, id=0101(Unregistered Agent ID) [ 141.299442] igb 0000:01:00.1: broadcast error_detected message [ 141.300539] igb 0000:01:00.0 enp1s0f0: PCIe link lost, device now detached [ 141.351019] igb 0000:01:00.1 enp1s0f1: PCIe link lost, device now detached [ 143.465904] pcieport 0000:00:1c.0: Root Port link has been reset [ 143.465994] igb 0000:01:00.1: broadcast slot_reset message [ 143.466039] igb 0000:01:00.0: enabling device (0000 -> 0002) [ 144.389078] igb 0000:01:00.1: enabling device (0000 -> 0002) [ 145.312078] igb 0000:01:00.1: broadcast resume message [ 145.322211] BUG: unable to handle kernel paging request at 0000000000003818 [ 145.361275] IP: [] igb_configure_tx_ring+0x14d/0x280 [igb] [ 145.400048] PGD 0 [ 145.438007] Oops: 0002 [#1] SMP A similar issue & solution could be found at: http://patchwork.ozlabs.org/patch/689592/ Signed-off-by: Cao jin Acked-by: Alexander Duyck Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/igb_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index a761001308dc..1e4bd84724b2 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -3394,7 +3394,7 @@ void igb_configure_tx_ring(struct igb_adapter *adapter, tdba & 0x00000000ffffffffULL); wr32(E1000_TDBAH(reg_idx), tdba >> 32); - ring->tail = hw->hw_addr + E1000_TDT(reg_idx); + ring->tail = adapter->io_addr + E1000_TDT(reg_idx); wr32(E1000_TDH(reg_idx), 0); writel(0, ring->tail); @@ -3733,7 +3733,7 @@ void igb_configure_rx_ring(struct igb_adapter *adapter, ring->count * sizeof(union e1000_adv_rx_desc)); /* initialize head and tail */ - ring->tail = hw->hw_addr + E1000_RDT(reg_idx); + ring->tail = adapter->io_addr + E1000_RDT(reg_idx); wr32(E1000_RDH(reg_idx), 0); writel(0, ring->tail); From 182785335447957409282ca745aa5bc3968facee Mon Sep 17 00:00:00 2001 From: Aaron Sierra Date: Tue, 29 Nov 2016 10:03:56 -0600 Subject: [PATCH 06/10] igb: reset the PHY before reading the PHY ID Several people have reported firmware leaving the I210/I211 PHY's page select register set to something other than the default of zero. This causes the first accesses, PHY_IDx register reads, to access something else, resulting in device probe failure: igb: Intel(R) Gigabit Ethernet Network Driver - version 5.4.0-k igb: Copyright (c) 2007-2014 Intel Corporation. igb: probe of 0000:01:00.0 failed with error -2 This problem began for them after a previous patch I submitted was applied: commit 2a3cdead8b408351fa1e3079b220fa331480ffbc Author: Aaron Sierra Date: Tue Nov 3 12:37:09 2015 -0600 igb: Remove GS40G specific defines/functions I personally experienced this problem after attempting to PXE boot from I210 devices using this firmware: Intel(R) Boot Agent GE v1.5.78 Copyright (C) 1997-2014, Intel Corporation Resetting the PHY before reading from it, ensures the page select register is in its default state and doesn't make assumptions about the PHY's register set before the PHY has been probed. Cc: Matwey V. Kornilov Cc: Chris Arges Cc: Jochen Henneberg Signed-off-by: Aaron Sierra Tested-by: Matwey V. Kornilov Tested-by: Chris J Arges Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/e1000_82575.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c index a61447fd778e..ee443985581f 100644 --- a/drivers/net/ethernet/intel/igb/e1000_82575.c +++ b/drivers/net/ethernet/intel/igb/e1000_82575.c @@ -245,6 +245,17 @@ static s32 igb_init_phy_params_82575(struct e1000_hw *hw) hw->bus.func = (rd32(E1000_STATUS) & E1000_STATUS_FUNC_MASK) >> E1000_STATUS_FUNC_SHIFT; + /* Make sure the PHY is in a good state. Several people have reported + * firmware leaving the PHY's page select register set to something + * other than the default of zero, which causes the PHY ID read to + * access something other than the intended register. + */ + ret_val = hw->phy.ops.reset(hw); + if (ret_val) { + hw_dbg("Error resetting the PHY.\n"); + goto out; + } + /* Set phy->phy_addr and phy->id. */ ret_val = igb_get_phy_id_82575(hw); if (ret_val) From 69b97cf6dbce7403845a28bbc75d57f5be7b12ac Mon Sep 17 00:00:00 2001 From: Guilherme G Piccoli Date: Thu, 10 Nov 2016 16:46:43 -0200 Subject: [PATCH 07/10] igb: re-assign hw address pointer on reset after PCI error Whenever the igb driver detects the result of a read operation returns a value composed only by F's (like 0xFFFFFFFF), it will detach the net_device, clear the hw_addr pointer and warn to the user that adapter's link is lost - those steps happen on igb_rd32(). In case a PCI error happens on Power architecture, there's a recovery mechanism called EEH, that will reset the PCI slot and call driver's handlers to reset the adapter and network functionality as well. We observed that once hw_addr is NULL after the error is detected on igb_rd32(), it's never assigned back, so in the process of resetting the network functionality we got a NULL pointer dereference in both igb_configure_tx_ring() and igb_configure_rx_ring(). In order to avoid such bug, this patch re-assigns the hw_addr value in the slot_reset handler. Reported-by: Anthony H Thai Reported-by: Harsha Thyagaraja Signed-off-by: Guilherme G Piccoli Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 1e4bd84724b2..82069bcabb9f 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -7898,6 +7898,11 @@ static pci_ers_result_t igb_io_slot_reset(struct pci_dev *pdev) pci_enable_wake(pdev, PCI_D3hot, 0); pci_enable_wake(pdev, PCI_D3cold, 0); + /* In case of PCI error, adapter lose its HW address + * so we should re-assign it here. + */ + hw->hw_addr = adapter->io_addr; + igb_reset(adapter); wr32(E1000_WUS, ~0); result = PCI_ERS_RESULT_RECOVERED; From 9474933caf21a4cb5147223dca1551f527aaac36 Mon Sep 17 00:00:00 2001 From: Todd Fujinaka Date: Tue, 15 Nov 2016 08:54:26 -0800 Subject: [PATCH 08/10] igb: close/suspend race in netif_device_detach Similar to ixgbe, when an interface is part of a namespace it is possible that igb_close() may be called while __igb_shutdown() is running which ends up in a double free WARN and/or a BUG in free_msi_irqs(). Extend the rtnl_lock() to protect the call to netif_device_detach() and igb_clear_interrupt_scheme() in __igb_shutdown() and check for netif_device_present() to avoid calling igb_clear_interrupt_scheme() a second time in igb_close(). Also extend the rtnl lock in igb_resume() to netif_device_attach(). Signed-off-by: Todd Fujinaka Acked-by: Alexander Duyck Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/igb_main.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 82069bcabb9f..594604e09f8d 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -3275,7 +3275,9 @@ static int __igb_close(struct net_device *netdev, bool suspending) int igb_close(struct net_device *netdev) { - return __igb_close(netdev, false); + if (netif_device_present(netdev)) + return __igb_close(netdev, false); + return 0; } /** @@ -7564,6 +7566,7 @@ static int __igb_shutdown(struct pci_dev *pdev, bool *enable_wake, int retval = 0; #endif + rtnl_lock(); netif_device_detach(netdev); if (netif_running(netdev)) @@ -7572,6 +7575,7 @@ static int __igb_shutdown(struct pci_dev *pdev, bool *enable_wake, igb_ptp_suspend(adapter); igb_clear_interrupt_scheme(adapter); + rtnl_unlock(); #ifdef CONFIG_PM retval = pci_save_state(pdev); @@ -7690,16 +7694,15 @@ static int igb_resume(struct device *dev) wr32(E1000_WUS, ~0); - if (netdev->flags & IFF_UP) { - rtnl_lock(); + rtnl_lock(); + if (!err && netif_running(netdev)) err = __igb_open(netdev, true); - rtnl_unlock(); - if (err) - return err; - } - netif_device_attach(netdev); - return 0; + if (!err) + netif_device_attach(netdev); + rtnl_unlock(); + + return err; } static int igb_runtime_idle(struct device *dev) From 5bc8c230e2a993b49244f9457499f17283da9ec7 Mon Sep 17 00:00:00 2001 From: Todd Fujinaka Date: Mon, 28 Nov 2016 09:09:57 -0800 Subject: [PATCH 09/10] igb: add i211 to i210 PHY workaround i210 and i211 share the same PHY but have different PCI IDs. Don't forget i211 for any i210 workarounds. Signed-off-by: Todd Fujinaka Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/e1000_phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c index 569ee25642b4..2788a5409023 100644 --- a/drivers/net/ethernet/intel/igb/e1000_phy.c +++ b/drivers/net/ethernet/intel/igb/e1000_phy.c @@ -78,7 +78,7 @@ s32 igb_get_phy_id(struct e1000_hw *hw) u16 phy_id; /* ensure PHY page selection to fix misconfigured i210 */ - if (hw->mac.type == e1000_i210) + if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211)) phy->ops.write_reg(hw, I347AT4_PAGE_SELECT, 0); ret_val = phy->ops.read_reg(hw, PHY_ID1, &phy_id); From 76ed5a8f47476e4984cc8c0c1bc4cee62650f7fd Mon Sep 17 00:00:00 2001 From: Hannu Lounento Date: Mon, 2 Jan 2017 18:26:06 +0100 Subject: [PATCH 10/10] igb: Fix hw_dbg logging in igb_update_flash_i210 Fix an if statement with hw_dbg lines where the logic was inverted with regards to the corresponding return value used in the if statement. Signed-off-by: Hannu Lounento Signed-off-by: Peter Senna Tschudin Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/igb/e1000_i210.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/e1000_i210.c b/drivers/net/ethernet/intel/igb/e1000_i210.c index 8aa798737d4d..07d48f2e3369 100644 --- a/drivers/net/ethernet/intel/igb/e1000_i210.c +++ b/drivers/net/ethernet/intel/igb/e1000_i210.c @@ -699,9 +699,9 @@ static s32 igb_update_flash_i210(struct e1000_hw *hw) ret_val = igb_pool_flash_update_done_i210(hw); if (ret_val) - hw_dbg("Flash update complete\n"); - else hw_dbg("Flash update time out\n"); + else + hw_dbg("Flash update complete\n"); out: return ret_val;