[PATCH for-next 00/20] IB/hns: Fixes for various misc. bugs

[PATCH for-next 00/20] IB/hns: Fixes for various misc. bugs

[Salil Mehta]

This patch-set introduces fixes for various bugs and potential
problems found during internal review and testing phases of
the HNS RoCE driver.

NOTE: These fixes have been done over already floated CM
patch: https://lkml.org/lkml/2016/9/1/609

Fixes have been done & tested  by Oulijun and Huwei.

Lijun Ou (17):
  IB/hns: Remove unused parameters in some functions
  IB/hns: Remove parameters of resize cq
  IB/hns: Add the check for pointer ctrl
  IB/hns: Fix bug of using uninit refcount and free
  IB/hns: Simplify function of pd alloc and qp alloc
  IB/hns: Remove unused parameter named qp_type
  IB/hns: Modify the init of iboe lock
  IB/hns: Fix bug of memory leakage for registering user mr
  IB/hns: Return bad wr while post send failed
  IB/hns: Add check for some NULL pointer scenes
  IB/hns: The Ack timeout need a lower limit value
  IB/hns: Some items of qpc need to take user param
  IB/hns: Validate mtu when modified qp
  IB/hns: Cq has not been freed
  IB/hns: Update the rq head when modify qp state
  IB/hns: Fix the bug when platform_get_resource() exec fail
  IB/hns: Delete the redundant lines in hns_roce_v1_m_qp()

Wei Hu (Xavier) (3):
  IB/hns: Fix bug of clear hem
  IB/hns: Delete the sqp_start from the structure hns_roce_caps
  IB/hns: Optimize code of aeq and ceq interrupt handle and fix the bug
    of qpn

 drivers/infiniband/hw/hns/hns_roce_cq.c     |   23 ++-
 drivers/infiniband/hw/hns/hns_roce_device.h |   13 +-
 drivers/infiniband/hw/hns/hns_roce_eq.c     |  146 +++++++++---------
 drivers/infiniband/hw/hns/hns_roce_eq.h     |    4 +
 drivers/infiniband/hw/hns/hns_roce_hem.c    |   76 +---------
 drivers/infiniband/hw/hns/hns_roce_hem.h    |    4 +
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c  |  217 +++++++++++++++++++++++----
 drivers/infiniband/hw/hns/hns_roce_hw_v1.h  |    9 ++
 drivers/infiniband/hw/hns/hns_roce_main.c   |    4 +-
 drivers/infiniband/hw/hns/hns_roce_mr.c     |    3 +
 drivers/infiniband/hw/hns/hns_roce_pd.c     |   18 +--
 drivers/infiniband/hw/hns/hns_roce_qp.c     |   39 ++---
 12 files changed, 324 insertions(+), 232 deletions(-)

-- 
1.7.9.5

[PATCH for-next 01/20] IB/hns: Remove unused parameters in some functions

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

The parameter named collapsed unused in hns_roce_cq_alloc.
Also, parameter named doorbell_lock unsed in
hns_roce_v1_cq_set_ci. This patch optimize these parameters.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_cq.c     |    7 +++----
 drivers/infiniband/hw/hns/hns_roce_device.h |    1 -
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c  |   13 ++++---------
 drivers/infiniband/hw/hns/hns_roce_main.c   |    1 -
 4 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_cq.c b/drivers/infiniband/hw/hns/hns_roce_cq.c
index 875597b..10fd209 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cq.c
@@ -83,8 +83,7 @@ static int hns_roce_sw2hw_cq(struct hns_roce_dev *dev,
 static int hns_roce_cq_alloc(struct hns_roce_dev *hr_dev, int nent,
 			     struct hns_roce_mtt *hr_mtt,
 			     struct hns_roce_uar *hr_uar,
-			     struct hns_roce_cq *hr_cq, int vector,
-			     int collapsed)
+			     struct hns_roce_cq *hr_cq, int vector)
 {
 	struct hns_roce_cmd_mailbox *mailbox = NULL;
 	struct hns_roce_cq_table *cq_table = NULL;
@@ -338,8 +337,8 @@ struct ib_cq *hns_roce_ib_create_cq(struct ib_device *ib_dev,
 	}
 
 	/* Allocate cq index, fill cq_context */
-	ret = hns_roce_cq_alloc(hr_dev, cq_entries, &hr_cq->hr_buf.hr_mtt,
-				uar, hr_cq, vector, 0);
+	ret = hns_roce_cq_alloc(hr_dev, cq_entries, &hr_cq->hr_buf.hr_mtt, uar,
+				hr_cq, vector);
 	if (ret) {
 		dev_err(dev, "Creat CQ .Failed to cq_alloc.\n");
 		goto err_mtt;
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 066bb3e..ddfef04 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -535,7 +535,6 @@ struct hns_roce_dev {
 	struct hns_roce_uar     priv_uar;
 	const char		*irq_names[HNS_ROCE_MAX_IRQ_NUM];
 	spinlock_t		sm_lock;
-	spinlock_t		cq_db_lock;
 	spinlock_t		bt_cmd_lock;
 	struct hns_roce_ib_iboe iboe;
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 3feac8a..5f76fc1 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -1189,9 +1189,7 @@ static struct hns_roce_cqe *next_cqe_sw(struct hns_roce_cq *hr_cq)
 	return get_sw_cqe(hr_cq, hr_cq->cons_index);
 }
 
-void hns_roce_v1_cq_set_ci(struct hns_roce_cq *hr_cq, u32 cons_index,
-			   spinlock_t *doorbell_lock)
-
+void hns_roce_v1_cq_set_ci(struct hns_roce_cq *hr_cq, u32 cons_index)
 {
 	u32 doorbell[2];
 
@@ -1251,8 +1249,7 @@ static void __hns_roce_v1_cq_clean(struct hns_roce_cq *hr_cq, u32 qpn,
 		*/
 		wmb();
 
-		hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index,
-				   &to_hr_dev(hr_cq->ib_cq.device)->cq_db_lock);
+		hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index);
 	}
 }
 
@@ -1588,10 +1585,8 @@ int hns_roce_v1_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
 			break;
 	}
 
-	if (npolled) {
-		hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index,
-				      &to_hr_dev(ibcq->device)->cq_db_lock);
-	}
+	if (npolled)
+		hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index);
 
 	spin_unlock_irqrestore(&hr_cq->lock, flags);
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index b54074d..2704076 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -964,7 +964,6 @@ static int hns_roce_setup_hca(struct hns_roce_dev *hr_dev)
 	struct device *dev = &hr_dev->pdev->dev;
 
 	spin_lock_init(&hr_dev->sm_lock);
-	spin_lock_init(&hr_dev->cq_db_lock);
 	spin_lock_init(&hr_dev->bt_cmd_lock);
 
 	ret = hns_roce_init_uar_table(hr_dev);
-- 
1.7.9.5

[PATCH for-next 02/20] IB/hns: Remove parameters of resize cq

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

In old version of RoCE, it doesn't support to resize cq.
So, we remove parameters related to resize cq.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_cq.c     |    3 ---
 drivers/infiniband/hw/hns/hns_roce_device.h |    9 ---------
 2 files changed, 12 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_cq.c b/drivers/infiniband/hw/hns/hns_roce_cq.c
index 10fd209..a52306f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cq.c
@@ -299,10 +299,7 @@ struct ib_cq *hns_roce_ib_create_cq(struct ib_device *ib_dev,
 
 	cq_entries = roundup_pow_of_two((unsigned int)cq_entries);
 	hr_cq->ib_cq.cqe = cq_entries - 1;
-	mutex_init(&hr_cq->resize_mutex);
 	spin_lock_init(&hr_cq->lock);
-	hr_cq->hr_resize_buf = NULL;
-	hr_cq->resize_umem = NULL;
 
 	if (context) {
 		if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd))) {
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index ddfef04..1c32e9a 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -283,20 +283,11 @@ struct hns_roce_cq_buf {
 	struct hns_roce_mtt hr_mtt;
 };
 
-struct hns_roce_cq_resize {
-	struct hns_roce_cq_buf	hr_buf;
-	int			cqe;
-};
-
 struct hns_roce_cq {
 	struct ib_cq			ib_cq;
 	struct hns_roce_cq_buf		hr_buf;
-	/* pointer to store information after resize*/
-	struct hns_roce_cq_resize	*hr_resize_buf;
 	spinlock_t			lock;
-	struct mutex			resize_mutex;
 	struct ib_umem			*umem;
-	struct ib_umem			*resize_umem;
 	void (*comp)(struct hns_roce_cq *);
 	void (*event)(struct hns_roce_cq *, enum hns_roce_event);
 
-- 
1.7.9.5

[PATCH for-next 03/20] IB/hns: Add the check for pointer ctrl

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

The pointer ctrl may be null, so we add check for it.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 5f76fc1..ea47ec4 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -336,6 +336,12 @@ int hns_roce_v1_post_recv(struct ib_qp *ibqp, struct ib_recv_wr *wr,
 		}
 
 		ctrl = get_recv_wqe(hr_qp, ind);
+		if (unlikely(!ctrl)) {
+			dev_err(dev, "Get recv wqe failed!\n");
+			ret = -EINVAL;
+			*bad_wr = wr;
+			goto out;
+		}
 
 		roce_set_field(ctrl->rwqe_byte_12,
 			       RQ_WQE_CTRL_RWQE_BYTE_12_RWQE_SGE_NUM_M,
-- 
1.7.9.5

Re: [PATCH for-next 03/20] IB/hns: Add the check for pointer ctrl

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]

On Fri, Sep 09, 2016 at 06:30:34PM +0800, Salil Mehta wrote:
> From: Lijun Ou <oulijun@huawei.com>
>
> The pointer ctrl may be null, so we add check for it.
>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 5f76fc1..ea47ec4 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -336,6 +336,12 @@ int hns_roce_v1_post_recv(struct ib_qp *ibqp, struct ib_recv_wr *wr,
>  		}
>
>  		ctrl = get_recv_wqe(hr_qp, ind);
> +		if (unlikely(!ctrl)) {

Yes, as you wrote, this check and patch are redundant too.

> +			dev_err(dev, "Get recv wqe failed!\n");
> +			ret = -EINVAL;
> +			*bad_wr = wr;
> +			goto out;
> +		}
>
>  		roce_set_field(ctrl->rwqe_byte_12,
>  			       RQ_WQE_CTRL_RWQE_BYTE_12_RWQE_SGE_NUM_M,
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 03/20] IB/hns: Add the check for pointer ctrl

[Salil Mehta]

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> Sent: Thursday, September 15, 2016 11:08 AM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH for-next 03/20] IB/hns: Add the check for pointer
> ctrl
> 
> On Fri, Sep 09, 2016 at 06:30:34PM +0800, Salil Mehta wrote:
> > From: Lijun Ou <oulijun@huawei.com>
> >
> > The pointer ctrl may be null, so we add check for it.
> >
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> >  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > index 5f76fc1..ea47ec4 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > @@ -336,6 +336,12 @@ int hns_roce_v1_post_recv(struct ib_qp *ibqp,
> struct ib_recv_wr *wr,
> >  		}
> >
> >  		ctrl = get_recv_wqe(hr_qp, ind);
> > +		if (unlikely(!ctrl)) {
> 
> Yes, as you wrote, this check and patch are redundant too.
I have dropped this patch in reworked V2 patch and introduced new patch
to remove the redundant code as well in get_recv_wqe() and get_send_wqe().

Thanks
Salil
> 
> > +			dev_err(dev, "Get recv wqe failed!\n");
> > +			ret = -EINVAL;
> > +			*bad_wr = wr;
> > +			goto out;
> > +		}
> >
> >  		roce_set_field(ctrl->rwqe_byte_12,
> >  			       RQ_WQE_CTRL_RWQE_BYTE_12_RWQE_SGE_NUM_M,
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 04/20] IB/hns: Fix bug of using uninit refcount and free

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

In current version, it uses uninitialized parameters named
refcount and free in hns_roce_cq_event.
This patch initializes these parameter in cq alloc and add
correspond process in cq free.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_cq.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_cq.c b/drivers/infiniband/hw/hns/hns_roce_cq.c
index a52306f..3095f06 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cq.c
@@ -152,6 +152,9 @@ static int hns_roce_cq_alloc(struct hns_roce_dev *hr_dev, int nent,
 	hr_cq->cons_index = 0;
 	hr_cq->uar = hr_uar;
 
+	atomic_set(&hr_cq->refcount, 1);
+	init_completion(&hr_cq->free);
+
 	return 0;
 
 err_radix:
@@ -191,6 +194,11 @@ static void hns_roce_free_cq(struct hns_roce_dev *hr_dev,
 	/* Waiting interrupt process procedure carried out */
 	synchronize_irq(hr_dev->eq_table.eq[hr_cq->vector].irq);
 
+	/* wait for all interrupt processed */
+	if (atomic_dec_and_test(&hr_cq->refcount))
+		complete(&hr_cq->free);
+	wait_for_completion(&hr_cq->free);
+
 	spin_lock_irq(&cq_table->lock);
 	radix_tree_delete(&cq_table->tree, hr_cq->cqn);
 	spin_unlock_irq(&cq_table->lock);
-- 
1.7.9.5

[PATCH for-next 05/20] IB/hns: Simplify function of pd alloc and qp alloc

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

Hns_roce_pd_alloc and hns_roce_reserve_range_qp use unnecessary
transformation of parameters. This patch simplify these two
functions.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_pd.c |   14 +-------------
 drivers/infiniband/hw/hns/hns_roce_qp.c |   10 +---------
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_pd.c b/drivers/infiniband/hw/hns/hns_roce_pd.c
index 4109f74..0cd6132 100644
--- a/drivers/infiniband/hw/hns/hns_roce_pd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_pd.c
@@ -35,19 +35,7 @@
 
 static int hns_roce_pd_alloc(struct hns_roce_dev *hr_dev, unsigned long *pdn)
 {
-	struct device *dev = &hr_dev->pdev->dev;
-	unsigned long pd_number;
-	int ret = 0;
-
-	ret = hns_roce_bitmap_alloc(&hr_dev->pd_bitmap, &pd_number);
-	if (ret == -1) {
-		dev_err(dev, "alloc pdn from pdbitmap failed\n");
-		return -ENOMEM;
-	}
-
-	*pdn = pd_number;
-
-	return 0;
+	return hns_roce_bitmap_alloc(&hr_dev->pd_bitmap, pdn);
 }
 
 static void hns_roce_pd_free(struct hns_roce_dev *hr_dev, unsigned long pdn)
diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
index 089da7f..6095cde 100644
--- a/drivers/infiniband/hw/hns/hns_roce_qp.c
+++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
@@ -113,16 +113,8 @@ static int hns_roce_reserve_range_qp(struct hns_roce_dev *hr_dev, int cnt,
 				     int align, unsigned long *base)
 {
 	struct hns_roce_qp_table *qp_table = &hr_dev->qp_table;
-	int ret = 0;
-	unsigned long qpn;
-
-	ret = hns_roce_bitmap_alloc_range(&qp_table->bitmap, cnt, align, &qpn);
-	if (ret == -1)
-		return -ENOMEM;
 
-	*base = qpn;
-
-	return 0;
+	return hns_roce_bitmap_alloc_range(&qp_table->bitmap, cnt, align, base);
 }
 
 enum hns_roce_qp_state to_hns_roce_state(enum ib_qp_state state)
-- 
1.7.9.5

[PATCH for-next 06/20] IB/hns: Remove unused parameter named qp_type

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

This patch removes the qp_type parameter in
hns_roce_set_kernel_sq_size().

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Ping Zhang <zhangping5@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_qp.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
index 6095cde..755a4b8 100644
--- a/drivers/infiniband/hw/hns/hns_roce_qp.c
+++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
@@ -337,12 +337,10 @@ static int hns_roce_set_user_sq_size(struct hns_roce_dev *hr_dev,
 
 static int hns_roce_set_kernel_sq_size(struct hns_roce_dev *hr_dev,
 				       struct ib_qp_cap *cap,
-				       enum ib_qp_type type,
 				       struct hns_roce_qp *hr_qp)
 {
 	struct device *dev = &hr_dev->pdev->dev;
 	u32 max_cnt;
-	(void)type;
 
 	if (cap->max_send_wr  > hr_dev->caps.max_wqes  ||
 	    cap->max_send_sge > hr_dev->caps.max_sq_sg ||
@@ -468,7 +466,7 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev,
 
 		/* Set SQ size */
 		ret = hns_roce_set_kernel_sq_size(hr_dev, &init_attr->cap,
-						  init_attr->qp_type, hr_qp);
+						  hr_qp);
 		if (ret) {
 			dev_err(dev, "hns_roce_set_kernel_sq_size error!\n");
 			goto err_out;
-- 
1.7.9.5

[PATCH for-next 07/20] IB/hns: Fix bug of clear hem

[Salil Mehta]

From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>

In hip06, there's no interface to release hem memory. So, hardware can't
identify whether hem memory released or not.
If all context in a hem memory released, the related hem memory will be
released by driver and reused by others. But, hardware don't know that
this memory can't be used already.

In order to fix this bug, hns roce driver reserved 128K memory for each
type of hem(QPC/CQC/MTPT). While unmap hem memory, hns roce driver will
write base address of reserved memory according to hem type.

Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_device.h |    2 +
 drivers/infiniband/hw/hns/hns_roce_hem.c    |   76 +--------------
 drivers/infiniband/hw/hns/hns_roce_hem.h    |    4 +
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c  |  140 +++++++++++++++++++++++++++
 drivers/infiniband/hw/hns/hns_roce_hw_v1.h  |    9 ++
 5 files changed, 157 insertions(+), 74 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 1c32e9a..f01cc2f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -505,6 +505,8 @@ struct hns_roce_hw {
 	void (*write_cqc)(struct hns_roce_dev *hr_dev,
 			  struct hns_roce_cq *hr_cq, void *mb_buf, u64 *mtts,
 			  dma_addr_t dma_handle, int nent, u32 vector);
+	int (*clear_hem)(struct hns_roce_dev *hr_dev,
+			 struct hns_roce_hem_table *table, int obj);
 	int (*query_qp)(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 			int qp_attr_mask, struct ib_qp_init_attr *qp_init_attr);
 	int (*modify_qp)(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c b/drivers/infiniband/hw/hns/hns_roce_hem.c
index d53d643..250d8f2 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
@@ -36,14 +36,10 @@
 #include "hns_roce_hem.h"
 #include "hns_roce_common.h"
 
-#define HW_SYNC_TIMEOUT_MSECS		500
-#define HW_SYNC_SLEEP_TIME_INTERVAL	20
-
 #define HNS_ROCE_HEM_ALLOC_SIZE		(1 << 17)
 #define HNS_ROCE_TABLE_CHUNK_SIZE	(1 << 17)
 
 #define DMA_ADDR_T_SHIFT		12
-#define BT_CMD_SYNC_SHIFT		31
 #define BT_BA_SHIFT			32
 
 struct hns_roce_hem *hns_roce_alloc_hem(struct hns_roce_dev *hr_dev, int npages,
@@ -213,74 +209,6 @@ static int hns_roce_set_hem(struct hns_roce_dev *hr_dev,
 	return ret;
 }
 
-static int hns_roce_clear_hem(struct hns_roce_dev *hr_dev,
-			      struct hns_roce_hem_table *table,
-			      unsigned long obj)
-{
-	struct device *dev = &hr_dev->pdev->dev;
-	unsigned long end = 0;
-	unsigned long flags;
-	void __iomem *bt_cmd;
-	uint32_t bt_cmd_val[2];
-	u32 bt_cmd_h_val = 0;
-	int ret = 0;
-
-	switch (table->type) {
-	case HEM_TYPE_QPC:
-		roce_set_field(bt_cmd_h_val, ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_M,
-			       ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_S, HEM_TYPE_QPC);
-		break;
-	case HEM_TYPE_MTPT:
-		roce_set_field(bt_cmd_h_val, ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_M,
-			       ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_S,
-			       HEM_TYPE_MTPT);
-		break;
-	case HEM_TYPE_CQC:
-		roce_set_field(bt_cmd_h_val, ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_M,
-			       ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_S, HEM_TYPE_CQC);
-		break;
-	case HEM_TYPE_SRQC:
-		roce_set_field(bt_cmd_h_val, ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_M,
-			       ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_S,
-			       HEM_TYPE_SRQC);
-		break;
-	default:
-		return ret;
-	}
-	roce_set_field(bt_cmd_h_val, ROCEE_BT_CMD_H_ROCEE_BT_CMD_IN_MDF_M,
-		       ROCEE_BT_CMD_H_ROCEE_BT_CMD_IN_MDF_S, obj);
-	roce_set_bit(bt_cmd_h_val, ROCEE_BT_CMD_H_ROCEE_BT_CMD_S, 0);
-	roce_set_bit(bt_cmd_h_val, ROCEE_BT_CMD_H_ROCEE_BT_CMD_HW_SYNS_S, 1);
-	roce_set_field(bt_cmd_h_val, ROCEE_BT_CMD_H_ROCEE_BT_CMD_BA_H_M,
-		       ROCEE_BT_CMD_H_ROCEE_BT_CMD_BA_H_S, 0);
-
-	spin_lock_irqsave(&hr_dev->bt_cmd_lock, flags);
-
-	bt_cmd = hr_dev->reg_base + ROCEE_BT_CMD_H_REG;
-
-	end = msecs_to_jiffies(HW_SYNC_TIMEOUT_MSECS) + jiffies;
-	while (1) {
-		if (readl(bt_cmd) >> BT_CMD_SYNC_SHIFT) {
-			if (!(time_before(jiffies, end))) {
-				dev_err(dev, "Write bt_cmd err,hw_sync is not zero.\n");
-				spin_unlock_irqrestore(&hr_dev->bt_cmd_lock,
-						       flags);
-				return -EBUSY;
-			}
-		} else {
-			break;
-		}
-		msleep(HW_SYNC_SLEEP_TIME_INTERVAL);
-	}
-
-	bt_cmd_val[0] = 0;
-	bt_cmd_val[1] = bt_cmd_h_val;
-	hns_roce_write64_k(bt_cmd_val, hr_dev->reg_base + ROCEE_BT_CMD_L_REG);
-	spin_unlock_irqrestore(&hr_dev->bt_cmd_lock, flags);
-
-	return ret;
-}
-
 int hns_roce_table_get(struct hns_roce_dev *hr_dev,
 		       struct hns_roce_hem_table *table, unsigned long obj)
 {
@@ -333,7 +261,7 @@ void hns_roce_table_put(struct hns_roce_dev *hr_dev,
 
 	if (--table->hem[i]->refcount == 0) {
 		/* Clear HEM base address */
-		if (hns_roce_clear_hem(hr_dev, table, obj))
+		if (hr_dev->hw->clear_hem(hr_dev, table, obj))
 			dev_warn(dev, "Clear HEM base address failed.\n");
 
 		hns_roce_free_hem(hr_dev, table->hem[i]);
@@ -456,7 +384,7 @@ void hns_roce_cleanup_hem_table(struct hns_roce_dev *hr_dev,
 
 	for (i = 0; i < table->num_hem; ++i)
 		if (table->hem[i]) {
-			if (hns_roce_clear_hem(hr_dev, table,
+			if (hr_dev->hw->clear_hem(hr_dev, table,
 			    i * HNS_ROCE_TABLE_CHUNK_SIZE / table->obj_size))
 				dev_err(dev, "Clear HEM base address failed.\n");
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.h b/drivers/infiniband/hw/hns/hns_roce_hem.h
index ad66175..4357488 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.h
@@ -34,6 +34,10 @@
 #ifndef _HNS_ROCE_HEM_H
 #define _HNS_ROCE_HEM_H
 
+#define HW_SYNC_TIMEOUT_MSECS		500
+#define HW_SYNC_SLEEP_TIME_INTERVAL	20
+#define BT_CMD_SYNC_SHIFT		31
+
 enum {
 	/* MAP HEM(Hardware Entry Memory) */
 	HEM_TYPE_QPC = 0,
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index ea47ec4..f6bb88c 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -792,6 +792,66 @@ static void hns_roce_port_enable(struct hns_roce_dev *hr_dev, int enable_flag)
 	}
 }
 
+static int hns_roce_bt_init(struct hns_roce_dev *hr_dev)
+{
+	struct device *dev = &hr_dev->pdev->dev;
+	struct hns_roce_v1_priv *priv;
+	int ret;
+
+	priv = (struct hns_roce_v1_priv *)hr_dev->hw->priv;
+
+	priv->bt_table.qpc_buf.buf = dma_alloc_coherent(dev,
+		HNS_ROCE_BT_RSV_BUF_SIZE, &priv->bt_table.qpc_buf.map,
+		GFP_KERNEL);
+	if (!priv->bt_table.qpc_buf.buf)
+		return -ENOMEM;
+
+	priv->bt_table.mtpt_buf.buf = dma_alloc_coherent(dev,
+		HNS_ROCE_BT_RSV_BUF_SIZE, &priv->bt_table.mtpt_buf.map,
+		GFP_KERNEL);
+	if (!priv->bt_table.mtpt_buf.buf) {
+		ret = -ENOMEM;
+		goto err_failed_alloc_mtpt_buf;
+	}
+
+	priv->bt_table.cqc_buf.buf = dma_alloc_coherent(dev,
+		HNS_ROCE_BT_RSV_BUF_SIZE, &priv->bt_table.cqc_buf.map,
+		GFP_KERNEL);
+	if (!priv->bt_table.cqc_buf.buf) {
+		ret = -ENOMEM;
+		goto err_failed_alloc_cqc_buf;
+	}
+
+	return 0;
+
+err_failed_alloc_cqc_buf:
+	dma_free_coherent(dev, HNS_ROCE_BT_RSV_BUF_SIZE,
+		priv->bt_table.mtpt_buf.buf, priv->bt_table.mtpt_buf.map);
+
+err_failed_alloc_mtpt_buf:
+	dma_free_coherent(dev, HNS_ROCE_BT_RSV_BUF_SIZE,
+		priv->bt_table.qpc_buf.buf, priv->bt_table.qpc_buf.map);
+
+	return ret;
+}
+
+static void hns_roce_bt_free(struct hns_roce_dev *hr_dev)
+{
+	struct device *dev = &hr_dev->pdev->dev;
+	struct hns_roce_v1_priv *priv;
+
+	priv = (struct hns_roce_v1_priv *)hr_dev->hw->priv;
+
+	dma_free_coherent(dev, HNS_ROCE_BT_RSV_BUF_SIZE,
+		priv->bt_table.cqc_buf.buf, priv->bt_table.cqc_buf.map);
+
+	dma_free_coherent(dev, HNS_ROCE_BT_RSV_BUF_SIZE,
+		priv->bt_table.mtpt_buf.buf, priv->bt_table.mtpt_buf.map);
+
+	dma_free_coherent(dev, HNS_ROCE_BT_RSV_BUF_SIZE,
+		priv->bt_table.qpc_buf.buf, priv->bt_table.qpc_buf.map);
+}
+
 /**
  * hns_roce_v1_reset - reset RoCE
  * @hr_dev: RoCE device struct pointer
@@ -947,8 +1007,18 @@ int hns_roce_v1_init(struct hns_roce_dev *hr_dev)
 
 	hns_roce_port_enable(hr_dev, HNS_ROCE_PORT_UP);
 
+	ret = hns_roce_bt_init(hr_dev);
+	if (ret) {
+		dev_err(dev, "bt init failed!\n");
+		goto error_failed_bt_init;
+	}
+
 	return 0;
 
+error_failed_bt_init:
+	hns_roce_port_enable(hr_dev, HNS_ROCE_PORT_DOWN);
+	hns_roce_raq_free(hr_dev);
+
 error_failed_raq_init:
 	hns_roce_db_free(hr_dev);
 	return ret;
@@ -956,6 +1026,7 @@ error_failed_raq_init:
 
 void hns_roce_v1_exit(struct hns_roce_dev *hr_dev)
 {
+	hns_roce_bt_free(hr_dev);
 	hns_roce_port_enable(hr_dev, HNS_ROCE_PORT_DOWN);
 	hns_roce_raq_free(hr_dev);
 	hns_roce_db_free(hr_dev);
@@ -1602,6 +1673,74 @@ int hns_roce_v1_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
 		return ret;
 }
 
+int hns_roce_v1_clear_hem(struct hns_roce_dev *hr_dev,
+		struct hns_roce_hem_table *table, int obj)
+{
+	struct device *dev = &hr_dev->pdev->dev;
+	struct hns_roce_v1_priv *priv;
+	unsigned long end = 0, flags = 0;
+	uint32_t bt_cmd_val[2] = {0};
+	void __iomem *bt_cmd;
+	u64 bt_ba = 0;
+
+	priv = (struct hns_roce_v1_priv *)hr_dev->hw->priv;
+
+	switch (table->type) {
+	case HEM_TYPE_QPC:
+		roce_set_field(bt_cmd_val[1], ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_M,
+			ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_S, HEM_TYPE_QPC);
+		bt_ba = priv->bt_table.qpc_buf.map >> 12;
+		break;
+	case HEM_TYPE_MTPT:
+		roce_set_field(bt_cmd_val[1], ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_M,
+			ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_S, HEM_TYPE_MTPT);
+		bt_ba = priv->bt_table.mtpt_buf.map >> 12;
+		break;
+	case HEM_TYPE_CQC:
+		roce_set_field(bt_cmd_val[1], ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_M,
+			ROCEE_BT_CMD_H_ROCEE_BT_CMD_MDF_S, HEM_TYPE_CQC);
+		bt_ba = priv->bt_table.cqc_buf.map >> 12;
+		break;
+	case HEM_TYPE_SRQC:
+		dev_dbg(dev, "HEM_TYPE_SRQC not support.\n");
+		return -EINVAL;
+	default:
+		return 0;
+	}
+	roce_set_field(bt_cmd_val[1], ROCEE_BT_CMD_H_ROCEE_BT_CMD_IN_MDF_M,
+		ROCEE_BT_CMD_H_ROCEE_BT_CMD_IN_MDF_S, obj);
+	roce_set_bit(bt_cmd_val[1], ROCEE_BT_CMD_H_ROCEE_BT_CMD_S, 0);
+	roce_set_bit(bt_cmd_val[1], ROCEE_BT_CMD_H_ROCEE_BT_CMD_HW_SYNS_S, 1);
+
+	spin_lock_irqsave(&hr_dev->bt_cmd_lock, flags);
+
+	bt_cmd = hr_dev->reg_base + ROCEE_BT_CMD_H_REG;
+
+	end = msecs_to_jiffies(HW_SYNC_TIMEOUT_MSECS) + jiffies;
+	while (1) {
+		if (readl(bt_cmd) >> BT_CMD_SYNC_SHIFT) {
+			if (!(time_before(jiffies, end))) {
+				dev_err(dev, "Write bt_cmd err,hw_sync is not zero.\n");
+				spin_unlock_irqrestore(&hr_dev->bt_cmd_lock,
+					flags);
+				return -EBUSY;
+			}
+		} else {
+			break;
+		}
+		msleep(HW_SYNC_SLEEP_TIME_INTERVAL);
+	}
+
+	bt_cmd_val[0] = (uint32_t)bt_ba;
+	roce_set_field(bt_cmd_val[1], ROCEE_BT_CMD_H_ROCEE_BT_CMD_BA_H_M,
+		ROCEE_BT_CMD_H_ROCEE_BT_CMD_BA_H_S, bt_ba >> 32);
+	hns_roce_write64_k(bt_cmd_val, hr_dev->reg_base + ROCEE_BT_CMD_L_REG);
+
+	spin_unlock_irqrestore(&hr_dev->bt_cmd_lock, flags);
+
+	return 0;
+}
+
 static int hns_roce_v1_qp_modify(struct hns_roce_dev *hr_dev,
 				 struct hns_roce_mtt *mtt,
 				 enum hns_roce_qp_state cur_state,
@@ -2774,6 +2913,7 @@ struct hns_roce_hw hns_roce_hw_v1 = {
 	.set_mtu = hns_roce_v1_set_mtu,
 	.write_mtpt = hns_roce_v1_write_mtpt,
 	.write_cqc = hns_roce_v1_write_cqc,
+	.clear_hem = hns_roce_v1_clear_hem,
 	.modify_qp = hns_roce_v1_modify_qp,
 	.query_qp = hns_roce_v1_query_qp,
 	.destroy_qp = hns_roce_v1_destroy_qp,
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.h b/drivers/infiniband/hw/hns/hns_roce_hw_v1.h
index 316b592..539b0a3b 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.h
@@ -102,6 +102,8 @@
 #define HNS_ROCE_V1_EXT_ODB_ALFUL	\
 	(HNS_ROCE_V1_EXT_ODB_DEPTH - HNS_ROCE_V1_DB_RSVD)
 
+#define HNS_ROCE_BT_RSV_BUF_SIZE			(1 << 17)
+
 #define HNS_ROCE_ODB_POLL_MODE				0
 
 #define HNS_ROCE_SDB_NORMAL_MODE			0
@@ -971,9 +973,16 @@ struct hns_roce_db_table {
 	struct hns_roce_ext_db *ext_db;
 };
 
+struct hns_roce_bt_table {
+	struct hns_roce_buf_list qpc_buf;
+	struct hns_roce_buf_list mtpt_buf;
+	struct hns_roce_buf_list cqc_buf;
+};
+
 struct hns_roce_v1_priv {
 	struct hns_roce_db_table  db_table;
 	struct hns_roce_raq_table raq_table;
+	struct hns_roce_bt_table  bt_table;
 };
 
 int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool dereset);
-- 
1.7.9.5

[PATCH for-next 08/20] IB/hns: Delete the sqp_start from the structure hns_roce_caps

[Salil Mehta]

From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>

This patch deleted the sqp_start from the structure hns_roce_caps, and
modified the calculation of the qp number.

Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_device.h |    1 -
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c  |    1 -
 drivers/infiniband/hw/hns/hns_roce_qp.c     |   10 ++++------
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index f01cc2f..7643716 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -464,7 +464,6 @@ struct hns_roce_caps {
 	u32		max_rq_desc_sz;	/* 64 */
 	int		max_qp_init_rdma;
 	int		max_qp_dest_rdma;
-	int		sqp_start;
 	int		num_cqs;
 	int		max_cqes;
 	int		reserved_cqs;
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index f6bb88c..f4d2515 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -942,7 +942,6 @@ void hns_roce_v1_profile(struct hns_roce_dev *hr_dev)
 	caps->mtt_entry_sz	= HNS_ROCE_V1_MTT_ENTRY_SIZE;
 	caps->cq_entry_sz	= HNS_ROCE_V1_CQE_ENTRY_SIZE;
 	caps->page_size_cap	= HNS_ROCE_V1_PAGE_SIZE_SUPPORT;
-	caps->sqp_start		= 0;
 	caps->reserved_lkey	= 0;
 	caps->reserved_pds	= 0;
 	caps->reserved_mrws	= 1;
diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
index 755a4b8..51fefbf 100644
--- a/drivers/infiniband/hw/hns/hns_roce_qp.c
+++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
@@ -39,7 +39,7 @@
 #include "hns_roce_user.h"
 
 #define DB_REG_OFFSET			0x1000
-#define SQP_NUM				12
+#define SQP_NUM				(2 * HNS_ROCE_MAX_PORTS)
 
 void hns_roce_qp_event(struct hns_roce_dev *hr_dev, u32 qpn, int event_type)
 {
@@ -247,7 +247,7 @@ void hns_roce_release_range_qp(struct hns_roce_dev *hr_dev, int base_qpn,
 {
 	struct hns_roce_qp_table *qp_table = &hr_dev->qp_table;
 
-	if (base_qpn < (hr_dev->caps.sqp_start + 2 * hr_dev->caps.num_ports))
+	if (base_qpn < SQP_NUM)
 		return;
 
 	hns_roce_bitmap_free_range(&qp_table->bitmap, base_qpn, cnt);
@@ -609,8 +609,7 @@ struct ib_qp *hns_roce_create_qp(struct ib_pd *pd,
 		hr_qp = &hr_sqp->hr_qp;
 		hr_qp->port = init_attr->port_num - 1;
 		hr_qp->phy_port = hr_dev->iboe.phy_port[hr_qp->port];
-		hr_qp->ibqp.qp_num = hr_dev->caps.sqp_start +
-				     HNS_ROCE_MAX_PORTS +
+		hr_qp->ibqp.qp_num = HNS_ROCE_MAX_PORTS +
 				     hr_dev->iboe.phy_port[hr_qp->port];
 
 		ret = hns_roce_create_qp_common(hr_dev, pd, init_attr, udata,
@@ -826,8 +825,7 @@ int hns_roce_init_qp_table(struct hns_roce_dev *hr_dev)
 
 	/* A port include two SQP, six port total 12 */
 	ret = hns_roce_bitmap_init(&qp_table->bitmap, hr_dev->caps.num_qps,
-				   hr_dev->caps.num_qps - 1,
-				   hr_dev->caps.sqp_start + SQP_NUM,
+				   hr_dev->caps.num_qps - 1, SQP_NUM,
 				   reserved_from_top);
 	if (ret) {
 		dev_err(&hr_dev->pdev->dev, "qp bitmap init failed!error=%d\n",
-- 
1.7.9.5

[PATCH for-next 09/20] IB/hns: Optimize code of aeq and ceq interrupt handle and fix the bug of qpn

[Salil Mehta]

From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>

This patch optimized the codes of aeq and ceq interrupt handle
and fixed the bug in the calculation of qpn. For the special
qp(GSI or SMI), calculated the qp number according to physical
port and the qpn reported in the event of async event queue.

Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_eq.c |  146 +++++++++++++++----------------
 drivers/infiniband/hw/hns/hns_roce_eq.h |    4 +
 2 files changed, 75 insertions(+), 75 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_eq.c b/drivers/infiniband/hw/hns/hns_roce_eq.c
index 98af7fe..fcab5fb 100644
--- a/drivers/infiniband/hw/hns/hns_roce_eq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_eq.c
@@ -66,9 +66,6 @@ static void hns_roce_wq_catas_err_handle(struct hns_roce_dev *hr_dev,
 {
 	struct device *dev = &hr_dev->pdev->dev;
 
-	qpn = roce_get_field(aeqe->event.qp_event.qp,
-			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
-			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S);
 	dev_warn(dev, "Local Work Queue Catastrophic Error.\n");
 	switch (roce_get_field(aeqe->asyn, HNS_ROCE_AEQE_U32_4_EVENT_SUB_TYPE_M,
 			       HNS_ROCE_AEQE_U32_4_EVENT_SUB_TYPE_S)) {
@@ -96,13 +93,6 @@ static void hns_roce_wq_catas_err_handle(struct hns_roce_dev *hr_dev,
 	default:
 		break;
 	}
-
-	hns_roce_qp_event(hr_dev, roce_get_field(aeqe->event.qp_event.qp,
-					HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
-					HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S),
-			  roce_get_field(aeqe->asyn,
-					HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
-					HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S));
 }
 
 static void hns_roce_local_wq_access_err_handle(struct hns_roce_dev *hr_dev,
@@ -111,9 +101,6 @@ static void hns_roce_local_wq_access_err_handle(struct hns_roce_dev *hr_dev,
 {
 	struct device *dev = &hr_dev->pdev->dev;
 
-	qpn = roce_get_field(aeqe->event.qp_event.qp,
-			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
-			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S);
 	dev_warn(dev, "Local Access Violation Work Queue Error.\n");
 	switch (roce_get_field(aeqe->asyn, HNS_ROCE_AEQE_U32_4_EVENT_SUB_TYPE_M,
 			       HNS_ROCE_AEQE_U32_4_EVENT_SUB_TYPE_S)) {
@@ -141,13 +128,69 @@ static void hns_roce_local_wq_access_err_handle(struct hns_roce_dev *hr_dev,
 	default:
 		break;
 	}
+}
+
+static void hns_roce_qp_err_handle(struct hns_roce_dev *hr_dev,
+				   struct hns_roce_aeqe *aeqe,
+				   int event_type)
+{
+	struct device *dev = &hr_dev->pdev->dev;
+	int phy_port;
+	int qpn;
+
+	qpn = roce_get_field(aeqe->event.qp_event.qp,
+			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
+			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S);
+	phy_port = roce_get_field(aeqe->event.qp_event.qp,
+			HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_M,
+			HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_S);
+	if (qpn <= 1)
+		qpn = HNS_ROCE_MAX_PORTS * qpn + phy_port;
+
+	switch (event_type) {
+	case HNS_ROCE_EVENT_TYPE_INV_REQ_LOCAL_WQ_ERROR:
+		dev_warn(dev, "Invalid Req Local Work Queue Error.\n");
+		dev_warn(dev, "QP %d, phy_port %d.\n", qpn, phy_port);
+		break;
+	case HNS_ROCE_EVENT_TYPE_WQ_CATAS_ERROR:
+		hns_roce_wq_catas_err_handle(hr_dev, aeqe, qpn);
+		break;
+	case HNS_ROCE_EVENT_TYPE_LOCAL_WQ_ACCESS_ERROR:
+		hns_roce_local_wq_access_err_handle(hr_dev, aeqe, qpn);
+		break;
+	default:
+		break;
+	}
+
+	hns_roce_qp_event(hr_dev, qpn, event_type);
+}
+
+static void hns_roce_cq_err_handle(struct hns_roce_dev *hr_dev,
+				   struct hns_roce_aeqe *aeqe,
+				   int event_type)
+{
+	struct device *dev = &hr_dev->pdev->dev;
+	u32 cqn;
+
+	cqn = le32_to_cpu(roce_get_field(aeqe->event.cq_event.cq,
+		    HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_M,
+		    HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_S));
+
+	switch (event_type) {
+	case HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR:
+		dev_warn(dev, "CQ 0x%x access err.\n", cqn);
+		break;
+	case HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW:
+		dev_warn(dev, "CQ 0x%x overflow\n", cqn);
+		break;
+	case HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID:
+		dev_warn(dev, "CQ 0x%x ID invalid.\n", cqn);
+		break;
+	default:
+		break;
+	}
 
-	hns_roce_qp_event(hr_dev, roce_get_field(aeqe->event.qp_event.qp,
-					 HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
-					 HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S),
-			  roce_get_field(aeqe->asyn,
-					 HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
-					 HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S));
+	hns_roce_cq_event(hr_dev, cqn, event_type);
 }
 
 static void hns_roce_db_overflow_handle(struct hns_roce_dev *hr_dev,
@@ -185,7 +228,7 @@ static int hns_roce_aeq_int(struct hns_roce_dev *hr_dev, struct hns_roce_eq *eq)
 	struct device *dev = &hr_dev->pdev->dev;
 	struct hns_roce_aeqe *aeqe;
 	int aeqes_found = 0;
-	int qpn = 0;
+	int event_type;
 
 	while ((aeqe = next_aeqe_sw(eq))) {
 		dev_dbg(dev, "aeqe = %p, aeqe->asyn.event_type = 0x%lx\n", aeqe,
@@ -195,9 +238,10 @@ static int hns_roce_aeq_int(struct hns_roce_dev *hr_dev, struct hns_roce_eq *eq)
 		/* Memory barrier */
 		rmb();
 
-		switch (roce_get_field(aeqe->asyn,
-			HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
-			HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S)) {
+		event_type = roce_get_field(aeqe->asyn,
+				HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
+				HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S);
+		switch (event_type) {
 		case HNS_ROCE_EVENT_TYPE_PATH_MIG:
 			dev_warn(dev, "PATH MIG not supported\n");
 			break;
@@ -211,23 +255,9 @@ static int hns_roce_aeq_int(struct hns_roce_dev *hr_dev, struct hns_roce_eq *eq)
 			dev_warn(dev, "PATH MIG failed\n");
 			break;
 		case HNS_ROCE_EVENT_TYPE_INV_REQ_LOCAL_WQ_ERROR:
-			dev_warn(dev, "qpn = 0x%lx\n",
-			roce_get_field(aeqe->event.qp_event.qp,
-				       HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
-				       HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S));
-			hns_roce_qp_event(hr_dev,
-				roce_get_field(aeqe->event.qp_event.qp,
-					HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
-					HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S),
-				roce_get_field(aeqe->asyn,
-					HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
-					HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S));
-			break;
 		case HNS_ROCE_EVENT_TYPE_WQ_CATAS_ERROR:
-			hns_roce_wq_catas_err_handle(hr_dev, aeqe, qpn);
-			break;
 		case HNS_ROCE_EVENT_TYPE_LOCAL_WQ_ACCESS_ERROR:
-			hns_roce_local_wq_access_err_handle(hr_dev, aeqe, qpn);
+			hns_roce_qp_err_handle(hr_dev, aeqe, event_type);
 			break;
 		case HNS_ROCE_EVENT_TYPE_SRQ_LIMIT_REACH:
 		case HNS_ROCE_EVENT_TYPE_SRQ_CATAS_ERROR:
@@ -235,40 +265,9 @@ static int hns_roce_aeq_int(struct hns_roce_dev *hr_dev, struct hns_roce_eq *eq)
 			dev_warn(dev, "SRQ not support!\n");
 			break;
 		case HNS_ROCE_EVENT_TYPE_CQ_ACCESS_ERROR:
-			dev_warn(dev, "CQ 0x%lx access err.\n",
-			roce_get_field(aeqe->event.cq_event.cq,
-				       HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_M,
-				       HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_S));
-			hns_roce_cq_event(hr_dev,
-			le32_to_cpu(roce_get_field(aeqe->event.cq_event.cq,
-				    HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_M,
-				    HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_S)),
-			roce_get_field(aeqe->asyn,
-				       HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
-				       HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S));
-			break;
 		case HNS_ROCE_EVENT_TYPE_CQ_OVERFLOW:
-			dev_warn(dev, "CQ 0x%lx overflow\n",
-			roce_get_field(aeqe->event.cq_event.cq,
-				       HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_M,
-				       HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_S));
-			hns_roce_cq_event(hr_dev,
-			le32_to_cpu(roce_get_field(aeqe->event.cq_event.cq,
-				    HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_M,
-				    HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_S)),
-			roce_get_field(aeqe->asyn,
-				       HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
-				       HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S));
-			break;
 		case HNS_ROCE_EVENT_TYPE_CQ_ID_INVALID:
-			dev_warn(dev, "CQ ID invalid.\n");
-			hns_roce_cq_event(hr_dev,
-			le32_to_cpu(roce_get_field(aeqe->event.cq_event.cq,
-				    HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_M,
-				    HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_S)),
-			roce_get_field(aeqe->asyn,
-				       HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
-				       HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S));
+			hns_roce_cq_err_handle(hr_dev, aeqe, event_type);
 			break;
 		case HNS_ROCE_EVENT_TYPE_PORT_CHANGE:
 			dev_warn(dev, "port change.\n");
@@ -290,11 +289,8 @@ static int hns_roce_aeq_int(struct hns_roce_dev *hr_dev, struct hns_roce_eq *eq)
 				     HNS_ROCE_AEQE_EVENT_CE_EVENT_CEQE_CEQN_S));
 			break;
 		default:
-			dev_warn(dev, "Unhandled event 0x%lx on EQ %d at index %u\n",
-				 roce_get_field(aeqe->asyn,
-					      HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
-					      HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S),
-				 eq->eqn, eq->cons_index);
+			dev_warn(dev, "Unhandled event %d on EQ %d at index %u\n",
+				 event_type, eq->eqn, eq->cons_index);
 			break;
 		};
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_eq.h b/drivers/infiniband/hw/hns/hns_roce_eq.h
index fe43881..c6d212d 100644
--- a/drivers/infiniband/hw/hns/hns_roce_eq.h
+++ b/drivers/infiniband/hw/hns/hns_roce_eq.h
@@ -107,6 +107,10 @@ struct hns_roce_aeqe {
 #define HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M   \
 	(((1UL << 24) - 1) << HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S)
 
+#define HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_S 25
+#define HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_M   \
+	(((1UL << 3) - 1) << HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_S)
+
 #define HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_S 0
 #define HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_M   \
 	(((1UL << 16) - 1) << HNS_ROCE_AEQE_EVENT_CQ_EVENT_CQ_CQN_S)
-- 
1.7.9.5

Re: [PATCH for-next 09/20] IB/hns: Optimize code of aeq and ceq interrupt handle and fix the bug of qpn

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

On Fri, Sep 09, 2016 at 06:30:40PM +0800, Salil Mehta wrote:

....

> +			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
> +			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S);
> +	phy_port = roce_get_field(aeqe->event.qp_event.qp,
> +			HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_M,
> +			HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_S);
> +	if (qpn <= 1)
> +		qpn = HNS_ROCE_MAX_PORTS * qpn + phy_port;
> +
> +	switch (event_type) {
> +	case HNS_ROCE_EVENT_TYPE_INV_REQ_LOCAL_WQ_ERROR:
> +		dev_warn(dev, "Invalid Req Local Work Queue Error.\n");
> +		dev_warn(dev, "QP %d, phy_port %d.\n", qpn, phy_port);

dev_warn like all other printk flavours supports multi-line messages.

> +		break;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 09/20] IB/hns: Optimize code of aeq and ceq interrupt handle and fix the bug of qpn

[Salil Mehta]

> -----Original Message-----
> From: Leon Romanovsky [mailto:leon@kernel.org]
> Sent: Monday, September 12, 2016 1:33 PM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH for-next 09/20] IB/hns: Optimize code of aeq and
> ceq interrupt handle and fix the bug of qpn
> 
> On Fri, Sep 09, 2016 at 06:30:40PM +0800, Salil Mehta wrote:
> 
> ....
> 
> > +			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_M,
> > +			     HNS_ROCE_AEQE_EVENT_QP_EVENT_QP_QPN_S);
> > +	phy_port = roce_get_field(aeqe->event.qp_event.qp,
> > +			HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_M,
> > +			HNS_ROCE_AEQE_EVENT_QP_EVENT_PORT_NUM_S);
> > +	if (qpn <= 1)
> > +		qpn = HNS_ROCE_MAX_PORTS * qpn + phy_port;
> > +
> > +	switch (event_type) {
> > +	case HNS_ROCE_EVENT_TYPE_INV_REQ_LOCAL_WQ_ERROR:
> > +		dev_warn(dev, "Invalid Req Local Work Queue Error.\n");
> > +		dev_warn(dev, "QP %d, phy_port %d.\n", qpn, phy_port);
> 
> dev_warn like all other printk flavours supports multi-line messages.
Yes, agreed. This can be done using single dev_warn as well, like below:
dev_warn(dev,
         "Invalid Req Local Work Queue Error.\n"
         "QP %d, phy_port %d.\n", qpn, phy_port);

Will change and refloat the patch.

Thanks
Salil
> 
> > +		break;

[PATCH for-next 10/20] IB/hns: Modify the init of iboe lock

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

This lock will be used in query port interface, and will be called
while IB device was registered to OFED frame. So, the lock of iboe
must be initiated before IB device was registered.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_main.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index 2704076..4721c0c 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -615,6 +615,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
 	struct device *dev = &hr_dev->pdev->dev;
 
 	iboe = &hr_dev->iboe;
+	spin_lock_init(&iboe->lock);
 
 	ib_dev = &hr_dev->ib_dev;
 	strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
@@ -701,8 +702,6 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
 		goto error_failed_setup_mtu_gids;
 	}
 
-	spin_lock_init(&iboe->lock);
-
 	iboe->nb.notifier_call = hns_roce_netdev_event;
 	ret = register_netdevice_notifier(&iboe->nb);
 	if (ret) {
-- 
1.7.9.5

Re: [PATCH for-next 10/20] IB/hns: Modify the init of iboe lock

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

On Fri, Sep 09, 2016 at 06:30:41PM +0800, Salil Mehta wrote:
> From: Lijun Ou <oulijun@huawei.com>
>
> This lock will be used in query port interface, and will be called
> while IB device was registered to OFED frame. So, the lock of iboe
> must be initiated before IB device was registered.

Sorry,
what did you mean by writing "OFED frame"?

>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
> Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_main.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
> index 2704076..4721c0c 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_main.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
> @@ -615,6 +615,7 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>  	struct device *dev = &hr_dev->pdev->dev;
>
>  	iboe = &hr_dev->iboe;
> +	spin_lock_init(&iboe->lock);
>
>  	ib_dev = &hr_dev->ib_dev;
>  	strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
> @@ -701,8 +702,6 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>  		goto error_failed_setup_mtu_gids;
>  	}
>
> -	spin_lock_init(&iboe->lock);
> -
>  	iboe->nb.notifier_call = hns_roce_netdev_event;
>  	ret = register_netdevice_notifier(&iboe->nb);
>  	if (ret) {
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 10/20] IB/hns: Modify the init of iboe lock

[Salil Mehta]

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> Sent: Tuesday, September 13, 2016 7:50 AM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm; Huangdongdong (Donald)
> Subject: Re: [PATCH for-next 10/20] IB/hns: Modify the init of iboe
> lock
> 
> On Fri, Sep 09, 2016 at 06:30:41PM +0800, Salil Mehta wrote:
> > From: Lijun Ou <oulijun@huawei.com>
> >
> > This lock will be used in query port interface, and will be called
> > while IB device was registered to OFED frame. So, the lock of iboe
> > must be initiated before IB device was registered.
> 
> Sorry,
> what did you mean by writing "OFED frame"?
It is a typo. It was OFED framework but I guess more appropriate word 
might have been 'IB core' layer of Infiniband. Will fix this. Thanks! 

Best regards
Salil
> 
> >
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
> > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> >  drivers/infiniband/hw/hns/hns_roce_main.c |    3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c
> b/drivers/infiniband/hw/hns/hns_roce_main.c
> > index 2704076..4721c0c 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_main.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
> > @@ -615,6 +615,7 @@ static int hns_roce_register_device(struct
> hns_roce_dev *hr_dev)
> >  	struct device *dev = &hr_dev->pdev->dev;
> >
> >  	iboe = &hr_dev->iboe;
> > +	spin_lock_init(&iboe->lock);
> >
> >  	ib_dev = &hr_dev->ib_dev;
> >  	strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
> > @@ -701,8 +702,6 @@ static int hns_roce_register_device(struct
> hns_roce_dev *hr_dev)
> >  		goto error_failed_setup_mtu_gids;
> >  	}
> >
> > -	spin_lock_init(&iboe->lock);
> > -
> >  	iboe->nb.notifier_call = hns_roce_netdev_event;
> >  	ret = register_netdevice_notifier(&iboe->nb);
> >  	if (ret) {
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 10/20] IB/hns: Modify the init of iboe lock

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2919 bytes --]

On Wed, Sep 14, 2016 at 02:09:37AM +0000, Salil Mehta wrote:
>
>
> > -----Original Message-----
> > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> > owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> > Sent: Tuesday, September 13, 2016 7:50 AM
> > To: Salil Mehta
> > Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> > xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> > linux-kernel@vger.kernel.org; Linuxarm; Huangdongdong (Donald)
> > Subject: Re: [PATCH for-next 10/20] IB/hns: Modify the init of iboe
> > lock
> >
> > On Fri, Sep 09, 2016 at 06:30:41PM +0800, Salil Mehta wrote:
> > > From: Lijun Ou <oulijun@huawei.com>
> > >
> > > This lock will be used in query port interface, and will be called
> > > while IB device was registered to OFED frame. So, the lock of iboe
> > > must be initiated before IB device was registered.
> >
> > Sorry,
> > what did you mean by writing "OFED frame"?
> It is a typo. It was OFED framework but I guess more appropriate word
> might have been 'IB core' layer of Infiniband. Will fix this. Thanks!

As a general note, and I understand that these contributors are not
native English speakers, and I understand the desire to submit the right
code and code should speak by itself, but can you invest more time in
commit messages and write them in English?

Thanks

>
> Best regards
> Salil
> >
> > >
> > > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > > Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
> > > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > > ---
> > >  drivers/infiniband/hw/hns/hns_roce_main.c |    3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c
> > b/drivers/infiniband/hw/hns/hns_roce_main.c
> > > index 2704076..4721c0c 100644
> > > --- a/drivers/infiniband/hw/hns/hns_roce_main.c
> > > +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
> > > @@ -615,6 +615,7 @@ static int hns_roce_register_device(struct
> > hns_roce_dev *hr_dev)
> > >  	struct device *dev = &hr_dev->pdev->dev;
> > >
> > >  	iboe = &hr_dev->iboe;
> > > +	spin_lock_init(&iboe->lock);
> > >
> > >  	ib_dev = &hr_dev->ib_dev;
> > >  	strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
> > > @@ -701,8 +702,6 @@ static int hns_roce_register_device(struct
> > hns_roce_dev *hr_dev)
> > >  		goto error_failed_setup_mtu_gids;
> > >  	}
> > >
> > > -	spin_lock_init(&iboe->lock);
> > > -
> > >  	iboe->nb.notifier_call = hns_roce_netdev_event;
> > >  	ret = register_netdevice_notifier(&iboe->nb);
> > >  	if (ret) {
> > > --
> > > 1.7.9.5
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> > in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 10/20] IB/hns: Modify the init of iboe lock

[Salil Mehta]

> -----Original Message-----
> From: Leon Romanovsky [mailto:leon@kernel.org]
> Sent: Wednesday, September 14, 2016 6:05 AM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm; Huangdongdong (Donald)
> Subject: Re: [PATCH for-next 10/20] IB/hns: Modify the init of iboe
> lock
> 
> On Wed, Sep 14, 2016 at 02:09:37AM +0000, Salil Mehta wrote:
> >
> >
> > > -----Original Message-----
> > > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> > > owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> > > Sent: Tuesday, September 13, 2016 7:50 AM
> > > To: Salil Mehta
> > > Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng
> (Yisen);
> > > xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; Linuxarm; Huangdongdong (Donald)
> > > Subject: Re: [PATCH for-next 10/20] IB/hns: Modify the init of iboe
> > > lock
> > >
> > > On Fri, Sep 09, 2016 at 06:30:41PM +0800, Salil Mehta wrote:
> > > > From: Lijun Ou <oulijun@huawei.com>
> > > >
> > > > This lock will be used in query port interface, and will be
> called
> > > > while IB device was registered to OFED frame. So, the lock of
> iboe
> > > > must be initiated before IB device was registered.
> > >
> > > Sorry,
> > > what did you mean by writing "OFED frame"?
> > It is a typo. It was OFED framework but I guess more appropriate word
> > might have been 'IB core' layer of Infiniband. Will fix this. Thanks!
> 
> As a general note, and I understand that these contributors are not
> native English speakers, and I understand the desire to submit the
> right
> code and code should speak by itself, but can you invest more time in
> commit messages and write them in English?
> 
> Thanks
Hi Leon,
Yes, will try to make them better.

Best regards
Salil
> >
> > Best regards
> > Salil
> > >
> > > >
> > > > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > > > Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
> > > > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > > > ---
> > > >  drivers/infiniband/hw/hns/hns_roce_main.c |    3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c
> > > b/drivers/infiniband/hw/hns/hns_roce_main.c
> > > > index 2704076..4721c0c 100644
> > > > --- a/drivers/infiniband/hw/hns/hns_roce_main.c
> > > > +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
> > > > @@ -615,6 +615,7 @@ static int hns_roce_register_device(struct
> > > hns_roce_dev *hr_dev)
> > > >  	struct device *dev = &hr_dev->pdev->dev;
> > > >
> > > >  	iboe = &hr_dev->iboe;
> > > > +	spin_lock_init(&iboe->lock);
> > > >
> > > >  	ib_dev = &hr_dev->ib_dev;
> > > >  	strlcpy(ib_dev->name, "hisi_%d", IB_DEVICE_NAME_MAX);
> > > > @@ -701,8 +702,6 @@ static int hns_roce_register_device(struct
> > > hns_roce_dev *hr_dev)
> > > >  		goto error_failed_setup_mtu_gids;
> > > >  	}
> > > >
> > > > -	spin_lock_init(&iboe->lock);
> > > > -
> > > >  	iboe->nb.notifier_call = hns_roce_netdev_event;
> > > >  	ret = register_netdevice_notifier(&iboe->nb);
> > > >  	if (ret) {
> > > > --
> > > > 1.7.9.5
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-
> rdma"
> > > in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-
> info.html

[PATCH for-next 11/20] IB/hns: Fix bug of memory leakage for registering user mr

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

While the page size attribute of umem is illegal, we should release
umem that get by ib_umem_get interface.

Also, we should return a non-zero value while pbl number is wrong.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_mr.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_mr.c b/drivers/infiniband/hw/hns/hns_roce_mr.c
index 59f5e2b..fb87883 100644
--- a/drivers/infiniband/hw/hns/hns_roce_mr.c
+++ b/drivers/infiniband/hw/hns/hns_roce_mr.c
@@ -564,11 +564,14 @@ struct ib_mr *hns_roce_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 	if (mr->umem->page_size != HNS_ROCE_HEM_PAGE_SIZE) {
 		dev_err(dev, "Just support 4K page size but is 0x%x now!\n",
 			mr->umem->page_size);
+		ret = -EINVAL;
+		goto err_umem;
 	}
 
 	if (n > HNS_ROCE_MAX_MTPT_PBL_NUM) {
 		dev_err(dev, " MR len %lld err. MR is limited to 4G at most!\n",
 			length);
+		ret = -EINVAL;
 		goto err_umem;
 	}
 
-- 
1.7.9.5

[PATCH for-next 12/20] IB/hns: Return bad wr while post send failed

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

While post failed, hns roce should return the wr failed to user.
We omitted this while qp type is wrong and fixed it.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index f4d2515..f0d6315 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -264,8 +264,10 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 			}
 			ind++;
 		} else {
-			dev_dbg(dev, "unSupported QP type\n");
-			break;
+			dev_err(dev, "unSupported QP type\n");
+			ret = -EOPNOTSUPP;
+			*bad_wr = wr;
+			goto out;
 		}
 	}
 
-- 
1.7.9.5

Re: [PATCH for-next 12/20] IB/hns: Return bad wr while post send failed

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]

On Fri, Sep 09, 2016 at 06:30:43PM +0800, Salil Mehta wrote:
> From: Lijun Ou <oulijun@huawei.com>
>
> While post failed, hns roce should return the wr failed to user.
> We omitted this while qp type is wrong and fixed it.
>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
> Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index f4d2515..f0d6315 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -264,8 +264,10 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
>  			}
>  			ind++;
>  		} else {

You should move the check of supported QPs to the beginning of post_send
and don't enter into the loop at all.

> -			dev_dbg(dev, "unSupported QP type\n");
> -			break;
> +			dev_err(dev, "unSupported QP type\n");

During initial review of hns driver, Lijun was asked to canonize debug
messages and don't use different styles and especially sUcH case.

> +			ret = -EOPNOTSUPP;
> +			*bad_wr = wr;

IMHO, the proper thing is to return NULL, because this is not wrong wr.

> +			goto out;
>  		}
>  	}
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 12/20] IB/hns: Return bad wr while post send failed

[Salil Mehta]

> -----Original Message-----
> From: Leon Romanovsky [mailto:leon@kernel.org]
> Sent: Thursday, September 15, 2016 10:08 AM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm; Huangdongdong (Donald)
> Subject: Re: [PATCH for-next 12/20] IB/hns: Return bad wr while post
> send failed
> 
> On Fri, Sep 09, 2016 at 06:30:43PM +0800, Salil Mehta wrote:
> > From: Lijun Ou <oulijun@huawei.com>
> >
> > While post failed, hns roce should return the wr failed to user.
> > We omitted this while qp type is wrong and fixed it.
> >
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
> > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> >  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > index f4d2515..f0d6315 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > @@ -264,8 +264,10 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp,
> struct ib_send_wr *wr,
> >  			}
> >  			ind++;
> >  		} else {
> 
> You should move the check of supported QPs to the beginning of
> post_send
> and don't enter into the loop at all.
Hi Leon,
Agreed. I have made this change in floated V2 patch.

Thanks
Salil
> 
> > -			dev_dbg(dev, "unSupported QP type\n");
> > -			break;
> > +			dev_err(dev, "unSupported QP type\n");
> 
> During initial review of hns driver, Lijun was asked to canonize debug
> messages and don't use different styles and especially sUcH case.
Ok. I have corrected for this instance in V2 patch.

Thanks
Salil
> 
> > +			ret = -EOPNOTSUPP;
> > +			*bad_wr = wr;
> 
> IMHO, the proper thing is to return NULL, because this is not wrong wr.
Ok. Done this change as well.

Thanks
Salil
> 
> > +			goto out;
> >  		}
> >  	}
> >
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 13/20] IB/hns: Add check for some NULL pointer scenes

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

Some pointers have not be checked when they are null,
so we add check for them.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index f0d6315..e3e154c 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -92,6 +92,12 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
 		}
 
 		wqe = get_send_wqe(qp, ind & (qp->sq.wqe_cnt - 1));
+		if (unlikely(!wqe)) {
+			dev_err(dev, "get send wqe failed\n");
+			ret = -EINVAL;
+			*bad_wr = wr;
+			goto out;
+		}
 		qp->sq.wrid[(qp->sq.head + nreq) & (qp->sq.wqe_cnt - 1)] =
 								      wr->wr_id;
 
@@ -1558,6 +1564,11 @@ static int hns_roce_v1_poll_one(struct hns_roce_cq *hr_cq,
 		sq_wqe = get_send_wqe(*cur_qp, roce_get_field(cqe->cqe_byte_4,
 						CQE_BYTE_4_WQE_INDEX_M,
 						CQE_BYTE_4_WQE_INDEX_S));
+		if (unlikely(!sq_wqe)) {
+			dev_err(dev, "Get send wqe failed!\n");
+			return -EFAULT;
+		}
+
 		switch (sq_wqe->flag & HNS_ROCE_WQE_OPCODE_MASK) {
 		case HNS_ROCE_WQE_OPCODE_SEND:
 			wc->opcode = IB_WC_SEND;
-- 
1.7.9.5

Re: [PATCH for-next 13/20] IB/hns: Add check for some NULL pointer scenes

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]

On Fri, Sep 09, 2016 at 06:30:44PM +0800, Salil Mehta wrote:
> From: Lijun Ou <oulijun@huawei.com>
>
> Some pointers have not be checked when they are null,
> so we add check for them.
>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
> Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>

I may admit that I didn't check your code to read the implementations of
get_send_wqe() and hns_roce_v1_poll_one(), but based on my assumption
that the code is similar to mlx4.

These failures can't occur.

Can you throw a light on how did you find them and check it?

> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index f0d6315..e3e154c 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -92,6 +92,12 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
>  		}
>
>  		wqe = get_send_wqe(qp, ind & (qp->sq.wqe_cnt - 1));
> +		if (unlikely(!wqe)) {
> +			dev_err(dev, "get send wqe failed\n");
> +			ret = -EINVAL;
> +			*bad_wr = wr;
> +			goto out;
> +		}
>  		qp->sq.wrid[(qp->sq.head + nreq) & (qp->sq.wqe_cnt - 1)] =
>  								      wr->wr_id;
>
> @@ -1558,6 +1564,11 @@ static int hns_roce_v1_poll_one(struct hns_roce_cq *hr_cq,
>  		sq_wqe = get_send_wqe(*cur_qp, roce_get_field(cqe->cqe_byte_4,
>  						CQE_BYTE_4_WQE_INDEX_M,
>  						CQE_BYTE_4_WQE_INDEX_S));
> +		if (unlikely(!sq_wqe)) {
> +			dev_err(dev, "Get send wqe failed!\n");
> +			return -EFAULT;
> +		}
> +
>  		switch (sq_wqe->flag & HNS_ROCE_WQE_OPCODE_MASK) {
>  		case HNS_ROCE_WQE_OPCODE_SEND:
>  			wc->opcode = IB_WC_SEND;
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 13/20] IB/hns: Add check for some NULL pointer scenes

[Salil Mehta]

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> Sent: Tuesday, September 13, 2016 8:00 AM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm; Huangdongdong (Donald)
> Subject: Re: [PATCH for-next 13/20] IB/hns: Add check for some NULL
> pointer scenes
> 
> On Fri, Sep 09, 2016 at 06:30:44PM +0800, Salil Mehta wrote:
> > From: Lijun Ou <oulijun@huawei.com>
> >
> > Some pointers have not be checked when they are null,
> > so we add check for them.
> >
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
> > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> 
> I may admit that I didn't check your code to read the implementations
> of
> get_send_wqe() and hns_roce_v1_poll_one(), but based on my assumption
> that the code is similar to mlx4.
> 
> These failures can't occur.
> 
> Can you throw a light on how did you find them and check it?
Hi Leon,
Looks like this is another redundant patch. These return checks should
never be required. I think the mistake lies in the wrong check placed
inside below function:

void *get_send_wqe(struct hns_roce_qp *hr_qp, int n)
{
.................................................
.................................................
/* To Be Deleted: Below check is redundantly placed. */
	if ((n < 0) || (n > hr_qp->sq.wqe_cnt)) {
		dev_err(&hr_dev->pdev->dev, "sq wqe index:%d,sq wqe cnt:%d\r\n",
			n, hr_qp->sq.wqe_cnt);
		return NULL;
	}

	return get_wqe(hr_qp, hr_qp->sq.offset + (n << hr_qp->sq.wqe_shift));
}

and perhaps same is the case in function get_rcv_wqe(). The same check needs
to be removed from there as well and also the error handling in the calling
functions. Thanks for figuring it out. Will correct in the subsequent patch.

Best regards
Salil 
> 
> > ---
> >  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |   11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > index f0d6315..e3e154c 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > @@ -92,6 +92,12 @@ int hns_roce_v1_post_send(struct ib_qp *ibqp,
> struct ib_send_wr *wr,
> >  		}
> >
> >  		wqe = get_send_wqe(qp, ind & (qp->sq.wqe_cnt - 1));
> > +		if (unlikely(!wqe)) {
> > +			dev_err(dev, "get send wqe failed\n");
> > +			ret = -EINVAL;
> > +			*bad_wr = wr;
> > +			goto out;
> > +		}
> >  		qp->sq.wrid[(qp->sq.head + nreq) & (qp->sq.wqe_cnt - 1)] =
> >  								      wr->wr_id;
> >
> > @@ -1558,6 +1564,11 @@ static int hns_roce_v1_poll_one(struct
> hns_roce_cq *hr_cq,
> >  		sq_wqe = get_send_wqe(*cur_qp, roce_get_field(cqe-
> >cqe_byte_4,
> >  						CQE_BYTE_4_WQE_INDEX_M,
> >  						CQE_BYTE_4_WQE_INDEX_S));
> > +		if (unlikely(!sq_wqe)) {
> > +			dev_err(dev, "Get send wqe failed!\n");
> > +			return -EFAULT;
> > +		}
> > +
> >  		switch (sq_wqe->flag & HNS_ROCE_WQE_OPCODE_MASK) {
> >  		case HNS_ROCE_WQE_OPCODE_SEND:
> >  			wc->opcode = IB_WC_SEND;
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 14/20] IB/hns: The Ack timeout need a lower limit value

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

The Ack timeout of qpc need a lower limit value,otherwise
the read performance will be very lower.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index e3e154c..836a2af 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -2428,10 +2428,19 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
 			       QP_CONTEXT_QPC_BYTES_156_RETRY_COUNT_INIT_M,
 			       QP_CONTEXT_QPC_BYTES_156_RETRY_COUNT_INIT_S,
 			       attr->retry_cnt);
-		roce_set_field(context->qpc_bytes_156,
-			       QP_CONTEXT_QPC_BYTES_156_ACK_TIMEOUT_M,
-			       QP_CONTEXT_QPC_BYTES_156_ACK_TIMEOUT_S,
-			       attr->timeout);
+		if (attr->timeout < 0x12) {
+			dev_info(dev, "ack timeout value(0x%x) must bigger than 0x12.\n",
+				 attr->timeout);
+			roce_set_field(context->qpc_bytes_156,
+				       QP_CONTEXT_QPC_BYTES_156_ACK_TIMEOUT_M,
+				       QP_CONTEXT_QPC_BYTES_156_ACK_TIMEOUT_S,
+				       0x12);
+		} else {
+			roce_set_field(context->qpc_bytes_156,
+				       QP_CONTEXT_QPC_BYTES_156_ACK_TIMEOUT_M,
+				       QP_CONTEXT_QPC_BYTES_156_ACK_TIMEOUT_S,
+				       attr->timeout);
+		}
 		roce_set_field(context->qpc_bytes_156,
 			       QP_CONTEXT_QPC_BYTES_156_RNR_RETRY_COUNT_INIT_M,
 			       QP_CONTEXT_QPC_BYTES_156_RNR_RETRY_COUNT_INIT_S,
-- 
1.7.9.5

[PATCH for-next 15/20] IB/hns: Some items of qpc need to take user param

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

Some items of qpc need to take user param when modified qp
state.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 836a2af..bdd9453 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -2414,10 +2414,12 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
 			       QP_CONTEXT_QPC_BYTES_148_CHECK_FLAG_S, 0);
 		roce_set_field(context->qpc_bytes_148,
 			       QP_CONTEXT_QPC_BYTES_148_RETRY_COUNT_M,
-			       QP_CONTEXT_QPC_BYTES_148_RETRY_COUNT_S, 0);
+			       QP_CONTEXT_QPC_BYTES_148_RETRY_COUNT_S,
+			       attr->retry_cnt);
 		roce_set_field(context->qpc_bytes_148,
 			       QP_CONTEXT_QPC_BYTES_148_RNR_RETRY_COUNT_M,
-			       QP_CONTEXT_QPC_BYTES_148_RNR_RETRY_COUNT_S, 0);
+			       QP_CONTEXT_QPC_BYTES_148_RNR_RETRY_COUNT_S,
+			       attr->rnr_retry);
 		roce_set_field(context->qpc_bytes_148,
 			       QP_CONTEXT_QPC_BYTES_148_LSN_M,
 			       QP_CONTEXT_QPC_BYTES_148_LSN_S, 0x100);
-- 
1.7.9.5

[PATCH for-next 16/20] IB/hns: Validate mtu when modified qp

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

The mtu should be validated when modify qp,so we check it.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Peter Chen <luck.chen@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_qp.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
index 51fefbf..1c5be59 100644
--- a/drivers/infiniband/hw/hns/hns_roce_qp.c
+++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
@@ -32,6 +32,7 @@
  */
 
 #include <linux/platform_device.h>
+#include <rdma/ib_addr.h>
 #include <rdma/ib_umem.h>
 #include "hns_roce_common.h"
 #include "hns_roce_device.h"
@@ -658,6 +659,7 @@ int hns_roce_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 	struct device *dev = &hr_dev->pdev->dev;
 	int ret = -EINVAL;
 	int p;
+	u32 active_mtu = 0;
 
 	mutex_lock(&hr_qp->mutex);
 
@@ -688,6 +690,19 @@ int hns_roce_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 		}
 	}
 
+	if (attr_mask & IB_QP_PATH_MTU) {
+		p = attr_mask & IB_QP_PORT ? (attr->port_num - 1) : hr_qp->port;
+		active_mtu = iboe_get_mtu(hr_dev->iboe.netdevs[p]->mtu);
+
+		if (attr->path_mtu > IB_MTU_2048 ||
+		    attr->path_mtu < IB_MTU_256 ||
+		    attr->path_mtu > active_mtu) {
+			dev_err(dev, "attr path_mtu(%d)invalid while modify qp",
+				attr->path_mtu);
+			goto out;
+		}
+	}
+
 	if (attr_mask & IB_QP_MAX_QP_RD_ATOMIC &&
 	    attr->max_rd_atomic > hr_dev->caps.max_qp_init_rdma) {
 		dev_err(dev, "attr max_rd_atomic invalid.attr->max_rd_atomic=%d\n",
-- 
1.7.9.5

Re: [PATCH for-next 16/20] IB/hns: Validate mtu when modified qp

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2181 bytes --]

On Fri, Sep 09, 2016 at 06:30:47PM +0800, Salil Mehta wrote:
> From: Lijun Ou <oulijun@huawei.com>
>
> The mtu should be validated when modify qp,so we check it.
>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Signed-off-by: Peter Chen <luck.chen@huawei.com>
> Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_qp.c |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
> index 51fefbf..1c5be59 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_qp.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
> @@ -32,6 +32,7 @@
>   */
>
>  #include <linux/platform_device.h>
> +#include <rdma/ib_addr.h>
>  #include <rdma/ib_umem.h>
>  #include "hns_roce_common.h"
>  #include "hns_roce_device.h"
> @@ -658,6 +659,7 @@ int hns_roce_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>  	struct device *dev = &hr_dev->pdev->dev;
>  	int ret = -EINVAL;
>  	int p;
> +	u32 active_mtu = 0;

There is no need to assign value to a variable which will be overwritten.

>
>  	mutex_lock(&hr_qp->mutex);
>
> @@ -688,6 +690,19 @@ int hns_roce_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>  		}
>  	}
>
> +	if (attr_mask & IB_QP_PATH_MTU) {
> +		p = attr_mask & IB_QP_PORT ? (attr->port_num - 1) : hr_qp->port;
> +		active_mtu = iboe_get_mtu(hr_dev->iboe.netdevs[p]->mtu);

ib_mtu iboe_get_mtu returns "enum ib_mtu" and not u32.

> +
> +		if (attr->path_mtu > IB_MTU_2048 ||
> +		    attr->path_mtu < IB_MTU_256 ||
> +		    attr->path_mtu > active_mtu) {
> +			dev_err(dev, "attr path_mtu(%d)invalid while modify qp",
> +				attr->path_mtu);
> +			goto out;
> +		}
> +	}
> +
>  	if (attr_mask & IB_QP_MAX_QP_RD_ATOMIC &&
>  	    attr->max_rd_atomic > hr_dev->caps.max_qp_init_rdma) {
>  		dev_err(dev, "attr max_rd_atomic invalid.attr->max_rd_atomic=%d\n",
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 16/20] IB/hns: Validate mtu when modified qp

[Salil Mehta]

> -----Original Message-----
> From: Leon Romanovsky [mailto:leon@kernel.org]
> Sent: Tuesday, September 13, 2016 7:33 AM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm; chenguolong (A)
> Subject: Re: [PATCH for-next 16/20] IB/hns: Validate mtu when modified
> qp
> 
> On Fri, Sep 09, 2016 at 06:30:47PM +0800, Salil Mehta wrote:
> > From: Lijun Ou <oulijun@huawei.com>
> >
> > The mtu should be validated when modify qp,so we check it.
> >
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > Signed-off-by: Peter Chen <luck.chen@huawei.com>
> > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> >  drivers/infiniband/hw/hns/hns_roce_qp.c |   15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c
> b/drivers/infiniband/hw/hns/hns_roce_qp.c
> > index 51fefbf..1c5be59 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_qp.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
> > @@ -32,6 +32,7 @@
> >   */
> >
> >  #include <linux/platform_device.h>
> > +#include <rdma/ib_addr.h>
> >  #include <rdma/ib_umem.h>
> >  #include "hns_roce_common.h"
> >  #include "hns_roce_device.h"
> > @@ -658,6 +659,7 @@ int hns_roce_modify_qp(struct ib_qp *ibqp, struct
> ib_qp_attr *attr,
> >  	struct device *dev = &hr_dev->pdev->dev;
> >  	int ret = -EINVAL;
> >  	int p;
> > +	u32 active_mtu = 0;
> 
> There is no need to assign value to a variable which will be
> overwritten.
Agreed. This initialization seems redundant. Will remove. Thanks!

Best regards
Salil
> 
> >
> >  	mutex_lock(&hr_qp->mutex);
> >
> > @@ -688,6 +690,19 @@ int hns_roce_modify_qp(struct ib_qp *ibqp,
> struct ib_qp_attr *attr,
> >  		}
> >  	}
> >
> > +	if (attr_mask & IB_QP_PATH_MTU) {
> > +		p = attr_mask & IB_QP_PORT ? (attr->port_num - 1) : hr_qp-
> >port;
> > +		active_mtu = iboe_get_mtu(hr_dev->iboe.netdevs[p]->mtu);
> 
> ib_mtu iboe_get_mtu returns "enum ib_mtu" and not u32.
Ok. This can be converted to 'enum'. Will change. Thanks!

Best regards
Salil 
> 
> > +
> > +		if (attr->path_mtu > IB_MTU_2048 ||
> > +		    attr->path_mtu < IB_MTU_256 ||
> > +		    attr->path_mtu > active_mtu) {
> > +			dev_err(dev, "attr path_mtu(%d)invalid while modify
> qp",
> > +				attr->path_mtu);
> > +			goto out;
> > +		}
> > +	}
> > +
> >  	if (attr_mask & IB_QP_MAX_QP_RD_ATOMIC &&
> >  	    attr->max_rd_atomic > hr_dev->caps.max_qp_init_rdma) {
> >  		dev_err(dev, "attr max_rd_atomic invalid.attr-
> >max_rd_atomic=%d\n",
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 17/20] IB/hns: Cq has not been freed

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

Cq has not been freed when fail to ib_copy_to_udata, so need to
free it.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Peter Chen <luck.chen@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_cq.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_cq.c b/drivers/infiniband/hw/hns/hns_roce_cq.c
index 3095f06..0973659 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cq.c
@@ -357,12 +357,15 @@ struct ib_cq *hns_roce_ib_create_cq(struct ib_device *ib_dev,
 	if (context) {
 		if (ib_copy_to_udata(udata, &hr_cq->cqn, sizeof(u64))) {
 			ret = -EFAULT;
-			goto err_mtt;
+			goto err_cqc;
 		}
 	}
 
 	return &hr_cq->ib_cq;
 
+err_cqc:
+	hns_roce_free_cq(hr_dev, hr_cq);
+
 err_mtt:
 	hns_roce_mtt_cleanup(hr_dev, &hr_cq->hr_buf.hr_mtt);
 	if (context)
-- 
1.7.9.5

[PATCH for-next 18/20] IB/hns: Update the rq head when modify qp state

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

The rq head in qpc was zero will miss the rq wqes which
have be sent, so here we should take the real value.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index bdd9453..7a61d7e 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -2266,7 +2266,8 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
 
 		roce_set_field(context->qpc_bytes_68,
 			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_M,
-			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S, 0);
+			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S,
+			       hr_qp->rq.head);
 		roce_set_field(context->qpc_bytes_68,
 			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_M,
 			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_S, 0);
-- 
1.7.9.5

Re: [PATCH for-next 18/20] IB/hns: Update the rq head when modify qp state

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1483 bytes --]

On Fri, Sep 09, 2016 at 06:30:49PM +0800, Salil Mehta wrote:
> From: Lijun Ou <oulijun@huawei.com>
>
> The rq head in qpc was zero will miss the rq wqes which
> have be sent, so here we should take the real value.
>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index bdd9453..7a61d7e 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -2266,7 +2266,8 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
>
>  		roce_set_field(context->qpc_bytes_68,
>  			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_M,
> -			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S, 0);
> +			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S,
> +			       hr_qp->rq.head);

Are you sure that it should be list_head and not value?

>  		roce_set_field(context->qpc_bytes_68,
>  			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_M,
>  			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_S, 0);
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 18/20] IB/hns: Update the rq head when modify qp state

[Salil Mehta]

> -----Original Message-----
> From: Leon Romanovsky [mailto:leon@kernel.org]
> Sent: Thursday, September 15, 2016 11:13 AM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH for-next 18/20] IB/hns: Update the rq head when
> modify qp state
> 
> On Fri, Sep 09, 2016 at 06:30:49PM +0800, Salil Mehta wrote:
> > From: Lijun Ou <oulijun@huawei.com>
> >
> > The rq head in qpc was zero will miss the rq wqes which
> > have be sent, so here we should take the real value.
> >
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> >  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > index bdd9453..7a61d7e 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > @@ -2266,7 +2266,8 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp,
> const struct ib_qp_attr *attr,
> >
> >  		roce_set_field(context->qpc_bytes_68,
> >  			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_M,
> > -			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S, 0);
> > +			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S,
> > +			       hr_qp->rq.head);
> 
> Are you sure that it should be list_head and not value?
This is correct. Variable head is not list_head but a u32 value. 
Hope I understood your point correctly.

Best regards
Salil
> 
> >  		roce_set_field(context->qpc_bytes_68,
> >  			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_M,
> >  			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_S, 0);
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 18/20] IB/hns: Update the rq head when modify qp state

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2354 bytes --]

On Tue, Sep 20, 2016 at 04:56:35PM +0000, Salil Mehta wrote:
>
>
> > -----Original Message-----
> > From: Leon Romanovsky [mailto:leon@kernel.org]
> > Sent: Thursday, September 15, 2016 11:13 AM
> > To: Salil Mehta
> > Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> > xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> > linux-kernel@vger.kernel.org; Linuxarm
> > Subject: Re: [PATCH for-next 18/20] IB/hns: Update the rq head when
> > modify qp state
> >
> > On Fri, Sep 09, 2016 at 06:30:49PM +0800, Salil Mehta wrote:
> > > From: Lijun Ou <oulijun@huawei.com>
> > >
> > > The rq head in qpc was zero will miss the rq wqes which
> > > have be sent, so here we should take the real value.
> > >
> > > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > > ---
> > >  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > index bdd9453..7a61d7e 100644
> > > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > @@ -2266,7 +2266,8 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp,
> > const struct ib_qp_attr *attr,
> > >
> > >  		roce_set_field(context->qpc_bytes_68,
> > >  			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_M,
> > > -			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S, 0);
> > > +			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S,
> > > +			       hr_qp->rq.head);
> >
> > Are you sure that it should be list_head and not value?
> This is correct. Variable head is not list_head but a u32 value.
> Hope I understood your point correctly.

Thanks, you understood me right.
The name "head" mislead me.
Sorry for that.

>
> Best regards
> Salil
> >
> > >  		roce_set_field(context->qpc_bytes_68,
> > >  			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_M,
> > >  			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_S, 0);
> > > --
> > > 1.7.9.5
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> > in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 18/20] IB/hns: Update the rq head when modify qp state

[Salil Mehta]

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> Sent: Tuesday, September 20, 2016 6:06 PM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH for-next 18/20] IB/hns: Update the rq head when
> modify qp state
> 
> On Tue, Sep 20, 2016 at 04:56:35PM +0000, Salil Mehta wrote:
> >
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky [mailto:leon@kernel.org]
> > > Sent: Thursday, September 15, 2016 11:13 AM
> > > To: Salil Mehta
> > > Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng
> (Yisen);
> > > xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; Linuxarm
> > > Subject: Re: [PATCH for-next 18/20] IB/hns: Update the rq head when
> > > modify qp state
> > >
> > > On Fri, Sep 09, 2016 at 06:30:49PM +0800, Salil Mehta wrote:
> > > > From: Lijun Ou <oulijun@huawei.com>
> > > >
> > > > The rq head in qpc was zero will miss the rq wqes which
> > > > have be sent, so here we should take the real value.
> > > >
> > > > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > > > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > > > ---
> > > >  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |    3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > > index bdd9453..7a61d7e 100644
> > > > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > > @@ -2266,7 +2266,8 @@ static int hns_roce_v1_m_qp(struct ib_qp
> *ibqp,
> > > const struct ib_qp_attr *attr,
> > > >
> > > >  		roce_set_field(context->qpc_bytes_68,
> > > >  			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_M,
> > > > -			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S, 0);
> > > > +			       QP_CONTEXT_QPC_BYTES_68_RQ_HEAD_S,
> > > > +			       hr_qp->rq.head);
> > >
> > > Are you sure that it should be list_head and not value?
> > This is correct. Variable head is not list_head but a u32 value.
> > Hope I understood your point correctly.
> 
> Thanks, you understood me right.
> The name "head" mislead me.
> Sorry for that.
No worries at all :)

Thanks
Salil
> 
> >
> > Best regards
> > Salil
> > >
> > > >  		roce_set_field(context->qpc_bytes_68,
> > > >  			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_M,
> > > >  			       QP_CONTEXT_QPC_BYTES_68_RQ_CUR_INDEX_S,
> 0);
> > > > --
> > > > 1.7.9.5
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-
> rdma"
> > > in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-
> info.html

[PATCH for-next 19/20] IB/hns: Fix the bug when platform_get_resource() exec fail

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

This patch mainly fixes the bug with platform_get_resource().
It should return NULL when platform_get_resource() exec fail.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_pd.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_pd.c b/drivers/infiniband/hw/hns/hns_roce_pd.c
index 0cd6132..05db7d5 100644
--- a/drivers/infiniband/hw/hns/hns_roce_pd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_pd.c
@@ -110,6 +110,10 @@ int hns_roce_uar_alloc(struct hns_roce_dev *hr_dev, struct hns_roce_uar *uar)
 			     (hr_dev->caps.phy_num_uars - 1) + 1;
 
 	res = platform_get_resource(hr_dev->pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&hr_dev->pdev->dev, "memory resource not found!\n");
+		return -EINVAL;
+	}
 	uar->pfn = ((res->start) >> PAGE_SHIFT) + uar->index;
 
 	return 0;
-- 
1.7.9.5

[PATCH for-next 20/20] IB/hns: Delete the redundant lines in hns_roce_v1_m_qp()

[Salil Mehta]

From: Lijun Ou <oulijun@huawei.com>

It doesn't need to assign for the filed of qp state in qpc separately
when qp happen to migrate state which supported in RoCE engine v1.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c |   14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 7a61d7e..3bc32fc 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -2405,11 +2405,6 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
 		roce_set_bit(context->qpc_bytes_140,
 			     QP_CONTEXT_QPC_BYTES_140_RNR_RETRY_FLG_S, 0);
 
-		roce_set_field(context->qpc_bytes_144,
-			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
-			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S,
-			       attr->qp_state);
-
 		roce_set_field(context->qpc_bytes_148,
 			       QP_CONTEXT_QPC_BYTES_148_CHECK_FLAG_M,
 			       QP_CONTEXT_QPC_BYTES_148_CHECK_FLAG_S, 0);
@@ -2524,13 +2519,10 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
 		   (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) ||
 		   (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) ||
 		   (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) {
-		roce_set_field(context->qpc_bytes_144,
-			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
-			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S,
-			       attr->qp_state);
-
+		/* It will not do additional options in engine v1 */
+		;
 	} else {
-		dev_err(dev, "not support this modify\n");
+		dev_err(dev, "not support this status migration\n");
 		goto out;
 	}
 
-- 
1.7.9.5

Re: [PATCH for-next 20/20] IB/hns: Delete the redundant lines in hns_roce_v1_m_qp()

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]

On Fri, Sep 09, 2016 at 06:30:51PM +0800, Salil Mehta wrote:
> From: Lijun Ou <oulijun@huawei.com>
>
> It doesn't need to assign for the filed of qp state in qpc separately
> when qp happen to migrate state which supported in RoCE engine v1.
>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |   14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 7a61d7e..3bc32fc 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -2405,11 +2405,6 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
>  		roce_set_bit(context->qpc_bytes_140,
>  			     QP_CONTEXT_QPC_BYTES_140_RNR_RETRY_FLG_S, 0);
>
> -		roce_set_field(context->qpc_bytes_144,
> -			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
> -			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S,
> -			       attr->qp_state);
> -
>  		roce_set_field(context->qpc_bytes_148,
>  			       QP_CONTEXT_QPC_BYTES_148_CHECK_FLAG_M,
>  			       QP_CONTEXT_QPC_BYTES_148_CHECK_FLAG_S, 0);
> @@ -2524,13 +2519,10 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
>  		   (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) ||
>  		   (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET) ||
>  		   (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) {
> -		roce_set_field(context->qpc_bytes_144,
> -			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
> -			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S,
> -			       attr->qp_state);
> -
> +		/* It will not do additional options in engine v1 */
> +		;

It will leave empty if() {..}, it is worth to rewrite to have one if for
error path only.

>  	} else {
> -		dev_err(dev, "not support this modify\n");
> +		dev_err(dev, "not support this status migration\n");
>  		goto out;
>  	}
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH for-next 20/20] IB/hns: Delete the redundant lines in hns_roce_v1_m_qp()

[Salil Mehta]

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> Sent: Thursday, September 15, 2016 11:15 AM
> To: Salil Mehta
> Cc: dledford@redhat.com; Huwei (Xavier); oulijun; Zhuangyuzeng (Yisen);
> xuwei (O); mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH for-next 20/20] IB/hns: Delete the redundant lines
> in hns_roce_v1_m_qp()
> 
> On Fri, Sep 09, 2016 at 06:30:51PM +0800, Salil Mehta wrote:
> > From: Lijun Ou <oulijun@huawei.com>
> >
> > It doesn't need to assign for the filed of qp state in qpc separately
> > when qp happen to migrate state which supported in RoCE engine v1.
> >
> > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > Reviewed-by:  Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> >  drivers/infiniband/hw/hns/hns_roce_hw_v1.c |   14 +++-----------
> >  1 file changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > index 7a61d7e..3bc32fc 100644
> > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > @@ -2405,11 +2405,6 @@ static int hns_roce_v1_m_qp(struct ib_qp
> *ibqp, const struct ib_qp_attr *attr,
> >  		roce_set_bit(context->qpc_bytes_140,
> >  			     QP_CONTEXT_QPC_BYTES_140_RNR_RETRY_FLG_S, 0);
> >
> > -		roce_set_field(context->qpc_bytes_144,
> > -			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
> > -			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S,
> > -			       attr->qp_state);
> > -
> >  		roce_set_field(context->qpc_bytes_148,
> >  			       QP_CONTEXT_QPC_BYTES_148_CHECK_FLAG_M,
> >  			       QP_CONTEXT_QPC_BYTES_148_CHECK_FLAG_S, 0);
> > @@ -2524,13 +2519,10 @@ static int hns_roce_v1_m_qp(struct ib_qp
> *ibqp, const struct ib_qp_attr *attr,
> >  		   (cur_state == IB_QPS_RTS && new_state == IB_QPS_ERR) ||
> >  		   (cur_state == IB_QPS_ERR && new_state == IB_QPS_RESET)
> ||
> >  		   (cur_state == IB_QPS_ERR && new_state == IB_QPS_ERR)) {
> > -		roce_set_field(context->qpc_bytes_144,
> > -			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_M,
> > -			       QP_CONTEXT_QPC_BYTES_144_QP_STATE_S,
> > -			       attr->qp_state);
> > -
> > +		/* It will not do additional options in engine v1 */
> > +		;
> 
> It will leave empty if() {..}, it is worth to rewrite to have one if
> for
> error path only.
Agreed & changed in patch v2!

Thanks
Salil
> 
> >  	} else {
> > -		dev_err(dev, "not support this modify\n");
> > +		dev_err(dev, "not support this status migration\n");
> >  		goto out;
> >  	}
> >
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html