[PATCH for-rc 0/3] RDMA/hns: Bugfix and improvements

[PATCH for-rc 0/3] RDMA/hns: Bugfix and improvements

[Junxian Huang]

1. #1-#2: The first two patches improves diagnostic information to
   the driver.

2. #3: The third patch removes unnecessary QP type checks.


Chengchang Tang (2):
  RDMA/hns: Fix hns_roce_table_get return value
  RDMA/hns: Add clear_hem return value to log

Junxian Huang (1):
  RDMA/hns: Remove unnecessary QP type checks

 drivers/infiniband/hw/hns/hns_roce_hem.c   | 51 ++++++++++++++--------
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 22 ++--------
 2 files changed, 37 insertions(+), 36 deletions(-)

--
2.30.0

[PATCH for-rc 1/3] RDMA/hns: Remove unnecessary QP type checks

[Junxian Huang]

It is not necessary to check the type of the queue on IO path
because unsupported QP type cannot be created.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index d4c6b9bc0a4e..673fd31b9ecf 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -373,17 +373,10 @@ static int check_send_valid(struct hns_roce_dev *hr_dev,
 			    struct hns_roce_qp *hr_qp)
 {
 	struct ib_device *ibdev = &hr_dev->ib_dev;
-	struct ib_qp *ibqp = &hr_qp->ibqp;
 
-	if (unlikely(ibqp->qp_type != IB_QPT_RC &&
-		     ibqp->qp_type != IB_QPT_GSI &&
-		     ibqp->qp_type != IB_QPT_UD)) {
-		ibdev_err(ibdev, "not supported QP(0x%x)type!\n",
-			  ibqp->qp_type);
-		return -EOPNOTSUPP;
-	} else if (unlikely(hr_qp->state == IB_QPS_RESET ||
-		   hr_qp->state == IB_QPS_INIT ||
-		   hr_qp->state == IB_QPS_RTR)) {
+	if (unlikely(hr_qp->state == IB_QPS_RESET ||
+		     hr_qp->state == IB_QPS_INIT ||
+		     hr_qp->state == IB_QPS_RTR)) {
 		ibdev_err(ibdev, "failed to post WQE, QP state %u!\n",
 			  hr_qp->state);
 		return -EINVAL;
@@ -772,15 +765,6 @@ static int check_recv_valid(struct hns_roce_dev *hr_dev,
 			    struct hns_roce_qp *hr_qp)
 {
 	struct ib_device *ibdev = &hr_dev->ib_dev;
-	struct ib_qp *ibqp = &hr_qp->ibqp;
-
-	if (unlikely(ibqp->qp_type != IB_QPT_RC &&
-		     ibqp->qp_type != IB_QPT_GSI &&
-		     ibqp->qp_type != IB_QPT_UD)) {
-		ibdev_err(ibdev, "unsupported qp type, qp_type = %d.\n",
-			  ibqp->qp_type);
-		return -EOPNOTSUPP;
-	}
 
 	if (unlikely(hr_dev->state >= HNS_ROCE_DEVICE_STATE_RST_DOWN))
 		return -EIO;
-- 
2.30.0

[PATCH for-rc 2/3] RDMA/hns: Fix hns_roce_table_get return value

[Junxian Huang]

From: Chengchang Tang <tangchengchang@huawei.com>

The return value of set_hem has been fixed to ENODEV, which will
lead a diagnostic information missing.

Fixes: 9a4435375cd1 ("IB/hns: Add driver files for hns RoCE driver")
Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
---
 drivers/infiniband/hw/hns/hns_roce_hem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c b/drivers/infiniband/hw/hns/hns_roce_hem.c
index aa8a08d1c014..f30274986c0d 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
@@ -595,11 +595,12 @@ int hns_roce_table_get(struct hns_roce_dev *hr_dev,
 	}
 
 	/* Set HEM base address(128K/page, pa) to Hardware */
-	if (hr_dev->hw->set_hem(hr_dev, table, obj, HEM_HOP_STEP_DIRECT)) {
+	ret = hr_dev->hw->set_hem(hr_dev, table, obj, HEM_HOP_STEP_DIRECT);
+	if (ret) {
 		hns_roce_free_hem(hr_dev, table->hem[i]);
 		table->hem[i] = NULL;
-		ret = -ENODEV;
-		dev_err(dev, "set HEM base address to HW failed.\n");
+		dev_err(dev, "set HEM base address to HW failed, ret = %d.\n",
+			ret);
 		goto out;
 	}
 
-- 
2.30.0

[PATCH for-rc 3/3] RDMA/hns: Add clear_hem return value to log

[Junxian Huang]

From: Chengchang Tang <tangchengchang@huawei.com>

Log return value of clear_hem() to help diagnose.

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
---
 drivers/infiniband/hw/hns/hns_roce_hem.c | 44 ++++++++++++++++--------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c b/drivers/infiniband/hw/hns/hns_roce_hem.c
index f30274986c0d..47c0efed1821 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
@@ -619,6 +619,7 @@ static void clear_mhop_hem(struct hns_roce_dev *hr_dev,
 	u32 hop_num = mhop->hop_num;
 	u32 chunk_ba_num;
 	u32 step_idx;
+	int ret;
 
 	index->inited = HEM_INDEX_BUF;
 	chunk_ba_num = mhop->bt_chunk_size / BA_BYTE_LEN;
@@ -642,16 +643,24 @@ static void clear_mhop_hem(struct hns_roce_dev *hr_dev,
 		else
 			step_idx = hop_num;
 
-		if (hr_dev->hw->clear_hem(hr_dev, table, obj, step_idx))
-			ibdev_warn(ibdev, "failed to clear hop%u HEM.\n", hop_num);
-
-		if (index->inited & HEM_INDEX_L1)
-			if (hr_dev->hw->clear_hem(hr_dev, table, obj, 1))
-				ibdev_warn(ibdev, "failed to clear HEM step 1.\n");
+		ret = hr_dev->hw->clear_hem(hr_dev, table, obj, step_idx);
+		if (ret)
+			ibdev_warn(ibdev, "failed to clear hop%u HEM, ret = %d.\n",
+				   hop_num, ret);
+
+		if (index->inited & HEM_INDEX_L1) {
+			ret = hr_dev->hw->clear_hem(hr_dev, table, obj, 1);
+			if (ret)
+				ibdev_warn(ibdev, "failed to clear HEM step 1, ret = %d.\n",
+					   ret);
+		}
 
-		if (index->inited & HEM_INDEX_L0)
-			if (hr_dev->hw->clear_hem(hr_dev, table, obj, 0))
-				ibdev_warn(ibdev, "failed to clear HEM step 0.\n");
+		if (index->inited & HEM_INDEX_L0) {
+			ret = hr_dev->hw->clear_hem(hr_dev, table, obj, 0);
+			if (ret)
+				ibdev_warn(ibdev, "failed to clear HEM step 0, ret = %d.\n",
+					   ret);
+		}
 	}
 }
 
@@ -688,6 +697,7 @@ void hns_roce_table_put(struct hns_roce_dev *hr_dev,
 {
 	struct device *dev = hr_dev->dev;
 	unsigned long i;
+	int ret;
 
 	if (hns_roce_check_whether_mhop(hr_dev, table->type)) {
 		hns_roce_table_mhop_put(hr_dev, table, obj, 1);
@@ -700,8 +710,10 @@ void hns_roce_table_put(struct hns_roce_dev *hr_dev,
 					 &table->mutex))
 		return;
 
-	if (hr_dev->hw->clear_hem(hr_dev, table, obj, HEM_HOP_STEP_DIRECT))
-		dev_warn(dev, "failed to clear HEM base address.\n");
+	ret = hr_dev->hw->clear_hem(hr_dev, table, obj, HEM_HOP_STEP_DIRECT);
+	if (ret)
+		dev_warn(dev, "failed to clear HEM base address, ret = %d.\n",
+			 ret);
 
 	hns_roce_free_hem(hr_dev, table->hem[i]);
 	table->hem[i] = NULL;
@@ -917,6 +929,8 @@ void hns_roce_cleanup_hem_table(struct hns_roce_dev *hr_dev,
 {
 	struct device *dev = hr_dev->dev;
 	unsigned long i;
+	int obj;
+	int ret;
 
 	if (hns_roce_check_whether_mhop(hr_dev, table->type)) {
 		hns_roce_cleanup_mhop_hem_table(hr_dev, table);
@@ -925,9 +939,11 @@ 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 (hr_dev->hw->clear_hem(hr_dev, table,
-			    i * table->table_chunk_size / table->obj_size, 0))
-				dev_err(dev, "clear HEM base address failed.\n");
+			obj = i * table->table_chunk_size / table->obj_size;
+			ret = hr_dev->hw->clear_hem(hr_dev, table, obj, 0);
+			if (ret)
+				dev_err(dev, "clear HEM base address failed, ret = %d.\n",
+					ret);
 
 			hns_roce_free_hem(hr_dev, table->hem[i]);
 		}
-- 
2.30.0

Re: [PATCH for-rc 0/3] RDMA/hns: Bugfix and improvements

[Jason Gunthorpe]

On Tue, May 23, 2023 at 08:16:38PM +0800, Junxian Huang wrote:
> 1. #1-#2: The first two patches improves diagnostic information to
>    the driver.
> 
> 2. #3: The third patch removes unnecessary QP type checks.
> 
> 
> Chengchang Tang (2):
>   RDMA/hns: Fix hns_roce_table_get return value
>   RDMA/hns: Add clear_hem return value to log
> 
> Junxian Huang (1):
>   RDMA/hns: Remove unnecessary QP type checks

These are not -rc patches, they don't fix user facing things, so I put
them in for-next

Also there was a compile warning I fixed

../drivers/infiniband/hw/hns/hns_roce_hw_v2.c:767:20: warning: unused variable 'ibdev' [-Wunused-variable]
        struct ib_device *ibdev = &hr_dev->ib_dev;

Although it could probably use more fixing.

Jason

Re: [PATCH for-rc 0/3] RDMA/hns: Bugfix and improvements

[Junxian Huang]

On 2023/6/2 7:00, Jason Gunthorpe wrote:
> On Tue, May 23, 2023 at 08:16:38PM +0800, Junxian Huang wrote:
>> 1. #1-#2: The first two patches improves diagnostic information to
>>    the driver.
>>
>> 2. #3: The third patch removes unnecessary QP type checks.
>>
>>
>> Chengchang Tang (2):
>>   RDMA/hns: Fix hns_roce_table_get return value
>>   RDMA/hns: Add clear_hem return value to log
>>
>> Junxian Huang (1):
>>   RDMA/hns: Remove unnecessary QP type checks
> 
> These are not -rc patches, they don't fix user facing things, so I put
> them in for-next
> 
> Also there was a compile warning I fixed
> 
> ../drivers/infiniband/hw/hns/hns_roce_hw_v2.c:767:20: warning: unused variable 'ibdev' [-Wunused-variable]
>         struct ib_device *ibdev = &hr_dev->ib_dev;
> 
> Although it could probably use more fixing.
> 
> Jason

Oops,that's our miss. Thanks!