Commit 7c1ddcee authored by Lennert Buytenhek's avatar Lennert Buytenhek Committed by Tony Nguyen

igc: Reinstate IGC_REMOVED logic and implement it properly

The initially merged version of the igc driver code (via commit
146740f9, "igc: Add support for PF") contained the following
IGC_REMOVED checks in the igc_rd32/wr32() MMIO accessors:

	u32 igc_rd32(struct igc_hw *hw, u32 reg)
	{
		u8 __iomem *hw_addr = READ_ONCE(hw->hw_addr);
		u32 value = 0;

		if (IGC_REMOVED(hw_addr))
			return ~value;

		value = readl(&hw_addr[reg]);

		/* reads should not return all F's */
		if (!(~value) && (!reg || !(~readl(hw_addr))))
			hw->hw_addr = NULL;

		return value;
	}

And:

	#define wr32(reg, val) \
	do { \
		u8 __iomem *hw_addr = READ_ONCE((hw)->hw_addr); \
		if (!IGC_REMOVED(hw_addr)) \
			writel((val), &hw_addr[(reg)]); \
	} while (0)

E.g. igb has similar checks in its MMIO accessors, and has a similar
macro E1000_REMOVED, which is implemented as follows:

	#define E1000_REMOVED(h) unlikely(!(h))

These checks serve to detect and take note of an 0xffffffff MMIO read
return from the device, which can be caused by a PCIe link flap or some
other kind of PCI bus error, and to avoid performing MMIO reads and
writes from that point onwards.

However, the IGC_REMOVED macro was not originally implemented:

	#ifndef IGC_REMOVED
	#define IGC_REMOVED(a) (0)
	#endif /* IGC_REMOVED */

This led to the IGC_REMOVED logic to be removed entirely in a
subsequent commit (commit 3c215fb1, "igc: remove IGC_REMOVED
function"), with the rationale that such checks matter only for
virtualization and that igc does not support virtualization -- but a
PCIe device can become detached even without virtualization being in
use, and without proper checks, a PCIe bus error affecting an igc
adapter will lead to various NULL pointer dereferences, as the first
access after the error will set hw->hw_addr to NULL, and subsequent
accesses will blindly dereference this now-NULL pointer.

This patch reinstates the IGC_REMOVED checks in igc_rd32/wr32(), and
implements IGC_REMOVED the way it is done for igb, by checking for the
unlikely() case of hw_addr being NULL.  This change prevents the oopses
seen when a PCIe link flap occurs on an igc adapter.

Fixes: 146740f9 ("igc: Add support for PF")
Signed-off-by: default avatarLennert Buytenhek <buytenh@arista.com>
Tested-by: default avatarNaama Meir <naamax.meir@linux.intel.com>
Acked-by: default avatarSasha Neftin <sasha.neftin@intel.com>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent 6cfa4536
...@@ -6171,6 +6171,9 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg) ...@@ -6171,6 +6171,9 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg)
u8 __iomem *hw_addr = READ_ONCE(hw->hw_addr); u8 __iomem *hw_addr = READ_ONCE(hw->hw_addr);
u32 value = 0; u32 value = 0;
if (IGC_REMOVED(hw_addr))
return ~value;
value = readl(&hw_addr[reg]); value = readl(&hw_addr[reg]);
/* reads should not return all F's */ /* reads should not return all F's */
......
...@@ -306,7 +306,8 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg); ...@@ -306,7 +306,8 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg);
#define wr32(reg, val) \ #define wr32(reg, val) \
do { \ do { \
u8 __iomem *hw_addr = READ_ONCE((hw)->hw_addr); \ u8 __iomem *hw_addr = READ_ONCE((hw)->hw_addr); \
writel((val), &hw_addr[(reg)]); \ if (!IGC_REMOVED(hw_addr)) \
writel((val), &hw_addr[(reg)]); \
} while (0) } while (0)
#define rd32(reg) (igc_rd32(hw, reg)) #define rd32(reg) (igc_rd32(hw, reg))
...@@ -318,4 +319,6 @@ do { \ ...@@ -318,4 +319,6 @@ do { \
#define array_rd32(reg, offset) (igc_rd32(hw, (reg) + ((offset) << 2))) #define array_rd32(reg, offset) (igc_rd32(hw, (reg) + ((offset) << 2)))
#define IGC_REMOVED(h) unlikely(!(h))
#endif #endif
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment