[PATCH v2 for-next 00/17] RDMA/bnxt_re: Control path updates

[PATCH v2 for-next 00/17] RDMA/bnxt_re: Control path updates

[Selvin Xavier]

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

This patch series from Kashyap includes code refactoring and some
optimizations in the FW control path of the driver. It also address
some of the issues seen as we scale up the HW resources.
It also includes few bug fixes in the control path.

Please review and apply.

Thanks,
Selvin Xavier

v1 -> v2:
 - Fix the warning reported by kbuild test robot in patch 10 about
   variables set and not used. Rebased the other patches based on
   the modified patch.
 - Reported-by: kernel test robot <lkp@intel.com>

Kashyap Desai (17):
  RDMA/bnxt_re: wraparound mbox producer index
  RDMA/bnxt_re: Avoid calling wake_up threads from spin_lock context
  RDMA/bnxt_re: remove virt_func check while creating RoCE FW channel
  RDMA/bnxt_re: set fixed command queue depth
  RDMA/bnxt_re: Enhance the existing functions that wait for FW
    responses
  RDMA/bnxt_re: Avoid the command wait if firmware is inactive
  RDMA/bnxt_re: use shadow qd while posting non blocking rcfw command
  RDMA/bnxt_re: Simplify the function that sends the FW commands
  RDMA/bnxt_re: add helper function __poll_for_resp
  RDMA/bnxt_re: handle command completions after driver detect a
    timedout
  RDMA/bnxt_re: Add firmware stall check detection
  RDMA/bnxt_re: post destroy_ah for delayed completion of AH creation
  RDMA/bnxt_re: consider timeout of destroy ah as success.
  RDMA/bnxt_re: cancel all control path command waiters upon error.
  RDMA/bnxt_re: use firmware provided max request timeout
  RDMA/bnxt_re: remove redundant cmdq_bitmap
  RDMA/bnxt_re: optimize the parameters passed to helper functions

 drivers/infiniband/hw/bnxt_re/bnxt_re.h    |   2 +
 drivers/infiniband/hw/bnxt_re/ib_verbs.c   |  16 +-
 drivers/infiniband/hw/bnxt_re/main.c       |  11 +-
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 627 ++++++++++++++++++++++-------
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  51 ++-
 drivers/infiniband/hw/bnxt_re/qplib_res.h  |   1 +
 drivers/infiniband/hw/bnxt_re/qplib_sp.c   |   8 +-
 drivers/infiniband/hw/bnxt_re/qplib_sp.h   |   4 +-
 8 files changed, 557 insertions(+), 163 deletions(-)

-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 01/17] RDMA/bnxt_re: wraparound mbox producer index

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

Driver is not handling the wraparound of the mbox producer index correctly.
Currently the wraparound happens once u32 max is reached.

Bit 31 of the producer index register is special and should be set
only once for the first command. Because the producer index overflow
setting bit31 after a long time, FW goes to initialization sequence
and this causes FW hang.

Fix is to wraparound the mbox producer index once it reaches u16 max.

Fixes: cee0c7bba486 ("RDMA/bnxt_re: Refactor command queue management code")
Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 688eaa0..d4ce82b 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -180,7 +180,7 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	} while (bsize > 0);
 	cmdq->seq_num++;
 
-	cmdq_prod = hwq->prod;
+	cmdq_prod = hwq->prod & 0xFFFF;
 	if (test_bit(FIRMWARE_FIRST_FLAG, &cmdq->flags)) {
 		/* The very first doorbell write
 		 * is required to set this flag
@@ -599,7 +599,7 @@ int bnxt_qplib_alloc_rcfw_channel(struct bnxt_qplib_res *res,
 		rcfw->cmdq_depth = BNXT_QPLIB_CMDQE_MAX_CNT_8192;
 
 	sginfo.pgsize = bnxt_qplib_cmdqe_page_size(rcfw->cmdq_depth);
-	hwq_attr.depth = rcfw->cmdq_depth;
+	hwq_attr.depth = rcfw->cmdq_depth & 0x7FFFFFFF;
 	hwq_attr.stride = BNXT_QPLIB_CMDQE_UNITS;
 	hwq_attr.type = HWQ_TYPE_CTX;
 	if (bnxt_qplib_alloc_init_hwq(&cmdq->hwq, &hwq_attr)) {
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 02/17] RDMA/bnxt_re: Avoid calling wake_up threads from spin_lock context

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

bnxt_qplib_service_creq can be called from interrupt or tasklet or
process context. So the function take irq variant  of spin_lock.
But when wake_up is invoked with the lock held, it is putting the
calling context to sleep.

[exception RIP: __wake_up_common+190]
RIP: ffffffffb7539d7e  RSP: ffffa73300207ad8  RFLAGS: 00000083
RAX: 0000000000000001  RBX: ffff91fa295f69b8  RCX: dead000000000200
RDX: ffffa733344af940  RSI: ffffa73336527940  RDI: ffffa73336527940
RBP: 000000000000001c   R8: 0000000000000002   R9: 00000000000299c0
R10: 0000017230de82c5  R11: 0000000000000002  R12: ffffa73300207b28
R13: 0000000000000000  R14: ffffa733341bf928  R15: 0000000000000000
ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018

Call the wakeup after releasing the lock.

Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index d4ce82b..c11b8e7 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -295,7 +295,8 @@ static int bnxt_qplib_process_func_event(struct bnxt_qplib_rcfw *rcfw,
 }
 
 static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
-				       struct creq_qp_event *qp_event)
+				       struct creq_qp_event *qp_event,
+				       u32 *num_wait)
 {
 	struct creq_qp_error_notification *err_event;
 	struct bnxt_qplib_hwq *hwq = &rcfw->cmdq.hwq;
@@ -304,6 +305,7 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 	u16 cbit, blocked = 0;
 	struct pci_dev *pdev;
 	unsigned long flags;
+	u32 wait_cmds = 0;
 	__le16  mcookie;
 	u16 cookie;
 	int rc = 0;
@@ -363,9 +365,10 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		crsqe->req_size = 0;
 
 		if (!blocked)
-			wake_up(&rcfw->cmdq.waitq);
+			wait_cmds++;
 		spin_unlock_irqrestore(&hwq->lock, flags);
 	}
+	*num_wait += wait_cmds;
 	return rc;
 }
 
@@ -379,6 +382,7 @@ static void bnxt_qplib_service_creq(struct tasklet_struct *t)
 	struct creq_base *creqe;
 	u32 sw_cons, raw_cons;
 	unsigned long flags;
+	u32 num_wakeup = 0;
 
 	/* Service the CREQ until budget is over */
 	spin_lock_irqsave(&hwq->lock, flags);
@@ -397,7 +401,8 @@ static void bnxt_qplib_service_creq(struct tasklet_struct *t)
 		switch (type) {
 		case CREQ_BASE_TYPE_QP_EVENT:
 			bnxt_qplib_process_qp_event
-				(rcfw, (struct creq_qp_event *)creqe);
+				(rcfw, (struct creq_qp_event *)creqe,
+				 &num_wakeup);
 			creq->stats.creq_qp_event_processed++;
 			break;
 		case CREQ_BASE_TYPE_FUNC_EVENT:
@@ -425,6 +430,8 @@ static void bnxt_qplib_service_creq(struct tasklet_struct *t)
 				      rcfw->res->cctx, true);
 	}
 	spin_unlock_irqrestore(&hwq->lock, flags);
+	if (num_wakeup)
+		wake_up_nr(&rcfw->cmdq.waitq, num_wakeup);
 }
 
 static irqreturn_t bnxt_qplib_creq_irq(int irq, void *dev_instance)
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 03/17] RDMA/bnxt_re: remove virt_func check while creating RoCE FW channel

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

There is a common FW communication offset for both PF and VF.
Removed code around virt_fn check while creating FW channel.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c       |  2 +-
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 15 ++++++---------
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  5 ++---
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 4718af6..51372de 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -1202,7 +1202,7 @@ static int bnxt_re_dev_init(struct bnxt_re_dev *rdev, u8 wqe_mode)
 	db_offt = bnxt_re_get_nqdb_offset(rdev, BNXT_RE_AEQ_IDX);
 	vid = rdev->en_dev->msix_entries[BNXT_RE_AEQ_IDX].vector;
 	rc = bnxt_qplib_enable_rcfw_channel(&rdev->rcfw,
-					    vid, db_offt, rdev->is_virtfn,
+					    vid, db_offt,
 					    &bnxt_re_aeq_handler);
 	if (rc) {
 		ibdev_err(&rdev->ibdev, "Failed to enable RCFW channel: %#x\n",
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index c11b8e7..67140eb 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -724,13 +724,11 @@ int bnxt_qplib_rcfw_start_irq(struct bnxt_qplib_rcfw *rcfw, int msix_vector,
 	return 0;
 }
 
-static int bnxt_qplib_map_cmdq_mbox(struct bnxt_qplib_rcfw *rcfw, bool is_vf)
+static int bnxt_qplib_map_cmdq_mbox(struct bnxt_qplib_rcfw *rcfw)
 {
 	struct bnxt_qplib_cmdq_mbox *mbox;
 	resource_size_t bar_reg;
 	struct pci_dev *pdev;
-	u16 prod_offt;
-	int rc = 0;
 
 	pdev = rcfw->pdev;
 	mbox = &rcfw->cmdq.cmdq_mbox;
@@ -755,11 +753,10 @@ static int bnxt_qplib_map_cmdq_mbox(struct bnxt_qplib_rcfw *rcfw, bool is_vf)
 		return -ENOMEM;
 	}
 
-	prod_offt = is_vf ? RCFW_VF_COMM_PROD_OFFSET :
-			    RCFW_PF_COMM_PROD_OFFSET;
-	mbox->prod = (void  __iomem *)(mbox->reg.bar_reg + prod_offt);
+	mbox->prod = (void  __iomem *)(mbox->reg.bar_reg +
+			RCFW_PF_VF_COMM_PROD_OFFSET);
 	mbox->db = (void __iomem *)(mbox->reg.bar_reg + RCFW_COMM_TRIG_OFFSET);
-	return rc;
+	return 0;
 }
 
 static int bnxt_qplib_map_creq_db(struct bnxt_qplib_rcfw *rcfw, u32 reg_offt)
@@ -820,7 +817,7 @@ static void bnxt_qplib_start_rcfw(struct bnxt_qplib_rcfw *rcfw)
 
 int bnxt_qplib_enable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw,
 				   int msix_vector,
-				   int cp_bar_reg_off, int virt_fn,
+				   int cp_bar_reg_off,
 				   aeq_handler_t aeq_handler)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
@@ -840,7 +837,7 @@ int bnxt_qplib_enable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw,
 	creq->stats.creq_func_event_processed = 0;
 	creq->aeq_handler = aeq_handler;
 
-	rc = bnxt_qplib_map_cmdq_mbox(rcfw, virt_fn);
+	rc = bnxt_qplib_map_cmdq_mbox(rcfw);
 	if (rc)
 		return rc;
 
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 92f7a25..0dff56e 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -45,8 +45,7 @@
 #define RCFW_COMM_PCI_BAR_REGION	0
 #define RCFW_COMM_CONS_PCI_BAR_REGION	2
 #define RCFW_COMM_BASE_OFFSET		0x600
-#define RCFW_PF_COMM_PROD_OFFSET	0xc
-#define RCFW_VF_COMM_PROD_OFFSET	0xc
+#define RCFW_PF_VF_COMM_PROD_OFFSET	0xc
 #define RCFW_COMM_TRIG_OFFSET		0x100
 #define RCFW_COMM_SIZE			0x104
 
@@ -235,7 +234,7 @@ int bnxt_qplib_rcfw_start_irq(struct bnxt_qplib_rcfw *rcfw, int msix_vector,
 			      bool need_init);
 int bnxt_qplib_enable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw,
 				   int msix_vector,
-				   int cp_bar_reg_off, int virt_fn,
+				   int cp_bar_reg_off,
 				   aeq_handler_t aeq_handler);
 
 struct bnxt_qplib_rcfw_sbuf *bnxt_qplib_rcfw_alloc_sbuf(
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 04/17] RDMA/bnxt_re: set fixed command queue depth

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

There is no need of setting max command queue entries based on
firmware version check.

Removing deperecated code.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 6 ++----
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h | 7 ++-----
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 67140eb..66121fb 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -600,10 +600,8 @@ int bnxt_qplib_alloc_rcfw_channel(struct bnxt_qplib_res *res,
 			"HW channel CREQ allocation failed\n");
 		goto fail;
 	}
-	if (ctx->hwrm_intf_ver < HWRM_VERSION_RCFW_CMDQ_DEPTH_CHECK)
-		rcfw->cmdq_depth = BNXT_QPLIB_CMDQE_MAX_CNT_256;
-	else
-		rcfw->cmdq_depth = BNXT_QPLIB_CMDQE_MAX_CNT_8192;
+
+	rcfw->cmdq_depth = BNXT_QPLIB_CMDQE_MAX_CNT;
 
 	sginfo.pgsize = bnxt_qplib_cmdqe_page_size(rcfw->cmdq_depth);
 	hwq_attr.depth = rcfw->cmdq_depth & 0x7FFFFFFF;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 0dff56e..32e5b67 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -69,8 +69,7 @@ static inline void bnxt_qplib_rcfw_cmd_prep(struct cmdq_base *req,
 #define RCFW_CMD_WAIT_TIME_MS		20000 /* 20 Seconds timeout */
 
 /* CMDQ elements */
-#define BNXT_QPLIB_CMDQE_MAX_CNT_256	256
-#define BNXT_QPLIB_CMDQE_MAX_CNT_8192	8192
+#define BNXT_QPLIB_CMDQE_MAX_CNT	8192
 #define BNXT_QPLIB_CMDQE_BYTES(depth)	((depth) * BNXT_QPLIB_CMDQE_UNITS)
 
 static inline u32 bnxt_qplib_cmdqe_npages(u32 depth)
@@ -105,12 +104,10 @@ static inline u32 bnxt_qplib_set_cmd_slots(struct cmdq_base *req)
 	return cmd_byte;
 }
 
-#define RCFW_MAX_COOKIE_VALUE		0x7FFF
+#define RCFW_MAX_COOKIE_VALUE		(BNXT_QPLIB_CMDQE_MAX_CNT - 1)
 #define RCFW_CMD_IS_BLOCKING		0x8000
 #define RCFW_BLOCKED_CMD_WAIT_COUNT	20000000UL /* 20 sec */
 
-#define HWRM_VERSION_RCFW_CMDQ_DEPTH_CHECK 0x1000900020011ULL
-
 /* Crsq buf is 1024-Byte */
 struct bnxt_qplib_crsbe {
 	u8			data[1024];
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 05/17] RDMA/bnxt_re: Enhance the existing functions that wait for FW responses

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

Use jiffies based timewait instead of counting iteration for
commands that block for FW response.

Also add a poll routine for control path commands. This is for
polling completion if the waiting commands timeout. This avoids cases
where the driver misses completion interrupts.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 65 +++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 14 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 66121fb..3b242cc 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -53,37 +53,74 @@
 
 static void bnxt_qplib_service_creq(struct tasklet_struct *t);
 
-/* Hardware communication channel */
+/**
+ * __wait_for_resp   -	Don't hold the cpu context and wait for response
+ * @rcfw      -   rcfw channel instance of rdev
+ * @cookie    -   cookie to track the command
+ *
+ * Wait for command completion in sleepable context.
+ *
+ * Returns:
+ * 0 if command is completed by firmware.
+ * Non zero error code for rest of the case.
+ */
 static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
 	u16 cbit;
-	int rc;
+	int ret;
 
 	cmdq = &rcfw->cmdq;
 	cbit = cookie % rcfw->cmdq_depth;
-	rc = wait_event_timeout(cmdq->waitq,
-				!test_bit(cbit, cmdq->cmdq_bitmap),
-				msecs_to_jiffies(RCFW_CMD_WAIT_TIME_MS));
-	return rc ? 0 : -ETIMEDOUT;
+
+	do {
+		/* Non zero means command completed */
+		ret = wait_event_timeout(cmdq->waitq,
+					 !test_bit(cbit, cmdq->cmdq_bitmap),
+					 msecs_to_jiffies(10000));
+
+		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+			return 0;
+
+		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
+
+		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+			return 0;
+
+	} while (true);
 };
 
+/**
+ * __block_for_resp   -	hold the cpu context and wait for response
+ * @rcfw      -   rcfw channel instance of rdev
+ * @cookie    -   cookie to track the command
+ *
+ * This function will hold the cpu (non-sleepable context) and
+ * wait for command completion. Maximum holding interval is 8 second.
+ *
+ * Returns:
+ * -ETIMEOUT if command is not completed in specific time interval.
+ * 0 if command is completed by firmware.
+ */
 static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 {
-	u32 count = RCFW_BLOCKED_CMD_WAIT_COUNT;
-	struct bnxt_qplib_cmdq_ctx *cmdq;
+	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
+	unsigned long issue_time = 0;
 	u16 cbit;
 
-	cmdq = &rcfw->cmdq;
 	cbit = cookie % rcfw->cmdq_depth;
-	if (!test_bit(cbit, cmdq->cmdq_bitmap))
-		goto done;
+	issue_time = jiffies;
+
 	do {
 		udelay(1);
+
 		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
-	} while (test_bit(cbit, cmdq->cmdq_bitmap) && --count);
-done:
-	return count ? 0 : -ETIMEDOUT;
+		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+			return 0;
+
+	} while (time_before(jiffies, issue_time + (8 * HZ)));
+
+	return -ETIMEDOUT;
 };
 
 static int __send_message(struct bnxt_qplib_rcfw *rcfw,
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 06/17] RDMA/bnxt_re: Avoid the command wait if firmware is inactive

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

Add a check to avoid waiting if driver already detects a
FW timeout. Return success for resource destroy in case
the device is detached. Add helper function to map timeout
error code to success.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 52 +++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 3b242cc..3c4f72a 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -54,9 +54,46 @@
 static void bnxt_qplib_service_creq(struct tasklet_struct *t);
 
 /**
+ * bnxt_qplib_map_rc  -  map return type based on opcode
+ * @opcode    -  roce slow path opcode
+ *
+ * In some cases like firmware halt is detected, the driver is supposed to
+ * remap the error code of the timed out command.
+ *
+ * It is not safe to assume hardware is really inactive so certain opcodes
+ * like destroy qp etc are not safe to be returned success, but this function
+ * will be called when FW already reports a timeout. This would be possible
+ * only when FW crashes and resets. This will clear all the HW resources.
+ *
+ * Returns:
+ * 0 to communicate success to caller.
+ * Non zero error code to communicate failure to caller.
+ */
+static int bnxt_qplib_map_rc(u8 opcode)
+{
+	switch (opcode) {
+	case CMDQ_BASE_OPCODE_DESTROY_QP:
+	case CMDQ_BASE_OPCODE_DESTROY_SRQ:
+	case CMDQ_BASE_OPCODE_DESTROY_CQ:
+	case CMDQ_BASE_OPCODE_DEALLOCATE_KEY:
+	case CMDQ_BASE_OPCODE_DEREGISTER_MR:
+	case CMDQ_BASE_OPCODE_DELETE_GID:
+	case CMDQ_BASE_OPCODE_DESTROY_QP1:
+	case CMDQ_BASE_OPCODE_DESTROY_AH:
+	case CMDQ_BASE_OPCODE_DEINITIALIZE_FW:
+	case CMDQ_BASE_OPCODE_MODIFY_ROCE_CC:
+	case CMDQ_BASE_OPCODE_SET_LINK_AGGR_MODE:
+		return 0;
+	default:
+		return -ETIMEDOUT;
+	}
+}
+
+/**
  * __wait_for_resp   -	Don't hold the cpu context and wait for response
  * @rcfw      -   rcfw channel instance of rdev
  * @cookie    -   cookie to track the command
+ * @opcode    -   rcfw submitted for given opcode
  *
  * Wait for command completion in sleepable context.
  *
@@ -64,7 +101,7 @@ static void bnxt_qplib_service_creq(struct tasklet_struct *t);
  * 0 if command is completed by firmware.
  * Non zero error code for rest of the case.
  */
-static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
+static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
 	u16 cbit;
@@ -74,6 +111,9 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 	cbit = cookie % rcfw->cmdq_depth;
 
 	do {
+		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
+			return bnxt_qplib_map_rc(opcode);
+
 		/* Non zero means command completed */
 		ret = wait_event_timeout(cmdq->waitq,
 					 !test_bit(cbit, cmdq->cmdq_bitmap),
@@ -94,6 +134,7 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
  * __block_for_resp   -	hold the cpu context and wait for response
  * @rcfw      -   rcfw channel instance of rdev
  * @cookie    -   cookie to track the command
+ * @opcode    -   rcfw submitted for given opcode
  *
  * This function will hold the cpu (non-sleepable context) and
  * wait for command completion. Maximum holding interval is 8 second.
@@ -102,7 +143,7 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
  * -ETIMEOUT if command is not completed in specific time interval.
  * 0 if command is completed by firmware.
  */
-static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
+static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
 	unsigned long issue_time = 0;
@@ -112,6 +153,9 @@ static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 	issue_time = jiffies;
 
 	do {
+		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
+			return bnxt_qplib_map_rc(opcode);
+
 		udelay(1);
 
 		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
@@ -267,9 +311,9 @@ int bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 	} while (retry_cnt--);
 
 	if (msg->block)
-		rc = __block_for_resp(rcfw, cookie);
+		rc = __block_for_resp(rcfw, cookie, opcode);
 	else
-		rc = __wait_for_resp(rcfw, cookie);
+		rc = __wait_for_resp(rcfw, cookie, opcode);
 	if (rc) {
 		/* timed out */
 		dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 07/17] RDMA/bnxt_re: use shadow qd while posting non blocking rcfw command

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

Whenever there is a fast path IO and create/destroy resources from
the slow path is happening in parallel, we may notice high latency
of slow path command completion.

Introduces a shadow queue depth to prevent the outstanding requests
to the FW. Driver will not allow more than #RCFW_CMD_NON_BLOCKING_SHADOW_QD
non-blocking commands to the Firmware.

Shadow queue depth is a soft limit only for non-blocking
commands. Blocking commands will be posted to the firmware
as long as there is a free slot.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 60 +++++++++++++++++++++++++++++-
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  3 ++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 3c4f72a..f7d1238 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -281,8 +281,21 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	return 0;
 }
 
-int bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
-				 struct bnxt_qplib_cmdqmsg *msg)
+/**
+ * __bnxt_qplib_rcfw_send_message   -	qplib interface to send
+ * and complete rcfw command.
+ * @rcfw      -   rcfw channel instance of rdev
+ * @msg      -    qplib message internal
+ *
+ * This function does not account shadow queue depth. It will send
+ * all the command unconditionally as long as send queue is not full.
+ *
+ * Returns:
+ * 0 if command completed by firmware.
+ * Non zero if the command is not completed by firmware.
+ */
+static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
+					  struct bnxt_qplib_cmdqmsg *msg)
 {
 	struct creq_qp_event *evnt = (struct creq_qp_event *)msg->resp;
 	u16 cookie;
@@ -331,6 +344,48 @@ int bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 
 	return rc;
 }
+
+/**
+ * bnxt_qplib_rcfw_send_message   -	qplib interface to send
+ * and complete rcfw command.
+ * @rcfw      -   rcfw channel instance of rdev
+ * @msg      -    qplib message internal
+ *
+ * Driver interact with Firmware through rcfw channel/slow path in two ways.
+ * a. Blocking rcfw command send. In this path, driver cannot hold
+ * the context for longer period since it is holding cpu until
+ * command is not completed.
+ * b. Non-blocking rcfw command send. In this path, driver can hold the
+ * context for longer period. There may be many pending command waiting
+ * for completion because of non-blocking nature.
+ *
+ * Driver will use shadow queue depth. Current queue depth of 8K
+ * (due to size of rcfw message there can be actual ~4K rcfw outstanding)
+ * is not optimal for rcfw command processing in firmware.
+ *
+ * Restrict at max #RCFW_CMD_NON_BLOCKING_SHADOW_QD Non-Blocking rcfw commands.
+ * Allow all blocking commands until there is no queue full.
+ *
+ * Returns:
+ * 0 if command completed by firmware.
+ * Non zero if the command is not completed by firmware.
+ */
+int bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
+				 struct bnxt_qplib_cmdqmsg *msg)
+{
+	int ret;
+
+	if (!msg->block) {
+		down(&rcfw->rcfw_inflight);
+		ret = __bnxt_qplib_rcfw_send_message(rcfw, msg);
+		up(&rcfw->rcfw_inflight);
+	} else {
+		ret = __bnxt_qplib_rcfw_send_message(rcfw, msg);
+	}
+
+	return ret;
+}
+
 /* Completions */
 static int bnxt_qplib_process_func_event(struct bnxt_qplib_rcfw *rcfw,
 					 struct creq_func_event *func_event)
@@ -932,6 +987,7 @@ int bnxt_qplib_enable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw,
 		return rc;
 	}
 
+	sema_init(&rcfw->rcfw_inflight, RCFW_CMD_NON_BLOCKING_SHADOW_QD);
 	bnxt_qplib_start_rcfw(rcfw);
 
 	return 0;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 32e5b67..862bfbf 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -66,6 +66,8 @@ static inline void bnxt_qplib_rcfw_cmd_prep(struct cmdq_base *req,
 	req->cmd_size = cmd_size;
 }
 
+/* Shadow queue depth for non blocking command */
+#define RCFW_CMD_NON_BLOCKING_SHADOW_QD	64
 #define RCFW_CMD_WAIT_TIME_MS		20000 /* 20 Seconds timeout */
 
 /* CMDQ elements */
@@ -197,6 +199,7 @@ struct bnxt_qplib_rcfw {
 	u64 oos_prev;
 	u32 init_oos_stats;
 	u32 cmdq_depth;
+	struct semaphore rcfw_inflight;
 };
 
 struct bnxt_qplib_cmdqmsg {
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 08/17] RDMA/bnxt_re: Simplify the function that sends the FW commands

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

 - Use __send_message_basic_sanity helper function.
 - Do not retry posting same command if there is a queue full detection.
 - ENXIO is used to indicate controller recovery.
 - In the case of ERR_DEVICE_DETACHED state, the driver should not post
   commands to the firmware, but also return fabricated written code.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 125 +++++++++++++++--------------
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  22 +++++
 2 files changed, 86 insertions(+), 61 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index f7d1238..15f6793 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -170,34 +170,22 @@ static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 			  struct bnxt_qplib_cmdqmsg *msg)
 {
-	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
-	struct bnxt_qplib_hwq *hwq = &cmdq->hwq;
+	u32 bsize, opcode, free_slots, required_slots;
+	struct bnxt_qplib_cmdq_ctx *cmdq;
 	struct bnxt_qplib_crsqe *crsqe;
 	struct bnxt_qplib_cmdqe *cmdqe;
+	struct bnxt_qplib_hwq *hwq;
 	u32 sw_prod, cmdq_prod;
 	struct pci_dev *pdev;
 	unsigned long flags;
-	u32 bsize, opcode;
 	u16 cookie, cbit;
 	u8 *preq;
 
+	cmdq = &rcfw->cmdq;
+	hwq = &cmdq->hwq;
 	pdev = rcfw->pdev;
 
 	opcode = __get_cmdq_base_opcode(msg->req, msg->req_sz);
-	if (!test_bit(FIRMWARE_INITIALIZED_FLAG, &cmdq->flags) &&
-	    (opcode != CMDQ_BASE_OPCODE_QUERY_FUNC &&
-	     opcode != CMDQ_BASE_OPCODE_INITIALIZE_FW &&
-	     opcode != CMDQ_BASE_OPCODE_QUERY_VERSION)) {
-		dev_err(&pdev->dev,
-			"RCFW not initialized, reject opcode 0x%x\n", opcode);
-		return -EINVAL;
-	}
-
-	if (test_bit(FIRMWARE_INITIALIZED_FLAG, &cmdq->flags) &&
-	    opcode == CMDQ_BASE_OPCODE_INITIALIZE_FW) {
-		dev_err(&pdev->dev, "RCFW already initialized!\n");
-		return -EINVAL;
-	}
 
 	if (test_bit(FIRMWARE_TIMED_OUT, &cmdq->flags))
 		return -ETIMEDOUT;
@@ -206,40 +194,37 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	 * cmdqe
 	 */
 	spin_lock_irqsave(&hwq->lock, flags);
-	if (msg->req->cmd_size >= HWQ_FREE_SLOTS(hwq)) {
-		dev_err(&pdev->dev, "RCFW: CMDQ is full!\n");
+	required_slots = bnxt_qplib_get_cmd_slots(msg->req);
+	free_slots = HWQ_FREE_SLOTS(hwq);
+	cookie = cmdq->seq_num & RCFW_MAX_COOKIE_VALUE;
+	cbit = cookie % rcfw->cmdq_depth;
+
+	if (required_slots >= free_slots ||
+	    test_bit(cbit, cmdq->cmdq_bitmap)) {
+		dev_info_ratelimited(&pdev->dev,
+				     "CMDQ is full req/free %d/%d!",
+				     required_slots, free_slots);
 		spin_unlock_irqrestore(&hwq->lock, flags);
 		return -EAGAIN;
 	}
-
-
-	cookie = cmdq->seq_num & RCFW_MAX_COOKIE_VALUE;
-	cbit = cookie % rcfw->cmdq_depth;
 	if (msg->block)
 		cookie |= RCFW_CMD_IS_BLOCKING;
-
 	set_bit(cbit, cmdq->cmdq_bitmap);
 	__set_cmdq_base_cookie(msg->req, msg->req_sz, cpu_to_le16(cookie));
 	crsqe = &rcfw->crsqe_tbl[cbit];
-	if (crsqe->resp) {
-		spin_unlock_irqrestore(&hwq->lock, flags);
-		return -EBUSY;
-	}
-
-	/* change the cmd_size to the number of 16byte cmdq unit.
-	 * req->cmd_size is modified here
-	 */
 	bsize = bnxt_qplib_set_cmd_slots(msg->req);
-
-	memset(msg->resp, 0, sizeof(*msg->resp));
+	crsqe->free_slots = free_slots;
 	crsqe->resp = (struct creq_qp_event *)msg->resp;
 	crsqe->resp->cookie = cpu_to_le16(cookie);
 	crsqe->req_size = __get_cmdq_base_cmd_size(msg->req, msg->req_sz);
 	if (__get_cmdq_base_resp_size(msg->req, msg->req_sz) && msg->sb) {
 		struct bnxt_qplib_rcfw_sbuf *sbuf = msg->sb;
-		__set_cmdq_base_resp_addr(msg->req, msg->req_sz, cpu_to_le64(sbuf->dma_addr));
+
+		__set_cmdq_base_resp_addr(msg->req, msg->req_sz,
+					  cpu_to_le64(sbuf->dma_addr));
 		__set_cmdq_base_resp_size(msg->req, msg->req_sz,
-					  ALIGN(sbuf->size, BNXT_QPLIB_CMDQE_UNITS));
+					  ALIGN(sbuf->size,
+						BNXT_QPLIB_CMDQE_UNITS));
 	}
 
 	preq = (u8 *)msg->req;
@@ -247,11 +232,6 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 		/* Locate the next cmdq slot */
 		sw_prod = HWQ_CMP(hwq->prod, hwq);
 		cmdqe = bnxt_qplib_get_qe(hwq, sw_prod, NULL);
-		if (!cmdqe) {
-			dev_err(&pdev->dev,
-				"RCFW request failed with no cmdqe!\n");
-			goto done;
-		}
 		/* Copy a segment of the req cmd to the cmdq */
 		memset(cmdqe, 0, sizeof(*cmdqe));
 		memcpy(cmdqe, preq, min_t(u32, bsize, sizeof(*cmdqe)));
@@ -275,12 +255,43 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	wmb();
 	writel(cmdq_prod, cmdq->cmdq_mbox.prod);
 	writel(RCFW_CMDQ_TRIG_VAL, cmdq->cmdq_mbox.db);
-done:
 	spin_unlock_irqrestore(&hwq->lock, flags);
 	/* Return the CREQ response pointer */
 	return 0;
 }
 
+static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
+				       struct bnxt_qplib_cmdqmsg *msg)
+{
+	struct bnxt_qplib_cmdq_ctx *cmdq;
+	u32 opcode;
+
+	cmdq = &rcfw->cmdq;
+	opcode = __get_cmdq_base_opcode(msg->req, msg->req_sz);
+
+	/* Prevent posting if f/w is not in a state to process */
+	if (test_bit(ERR_DEVICE_DETACHED, &rcfw->cmdq.flags))
+		return -ENXIO;
+
+	if (test_bit(FIRMWARE_INITIALIZED_FLAG, &cmdq->flags) &&
+	    opcode == CMDQ_BASE_OPCODE_INITIALIZE_FW) {
+		dev_err(&rcfw->pdev->dev, "QPLIB: RCFW already initialized!");
+		return -EINVAL;
+	}
+
+	if (!test_bit(FIRMWARE_INITIALIZED_FLAG, &cmdq->flags) &&
+	    (opcode != CMDQ_BASE_OPCODE_QUERY_FUNC &&
+	     opcode != CMDQ_BASE_OPCODE_INITIALIZE_FW &&
+	     opcode != CMDQ_BASE_OPCODE_QUERY_VERSION)) {
+		dev_err(&rcfw->pdev->dev,
+			"QPLIB: RCFW not initialized, reject opcode 0x%x",
+			opcode);
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 /**
  * __bnxt_qplib_rcfw_send_message   -	qplib interface to send
  * and complete rcfw command.
@@ -299,29 +310,21 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 {
 	struct creq_qp_event *evnt = (struct creq_qp_event *)msg->resp;
 	u16 cookie;
-	u8 opcode, retry_cnt = 0xFF;
 	int rc = 0;
+	u8 opcode;
 
-	/* Prevent posting if f/w is not in a state to process */
-	if (test_bit(ERR_DEVICE_DETACHED, &rcfw->cmdq.flags))
-		return 0;
+	opcode = __get_cmdq_base_opcode(msg->req, msg->req_sz);
 
-	do {
-		opcode = __get_cmdq_base_opcode(msg->req, msg->req_sz);
-		rc = __send_message(rcfw, msg);
-		cookie = le16_to_cpu(__get_cmdq_base_cookie(msg->req, msg->req_sz)) &
-				RCFW_MAX_COOKIE_VALUE;
-		if (!rc)
-			break;
-		if (!retry_cnt || (rc != -EAGAIN && rc != -EBUSY)) {
-			/* send failed */
-			dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x send failed\n",
-				cookie, opcode);
-			return rc;
-		}
-		msg->block ? mdelay(1) : usleep_range(500, 1000);
+	rc = __send_message_basic_sanity(rcfw, msg);
+	if (rc)
+		return rc == -ENXIO ? bnxt_qplib_map_rc(opcode) : rc;
+
+	rc = __send_message(rcfw, msg);
+	if (rc)
+		return rc;
 
-	} while (retry_cnt--);
+	cookie = le16_to_cpu(__get_cmdq_base_cookie(msg->req, msg->req_sz))
+				& RCFW_MAX_COOKIE_VALUE;
 
 	if (msg->block)
 		rc = __block_for_resp(rcfw, cookie, opcode);
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 862bfbf..b7bbbae 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -89,6 +89,26 @@ static inline u32 bnxt_qplib_cmdqe_page_size(u32 depth)
 	return (bnxt_qplib_cmdqe_npages(depth) * PAGE_SIZE);
 }
 
+/* Get the number of command units required for the req. The
+ * function returns correct value only if called before
+ * setting using bnxt_qplib_set_cmd_slots
+ */
+static inline u32 bnxt_qplib_get_cmd_slots(struct cmdq_base *req)
+{
+	u32 cmd_units = 0;
+
+	if (HAS_TLV_HEADER(req)) {
+		struct roce_tlv *tlv_req = (struct roce_tlv *)req;
+
+		cmd_units = tlv_req->total_size;
+	} else {
+		cmd_units = (req->cmd_size + BNXT_QPLIB_CMDQE_UNITS - 1) /
+			    BNXT_QPLIB_CMDQE_UNITS;
+	}
+
+	return cmd_units;
+}
+
 static inline u32 bnxt_qplib_set_cmd_slots(struct cmdq_base *req)
 {
 	u32 cmd_byte = 0;
@@ -130,6 +150,8 @@ typedef int (*aeq_handler_t)(struct bnxt_qplib_rcfw *, void *, void *);
 struct bnxt_qplib_crsqe {
 	struct creq_qp_event	*resp;
 	u32			req_size;
+	/* Free slots at the time of submission */
+	u32			free_slots;
 };
 
 struct bnxt_qplib_rcfw_sbuf {
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 09/17] RDMA/bnxt_re: add helper function __poll_for_resp

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

This interface will be used if the driver has not enabled interrupt
and/or interrupt is disabled for a short period of time.
Completion is not possible from interrupt so this interface does
self-polling.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 44 +++++++++++++++++++++++++++++-
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  1 +
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 15f6793..3215f8a 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -260,6 +260,44 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	return 0;
 }
 
+/**
+ * __poll_for_resp   -	self poll completion for rcfw command
+ * @rcfw      -   rcfw channel instance of rdev
+ * @cookie    -   cookie to track the command
+ * @opcode    -   rcfw submitted for given opcode
+ *
+ * It works same as __wait_for_resp except this function will
+ * do self polling in sort interval since interrupt is disabled.
+ * This function can not be called from non-sleepable context.
+ *
+ * Returns:
+ * -ETIMEOUT if command is not completed in specific time interval.
+ * 0 if command is completed by firmware.
+ */
+static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
+			   u8 opcode)
+{
+	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
+	unsigned long issue_time;
+	u16 cbit;
+
+	cbit = cookie % rcfw->cmdq_depth;
+	issue_time = jiffies;
+
+	do {
+		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
+			return bnxt_qplib_map_rc(opcode);
+
+		usleep_range(1000, 1001);
+
+		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
+		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+			return 0;
+		if (jiffies_to_msecs(jiffies - issue_time) > 10000)
+			return -ETIMEDOUT;
+	} while (true);
+};
+
 static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
 				       struct bnxt_qplib_cmdqmsg *msg)
 {
@@ -328,8 +366,10 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 
 	if (msg->block)
 		rc = __block_for_resp(rcfw, cookie, opcode);
-	else
+	else if (atomic_read(&rcfw->rcfw_intr_enabled))
 		rc = __wait_for_resp(rcfw, cookie, opcode);
+	else
+		rc = __poll_for_resp(rcfw, cookie, opcode);
 	if (rc) {
 		/* timed out */
 		dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
@@ -796,6 +836,7 @@ void bnxt_qplib_rcfw_stop_irq(struct bnxt_qplib_rcfw *rcfw, bool kill)
 	kfree(creq->irq_name);
 	creq->irq_name = NULL;
 	creq->requested = false;
+	atomic_set(&rcfw->rcfw_intr_enabled, 0);
 }
 
 void bnxt_qplib_disable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw)
@@ -857,6 +898,7 @@ int bnxt_qplib_rcfw_start_irq(struct bnxt_qplib_rcfw *rcfw, int msix_vector,
 	creq->requested = true;
 
 	bnxt_qplib_ring_nq_db(&creq->creq_db.dbinfo, res->cctx, true);
+	atomic_inc(&rcfw->rcfw_intr_enabled);
 
 	return 0;
 }
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index b7bbbae..089e616 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -221,6 +221,7 @@ struct bnxt_qplib_rcfw {
 	u64 oos_prev;
 	u32 init_oos_stats;
 	u32 cmdq_depth;
+	atomic_t rcfw_intr_enabled;
 	struct semaphore rcfw_inflight;
 };
 
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 10/17] RDMA/bnxt_re: handle command completions after driver detect a timedout

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

If calling context detect command timeout, associated memory stored on
stack will not be valid. If firmware complete the same command later,
this causes incorrect memory access by driver.

Added is_waiter_alive to handle delayed completion by firmware.
is_waiter_alive is set and reset under command queue lock.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 59 +++++++++++++++++-------------
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  1 +
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 3215f8a..4e5f66e 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -105,7 +105,6 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
 	u16 cbit;
-	int ret;
 
 	cmdq = &rcfw->cmdq;
 	cbit = cookie % rcfw->cmdq_depth;
@@ -115,9 +114,9 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 			return bnxt_qplib_map_rc(opcode);
 
 		/* Non zero means command completed */
-		ret = wait_event_timeout(cmdq->waitq,
-					 !test_bit(cbit, cmdq->cmdq_bitmap),
-					 msecs_to_jiffies(10000));
+		wait_event_timeout(cmdq->waitq,
+				   !test_bit(cbit, cmdq->cmdq_bitmap),
+				   msecs_to_jiffies(10000));
 
 		if (!test_bit(cbit, cmdq->cmdq_bitmap))
 			return 0;
@@ -170,7 +169,7 @@ static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 			  struct bnxt_qplib_cmdqmsg *msg)
 {
-	u32 bsize, opcode, free_slots, required_slots;
+	u32 bsize, free_slots, required_slots;
 	struct bnxt_qplib_cmdq_ctx *cmdq;
 	struct bnxt_qplib_crsqe *crsqe;
 	struct bnxt_qplib_cmdqe *cmdqe;
@@ -185,8 +184,6 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	hwq = &cmdq->hwq;
 	pdev = rcfw->pdev;
 
-	opcode = __get_cmdq_base_opcode(msg->req, msg->req_sz);
-
 	if (test_bit(FIRMWARE_TIMED_OUT, &cmdq->flags))
 		return -ETIMEDOUT;
 
@@ -216,6 +213,7 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	crsqe->free_slots = free_slots;
 	crsqe->resp = (struct creq_qp_event *)msg->resp;
 	crsqe->resp->cookie = cpu_to_le16(cookie);
+	crsqe->is_waiter_alive = true;
 	crsqe->req_size = __get_cmdq_base_cmd_size(msg->req, msg->req_sz);
 	if (__get_cmdq_base_resp_size(msg->req, msg->req_sz) && msg->sb) {
 		struct bnxt_qplib_rcfw_sbuf *sbuf = msg->sb;
@@ -347,7 +345,9 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 					  struct bnxt_qplib_cmdqmsg *msg)
 {
 	struct creq_qp_event *evnt = (struct creq_qp_event *)msg->resp;
-	u16 cookie;
+	struct bnxt_qplib_crsqe *crsqe;
+	unsigned long flags;
+	u16 cookie, cbit;
 	int rc = 0;
 	u8 opcode;
 
@@ -363,6 +363,7 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 
 	cookie = le16_to_cpu(__get_cmdq_base_cookie(msg->req, msg->req_sz))
 				& RCFW_MAX_COOKIE_VALUE;
+	cbit = cookie % rcfw->cmdq_depth;
 
 	if (msg->block)
 		rc = __block_for_resp(rcfw, cookie, opcode);
@@ -378,6 +379,14 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 		return rc;
 	}
 
+	if (rc) {
+		spin_lock_irqsave(&rcfw->cmdq.hwq.lock, flags);
+		crsqe = &rcfw->crsqe_tbl[cbit];
+		crsqe->is_waiter_alive = false;
+		spin_unlock_irqrestore(&rcfw->cmdq.hwq.lock, flags);
+		return -ETIMEDOUT;
+	}
+
 	if (evnt->status) {
 		/* failed with status */
 		dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x status %#x\n",
@@ -480,15 +489,15 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 	struct creq_qp_error_notification *err_event;
 	struct bnxt_qplib_hwq *hwq = &rcfw->cmdq.hwq;
 	struct bnxt_qplib_crsqe *crsqe;
+	u32 qp_id, tbl_indx, req_size;
 	struct bnxt_qplib_qp *qp;
 	u16 cbit, blocked = 0;
+	bool is_waiter_alive;
 	struct pci_dev *pdev;
 	unsigned long flags;
 	u32 wait_cmds = 0;
-	__le16  mcookie;
 	u16 cookie;
 	int rc = 0;
-	u32 qp_id, tbl_indx;
 
 	pdev = rcfw->pdev;
 	switch (qp_event->event) {
@@ -520,31 +529,29 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		spin_lock_irqsave_nested(&hwq->lock, flags,
 					 SINGLE_DEPTH_NESTING);
 		cookie = le16_to_cpu(qp_event->cookie);
-		mcookie = qp_event->cookie;
 		blocked = cookie & RCFW_CMD_IS_BLOCKING;
 		cookie &= RCFW_MAX_COOKIE_VALUE;
 		cbit = cookie % rcfw->cmdq_depth;
 		crsqe = &rcfw->crsqe_tbl[cbit];
-		if (crsqe->resp &&
-		    crsqe->resp->cookie  == mcookie) {
-			memcpy(crsqe->resp, qp_event, sizeof(*qp_event));
-			crsqe->resp = NULL;
-		} else {
-			if (crsqe->resp && crsqe->resp->cookie)
-				dev_err(&pdev->dev,
-					"CMD %s cookie sent=%#x, recd=%#x\n",
-					crsqe->resp ? "mismatch" : "collision",
-					crsqe->resp ? crsqe->resp->cookie : 0,
-					mcookie);
-		}
 		if (!test_and_clear_bit(cbit, rcfw->cmdq.cmdq_bitmap))
 			dev_warn(&pdev->dev,
 				 "CMD bit %d was not requested\n", cbit);
-		hwq->cons += crsqe->req_size;
+
+		if (crsqe->is_waiter_alive) {
+			if (crsqe->resp)
+				memcpy(crsqe->resp, qp_event, sizeof(*qp_event));
+			if (!blocked)
+				wait_cmds++;
+		}
+
+		req_size = crsqe->req_size;
+		is_waiter_alive = crsqe->is_waiter_alive;
+
 		crsqe->req_size = 0;
+		if (!is_waiter_alive)
+			crsqe->resp = NULL;
 
-		if (!blocked)
-			wait_cmds++;
+		hwq->cons += req_size;
 		spin_unlock_irqrestore(&hwq->lock, flags);
 	}
 	*num_wait += wait_cmds;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 089e616..6ed81c1 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -152,6 +152,7 @@ struct bnxt_qplib_crsqe {
 	u32			req_size;
 	/* Free slots at the time of submission */
 	u32			free_slots;
+	bool			is_waiter_alive;
 };
 
 struct bnxt_qplib_rcfw_sbuf {
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 11/17] RDMA/bnxt_re: Add firmware stall check detection

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

Every completion will update last_seen value in the unit of jiffies.
last_seen field will be used to know if firmware is alive and
is useful to detect firmware stall.

Non blocking interface __wait_for_resp will have logic to detect
firmware stall. After every 10 second interval if __wait_for_resp
has not received completion for a given command it will check for
firmware stall condition.

If current jiffies is greater than last_seen
jiffies + RCFW_FW_STALL_TIMEOUT_SEC * HZ, it is a firmware stall.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 44 ++++++++++++++++++++++++------
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  5 ++--
 2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 4e5f66e..349fbed 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -112,11 +112,13 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
 			return bnxt_qplib_map_rc(opcode);
+		if (test_bit(FIRMWARE_STALL_DETECTED, &cmdq->flags))
+			return -ETIMEDOUT;
 
-		/* Non zero means command completed */
 		wait_event_timeout(cmdq->waitq,
 				   !test_bit(cbit, cmdq->cmdq_bitmap),
-				   msecs_to_jiffies(10000));
+				   msecs_to_jiffies(RCFW_FW_STALL_TIMEOUT_SEC
+						    * 1000));
 
 		if (!test_bit(cbit, cmdq->cmdq_bitmap))
 			return 0;
@@ -126,6 +128,11 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 		if (!test_bit(cbit, cmdq->cmdq_bitmap))
 			return 0;
 
+		/* Firmware stall is detected */
+		if (time_after(jiffies, cmdq->last_seen +
+			      (RCFW_FW_STALL_TIMEOUT_SEC * HZ)))
+			return -ENODEV;
+
 	} while (true);
 };
 
@@ -154,6 +161,8 @@ static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
 			return bnxt_qplib_map_rc(opcode);
+		if (test_bit(FIRMWARE_STALL_DETECTED, &cmdq->flags))
+			return -ETIMEDOUT;
 
 		udelay(1);
 
@@ -184,9 +193,6 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	hwq = &cmdq->hwq;
 	pdev = rcfw->pdev;
 
-	if (test_bit(FIRMWARE_TIMED_OUT, &cmdq->flags))
-		return -ETIMEDOUT;
-
 	/* Cmdq are in 16-byte units, each request can consume 1 or more
 	 * cmdqe
 	 */
@@ -285,14 +291,21 @@ static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
 			return bnxt_qplib_map_rc(opcode);
+		if (test_bit(FIRMWARE_STALL_DETECTED, &cmdq->flags))
+			return -ETIMEDOUT;
 
 		usleep_range(1000, 1001);
 
 		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
 		if (!test_bit(cbit, cmdq->cmdq_bitmap))
 			return 0;
-		if (jiffies_to_msecs(jiffies - issue_time) > 10000)
-			return -ETIMEDOUT;
+		if (jiffies_to_msecs(jiffies - issue_time) >
+		    (RCFW_FW_STALL_TIMEOUT_SEC * 1000)) {
+			/* Firmware stall is detected */
+			if (time_after(jiffies, cmdq->last_seen +
+				      (RCFW_FW_STALL_TIMEOUT_SEC * HZ)))
+				return -ENODEV;
+		}
 	} while (true);
 };
 
@@ -308,6 +321,8 @@ static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
 	/* Prevent posting if f/w is not in a state to process */
 	if (test_bit(ERR_DEVICE_DETACHED, &rcfw->cmdq.flags))
 		return -ENXIO;
+	if (test_bit(FIRMWARE_STALL_DETECTED, &cmdq->flags))
+		return -ETIMEDOUT;
 
 	if (test_bit(FIRMWARE_INITIALIZED_FLAG, &cmdq->flags) &&
 	    opcode == CMDQ_BASE_OPCODE_INITIALIZE_FW) {
@@ -375,7 +390,6 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 		/* timed out */
 		dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
 			cookie, opcode, RCFW_CMD_WAIT_TIME_MS);
-		set_bit(FIRMWARE_TIMED_OUT, &rcfw->cmdq.flags);
 		return rc;
 	}
 
@@ -383,6 +397,8 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 		spin_lock_irqsave(&rcfw->cmdq.hwq.lock, flags);
 		crsqe = &rcfw->crsqe_tbl[cbit];
 		crsqe->is_waiter_alive = false;
+		if (rc == -ENODEV)
+			set_bit(FIRMWARE_STALL_DETECTED, &rcfw->cmdq.flags);
 		spin_unlock_irqrestore(&rcfw->cmdq.hwq.lock, flags);
 		return -ETIMEDOUT;
 	}
@@ -533,6 +549,17 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		cookie &= RCFW_MAX_COOKIE_VALUE;
 		cbit = cookie % rcfw->cmdq_depth;
 		crsqe = &rcfw->crsqe_tbl[cbit];
+
+		if (WARN_ONCE(test_bit(FIRMWARE_STALL_DETECTED,
+				       &rcfw->cmdq.flags),
+		    "QPLIB: Unreponsive rcfw channel detected.!!")) {
+			dev_info(&pdev->dev,
+				 "rcfw timedout: cookie = %#x, free_slots = %d",
+				 cookie, crsqe->free_slots);
+			spin_unlock_irqrestore(&hwq->lock, flags);
+			return rc;
+		}
+
 		if (!test_and_clear_bit(cbit, rcfw->cmdq.cmdq_bitmap))
 			dev_warn(&pdev->dev,
 				 "CMD bit %d was not requested\n", cbit);
@@ -582,6 +609,7 @@ static void bnxt_qplib_service_creq(struct tasklet_struct *t)
 		 * reading any further.
 		 */
 		dma_rmb();
+		rcfw->cmdq.last_seen = jiffies;
 
 		type = creqe->type & CREQ_BASE_TYPE_MASK;
 		switch (type) {
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 6ed81c1..54576f1 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -51,6 +51,7 @@
 
 #define RCFW_DBR_PCI_BAR_REGION		2
 #define RCFW_DBR_BASE_PAGE_SHIFT	12
+#define RCFW_FW_STALL_TIMEOUT_SEC	40
 
 /* Cmdq contains a fix number of a 16-Byte slots */
 struct bnxt_qplib_cmdqe {
@@ -128,7 +129,6 @@ static inline u32 bnxt_qplib_set_cmd_slots(struct cmdq_base *req)
 
 #define RCFW_MAX_COOKIE_VALUE		(BNXT_QPLIB_CMDQE_MAX_CNT - 1)
 #define RCFW_CMD_IS_BLOCKING		0x8000
-#define RCFW_BLOCKED_CMD_WAIT_COUNT	20000000UL /* 20 sec */
 
 /* Crsq buf is 1024-Byte */
 struct bnxt_qplib_crsbe {
@@ -170,7 +170,7 @@ struct bnxt_qplib_qp_node {
 
 #define FIRMWARE_INITIALIZED_FLAG	(0)
 #define FIRMWARE_FIRST_FLAG		(31)
-#define FIRMWARE_TIMED_OUT		(3)
+#define FIRMWARE_STALL_DETECTED		(3)
 #define ERR_DEVICE_DETACHED             (4)
 
 struct bnxt_qplib_cmdq_mbox {
@@ -185,6 +185,7 @@ struct bnxt_qplib_cmdq_ctx {
 	wait_queue_head_t		waitq;
 	unsigned long			flags;
 	unsigned long			*cmdq_bitmap;
+	unsigned long			last_seen;
 	u32				seq_num;
 };
 
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 12/17] RDMA/bnxt_re: post destroy_ah for delayed completion of AH creation

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

AH create may be called from interrpt context and driver has a special
timeout (8 sec) for this command. This is to avoid soft lockups when
the FW command takes more time. Driver returns -ETIMEOUT and fail
create AH, without waiting for actual completion from firmware.
When FW completion is received, use is_waiter_alive flag to avoid
a regular completion path.

If create_ah opcode is detected in completion path which does not have
waiter alive, driver will fetch ah_id from successful firmware
completion in the interrupt context and sends destroy_ah command
for same ah_id. This special post is done in quick manner using helper
function __send_message_no_waiter.

timeout_send is only used for debugging purposes.
If timeout_send value keeps incrementing, it indicates out of sync
active ah counter between driver and firmware. This is a limitation
but graceful handling is possible in future.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 108 +++++++++++++++++++++++++++++
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |   2 +
 2 files changed, 110 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 349fbed..8dd8216 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -175,6 +175,73 @@ static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 	return -ETIMEDOUT;
 };
 
+/*  __send_message_no_waiter -	get cookie and post the message.
+ * @rcfw      -   rcfw channel instance of rdev
+ * @msg      -    qplib message internal
+ *
+ * This function will just post and don't bother about completion.
+ * Current design of this function is -
+ * user must hold the completion queue hwq->lock.
+ * user must have used existing completion and free the resources.
+ * this function will not check queue full condition.
+ * this function will explicitly set is_waiter_alive=false.
+ * current use case is - send destroy_ah if create_ah is return
+ * after waiter of create_ah is lost. It can be extended for other
+ * use case as well.
+ *
+ * Returns: Nothing
+ *
+ */
+static void __send_message_no_waiter(struct bnxt_qplib_rcfw *rcfw,
+				     struct bnxt_qplib_cmdqmsg *msg)
+{
+	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
+	struct bnxt_qplib_hwq *hwq = &cmdq->hwq;
+	struct bnxt_qplib_crsqe *crsqe;
+	struct bnxt_qplib_cmdqe *cmdqe;
+	u32 sw_prod, cmdq_prod;
+	u16 cookie, cbit;
+	u32 bsize;
+	u8 *preq;
+
+	cookie = cmdq->seq_num & RCFW_MAX_COOKIE_VALUE;
+	cbit = cookie % rcfw->cmdq_depth;
+
+	set_bit(cbit, cmdq->cmdq_bitmap);
+	__set_cmdq_base_cookie(msg->req, msg->req_sz, cpu_to_le16(cookie));
+	crsqe = &rcfw->crsqe_tbl[cbit];
+
+	/* Set cmd_size in terms of 16B slots in req. */
+	bsize = bnxt_qplib_set_cmd_slots(msg->req);
+	/* GET_CMD_SIZE would return number of slots in either case of tlv
+	 * and non-tlv commands after call to bnxt_qplib_set_cmd_slots()
+	 */
+	crsqe->is_internal_cmd = true;
+	crsqe->is_waiter_alive = false;
+	crsqe->req_size = __get_cmdq_base_cmd_size(msg->req, msg->req_sz);
+
+	preq = (u8 *)msg->req;
+	do {
+		/* Locate the next cmdq slot */
+		sw_prod = HWQ_CMP(hwq->prod, hwq);
+		cmdqe = bnxt_qplib_get_qe(hwq, sw_prod, NULL);
+		/* Copy a segment of the req cmd to the cmdq */
+		memset(cmdqe, 0, sizeof(*cmdqe));
+		memcpy(cmdqe, preq, min_t(u32, bsize, sizeof(*cmdqe)));
+		preq += min_t(u32, bsize, sizeof(*cmdqe));
+		bsize -= min_t(u32, bsize, sizeof(*cmdqe));
+		hwq->prod++;
+	} while (bsize > 0);
+	cmdq->seq_num++;
+
+	cmdq_prod = hwq->prod;
+	atomic_inc(&rcfw->timeout_send);
+	/* ring CMDQ DB */
+	wmb();
+	writel(cmdq_prod, cmdq->cmdq_mbox.prod);
+	writel(RCFW_CMDQ_TRIG_VAL, cmdq->cmdq_mbox.db);
+}
+
 static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 			  struct bnxt_qplib_cmdqmsg *msg)
 {
@@ -219,6 +286,7 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	crsqe->free_slots = free_slots;
 	crsqe->resp = (struct creq_qp_event *)msg->resp;
 	crsqe->resp->cookie = cpu_to_le16(cookie);
+	crsqe->is_internal_cmd = false;
 	crsqe->is_waiter_alive = true;
 	crsqe->req_size = __get_cmdq_base_cmd_size(msg->req, msg->req_sz);
 	if (__get_cmdq_base_resp_size(msg->req, msg->req_sz) && msg->sb) {
@@ -343,6 +411,26 @@ static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
 	return 0;
 }
 
+/* This function will just post and do not bother about completion */
+static void __destroy_timedout_ah(struct bnxt_qplib_rcfw *rcfw,
+				  struct creq_create_ah_resp *create_ah_resp)
+{
+	struct bnxt_qplib_cmdqmsg msg = {};
+	struct cmdq_destroy_ah req = {};
+
+	bnxt_qplib_rcfw_cmd_prep((struct cmdq_base *)&req,
+				 CMDQ_BASE_OPCODE_DESTROY_AH,
+				 sizeof(req));
+	req.ah_cid = create_ah_resp->xid;
+	msg.req = (struct cmdq_base *)&req;
+	msg.req_sz = sizeof(req);
+	__send_message_no_waiter(rcfw, &msg);
+	dev_info_ratelimited(&rcfw->pdev->dev,
+			     "From %s: ah_cid = %d timeout_send %d\n",
+			     __func__, req.ah_cid,
+			     atomic_read(&rcfw->timeout_send));
+}
+
 /**
  * __bnxt_qplib_rcfw_send_message   -	qplib interface to send
  * and complete rcfw command.
@@ -563,6 +651,8 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		if (!test_and_clear_bit(cbit, rcfw->cmdq.cmdq_bitmap))
 			dev_warn(&pdev->dev,
 				 "CMD bit %d was not requested\n", cbit);
+		if (crsqe->is_internal_cmd && !qp_event->status)
+			atomic_dec(&rcfw->timeout_send);
 
 		if (crsqe->is_waiter_alive) {
 			if (crsqe->resp)
@@ -579,6 +669,24 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 			crsqe->resp = NULL;
 
 		hwq->cons += req_size;
+
+		/* This is a case to handle below scenario -
+		 * Create AH is completed successfully by firmware,
+		 * but completion took more time and driver already lost
+		 * the context of create_ah from caller.
+		 * We have already return failure for create_ah verbs,
+		 * so let's destroy the same address vector since it is
+		 * no more used in stack. We don't care about completion
+		 * in __send_message_no_waiter.
+		 * If destroy_ah is failued by firmware, there will be AH
+		 * resource leak and relatively not critical +  unlikely
+		 * scenario. Current design is not to handle such case.
+		 */
+		if (!is_waiter_alive && !qp_event->status &&
+		    qp_event->event == CREQ_QP_EVENT_EVENT_CREATE_AH)
+			__destroy_timedout_ah(rcfw,
+					      (struct creq_create_ah_resp *)
+					      qp_event);
 		spin_unlock_irqrestore(&hwq->lock, flags);
 	}
 	*num_wait += wait_cmds;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 54576f1..338bf6a 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -153,6 +153,7 @@ struct bnxt_qplib_crsqe {
 	/* Free slots at the time of submission */
 	u32			free_slots;
 	bool			is_waiter_alive;
+	bool			is_internal_cmd;
 };
 
 struct bnxt_qplib_rcfw_sbuf {
@@ -225,6 +226,7 @@ struct bnxt_qplib_rcfw {
 	u32 cmdq_depth;
 	atomic_t rcfw_intr_enabled;
 	struct semaphore rcfw_inflight;
+	atomic_t timeout_send;
 };
 
 struct bnxt_qplib_cmdqmsg {
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 13/17] RDMA/bnxt_re: consider timeout of destroy ah as success.

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

If destroy_ah is timed out, it is likely to be destroyed by firmware
but it is taking longer time due to temporary slowness
in processing the rcfw command. In worst case, there might be
AH resource leak in firmware.

Sending timeout return value can dump warning message from ib_core
which can be avoided if we map timeout of destroy_ah as success.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/bnxt_re.h  |  2 ++
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 16 ++++++++++++----
 drivers/infiniband/hw/bnxt_re/qplib_sp.c |  8 +++++---
 drivers/infiniband/hw/bnxt_re/qplib_sp.h |  4 ++--
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
index 5a2baf4..f34fb87 100644
--- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
+++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
@@ -181,6 +181,8 @@ struct bnxt_re_dev {
 #define BNXT_RE_ROCEV2_IPV4_PACKET	2
 #define BNXT_RE_ROCEV2_IPV6_PACKET	3
 
+#define BNXT_RE_CHECK_RC(x) ((x) && ((x) != -ETIMEDOUT))
+
 static inline struct device *rdev_to_dev(struct bnxt_re_dev *rdev)
 {
 	if (rdev)
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index e86afec..053afc9 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -613,12 +613,20 @@ int bnxt_re_destroy_ah(struct ib_ah *ib_ah, u32 flags)
 {
 	struct bnxt_re_ah *ah = container_of(ib_ah, struct bnxt_re_ah, ib_ah);
 	struct bnxt_re_dev *rdev = ah->rdev;
+	bool block = true;
+	int rc = 0;
 
-	bnxt_qplib_destroy_ah(&rdev->qplib_res, &ah->qplib_ah,
-			      !(flags & RDMA_DESTROY_AH_SLEEPABLE));
+	block = !(flags & RDMA_DESTROY_AH_SLEEPABLE);
+	rc = bnxt_qplib_destroy_ah(&rdev->qplib_res, &ah->qplib_ah, block);
+	if (BNXT_RE_CHECK_RC(rc)) {
+		if (rc == -ETIMEDOUT)
+			rc = 0;
+		else
+			goto fail;
+	}
 	atomic_dec(&rdev->ah_count);
-
-	return 0;
+fail:
+	return rc;
 }
 
 static u8 bnxt_re_stack_to_dev_nw_type(enum rdma_network_type ntype)
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
index dbb0e4e..910d17d 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
@@ -460,13 +460,14 @@ int bnxt_qplib_create_ah(struct bnxt_qplib_res *res, struct bnxt_qplib_ah *ah,
 	return 0;
 }
 
-void bnxt_qplib_destroy_ah(struct bnxt_qplib_res *res, struct bnxt_qplib_ah *ah,
-			   bool block)
+int bnxt_qplib_destroy_ah(struct bnxt_qplib_res *res, struct bnxt_qplib_ah *ah,
+			  bool block)
 {
 	struct bnxt_qplib_rcfw *rcfw = res->rcfw;
 	struct creq_destroy_ah_resp resp = {};
 	struct bnxt_qplib_cmdqmsg msg = {};
 	struct cmdq_destroy_ah req = {};
+	int rc;
 
 	/* Clean up the AH table in the device */
 	bnxt_qplib_rcfw_cmd_prep((struct cmdq_base *)&req,
@@ -477,7 +478,8 @@ void bnxt_qplib_destroy_ah(struct bnxt_qplib_res *res, struct bnxt_qplib_ah *ah,
 
 	bnxt_qplib_fill_cmdqmsg(&msg, &req, &resp, NULL, sizeof(req),
 				sizeof(resp), block);
-	bnxt_qplib_rcfw_send_message(rcfw, &msg);
+	rc = bnxt_qplib_rcfw_send_message(rcfw, &msg);
+	return rc;
 }
 
 /* MRW */
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.h b/drivers/infiniband/hw/bnxt_re/qplib_sp.h
index 5de87465..4061616 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_sp.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.h
@@ -327,8 +327,8 @@ int bnxt_qplib_set_func_resources(struct bnxt_qplib_res *res,
 				  struct bnxt_qplib_ctx *ctx);
 int bnxt_qplib_create_ah(struct bnxt_qplib_res *res, struct bnxt_qplib_ah *ah,
 			 bool block);
-void bnxt_qplib_destroy_ah(struct bnxt_qplib_res *res, struct bnxt_qplib_ah *ah,
-			   bool block);
+int bnxt_qplib_destroy_ah(struct bnxt_qplib_res *res, struct bnxt_qplib_ah *ah,
+			  bool block);
 int bnxt_qplib_alloc_mrw(struct bnxt_qplib_res *res,
 			 struct bnxt_qplib_mrw *mrw);
 int bnxt_qplib_dereg_mrw(struct bnxt_qplib_res *res, struct bnxt_qplib_mrw *mrw,
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 14/17] RDMA/bnxt_re: cancel all control path command waiters upon error.

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

When an error is detected in FW, wake up all the waiters as the
all of them need to be completed with timeout. Add the device
error state also as a wait condition.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c       | 1 +
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 51372de..8241154 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -1495,6 +1495,7 @@ static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
 	 */
 	set_bit(BNXT_RE_FLAG_ERR_DEVICE_DETACHED, &rdev->flags);
 	set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
+	wake_up_all(&rdev->rcfw.cmdq.waitq);
 	mutex_unlock(&bnxt_re_mutex);
 
 	return 0;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 8dd8216..8b1b413 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -116,10 +116,10 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 			return -ETIMEDOUT;
 
 		wait_event_timeout(cmdq->waitq,
-				   !test_bit(cbit, cmdq->cmdq_bitmap),
+				   !test_bit(cbit, cmdq->cmdq_bitmap) ||
+				   test_bit(ERR_DEVICE_DETACHED, &cmdq->flags),
 				   msecs_to_jiffies(RCFW_FW_STALL_TIMEOUT_SEC
 						    * 1000));
-
 		if (!test_bit(cbit, cmdq->cmdq_bitmap))
 			return 0;
 
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 15/17] RDMA/bnxt_re: use firmware provided max request timeout

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

Firmware provides max request timeout value as part of hwrm_ver_get
API. Driver gets the timeout from firmware and if that interface is
not available then fall back to hardcoded timeout value.
Also, Add a helper function to check the FW status.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c       |  8 ++++
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 59 ++++++++++++++++++++++++------
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  4 +-
 drivers/infiniband/hw/bnxt_re/qplib_res.h  |  1 +
 4 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 8241154..a2c7d3f 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -1041,6 +1041,7 @@ static void bnxt_re_query_hwrm_intf_version(struct bnxt_re_dev *rdev)
 	struct bnxt_en_dev *en_dev = rdev->en_dev;
 	struct hwrm_ver_get_output resp = {0};
 	struct hwrm_ver_get_input req = {0};
+	struct bnxt_qplib_chip_ctx *cctx;
 	struct bnxt_fw_msg fw_msg;
 	int rc = 0;
 
@@ -1058,11 +1059,18 @@ static void bnxt_re_query_hwrm_intf_version(struct bnxt_re_dev *rdev)
 			  rc);
 		return;
 	}
+
+	cctx = rdev->chip_ctx;
 	rdev->qplib_ctx.hwrm_intf_ver =
 		(u64)le16_to_cpu(resp.hwrm_intf_major) << 48 |
 		(u64)le16_to_cpu(resp.hwrm_intf_minor) << 32 |
 		(u64)le16_to_cpu(resp.hwrm_intf_build) << 16 |
 		le16_to_cpu(resp.hwrm_intf_patch);
+
+	cctx->hwrm_cmd_max_timeout = le16_to_cpu(resp.max_req_timeout);
+
+	if (!cctx->hwrm_cmd_max_timeout)
+		cctx->hwrm_cmd_max_timeout = RCFW_FW_STALL_MAX_TIMEOUT;
 }
 
 static int bnxt_re_ib_init(struct bnxt_re_dev *rdev)
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 8b1b413..99aa1ae 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -90,6 +90,41 @@ static int bnxt_qplib_map_rc(u8 opcode)
 }
 
 /**
+ * bnxt_re_is_fw_stalled   -	Check firmware health
+ * @rcfw      -   rcfw channel instance of rdev
+ * @cookie    -   cookie to track the command
+ * @opcode    -   rcfw submitted for given opcode
+ * @cbit      -   bitmap entry of cookie
+ *
+ * If firmware has not responded any rcfw command within
+ * rcfw->max_timeout, consider firmware as stalled.
+ *
+ * Returns:
+ * 0 if firmware is responding
+ * -ENODEV if firmware is not responding
+ */
+static int bnxt_re_is_fw_stalled(struct bnxt_qplib_rcfw *rcfw,
+				 u16 cookie, u8 opcode, u16 cbit)
+{
+	struct bnxt_qplib_cmdq_ctx *cmdq;
+
+	cmdq = &rcfw->cmdq;
+
+	if (time_after(jiffies, cmdq->last_seen +
+		      (rcfw->max_timeout * HZ))) {
+		dev_warn_ratelimited(&rcfw->pdev->dev,
+				     "%s: FW STALL Detected. cmdq[%#x]=%#x waited (%d > %d) msec active %d ",
+				     __func__, cookie, opcode,
+				     jiffies_to_msecs(jiffies - cmdq->last_seen),
+				     rcfw->max_timeout * 1000,
+				     test_bit(cbit, cmdq->cmdq_bitmap));
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+/**
  * __wait_for_resp   -	Don't hold the cpu context and wait for response
  * @rcfw      -   rcfw channel instance of rdev
  * @cookie    -   cookie to track the command
@@ -105,6 +140,7 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
 	u16 cbit;
+	int ret;
 
 	cmdq = &rcfw->cmdq;
 	cbit = cookie % rcfw->cmdq_depth;
@@ -118,8 +154,8 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 		wait_event_timeout(cmdq->waitq,
 				   !test_bit(cbit, cmdq->cmdq_bitmap) ||
 				   test_bit(ERR_DEVICE_DETACHED, &cmdq->flags),
-				   msecs_to_jiffies(RCFW_FW_STALL_TIMEOUT_SEC
-						    * 1000));
+				   msecs_to_jiffies(rcfw->max_timeout * 1000));
+
 		if (!test_bit(cbit, cmdq->cmdq_bitmap))
 			return 0;
 
@@ -128,10 +164,9 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 		if (!test_bit(cbit, cmdq->cmdq_bitmap))
 			return 0;
 
-		/* Firmware stall is detected */
-		if (time_after(jiffies, cmdq->last_seen +
-			      (RCFW_FW_STALL_TIMEOUT_SEC * HZ)))
-			return -ENODEV;
+		ret = bnxt_re_is_fw_stalled(rcfw, cookie, opcode, cbit);
+		if (ret)
+			return ret;
 
 	} while (true);
 };
@@ -352,6 +387,7 @@ static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
 	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
 	unsigned long issue_time;
 	u16 cbit;
+	int ret;
 
 	cbit = cookie % rcfw->cmdq_depth;
 	issue_time = jiffies;
@@ -368,11 +404,10 @@ static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
 		if (!test_bit(cbit, cmdq->cmdq_bitmap))
 			return 0;
 		if (jiffies_to_msecs(jiffies - issue_time) >
-		    (RCFW_FW_STALL_TIMEOUT_SEC * 1000)) {
-			/* Firmware stall is detected */
-			if (time_after(jiffies, cmdq->last_seen +
-				      (RCFW_FW_STALL_TIMEOUT_SEC * HZ)))
-				return -ENODEV;
+		    (rcfw->max_timeout * 1000)) {
+			ret = bnxt_re_is_fw_stalled(rcfw, cookie, opcode, cbit);
+			if (ret)
+				return ret;
 		}
 	} while (true);
 };
@@ -951,6 +986,8 @@ int bnxt_qplib_alloc_rcfw_channel(struct bnxt_qplib_res *res,
 	if (!rcfw->qp_tbl)
 		goto fail;
 
+	rcfw->max_timeout = res->cctx->hwrm_cmd_max_timeout;
+
 	return 0;
 
 fail:
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 338bf6a..b644dcc 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -51,7 +51,7 @@
 
 #define RCFW_DBR_PCI_BAR_REGION		2
 #define RCFW_DBR_BASE_PAGE_SHIFT	12
-#define RCFW_FW_STALL_TIMEOUT_SEC	40
+#define RCFW_FW_STALL_MAX_TIMEOUT	40
 
 /* Cmdq contains a fix number of a 16-Byte slots */
 struct bnxt_qplib_cmdqe {
@@ -227,6 +227,8 @@ struct bnxt_qplib_rcfw {
 	atomic_t rcfw_intr_enabled;
 	struct semaphore rcfw_inflight;
 	atomic_t timeout_send;
+	/* cached from chip cctx for quick reference in slow path */
+	u16 max_timeout;
 };
 
 struct bnxt_qplib_cmdqmsg {
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.h b/drivers/infiniband/hw/bnxt_re/qplib_res.h
index 982e2c9..77f0b84 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_res.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_res.h
@@ -55,6 +55,7 @@ struct bnxt_qplib_chip_ctx {
 	u8	chip_rev;
 	u8	chip_metal;
 	u16	hw_stats_size;
+	u16	hwrm_cmd_max_timeout;
 	struct bnxt_qplib_drv_modes modes;
 };
 
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 16/17] RDMA/bnxt_re: remove redundant cmdq_bitmap

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

cmdq_bitmap is used to derive the next available index in the CMDQ.
This is not required as the we can get the next index
using the existing bnxt_qplib_crsqe array.

Driver will use bnxt_qplib_crsqe array and flag is_in_used to
derive valid entries. is_in_used  is replacement of cmdq_bitmap.
There is no change in the existing mechanism of the circular buffer
used to get index.

Added opcode field in bnxt_qplib_crsqe array so that it is easy to map
opcode associated with pending rcfw command.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 79 ++++++++++++------------------
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  3 +-
 2 files changed, 34 insertions(+), 48 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 99aa1ae..3ae2f82ff 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -95,6 +95,7 @@ static int bnxt_qplib_map_rc(u8 opcode)
  * @cookie    -   cookie to track the command
  * @opcode    -   rcfw submitted for given opcode
  * @cbit      -   bitmap entry of cookie
+ * @in_used   -   command is in used or freed
  *
  * If firmware has not responded any rcfw command within
  * rcfw->max_timeout, consider firmware as stalled.
@@ -104,7 +105,7 @@ static int bnxt_qplib_map_rc(u8 opcode)
  * -ENODEV if firmware is not responding
  */
 static int bnxt_re_is_fw_stalled(struct bnxt_qplib_rcfw *rcfw,
-				 u16 cookie, u8 opcode, u16 cbit)
+				 u16 cookie, u8 opcode, bool in_used)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
 
@@ -117,7 +118,7 @@ static int bnxt_re_is_fw_stalled(struct bnxt_qplib_rcfw *rcfw,
 				     __func__, cookie, opcode,
 				     jiffies_to_msecs(jiffies - cmdq->last_seen),
 				     rcfw->max_timeout * 1000,
-				     test_bit(cbit, cmdq->cmdq_bitmap));
+				     in_used);
 		return -ENODEV;
 	}
 
@@ -139,11 +140,11 @@ static int bnxt_re_is_fw_stalled(struct bnxt_qplib_rcfw *rcfw,
 static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
-	u16 cbit;
+	struct bnxt_qplib_crsqe *crsqe;
 	int ret;
 
 	cmdq = &rcfw->cmdq;
-	cbit = cookie % rcfw->cmdq_depth;
+	crsqe = &rcfw->crsqe_tbl[cookie];
 
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
@@ -152,19 +153,19 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 			return -ETIMEDOUT;
 
 		wait_event_timeout(cmdq->waitq,
-				   !test_bit(cbit, cmdq->cmdq_bitmap) ||
+				   !crsqe->is_in_used ||
 				   test_bit(ERR_DEVICE_DETACHED, &cmdq->flags),
 				   msecs_to_jiffies(rcfw->max_timeout * 1000));
 
-		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+		if (!crsqe->is_in_used)
 			return 0;
 
 		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
 
-		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+		if (!crsqe->is_in_used)
 			return 0;
 
-		ret = bnxt_re_is_fw_stalled(rcfw, cookie, opcode, cbit);
+		ret = bnxt_re_is_fw_stalled(rcfw, cookie, opcode, crsqe->is_in_used);
 		if (ret)
 			return ret;
 
@@ -187,11 +188,11 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
+	struct bnxt_qplib_crsqe *crsqe;
 	unsigned long issue_time = 0;
-	u16 cbit;
 
-	cbit = cookie % rcfw->cmdq_depth;
 	issue_time = jiffies;
+	crsqe = &rcfw->crsqe_tbl[cookie];
 
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
@@ -202,7 +203,7 @@ static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 		udelay(1);
 
 		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
-		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+		if (!crsqe->is_in_used)
 			return 0;
 
 	} while (time_before(jiffies, issue_time + (8 * HZ)));
@@ -235,16 +236,13 @@ static void __send_message_no_waiter(struct bnxt_qplib_rcfw *rcfw,
 	struct bnxt_qplib_crsqe *crsqe;
 	struct bnxt_qplib_cmdqe *cmdqe;
 	u32 sw_prod, cmdq_prod;
-	u16 cookie, cbit;
+	u16 cookie;
 	u32 bsize;
 	u8 *preq;
 
 	cookie = cmdq->seq_num & RCFW_MAX_COOKIE_VALUE;
-	cbit = cookie % rcfw->cmdq_depth;
-
-	set_bit(cbit, cmdq->cmdq_bitmap);
 	__set_cmdq_base_cookie(msg->req, msg->req_sz, cpu_to_le16(cookie));
-	crsqe = &rcfw->crsqe_tbl[cbit];
+	crsqe = &rcfw->crsqe_tbl[cookie];
 
 	/* Set cmd_size in terms of 16B slots in req. */
 	bsize = bnxt_qplib_set_cmd_slots(msg->req);
@@ -253,6 +251,7 @@ static void __send_message_no_waiter(struct bnxt_qplib_rcfw *rcfw,
 	 */
 	crsqe->is_internal_cmd = true;
 	crsqe->is_waiter_alive = false;
+	crsqe->is_in_used = true;
 	crsqe->req_size = __get_cmdq_base_cmd_size(msg->req, msg->req_sz);
 
 	preq = (u8 *)msg->req;
@@ -288,7 +287,8 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	u32 sw_prod, cmdq_prod;
 	struct pci_dev *pdev;
 	unsigned long flags;
-	u16 cookie, cbit;
+	u16 cookie;
+	u8 opcode;
 	u8 *preq;
 
 	cmdq = &rcfw->cmdq;
@@ -302,10 +302,9 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	required_slots = bnxt_qplib_get_cmd_slots(msg->req);
 	free_slots = HWQ_FREE_SLOTS(hwq);
 	cookie = cmdq->seq_num & RCFW_MAX_COOKIE_VALUE;
-	cbit = cookie % rcfw->cmdq_depth;
+	crsqe = &rcfw->crsqe_tbl[cookie];
 
-	if (required_slots >= free_slots ||
-	    test_bit(cbit, cmdq->cmdq_bitmap)) {
+	if (required_slots >= free_slots) {
 		dev_info_ratelimited(&pdev->dev,
 				     "CMDQ is full req/free %d/%d!",
 				     required_slots, free_slots);
@@ -314,15 +313,17 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
 	}
 	if (msg->block)
 		cookie |= RCFW_CMD_IS_BLOCKING;
-	set_bit(cbit, cmdq->cmdq_bitmap);
 	__set_cmdq_base_cookie(msg->req, msg->req_sz, cpu_to_le16(cookie));
-	crsqe = &rcfw->crsqe_tbl[cbit];
+
 	bsize = bnxt_qplib_set_cmd_slots(msg->req);
 	crsqe->free_slots = free_slots;
 	crsqe->resp = (struct creq_qp_event *)msg->resp;
 	crsqe->resp->cookie = cpu_to_le16(cookie);
 	crsqe->is_internal_cmd = false;
 	crsqe->is_waiter_alive = true;
+	crsqe->is_in_used = true;
+	crsqe->opcode = opcode;
+
 	crsqe->req_size = __get_cmdq_base_cmd_size(msg->req, msg->req_sz);
 	if (__get_cmdq_base_resp_size(msg->req, msg->req_sz) && msg->sb) {
 		struct bnxt_qplib_rcfw_sbuf *sbuf = msg->sb;
@@ -385,12 +386,12 @@ static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
 			   u8 opcode)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
+	struct bnxt_qplib_crsqe *crsqe;
 	unsigned long issue_time;
-	u16 cbit;
 	int ret;
 
-	cbit = cookie % rcfw->cmdq_depth;
 	issue_time = jiffies;
+	crsqe = &rcfw->crsqe_tbl[cookie];
 
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
@@ -401,11 +402,11 @@ static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
 		usleep_range(1000, 1001);
 
 		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
-		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+		if (!crsqe->is_in_used)
 			return 0;
 		if (jiffies_to_msecs(jiffies - issue_time) >
 		    (rcfw->max_timeout * 1000)) {
-			ret = bnxt_re_is_fw_stalled(rcfw, cookie, opcode, cbit);
+			ret = bnxt_re_is_fw_stalled(rcfw, cookie, opcode, crsqe->is_in_used);
 			if (ret)
 				return ret;
 		}
@@ -485,7 +486,7 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 	struct creq_qp_event *evnt = (struct creq_qp_event *)msg->resp;
 	struct bnxt_qplib_crsqe *crsqe;
 	unsigned long flags;
-	u16 cookie, cbit;
+	u16 cookie;
 	int rc = 0;
 	u8 opcode;
 
@@ -501,7 +502,6 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 
 	cookie = le16_to_cpu(__get_cmdq_base_cookie(msg->req, msg->req_sz))
 				& RCFW_MAX_COOKIE_VALUE;
-	cbit = cookie % rcfw->cmdq_depth;
 
 	if (msg->block)
 		rc = __block_for_resp(rcfw, cookie, opcode);
@@ -518,7 +518,7 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 
 	if (rc) {
 		spin_lock_irqsave(&rcfw->cmdq.hwq.lock, flags);
-		crsqe = &rcfw->crsqe_tbl[cbit];
+		crsqe = &rcfw->crsqe_tbl[cookie];
 		crsqe->is_waiter_alive = false;
 		if (rc == -ENODEV)
 			set_bit(FIRMWARE_STALL_DETECTED, &rcfw->cmdq.flags);
@@ -630,12 +630,11 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 	struct bnxt_qplib_crsqe *crsqe;
 	u32 qp_id, tbl_indx, req_size;
 	struct bnxt_qplib_qp *qp;
-	u16 cbit, blocked = 0;
+	u16 cookie, blocked = 0;
 	bool is_waiter_alive;
 	struct pci_dev *pdev;
 	unsigned long flags;
 	u32 wait_cmds = 0;
-	u16 cookie;
 	int rc = 0;
 
 	pdev = rcfw->pdev;
@@ -670,8 +669,8 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 		cookie = le16_to_cpu(qp_event->cookie);
 		blocked = cookie & RCFW_CMD_IS_BLOCKING;
 		cookie &= RCFW_MAX_COOKIE_VALUE;
-		cbit = cookie % rcfw->cmdq_depth;
-		crsqe = &rcfw->crsqe_tbl[cbit];
+		crsqe = &rcfw->crsqe_tbl[cookie];
+		crsqe->is_in_used = false;
 
 		if (WARN_ONCE(test_bit(FIRMWARE_STALL_DETECTED,
 				       &rcfw->cmdq.flags),
@@ -683,9 +682,6 @@ static int bnxt_qplib_process_qp_event(struct bnxt_qplib_rcfw *rcfw,
 			return rc;
 		}
 
-		if (!test_and_clear_bit(cbit, rcfw->cmdq.cmdq_bitmap))
-			dev_warn(&pdev->dev,
-				 "CMD bit %d was not requested\n", cbit);
 		if (crsqe->is_internal_cmd && !qp_event->status)
 			atomic_dec(&rcfw->timeout_send);
 
@@ -920,7 +916,6 @@ int bnxt_qplib_init_rcfw(struct bnxt_qplib_rcfw *rcfw,
 
 void bnxt_qplib_free_rcfw_channel(struct bnxt_qplib_rcfw *rcfw)
 {
-	bitmap_free(rcfw->cmdq.cmdq_bitmap);
 	kfree(rcfw->qp_tbl);
 	kfree(rcfw->crsqe_tbl);
 	bnxt_qplib_free_hwq(rcfw->res, &rcfw->cmdq.hwq);
@@ -975,10 +970,6 @@ int bnxt_qplib_alloc_rcfw_channel(struct bnxt_qplib_res *res,
 	if (!rcfw->crsqe_tbl)
 		goto fail;
 
-	cmdq->cmdq_bitmap = bitmap_zalloc(rcfw->cmdq_depth, GFP_KERNEL);
-	if (!cmdq->cmdq_bitmap)
-		goto fail;
-
 	/* Allocate one extra to hold the QP1 entries */
 	rcfw->qp_tbl_size = qp_tbl_sz + 1;
 	rcfw->qp_tbl = kcalloc(rcfw->qp_tbl_size, sizeof(struct bnxt_qplib_qp_node),
@@ -1023,7 +1014,6 @@ void bnxt_qplib_disable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw)
 {
 	struct bnxt_qplib_creq_ctx *creq;
 	struct bnxt_qplib_cmdq_ctx *cmdq;
-	unsigned long indx;
 
 	creq = &rcfw->creq;
 	cmdq = &rcfw->cmdq;
@@ -1033,11 +1023,6 @@ void bnxt_qplib_disable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw)
 	iounmap(cmdq->cmdq_mbox.reg.bar_reg);
 	iounmap(creq->creq_db.reg.bar_reg);
 
-	indx = find_first_bit(cmdq->cmdq_bitmap, rcfw->cmdq_depth);
-	if (indx != rcfw->cmdq_depth)
-		dev_err(&rcfw->pdev->dev,
-			"disabling RCFW with pending cmd-bit %lx\n", indx);
-
 	cmdq->cmdq_mbox.reg.bar_reg = NULL;
 	creq->creq_db.reg.bar_reg = NULL;
 	creq->aeq_handler = NULL;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index b644dcc..f46de07 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -152,8 +152,10 @@ struct bnxt_qplib_crsqe {
 	u32			req_size;
 	/* Free slots at the time of submission */
 	u32			free_slots;
+	u8			opcode;
 	bool			is_waiter_alive;
 	bool			is_internal_cmd;
+	bool			is_in_used;
 };
 
 struct bnxt_qplib_rcfw_sbuf {
@@ -185,7 +187,6 @@ struct bnxt_qplib_cmdq_ctx {
 	struct bnxt_qplib_cmdq_mbox	cmdq_mbox;
 	wait_queue_head_t		waitq;
 	unsigned long			flags;
-	unsigned long			*cmdq_bitmap;
 	unsigned long			last_seen;
 	u32				seq_num;
 };
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

[PATCH v2 for-next 17/17] RDMA/bnxt_re: optimize the parameters passed to helper functions

[Selvin Xavier]

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

From: Kashyap Desai <kashyap.desai@broadcom.com>

Avoid passing arguments like Opcode which can be retrieved from
bnxt_qplib_crsqe structure.

Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 44 +++++++++++++-----------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 3ae2f82ff..bb5aeba 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -93,9 +93,6 @@ static int bnxt_qplib_map_rc(u8 opcode)
  * bnxt_re_is_fw_stalled   -	Check firmware health
  * @rcfw      -   rcfw channel instance of rdev
  * @cookie    -   cookie to track the command
- * @opcode    -   rcfw submitted for given opcode
- * @cbit      -   bitmap entry of cookie
- * @in_used   -   command is in used or freed
  *
  * If firmware has not responded any rcfw command within
  * rcfw->max_timeout, consider firmware as stalled.
@@ -105,20 +102,22 @@ static int bnxt_qplib_map_rc(u8 opcode)
  * -ENODEV if firmware is not responding
  */
 static int bnxt_re_is_fw_stalled(struct bnxt_qplib_rcfw *rcfw,
-				 u16 cookie, u8 opcode, bool in_used)
+				 u16 cookie)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
+	struct bnxt_qplib_crsqe *crsqe;
 
+	crsqe = &rcfw->crsqe_tbl[cookie];
 	cmdq = &rcfw->cmdq;
 
 	if (time_after(jiffies, cmdq->last_seen +
 		      (rcfw->max_timeout * HZ))) {
 		dev_warn_ratelimited(&rcfw->pdev->dev,
 				     "%s: FW STALL Detected. cmdq[%#x]=%#x waited (%d > %d) msec active %d ",
-				     __func__, cookie, opcode,
+				     __func__, cookie, crsqe->opcode,
 				     jiffies_to_msecs(jiffies - cmdq->last_seen),
 				     rcfw->max_timeout * 1000,
-				     in_used);
+				     crsqe->is_in_used);
 		return -ENODEV;
 	}
 
@@ -129,7 +128,6 @@ static int bnxt_re_is_fw_stalled(struct bnxt_qplib_rcfw *rcfw,
  * __wait_for_resp   -	Don't hold the cpu context and wait for response
  * @rcfw      -   rcfw channel instance of rdev
  * @cookie    -   cookie to track the command
- * @opcode    -   rcfw submitted for given opcode
  *
  * Wait for command completion in sleepable context.
  *
@@ -137,7 +135,7 @@ static int bnxt_re_is_fw_stalled(struct bnxt_qplib_rcfw *rcfw,
  * 0 if command is completed by firmware.
  * Non zero error code for rest of the case.
  */
-static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
+static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
 	struct bnxt_qplib_crsqe *crsqe;
@@ -148,7 +146,7 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
-			return bnxt_qplib_map_rc(opcode);
+			return bnxt_qplib_map_rc(crsqe->opcode);
 		if (test_bit(FIRMWARE_STALL_DETECTED, &cmdq->flags))
 			return -ETIMEDOUT;
 
@@ -165,7 +163,7 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 		if (!crsqe->is_in_used)
 			return 0;
 
-		ret = bnxt_re_is_fw_stalled(rcfw, cookie, opcode, crsqe->is_in_used);
+		ret = bnxt_re_is_fw_stalled(rcfw, cookie);
 		if (ret)
 			return ret;
 
@@ -176,7 +174,6 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
  * __block_for_resp   -	hold the cpu context and wait for response
  * @rcfw      -   rcfw channel instance of rdev
  * @cookie    -   cookie to track the command
- * @opcode    -   rcfw submitted for given opcode
  *
  * This function will hold the cpu (non-sleepable context) and
  * wait for command completion. Maximum holding interval is 8 second.
@@ -185,7 +182,7 @@ static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
  * -ETIMEOUT if command is not completed in specific time interval.
  * 0 if command is completed by firmware.
  */
-static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
+static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
 	struct bnxt_qplib_crsqe *crsqe;
@@ -196,7 +193,7 @@ static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie, u8 opcode)
 
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
-			return bnxt_qplib_map_rc(opcode);
+			return bnxt_qplib_map_rc(crsqe->opcode);
 		if (test_bit(FIRMWARE_STALL_DETECTED, &cmdq->flags))
 			return -ETIMEDOUT;
 
@@ -372,7 +369,6 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
  * __poll_for_resp   -	self poll completion for rcfw command
  * @rcfw      -   rcfw channel instance of rdev
  * @cookie    -   cookie to track the command
- * @opcode    -   rcfw submitted for given opcode
  *
  * It works same as __wait_for_resp except this function will
  * do self polling in sort interval since interrupt is disabled.
@@ -382,8 +378,7 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
  * -ETIMEOUT if command is not completed in specific time interval.
  * 0 if command is completed by firmware.
  */
-static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
-			   u8 opcode)
+static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
 	struct bnxt_qplib_crsqe *crsqe;
@@ -395,7 +390,7 @@ static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
 
 	do {
 		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
-			return bnxt_qplib_map_rc(opcode);
+			return bnxt_qplib_map_rc(crsqe->opcode);
 		if (test_bit(FIRMWARE_STALL_DETECTED, &cmdq->flags))
 			return -ETIMEDOUT;
 
@@ -406,7 +401,7 @@ static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
 			return 0;
 		if (jiffies_to_msecs(jiffies - issue_time) >
 		    (rcfw->max_timeout * 1000)) {
-			ret = bnxt_re_is_fw_stalled(rcfw, cookie, opcode, crsqe->is_in_used);
+			ret = bnxt_re_is_fw_stalled(rcfw, cookie);
 			if (ret)
 				return ret;
 		}
@@ -414,13 +409,12 @@ static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
 };
 
 static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
-				       struct bnxt_qplib_cmdqmsg *msg)
+				       struct bnxt_qplib_cmdqmsg *msg,
+				       u8 opcode)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
-	u32 opcode;
 
 	cmdq = &rcfw->cmdq;
-	opcode = __get_cmdq_base_opcode(msg->req, msg->req_sz);
 
 	/* Prevent posting if f/w is not in a state to process */
 	if (test_bit(ERR_DEVICE_DETACHED, &rcfw->cmdq.flags))
@@ -492,7 +486,7 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 
 	opcode = __get_cmdq_base_opcode(msg->req, msg->req_sz);
 
-	rc = __send_message_basic_sanity(rcfw, msg);
+	rc = __send_message_basic_sanity(rcfw, msg, opcode);
 	if (rc)
 		return rc == -ENXIO ? bnxt_qplib_map_rc(opcode) : rc;
 
@@ -504,11 +498,11 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 				& RCFW_MAX_COOKIE_VALUE;
 
 	if (msg->block)
-		rc = __block_for_resp(rcfw, cookie, opcode);
+		rc = __block_for_resp(rcfw, cookie);
 	else if (atomic_read(&rcfw->rcfw_intr_enabled))
-		rc = __wait_for_resp(rcfw, cookie, opcode);
+		rc = __wait_for_resp(rcfw, cookie);
 	else
-		rc = __poll_for_resp(rcfw, cookie, opcode);
+		rc = __poll_for_resp(rcfw, cookie);
 	if (rc) {
 		/* timed out */
 		dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
-- 
2.5.5


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]

Re: [PATCH v2 for-next 05/17] RDMA/bnxt_re: Enhance the existing functions that wait for FW responses

[Leon Romanovsky]

On Fri, Jun 09, 2023 at 04:01:42AM -0700, Selvin Xavier wrote:
> From: Kashyap Desai <kashyap.desai@broadcom.com>
> 
> Use jiffies based timewait instead of counting iteration for
> commands that block for FW response.
> 
> Also add a poll routine for control path commands. This is for
> polling completion if the waiting commands timeout. This avoids cases
> where the driver misses completion interrupts.
> 
> Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 65 +++++++++++++++++++++++-------
>  1 file changed, 51 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> index 66121fb..3b242cc 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> @@ -53,37 +53,74 @@
>  
>  static void bnxt_qplib_service_creq(struct tasklet_struct *t);
>  
> -/* Hardware communication channel */
> +/**
> + * __wait_for_resp   -	Don't hold the cpu context and wait for response
> + * @rcfw      -   rcfw channel instance of rdev
> + * @cookie    -   cookie to track the command
> + *
> + * Wait for command completion in sleepable context.
> + *
> + * Returns:
> + * 0 if command is completed by firmware.
> + * Non zero error code for rest of the case.

I don't see "return ret;" in this function.

> + */
>  static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
>  {
>  	struct bnxt_qplib_cmdq_ctx *cmdq;
>  	u16 cbit;
> -	int rc;
> +	int ret;
>  
>  	cmdq = &rcfw->cmdq;
>  	cbit = cookie % rcfw->cmdq_depth;
> -	rc = wait_event_timeout(cmdq->waitq,
> -				!test_bit(cbit, cmdq->cmdq_bitmap),
> -				msecs_to_jiffies(RCFW_CMD_WAIT_TIME_MS));
> -	return rc ? 0 : -ETIMEDOUT;
> +
> +	do {
> +		/* Non zero means command completed */
> +		ret = wait_event_timeout(cmdq->waitq,
> +					 !test_bit(cbit, cmdq->cmdq_bitmap),
> +					 msecs_to_jiffies(10000));

Don't you need to check ret here?

> +
> +		if (!test_bit(cbit, cmdq->cmdq_bitmap))
> +			return 0;
> +
> +		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
> +
> +		if (!test_bit(cbit, cmdq->cmdq_bitmap))
> +			return 0;
> +
> +	} while (true);
>  };
>  
> +/**
> + * __block_for_resp   -	hold the cpu context and wait for response
> + * @rcfw      -   rcfw channel instance of rdev
> + * @cookie    -   cookie to track the command
> + *
> + * This function will hold the cpu (non-sleepable context) and
> + * wait for command completion. Maximum holding interval is 8 second.
> + *
> + * Returns:
> + * -ETIMEOUT if command is not completed in specific time interval.
> + * 0 if command is completed by firmware.
> + */
>  static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
>  {
> -	u32 count = RCFW_BLOCKED_CMD_WAIT_COUNT;
> -	struct bnxt_qplib_cmdq_ctx *cmdq;
> +	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
> +	unsigned long issue_time = 0;
>  	u16 cbit;
>  
> -	cmdq = &rcfw->cmdq;
>  	cbit = cookie % rcfw->cmdq_depth;
> -	if (!test_bit(cbit, cmdq->cmdq_bitmap))
> -		goto done;
> +	issue_time = jiffies;
> +
>  	do {
>  		udelay(1);
> +
>  		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
> -	} while (test_bit(cbit, cmdq->cmdq_bitmap) && --count);
> -done:
> -	return count ? 0 : -ETIMEDOUT;
> +		if (!test_bit(cbit, cmdq->cmdq_bitmap))
> +			return 0;
> +
> +	} while (time_before(jiffies, issue_time + (8 * HZ)));
> +
> +	return -ETIMEDOUT;
>  };
>  
>  static int __send_message(struct bnxt_qplib_rcfw *rcfw,
> -- 
> 2.5.5
> 

Re: [PATCH v2 for-next 09/17] RDMA/bnxt_re: add helper function __poll_for_resp

[Leon Romanovsky]

On Fri, Jun 09, 2023 at 04:01:46AM -0700, Selvin Xavier wrote:
> From: Kashyap Desai <kashyap.desai@broadcom.com>
> 
> This interface will be used if the driver has not enabled interrupt
> and/or interrupt is disabled for a short period of time.
> Completion is not possible from interrupt so this interface does
> self-polling.
> 
> Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 44 +++++++++++++++++++++++++++++-
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  1 +
>  2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> index 15f6793..3215f8a 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> @@ -260,6 +260,44 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
>  	return 0;
>  }
>  
> +/**
> + * __poll_for_resp   -	self poll completion for rcfw command
> + * @rcfw      -   rcfw channel instance of rdev
> + * @cookie    -   cookie to track the command
> + * @opcode    -   rcfw submitted for given opcode
> + *
> + * It works same as __wait_for_resp except this function will
> + * do self polling in sort interval since interrupt is disabled.
> + * This function can not be called from non-sleepable context.
> + *
> + * Returns:
> + * -ETIMEOUT if command is not completed in specific time interval.
> + * 0 if command is completed by firmware.
> + */
> +static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
> +			   u8 opcode)
> +{
> +	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
> +	unsigned long issue_time;
> +	u16 cbit;
> +
> +	cbit = cookie % rcfw->cmdq_depth;
> +	issue_time = jiffies;
> +
> +	do {
> +		if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
> +			return bnxt_qplib_map_rc(opcode);
> +
> +		usleep_range(1000, 1001);
> +
> +		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
> +		if (!test_bit(cbit, cmdq->cmdq_bitmap))
> +			return 0;
> +		if (jiffies_to_msecs(jiffies - issue_time) > 10000)
> +			return -ETIMEDOUT;
> +	} while (true);
> +};
> +
>  static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
>  				       struct bnxt_qplib_cmdqmsg *msg)
>  {
> @@ -328,8 +366,10 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
>  
>  	if (msg->block)
>  		rc = __block_for_resp(rcfw, cookie, opcode);
> -	else
> +	else if (atomic_read(&rcfw->rcfw_intr_enabled))

Why atomic_t? It doesn't eliminate the need of locking and if locking
exists, you don't need atomic_t.

>  		rc = __wait_for_resp(rcfw, cookie, opcode);
> +	else
> +		rc = __poll_for_resp(rcfw, cookie, opcode);
>  	if (rc) {
>  		/* timed out */
>  		dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
> @@ -796,6 +836,7 @@ void bnxt_qplib_rcfw_stop_irq(struct bnxt_qplib_rcfw *rcfw, bool kill)
>  	kfree(creq->irq_name);
>  	creq->irq_name = NULL;
>  	creq->requested = false;
> +	atomic_set(&rcfw->rcfw_intr_enabled, 0);
>  }
>  
>  void bnxt_qplib_disable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw)
> @@ -857,6 +898,7 @@ int bnxt_qplib_rcfw_start_irq(struct bnxt_qplib_rcfw *rcfw, int msix_vector,
>  	creq->requested = true;
>  
>  	bnxt_qplib_ring_nq_db(&creq->creq_db.dbinfo, res->cctx, true);
> +	atomic_inc(&rcfw->rcfw_intr_enabled);
>  
>  	return 0;
>  }
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> index b7bbbae..089e616 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> @@ -221,6 +221,7 @@ struct bnxt_qplib_rcfw {
>  	u64 oos_prev;
>  	u32 init_oos_stats;
>  	u32 cmdq_depth;
> +	atomic_t rcfw_intr_enabled;
>  	struct semaphore rcfw_inflight;
>  };
>  
> -- 
> 2.5.5
> 

Re: [PATCH v2 for-next 10/17] RDMA/bnxt_re: handle command completions after driver detect a timedout

[Leon Romanovsky]

On Fri, Jun 09, 2023 at 04:01:47AM -0700, Selvin Xavier wrote:
> From: Kashyap Desai <kashyap.desai@broadcom.com>
> 
> If calling context detect command timeout, associated memory stored on
> stack will not be valid. If firmware complete the same command later,
> this causes incorrect memory access by driver.
> 
> Added is_waiter_alive to handle delayed completion by firmware.
> is_waiter_alive is set and reset under command queue lock.
> 
> Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 59 +++++++++++++++++-------------
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  1 +
>  2 files changed, 34 insertions(+), 26 deletions(-)

<...>

>  		/* Non zero means command completed */

This comment is not valid anymore.

> -		ret = wait_event_timeout(cmdq->waitq,
> -					 !test_bit(cbit, cmdq->cmdq_bitmap),
> -					 msecs_to_jiffies(10000));
> +		wait_event_timeout(cmdq->waitq,
> +				   !test_bit(cbit, cmdq->cmdq_bitmap),
> +				   msecs_to_jiffies(10000));

Thanks

Re: [PATCH v2 for-next 00/17] RDMA/bnxt_re: Control path updates

[Leon Romanovsky]

On Fri, Jun 09, 2023 at 04:01:37AM -0700, Selvin Xavier wrote:
> This patch series from Kashyap includes code refactoring and some
> optimizations in the FW control path of the driver. It also address
> some of the issues seen as we scale up the HW resources.
> It also includes few bug fixes in the control path.
> 
> Please review and apply.
> 
> Thanks,
> Selvin Xavier
> 
> v1 -> v2:
>  - Fix the warning reported by kbuild test robot in patch 10 about
>    variables set and not used. Rebased the other patches based on
>    the modified patch.
>  - Reported-by: kernel test robot <lkp@intel.com>
> 
> Kashyap Desai (17):
>   RDMA/bnxt_re: wraparound mbox producer index
>   RDMA/bnxt_re: Avoid calling wake_up threads from spin_lock context
>   RDMA/bnxt_re: remove virt_func check while creating RoCE FW channel
>   RDMA/bnxt_re: set fixed command queue depth
>   RDMA/bnxt_re: Enhance the existing functions that wait for FW
>     responses
>   RDMA/bnxt_re: Avoid the command wait if firmware is inactive
>   RDMA/bnxt_re: use shadow qd while posting non blocking rcfw command
>   RDMA/bnxt_re: Simplify the function that sends the FW commands
>   RDMA/bnxt_re: add helper function __poll_for_resp
>   RDMA/bnxt_re: handle command completions after driver detect a
>     timedout
>   RDMA/bnxt_re: Add firmware stall check detection
>   RDMA/bnxt_re: post destroy_ah for delayed completion of AH creation
>   RDMA/bnxt_re: consider timeout of destroy ah as success.
>   RDMA/bnxt_re: cancel all control path command waiters upon error.
>   RDMA/bnxt_re: use firmware provided max request timeout
>   RDMA/bnxt_re: remove redundant cmdq_bitmap
>   RDMA/bnxt_re: optimize the parameters passed to helper functions

I applied whole series as is. It looks good enough, but please address
my concern about atomit_t.

Thanks

> 
>  drivers/infiniband/hw/bnxt_re/bnxt_re.h    |   2 +
>  drivers/infiniband/hw/bnxt_re/ib_verbs.c   |  16 +-
>  drivers/infiniband/hw/bnxt_re/main.c       |  11 +-
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 627 ++++++++++++++++++++++-------
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  51 ++-
>  drivers/infiniband/hw/bnxt_re/qplib_res.h  |   1 +
>  drivers/infiniband/hw/bnxt_re/qplib_sp.c   |   8 +-
>  drivers/infiniband/hw/bnxt_re/qplib_sp.h   |   4 +-
>  8 files changed, 557 insertions(+), 163 deletions(-)

> 
> -- 
> 2.5.5
> 

Re: [PATCH v2 for-next 00/17] RDMA/bnxt_re: Control path updates

[Leon Romanovsky]

On Fri, 09 Jun 2023 04:01:37 -0700, Selvin Xavier wrote:
> This patch series from Kashyap includes code refactoring and some
> optimizations in the FW control path of the driver. It also address
> some of the issues seen as we scale up the HW resources.
> It also includes few bug fixes in the control path.
> 
> Please review and apply.
> 
> [...]

Applied, thanks!

[01/17] RDMA/bnxt_re: wraparound mbox producer index
        https://git.kernel.org/rdma/rdma/c/0af91306e17ef3
[02/17] RDMA/bnxt_re: Avoid calling wake_up threads from spin_lock context
        https://git.kernel.org/rdma/rdma/c/3099bcdc19b701
[03/17] RDMA/bnxt_re: remove virt_func check while creating RoCE FW channel
        https://git.kernel.org/rdma/rdma/c/b021186bca9d6b
[04/17] RDMA/bnxt_re: set fixed command queue depth
        https://git.kernel.org/rdma/rdma/c/258ee04317dacf
[05/17] RDMA/bnxt_re: Enhance the existing functions that wait for FW responses
        https://git.kernel.org/rdma/rdma/c/8cf1d12ad56beb
[06/17] RDMA/bnxt_re: Avoid the command wait if firmware is inactive
        https://git.kernel.org/rdma/rdma/c/3022cc15119733
[07/17] RDMA/bnxt_re: use shadow qd while posting non blocking rcfw command
        https://git.kernel.org/rdma/rdma/c/65288a22ddd814
[08/17] RDMA/bnxt_re: Simplify the function that sends the FW commands
        https://git.kernel.org/rdma/rdma/c/159cf95e42a7ca
[09/17] RDMA/bnxt_re: add helper function __poll_for_resp
        https://git.kernel.org/rdma/rdma/c/354f5bd985af95
[10/17] RDMA/bnxt_re: handle command completions after driver detect a timedout
        https://git.kernel.org/rdma/rdma/c/691eb7c6110fe0
[11/17] RDMA/bnxt_re: Add firmware stall check detection
        https://git.kernel.org/rdma/rdma/c/b6c7256688264f
[12/17] RDMA/bnxt_re: post destroy_ah for delayed completion of AH creation
        https://git.kernel.org/rdma/rdma/c/84911cf3b2aa8d
[13/17] RDMA/bnxt_re: consider timeout of destroy ah as success.
        https://git.kernel.org/rdma/rdma/c/bb8c93618fb0b8
[14/17] RDMA/bnxt_re: cancel all control path command waiters upon error.
        https://git.kernel.org/rdma/rdma/c/a00278521c9107
[15/17] RDMA/bnxt_re: use firmware provided max request timeout
        https://git.kernel.org/rdma/rdma/c/f0c875ff629396
[16/17] RDMA/bnxt_re: remove redundant cmdq_bitmap
        https://git.kernel.org/rdma/rdma/c/bcfee4ce3e0139
[17/17] RDMA/bnxt_re: optimize the parameters passed to helper functions
        https://git.kernel.org/rdma/rdma/c/830f93f47068b1

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>

Re: [PATCH v2 for-next 09/17] RDMA/bnxt_re: add helper function __poll_for_resp

[Selvin Xavier]

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

On Mon, Jun 12, 2023 at 12:34 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Fri, Jun 09, 2023 at 04:01:46AM -0700, Selvin Xavier wrote:
> > From: Kashyap Desai <kashyap.desai@broadcom.com>
> >
> > This interface will be used if the driver has not enabled interrupt
> > and/or interrupt is disabled for a short period of time.
> > Completion is not possible from interrupt so this interface does
> > self-polling.
> >
> > Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
> > ---
> >  drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 44 +++++++++++++++++++++++++++++-
> >  drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  1 +
> >  2 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> > index 15f6793..3215f8a 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> > @@ -260,6 +260,44 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
> >       return 0;
> >  }
> >
> > +/**
> > + * __poll_for_resp   -       self poll completion for rcfw command
> > + * @rcfw      -   rcfw channel instance of rdev
> > + * @cookie    -   cookie to track the command
> > + * @opcode    -   rcfw submitted for given opcode
> > + *
> > + * It works same as __wait_for_resp except this function will
> > + * do self polling in sort interval since interrupt is disabled.
> > + * This function can not be called from non-sleepable context.
> > + *
> > + * Returns:
> > + * -ETIMEOUT if command is not completed in specific time interval.
> > + * 0 if command is completed by firmware.
> > + */
> > +static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
> > +                        u8 opcode)
> > +{
> > +     struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
> > +     unsigned long issue_time;
> > +     u16 cbit;
> > +
> > +     cbit = cookie % rcfw->cmdq_depth;
> > +     issue_time = jiffies;
> > +
> > +     do {
> > +             if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
> > +                     return bnxt_qplib_map_rc(opcode);
> > +
> > +             usleep_range(1000, 1001);
> > +
> > +             bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
> > +             if (!test_bit(cbit, cmdq->cmdq_bitmap))
> > +                     return 0;
> > +             if (jiffies_to_msecs(jiffies - issue_time) > 10000)
> > +                     return -ETIMEDOUT;
> > +     } while (true);
> > +};
> > +
> >  static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
> >                                      struct bnxt_qplib_cmdqmsg *msg)
> >  {
> > @@ -328,8 +366,10 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
> >
> >       if (msg->block)
> >               rc = __block_for_resp(rcfw, cookie, opcode);
> > -     else
> > +     else if (atomic_read(&rcfw->rcfw_intr_enabled))
>
> Why atomic_t? It doesn't eliminate the need of locking and if locking
> exists, you don't need atomic_t.
The intent of rcfw_intr_enabled was to avoid the locks.  But I agree
that it will not help. We will  refactor this
code and move it under a better locking scheme. Will remove atomic
variables in this path.
Thanks
>
> >               rc = __wait_for_resp(rcfw, cookie, opcode);
> > +     else
> > +             rc = __poll_for_resp(rcfw, cookie, opcode);
> >       if (rc) {
> >               /* timed out */
> >               dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
> > @@ -796,6 +836,7 @@ void bnxt_qplib_rcfw_stop_irq(struct bnxt_qplib_rcfw *rcfw, bool kill)
> >       kfree(creq->irq_name);
> >       creq->irq_name = NULL;
> >       creq->requested = false;
> > +     atomic_set(&rcfw->rcfw_intr_enabled, 0);
> >  }
> >
> >  void bnxt_qplib_disable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw)
> > @@ -857,6 +898,7 @@ int bnxt_qplib_rcfw_start_irq(struct bnxt_qplib_rcfw *rcfw, int msix_vector,
> >       creq->requested = true;
> >
> >       bnxt_qplib_ring_nq_db(&creq->creq_db.dbinfo, res->cctx, true);
> > +     atomic_inc(&rcfw->rcfw_intr_enabled);
> >
> >       return 0;
> >  }
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> > index b7bbbae..089e616 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> > @@ -221,6 +221,7 @@ struct bnxt_qplib_rcfw {
> >       u64 oos_prev;
> >       u32 init_oos_stats;
> >       u32 cmdq_depth;
> > +     atomic_t rcfw_intr_enabled;
> >       struct semaphore rcfw_inflight;
> >  };
> >
> > --
> > 2.5.5
> >
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]