[PATCH v2 for-next 0/2] Bugfixes for eq support in hns

[PATCH v2 for-next 0/2] Bugfixes for eq support in hns

[Yixian Liu]

This patch set fixes the bugs according to Jason's two comments
on my previous patch set, which listed in the NOTE.

NOTE: This patch set is dependent on the following patches.
   "RDMA/hns: Add eq support of hip08"
   "RDMA/hns: Refactor eq code for hip06"

v1 -> v2:
1. Update according to Leon's comment, see the changelog
   of patch [2/2].

Yixian Liu (2):
  RDMA/hns: Add detailed comments for mb() call
  RDMA/hns: Fix alignment problem of the doorbell

 drivers/infiniband/hw/hns/hns_roce_device.h |  5 ++++
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c  | 19 ++++++++++----
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 40 +++++++++++++++--------------
 3 files changed, 40 insertions(+), 24 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 for-next 1/2] RDMA/hns: Add detailed comments for mb() call

[Yixian Liu]

This patch adds more detailed comments when we call the
memory barrier function, such as rmb, wmb and mb. Three
mb() callers are deleted since they are unnecessary.

Signed-off-by: Yixian Liu <liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 19 ++++++++++++++-----
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 22 +++++++++++-----------
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 6100ace..abcc64e 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -3963,7 +3963,10 @@ static void set_eq_cons_index_v1(struct hns_roce_eq *eq, int req_not)
 {
 	roce_raw_write((eq->cons_index & HNS_ROCE_V1_CONS_IDX_M) |
 		      (req_not << eq->log_entries), eq->doorbell);
-	/* Memory barrier */
+
+	/* Make sure we update the consumer index of EQ before
+	 * accessing it later
+	 */
 	mb();
 }
 
@@ -4156,13 +4159,16 @@ static int hns_roce_v1_aeq_int(struct hns_roce_dev *hr_dev,
 	int event_type;
 
 	while ((aeqe = next_aeqe_sw_v1(eq))) {
+
+		/* Make sure we read the AEQ entry after we have checked the
+		 * ownership bit
+		 */
+		rmb();
+
 		dev_dbg(dev, "aeqe = %p, aeqe->asyn.event_type = 0x%lx\n", aeqe,
 			roce_get_field(aeqe->asyn,
 				       HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
 				       HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S));
-		/* Memory barrier */
-		rmb();
-
 		event_type = roce_get_field(aeqe->asyn,
 					    HNS_ROCE_AEQE_U32_4_EVENT_TYPE_M,
 					    HNS_ROCE_AEQE_U32_4_EVENT_TYPE_S);
@@ -4260,7 +4266,10 @@ static int hns_roce_v1_ceq_int(struct hns_roce_dev *hr_dev,
 	u32 cqn;
 
 	while ((ceqe = next_ceqe_sw_v1(eq))) {
-		/* Memory barrier */
+
+		/* Make sure we read CEQ entry after we have checked the
+		 * ownership bit
+		 */
 		rmb();
 		cqn = roce_get_field(ceqe->comp,
 				     HNS_ROCE_CEQE_CEQE_COMP_CQN_M,
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 04281d0..7f393f6 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -3200,7 +3200,9 @@ static void set_eq_cons_index_v2(struct hns_roce_eq *eq)
 
 	hns_roce_write64_k(doorbell, eq->doorbell);
 
-	/* Memory barrier */
+	/* Make sure we update the consumer index of EQ before
+	 * accessing it later
+	 */
 	mb();
 
 }
@@ -3384,7 +3386,10 @@ static int hns_roce_v2_aeq_int(struct hns_roce_dev *hr_dev,
 	int event_type;
 
 	while ((aeqe = next_aeqe_sw_v2(eq))) {
-		/* Memory barrier */
+
+		/* Make sure we read AEQ entry after we have checked the
+		 * ownership bit
+		 */
 		rmb();
 
 		event_type = roce_get_field(aeqe->asyn,
@@ -3500,8 +3505,11 @@ static int hns_roce_v2_ceq_int(struct hns_roce_dev *hr_dev,
 
 	while ((ceqe = next_ceqe_sw_v2(eq))) {
 
-		/* Memory barrier */
+		/* Make sure we read CEQ entry after we have checked the
+		 * ownership bit
+		 */
 		rmb();
+
 		cqn = roce_get_field(ceqe->comp,
 				     HNS_ROCE_V2_CEQE_COMP_CQN_M,
 				     HNS_ROCE_V2_CEQE_COMP_CQN_S);
@@ -3556,9 +3564,6 @@ static irqreturn_t hns_roce_v2_msix_interrupt_abn(int irq, void *dev_id)
 		roce_set_bit(int_st, HNS_ROCE_V2_VF_INT_ST_AEQ_OVERFLOW_S, 1);
 		roce_write(hr_dev, ROCEE_VF_ABN_INT_ST_REG, int_st);
 
-		/* Memory barrier */
-		mb();
-
 		roce_set_bit(int_en, HNS_ROCE_V2_VF_ABN_INT_EN_S, 1);
 		roce_write(hr_dev, ROCEE_VF_ABN_INT_EN_REG, int_en);
 
@@ -3569,9 +3574,6 @@ static irqreturn_t hns_roce_v2_msix_interrupt_abn(int irq, void *dev_id)
 		roce_set_bit(int_st, HNS_ROCE_V2_VF_INT_ST_BUS_ERR_S, 1);
 		roce_write(hr_dev, ROCEE_VF_ABN_INT_ST_REG, int_st);
 
-		/* Memory barrier */
-		mb();
-
 		roce_set_bit(int_en, HNS_ROCE_V2_VF_ABN_INT_EN_S, 1);
 		roce_write(hr_dev, ROCEE_VF_ABN_INT_EN_REG, int_en);
 
@@ -3582,8 +3584,6 @@ static irqreturn_t hns_roce_v2_msix_interrupt_abn(int irq, void *dev_id)
 		roce_set_bit(int_st, HNS_ROCE_V2_VF_INT_ST_OTHER_ERR_S, 1);
 		roce_write(hr_dev, ROCEE_VF_ABN_INT_ST_REG, int_st);
 
-		/* Memory barrier */
-		mb();
 		roce_set_bit(int_en, HNS_ROCE_V2_VF_ABN_INT_EN_S, 1);
 		roce_write(hr_dev, ROCEE_VF_ABN_INT_EN_REG, int_en);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 for-next 2/2] RDMA/hns: Fix alignment problem of the doorbell

[Yixian Liu]

The array defined for doorbell is not guaranteed to be 64 bit
aligned while we write it to the hardware with 64 bit
alignment required.

This patch fixes this problem by defining a union for doorbell
to make sure it 64 bit alignment.

v2:
1. Consider with Leon's comments, make sure it is 64 bit
   alignment by assigning the expected value to the u64
   member of the union type, i.e., assign doorbell array
   to doorbell64.

Signed-off-by: Yixian Liu <liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/hns/hns_roce_device.h |  5 +++++
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 18 ++++++++++--------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index dde5178..de0534f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -569,6 +569,11 @@ struct hns_roce_eq_table {
 	void __iomem		**eqc_base; /* only for hw v1 */
 };
 
+union hns_roce_db64 {
+	u64	doorbell64;
+	u32	doorbell[2];
+};
+
 struct hns_roce_caps {
 	u8		num_ports;
 	int		gid_table_len[HNS_ROCE_MAX_PORTS];
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 7f393f6..4681220 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -3172,33 +3172,35 @@ static int hns_roce_v2_modify_cq(struct ib_cq *cq, u16 cq_count, u16 cq_period)
 
 static void set_eq_cons_index_v2(struct hns_roce_eq *eq)
 {
-	u32 doorbell[2];
+	union hns_roce_db64 db;
 
-	doorbell[0] = 0;
-	doorbell[1] = 0;
+	db.doorbell[0] = 0;
+	db.doorbell[1] = 0;
 
 	if (eq->type_flag == HNS_ROCE_AEQ) {
-		roce_set_field(doorbell[0], HNS_ROCE_V2_EQ_DB_CMD_M,
+		roce_set_field(db.doorbell[0], HNS_ROCE_V2_EQ_DB_CMD_M,
 			       HNS_ROCE_V2_EQ_DB_CMD_S,
 			       eq->arm_st == HNS_ROCE_V2_EQ_ALWAYS_ARMED ?
 			       HNS_ROCE_EQ_DB_CMD_AEQ :
 			       HNS_ROCE_EQ_DB_CMD_AEQ_ARMED);
 	} else {
-		roce_set_field(doorbell[0], HNS_ROCE_V2_EQ_DB_TAG_M,
+		roce_set_field(db.doorbell[0], HNS_ROCE_V2_EQ_DB_TAG_M,
 			       HNS_ROCE_V2_EQ_DB_TAG_S, eq->eqn);
 
-		roce_set_field(doorbell[0], HNS_ROCE_V2_EQ_DB_CMD_M,
+		roce_set_field(db.doorbell[0], HNS_ROCE_V2_EQ_DB_CMD_M,
 			       HNS_ROCE_V2_EQ_DB_CMD_S,
 			       eq->arm_st == HNS_ROCE_V2_EQ_ALWAYS_ARMED ?
 			       HNS_ROCE_EQ_DB_CMD_CEQ :
 			       HNS_ROCE_EQ_DB_CMD_CEQ_ARMED);
 	}
 
-	roce_set_field(doorbell[1], HNS_ROCE_V2_EQ_DB_PARA_M,
+	roce_set_field(db.doorbell[1], HNS_ROCE_V2_EQ_DB_PARA_M,
 		       HNS_ROCE_V2_EQ_DB_PARA_S,
 		       (eq->cons_index & HNS_ROCE_V2_CONS_IDX_M));
 
-	hns_roce_write64_k(doorbell, eq->doorbell);
+	db.doorbell64 = ((u64) db.doorbell[0] << 32) | db.doorbell[1];
+
+	hns_roce_write64_k(db.doorbell, eq->doorbell);
 
 	/* Make sure we update the consumer index of EQ before
 	 * accessing it later
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 for-next 1/2] RDMA/hns: Add detailed comments for mb() call

[Jason Gunthorpe]

On Wed, Dec 27, 2017 at 07:04:03PM +0800, Yixian Liu wrote:
> This patch adds more detailed comments when we call the
> memory barrier function, such as rmb, wmb and mb. Three
> mb() callers are deleted since they are unnecessary.

Are you sure none of these need to be dma_*mb ?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 for-next 1/2] RDMA/hns: Add detailed comments for mb() call

[Liuyixian (Eason)]

On 2017/12/28 0:45, Jason Gunthorpe wrote:
> On Wed, Dec 27, 2017 at 07:04:03PM +0800, Yixian Liu wrote:
>> This patch adds more detailed comments when we call the
>> memory barrier function, such as rmb, wmb and mb. Three
>> mb() callers are deleted since they are unnecessary.
> 
> Are you sure none of these need to be dma_*mb ?
> 
> Jason
> 

Thanks for your reminder, I will update in patch v3.

> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html