• Lennert Buytenhek's avatar
    igc: Reinstate IGC_REMOVED logic and implement it properly · 7c1ddcee
    Lennert Buytenhek authored
    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>
    7c1ddcee
igc_regs.h 14.5 KB