qed*: Fix link indication race

Orabug: 25477939

Driver changes the link properties via communication with
the management firmware, and re-reads the resulting link status
when it receives an indication that the link has changed.
However, there are certain scenarios where such indications
might be missing, and so driver also re-reads the current link
results without attention in several places. Specifically, it
does so during load and when resetting the link.

This creates a race where driver might reflect incorrect
link status - e.g., when explicit reading of the link status is
switched by attention with the changed configuration.

Correct this flow by a lock syncronizing the handling of the
link indications [both explicit requests and attention].

Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 65ed2ffd640578166e4ec149573bcf1d10f81b81)
Signed-off-by: Brian Maly <brian.maly@oracle.com>
Conflicts:
	drivers/net/ethernet/qlogic/qed/qed_sriov.h

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 6b217f957ed02bd8169147fae99e9e5009df6c12..9f17309a52a9625bfa9aac527e9abb24f0a1a669 100644 (file)
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1144,12 +1144,18 @@ static int qed_set_link(struct qed_dev *cdev, struct qed_link_params *params)
        if (!cdev)
                return -ENODEV;
 
-       if (IS_VF(cdev))
-               return 0;
-
        /* The link should be set only once per PF */
        hwfn = &cdev->hwfns[0];
 
+       /* When VF wants to set link, force it to read the bulletin instead.
+        * This mimics the PF behavior, where a noitification [both immediate
+        * and possible later] would be generated when changing properties.
+        */
+       if (IS_VF(cdev)) {
+               qed_schedule_iov(hwfn, QED_IOV_WQ_VF_FORCE_LINK_QUERY_FLAG);
+               return 0;
+       }
+
        ptt = qed_ptt_acquire(hwfn);
        if (!ptt)
                return -EBUSY;

diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 5ecf42df9f1f4028e475cc69ae76de1e3060556f..64c39bce0e6a8af4fd9255648481080f8b6f86e4 100644 (file)
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -192,6 +192,7 @@ int qed_mcp_cmd_init(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 
        /* Initialize the MFW spinlock */
        spin_lock_init(&p_info->lock);
+       spin_lock_init(&p_info->link_lock);
 
        return 0;
 
@@ -610,6 +611,9 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn,
        u8 max_bw, min_bw;
        u32 status = 0;
 
+       /* Prevent SW/attentions from doing this at the same time */
+       spin_lock_bh(&p_hwfn->mcp_info->link_lock);
+
        p_link = &p_hwfn->mcp_info->link_output;
        memset(p_link, 0, sizeof(*p_link));
        if (!b_reset) {
@@ -624,7 +628,7 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn,
        } else {
                DP_VERBOSE(p_hwfn, NETIF_MSG_LINK,
                           "Resetting link indications\n");
-               return;
+               goto out;
        }
 
        if (p_hwfn->b_drv_link_init)
@@ -728,6 +732,8 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn,
        p_link->sfp_tx_fault = !!(status & LINK_STATUS_SFP_TX_FAULT);
 
        qed_link_update(p_hwfn);
+out:
+       spin_unlock_bh(&p_hwfn->mcp_info->link_lock);
 }
 
 int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool b_up)
@@ -777,9 +783,13 @@ int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool b_up)
                return rc;
        }
 
-       /* Reset the link status if needed */
-       if (!b_up)
-               qed_mcp_handle_link_change(p_hwfn, p_ptt, true);
+       /* Mimic link-change attention, done for several reasons:
+        *  - On reset, there's no guarantee MFW would trigger
+        *    an attention.
+        *  - On initialization, older MFWs might not indicate link change
+        *    during LFA, so we'll never get an UP indication.
+        */
+       qed_mcp_handle_link_change(p_hwfn, p_ptt, !b_up);
 
        return 0;
 }

diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index a941a82de32cacc08f0428a8fbdadcb136a91b70..6560d106d4e7b2eeeaebe42f8dc8045fb22cb860 100644 (file)
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -484,7 +484,13 @@ int qed_mcp_bist_nvm_test_get_image_att(struct qed_hwfn *p_hwfn,
 #define MFW_PORT(_p_hwfn)       ((_p_hwfn)->abs_pf_id %        \
                                 ((_p_hwfn)->cdev->num_ports_in_engines * 2))
 struct qed_mcp_info {
+       /* Spinlock used for protecting the access to the MFW mailbox */
        spinlock_t                              lock;
+
+       /* Spinlock used for syncing SW link-changes and link-changes
+        * originating from attention context.
+        */
+       spinlock_t                              link_lock;
        bool                                    block_mb_sending;
        u32                                     public_base;
        u32                                     drv_mb_addr;

diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.h b/drivers/net/ethernet/qlogic/qed/qed_sriov.h
index 1738d5bd2e8e504dd6c8d8f3bbb2590887248e36..9bb3276f1d0fe23698223639b93e960c04002e4f 100644 (file)
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.h
@@ -245,6 +245,7 @@ enum qed_iov_wq_flag {
        QED_IOV_WQ_BULLETIN_UPDATE_FLAG,
        QED_IOV_WQ_STOP_WQ_FLAG,
        QED_IOV_WQ_FLR_FLAG,
+       QED_IOV_WQ_VF_FORCE_LINK_QUERY_FLAG,
 };
 
 #ifdef CONFIG_QED_SRIOV

diff --git a/drivers/net/ethernet/qlogic/qed/qed_vf.c b/drivers/net/ethernet/qlogic/qed/qed_vf.c
index af0542c0351cff4899e22e1db6832ca47a4c9872..c5b5eefea892d3004e602d07d47821a6184fe198 100644 (file)
--- a/drivers/net/ethernet/qlogic/qed/qed_vf.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_vf.c
@@ -1277,6 +1277,9 @@ void qed_iov_vf_task(struct work_struct *work)
 
        /* Handle bulletin board changes */
        qed_vf_read_bulletin(hwfn, &change);
+       if (test_and_clear_bit(QED_IOV_WQ_VF_FORCE_LINK_QUERY_FLAG,
+                              &hwfn->iov_task_flags))
+               change = 1;
        if (change)
                qed_handle_bulletin_change(hwfn);
 

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 57267e54c69e8ec352292e0b4101183458695a4d..a0845d9a8f2c025ad117b9b4c67f40a00c1bf750 100644 (file)
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -3638,7 +3638,6 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode,
                     bool is_locked)
 {
        struct qed_link_params link_params;
-       struct qed_link_output link_output;
        int rc;
 
        DP_INFO(edev, "Starting qede load\n");
@@ -3690,11 +3689,7 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode,
        link_params.link_up = true;
        edev->ops->common->set_link(edev->cdev, &link_params);
 
-       /* Query whether link is already-up */
-       memset(&link_output, 0, sizeof(link_output));
-       edev->ops->common->get_link(edev->cdev, &link_output);
        qede_roce_dev_event_open(edev);
-       qede_link_update(edev, &link_output);
 
        edev->state = QEDE_STATE_OPEN;