Commit 65ed2ffd authored by Mintz, Yuval's avatar Mintz, Yuval Committed by David S. Miller

qed*: Fix link indication race

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: default avatarYuval Mintz <Yuval.Mintz@cavium.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 85750d74
...@@ -1146,12 +1146,18 @@ static int qed_set_link(struct qed_dev *cdev, struct qed_link_params *params) ...@@ -1146,12 +1146,18 @@ static int qed_set_link(struct qed_dev *cdev, struct qed_link_params *params)
if (!cdev) if (!cdev)
return -ENODEV; return -ENODEV;
if (IS_VF(cdev))
return 0;
/* The link should be set only once per PF */ /* The link should be set only once per PF */
hwfn = &cdev->hwfns[0]; 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); ptt = qed_ptt_acquire(hwfn);
if (!ptt) if (!ptt)
return -EBUSY; return -EBUSY;
......
...@@ -192,6 +192,7 @@ int qed_mcp_cmd_init(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) ...@@ -192,6 +192,7 @@ int qed_mcp_cmd_init(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
/* Initialize the MFW spinlock */ /* Initialize the MFW spinlock */
spin_lock_init(&p_info->lock); spin_lock_init(&p_info->lock);
spin_lock_init(&p_info->link_lock);
return 0; return 0;
...@@ -610,6 +611,9 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn, ...@@ -610,6 +611,9 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn,
u8 max_bw, min_bw; u8 max_bw, min_bw;
u32 status = 0; 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; p_link = &p_hwfn->mcp_info->link_output;
memset(p_link, 0, sizeof(*p_link)); memset(p_link, 0, sizeof(*p_link));
if (!b_reset) { if (!b_reset) {
...@@ -624,7 +628,7 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn, ...@@ -624,7 +628,7 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn,
} else { } else {
DP_VERBOSE(p_hwfn, NETIF_MSG_LINK, DP_VERBOSE(p_hwfn, NETIF_MSG_LINK,
"Resetting link indications\n"); "Resetting link indications\n");
return; goto out;
} }
if (p_hwfn->b_drv_link_init) if (p_hwfn->b_drv_link_init)
...@@ -731,6 +735,8 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn, ...@@ -731,6 +735,8 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn,
p_link->sfp_tx_fault = !!(status & LINK_STATUS_SFP_TX_FAULT); p_link->sfp_tx_fault = !!(status & LINK_STATUS_SFP_TX_FAULT);
qed_link_update(p_hwfn); 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) int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool b_up)
...@@ -780,9 +786,13 @@ int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool b_up) ...@@ -780,9 +786,13 @@ int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool b_up)
return rc; return rc;
} }
/* Reset the link status if needed */ /* Mimic link-change attention, done for several reasons:
if (!b_up) * - On reset, there's no guarantee MFW would trigger
qed_mcp_handle_link_change(p_hwfn, p_ptt, true); * 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; return 0;
} }
......
...@@ -485,7 +485,13 @@ int qed_mcp_bist_nvm_test_get_image_att(struct qed_hwfn *p_hwfn, ...@@ -485,7 +485,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 % \ #define MFW_PORT(_p_hwfn) ((_p_hwfn)->abs_pf_id % \
((_p_hwfn)->cdev->num_ports_in_engines * 2)) ((_p_hwfn)->cdev->num_ports_in_engines * 2))
struct qed_mcp_info { struct qed_mcp_info {
/* Spinlock used for protecting the access to the MFW mailbox */
spinlock_t lock; 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; bool block_mb_sending;
u32 public_base; u32 public_base;
u32 drv_mb_addr; u32 drv_mb_addr;
......
...@@ -254,6 +254,7 @@ enum qed_iov_wq_flag { ...@@ -254,6 +254,7 @@ enum qed_iov_wq_flag {
QED_IOV_WQ_STOP_WQ_FLAG, QED_IOV_WQ_STOP_WQ_FLAG,
QED_IOV_WQ_FLR_FLAG, QED_IOV_WQ_FLR_FLAG,
QED_IOV_WQ_TRUST_FLAG, QED_IOV_WQ_TRUST_FLAG,
QED_IOV_WQ_VF_FORCE_LINK_QUERY_FLAG,
}; };
#ifdef CONFIG_QED_SRIOV #ifdef CONFIG_QED_SRIOV
......
...@@ -1285,6 +1285,9 @@ void qed_iov_vf_task(struct work_struct *work) ...@@ -1285,6 +1285,9 @@ void qed_iov_vf_task(struct work_struct *work)
/* Handle bulletin board changes */ /* Handle bulletin board changes */
qed_vf_read_bulletin(hwfn, &change); 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) if (change)
qed_handle_bulletin_change(hwfn); qed_handle_bulletin_change(hwfn);
......
...@@ -1872,7 +1872,6 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode, ...@@ -1872,7 +1872,6 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode,
bool is_locked) bool is_locked)
{ {
struct qed_link_params link_params; struct qed_link_params link_params;
struct qed_link_output link_output;
int rc; int rc;
DP_INFO(edev, "Starting qede load\n"); DP_INFO(edev, "Starting qede load\n");
...@@ -1924,11 +1923,7 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode, ...@@ -1924,11 +1923,7 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode,
link_params.link_up = true; link_params.link_up = true;
edev->ops->common->set_link(edev->cdev, &link_params); 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_roce_dev_event_open(edev);
qede_link_update(edev, &link_output);
qede_ptp_start(edev, (mode == QEDE_LOAD_NORMAL)); qede_ptp_start(edev, (mode == QEDE_LOAD_NORMAL));
......
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