[PATCH net 00/10] qed*: Bug fixes

[PATCH net 00/10] qed*: Bug fixes

[Yuval Mintz]

Hi Dave,

The 3 first patches here are a repost of
("qed: RoCE infrastructure fixes"). The fourth is an additional
RoCE-related infrastructure fix, and the latter contain various fixes
to qed/qede.

Notice patch #10 will create a conflict when net and net-next would
be merged [although its fix is quite trivial].

Please consider applying these to `net'.

Thanks,
Yuval

Ram Amrani (4):
  qed: Release CQ resource under lock on failure
  qed: Read queue state before releasing buffer
  qed: Don't free a QP more than once
  qed: Reserve doorbell BAR space for all present CPUs

Sudarsana Reddy Kalluru (1):
  qede: Prevent index problems in loopback test

Yuval Mintz (5):
  qede: Initialize lock and slowpath workqueue early
  qede: Free netdevice only after stoping slowpath
  qed: Reflect PF link when initializing VF
  qed: Don't allocate SBs using main PTT
  qed*: Fix link indication race

 drivers/net/ethernet/qlogic/qed/qed_dev.c       |  2 +-
 drivers/net/ethernet/qlogic/qed/qed_main.c      | 27 ++++++--
 drivers/net/ethernet/qlogic/qed/qed_mcp.c       | 18 +++--
 drivers/net/ethernet/qlogic/qed/qed_mcp.h       |  6 ++
 drivers/net/ethernet/qlogic/qed/qed_roce.c      | 63 +++++++++--------
 drivers/net/ethernet/qlogic/qed/qed_sriov.c     | 90 ++++++++++++++-----------
 drivers/net/ethernet/qlogic/qed/qed_sriov.h     |  1 +
 drivers/net/ethernet/qlogic/qed/qed_vf.c        |  3 +
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c |  6 +-
 drivers/net/ethernet/qlogic/qede/qede_main.c    | 24 ++++---
 10 files changed, 151 insertions(+), 89 deletions(-)

-- 
1.9.3

[PATCH net 01/10] qed: Release CQ resource under lock on failure

[Yuval Mintz]

From: Ram Amrani <Ram.Amrani@cavium.com>

The CQ resource pool is protected by a spin lock. When a CQ creation
fails it now deallocates under that lock as well.

Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_roce.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c b/drivers/net/ethernet/qlogic/qed/qed_roce.c
index 2dbdb32..d1ad48d 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
@@ -948,7 +948,9 @@ static int qed_rdma_create_cq(void *rdma_cxt,
 
 err:
 	/* release allocated icid */
+	spin_lock_bh(&p_info->lock);
 	qed_bmap_release_id(p_hwfn, &p_info->cq_map, returned_id);
+	spin_unlock_bh(&p_info->lock);
 	DP_NOTICE(p_hwfn, "Create CQ failed, rc = %d\n", rc);
 
 	return rc;
-- 
1.9.3

[PATCH net 02/10] qed: Read queue state before releasing buffer

[Yuval Mintz]

From: Ram Amrani <Ram.Amrani@cavium.com>

Currently the state is read only after the buffers are relesed.

Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_roce.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c b/drivers/net/ethernet/qlogic/qed/qed_roce.c
index d1ad48d..685d74c 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
@@ -1768,13 +1768,13 @@ static int qed_roce_query_qp(struct qed_hwfn *p_hwfn,
 	if (rc)
 		goto err_resp;
 
-	dma_free_coherent(&p_hwfn->cdev->pdev->dev, sizeof(*p_resp_ramrod_res),
-			  p_resp_ramrod_res, resp_ramrod_res_phys);
-
 	out_params->rq_psn = le32_to_cpu(p_resp_ramrod_res->psn);
 	rq_err_state = GET_FIELD(le32_to_cpu(p_resp_ramrod_res->err_flag),
 				 ROCE_QUERY_QP_RESP_OUTPUT_PARAMS_ERROR_FLG);
 
+	dma_free_coherent(&p_hwfn->cdev->pdev->dev, sizeof(*p_resp_ramrod_res),
+			  p_resp_ramrod_res, resp_ramrod_res_phys);
+
 	if (!(qp->req_offloaded)) {
 		/* Don't send query qp for the requester */
 		out_params->sq_psn = qp->sq_psn;
@@ -1815,9 +1815,6 @@ static int qed_roce_query_qp(struct qed_hwfn *p_hwfn,
 	if (rc)
 		goto err_req;
 
-	dma_free_coherent(&p_hwfn->cdev->pdev->dev, sizeof(*p_req_ramrod_res),
-			  p_req_ramrod_res, req_ramrod_res_phys);
-
 	out_params->sq_psn = le32_to_cpu(p_req_ramrod_res->psn);
 	sq_err_state = GET_FIELD(le32_to_cpu(p_req_ramrod_res->flags),
 				 ROCE_QUERY_QP_REQ_OUTPUT_PARAMS_ERR_FLG);
@@ -1825,6 +1822,9 @@ static int qed_roce_query_qp(struct qed_hwfn *p_hwfn,
 		GET_FIELD(le32_to_cpu(p_req_ramrod_res->flags),
 			  ROCE_QUERY_QP_REQ_OUTPUT_PARAMS_SQ_DRAINING_FLG);
 
+	dma_free_coherent(&p_hwfn->cdev->pdev->dev, sizeof(*p_req_ramrod_res),
+			  p_req_ramrod_res, req_ramrod_res_phys);
+
 	out_params->draining = false;
 
 	if (rq_err_state)
-- 
1.9.3

[PATCH net 03/10] qed: Don't free a QP more than once

[Yuval Mintz]

From: Ram Amrani <Ram.Amrani@cavium.com>

If QP is in reset state then there are no resources to free so avoid
freeing any.

Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_roce.c | 49 ++++++++++++++++--------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c b/drivers/net/ethernet/qlogic/qed/qed_roce.c
index 685d74c..5aeb4fa 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
@@ -1849,6 +1849,7 @@ static int qed_roce_query_qp(struct qed_hwfn *p_hwfn,
 
 static int qed_roce_destroy_qp(struct qed_hwfn *p_hwfn, struct qed_rdma_qp *qp)
 {
+	struct qed_rdma_info *p_rdma_info = p_hwfn->p_rdma_info;
 	u32 num_invalidated_mw = 0;
 	u32 num_bound_mw = 0;
 	u32 start_cid;
@@ -1863,35 +1864,39 @@ static int qed_roce_destroy_qp(struct qed_hwfn *p_hwfn, struct qed_rdma_qp *qp)
 		return -EINVAL;
 	}
 
-	rc = qed_roce_sp_destroy_qp_responder(p_hwfn, qp, &num_invalidated_mw);
-	if (rc)
-		return rc;
+	if (qp->cur_state != QED_ROCE_QP_STATE_RESET) {
+		rc = qed_roce_sp_destroy_qp_responder(p_hwfn, qp,
+						      &num_invalidated_mw);
+		if (rc)
+			return rc;
 
-	/* Send destroy requester ramrod */
-	rc = qed_roce_sp_destroy_qp_requester(p_hwfn, qp, &num_bound_mw);
-	if (rc)
-		return rc;
+		/* Send destroy requester ramrod */
+		rc = qed_roce_sp_destroy_qp_requester(p_hwfn, qp,
+						      &num_bound_mw);
+		if (rc)
+			return rc;
 
-	if (num_invalidated_mw != num_bound_mw) {
-		DP_NOTICE(p_hwfn,
-			  "number of invalidate memory windows is different from bounded ones\n");
-		return -EINVAL;
-	}
+		if (num_invalidated_mw != num_bound_mw) {
+			DP_NOTICE(p_hwfn,
+				  "number of invalidate memory windows is different from bounded ones\n");
+			return -EINVAL;
+		}
 
-	spin_lock_bh(&p_hwfn->p_rdma_info->lock);
+		spin_lock_bh(&p_rdma_info->lock);
 
-	start_cid = qed_cxt_get_proto_cid_start(p_hwfn,
-						p_hwfn->p_rdma_info->proto);
+		start_cid = qed_cxt_get_proto_cid_start(p_hwfn,
+							p_rdma_info->proto);
 
-	/* Release responder's icid */
-	qed_bmap_release_id(p_hwfn, &p_hwfn->p_rdma_info->cid_map,
-			    qp->icid - start_cid);
+		/* Release responder's icid */
+		qed_bmap_release_id(p_hwfn, &p_rdma_info->cid_map,
+				    qp->icid - start_cid);
 
-	/* Release requester's icid */
-	qed_bmap_release_id(p_hwfn, &p_hwfn->p_rdma_info->cid_map,
-			    qp->icid + 1 - start_cid);
+		/* Release requester's icid */
+		qed_bmap_release_id(p_hwfn, &p_rdma_info->cid_map,
+				    qp->icid + 1 - start_cid);
 
-	spin_unlock_bh(&p_hwfn->p_rdma_info->lock);
+		spin_unlock_bh(&p_rdma_info->lock);
+	}
 
 	return 0;
 }
-- 
1.9.3

[PATCH net 04/10] qed: Reserve doorbell BAR space for present CPUs

[Yuval Mintz]

From: Ram Amrani <Ram.Amrani@cavium.com>

Reserving doorbell BAR space according to the currently active CPUs
may result in a bug if disabled CPUs are later enabled but no
doorbell space was reserved for them.

Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 3b22500..993df15 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -873,7 +873,7 @@ enum QED_ROCE_EDPM_MODE {
 		/* Either EDPM is mandatory, or we are attempting to allocate a
 		 * WID per CPU.
 		 */
-		n_cpus = num_active_cpus();
+		n_cpus = num_present_cpus();
 		rc = qed_hw_init_dpi_size(p_hwfn, p_ptt, pwm_regsize, n_cpus);
 	}
 
-- 
1.9.3

[PATCH net 05/10] qede: Initialize lock and slowpath workqueue early

[Yuval Mintz]

Need to make sure the slowpath workqueue and the qede lock
are ready for the registration of the netdevice, as once
registered there's no guarantee those wouldn't be used.

Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qede/qede_main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index aecdd1c..5e76249 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2883,6 +2883,13 @@ static int __qede_probe(struct pci_dev *pdev, u32 dp_module, u8 dp_level,
 	if (rc)
 		goto err3;
 
+	/* Prepare the lock prior to the registeration of the netdev,
+	 * as once it's registered we might reach flows requiring it
+	 * [it's even possible to reach a flow needing it directly
+	 * from there, although it's unlikely].
+	 */
+	INIT_DELAYED_WORK(&edev->sp_task, qede_sp_task);
+	mutex_init(&edev->qede_lock);
 	rc = register_netdev(edev->ndev);
 	if (rc) {
 		DP_NOTICE(edev, "Cannot register net-device\n");
@@ -2898,8 +2905,6 @@ static int __qede_probe(struct pci_dev *pdev, u32 dp_module, u8 dp_level,
 		qede_set_dcbnl_ops(edev->ndev);
 #endif
 
-	INIT_DELAYED_WORK(&edev->sp_task, qede_sp_task);
-	mutex_init(&edev->qede_lock);
 	edev->rx_copybreak = QEDE_RX_HDR_SIZE;
 
 	DP_INFO(edev, "Ending successfully qede probe\n");
-- 
1.9.3

[PATCH net 06/10] qede: Free netdevice only after stoping slowpath

[Yuval Mintz]

qed needs to be informed of the removal of the qede interface
prior to its actual removal, as qede has some registered callbacks
that might get called async to the removal flow.

Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qede/qede_main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 5e76249..9c9f50c 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2972,14 +2972,20 @@ static void __qede_remove(struct pci_dev *pdev, enum qede_remove_mode mode)
 	if (edev->xdp_prog)
 		bpf_prog_put(edev->xdp_prog);
 
-	free_netdev(ndev);
-
 	/* Use global ops since we've freed edev */
 	qed_ops->common->slowpath_stop(cdev);
 	if (system_state == SYSTEM_POWER_OFF)
 		return;
 	qed_ops->common->remove(cdev);
 
+	/* Since this can happen out-of-sync with other flows,
+	 * don't release the netdevice until after slowpath stop
+	 * has been called to guarantee various other contexts
+	 * [e.g., QED register callbacks] won't break anything when
+	 * accessing the netdevice.
+	 */
+	 free_netdev(ndev);
+
 	dev_info(&pdev->dev, "Ending qede_remove successfully\n");
 }
 
-- 
1.9.3

[PATCH net 07/10] qed: Reflect PF link when initializing VF

[Yuval Mintz]

VF learns of the current link state via its bulletin board,
which might reflect either the physical link state or some
user-configured logical state.
Whenever the physical link changes or whnever such a configuration
is explicitly made by user the PF driver would update the bulletin
that the VF reads. But if neither has happened - i.e., PF still
hasn't got a physical link up and no additional configuration was
done the VF wouldn't have a valid link information available.

Simply reflect the physical link state whenever the VF is
initialized. The user could then affect it however he wants.

Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_sriov.c | 90 ++++++++++++++++-------------
 1 file changed, 51 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
index 85b09dd..7d9941a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
@@ -806,10 +806,52 @@ static void qed_iov_free_vf_igu_sbs(struct qed_hwfn *p_hwfn,
 	vf->num_sbs = 0;
 }
 
+static void qed_iov_set_link(struct qed_hwfn *p_hwfn,
+			     u16 vfid,
+			     struct qed_mcp_link_params *params,
+			     struct qed_mcp_link_state *link,
+			     struct qed_mcp_link_capabilities *p_caps)
+{
+	struct qed_vf_info *p_vf = qed_iov_get_vf_info(p_hwfn,
+						       vfid,
+						       false);
+	struct qed_bulletin_content *p_bulletin;
+
+	if (!p_vf)
+		return;
+
+	p_bulletin = p_vf->bulletin.p_virt;
+	p_bulletin->req_autoneg = params->speed.autoneg;
+	p_bulletin->req_adv_speed = params->speed.advertised_speeds;
+	p_bulletin->req_forced_speed = params->speed.forced_speed;
+	p_bulletin->req_autoneg_pause = params->pause.autoneg;
+	p_bulletin->req_forced_rx = params->pause.forced_rx;
+	p_bulletin->req_forced_tx = params->pause.forced_tx;
+	p_bulletin->req_loopback = params->loopback_mode;
+
+	p_bulletin->link_up = link->link_up;
+	p_bulletin->speed = link->speed;
+	p_bulletin->full_duplex = link->full_duplex;
+	p_bulletin->autoneg = link->an;
+	p_bulletin->autoneg_complete = link->an_complete;
+	p_bulletin->parallel_detection = link->parallel_detection;
+	p_bulletin->pfc_enabled = link->pfc_enabled;
+	p_bulletin->partner_adv_speed = link->partner_adv_speed;
+	p_bulletin->partner_tx_flow_ctrl_en = link->partner_tx_flow_ctrl_en;
+	p_bulletin->partner_rx_flow_ctrl_en = link->partner_rx_flow_ctrl_en;
+	p_bulletin->partner_adv_pause = link->partner_adv_pause;
+	p_bulletin->sfp_tx_fault = link->sfp_tx_fault;
+
+	p_bulletin->capability_speed = p_caps->speed_capabilities;
+}
+
 static int qed_iov_init_hw_for_vf(struct qed_hwfn *p_hwfn,
 				  struct qed_ptt *p_ptt,
 				  struct qed_iov_vf_init_params *p_params)
 {
+	struct qed_mcp_link_capabilities link_caps;
+	struct qed_mcp_link_params link_params;
+	struct qed_mcp_link_state link_state;
 	u8 num_of_vf_avaiable_chains = 0;
 	struct qed_vf_info *vf = NULL;
 	u16 qid, num_irqs;
@@ -898,6 +940,15 @@ static int qed_iov_init_hw_for_vf(struct qed_hwfn *p_hwfn,
 			   p_queue->fw_tx_qid, p_queue->fw_cid);
 	}
 
+	/* Update the link configuration in bulletin */
+	memcpy(&link_params, qed_mcp_get_link_params(p_hwfn),
+	       sizeof(link_params));
+	memcpy(&link_state, qed_mcp_get_link_state(p_hwfn), sizeof(link_state));
+	memcpy(&link_caps, qed_mcp_get_link_capabilities(p_hwfn),
+	       sizeof(link_caps));
+	qed_iov_set_link(p_hwfn, p_params->rel_vf_id,
+			 &link_params, &link_state, &link_caps);
+
 	rc = qed_iov_enable_vf_access(p_hwfn, p_ptt, vf);
 	if (!rc) {
 		vf->b_init = true;
@@ -909,45 +960,6 @@ static int qed_iov_init_hw_for_vf(struct qed_hwfn *p_hwfn,
 	return rc;
 }
 
-static void qed_iov_set_link(struct qed_hwfn *p_hwfn,
-			     u16 vfid,
-			     struct qed_mcp_link_params *params,
-			     struct qed_mcp_link_state *link,
-			     struct qed_mcp_link_capabilities *p_caps)
-{
-	struct qed_vf_info *p_vf = qed_iov_get_vf_info(p_hwfn,
-						       vfid,
-						       false);
-	struct qed_bulletin_content *p_bulletin;
-
-	if (!p_vf)
-		return;
-
-	p_bulletin = p_vf->bulletin.p_virt;
-	p_bulletin->req_autoneg = params->speed.autoneg;
-	p_bulletin->req_adv_speed = params->speed.advertised_speeds;
-	p_bulletin->req_forced_speed = params->speed.forced_speed;
-	p_bulletin->req_autoneg_pause = params->pause.autoneg;
-	p_bulletin->req_forced_rx = params->pause.forced_rx;
-	p_bulletin->req_forced_tx = params->pause.forced_tx;
-	p_bulletin->req_loopback = params->loopback_mode;
-
-	p_bulletin->link_up = link->link_up;
-	p_bulletin->speed = link->speed;
-	p_bulletin->full_duplex = link->full_duplex;
-	p_bulletin->autoneg = link->an;
-	p_bulletin->autoneg_complete = link->an_complete;
-	p_bulletin->parallel_detection = link->parallel_detection;
-	p_bulletin->pfc_enabled = link->pfc_enabled;
-	p_bulletin->partner_adv_speed = link->partner_adv_speed;
-	p_bulletin->partner_tx_flow_ctrl_en = link->partner_tx_flow_ctrl_en;
-	p_bulletin->partner_rx_flow_ctrl_en = link->partner_rx_flow_ctrl_en;
-	p_bulletin->partner_adv_pause = link->partner_adv_pause;
-	p_bulletin->sfp_tx_fault = link->sfp_tx_fault;
-
-	p_bulletin->capability_speed = p_caps->speed_capabilities;
-}
-
 static int qed_iov_release_hw_for_vf(struct qed_hwfn *p_hwfn,
 				     struct qed_ptt *p_ptt, u16 rel_vf_id)
 {
-- 
1.9.3

[PATCH net 08/10] qede: Prevent index problems in loopback test

[Yuval Mintz]

From: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>

Driver currently utilizes the same loop variable in two
nested loops.

Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 1c48f44..6860f02 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -1296,7 +1296,7 @@ static int qede_selftest_receive_traffic(struct qede_dev *edev)
 	struct qede_rx_queue *rxq = NULL;
 	struct sw_rx_data *sw_rx_data;
 	union eth_rx_cqe *cqe;
-	int i, rc = 0;
+	int i, iter, rc = 0;
 	u8 *data_ptr;
 
 	for_each_queue(i) {
@@ -1315,7 +1315,7 @@ static int qede_selftest_receive_traffic(struct qede_dev *edev)
 	 * enabled. This is because the queue 0 is configured as the default
 	 * queue and that the loopback traffic is not IP.
 	 */
-	for (i = 0; i < QEDE_SELFTEST_POLL_COUNT; i++) {
+	for (iter = 0; iter < QEDE_SELFTEST_POLL_COUNT; iter++) {
 		if (!qede_has_rx_work(rxq)) {
 			usleep_range(100, 200);
 			continue;
@@ -1362,7 +1362,7 @@ static int qede_selftest_receive_traffic(struct qede_dev *edev)
 		qed_chain_recycle_consumed(&rxq->rx_comp_ring);
 	}
 
-	if (i == QEDE_SELFTEST_POLL_COUNT) {
+	if (iter == QEDE_SELFTEST_POLL_COUNT) {
 		DP_NOTICE(edev, "Failed to receive the traffic\n");
 		return -1;
 	}
-- 
1.9.3

[PATCH net 09/10] qed: Don't allocate SBs using main PTT

[Yuval Mintz]

Flows accessing registers require the flow to hold a PTT entry.
To protect 'major' load/unload flows a main_ptt is pre-allocated
to guarantee such flows wouldn't be blocked by PTT being
unavailable.

Status block initialization currently uses the main_ptt which
is incorrect, as this flow might run concurrently to others
[E.g., loading qedr while toggling qede]. That would have dire
effects as it means registers' access to device breaks and further
read/writes might access incorrect addresses.

Instead, when initializing status blocks acquire/release a PTT
as part of the flow.

Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_main.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index aeb98d8..b43b0e3 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1020,6 +1020,7 @@ static u32 qed_sb_init(struct qed_dev *cdev,
 		       enum qed_sb_type type)
 {
 	struct qed_hwfn *p_hwfn;
+	struct qed_ptt *p_ptt;
 	int hwfn_index;
 	u16 rel_sb_id;
 	u8 n_hwfns;
@@ -1041,8 +1042,18 @@ static u32 qed_sb_init(struct qed_dev *cdev,
 		   "hwfn [%d] <--[init]-- SB %04x [0x%04x upper]\n",
 		   hwfn_index, rel_sb_id, sb_id);
 
-	rc = qed_int_sb_init(p_hwfn, p_hwfn->p_main_ptt, sb_info,
-			     sb_virt_addr, sb_phy_addr, rel_sb_id);
+	if (IS_PF(p_hwfn->cdev)) {
+		p_ptt = qed_ptt_acquire(p_hwfn);
+		if (!p_ptt)
+			return -EBUSY;
+
+		rc = qed_int_sb_init(p_hwfn, p_ptt, sb_info, sb_virt_addr,
+				     sb_phy_addr, rel_sb_id);
+		qed_ptt_release(p_hwfn, p_ptt);
+	} else {
+		rc = qed_int_sb_init(p_hwfn, NULL, sb_info, sb_virt_addr,
+				     sb_phy_addr, rel_sb_id);
+	}
 
 	return rc;
 }
-- 
1.9.3

[PATCH net 10/10] qed*: Fix link indication race

[Yuval Mintz]

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>
---
 drivers/net/ethernet/qlogic/qed/qed_main.c   | 12 +++++++++---
 drivers/net/ethernet/qlogic/qed/qed_mcp.c    | 18 ++++++++++++++----
 drivers/net/ethernet/qlogic/qed/qed_mcp.h    |  6 ++++++
 drivers/net/ethernet/qlogic/qed/qed_sriov.h  |  1 +
 drivers/net/ethernet/qlogic/qed/qed_vf.c     |  3 +++
 drivers/net/ethernet/qlogic/qede/qede_main.c |  5 -----
 6 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index b43b0e3..756209e 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1094,12 +1094,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 6dd3ce4..5d8d0c2 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -168,6 +168,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;
 
@@ -586,6 +587,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) {
@@ -600,7 +604,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)
@@ -707,6 +711,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)
@@ -756,9 +762,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 407a2c1..b7dc40c 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -460,7 +460,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 509c02b..e0c04e9 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.h
@@ -221,6 +221,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 60b31a8..40f2752 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_vf.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_vf.c
@@ -1253,6 +1253,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 9c9f50c..d36f570 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -3940,7 +3940,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");
@@ -3992,11 +3991,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;
 
-- 
1.9.3

Re: [PATCH net 00/10] qed*: Bug fixes

[David Miller]

From: Yuval Mintz <Yuval.Mintz@cavium.com>
Date: Sun, 19 Feb 2017 15:26:13 +0200

> Hi Dave,
> 
> The 3 first patches here are a repost of
> ("qed: RoCE infrastructure fixes"). The fourth is an additional
> RoCE-related infrastructure fix, and the latter contain various fixes
> to qed/qede.
> 
> Notice patch #10 will create a conflict when net and net-next would
> be merged [although its fix is quite trivial].
> 
> Please consider applying these to `net'.

Linus made his release so all patches are going into net-next, which
this series doesn't apply cleanly to.  Please therefore respin your
series against net-next, thanks.

RE: [PATCH net 00/10] qed*: Bug fixes

[Mintz, Yuval]

 > > Hi Dave,
> >
> > The 3 first patches here are a repost of
> > ("qed: RoCE infrastructure fixes"). The fourth is an additional
> > RoCE-related infrastructure fix, and the latter contain various fixes
> > to qed/qede.
> >
> > Notice patch #10 will create a conflict when net and net-next would be
> > merged [although its fix is quite trivial].
> >
> > Please consider applying these to `net'.
> 
> Linus made his release so all patches are going into net-next, which this
> series doesn't apply cleanly to.  Please therefore respin your series against
> net-next, thanks.

Just to clarify - do you want me to re-spin the fixes against next-next *now*,
or do you want me to wait until it re-opens? [not sure on its exact state]

Re: [PATCH net 00/10] qed*: Bug fixes

[David Miller]

From: "Mintz, Yuval" <Yuval.Mintz@cavium.com>
Date: Mon, 20 Feb 2017 20:14:17 +0000

>  > > Hi Dave,
>> >
>> > The 3 first patches here are a repost of
>> > ("qed: RoCE infrastructure fixes"). The fourth is an additional
>> > RoCE-related infrastructure fix, and the latter contain various fixes
>> > to qed/qede.
>> >
>> > Notice patch #10 will create a conflict when net and net-next would be
>> > merged [although its fix is quite trivial].
>> >
>> > Please consider applying these to `net'.
>> 
>> Linus made his release so all patches are going into net-next, which this
>> series doesn't apply cleanly to.  Please therefore respin your series against
>> net-next, thanks.
> 
> Just to clarify - do you want me to re-spin the fixes against next-next *now*,
> or do you want me to wait until it re-opens? [not sure on its exact state]

Yes, now.