lkml.org 
[lkml]   [2022]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 053/201] igc: Reinstate IGC_REMOVED logic and implement it properly
    Date
    From: Lennert Buytenhek <buytenh@wantstofly.org>

    [ Upstream commit 7c1ddcee5311f3315096217881d2dbe47cc683f9 ]

    The initially merged version of the igc driver code (via commit
    146740f9abc4, "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 3c215fb18e70, "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: 146740f9abc4 ("igc: Add support for PF")
    Signed-off-by: Lennert Buytenhek <buytenh@arista.com>
    Tested-by: Naama Meir <naamax.meir@linux.intel.com>
    Acked-by: Sasha Neftin <sasha.neftin@intel.com>
    Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
    drivers/net/ethernet/intel/igc/igc_regs.h | 5 ++++-
    2 files changed, 7 insertions(+), 1 deletion(-)

    diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
    index f99819fc559d..2a84f57ea68b 100644
    --- a/drivers/net/ethernet/intel/igc/igc_main.c
    +++ b/drivers/net/ethernet/intel/igc/igc_main.c
    @@ -6159,6 +6159,9 @@ 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 */
    diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h
    index e197a33d93a0..026c3b65fc37 100644
    --- a/drivers/net/ethernet/intel/igc/igc_regs.h
    +++ b/drivers/net/ethernet/intel/igc/igc_regs.h
    @@ -306,7 +306,8 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg);
    #define wr32(reg, val) \
    do { \
    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)

    #define rd32(reg) (igc_rd32(hw, reg))
    @@ -318,4 +319,6 @@ do { \

    #define array_rd32(reg, offset) (igc_rd32(hw, (reg) + ((offset) << 2)))

    +#define IGC_REMOVED(h) unlikely(!(h))
    +
    #endif
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-07-27 19:07    [W:2.237 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site