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

public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2 for-next 0/2] Bugfixes for eq support in hns
@ 2017-12-27 11:04 Yixian Liu
       [not found] ` <1514372644-171663-1-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Yixian Liu @ 2017-12-27 11:04 UTC (permalink / raw)
  To: leon-DgEjT+Ai2ygdnm+yROfE0A, jgg-uk2M96/98Pc
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

[parent not found: <1514372644-171663-1-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>]

* [PATCH v2 for-next 1/2] RDMA/hns: Add detailed comments for mb() call
       [not found] ` <1514372644-171663-1-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
@ 2017-12-27 11:04   ` Yixian Liu
       [not found]     ` <1514372644-171663-2-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
  2017-12-27 11:04   ` [PATCH v2 for-next 2/2] RDMA/hns: Fix alignment problem of the doorbell Yixian Liu
  1 sibling, 1 reply; 5+ messages in thread
From: Yixian Liu @ 2017-12-27 11:04 UTC (permalink / raw)
  To: leon-DgEjT+Ai2ygdnm+yROfE0A, jgg-uk2M96/98Pc
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

[parent not found: <1514372644-171663-2-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>]

* Re: [PATCH v2 for-next 1/2] RDMA/hns: Add detailed comments for mb() call
       [not found]     ` <1514372644-171663-2-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
@ 2017-12-27 16:45       ` Jason Gunthorpe
       [not found]         ` <20171227164551.GC25436-uk2M96/98Pc@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2017-12-27 16:45 UTC (permalink / raw)
  To: Yixian Liu
  Cc: leon-DgEjT+Ai2ygdnm+yROfE0A, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

[parent not found: <20171227164551.GC25436-uk2M96/98Pc@public.gmane.org>]

* Re: [PATCH v2 for-next 1/2] RDMA/hns: Add detailed comments for mb() call
       [not found]         ` <20171227164551.GC25436-uk2M96/98Pc@public.gmane.org>
@ 2017-12-28 13:13           ` Liuyixian (Eason)
  0 siblings, 0 replies; 5+ messages in thread
From: Liuyixian (Eason) @ 2017-12-28 13:13 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: leon-DgEjT+Ai2ygdnm+yROfE0A, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA



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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 for-next 2/2] RDMA/hns: Fix alignment problem of the doorbell
       [not found] ` <1514372644-171663-1-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
  2017-12-27 11:04   ` [PATCH v2 for-next 1/2] RDMA/hns: Add detailed comments for mb() call Yixian Liu
@ 2017-12-27 11:04   ` Yixian Liu
  1 sibling, 0 replies; 5+ messages in thread
From: Yixian Liu @ 2017-12-27 11:04 UTC (permalink / raw)
  To: leon-DgEjT+Ai2ygdnm+yROfE0A, jgg-uk2M96/98Pc
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-12-28 13:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-27 11:04 [PATCH v2 for-next 0/2] Bugfixes for eq support in hns Yixian Liu
     [not found] ` <1514372644-171663-1-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2017-12-27 11:04   ` [PATCH v2 for-next 1/2] RDMA/hns: Add detailed comments for mb() call Yixian Liu
     [not found]     ` <1514372644-171663-2-git-send-email-liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2017-12-27 16:45       ` Jason Gunthorpe
     [not found]         ` <20171227164551.GC25436-uk2M96/98Pc@public.gmane.org>
2017-12-28 13:13           ` Liuyixian (Eason)
2017-12-27 11:04   ` [PATCH v2 for-next 2/2] RDMA/hns: Fix alignment problem of the doorbell Yixian Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox