[PATCH 00/15] be2iscsi: driver update 11.0.0.0

[PATCH 00/15] be2iscsi: driver update 11.0.0.0

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

This patch is generated against for-next branch.

Jitendra (15):
  be2iscsi: Fix soft lockup in mgmt_get_all_if_id path using bmbx
  be2iscsi: Fix mbox synchronization replacing spinlock with mutex
  be2iscsi: Fix to use atomic operations for tag_state
  be2iscsi: Fix to synchronize tag allocation using spin_lock
  be2iscsi: Set mbox timeout to 30s
  be2iscsi: Added return value check for mgmt_get_all_if_id
  be2iscsi: Fix to remove shutdown entry point
  be2iscsi: Fix VLAN support for IPv6 network
  be2iscsi: Fix to handle misconfigured optics events
  be2iscsi: Add FW config validation
  be2iscsi: Fix return value for MCC completion
  be2iscsi: Fix IOPOLL implementation
  be2iscsi: Fix to process 25G link speed info from FW
  be2iscsi: Fix WRB leak in login/logout path
  be2iscsi: Update the driver version

 drivers/scsi/be2iscsi/be.h          |    4 +-
 drivers/scsi/be2iscsi/be_cmds.c     |  368 ++++++++++++++++++---------------
 drivers/scsi/be2iscsi/be_cmds.h     |  114 ++++++-----
 drivers/scsi/be2iscsi/be_iscsi.c    |   54 ++---
 drivers/scsi/be2iscsi/be_main.c     |  210 ++++++++++---------
 drivers/scsi/be2iscsi/be_main.h     |   23 ++-
 drivers/scsi/be2iscsi/be_mgmt.c     |  387 +++++++++++++++++++++--------------
 drivers/scsi/be2iscsi/be_mgmt.h     |    2 +
 drivers/scsi/scsi_transport_iscsi.c |    2 +
 include/scsi/iscsi_if.h             |    2 +
 10 files changed, 648 insertions(+), 518 deletions(-)

[PATCH 01/15] be2iscsi: Fix soft lockup in mgmt_get_all_if_id path using bmbx

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

We are taking mbox_lock spinlock which disables pre-emption before we poll
for mbox completion. Waiting there with spinlock held in excess of 20s will
cause soft lockup.

Actual fix is to change mbox_lock to mutex.
The changes are done in phases. This is the first part.
1. Changed mgmt_get_all_if_id to use MCC where after posting lock is released.
2. Changed be_mbox_db_ready_wait to busy wait for 12s max and removed
wait_event_timeout. Added error handling code for IO reads.
OPCODE_COMMON_QUERY_FIRMWARE_CONFIG mbox command takes 8s time when unreachable
boot targets configured.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_cmds.c |   60 +++++++++++++++++++--------------------
 drivers/scsi/be2iscsi/be_mgmt.c |   32 ++++++++++++++------
 2 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 2778089..cd50e3c 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -587,47 +587,42 @@ int be_mcc_notify_wait(struct beiscsi_hba *phba)
  **/
 static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl)
 {
-#define BEISCSI_MBX_RDY_BIT_TIMEOUT	4000	/* 4sec */
+#define BEISCSI_MBX_RDY_BIT_TIMEOUT	12000	/* 12sec */
 	void __iomem *db = ctrl->db + MPU_MAILBOX_DB_OFFSET;
 	struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev);
 	unsigned long timeout;
-	bool read_flag = false;
-	int ret = 0, i;
 	u32 ready;
-	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(rdybit_check_q);
 
-	if (beiscsi_error(phba))
-		return -EIO;
+	/*
+	 * This BMBX busy wait path is used during init only.
+	 * For the commands executed during init, 5s should suffice.
+	 */
+	timeout = jiffies + msecs_to_jiffies(BEISCSI_MBX_RDY_BIT_TIMEOUT);
+	do {
+		if (beiscsi_error(phba))
+			return -EIO;
 
-	timeout = jiffies + (HZ * 110);
+		ready = ioread32(db);
+		if (ready == 0xffffffff)
+			return -EIO;
 
-	do {
-		for (i = 0; i < BEISCSI_MBX_RDY_BIT_TIMEOUT; i++) {
-			ready = ioread32(db) & MPU_MAILBOX_DB_RDY_MASK;
-			if (ready) {
-				read_flag = true;
-				break;
-			}
-			mdelay(1);
-		}
+		ready &= MPU_MAILBOX_DB_RDY_MASK;
+		if (ready)
+			return 0;
 
-		if (!read_flag) {
-			wait_event_timeout(rdybit_check_q,
-					  (read_flag != true),
-					   HZ * 5);
-		}
-	} while ((time_before(jiffies, timeout)) && !read_flag);
+		if (time_after(jiffies, timeout))
+			break;
+		mdelay(1);
+	} while (!ready);
 
-	if (!read_flag) {
-		beiscsi_log(phba, KERN_ERR,
-			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
-			    "BC_%d : FW Timed Out\n");
-			phba->fw_timeout = true;
-			beiscsi_ue_detect(phba);
-			ret = -EBUSY;
-	}
+	beiscsi_log(phba, KERN_ERR,
+			BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
+			"BC_%d : FW Timed Out\n");
 
-	return ret;
+	phba->fw_timeout = true;
+	beiscsi_ue_detect(phba);
+
+	return -EBUSY;
 }
 
 /*
@@ -674,6 +669,9 @@ int be_mbox_notify(struct be_ctrl_info *ctrl)
 	if (status)
 		return status;
 
+	/* RDY is set; small delay before CQE read. */
+	udelay(1);
+
 	if (be_mcc_compl_is_new(compl)) {
 		status = be_mcc_compl_process(ctrl, &mbox->compl);
 		be_mcc_compl_use(compl);
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index aea3e6b..7b54b23 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -809,27 +809,39 @@ int mgmt_open_connection(struct beiscsi_hba *phba,
 unsigned int mgmt_get_all_if_id(struct beiscsi_hba *phba)
 {
 	struct be_ctrl_info *ctrl = &phba->ctrl;
-	struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem);
-	struct be_cmd_get_all_if_id_req *req = embedded_payload(wrb);
-	struct be_cmd_get_all_if_id_req *pbe_allid = req;
+	struct be_mcc_wrb *wrb;
+	struct be_cmd_get_all_if_id_req *req;
+	struct be_cmd_get_all_if_id_req *pbe_allid;
+	unsigned int tag;
 	int status = 0;
 
-	memset(wrb, 0, sizeof(*wrb));
-
 	spin_lock(&ctrl->mbox_lock);
+	tag = alloc_mcc_tag(phba);
+	if (!tag) {
+		spin_unlock(&ctrl->mbox_lock);
+		return -ENOMEM;
+	}
+
+	wrb = wrb_from_mccq(phba);
+	req = embedded_payload(wrb);
+	wrb->tag0 |= tag;
 
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
 	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_ISCSI,
 			   OPCODE_COMMON_ISCSI_NTWK_GET_ALL_IF_ID,
 			   sizeof(*req));
-	status = be_mbox_notify(ctrl);
-	if (!status)
-		phba->interface_handle = pbe_allid->if_hndl_list[0];
-	else {
+	be_mcc_notify(phba);
+	spin_unlock(&ctrl->mbox_lock);
+
+	status = beiscsi_mccq_compl(phba, tag, &wrb, NULL);
+	if (status) {
 		beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
 			    "BG_%d : Failed in mgmt_get_all_if_id\n");
+		return -EBUSY;
 	}
-	spin_unlock(&ctrl->mbox_lock);
+
+	pbe_allid = embedded_payload(wrb);
+	phba->interface_handle = pbe_allid->if_hndl_list[0];
 
 	return status;
 }
-- 
1.7.1

Re: [PATCH 01/15] be2iscsi: Fix soft lockup in mgmt_get_all_if_id path using bmbx

[Hannes Reinecke]

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> We are taking mbox_lock spinlock which disables pre-emption before we poll
> for mbox completion. Waiting there with spinlock held in excess of 20s will
> cause soft lockup.
>
> Actual fix is to change mbox_lock to mutex.
> The changes are done in phases. This is the first part.
> 1. Changed mgmt_get_all_if_id to use MCC where after posting lock is released.
> 2. Changed be_mbox_db_ready_wait to busy wait for 12s max and removed
> wait_event_timeout. Added error handling code for IO reads.
> OPCODE_COMMON_QUERY_FIRMWARE_CONFIG mbox command takes 8s time when unreachable
> boot targets configured.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_cmds.c |   60 +++++++++++++++++++--------------------
>   drivers/scsi/be2iscsi/be_mgmt.c |   32 ++++++++++++++------
>   2 files changed, 51 insertions(+), 41 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/15] be2iscsi: Fix mbox synchronization replacing spinlock with mutex

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

This is second part of actual fix for soft lockup.

All mbox cmds issued using BMBX and MCC are synchronized using mutex
mbox_lock instead of spin_lock. Used mutex_lock_interruptible where
ever possible.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be.h      |    2 +-
 drivers/scsi/be2iscsi/be_cmds.c |   73 ++++++++++++++-------------
 drivers/scsi/be2iscsi/be_main.c |    2 +-
 drivers/scsi/be2iscsi/be_mgmt.c |  105 ++++++++++++++++++++------------------
 4 files changed, 94 insertions(+), 88 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
index 77f992e..cf19bce 100644
--- a/drivers/scsi/be2iscsi/be.h
+++ b/drivers/scsi/be2iscsi/be.h
@@ -124,7 +124,7 @@ struct be_ctrl_info {
 	struct pci_dev *pdev;
 
 	/* Mbox used for cmd request/response */
-	spinlock_t mbox_lock;	/* For serializing mbox cmds to BE card */
+	struct mutex mbox_lock;	/* For serializing mbox cmds to BE card */
 	struct be_dma_mem mbox_mem;
 	/* Mbox mem is adjusted to align to 16 bytes. The allocated addr
 	 * is stored for freeing purpose */
diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index cd50e3c..6fabded 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -164,9 +164,9 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 	}
 
 	/* Set MBX Tag state to Active */
-	spin_lock(&phba->ctrl.mbox_lock);
+	mutex_lock(&phba->ctrl.mbox_lock);
 	phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_RUNNING;
-	spin_unlock(&phba->ctrl.mbox_lock);
+	mutex_unlock(&phba->ctrl.mbox_lock);
 
 	/* wait for the mccq completion */
 	rc = wait_event_interruptible_timeout(
@@ -178,9 +178,9 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 	if (rc <= 0) {
 		struct be_dma_mem *tag_mem;
 		/* Set MBX Tag state to timeout */
-		spin_lock(&phba->ctrl.mbox_lock);
+		mutex_lock(&phba->ctrl.mbox_lock);
 		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_TIMEOUT;
-		spin_unlock(&phba->ctrl.mbox_lock);
+		mutex_unlock(&phba->ctrl.mbox_lock);
 
 		/* Store resource addr to be freed later */
 		tag_mem = &phba->ctrl.ptag_state[tag].tag_mem_state;
@@ -199,9 +199,9 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 	} else {
 		rc = 0;
 		/* Set MBX Tag state to completed */
-		spin_lock(&phba->ctrl.mbox_lock);
+		mutex_lock(&phba->ctrl.mbox_lock);
 		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
-		spin_unlock(&phba->ctrl.mbox_lock);
+		mutex_unlock(&phba->ctrl.mbox_lock);
 	}
 
 	mcc_tag_response = phba->ctrl.mcc_numtag[tag];
@@ -390,9 +390,9 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
 					    tag_mem->va, tag_mem->dma);
 
 		/* Change tag state */
-		spin_lock(&phba->ctrl.mbox_lock);
+		mutex_lock(&phba->ctrl.mbox_lock);
 		ctrl->ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
-		spin_unlock(&phba->ctrl.mbox_lock);
+		mutex_unlock(&phba->ctrl.mbox_lock);
 
 		/* Free MCC Tag */
 		free_mcc_tag(ctrl, tag);
@@ -831,7 +831,7 @@ int beiscsi_cmd_eq_create(struct be_ctrl_info *ctrl,
 	struct be_dma_mem *q_mem = &eq->dma_mem;
 	int status;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -858,7 +858,7 @@ int beiscsi_cmd_eq_create(struct be_ctrl_info *ctrl,
 		eq->id = le16_to_cpu(resp->eq_id);
 		eq->created = true;
 	}
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -879,7 +879,7 @@ int be_cmd_fw_initialize(struct be_ctrl_info *ctrl)
 	int status;
 	u8 *endian_check;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 
 	endian_check = (u8 *) wrb;
@@ -898,7 +898,7 @@ int be_cmd_fw_initialize(struct be_ctrl_info *ctrl)
 		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
 			    "BC_%d : be_cmd_fw_initialize Failed\n");
 
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -919,7 +919,7 @@ int be_cmd_fw_uninit(struct be_ctrl_info *ctrl)
 	int status;
 	u8 *endian_check;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 
 	endian_check = (u8 *) wrb;
@@ -939,7 +939,7 @@ int be_cmd_fw_uninit(struct be_ctrl_info *ctrl)
 		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
 			    "BC_%d : be_cmd_fw_uninit Failed\n");
 
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -955,7 +955,7 @@ int beiscsi_cmd_cq_create(struct be_ctrl_info *ctrl,
 	void *ctxt = &req->context;
 	int status;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -1005,7 +1005,7 @@ int beiscsi_cmd_cq_create(struct be_ctrl_info *ctrl,
 			    "BC_%d : In be_cmd_cq_create, status=ox%08x\n",
 			    status);
 
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 
 	return status;
 }
@@ -1029,7 +1029,7 @@ int beiscsi_cmd_mccq_create(struct beiscsi_hba *phba,
 	void *ctxt;
 	int status;
 
-	spin_lock(&phba->ctrl.mbox_lock);
+	mutex_lock(&phba->ctrl.mbox_lock);
 	ctrl = &phba->ctrl;
 	wrb = wrb_from_mbox(&ctrl->mbox_mem);
 	memset(wrb, 0, sizeof(*wrb));
@@ -1060,7 +1060,7 @@ int beiscsi_cmd_mccq_create(struct beiscsi_hba *phba,
 		mccq->id = le16_to_cpu(resp->id);
 		mccq->created = true;
 	}
-	spin_unlock(&phba->ctrl.mbox_lock);
+	mutex_unlock(&phba->ctrl.mbox_lock);
 
 	return status;
 }
@@ -1078,7 +1078,7 @@ int beiscsi_cmd_q_destroy(struct be_ctrl_info *ctrl, struct be_queue_info *q,
 		    "BC_%d : In beiscsi_cmd_q_destroy "
 		    "queue_type : %d\n", queue_type);
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
 
@@ -1108,7 +1108,7 @@ int beiscsi_cmd_q_destroy(struct be_ctrl_info *ctrl, struct be_queue_info *q,
 		opcode = OPCODE_COMMON_ISCSI_CFG_REMOVE_SGL_PAGES;
 		break;
 	default:
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		BUG();
 		return -ENXIO;
 	}
@@ -1118,7 +1118,7 @@ int beiscsi_cmd_q_destroy(struct be_ctrl_info *ctrl, struct be_queue_info *q,
 
 	status = be_mbox_notify(ctrl);
 
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -1153,7 +1153,7 @@ int be_cmd_create_default_pdu_queue(struct be_ctrl_info *ctrl,
 	void *ctxt = &req->context;
 	int status;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -1225,7 +1225,7 @@ int be_cmd_create_default_pdu_queue(struct be_ctrl_info *ctrl,
 			defq_ring->doorbell_offset = resp->doorbell_offset;
 		}
 	}
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 
 	return status;
 }
@@ -1253,7 +1253,7 @@ int be_cmd_wrbq_create(struct be_ctrl_info *ctrl,
 	struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev);
 	int status;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -1284,7 +1284,7 @@ int be_cmd_wrbq_create(struct be_ctrl_info *ctrl,
 			pwrb_context->doorbell_offset = resp->doorbell_offset;
 		}
 	}
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -1295,7 +1295,7 @@ int be_cmd_iscsi_post_template_hdr(struct be_ctrl_info *ctrl,
 	struct be_post_template_pages_req *req = embedded_payload(wrb);
 	int status;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 
 	memset(wrb, 0, sizeof(*wrb));
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -1308,7 +1308,7 @@ int be_cmd_iscsi_post_template_hdr(struct be_ctrl_info *ctrl,
 	be_cmd_page_addrs_prepare(req->pages, ARRAY_SIZE(req->pages), q_mem);
 
 	status = be_mbox_notify(ctrl);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -1318,7 +1318,7 @@ int be_cmd_iscsi_remove_template_hdr(struct be_ctrl_info *ctrl)
 	struct be_remove_template_pages_req *req = embedded_payload(wrb);
 	int status;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 
 	memset(wrb, 0, sizeof(*wrb));
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -1329,7 +1329,7 @@ int be_cmd_iscsi_remove_template_hdr(struct be_ctrl_info *ctrl)
 	req->type = BEISCSI_TEMPLATE_HDR_TYPE_ISCSI;
 
 	status = be_mbox_notify(ctrl);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -1348,7 +1348,7 @@ int be_cmd_iscsi_post_sgl_pages(struct be_ctrl_info *ctrl,
 	if (num_pages == 0xff)
 		num_pages = 1;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	do {
 		memset(wrb, 0, sizeof(*wrb));
 		be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -1377,7 +1377,7 @@ int be_cmd_iscsi_post_sgl_pages(struct be_ctrl_info *ctrl,
 		}
 	} while (num_pages > 0);
 error:
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	if (status != 0)
 		beiscsi_cmd_q_destroy(ctrl, NULL, QTYPE_SGL);
 	return status;
@@ -1390,7 +1390,7 @@ int beiscsi_cmd_reset_function(struct beiscsi_hba  *phba)
 	struct be_post_sgl_pages_req *req = embedded_payload(wrb);
 	int status;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 
 	req = embedded_payload(wrb);
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -1398,7 +1398,7 @@ int beiscsi_cmd_reset_function(struct beiscsi_hba  *phba)
 			   OPCODE_COMMON_FUNCTION_RESET, sizeof(*req));
 	status = be_mbox_notify_wait(phba);
 
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -1420,10 +1420,11 @@ int be_cmd_set_vlan(struct beiscsi_hba *phba,
 	struct be_cmd_set_vlan_req *req;
 	struct be_ctrl_info *ctrl = &phba->ctrl;
 
-	spin_lock(&ctrl->mbox_lock);
+	if (mutex_lock_interruptible(&ctrl->mbox_lock))
+		return 0;
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -1439,7 +1440,7 @@ int be_cmd_set_vlan(struct beiscsi_hba *phba,
 	req->vlan_priority = vlan_tag;
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 
 	return tag;
 }
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index fe0c514..61ce86b 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -727,7 +727,7 @@ static int be_ctrl_init(struct beiscsi_hba *phba, struct pci_dev *pdev)
 	mbox_mem_align->va = PTR_ALIGN(mbox_mem_alloc->va, 16);
 	mbox_mem_align->dma = PTR_ALIGN(mbox_mem_alloc->dma, 16);
 	memset(mbox_mem_align->va, 0, sizeof(struct be_mcc_mailbox));
-	spin_lock_init(&ctrl->mbox_lock);
+	mutex_init(&ctrl->mbox_lock);
 	spin_lock_init(&phba->ctrl.mcc_lock);
 	spin_lock_init(&phba->ctrl.mcc_cq_lock);
 
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index 7b54b23..a41013e 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -164,10 +164,10 @@ int be_cmd_modify_eq_delay(struct beiscsi_hba *phba,
 	unsigned int tag = 0;
 	int i;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -188,7 +188,7 @@ int be_cmd_modify_eq_delay(struct beiscsi_hba *phba,
 	}
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -215,10 +215,10 @@ unsigned int mgmt_reopen_session(struct beiscsi_hba *phba,
 		    BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
 		    "BG_%d : In bescsi_get_boot_target\n");
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -235,7 +235,7 @@ unsigned int mgmt_reopen_session(struct beiscsi_hba *phba,
 	req->session_handle = sess_handle;
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -250,10 +250,10 @@ unsigned int mgmt_get_boot_target(struct beiscsi_hba *phba)
 		    BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
 		    "BG_%d : In bescsi_get_boot_target\n");
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -266,7 +266,7 @@ unsigned int mgmt_get_boot_target(struct beiscsi_hba *phba)
 			   sizeof(struct be_cmd_get_boot_target_resp));
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -285,10 +285,10 @@ unsigned int mgmt_get_session_info(struct beiscsi_hba *phba,
 		    BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
 		    "BG_%d : In beiscsi_get_session_info\n");
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -311,7 +311,7 @@ unsigned int mgmt_get_session_info(struct beiscsi_hba *phba,
 	sge->len = cpu_to_le32(nonemb_cmd->size);
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -334,7 +334,7 @@ int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
 	struct be_fw_cfg *req = embedded_payload(wrb);
 	int status = 0;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -415,7 +415,7 @@ int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
 		status = -EINVAL;
 	}
 
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -440,7 +440,7 @@ int mgmt_check_supported_fw(struct be_ctrl_info *ctrl,
 	nonemb_cmd.size = sizeof(struct be_mgmt_controller_attributes);
 	req = nonemb_cmd.va;
 	memset(req, 0, sizeof(*req));
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
 	be_wrb_hdr_prepare(wrb, sizeof(*req), false, 1);
 	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
@@ -470,7 +470,7 @@ int mgmt_check_supported_fw(struct be_ctrl_info *ctrl,
 	} else
 		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
 			    "BG_%d :  Failed in mgmt_check_supported_fw\n");
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	if (nonemb_cmd.va)
 		pci_free_consistent(ctrl->pdev, nonemb_cmd.size,
 				    nonemb_cmd.va, nonemb_cmd.dma);
@@ -501,8 +501,9 @@ unsigned int mgmt_vendor_specific_fw_cmd(struct be_ctrl_info *ctrl,
 	req->region = region;
 	req->sector = sector;
 	req->offset = offset;
-	spin_lock(&ctrl->mbox_lock);
 
+	if (mutex_lock_interruptible(&ctrl->mbox_lock))
+		return 0;
 	switch (bsg_req->rqst_data.h_vendor.vendor_cmd[0]) {
 	case BEISCSI_WRITE_FLASH:
 		offset = sector * sector_size + offset;
@@ -521,13 +522,13 @@ unsigned int mgmt_vendor_specific_fw_cmd(struct be_ctrl_info *ctrl,
 			    "BG_%d : Unsupported cmd = 0x%x\n\n",
 			    bsg_req->rqst_data.h_vendor.vendor_cmd[0]);
 
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return -ENOSYS;
 	}
 
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -542,7 +543,7 @@ unsigned int mgmt_vendor_specific_fw_cmd(struct be_ctrl_info *ctrl,
 
 	be_mcc_notify(phba);
 
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -562,7 +563,7 @@ int mgmt_epfw_cleanup(struct beiscsi_hba *phba, unsigned short ulp_num)
 	struct iscsi_cleanup_req *req = embedded_payload(wrb);
 	int status = 0;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
 	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_ISCSI,
@@ -576,7 +577,7 @@ int mgmt_epfw_cleanup(struct beiscsi_hba *phba, unsigned short ulp_num)
 	if (status)
 		beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_INIT,
 			    "BG_%d : mgmt_epfw_cleanup , FAILED\n");
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
 
@@ -592,10 +593,10 @@ unsigned int  mgmt_invalidate_icds(struct beiscsi_hba *phba,
 	struct invalidate_commands_params_in *req;
 	unsigned int i, tag = 0;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -622,7 +623,7 @@ unsigned int  mgmt_invalidate_icds(struct beiscsi_hba *phba,
 	sge->len = cpu_to_le32(nonemb_cmd->size);
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -637,10 +638,10 @@ unsigned int mgmt_invalidate_connection(struct beiscsi_hba *phba,
 	struct iscsi_invalidate_connection_params_in *req;
 	unsigned int tag = 0;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 	wrb = wrb_from_mccq(phba);
@@ -659,7 +660,7 @@ unsigned int mgmt_invalidate_connection(struct beiscsi_hba *phba,
 		req->cleanup_type = CMD_ISCSI_CONNECTION_INVALIDATE;
 	req->save_cfg = savecfg_flag;
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -671,10 +672,10 @@ unsigned int mgmt_upload_connection(struct beiscsi_hba *phba,
 	struct tcp_upload_params_in *req;
 	unsigned int tag = 0;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 	wrb = wrb_from_mccq(phba);
@@ -687,7 +688,7 @@ unsigned int mgmt_upload_connection(struct beiscsi_hba *phba,
 	req->id = (unsigned short)cid;
 	req->upload_type = (unsigned char)upload_flag;
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -732,10 +733,11 @@ int mgmt_open_connection(struct beiscsi_hba *phba,
 
 	ptemplate_address = &template_address;
 	ISCSI_GET_PDU_TEMPLATE_ADDRESS(phba, ptemplate_address);
-	spin_lock(&ctrl->mbox_lock);
+	if (mutex_lock_interruptible(&ctrl->mbox_lock))
+		return 0;
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 	wrb = wrb_from_mccq(phba);
@@ -773,7 +775,7 @@ int mgmt_open_connection(struct beiscsi_hba *phba,
 		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
 			    "BG_%d : unknown addr family %d\n",
 			    dst_addr->sa_family);
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		free_mcc_tag(&phba->ctrl, tag);
 		return -EINVAL;
 
@@ -802,7 +804,7 @@ int mgmt_open_connection(struct beiscsi_hba *phba,
 	}
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -815,10 +817,11 @@ unsigned int mgmt_get_all_if_id(struct beiscsi_hba *phba)
 	unsigned int tag;
 	int status = 0;
 
-	spin_lock(&ctrl->mbox_lock);
+	if (mutex_lock_interruptible(&ctrl->mbox_lock))
+		return -EINTR;
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return -ENOMEM;
 	}
 
@@ -831,7 +834,7 @@ unsigned int mgmt_get_all_if_id(struct beiscsi_hba *phba)
 			   OPCODE_COMMON_ISCSI_NTWK_GET_ALL_IF_ID,
 			   sizeof(*req));
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 
 	status = beiscsi_mccq_compl(phba, tag, &wrb, NULL);
 	if (status) {
@@ -864,10 +867,10 @@ static int mgmt_exec_nonemb_cmd(struct beiscsi_hba *phba,
 	unsigned int tag;
 	int rc = 0;
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		rc = -ENOMEM;
 		goto free_cmd;
 	}
@@ -882,7 +885,7 @@ static int mgmt_exec_nonemb_cmd(struct beiscsi_hba *phba,
 	sge->len = cpu_to_le32(nonemb_cmd->size);
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 
 	rc = beiscsi_mccq_compl(phba, tag, NULL, nonemb_cmd);
 
@@ -1262,10 +1265,11 @@ unsigned int be_cmd_get_initname(struct beiscsi_hba *phba)
 	struct be_cmd_hba_name *req;
 	struct be_ctrl_info *ctrl = &phba->ctrl;
 
-	spin_lock(&ctrl->mbox_lock);
+	if (mutex_lock_interruptible(&ctrl->mbox_lock))
+		return 0;
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -1278,7 +1282,7 @@ unsigned int be_cmd_get_initname(struct beiscsi_hba *phba)
 			sizeof(*req));
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -1289,10 +1293,11 @@ unsigned int be_cmd_get_port_speed(struct beiscsi_hba *phba)
 	struct be_cmd_ntwk_link_status_req *req;
 	struct be_ctrl_info *ctrl = &phba->ctrl;
 
-	spin_lock(&ctrl->mbox_lock);
+	if (mutex_lock_interruptible(&ctrl->mbox_lock))
+		return 0;
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		return tag;
 	}
 
@@ -1305,7 +1310,7 @@ unsigned int be_cmd_get_port_speed(struct beiscsi_hba *phba)
 			sizeof(*req));
 
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
 
@@ -1761,10 +1766,10 @@ int beiscsi_logout_fw_sess(struct beiscsi_hba *phba,
 		    BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
 		    "BG_%d : In bescsi_logout_fwboot_sess\n");
 
-	spin_lock(&ctrl->mbox_lock);
+	mutex_lock(&ctrl->mbox_lock);
 	tag = alloc_mcc_tag(phba);
 	if (!tag) {
-		spin_unlock(&ctrl->mbox_lock);
+		mutex_unlock(&ctrl->mbox_lock);
 		beiscsi_log(phba, KERN_INFO,
 			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
 			    "BG_%d : MBX Tag Failure\n");
@@ -1782,7 +1787,7 @@ int beiscsi_logout_fw_sess(struct beiscsi_hba *phba,
 	/* Set the session handle */
 	req->session_handle = fw_sess_handle;
 	be_mcc_notify(phba);
-	spin_unlock(&ctrl->mbox_lock);
+	mutex_unlock(&ctrl->mbox_lock);
 
 	rc = beiscsi_mccq_compl(phba, tag, &wrb, NULL);
 	if (rc) {
-- 
1.7.1

Re: [PATCH 02/15] be2iscsi: Fix mbox synchronization replacing spinlock with mutex

[Hannes Reinecke]

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> This is second part of actual fix for soft lockup.
>
> All mbox cmds issued using BMBX and MCC are synchronized using mutex
> mbox_lock instead of spin_lock. Used mutex_lock_interruptible where
> ever possible.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be.h      |    2 +-
>   drivers/scsi/be2iscsi/be_cmds.c |   73 ++++++++++++++-------------
>   drivers/scsi/be2iscsi/be_main.c |    2 +-
>   drivers/scsi/be2iscsi/be_mgmt.c |  105 ++++++++++++++++++++------------------
>   4 files changed, 94 insertions(+), 88 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/15] be2iscsi: Fix mbox synchronization replacing spinlock with mutex

[Mike Christie]

On 12/15/2015 09:54 AM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
> 
> This is second part of actual fix for soft lockup.
> 
> All mbox cmds issued using BMBX and MCC are synchronized using mutex
> mbox_lock instead of spin_lock. Used mutex_lock_interruptible where
> ever possible.
> 
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>  drivers/scsi/be2iscsi/be.h      |    2 +-
>  drivers/scsi/be2iscsi/be_cmds.c |   73 ++++++++++++++-------------
>  drivers/scsi/be2iscsi/be_main.c |    2 +-
>  drivers/scsi/be2iscsi/be_mgmt.c |  105 ++++++++++++++++++++------------------
>  4 files changed, 94 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
> index 77f992e..cf19bce 100644
> --- a/drivers/scsi/be2iscsi/be.h
> +++ b/drivers/scsi/be2iscsi/be.h
> @@ -124,7 +124,7 @@ struct be_ctrl_info {
>  	struct pci_dev *pdev;
>  
>  	/* Mbox used for cmd request/response */
> -	spinlock_t mbox_lock;	/* For serializing mbox cmds to BE card */
> +	struct mutex mbox_lock;	/* For serializing mbox cmds to BE card */

You need to update free_mcc_tag. It still does spin_lock calls on the
mbox_lock.

Re: [PATCH 02/15] be2iscsi: Fix mbox synchronization replacing spinlock with mutex

[Mike Christie]

On 12/20/2015 01:35 AM, Mike Christie wrote:
> On 12/15/2015 09:54 AM, Jitendra Bhivare wrote:
>> From: Jitendra <jitendra.bhivare@avagotech.com>
>>
>> This is second part of actual fix for soft lockup.
>>
>> All mbox cmds issued using BMBX and MCC are synchronized using mutex
>> mbox_lock instead of spin_lock. Used mutex_lock_interruptible where
>> ever possible.
>>
>> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
>> ---
>>  drivers/scsi/be2iscsi/be.h      |    2 +-
>>  drivers/scsi/be2iscsi/be_cmds.c |   73 ++++++++++++++-------------
>>  drivers/scsi/be2iscsi/be_main.c |    2 +-
>>  drivers/scsi/be2iscsi/be_mgmt.c |  105 ++++++++++++++++++++------------------
>>  4 files changed, 94 insertions(+), 88 deletions(-)
>>
>> diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
>> index 77f992e..cf19bce 100644
>> --- a/drivers/scsi/be2iscsi/be.h
>> +++ b/drivers/scsi/be2iscsi/be.h
>> @@ -124,7 +124,7 @@ struct be_ctrl_info {
>>  	struct pci_dev *pdev;
>>  
>>  	/* Mbox used for cmd request/response */
>> -	spinlock_t mbox_lock;	/* For serializing mbox cmds to BE card */
>> +	struct mutex mbox_lock;	/* For serializing mbox cmds to BE card */
> 
> You need to update free_mcc_tag. It still does spin_lock calls on the
> mbox_lock.
> 

Ignore this. I see it is done in the 4th patch.

[PATCH 03/15] be2iscsi: Fix to use atomic operations for tag_state

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Replace lock based tag_state manipulations with atomic operations.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be.h      |    2 +-
 drivers/scsi/be2iscsi/be_cmds.c |   26 ++++++++++++--------------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
index cf19bce..419b53f 100644
--- a/drivers/scsi/be2iscsi/be.h
+++ b/drivers/scsi/be2iscsi/be.h
@@ -113,7 +113,7 @@ struct beiscsi_mcc_tag_state {
 #define MCC_TAG_STATE_COMPLETED 0x00
 #define MCC_TAG_STATE_RUNNING   0x01
 #define MCC_TAG_STATE_TIMEOUT   0x02
-	uint8_t tag_state;
+	atomic_t tag_state;
 	struct be_dma_mem tag_mem_state;
 };
 
diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 6fabded..21c806f 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -164,9 +164,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 	}
 
 	/* Set MBX Tag state to Active */
-	mutex_lock(&phba->ctrl.mbox_lock);
-	phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_RUNNING;
-	mutex_unlock(&phba->ctrl.mbox_lock);
+	atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
+			MCC_TAG_STATE_RUNNING);
 
 	/* wait for the mccq completion */
 	rc = wait_event_interruptible_timeout(
@@ -178,9 +177,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 	if (rc <= 0) {
 		struct be_dma_mem *tag_mem;
 		/* Set MBX Tag state to timeout */
-		mutex_lock(&phba->ctrl.mbox_lock);
-		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_TIMEOUT;
-		mutex_unlock(&phba->ctrl.mbox_lock);
+		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
+				MCC_TAG_STATE_TIMEOUT);
 
 		/* Store resource addr to be freed later */
 		tag_mem = &phba->ctrl.ptag_state[tag].tag_mem_state;
@@ -199,9 +197,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 	} else {
 		rc = 0;
 		/* Set MBX Tag state to completed */
-		mutex_lock(&phba->ctrl.mbox_lock);
-		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
-		mutex_unlock(&phba->ctrl.mbox_lock);
+		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
+				MCC_TAG_STATE_COMPLETED);
 	}
 
 	mcc_tag_response = phba->ctrl.mcc_numtag[tag];
@@ -373,9 +370,11 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
 	ctrl->mcc_numtag[tag] |= (extd_status & 0x000000FF) << 8;
 	ctrl->mcc_numtag[tag] |= (compl_status & 0x000000FF);
 
-	if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_RUNNING) {
+	if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
+		MCC_TAG_STATE_RUNNING) {
 		wake_up_interruptible(&ctrl->mcc_wait[tag]);
-	} else if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_TIMEOUT) {
+	} else if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
+			MCC_TAG_STATE_TIMEOUT) {
 		struct be_dma_mem *tag_mem;
 		tag_mem = &ctrl->ptag_state[tag].tag_mem_state;
 
@@ -390,9 +389,8 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
 					    tag_mem->va, tag_mem->dma);
 
 		/* Change tag state */
-		mutex_lock(&phba->ctrl.mbox_lock);
-		ctrl->ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
-		mutex_unlock(&phba->ctrl.mbox_lock);
+		atomic_set(&ctrl->ptag_state[tag].tag_state,
+				MCC_TAG_STATE_COMPLETED);
 
 		/* Free MCC Tag */
 		free_mcc_tag(ctrl, tag);
-- 
1.7.1

Re: [PATCH 03/15] be2iscsi: Fix to use atomic operations for tag_state

[Hannes Reinecke]

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Replace lock based tag_state manipulations with atomic operations.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be.h      |    2 +-
>   drivers/scsi/be2iscsi/be_cmds.c |   26 ++++++++++++--------------
>   2 files changed, 13 insertions(+), 15 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/15] be2iscsi: Fix to use atomic operations for tag_state

[Mike Christie]

On 12/15/2015 09:54 AM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
> 
> Replace lock based tag_state manipulations with atomic operations.
> 
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>  drivers/scsi/be2iscsi/be.h      |    2 +-
>  drivers/scsi/be2iscsi/be_cmds.c |   26 ++++++++++++--------------
>  2 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
> index cf19bce..419b53f 100644
> --- a/drivers/scsi/be2iscsi/be.h
> +++ b/drivers/scsi/be2iscsi/be.h
> @@ -113,7 +113,7 @@ struct beiscsi_mcc_tag_state {
>  #define MCC_TAG_STATE_COMPLETED 0x00
>  #define MCC_TAG_STATE_RUNNING   0x01
>  #define MCC_TAG_STATE_TIMEOUT   0x02
> -	uint8_t tag_state;
> +	atomic_t tag_state;
>  	struct be_dma_mem tag_mem_state;
>  };
>  
> diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
> index 6fabded..21c806f 100644
> --- a/drivers/scsi/be2iscsi/be_cmds.c
> +++ b/drivers/scsi/be2iscsi/be_cmds.c
> @@ -164,9 +164,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>  	}
>  
>  	/* Set MBX Tag state to Active */
> -	mutex_lock(&phba->ctrl.mbox_lock);
> -	phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_RUNNING;
> -	mutex_unlock(&phba->ctrl.mbox_lock);
> +	atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
> +			MCC_TAG_STATE_RUNNING);
>  
>  	/* wait for the mccq completion */
>  	rc = wait_event_interruptible_timeout(
> @@ -178,9 +177,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>  	if (rc <= 0) {
>  		struct be_dma_mem *tag_mem;
>  		/* Set MBX Tag state to timeout */
> -		mutex_lock(&phba->ctrl.mbox_lock);
> -		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_TIMEOUT;
> -		mutex_unlock(&phba->ctrl.mbox_lock);
> +		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
> +				MCC_TAG_STATE_TIMEOUT);
>  
>  		/* Store resource addr to be freed later */
>  		tag_mem = &phba->ctrl.ptag_state[tag].tag_mem_state;
> @@ -199,9 +197,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>  	} else {
>  		rc = 0;
>  		/* Set MBX Tag state to completed */
> -		mutex_lock(&phba->ctrl.mbox_lock);
> -		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
> -		mutex_unlock(&phba->ctrl.mbox_lock);
> +		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
> +				MCC_TAG_STATE_COMPLETED);
>  	}
>  
>  	mcc_tag_response = phba->ctrl.mcc_numtag[tag];
> @@ -373,9 +370,11 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
>  	ctrl->mcc_numtag[tag] |= (extd_status & 0x000000FF) << 8;
>  	ctrl->mcc_numtag[tag] |= (compl_status & 0x000000FF);
>  
> -	if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_RUNNING) {
> +	if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
> +		MCC_TAG_STATE_RUNNING) {
>  		wake_up_interruptible(&ctrl->mcc_wait[tag]);
> -	} else if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_TIMEOUT) {
> +	} else if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
> +			MCC_TAG_STATE_TIMEOUT) {
>  		struct be_dma_mem *tag_mem;
>  		tag_mem = &ctrl->ptag_state[tag].tag_mem_state;
>  
> @@ -390,9 +389,8 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
>  					    tag_mem->va, tag_mem->dma);
>  
>  		/* Change tag state */
> -		mutex_lock(&phba->ctrl.mbox_lock);
> -		ctrl->ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
> -		mutex_unlock(&phba->ctrl.mbox_lock);
> +		atomic_set(&ctrl->ptag_state[tag].tag_state,
> +				MCC_TAG_STATE_COMPLETED);
>  
>  		/* Free MCC Tag */
>  		free_mcc_tag(ctrl, tag);
> 

I think if you only need to get and set a u8 then you can just use a u8
since the operation will be atomic. No need for a atomic_t.

Re: [PATCH 03/15] be2iscsi: Fix to use atomic operations for tag_state

[Mike Christie]

On 12/20/2015 01:44 AM, Mike Christie wrote:

>> diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
>> index 6fabded..21c806f 100644
>> --- a/drivers/scsi/be2iscsi/be_cmds.c
>> +++ b/drivers/scsi/be2iscsi/be_cmds.c
>> @@ -164,9 +164,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>  	}
>>  
>>  	/* Set MBX Tag state to Active */
>> -	mutex_lock(&phba->ctrl.mbox_lock);
>> -	phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_RUNNING;
>> -	mutex_unlock(&phba->ctrl.mbox_lock);
>> +	atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>> +			MCC_TAG_STATE_RUNNING);
>>  


Is it possible for be_mcc_compl_process_isr to run before we have set
the state to MCC_TAG_STATE_RUNNING, so the
wait_event_interruptible_timeout call timesout?


>>  	/* wait for the mccq completion */
>>  	rc = wait_event_interruptible_timeout(
>> @@ -178,9 +177,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>  	if (rc <= 0) {
>>  		struct be_dma_mem *tag_mem;
>>  		/* Set MBX Tag state to timeout */
>> -		mutex_lock(&phba->ctrl.mbox_lock);
>> -		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_TIMEOUT;
>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>> +		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>> +				MCC_TAG_STATE_TIMEOUT);
>>  
>>  		/* Store resource addr to be freed later */
>>  		tag_mem = &phba->ctrl.ptag_state[tag].tag_mem_state;
>> @@ -199,9 +197,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>  	} else {
>>  		rc = 0;
>>  		/* Set MBX Tag state to completed */
>> -		mutex_lock(&phba->ctrl.mbox_lock);
>> -		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>> +		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>> +				MCC_TAG_STATE_COMPLETED);
>>  	}
>>  
>>  	mcc_tag_response = phba->ctrl.mcc_numtag[tag];
>> @@ -373,9 +370,11 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
>>  	ctrl->mcc_numtag[tag] |= (extd_status & 0x000000FF) << 8;
>>  	ctrl->mcc_numtag[tag] |= (compl_status & 0x000000FF);
>>  
>> -	if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_RUNNING) {
>> +	if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
>> +		MCC_TAG_STATE_RUNNING) {
>>  		wake_up_interruptible(&ctrl->mcc_wait[tag]);
>> -	} else if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_TIMEOUT) {
>> +	} else if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
>> +			MCC_TAG_STATE_TIMEOUT) {
>>  		struct be_dma_mem *tag_mem;
>>  		tag_mem = &ctrl->ptag_state[tag].tag_mem_state;
>>  
>> @@ -390,9 +389,8 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
>>  					    tag_mem->va, tag_mem->dma);
>>  
>>  		/* Change tag state */
>> -		mutex_lock(&phba->ctrl.mbox_lock);
>> -		ctrl->ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>> +		atomic_set(&ctrl->ptag_state[tag].tag_state,
>> +				MCC_TAG_STATE_COMPLETED);
>>  
>>  		/* Free MCC Tag */
>>  		free_mcc_tag(ctrl, tag);
>>
> 
> I think if you only need to get and set a u8 then you can just use a u8
> since the operation will be atomic. No need for a atomic_t.

I think you can ignore this. I think you would need some barriers in
there too and it might be more complicated for no gain.

Re: [PATCH 03/15] be2iscsi: Fix to use atomic operations for tag_state

[Mike Christie]

On 12/20/15 3:01 AM, Mike Christie wrote:
> On 12/20/2015 01:44 AM, Mike Christie wrote:
>
>>> diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
>>> index 6fabded..21c806f 100644
>>> --- a/drivers/scsi/be2iscsi/be_cmds.c
>>> +++ b/drivers/scsi/be2iscsi/be_cmds.c
>>> @@ -164,9 +164,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>>   	}
>>>
>>>   	/* Set MBX Tag state to Active */
>>> -	mutex_lock(&phba->ctrl.mbox_lock);
>>> -	phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_RUNNING;
>>> -	mutex_unlock(&phba->ctrl.mbox_lock);
>>> +	atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>>> +			MCC_TAG_STATE_RUNNING);
>>>
>
>
> Is it possible for be_mcc_compl_process_isr to run before we have set
> the state to MCC_TAG_STATE_RUNNING, so the
> wait_event_interruptible_timeout call timesout?
>
>
>>>   	/* wait for the mccq completion */
>>>   	rc = wait_event_interruptible_timeout(
>>> @@ -178,9 +177,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>>   	if (rc <= 0) {
>>>   		struct be_dma_mem *tag_mem;
>>>   		/* Set MBX Tag state to timeout */
>>> -		mutex_lock(&phba->ctrl.mbox_lock);
>>> -		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_TIMEOUT;
>>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>>> +		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>>> +				MCC_TAG_STATE_TIMEOUT);
>>>
>>>   		/* Store resource addr to be freed later */
>>>   		tag_mem = &phba->ctrl.ptag_state[tag].tag_mem_state;
>>> @@ -199,9 +197,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>>   	} else {
>>>   		rc = 0;
>>>   		/* Set MBX Tag state to completed */
>>> -		mutex_lock(&phba->ctrl.mbox_lock);
>>> -		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
>>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>>> +		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>>> +				MCC_TAG_STATE_COMPLETED);
>>>   	}
>>>
>>>   	mcc_tag_response = phba->ctrl.mcc_numtag[tag];
>>> @@ -373,9 +370,11 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
>>>   	ctrl->mcc_numtag[tag] |= (extd_status & 0x000000FF) << 8;
>>>   	ctrl->mcc_numtag[tag] |= (compl_status & 0x000000FF);
>>>
>>> -	if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_RUNNING) {
>>> +	if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
>>> +		MCC_TAG_STATE_RUNNING) {
>>>   		wake_up_interruptible(&ctrl->mcc_wait[tag]);
>>> -	} else if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_TIMEOUT) {
>>> +	} else if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
>>> +			MCC_TAG_STATE_TIMEOUT) {
>>>   		struct be_dma_mem *tag_mem;
>>>   		tag_mem = &ctrl->ptag_state[tag].tag_mem_state;
>>>
>>> @@ -390,9 +389,8 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
>>>   					    tag_mem->va, tag_mem->dma);
>>>
>>>   		/* Change tag state */
>>> -		mutex_lock(&phba->ctrl.mbox_lock);
>>> -		ctrl->ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
>>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>>> +		atomic_set(&ctrl->ptag_state[tag].tag_state,
>>> +				MCC_TAG_STATE_COMPLETED);
>>>
>>>   		/* Free MCC Tag */
>>>   		free_mcc_tag(ctrl, tag);
>>>
>>
>> I think if you only need to get and set a u8 then you can just use a u8
>> since the operation will be atomic. No need for a atomic_t.
>
> I think you can ignore this. I think you would need some barriers in
> there too and it might be more complicated for no gain.
>

Ughhh. Sorry.

The atomic_ops.txt doc says atomic_read/atomic_set use still needs 
barriers, so I guess they do not do anything for you and a u8 is ok.

The memory-barrier.txt doc says wake_up/wait calls have barriers if the 
wake_up path is hit, so you are ok there.

However, besides the timeout issue in the previous mail, can 
beiscsi_mccq_compl set the tag_mem_state to MCC_TAG_STATE_TIMEOUT and 
be_mcc_compl_process_isr does not see the tag_mem values updated?

RE: [PATCH 03/15] be2iscsi: Fix to use atomic operations for tag_state

[Jitendra Bhivare]

Yep, there is a need for barrier when setting MCC_TAG_STATE_TIMEOUT.
I am sorry, was under the assumption that lock prefix would be used for
that atomic operation as well.

Thanks,

JB

-----Original Message-----
From: Mike Christie [mailto:michaelc@cs.wisc.edu]
Sent: Sunday, December 20, 2015 3:44 PM
To: Jitendra Bhivare; linux-scsi@vger.kernel.org
Subject: Re: [PATCH 03/15] be2iscsi: Fix to use atomic operations for
tag_state

On 12/20/15 3:01 AM, Mike Christie wrote:
> On 12/20/2015 01:44 AM, Mike Christie wrote:
>
>>> diff --git a/drivers/scsi/be2iscsi/be_cmds.c
>>> b/drivers/scsi/be2iscsi/be_cmds.c index 6fabded..21c806f 100644
>>> --- a/drivers/scsi/be2iscsi/be_cmds.c
>>> +++ b/drivers/scsi/be2iscsi/be_cmds.c
>>> @@ -164,9 +164,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>>   	}
>>>
>>>   	/* Set MBX Tag state to Active */
>>> -	mutex_lock(&phba->ctrl.mbox_lock);
>>> -	phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_RUNNING;
>>> -	mutex_unlock(&phba->ctrl.mbox_lock);
>>> +	atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>>> +			MCC_TAG_STATE_RUNNING);
>>>
>
>
> Is it possible for be_mcc_compl_process_isr to run before we have set
> the state to MCC_TAG_STATE_RUNNING, so the
> wait_event_interruptible_timeout call timesout?
>
>
>>>   	/* wait for the mccq completion */
>>>   	rc = wait_event_interruptible_timeout( @@ -178,9 +177,8 @@ int
>>> beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>>   	if (rc <= 0) {
>>>   		struct be_dma_mem *tag_mem;
>>>   		/* Set MBX Tag state to timeout */
>>> -		mutex_lock(&phba->ctrl.mbox_lock);
>>> -		phba->ctrl.ptag_state[tag].tag_state =
MCC_TAG_STATE_TIMEOUT;
>>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>>> +		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>>> +				MCC_TAG_STATE_TIMEOUT);
>>>
>>>   		/* Store resource addr to be freed later */
>>>   		tag_mem = &phba->ctrl.ptag_state[tag].tag_mem_state;
>>> @@ -199,9 +197,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
>>>   	} else {
>>>   		rc = 0;
>>>   		/* Set MBX Tag state to completed */
>>> -		mutex_lock(&phba->ctrl.mbox_lock);
>>> -		phba->ctrl.ptag_state[tag].tag_state =
MCC_TAG_STATE_COMPLETED;
>>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>>> +		atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
>>> +				MCC_TAG_STATE_COMPLETED);
>>>   	}
>>>
>>>   	mcc_tag_response = phba->ctrl.mcc_numtag[tag]; @@ -373,9 +370,11
>>> @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
>>>   	ctrl->mcc_numtag[tag] |= (extd_status & 0x000000FF) << 8;
>>>   	ctrl->mcc_numtag[tag] |= (compl_status & 0x000000FF);
>>>
>>> -	if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_RUNNING) {
>>> +	if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
>>> +		MCC_TAG_STATE_RUNNING) {
>>>   		wake_up_interruptible(&ctrl->mcc_wait[tag]);
>>> -	} else if (ctrl->ptag_state[tag].tag_state ==
MCC_TAG_STATE_TIMEOUT) {
>>> +	} else if (atomic_read(&ctrl->ptag_state[tag].tag_state) ==
>>> +			MCC_TAG_STATE_TIMEOUT) {
>>>   		struct be_dma_mem *tag_mem;
>>>   		tag_mem = &ctrl->ptag_state[tag].tag_mem_state;
>>>
>>> @@ -390,9 +389,8 @@ int be_mcc_compl_process_isr(struct be_ctrl_info
*ctrl,
>>>   					    tag_mem->va, tag_mem->dma);
>>>
>>>   		/* Change tag state */
>>> -		mutex_lock(&phba->ctrl.mbox_lock);
>>> -		ctrl->ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
>>> -		mutex_unlock(&phba->ctrl.mbox_lock);
>>> +		atomic_set(&ctrl->ptag_state[tag].tag_state,
>>> +				MCC_TAG_STATE_COMPLETED);
>>>
>>>   		/* Free MCC Tag */
>>>   		free_mcc_tag(ctrl, tag);
>>>
>>
>> I think if you only need to get and set a u8 then you can just use a
>> u8 since the operation will be atomic. No need for a atomic_t.
>
> I think you can ignore this. I think you would need some barriers in
> there too and it might be more complicated for no gain.
>

Ughhh. Sorry.

The atomic_ops.txt doc says atomic_read/atomic_set use still needs
barriers, so I guess they do not do anything for you and a u8 is ok.

The memory-barrier.txt doc says wake_up/wait calls have barriers if the
wake_up path is hit, so you are ok there.

However, besides the timeout issue in the previous mail, can
beiscsi_mccq_compl set the tag_mem_state to MCC_TAG_STATE_TIMEOUT and
be_mcc_compl_process_isr does not see the tag_mem values updated?

[PATCH 04/15] be2iscsi: Fix to synchronize tag allocation using spin_lock

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

alloc_mcc_tag/free_mcc_tag is now done under mcc_lock spin_lock

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_cmds.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 21c806f..6f58063 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -118,6 +118,7 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba)
 {
 	unsigned int tag = 0;
 
+	spin_lock(&phba->ctrl.mcc_lock);
 	if (phba->ctrl.mcc_tag_available) {
 		tag = phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index];
 		phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index] = 0;
@@ -130,6 +131,7 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba)
 		else
 			phba->ctrl.mcc_alloc_index++;
 	}
+	spin_unlock(&phba->ctrl.mcc_lock);
 	return tag;
 }
 
@@ -254,7 +256,7 @@ release_mcc_tag:
 
 void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag)
 {
-	spin_lock(&ctrl->mbox_lock);
+	spin_lock(&ctrl->mcc_lock);
 	tag = tag & 0x000000FF;
 	ctrl->mcc_tag[ctrl->mcc_free_index] = tag;
 	if (ctrl->mcc_free_index == (MAX_MCC_CMD - 1))
@@ -262,7 +264,7 @@ void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag)
 	else
 		ctrl->mcc_free_index++;
 	ctrl->mcc_tag_available++;
-	spin_unlock(&ctrl->mbox_lock);
+	spin_unlock(&ctrl->mcc_lock);
 }
 
 bool is_link_state_evt(u32 trailer)
-- 
1.7.1

Re: [PATCH 04/15] be2iscsi: Fix to synchronize tag allocation using spin_lock

[Hannes Reinecke]

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> alloc_mcc_tag/free_mcc_tag is now done under mcc_lock spin_lock
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_cmds.c |    6 ++++--
>   1 files changed, 4 insertions(+), 2 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/15] be2iscsi: Set mbox timeout to 30s

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

FW recommended timeout for all mbox command is 30s.
Use msleep instead mdelay to relinquish CPU when polling for
mbox completion.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_cmds.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 6f58063..1e70053 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -587,7 +587,8 @@ int be_mcc_notify_wait(struct beiscsi_hba *phba)
  **/
 static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl)
 {
-#define BEISCSI_MBX_RDY_BIT_TIMEOUT	12000	/* 12sec */
+	/* wait 30s for generic non-flash MBOX operation */
+#define BEISCSI_MBX_RDY_BIT_TIMEOUT	30000
 	void __iomem *db = ctrl->db + MPU_MAILBOX_DB_OFFSET;
 	struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev);
 	unsigned long timeout;
@@ -612,7 +613,7 @@ static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl)
 
 		if (time_after(jiffies, timeout))
 			break;
-		mdelay(1);
+		msleep(20);
 	} while (!ready);
 
 	beiscsi_log(phba, KERN_ERR,
-- 
1.7.1

Re: [PATCH 05/15] be2iscsi: Set mbox timeout to 30s

[Hannes Reinecke]

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> FW recommended timeout for all mbox command is 30s.
> Use msleep instead mdelay to relinquish CPU when polling for
> mbox completion.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_cmds.c |    5 +++--
>   1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
> index 6f58063..1e70053 100644
> --- a/drivers/scsi/be2iscsi/be_cmds.c
> +++ b/drivers/scsi/be2iscsi/be_cmds.c
> @@ -587,7 +587,8 @@ int be_mcc_notify_wait(struct beiscsi_hba *phba)
>    **/
>   static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl)
>   {
> -#define BEISCSI_MBX_RDY_BIT_TIMEOUT	12000	/* 12sec */
> +	/* wait 30s for generic non-flash MBOX operation */
> +#define BEISCSI_MBX_RDY_BIT_TIMEOUT	30000
>   	void __iomem *db = ctrl->db + MPU_MAILBOX_DB_OFFSET;
>   	struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev);
>   	unsigned long timeout;
> @@ -612,7 +613,7 @@ static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl)
>
>   		if (time_after(jiffies, timeout))
>   			break;
> -		mdelay(1);
> +		msleep(20);
>   	} while (!ready);
>
>   	beiscsi_log(phba, KERN_ERR,
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/15] be2iscsi: Added return value check for mgmt_get_all_if_id

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Use of mutex_lock_interruptible can return -EINTR, handle and log
the error.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_iscsi.c |    7 ++++---
 drivers/scsi/be2iscsi/be_mgmt.c  |   10 ++++++----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index b7087ba..188d83f 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -367,13 +367,14 @@ beiscsi_set_vlan_tag(struct Scsi_Host *shost,
 		      struct iscsi_iface_param_info *iface_param)
 {
 	struct beiscsi_hba *phba = iscsi_host_priv(shost);
-	int ret = 0;
+	int ret;
 
 	/* Get the Interface Handle */
-	if (mgmt_get_all_if_id(phba)) {
+	ret = mgmt_get_all_if_id(phba);
+	if (ret) {
 		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
 			    "BS_%d : Getting Interface Handle Failed\n");
-		return -EIO;
+		return ret;
 	}
 
 	switch (iface_param->param) {
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index a41013e..a8a1670 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -1018,8 +1018,9 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
 	uint32_t ip_type;
 	int rc;
 
-	if (mgmt_get_all_if_id(phba))
-		return -EIO;
+	rc = mgmt_get_all_if_id(phba);
+	if (rc)
+		return rc;
 
 	ip_type = (ip_param->param == ISCSI_NET_PARAM_IPV6_ADDR) ?
 		BE2_IPV6 : BE2_IPV4 ;
@@ -1188,8 +1189,9 @@ int mgmt_get_if_info(struct beiscsi_hba *phba, int ip_type,
 	uint32_t ioctl_size = sizeof(struct be_cmd_get_if_info_resp);
 	int rc;
 
-	if (mgmt_get_all_if_id(phba))
-		return -EIO;
+	rc = mgmt_get_all_if_id(phba);
+	if (rc)
+		return rc;
 
 	do {
 		rc = mgmt_alloc_cmd_data(phba, &nonemb_cmd,
-- 
1.7.1

Re: [PATCH 06/15] be2iscsi: Added return value check for mgmt_get_all_if_id

[Hannes Reinecke]

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Use of mutex_lock_interruptible can return -EINTR, handle and log
> the error.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_iscsi.c |    7 ++++---
>   drivers/scsi/be2iscsi/be_mgmt.c  |   10 ++++++----
>   2 files changed, 10 insertions(+), 7 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/15] be2iscsi: Fix to remove shutdown entry point

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Null pointer dereference in shutdown path after taking dump.

Shutdown path is not needed as FW comes up clean every time during probe
after issuing FUNCTION reset MBOX command.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_cmds.c |   17 +++--------------
 drivers/scsi/be2iscsi/be_cmds.h |    2 +-
 drivers/scsi/be2iscsi/be_main.c |   24 ++++--------------------
 drivers/scsi/be2iscsi/be_main.h |    3 +--
 4 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 1e70053..e4cc98f 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -414,22 +414,11 @@ static struct be_mcc_compl *be_mcc_compl_get(struct beiscsi_hba *phba)
 }
 
 /**
- * be2iscsi_fail_session(): Closing session with appropriate error
+ * beiscsi_fail_session(): Closing session with appropriate error
  * @cls_session: ptr to session
- *
- * Depending on adapter state appropriate error flag is passed.
  **/
-void be2iscsi_fail_session(struct iscsi_cls_session *cls_session)
+void beiscsi_fail_session(struct iscsi_cls_session *cls_session)
 {
-	struct Scsi_Host *shost = iscsi_session_to_shost(cls_session);
-	struct beiscsi_hba *phba = iscsi_host_priv(shost);
-	uint32_t iscsi_err_flag;
-
-	if (phba->state & BE_ADAPTER_STATE_SHUTDOWN)
-		iscsi_err_flag = ISCSI_ERR_INVALID_HOST;
-	else
-		iscsi_err_flag = ISCSI_ERR_CONN_FAILED;
-
 	iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED);
 }
 
@@ -447,7 +436,7 @@ void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
 			    evt->physical_port);
 
 		iscsi_host_for_each_session(phba->shost,
-					    be2iscsi_fail_session);
+					    beiscsi_fail_session);
 	} else if ((evt->port_link_status & ASYNC_EVENT_LINK_UP) ||
 		    ((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
 		     (evt->port_fault == BEISCSI_PHY_LINK_FAULT_NONE))) {
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index 4bfca35..5d165ee 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -1367,5 +1367,5 @@ void be_wrb_hdr_prepare(struct be_mcc_wrb *wrb, int payload_len,
 void be_cmd_hdr_prepare(struct be_cmd_req_hdr *req_hdr,
 			u8 subsystem, u8 opcode, int cmd_len);
 
-void be2iscsi_fail_session(struct iscsi_cls_session *cls_session);
+void beiscsi_fail_session(struct iscsi_cls_session *cls_session);
 #endif /* !BEISCSI_CMDS_H */
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 61ce86b..2f3e118 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -5315,7 +5315,6 @@ static void beiscsi_quiesce(struct beiscsi_hba *phba,
 
 static void beiscsi_remove(struct pci_dev *pcidev)
 {
-
 	struct beiscsi_hba *phba = NULL;
 
 	phba = pci_get_drvdata(pcidev);
@@ -5325,9 +5324,9 @@ static void beiscsi_remove(struct pci_dev *pcidev)
 	}
 
 	beiscsi_destroy_def_ifaces(phba);
-	beiscsi_quiesce(phba, BEISCSI_CLEAN_UNLOAD);
 	iscsi_boot_destroy_kset(phba->boot_kset);
 	iscsi_host_remove(phba->shost);
+	beiscsi_quiesce(phba, BEISCSI_CLEAN_UNLOAD);
 	pci_dev_put(phba->pcidev);
 	iscsi_host_free(phba->shost);
 	pci_disable_pcie_error_reporting(pcidev);
@@ -5336,23 +5335,6 @@ static void beiscsi_remove(struct pci_dev *pcidev)
 	pci_disable_device(pcidev);
 }
 
-static void beiscsi_shutdown(struct pci_dev *pcidev)
-{
-
-	struct beiscsi_hba *phba = NULL;
-
-	phba = (struct beiscsi_hba *)pci_get_drvdata(pcidev);
-	if (!phba) {
-		dev_err(&pcidev->dev, "beiscsi_shutdown called with no phba\n");
-		return;
-	}
-
-	phba->state = BE_ADAPTER_STATE_SHUTDOWN;
-	iscsi_host_for_each_session(phba->shost, be2iscsi_fail_session);
-	beiscsi_quiesce(phba, BEISCSI_CLEAN_UNLOAD);
-	pci_disable_device(pcidev);
-}
-
 static void beiscsi_msix_enable(struct beiscsi_hba *phba)
 {
 	int i, status;
@@ -5673,6 +5655,9 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
 		goto hba_free;
 	}
 
+	/*
+	 * FUNCTION_RESET should clean up any stale info in FW for this fn
+	 */
 	ret = beiscsi_cmd_reset_function(phba);
 	if (ret) {
 		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
@@ -5861,7 +5846,6 @@ static struct pci_driver beiscsi_pci_driver = {
 	.name = DRV_NAME,
 	.probe = beiscsi_dev_probe,
 	.remove = beiscsi_remove,
-	.shutdown = beiscsi_shutdown,
 	.id_table = beiscsi_pci_id_table,
 	.err_handler = &beiscsi_eeh_handlers
 };
diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index 5c67c07..bd9d1e1 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -103,8 +103,7 @@
 #define BE_ADAPTER_LINK_UP	0x001
 #define BE_ADAPTER_LINK_DOWN	0x002
 #define BE_ADAPTER_PCI_ERR	0x004
-#define BE_ADAPTER_STATE_SHUTDOWN	0x008
-#define BE_ADAPTER_CHECK_BOOT	0x010
+#define BE_ADAPTER_CHECK_BOOT	0x008
 
 
 #define BEISCSI_CLEAN_UNLOAD	0x01
-- 
1.7.1

Re: [PATCH 07/15] be2iscsi: Fix to remove shutdown entry point

[Hannes Reinecke]

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Null pointer dereference in shutdown path after taking dump.
>
> Shutdown path is not needed as FW comes up clean every time during probe
> after issuing FUNCTION reset MBOX command.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_cmds.c |   17 +++--------------
>   drivers/scsi/be2iscsi/be_cmds.h |    2 +-
>   drivers/scsi/be2iscsi/be_main.c |   24 ++++--------------------
>   drivers/scsi/be2iscsi/be_main.h |    3 +--
>   4 files changed, 9 insertions(+), 37 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/15] be2iscsi: Fix VLAN support for IPv6 network

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Configuring VLAN parameters through IPv6 interface was not supported in driver.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_iscsi.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index 188d83f..c89a025 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -466,6 +466,10 @@ beiscsi_set_ipv6(struct Scsi_Host *shost,
 		ret = mgmt_set_ip(phba, iface_param, NULL,
 				  ISCSI_BOOTPROTO_STATIC);
 		break;
+	case ISCSI_NET_PARAM_VLAN_ENABLED:
+	case ISCSI_NET_PARAM_VLAN_TAG:
+		ret = beiscsi_set_vlan_tag(shost, iface_param);
+		break;
 	default:
 		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
 			    "BS_%d : Param %d not supported\n",
-- 
1.7.1

Re: [PATCH 08/15] be2iscsi: Fix VLAN support for IPv6 network

[Hannes Reinecke]

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Configuring VLAN parameters through IPv6 interface was not supported in driver.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_iscsi.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
> index 188d83f..c89a025 100644
> --- a/drivers/scsi/be2iscsi/be_iscsi.c
> +++ b/drivers/scsi/be2iscsi/be_iscsi.c
> @@ -466,6 +466,10 @@ beiscsi_set_ipv6(struct Scsi_Host *shost,
>   		ret = mgmt_set_ip(phba, iface_param, NULL,
>   				  ISCSI_BOOTPROTO_STATIC);
>   		break;
> +	case ISCSI_NET_PARAM_VLAN_ENABLED:
> +	case ISCSI_NET_PARAM_VLAN_TAG:
> +		ret = beiscsi_set_vlan_tag(shost, iface_param);
> +		break;
>   	default:
>   		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
>   			    "BS_%d : Param %d not supported\n",
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/15] be2iscsi: Fix to handle misconfigured optics events

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Log messages for misconfigured transceivers reported by FW.

Register async events that driver handles using MCC_CREATE_EXT ioctl.
Errors messages for faulted/uncertified/unqualified optics are logged.
Added IOCTL to get port_name to be displayed in error message.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_cmds.c |  164 ++++++++++++++++++++++++--------------
 drivers/scsi/be2iscsi/be_cmds.h |   47 ++++++++++--
 drivers/scsi/be2iscsi/be_main.c |   19 +----
 drivers/scsi/be2iscsi/be_main.h |   10 ++-
 drivers/scsi/be2iscsi/be_mgmt.c |   42 ++++++++++
 drivers/scsi/be2iscsi/be_mgmt.h |    2 +
 6 files changed, 199 insertions(+), 85 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index e4cc98f..9bb5905 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -267,26 +267,6 @@ void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag)
 	spin_unlock(&ctrl->mcc_lock);
 }
 
-bool is_link_state_evt(u32 trailer)
-{
-	return (((trailer >> ASYNC_TRAILER_EVENT_CODE_SHIFT) &
-		  ASYNC_TRAILER_EVENT_CODE_MASK) ==
-		  ASYNC_EVENT_CODE_LINK_STATE);
-}
-
-static bool is_iscsi_evt(u32 trailer)
-{
-	return ((trailer >> ASYNC_TRAILER_EVENT_CODE_SHIFT) &
-		  ASYNC_TRAILER_EVENT_CODE_MASK) ==
-		  ASYNC_EVENT_CODE_ISCSI;
-}
-
-static int iscsi_evt_type(u32 trailer)
-{
-	return (trailer >> ASYNC_TRAILER_EVENT_TYPE_SHIFT) &
-		 ASYNC_TRAILER_EVENT_TYPE_MASK;
-}
-
 static inline bool be_mcc_compl_is_new(struct be_mcc_compl *compl)
 {
 	if (compl->flags != 0) {
@@ -422,7 +402,7 @@ void beiscsi_fail_session(struct iscsi_cls_session *cls_session)
 	iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED);
 }
 
-void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
+static void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
 		struct be_async_event_link_state *evt)
 {
 	if ((evt->port_link_status == ASYNC_EVENT_LINK_DOWN) ||
@@ -450,6 +430,100 @@ void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
 	}
 }
 
+static char *beiscsi_port_misconf_event_msg[] = {
+	"Physical Link is functional.",
+	"Optics faulted/incorrectly installed/not installed - Reseat optics, if issue not resolved, replace.",
+	"Optics of two types installed - Remove one optic or install matching pair of optics.",
+	"Incompatible optics - Replace with compatible optics for card to function.",
+	"Unqualified optics - Replace with Avago optics for Warranty and Technical Support.",
+	"Uncertified optics - Replace with Avago Certified optics to enable link operation."
+};
+
+static void beiscsi_process_async_sli(struct beiscsi_hba *phba,
+				      struct be_mcc_compl *compl)
+{
+	struct be_async_event_sli *async_sli;
+	u8 evt_type, state, old_state, le;
+	char *sev = KERN_WARNING;
+	char *msg = NULL;
+
+	evt_type = compl->flags >> ASYNC_TRAILER_EVENT_TYPE_SHIFT;
+	evt_type &= ASYNC_TRAILER_EVENT_TYPE_MASK;
+
+	/* processing only MISCONFIGURED physical port event */
+	if (evt_type != ASYNC_SLI_EVENT_TYPE_MISCONFIGURED)
+		return;
+
+	async_sli = (struct be_async_event_sli *)compl;
+	state = async_sli->event_data1 >>
+		 (phba->fw_config.phys_port * 8) & 0xff;
+	le = async_sli->event_data2 >>
+		 (phba->fw_config.phys_port * 8) & 0xff;
+
+	old_state = phba->optic_state;
+	phba->optic_state = state;
+
+	if (state >= ARRAY_SIZE(beiscsi_port_misconf_event_msg)) {
+		/* fw is reporting a state we don't know, log and return */
+		__beiscsi_log(phba, KERN_ERR,
+			    "BC_%d : Port %c: Unrecognized optic state 0x%x\n",
+			    phba->port_name, async_sli->event_data1);
+		return;
+	}
+
+	if (ASYNC_SLI_LINK_EFFECT_VALID(le)) {
+		/* log link effect for unqualified-4, uncertified-5 optics */
+		if (state > 3)
+			msg = (ASYNC_SLI_LINK_EFFECT_STATE(le)) ?
+				" Link is non-operational." :
+				" Link is operational.";
+		/* 1 - info */
+		if (ASYNC_SLI_LINK_EFFECT_SEV(le) == 1)
+			sev = KERN_INFO;
+		/* 2 - error */
+		if (ASYNC_SLI_LINK_EFFECT_SEV(le) == 2)
+			sev = KERN_ERR;
+	}
+
+	if (old_state != phba->optic_state)
+		__beiscsi_log(phba, sev, "BC_%d : Port %c: %s%s\n",
+			      phba->port_name,
+			      beiscsi_port_misconf_event_msg[state],
+			      !msg ? "" : msg);
+}
+
+void beiscsi_process_async_event(struct beiscsi_hba *phba,
+				struct be_mcc_compl *compl)
+{
+	char *sev = KERN_INFO;
+	u8 evt_code;
+
+	/* interpret flags as an async trailer */
+	evt_code = compl->flags >> ASYNC_TRAILER_EVENT_CODE_SHIFT;
+	evt_code &= ASYNC_TRAILER_EVENT_CODE_MASK;
+	switch (evt_code) {
+	case ASYNC_EVENT_CODE_LINK_STATE:
+		beiscsi_async_link_state_process(phba,
+				(struct be_async_event_link_state *)compl);
+		break;
+	case ASYNC_EVENT_CODE_ISCSI:
+		phba->state |= BE_ADAPTER_CHECK_BOOT;
+		phba->get_boot = BE_GET_BOOT_RETRIES;
+		sev = KERN_ERR;
+		break;
+	case ASYNC_EVENT_CODE_SLI:
+		beiscsi_process_async_sli(phba, compl);
+		break;
+	default:
+		/* event not registered */
+		sev = KERN_ERR;
+	}
+
+	beiscsi_log(phba, sev, BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
+		    "BC_%d : ASYNC Event: status 0x%08x flags 0x%08x\n",
+		    compl->status, compl->flags);
+}
+
 int beiscsi_process_mcc(struct beiscsi_hba *phba)
 {
 	struct be_mcc_compl *compl;
@@ -459,45 +533,10 @@ int beiscsi_process_mcc(struct beiscsi_hba *phba)
 	spin_lock_bh(&phba->ctrl.mcc_cq_lock);
 	while ((compl = be_mcc_compl_get(phba))) {
 		if (compl->flags & CQE_FLAGS_ASYNC_MASK) {
-			/* Interpret flags as an async trailer */
-			if (is_link_state_evt(compl->flags))
-				/* Interpret compl as a async link evt */
-				beiscsi_async_link_state_process(phba,
-				   (struct be_async_event_link_state *) compl);
-			else if (is_iscsi_evt(compl->flags)) {
-				switch (iscsi_evt_type(compl->flags)) {
-				case ASYNC_EVENT_NEW_ISCSI_TGT_DISC:
-				case ASYNC_EVENT_NEW_ISCSI_CONN:
-				case ASYNC_EVENT_NEW_TCP_CONN:
-					phba->state |= BE_ADAPTER_CHECK_BOOT;
-					phba->get_boot = BE_GET_BOOT_RETRIES;
-					beiscsi_log(phba, KERN_ERR,
-						    BEISCSI_LOG_CONFIG |
-						    BEISCSI_LOG_MBOX,
-						    "BC_%d : Async iscsi Event,"
-						    " flags handled = 0x%08x\n",
-						    compl->flags);
-					break;
-				default:
-					phba->state |= BE_ADAPTER_CHECK_BOOT;
-					phba->get_boot = BE_GET_BOOT_RETRIES;
-					beiscsi_log(phba, KERN_ERR,
-						    BEISCSI_LOG_CONFIG |
-						    BEISCSI_LOG_MBOX,
-						    "BC_%d : Unsupported Async"
-						    " Event, flags = 0x%08x\n",
-						    compl->flags);
-				}
-			} else
-				beiscsi_log(phba, KERN_ERR,
-					    BEISCSI_LOG_CONFIG |
-					    BEISCSI_LOG_MBOX,
-					    "BC_%d : Unsupported Async Event, flags"
-					    " = 0x%08x\n", compl->flags);
-
+			beiscsi_process_async_event(phba, compl);
 		} else if (compl->flags & CQE_FLAGS_COMPLETED_MASK) {
-				status = be_mcc_compl_process(ctrl, compl);
-				atomic_dec(&phba->ctrl.mcc_obj.q.used);
+			status = be_mcc_compl_process(ctrl, compl);
+			atomic_dec(&phba->ctrl.mcc_obj.q.used);
 		}
 		be_mcc_compl_use(compl);
 		num++;
@@ -1013,7 +1052,7 @@ int beiscsi_cmd_mccq_create(struct beiscsi_hba *phba,
 			struct be_queue_info *cq)
 {
 	struct be_mcc_wrb *wrb;
-	struct be_cmd_req_mcc_create *req;
+	struct be_cmd_req_mcc_create_ext *req;
 	struct be_dma_mem *q_mem = &mccq->dma_mem;
 	struct be_ctrl_info *ctrl;
 	void *ctxt;
@@ -1029,9 +1068,12 @@ int beiscsi_cmd_mccq_create(struct beiscsi_hba *phba,
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
 
 	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
-			OPCODE_COMMON_MCC_CREATE, sizeof(*req));
+			OPCODE_COMMON_MCC_CREATE_EXT, sizeof(*req));
 
 	req->num_pages = PAGES_4K_SPANNED(q_mem->va, q_mem->size);
+	req->async_evt_bitmap = 1 << ASYNC_EVENT_CODE_LINK_STATE;
+	req->async_evt_bitmap |= 1 << ASYNC_EVENT_CODE_ISCSI;
+	req->async_evt_bitmap |= 1 << ASYNC_EVENT_CODE_SLI;
 
 	AMAP_SET_BITS(struct amap_mcc_context, fid, ctxt,
 		      PCI_FUNC(phba->pcidev->devfn));
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index 5d165ee..6411f7b 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -119,13 +119,22 @@ struct be_mcc_compl {
 #define ASYNC_TRAILER_EVENT_CODE_MASK	0xFF
 #define ASYNC_EVENT_CODE_LINK_STATE	0x1
 #define ASYNC_EVENT_CODE_ISCSI		0x4
+#define ASYNC_EVENT_CODE_SLI		0x11
 
 #define ASYNC_TRAILER_EVENT_TYPE_SHIFT	16	/* bits 16 - 23 */
-#define ASYNC_TRAILER_EVENT_TYPE_MASK	0xF
+#define ASYNC_TRAILER_EVENT_TYPE_MASK	0xFF
+
+/* iSCSI events */
 #define ASYNC_EVENT_NEW_ISCSI_TGT_DISC	0x4
 #define ASYNC_EVENT_NEW_ISCSI_CONN	0x5
 #define ASYNC_EVENT_NEW_TCP_CONN	0x7
 
+/* SLI events */
+#define ASYNC_SLI_EVENT_TYPE_MISCONFIGURED	0x9
+#define ASYNC_SLI_LINK_EFFECT_VALID(le)		(le & 0x80)
+#define ASYNC_SLI_LINK_EFFECT_SEV(le)		((le >> 1)  & 0x03)
+#define ASYNC_SLI_LINK_EFFECT_STATE(le)		(le & 0x01)
+
 struct be_async_event_trailer {
 	u32 code;
 };
@@ -153,6 +162,16 @@ struct be_async_event_link_state {
 	struct be_async_event_trailer trailer;
 } __packed;
 
+/**
+ * When async-trailer is SLI event, mcc_compl is interpreted as
+ */
+struct be_async_event_sli {
+	u32 event_data1;
+	u32 event_data2;
+	u32 reserved;
+	u32 trailer;
+} __packed;
+
 struct be_mcc_mailbox {
 	struct be_mcc_wrb wrb;
 	struct be_mcc_compl compl;
@@ -172,6 +191,7 @@ struct be_mcc_mailbox {
 #define OPCODE_COMMON_CQ_CREATE				12
 #define OPCODE_COMMON_EQ_CREATE				13
 #define OPCODE_COMMON_MCC_CREATE			21
+#define OPCODE_COMMON_MCC_CREATE_EXT			90
 #define OPCODE_COMMON_ADD_TEMPLATE_HEADER_BUFFERS	24
 #define OPCODE_COMMON_REMOVE_TEMPLATE_HEADER_BUFFERS	25
 #define OPCODE_COMMON_GET_CNTL_ATTRIBUTES		32
@@ -183,6 +203,7 @@ struct be_mcc_mailbox {
 #define OPCODE_COMMON_EQ_DESTROY			55
 #define OPCODE_COMMON_QUERY_FIRMWARE_CONFIG		58
 #define OPCODE_COMMON_FUNCTION_RESET			61
+#define OPCODE_COMMON_GET_PORT_NAME			77
 
 /**
  * LIST of opcodes that are common between Initiator and Target
@@ -587,10 +608,11 @@ struct amap_mcc_context {
 	u8 rsvd2[32];
 } __packed;
 
-struct be_cmd_req_mcc_create {
+struct be_cmd_req_mcc_create_ext {
 	struct be_cmd_req_hdr hdr;
 	u16 num_pages;
 	u16 rsvd0;
+	u32 async_evt_bitmap;
 	u8 context[sizeof(struct amap_mcc_context) / 8];
 	struct phys_addr pages[8];
 } __packed;
@@ -748,8 +770,8 @@ struct be_mcc_wrb *wrb_from_mccq(struct beiscsi_hba *phba);
 int be_mcc_notify_wait(struct beiscsi_hba *phba);
 void be_mcc_notify(struct beiscsi_hba *phba);
 unsigned int alloc_mcc_tag(struct beiscsi_hba *phba);
-void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
-		struct be_async_event_link_state *evt);
+void beiscsi_process_async_event(struct beiscsi_hba *phba,
+				struct be_mcc_compl *compl);
 int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
 				    struct be_mcc_compl *compl);
 
@@ -777,8 +799,6 @@ int be_cmd_wrbq_create(struct be_ctrl_info *ctrl, struct be_dma_mem *q_mem,
 		       struct hwi_wrb_context *pwrb_context,
 		       uint8_t ulp_num);
 
-bool is_link_state_evt(u32 trailer);
-
 /* Configuration Functions */
 int be_cmd_set_vlan(struct beiscsi_hba *phba, uint16_t vlan_tag);
 
@@ -1137,6 +1157,21 @@ struct be_cmd_get_all_if_id_req {
 	u32 if_hndl_list[1];
 } __packed;
 
+struct be_cmd_get_port_name {
+	union {
+		struct be_cmd_req_hdr req_hdr;
+		struct be_cmd_resp_hdr resp_hdr;
+	} h;
+	union {
+		struct {
+			u32 reserved;
+		} req;
+		struct {
+			u32 port_names;
+		} resp;
+	} p;
+} __packed;
+
 #define ISCSI_OPCODE_SCSI_DATA_OUT		5
 #define OPCODE_COMMON_NTWK_LINK_STATUS_QUERY 5
 #define OPCODE_COMMON_MODIFY_EQ_DELAY		41
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 2f3e118..8967e05 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -2048,21 +2048,7 @@ static void  beiscsi_process_mcc_isr(struct beiscsi_hba *phba)
 			num_processed = 0;
 		}
 		if (mcc_compl->flags & CQE_FLAGS_ASYNC_MASK) {
-			/* Interpret flags as an async trailer */
-			if (is_link_state_evt(mcc_compl->flags))
-				/* Interpret compl as a async link evt */
-				beiscsi_async_link_state_process(phba,
-				(struct be_async_event_link_state *) mcc_compl);
-			else {
-				beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_MBOX,
-					    "BM_%d :  Unsupported Async Event, flags"
-					    " = 0x%08x\n",
-					    mcc_compl->flags);
-				if (phba->state & BE_ADAPTER_LINK_UP) {
-					phba->state |= BE_ADAPTER_CHECK_BOOT;
-					phba->get_boot = BE_GET_BOOT_RETRIES;
-				}
-			}
+			beiscsi_process_async_event(phba, mcc_compl);
 		} else if (mcc_compl->flags & CQE_FLAGS_COMPLETED_MASK) {
 			be_mcc_compl_process_isr(&phba->ctrl, mcc_compl);
 			atomic_dec(&phba->ctrl.mcc_obj.q.used);
@@ -3868,6 +3854,8 @@ static int hwi_init_port(struct beiscsi_hba *phba)
 	phwi_context->min_eqd = 0;
 	phwi_context->cur_eqd = 0;
 	be_cmd_fw_initialize(&phba->ctrl);
+	/* set optic state to unknown */
+	phba->optic_state = 0xff;
 
 	status = beiscsi_create_eqs(phba, phwi_context);
 	if (status != 0) {
@@ -5681,6 +5669,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
 			    "BM_%d : Error getting fw config\n");
 		goto free_port;
 	}
+	mgmt_get_port_name(&phba->ctrl, phba);
 
 	if (enable_msix)
 		find_num_cpus(phba);
diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index bd9d1e1..c09082a 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -415,6 +415,7 @@ struct beiscsi_hba {
 	} fw_config;
 
 	unsigned int state;
+	u8 optic_state;
 	int get_boot;
 	bool fw_timeout;
 	bool ue_detected;
@@ -422,6 +423,7 @@ struct beiscsi_hba {
 
 	bool mac_addr_set;
 	u8 mac_address[ETH_ALEN];
+	u8 port_name;
 	char fw_ver_str[BEISCSI_VER_STRLEN];
 	char wq_name[20];
 	struct workqueue_struct *wq;	/* The actuak work queue */
@@ -1073,12 +1075,14 @@ struct hwi_context_memory {
 #define BEISCSI_LOG_CONFIG	0x0020	/* CONFIG Code Path */
 #define BEISCSI_LOG_ISCSI	0x0040	/* SCSI/iSCSI Protocol related Logs */
 
+#define __beiscsi_log(phba, level, fmt, arg...) \
+	shost_printk(level, phba->shost, fmt, __LINE__, ##arg)
+
 #define beiscsi_log(phba, level, mask, fmt, arg...) \
 do { \
 	uint32_t log_value = phba->attr_log_enable; \
 		if (((mask) & log_value) || (level[1] <= '3')) \
-			shost_printk(level, phba->shost, \
-				     fmt, __LINE__, ##arg); \
-} while (0)
+			__beiscsi_log(phba, level, fmt, ##arg); \
+} while (0);
 
 #endif
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index a8a1670..ad7aa75 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -316,6 +316,48 @@ unsigned int mgmt_get_session_info(struct beiscsi_hba *phba,
 }
 
 /**
+ * mgmt_get_port_name()- Get port name for the function
+ * @ctrl: ptr to Ctrl Info
+ * @phba: ptr to the dev priv structure
+ *
+ * Get the alphanumeric character for port
+ *
+ **/
+int mgmt_get_port_name(struct be_ctrl_info *ctrl,
+		       struct beiscsi_hba *phba)
+{
+	int ret = 0;
+	struct be_mcc_wrb *wrb;
+	struct be_cmd_get_port_name *ioctl;
+
+	mutex_lock(&ctrl->mbox_lock);
+	wrb = wrb_from_mbox(&ctrl->mbox_mem);
+	memset(wrb, 0, sizeof(*wrb));
+	ioctl = embedded_payload(wrb);
+
+	be_wrb_hdr_prepare(wrb, sizeof(*ioctl), true, 0);
+	be_cmd_hdr_prepare(&ioctl->h.req_hdr, CMD_SUBSYSTEM_COMMON,
+			   OPCODE_COMMON_GET_PORT_NAME,
+			   EMBED_MBX_MAX_PAYLOAD_SIZE);
+	ret = be_mbox_notify(ctrl);
+	phba->port_name = 0;
+	if (!ret) {
+		phba->port_name = ioctl->p.resp.port_names >>
+				  (phba->fw_config.phys_port * 8) & 0xff;
+	} else {
+		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
+			    "BG_%d : GET_PORT_NAME ret 0x%x status 0x%x\n",
+			    ret, ioctl->h.resp_hdr.status);
+	}
+
+	if (phba->port_name == 0)
+		phba->port_name = '?';
+
+	mutex_unlock(&ctrl->mbox_lock);
+	return ret;
+}
+
+/**
  * mgmt_get_fw_config()- Get the FW config for the function
  * @ctrl: ptr to Ctrl Info
  * @phba: ptr to the dev priv structure
diff --git a/drivers/scsi/be2iscsi/be_mgmt.h b/drivers/scsi/be2iscsi/be_mgmt.h
index c1dbb69..f3a48a0 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.h
+++ b/drivers/scsi/be2iscsi/be_mgmt.h
@@ -268,6 +268,8 @@ struct beiscsi_endpoint {
 
 int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
 				 struct beiscsi_hba *phba);
+int mgmt_get_port_name(struct be_ctrl_info *ctrl,
+		       struct beiscsi_hba *phba);
 
 unsigned int mgmt_invalidate_connection(struct beiscsi_hba *phba,
 					 struct beiscsi_endpoint *beiscsi_ep,
-- 
1.7.1

Re: [PATCH 09/15] be2iscsi: Fix to handle misconfigured optics events

[Hannes Reinecke]

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Log messages for misconfigured transceivers reported by FW.
>
> Register async events that driver handles using MCC_CREATE_EXT ioctl.
> Errors messages for faulted/uncertified/unqualified optics are logged.
> Added IOCTL to get port_name to be displayed in error message.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_cmds.c |  164 ++++++++++++++++++++++++--------------
>   drivers/scsi/be2iscsi/be_cmds.h |   47 ++++++++++--
>   drivers/scsi/be2iscsi/be_main.c |   19 +----
>   drivers/scsi/be2iscsi/be_main.h |   10 ++-
>   drivers/scsi/be2iscsi/be_mgmt.c |   42 ++++++++++
>   drivers/scsi/be2iscsi/be_mgmt.h |    2 +
>   6 files changed, 199 insertions(+), 85 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/15] be2iscsi: Add FW config validation

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

System crash in I+T card personality

Fix to add validation for ULP in initiator mode, physical port number,
and supported queue, icd, cid counts.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_main.c |    2 +-
 drivers/scsi/be2iscsi/be_main.h |    2 +
 drivers/scsi/be2iscsi/be_mgmt.c |  188 +++++++++++++++++++++++++--------------
 3 files changed, 123 insertions(+), 69 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 8967e05..a665e6a 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -5670,6 +5670,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
 		goto free_port;
 	}
 	mgmt_get_port_name(&phba->ctrl, phba);
+	beiscsi_get_params(phba);
 
 	if (enable_msix)
 		find_num_cpus(phba);
@@ -5687,7 +5688,6 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
 	}
 
 	phba->shost->max_id = phba->params.cxns_per_ctrl;
-	beiscsi_get_params(phba);
 	phba->shost->can_queue = phba->params.ios_per_ctrl;
 	ret = beiscsi_init_port(phba);
 	if (ret < 0) {
diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index c09082a..f89861b 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -397,7 +397,9 @@ struct beiscsi_hba {
 		 * group together since they are used most frequently
 		 * for cid to cri conversion
 		 */
+#define BEISCSI_PHYS_PORT_MAX	4
 		unsigned int phys_port;
+		/* valid values of phys_port id are 0, 1, 2, 3 */
 		unsigned int eqid_count;
 		unsigned int cqid_count;
 		unsigned int iscsi_cid_start[BEISCSI_ULP_COUNT];
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index ad7aa75..15f7ad7 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -373,90 +373,142 @@ int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
 				struct beiscsi_hba *phba)
 {
 	struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem);
-	struct be_fw_cfg *req = embedded_payload(wrb);
-	int status = 0;
+	struct be_fw_cfg *pfw_cfg = embedded_payload(wrb);
+	uint32_t cid_count, icd_count;
+	int status = -EINVAL;
+	uint8_t ulp_num = 0;
 
 	mutex_lock(&ctrl->mbox_lock);
 	memset(wrb, 0, sizeof(*wrb));
+	be_wrb_hdr_prepare(wrb, sizeof(*pfw_cfg), true, 0);
 
-	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
-
-	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
+	be_cmd_hdr_prepare(&pfw_cfg->hdr, CMD_SUBSYSTEM_COMMON,
 			   OPCODE_COMMON_QUERY_FIRMWARE_CONFIG,
 			   EMBED_MBX_MAX_PAYLOAD_SIZE);
-	status = be_mbox_notify(ctrl);
-	if (!status) {
-		uint8_t ulp_num = 0;
-		struct be_fw_cfg *pfw_cfg;
-		pfw_cfg = req;
 
-		if (!is_chip_be2_be3r(phba)) {
-			phba->fw_config.eqid_count = pfw_cfg->eqid_count;
-			phba->fw_config.cqid_count = pfw_cfg->cqid_count;
+	if (be_mbox_notify(ctrl)) {
+		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+			    "BG_%d : Failed in mgmt_get_fw_config\n");
+		goto fail_init;
+	}
 
-			beiscsi_log(phba, KERN_INFO,
-				    BEISCSI_LOG_INIT,
-				    "BG_%d : EQ_Count : %d CQ_Count : %d\n",
-				    phba->fw_config.eqid_count,
+	/* FW response formats depend on port id */
+	phba->fw_config.phys_port = pfw_cfg->phys_port;
+	if (phba->fw_config.phys_port >= BEISCSI_PHYS_PORT_MAX) {
+		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+			    "BG_%d : invalid physical port id %d\n",
+			    phba->fw_config.phys_port);
+		goto fail_init;
+	}
+
+	/* populate and check FW config against min and max values */
+	if (!is_chip_be2_be3r(phba)) {
+		phba->fw_config.eqid_count = pfw_cfg->eqid_count;
+		phba->fw_config.cqid_count = pfw_cfg->cqid_count;
+		if (phba->fw_config.eqid_count == 0 ||
+		    phba->fw_config.eqid_count > 2048) {
+			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+				    "BG_%d : invalid EQ count %d\n",
+				    phba->fw_config.eqid_count);
+			goto fail_init;
+		}
+		if (phba->fw_config.cqid_count == 0 ||
+		    phba->fw_config.cqid_count > 4096) {
+			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+				    "BG_%d : invalid CQ count %d\n",
 				    phba->fw_config.cqid_count);
+			goto fail_init;
 		}
+		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
+			    "BG_%d : EQ_Count : %d CQ_Count : %d\n",
+			    phba->fw_config.eqid_count,
+			    phba->fw_config.cqid_count);
+	}
 
-		for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
-			if (pfw_cfg->ulp[ulp_num].ulp_mode &
-			    BEISCSI_ULP_ISCSI_INI_MODE)
-				set_bit(ulp_num,
-				&phba->fw_config.ulp_supported);
-
-		phba->fw_config.phys_port = pfw_cfg->phys_port;
-		for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
-			if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
-
-				phba->fw_config.iscsi_cid_start[ulp_num] =
-					pfw_cfg->ulp[ulp_num].sq_base;
-				phba->fw_config.iscsi_cid_count[ulp_num] =
-					pfw_cfg->ulp[ulp_num].sq_count;
-
-				phba->fw_config.iscsi_icd_start[ulp_num] =
-					pfw_cfg->ulp[ulp_num].icd_base;
-				phba->fw_config.iscsi_icd_count[ulp_num] =
-					pfw_cfg->ulp[ulp_num].icd_count;
-
-				phba->fw_config.iscsi_chain_start[ulp_num] =
-					pfw_cfg->chain_icd[ulp_num].chain_base;
-				phba->fw_config.iscsi_chain_count[ulp_num] =
-					pfw_cfg->chain_icd[ulp_num].chain_count;
-
-				beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
-					    "BG_%d : Function loaded on ULP : %d\n"
-					    "\tiscsi_cid_count : %d\n"
-					    "\tiscsi_cid_start : %d\n"
-					    "\t iscsi_icd_count : %d\n"
-					    "\t iscsi_icd_start : %d\n",
-					    ulp_num,
-					    phba->fw_config.
-					    iscsi_cid_count[ulp_num],
-					    phba->fw_config.
-					    iscsi_cid_start[ulp_num],
-					    phba->fw_config.
-					    iscsi_icd_count[ulp_num],
-					    phba->fw_config.
-					    iscsi_icd_start[ulp_num]);
-			}
+	/**
+	 * Check on which all ULP iSCSI Protocol is loaded.
+	 * Set the Bit for those ULP. This set flag is used
+	 * at all places in the code to check on which ULP
+	 * iSCSi Protocol is loaded
+	 **/
+	for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
+		if (pfw_cfg->ulp[ulp_num].ulp_mode &
+		    BEISCSI_ULP_ISCSI_INI_MODE) {
+			set_bit(ulp_num, &phba->fw_config.ulp_supported);
+
+			/* Get the CID, ICD and Chain count for each ULP */
+			phba->fw_config.iscsi_cid_start[ulp_num] =
+				pfw_cfg->ulp[ulp_num].sq_base;
+			phba->fw_config.iscsi_cid_count[ulp_num] =
+				pfw_cfg->ulp[ulp_num].sq_count;
+
+			phba->fw_config.iscsi_icd_start[ulp_num] =
+				pfw_cfg->ulp[ulp_num].icd_base;
+			phba->fw_config.iscsi_icd_count[ulp_num] =
+				pfw_cfg->ulp[ulp_num].icd_count;
+
+			phba->fw_config.iscsi_chain_start[ulp_num] =
+				pfw_cfg->chain_icd[ulp_num].chain_base;
+			phba->fw_config.iscsi_chain_count[ulp_num] =
+				pfw_cfg->chain_icd[ulp_num].chain_count;
+
+			beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
+				    "BG_%d : Function loaded on ULP : %d\n"
+				    "\tiscsi_cid_count : %d\n"
+				    "\tiscsi_cid_start : %d\n"
+				    "\t iscsi_icd_count : %d\n"
+				    "\t iscsi_icd_start : %d\n",
+				    ulp_num,
+				    phba->fw_config.
+				    iscsi_cid_count[ulp_num],
+				    phba->fw_config.
+				    iscsi_cid_start[ulp_num],
+				    phba->fw_config.
+				    iscsi_icd_count[ulp_num],
+				    phba->fw_config.
+				    iscsi_icd_start[ulp_num]);
 		}
+	}
 
-		phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
-						  BEISCSI_FUNC_DUA_MODE);
+	if (phba->fw_config.ulp_supported == 0) {
+		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+			    "BG_%d : iSCSI initiator mode not set: ULP0 %x ULP1 %x\n",
+			    pfw_cfg->ulp[BEISCSI_ULP0].ulp_mode,
+			    pfw_cfg->ulp[BEISCSI_ULP1].ulp_mode);
+		goto fail_init;
+	}
 
-		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
-			    "BG_%d : DUA Mode : 0x%x\n",
-			    phba->fw_config.dual_ulp_aware);
+	/**
+	 * ICD is shared among ULPs. Use icd_count of any one loaded ULP
+	 **/
+	for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
+		if (test_bit(ulp_num, &phba->fw_config.ulp_supported))
+			break;
+	icd_count = phba->fw_config.iscsi_icd_count[ulp_num];
+	if (icd_count == 0 || icd_count > 65536) {
+		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+			    "BG_%d: invalid ICD count %d\n", icd_count);
+		goto fail_init;
+	}
 
-	} else {
+	cid_count = BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP0) +
+		    BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP1);
+	if (cid_count == 0 || cid_count > 4096) {
 		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
-			    "BG_%d : Failed in mgmt_get_fw_config\n");
-		status = -EINVAL;
+			    "BG_%d: invalid CID count %d\n", cid_count);
+		goto fail_init;
 	}
 
+	phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
+					  BEISCSI_FUNC_DUA_MODE);
+
+	beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
+		    "BG_%d : DUA Mode : 0x%x\n",
+		    phba->fw_config.dual_ulp_aware);
+
+	/* all set, continue using this FW config */
+	status = 0;
+fail_init:
 	mutex_unlock(&ctrl->mbox_lock);
 	return status;
 }
-- 
1.7.1

Re: [PATCH 10/15] be2iscsi: Add FW config validation

[Hannes Reinecke]

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> System crash in I+T card personality
>
> Fix to add validation for ULP in initiator mode, physical port number,
> and supported queue, icd, cid counts.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_main.c |    2 +-
>   drivers/scsi/be2iscsi/be_main.h |    2 +
>   drivers/scsi/be2iscsi/be_mgmt.c |  188 +++++++++++++++++++++++++--------------
>   3 files changed, 123 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index 8967e05..a665e6a 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -5670,6 +5670,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
>   		goto free_port;
>   	}
>   	mgmt_get_port_name(&phba->ctrl, phba);
> +	beiscsi_get_params(phba);
>
>   	if (enable_msix)
>   		find_num_cpus(phba);
> @@ -5687,7 +5688,6 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
>   	}
>
>   	phba->shost->max_id = phba->params.cxns_per_ctrl;
> -	beiscsi_get_params(phba);
>   	phba->shost->can_queue = phba->params.ios_per_ctrl;
>   	ret = beiscsi_init_port(phba);
>   	if (ret < 0) {
> diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
> index c09082a..f89861b 100644
> --- a/drivers/scsi/be2iscsi/be_main.h
> +++ b/drivers/scsi/be2iscsi/be_main.h
> @@ -397,7 +397,9 @@ struct beiscsi_hba {
>   		 * group together since they are used most frequently
>   		 * for cid to cri conversion
>   		 */
> +#define BEISCSI_PHYS_PORT_MAX	4
>   		unsigned int phys_port;
> +		/* valid values of phys_port id are 0, 1, 2, 3 */
>   		unsigned int eqid_count;
>   		unsigned int cqid_count;
>   		unsigned int iscsi_cid_start[BEISCSI_ULP_COUNT];
> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
> index ad7aa75..15f7ad7 100644
> --- a/drivers/scsi/be2iscsi/be_mgmt.c
> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
> @@ -373,90 +373,142 @@ int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
>   				struct beiscsi_hba *phba)
>   {
>   	struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem);
> -	struct be_fw_cfg *req = embedded_payload(wrb);
> -	int status = 0;
> +	struct be_fw_cfg *pfw_cfg = embedded_payload(wrb);
> +	uint32_t cid_count, icd_count;
> +	int status = -EINVAL;
> +	uint8_t ulp_num = 0;
>
>   	mutex_lock(&ctrl->mbox_lock);
>   	memset(wrb, 0, sizeof(*wrb));
> +	be_wrb_hdr_prepare(wrb, sizeof(*pfw_cfg), true, 0);
>
> -	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
> -
> -	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
> +	be_cmd_hdr_prepare(&pfw_cfg->hdr, CMD_SUBSYSTEM_COMMON,
>   			   OPCODE_COMMON_QUERY_FIRMWARE_CONFIG,
>   			   EMBED_MBX_MAX_PAYLOAD_SIZE);
> -	status = be_mbox_notify(ctrl);
> -	if (!status) {
> -		uint8_t ulp_num = 0;
> -		struct be_fw_cfg *pfw_cfg;
> -		pfw_cfg = req;
>
> -		if (!is_chip_be2_be3r(phba)) {
> -			phba->fw_config.eqid_count = pfw_cfg->eqid_count;
> -			phba->fw_config.cqid_count = pfw_cfg->cqid_count;
> +	if (be_mbox_notify(ctrl)) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : Failed in mgmt_get_fw_config\n");
> +		goto fail_init;
> +	}
>
> -			beiscsi_log(phba, KERN_INFO,
> -				    BEISCSI_LOG_INIT,
> -				    "BG_%d : EQ_Count : %d CQ_Count : %d\n",
> -				    phba->fw_config.eqid_count,
> +	/* FW response formats depend on port id */
> +	phba->fw_config.phys_port = pfw_cfg->phys_port;
> +	if (phba->fw_config.phys_port >= BEISCSI_PHYS_PORT_MAX) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : invalid physical port id %d\n",
> +			    phba->fw_config.phys_port);
> +		goto fail_init;
> +	}
> +
> +	/* populate and check FW config against min and max values */
> +	if (!is_chip_be2_be3r(phba)) {
> +		phba->fw_config.eqid_count = pfw_cfg->eqid_count;
> +		phba->fw_config.cqid_count = pfw_cfg->cqid_count;
> +		if (phba->fw_config.eqid_count == 0 ||
> +		    phba->fw_config.eqid_count > 2048) {
> +			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +				    "BG_%d : invalid EQ count %d\n",
> +				    phba->fw_config.eqid_count);
> +			goto fail_init;
> +		}
> +		if (phba->fw_config.cqid_count == 0 ||
> +		    phba->fw_config.cqid_count > 4096) {
> +			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +				    "BG_%d : invalid CQ count %d\n",
>   				    phba->fw_config.cqid_count);
> +			goto fail_init;
>   		}
> +		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +			    "BG_%d : EQ_Count : %d CQ_Count : %d\n",
> +			    phba->fw_config.eqid_count,
> +			    phba->fw_config.cqid_count);
> +	}
>
> -		for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
> -			if (pfw_cfg->ulp[ulp_num].ulp_mode &
> -			    BEISCSI_ULP_ISCSI_INI_MODE)
> -				set_bit(ulp_num,
> -				&phba->fw_config.ulp_supported);
> -
> -		phba->fw_config.phys_port = pfw_cfg->phys_port;
> -		for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
> -			if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
> -
> -				phba->fw_config.iscsi_cid_start[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].sq_base;
> -				phba->fw_config.iscsi_cid_count[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].sq_count;
> -
> -				phba->fw_config.iscsi_icd_start[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].icd_base;
> -				phba->fw_config.iscsi_icd_count[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].icd_count;
> -
> -				phba->fw_config.iscsi_chain_start[ulp_num] =
> -					pfw_cfg->chain_icd[ulp_num].chain_base;
> -				phba->fw_config.iscsi_chain_count[ulp_num] =
> -					pfw_cfg->chain_icd[ulp_num].chain_count;
> -
> -				beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> -					    "BG_%d : Function loaded on ULP : %d\n"
> -					    "\tiscsi_cid_count : %d\n"
> -					    "\tiscsi_cid_start : %d\n"
> -					    "\t iscsi_icd_count : %d\n"
> -					    "\t iscsi_icd_start : %d\n",
> -					    ulp_num,
> -					    phba->fw_config.
> -					    iscsi_cid_count[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_cid_start[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_icd_count[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_icd_start[ulp_num]);
> -			}
> +	/**
> +	 * Check on which all ULP iSCSI Protocol is loaded.
> +	 * Set the Bit for those ULP. This set flag is used
> +	 * at all places in the code to check on which ULP
> +	 * iSCSi Protocol is loaded
> +	 **/
> +	for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
> +		if (pfw_cfg->ulp[ulp_num].ulp_mode &
> +		    BEISCSI_ULP_ISCSI_INI_MODE) {
> +			set_bit(ulp_num, &phba->fw_config.ulp_supported);
> +
> +			/* Get the CID, ICD and Chain count for each ULP */
> +			phba->fw_config.iscsi_cid_start[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].sq_base;
> +			phba->fw_config.iscsi_cid_count[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].sq_count;
> +
> +			phba->fw_config.iscsi_icd_start[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].icd_base;
> +			phba->fw_config.iscsi_icd_count[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].icd_count;
> +
> +			phba->fw_config.iscsi_chain_start[ulp_num] =
> +				pfw_cfg->chain_icd[ulp_num].chain_base;
> +			phba->fw_config.iscsi_chain_count[ulp_num] =
> +				pfw_cfg->chain_icd[ulp_num].chain_count;
> +
> +			beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +				    "BG_%d : Function loaded on ULP : %d\n"
> +				    "\tiscsi_cid_count : %d\n"
> +				    "\tiscsi_cid_start : %d\n"
> +				    "\t iscsi_icd_count : %d\n"
> +				    "\t iscsi_icd_start : %d\n",
> +				    ulp_num,
> +				    phba->fw_config.
> +				    iscsi_cid_count[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_cid_start[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_icd_count[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_icd_start[ulp_num]);
>   		}
> +	}
>
> -		phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
> -						  BEISCSI_FUNC_DUA_MODE);
> +	if (phba->fw_config.ulp_supported == 0) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : iSCSI initiator mode not set: ULP0 %x ULP1 %x\n",
> +			    pfw_cfg->ulp[BEISCSI_ULP0].ulp_mode,
> +			    pfw_cfg->ulp[BEISCSI_ULP1].ulp_mode);
> +		goto fail_init;
> +	}
>
> -		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> -			    "BG_%d : DUA Mode : 0x%x\n",
> -			    phba->fw_config.dual_ulp_aware);
> +	/**
> +	 * ICD is shared among ULPs. Use icd_count of any one loaded ULP
> +	 **/
> +	for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
> +		if (test_bit(ulp_num, &phba->fw_config.ulp_supported))
> +			break;
> +	icd_count = phba->fw_config.iscsi_icd_count[ulp_num];
> +	if (icd_count == 0 || icd_count > 65536) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d: invalid ICD count %d\n", icd_count);
> +		goto fail_init;
> +	}
>
> -	} else {
> +	cid_count = BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP0) +
> +		    BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP1);
> +	if (cid_count == 0 || cid_count > 4096) {
>   		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> -			    "BG_%d : Failed in mgmt_get_fw_config\n");
> -		status = -EINVAL;
> +			    "BG_%d: invalid CID count %d\n", cid_count);
> +		goto fail_init;
>   	}
>
> +	phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
> +					  BEISCSI_FUNC_DUA_MODE);
> +
> +	beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +		    "BG_%d : DUA Mode : 0x%x\n",
> +		    phba->fw_config.dual_ulp_aware);
> +
> +	/* all set, continue using this FW config */
> +	status = 0;
> +fail_init:
>   	mutex_unlock(&ctrl->mbox_lock);
>   	return status;
>   }
>
Shouldn't that be 'DUAL mode' ?
But then, even the original had 'DUA'. So no need to redo the patch 
for that.

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 10/15] be2iscsi: Add FW config validation

[Jitendra Bhivare]

It stands for Dual ULP Aware. Will add a comment explaining that.

Thanks,

JB

-----Original Message-----
From: Hannes Reinecke [mailto:hare@suse.de]
Sent: Friday, December 18, 2015 2:34 PM
To: Jitendra Bhivare; linux-scsi@vger.kernel.org; michaelc@cs.wisc.edu
Subject: Re: [PATCH 10/15] be2iscsi: Add FW config validation

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> System crash in I+T card personality
>
> Fix to add validation for ULP in initiator mode, physical port number,
> and supported queue, icd, cid counts.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_main.c |    2 +-
>   drivers/scsi/be2iscsi/be_main.h |    2 +
>   drivers/scsi/be2iscsi/be_mgmt.c |  188
+++++++++++++++++++++++++--------------
>   3 files changed, 123 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.c
> b/drivers/scsi/be2iscsi/be_main.c index 8967e05..a665e6a 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -5670,6 +5670,7 @@ static int beiscsi_dev_probe(struct pci_dev
*pcidev,
>   		goto free_port;
>   	}
>   	mgmt_get_port_name(&phba->ctrl, phba);
> +	beiscsi_get_params(phba);
>
>   	if (enable_msix)
>   		find_num_cpus(phba);
> @@ -5687,7 +5688,6 @@ static int beiscsi_dev_probe(struct pci_dev
*pcidev,
>   	}
>
>   	phba->shost->max_id = phba->params.cxns_per_ctrl;
> -	beiscsi_get_params(phba);
>   	phba->shost->can_queue = phba->params.ios_per_ctrl;
>   	ret = beiscsi_init_port(phba);
>   	if (ret < 0) {
> diff --git a/drivers/scsi/be2iscsi/be_main.h
> b/drivers/scsi/be2iscsi/be_main.h index c09082a..f89861b 100644
> --- a/drivers/scsi/be2iscsi/be_main.h
> +++ b/drivers/scsi/be2iscsi/be_main.h
> @@ -397,7 +397,9 @@ struct beiscsi_hba {
>   		 * group together since they are used most frequently
>   		 * for cid to cri conversion
>   		 */
> +#define BEISCSI_PHYS_PORT_MAX	4
>   		unsigned int phys_port;
> +		/* valid values of phys_port id are 0, 1, 2, 3 */
>   		unsigned int eqid_count;
>   		unsigned int cqid_count;
>   		unsigned int iscsi_cid_start[BEISCSI_ULP_COUNT];
> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c
> b/drivers/scsi/be2iscsi/be_mgmt.c index ad7aa75..15f7ad7 100644
> --- a/drivers/scsi/be2iscsi/be_mgmt.c
> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
> @@ -373,90 +373,142 @@ int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
>   				struct beiscsi_hba *phba)
>   {
>   	struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem);
> -	struct be_fw_cfg *req = embedded_payload(wrb);
> -	int status = 0;
> +	struct be_fw_cfg *pfw_cfg = embedded_payload(wrb);
> +	uint32_t cid_count, icd_count;
> +	int status = -EINVAL;
> +	uint8_t ulp_num = 0;
>
>   	mutex_lock(&ctrl->mbox_lock);
>   	memset(wrb, 0, sizeof(*wrb));
> +	be_wrb_hdr_prepare(wrb, sizeof(*pfw_cfg), true, 0);
>
> -	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
> -
> -	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
> +	be_cmd_hdr_prepare(&pfw_cfg->hdr, CMD_SUBSYSTEM_COMMON,
>   			   OPCODE_COMMON_QUERY_FIRMWARE_CONFIG,
>   			   EMBED_MBX_MAX_PAYLOAD_SIZE);
> -	status = be_mbox_notify(ctrl);
> -	if (!status) {
> -		uint8_t ulp_num = 0;
> -		struct be_fw_cfg *pfw_cfg;
> -		pfw_cfg = req;
>
> -		if (!is_chip_be2_be3r(phba)) {
> -			phba->fw_config.eqid_count = pfw_cfg->eqid_count;
> -			phba->fw_config.cqid_count = pfw_cfg->cqid_count;
> +	if (be_mbox_notify(ctrl)) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : Failed in mgmt_get_fw_config\n");
> +		goto fail_init;
> +	}
>
> -			beiscsi_log(phba, KERN_INFO,
> -				    BEISCSI_LOG_INIT,
> -				    "BG_%d : EQ_Count : %d CQ_Count :
%d\n",
> -				    phba->fw_config.eqid_count,
> +	/* FW response formats depend on port id */
> +	phba->fw_config.phys_port = pfw_cfg->phys_port;
> +	if (phba->fw_config.phys_port >= BEISCSI_PHYS_PORT_MAX) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : invalid physical port id %d\n",
> +			    phba->fw_config.phys_port);
> +		goto fail_init;
> +	}
> +
> +	/* populate and check FW config against min and max values */
> +	if (!is_chip_be2_be3r(phba)) {
> +		phba->fw_config.eqid_count = pfw_cfg->eqid_count;
> +		phba->fw_config.cqid_count = pfw_cfg->cqid_count;
> +		if (phba->fw_config.eqid_count == 0 ||
> +		    phba->fw_config.eqid_count > 2048) {
> +			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +				    "BG_%d : invalid EQ count %d\n",
> +				    phba->fw_config.eqid_count);
> +			goto fail_init;
> +		}
> +		if (phba->fw_config.cqid_count == 0 ||
> +		    phba->fw_config.cqid_count > 4096) {
> +			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +				    "BG_%d : invalid CQ count %d\n",
>   				    phba->fw_config.cqid_count);
> +			goto fail_init;
>   		}
> +		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +			    "BG_%d : EQ_Count : %d CQ_Count : %d\n",
> +			    phba->fw_config.eqid_count,
> +			    phba->fw_config.cqid_count);
> +	}
>
> -		for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
> -			if (pfw_cfg->ulp[ulp_num].ulp_mode &
> -			    BEISCSI_ULP_ISCSI_INI_MODE)
> -				set_bit(ulp_num,
> -				&phba->fw_config.ulp_supported);
> -
> -		phba->fw_config.phys_port = pfw_cfg->phys_port;
> -		for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
{
> -			if (test_bit(ulp_num,
&phba->fw_config.ulp_supported)) {
> -
> -				phba->fw_config.iscsi_cid_start[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].sq_base;
> -				phba->fw_config.iscsi_cid_count[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].sq_count;
> -
> -				phba->fw_config.iscsi_icd_start[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].icd_base;
> -				phba->fw_config.iscsi_icd_count[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].icd_count;
> -
> -				phba->fw_config.iscsi_chain_start[ulp_num]
=
> -
pfw_cfg->chain_icd[ulp_num].chain_base;
> -				phba->fw_config.iscsi_chain_count[ulp_num]
=
> -
pfw_cfg->chain_icd[ulp_num].chain_count;
> -
> -				beiscsi_log(phba, KERN_INFO,
BEISCSI_LOG_INIT,
> -					    "BG_%d : Function loaded on
ULP : %d\n"
> -					    "\tiscsi_cid_count : %d\n"
> -					    "\tiscsi_cid_start : %d\n"
> -					    "\t iscsi_icd_count : %d\n"
> -					    "\t iscsi_icd_start : %d\n",
> -					    ulp_num,
> -					    phba->fw_config.
> -					    iscsi_cid_count[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_cid_start[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_icd_count[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_icd_start[ulp_num]);
> -			}
> +	/**
> +	 * Check on which all ULP iSCSI Protocol is loaded.
> +	 * Set the Bit for those ULP. This set flag is used
> +	 * at all places in the code to check on which ULP
> +	 * iSCSi Protocol is loaded
> +	 **/
> +	for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
> +		if (pfw_cfg->ulp[ulp_num].ulp_mode &
> +		    BEISCSI_ULP_ISCSI_INI_MODE) {
> +			set_bit(ulp_num, &phba->fw_config.ulp_supported);
> +
> +			/* Get the CID, ICD and Chain count for each ULP
*/
> +			phba->fw_config.iscsi_cid_start[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].sq_base;
> +			phba->fw_config.iscsi_cid_count[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].sq_count;
> +
> +			phba->fw_config.iscsi_icd_start[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].icd_base;
> +			phba->fw_config.iscsi_icd_count[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].icd_count;
> +
> +			phba->fw_config.iscsi_chain_start[ulp_num] =
> +				pfw_cfg->chain_icd[ulp_num].chain_base;
> +			phba->fw_config.iscsi_chain_count[ulp_num] =
> +				pfw_cfg->chain_icd[ulp_num].chain_count;
> +
> +			beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +				    "BG_%d : Function loaded on ULP :
%d\n"
> +				    "\tiscsi_cid_count : %d\n"
> +				    "\tiscsi_cid_start : %d\n"
> +				    "\t iscsi_icd_count : %d\n"
> +				    "\t iscsi_icd_start : %d\n",
> +				    ulp_num,
> +				    phba->fw_config.
> +				    iscsi_cid_count[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_cid_start[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_icd_count[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_icd_start[ulp_num]);
>   		}
> +	}
>
> -		phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
> -						  BEISCSI_FUNC_DUA_MODE);
> +	if (phba->fw_config.ulp_supported == 0) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : iSCSI initiator mode not set: ULP0 %x
ULP1 %x\n",
> +			    pfw_cfg->ulp[BEISCSI_ULP0].ulp_mode,
> +			    pfw_cfg->ulp[BEISCSI_ULP1].ulp_mode);
> +		goto fail_init;
> +	}
>
> -		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> -			    "BG_%d : DUA Mode : 0x%x\n",
> -			    phba->fw_config.dual_ulp_aware);
> +	/**
> +	 * ICD is shared among ULPs. Use icd_count of any one loaded ULP
> +	 **/
> +	for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
> +		if (test_bit(ulp_num, &phba->fw_config.ulp_supported))
> +			break;
> +	icd_count = phba->fw_config.iscsi_icd_count[ulp_num];
> +	if (icd_count == 0 || icd_count > 65536) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d: invalid ICD count %d\n", icd_count);
> +		goto fail_init;
> +	}
>
> -	} else {
> +	cid_count = BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP0) +
> +		    BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP1);
> +	if (cid_count == 0 || cid_count > 4096) {
>   		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> -			    "BG_%d : Failed in mgmt_get_fw_config\n");
> -		status = -EINVAL;
> +			    "BG_%d: invalid CID count %d\n", cid_count);
> +		goto fail_init;
>   	}
>
> +	phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
> +					  BEISCSI_FUNC_DUA_MODE);
> +
> +	beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +		    "BG_%d : DUA Mode : 0x%x\n",
> +		    phba->fw_config.dual_ulp_aware);
> +
> +	/* all set, continue using this FW config */
> +	status = 0;
> +fail_init:
>   	mutex_unlock(&ctrl->mbox_lock);
>   	return status;
>   }
>
Shouldn't that be 'DUAL mode' ?
But then, even the original had 'DUA'. So no need to redo the patch for
that.

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
--
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284
(AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 10/15] be2iscsi: Add FW config validation

[Jitendra Bhivare]

Yes, the faults are not being used, it just adds to the confusion, we just
intend to get the link state change - UP or DOWN.

-----Original Message-----
From: Hannes Reinecke [mailto:hare@suse.de]
Sent: Friday, December 18, 2015 2:34 PM
To: Jitendra Bhivare; linux-scsi@vger.kernel.org; michaelc@cs.wisc.edu
Subject: Re: [PATCH 10/15] be2iscsi: Add FW config validation

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> System crash in I+T card personality
>
> Fix to add validation for ULP in initiator mode, physical port number,
> and supported queue, icd, cid counts.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_main.c |    2 +-
>   drivers/scsi/be2iscsi/be_main.h |    2 +
>   drivers/scsi/be2iscsi/be_mgmt.c |  188
+++++++++++++++++++++++++--------------
>   3 files changed, 123 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.c
> b/drivers/scsi/be2iscsi/be_main.c index 8967e05..a665e6a 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -5670,6 +5670,7 @@ static int beiscsi_dev_probe(struct pci_dev
*pcidev,
>   		goto free_port;
>   	}
>   	mgmt_get_port_name(&phba->ctrl, phba);
> +	beiscsi_get_params(phba);
>
>   	if (enable_msix)
>   		find_num_cpus(phba);
> @@ -5687,7 +5688,6 @@ static int beiscsi_dev_probe(struct pci_dev
*pcidev,
>   	}
>
>   	phba->shost->max_id = phba->params.cxns_per_ctrl;
> -	beiscsi_get_params(phba);
>   	phba->shost->can_queue = phba->params.ios_per_ctrl;
>   	ret = beiscsi_init_port(phba);
>   	if (ret < 0) {
> diff --git a/drivers/scsi/be2iscsi/be_main.h
> b/drivers/scsi/be2iscsi/be_main.h index c09082a..f89861b 100644
> --- a/drivers/scsi/be2iscsi/be_main.h
> +++ b/drivers/scsi/be2iscsi/be_main.h
> @@ -397,7 +397,9 @@ struct beiscsi_hba {
>   		 * group together since they are used most frequently
>   		 * for cid to cri conversion
>   		 */
> +#define BEISCSI_PHYS_PORT_MAX	4
>   		unsigned int phys_port;
> +		/* valid values of phys_port id are 0, 1, 2, 3 */
>   		unsigned int eqid_count;
>   		unsigned int cqid_count;
>   		unsigned int iscsi_cid_start[BEISCSI_ULP_COUNT];
> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c
> b/drivers/scsi/be2iscsi/be_mgmt.c index ad7aa75..15f7ad7 100644
> --- a/drivers/scsi/be2iscsi/be_mgmt.c
> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
> @@ -373,90 +373,142 @@ int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
>   				struct beiscsi_hba *phba)
>   {
>   	struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem);
> -	struct be_fw_cfg *req = embedded_payload(wrb);
> -	int status = 0;
> +	struct be_fw_cfg *pfw_cfg = embedded_payload(wrb);
> +	uint32_t cid_count, icd_count;
> +	int status = -EINVAL;
> +	uint8_t ulp_num = 0;
>
>   	mutex_lock(&ctrl->mbox_lock);
>   	memset(wrb, 0, sizeof(*wrb));
> +	be_wrb_hdr_prepare(wrb, sizeof(*pfw_cfg), true, 0);
>
> -	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
> -
> -	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
> +	be_cmd_hdr_prepare(&pfw_cfg->hdr, CMD_SUBSYSTEM_COMMON,
>   			   OPCODE_COMMON_QUERY_FIRMWARE_CONFIG,
>   			   EMBED_MBX_MAX_PAYLOAD_SIZE);
> -	status = be_mbox_notify(ctrl);
> -	if (!status) {
> -		uint8_t ulp_num = 0;
> -		struct be_fw_cfg *pfw_cfg;
> -		pfw_cfg = req;
>
> -		if (!is_chip_be2_be3r(phba)) {
> -			phba->fw_config.eqid_count = pfw_cfg->eqid_count;
> -			phba->fw_config.cqid_count = pfw_cfg->cqid_count;
> +	if (be_mbox_notify(ctrl)) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : Failed in mgmt_get_fw_config\n");
> +		goto fail_init;
> +	}
>
> -			beiscsi_log(phba, KERN_INFO,
> -				    BEISCSI_LOG_INIT,
> -				    "BG_%d : EQ_Count : %d CQ_Count :
%d\n",
> -				    phba->fw_config.eqid_count,
> +	/* FW response formats depend on port id */
> +	phba->fw_config.phys_port = pfw_cfg->phys_port;
> +	if (phba->fw_config.phys_port >= BEISCSI_PHYS_PORT_MAX) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : invalid physical port id %d\n",
> +			    phba->fw_config.phys_port);
> +		goto fail_init;
> +	}
> +
> +	/* populate and check FW config against min and max values */
> +	if (!is_chip_be2_be3r(phba)) {
> +		phba->fw_config.eqid_count = pfw_cfg->eqid_count;
> +		phba->fw_config.cqid_count = pfw_cfg->cqid_count;
> +		if (phba->fw_config.eqid_count == 0 ||
> +		    phba->fw_config.eqid_count > 2048) {
> +			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +				    "BG_%d : invalid EQ count %d\n",
> +				    phba->fw_config.eqid_count);
> +			goto fail_init;
> +		}
> +		if (phba->fw_config.cqid_count == 0 ||
> +		    phba->fw_config.cqid_count > 4096) {
> +			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +				    "BG_%d : invalid CQ count %d\n",
>   				    phba->fw_config.cqid_count);
> +			goto fail_init;
>   		}
> +		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +			    "BG_%d : EQ_Count : %d CQ_Count : %d\n",
> +			    phba->fw_config.eqid_count,
> +			    phba->fw_config.cqid_count);
> +	}
>
> -		for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
> -			if (pfw_cfg->ulp[ulp_num].ulp_mode &
> -			    BEISCSI_ULP_ISCSI_INI_MODE)
> -				set_bit(ulp_num,
> -				&phba->fw_config.ulp_supported);
> -
> -		phba->fw_config.phys_port = pfw_cfg->phys_port;
> -		for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
{
> -			if (test_bit(ulp_num,
&phba->fw_config.ulp_supported)) {
> -
> -				phba->fw_config.iscsi_cid_start[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].sq_base;
> -				phba->fw_config.iscsi_cid_count[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].sq_count;
> -
> -				phba->fw_config.iscsi_icd_start[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].icd_base;
> -				phba->fw_config.iscsi_icd_count[ulp_num] =
> -					pfw_cfg->ulp[ulp_num].icd_count;
> -
> -				phba->fw_config.iscsi_chain_start[ulp_num]
=
> -
pfw_cfg->chain_icd[ulp_num].chain_base;
> -				phba->fw_config.iscsi_chain_count[ulp_num]
=
> -
pfw_cfg->chain_icd[ulp_num].chain_count;
> -
> -				beiscsi_log(phba, KERN_INFO,
BEISCSI_LOG_INIT,
> -					    "BG_%d : Function loaded on
ULP : %d\n"
> -					    "\tiscsi_cid_count : %d\n"
> -					    "\tiscsi_cid_start : %d\n"
> -					    "\t iscsi_icd_count : %d\n"
> -					    "\t iscsi_icd_start : %d\n",
> -					    ulp_num,
> -					    phba->fw_config.
> -					    iscsi_cid_count[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_cid_start[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_icd_count[ulp_num],
> -					    phba->fw_config.
> -					    iscsi_icd_start[ulp_num]);
> -			}
> +	/**
> +	 * Check on which all ULP iSCSI Protocol is loaded.
> +	 * Set the Bit for those ULP. This set flag is used
> +	 * at all places in the code to check on which ULP
> +	 * iSCSi Protocol is loaded
> +	 **/
> +	for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
> +		if (pfw_cfg->ulp[ulp_num].ulp_mode &
> +		    BEISCSI_ULP_ISCSI_INI_MODE) {
> +			set_bit(ulp_num, &phba->fw_config.ulp_supported);
> +
> +			/* Get the CID, ICD and Chain count for each ULP
*/
> +			phba->fw_config.iscsi_cid_start[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].sq_base;
> +			phba->fw_config.iscsi_cid_count[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].sq_count;
> +
> +			phba->fw_config.iscsi_icd_start[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].icd_base;
> +			phba->fw_config.iscsi_icd_count[ulp_num] =
> +				pfw_cfg->ulp[ulp_num].icd_count;
> +
> +			phba->fw_config.iscsi_chain_start[ulp_num] =
> +				pfw_cfg->chain_icd[ulp_num].chain_base;
> +			phba->fw_config.iscsi_chain_count[ulp_num] =
> +				pfw_cfg->chain_icd[ulp_num].chain_count;
> +
> +			beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +				    "BG_%d : Function loaded on ULP :
%d\n"
> +				    "\tiscsi_cid_count : %d\n"
> +				    "\tiscsi_cid_start : %d\n"
> +				    "\t iscsi_icd_count : %d\n"
> +				    "\t iscsi_icd_start : %d\n",
> +				    ulp_num,
> +				    phba->fw_config.
> +				    iscsi_cid_count[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_cid_start[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_icd_count[ulp_num],
> +				    phba->fw_config.
> +				    iscsi_icd_start[ulp_num]);
>   		}
> +	}
>
> -		phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
> -						  BEISCSI_FUNC_DUA_MODE);
> +	if (phba->fw_config.ulp_supported == 0) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d : iSCSI initiator mode not set: ULP0 %x
ULP1 %x\n",
> +			    pfw_cfg->ulp[BEISCSI_ULP0].ulp_mode,
> +			    pfw_cfg->ulp[BEISCSI_ULP1].ulp_mode);
> +		goto fail_init;
> +	}
>
> -		beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> -			    "BG_%d : DUA Mode : 0x%x\n",
> -			    phba->fw_config.dual_ulp_aware);
> +	/**
> +	 * ICD is shared among ULPs. Use icd_count of any one loaded ULP
> +	 **/
> +	for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
> +		if (test_bit(ulp_num, &phba->fw_config.ulp_supported))
> +			break;
> +	icd_count = phba->fw_config.iscsi_icd_count[ulp_num];
> +	if (icd_count == 0 || icd_count > 65536) {
> +		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> +			    "BG_%d: invalid ICD count %d\n", icd_count);
> +		goto fail_init;
> +	}
>
> -	} else {
> +	cid_count = BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP0) +
> +		    BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP1);
> +	if (cid_count == 0 || cid_count > 4096) {
>   		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> -			    "BG_%d : Failed in mgmt_get_fw_config\n");
> -		status = -EINVAL;
> +			    "BG_%d: invalid CID count %d\n", cid_count);
> +		goto fail_init;
>   	}
>
> +	phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
> +					  BEISCSI_FUNC_DUA_MODE);
> +
> +	beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> +		    "BG_%d : DUA Mode : 0x%x\n",
> +		    phba->fw_config.dual_ulp_aware);
> +
> +	/* all set, continue using this FW config */
> +	status = 0;
> +fail_init:
>   	mutex_unlock(&ctrl->mbox_lock);
>   	return status;
>   }
>
Shouldn't that be 'DUAL mode' ?
But then, even the original had 'DUA'. So no need to redo the patch for
that.

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
--
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284
(AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/15] be2iscsi: Fix return value for MCC completion

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Change return value of completed MCC EBUSY to EINVAL.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_cmds.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 9bb5905..eced82b 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -323,7 +323,7 @@ static int be_mcc_compl_process(struct be_ctrl_info *ctrl,
 			if (resp_hdr->response_length)
 				return 0;
 		}
-		return -EBUSY;
+		return -EINVAL;
 	}
 	return 0;
 }
-- 
1.7.1

Re: [PATCH 11/15] be2iscsi: Fix return value for MCC completion

[Hannes Reinecke]

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Change return value of completed MCC EBUSY to EINVAL.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_cmds.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
> index 9bb5905..eced82b 100644
> --- a/drivers/scsi/be2iscsi/be_cmds.c
> +++ b/drivers/scsi/be2iscsi/be_cmds.c
> @@ -323,7 +323,7 @@ static int be_mcc_compl_process(struct be_ctrl_info *ctrl,
>   			if (resp_hdr->response_length)
>   				return 0;
>   		}
> -		return -EBUSY;
> +		return -EINVAL;
>   	}
>   	return 0;
>   }
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/15] be2iscsi: Fix IOPOLL implementation

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

OS not responding when running 2 port traffic on 72 CPUs system.

be2iscsi IRQs gets affined to CPU0 when irqbalancer is disabled.
be_iopoll processing completions in BLOCK_IOPOLL_SOFTIRQ hogged CPU0.

1. Use budget to exit the polling loop. beiscsi_process_cq didn't honour it.
2. Rearming of EQ is done only after iopoll completes.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_cmds.c  |    2 +-
 drivers/scsi/be2iscsi/be_iscsi.c |    2 +-
 drivers/scsi/be2iscsi/be_main.c  |   91 +++++++++++++++++++++-----------------
 drivers/scsi/be2iscsi/be_main.h  |    5 +-
 4 files changed, 56 insertions(+), 44 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index eced82b..3dbf6af 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -543,7 +543,7 @@ int beiscsi_process_mcc(struct beiscsi_hba *phba)
 	}
 
 	if (num)
-		hwi_ring_cq_db(phba, phba->ctrl.mcc_obj.cq.id, num, 1, 0);
+		hwi_ring_cq_db(phba, phba->ctrl.mcc_obj.cq.id, num, 1);
 
 	spin_unlock_bh(&phba->ctrl.mcc_cq_lock);
 	return status;
diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index c89a025..3545721 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -1298,7 +1298,7 @@ static void beiscsi_flush_cq(struct beiscsi_hba *phba)
 	for (i = 0; i < phba->num_cpus; i++) {
 		pbe_eq = &phwi_context->be_eq[i];
 		blk_iopoll_disable(&pbe_eq->iopoll);
-		beiscsi_process_cq(pbe_eq);
+		beiscsi_process_cq(pbe_eq, BE2_MAX_NUM_CQ_PROC);
 		blk_iopoll_enable(&pbe_eq->iopoll);
 	}
 }
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index a665e6a..9a86044 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -895,32 +895,21 @@ static irqreturn_t be_isr_mcc(int irq, void *dev_id)
 static irqreturn_t be_isr_msix(int irq, void *dev_id)
 {
 	struct beiscsi_hba *phba;
-	struct be_eq_entry *eqe = NULL;
 	struct be_queue_info *eq;
-	struct be_queue_info *cq;
-	unsigned int num_eq_processed;
 	struct be_eq_obj *pbe_eq;
 
 	pbe_eq = dev_id;
 	eq = &pbe_eq->q;
-	cq = pbe_eq->cq;
-	eqe = queue_tail_node(eq);
 
 	phba = pbe_eq->phba;
-	num_eq_processed = 0;
-	while (eqe->dw[offsetof(struct amap_eq_entry, valid) / 32]
-				& EQE_VALID_MASK) {
-		if (!blk_iopoll_sched_prep(&pbe_eq->iopoll))
-			blk_iopoll_sched(&pbe_eq->iopoll);
-
-		AMAP_SET_BITS(struct amap_eq_entry, valid, eqe, 0);
-		queue_tail_inc(eq);
-		eqe = queue_tail_node(eq);
-		num_eq_processed++;
-	}
-
-	if (num_eq_processed)
-		hwi_ring_eq_db(phba, eq->id, 1,	num_eq_processed, 0, 1);
+	/* disable interrupt till iopoll completes */
+	hwi_ring_eq_db(phba, eq->id, 1,	0, 0, 1);
+	if (!blk_iopoll_sched_prep(&pbe_eq->iopoll))
+		blk_iopoll_sched(&pbe_eq->iopoll);
+	else
+		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_IO,
+			    "BM_%d: received event while polling eq %d cq %d\n",
+			    eq->id, pbe_eq->cq->id);
 
 	return IRQ_HANDLED;
 }
@@ -998,6 +987,7 @@ static irqreturn_t be_isr(int irq, void *dev_id)
 		return IRQ_NONE;
 }
 
+
 static int beiscsi_init_irqs(struct beiscsi_hba *phba)
 {
 	struct pci_dev *pcidev = phba->pcidev;
@@ -1072,7 +1062,7 @@ free_msix_irqs:
 
 void hwi_ring_cq_db(struct beiscsi_hba *phba,
 			   unsigned int id, unsigned int num_processed,
-			   unsigned char rearm, unsigned char event)
+			   unsigned char rearm)
 {
 	u32 val = 0;
 
@@ -2044,7 +2034,7 @@ static void  beiscsi_process_mcc_isr(struct beiscsi_hba *phba)
 
 		if (num_processed >= 32) {
 			hwi_ring_cq_db(phba, mcc_cq->id,
-					num_processed, 0, 0);
+					num_processed, 0);
 			num_processed = 0;
 		}
 		if (mcc_compl->flags & CQE_FLAGS_ASYNC_MASK) {
@@ -2062,24 +2052,25 @@ static void  beiscsi_process_mcc_isr(struct beiscsi_hba *phba)
 	}
 
 	if (num_processed > 0)
-		hwi_ring_cq_db(phba, mcc_cq->id, num_processed, 1, 0);
+		hwi_ring_cq_db(phba, mcc_cq->id, num_processed, 1);
 
 }
 
 /**
  * beiscsi_process_cq()- Process the Completion Queue
  * @pbe_eq: Event Q on which the Completion has come
+ * @budget: Max number of events to processed
  *
  * return
  *     Number of Completion Entries processed.
  **/
-unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq)
+unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq, int budget)
 {
 	struct be_queue_info *cq;
 	struct sol_cqe *sol;
 	struct dmsg_cqe *dmsg;
+	unsigned int total = 0;
 	unsigned int num_processed = 0;
-	unsigned int tot_nump = 0;
 	unsigned short code = 0, cid = 0;
 	uint16_t cri_index = 0;
 	struct beiscsi_conn *beiscsi_conn;
@@ -2130,12 +2121,12 @@ unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq)
 		beiscsi_ep = ep->dd_data;
 		beiscsi_conn = beiscsi_ep->conn;
 
-		if (num_processed >= 32) {
-			hwi_ring_cq_db(phba, cq->id,
-					num_processed, 0, 0);
-			tot_nump += num_processed;
+		/* replenish cq */
+		if (num_processed == 32) {
+			hwi_ring_cq_db(phba, cq->id, 32, 0);
 			num_processed = 0;
 		}
+		total++;
 
 		switch (code) {
 		case SOL_CMD_COMPLETE:
@@ -2180,7 +2171,13 @@ unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq)
 				    "BM_%d : Ignoring %s[%d] on CID : %d\n",
 				    cqe_desc[code], code, cid);
 			break;
+		case CXN_KILLED_HDR_DIGEST_ERR:
 		case SOL_CMD_KILLED_DATA_DIGEST_ERR:
+			beiscsi_log(phba, KERN_ERR,
+				    BEISCSI_LOG_CONFIG | BEISCSI_LOG_IO,
+				    "BM_%d : Cmd Notification %s[%d] on CID : %d\n",
+				    cqe_desc[code], code,  cid);
+			break;
 		case CMD_KILLED_INVALID_STATSN_RCVD:
 		case CMD_KILLED_INVALID_R2T_RCVD:
 		case CMD_CXN_KILLED_LUN_INVALID:
@@ -2206,7 +2203,6 @@ unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq)
 		case CXN_KILLED_PDU_SIZE_EXCEEDS_DSL:
 		case CXN_KILLED_BURST_LEN_MISMATCH:
 		case CXN_KILLED_AHS_RCVD:
-		case CXN_KILLED_HDR_DIGEST_ERR:
 		case CXN_KILLED_UNKNOWN_HDR:
 		case CXN_KILLED_STALE_ITT_TTT_RCVD:
 		case CXN_KILLED_INVALID_ITT_TTT_RCVD:
@@ -2241,13 +2237,12 @@ proc_next_cqe:
 		queue_tail_inc(cq);
 		sol = queue_tail_node(cq);
 		num_processed++;
+		if (total == budget)
+			break;
 	}
 
-	if (num_processed > 0) {
-		tot_nump += num_processed;
-		hwi_ring_cq_db(phba, cq->id, num_processed, 1, 0);
-	}
-	return tot_nump;
+	hwi_ring_cq_db(phba, cq->id, num_processed, 1);
+	return total;
 }
 
 void beiscsi_process_all_cqs(struct work_struct *work)
@@ -2274,7 +2269,7 @@ void beiscsi_process_all_cqs(struct work_struct *work)
 		spin_lock_irqsave(&phba->isr_lock, flags);
 		pbe_eq->todo_cq = false;
 		spin_unlock_irqrestore(&phba->isr_lock, flags);
-		beiscsi_process_cq(pbe_eq);
+		beiscsi_process_cq(pbe_eq, BE2_MAX_NUM_CQ_PROC);
 	}
 
 	/* rearm EQ for further interrupts */
@@ -2283,20 +2278,36 @@ void beiscsi_process_all_cqs(struct work_struct *work)
 
 static int be_iopoll(struct blk_iopoll *iop, int budget)
 {
-	unsigned int ret;
+	unsigned int ret, num_eq_processed;
 	struct beiscsi_hba *phba;
 	struct be_eq_obj *pbe_eq;
+	struct be_eq_entry *eqe = NULL;
+	struct be_queue_info *eq;
 
+	num_eq_processed = 0;
 	pbe_eq = container_of(iop, struct be_eq_obj, iopoll);
-	ret = beiscsi_process_cq(pbe_eq);
+	phba = pbe_eq->phba;
+	eq = &pbe_eq->q;
+	eqe = queue_tail_node(eq);
+
+	while (eqe->dw[offsetof(struct amap_eq_entry, valid) / 32] &
+			EQE_VALID_MASK) {
+		AMAP_SET_BITS(struct amap_eq_entry, valid, eqe, 0);
+		queue_tail_inc(eq);
+		eqe = queue_tail_node(eq);
+		num_eq_processed++;
+	}
+
+	hwi_ring_eq_db(phba, eq->id, 1, num_eq_processed, 0, 1);
+
+	ret = beiscsi_process_cq(pbe_eq, budget);
 	pbe_eq->cq_count += ret;
 	if (ret < budget) {
-		phba = pbe_eq->phba;
 		blk_iopoll_complete(iop);
 		beiscsi_log(phba, KERN_INFO,
 			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_IO,
-			    "BM_%d : rearm pbe_eq->q.id =%d\n",
-			    pbe_eq->q.id);
+			    "BM_%d : rearm pbe_eq->q.id =%d ret %d\n",
+			    pbe_eq->q.id, ret);
 		hwi_ring_eq_db(phba, pbe_eq->q.id, 0, 0, 1, 1);
 	}
 	return ret;
diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index f89861b..fabade3 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -63,6 +63,7 @@
 #define BE2_SGE			32
 #define BE2_DEFPDU_HDR_SZ	64
 #define BE2_DEFPDU_DATA_SZ	8192
+#define BE2_MAX_NUM_CQ_PROC	512
 
 #define MAX_CPUS		64
 #define BEISCSI_MAX_NUM_CPUS	7
@@ -848,9 +849,9 @@ void beiscsi_free_mgmt_task_handles(struct beiscsi_conn *beiscsi_conn,
 
 void hwi_ring_cq_db(struct beiscsi_hba *phba,
 		     unsigned int id, unsigned int num_processed,
-		     unsigned char rearm, unsigned char event);
+		     unsigned char rearm);
 
-unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq);
+unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq, int budget);
 
 static inline bool beiscsi_error(struct beiscsi_hba *phba)
 {
-- 
1.7.1

Re: [PATCH 12/15] be2iscsi: Fix IOPOLL implementation

[Hannes Reinecke]

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> OS not responding when running 2 port traffic on 72 CPUs system.
>
> be2iscsi IRQs gets affined to CPU0 when irqbalancer is disabled.
> be_iopoll processing completions in BLOCK_IOPOLL_SOFTIRQ hogged CPU0.
>
> 1. Use budget to exit the polling loop. beiscsi_process_cq didn't honour it.
> 2. Rearming of EQ is done only after iopoll completes.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_cmds.c  |    2 +-
>   drivers/scsi/be2iscsi/be_iscsi.c |    2 +-
>   drivers/scsi/be2iscsi/be_main.c  |   91 +++++++++++++++++++++-----------------
>   drivers/scsi/be2iscsi/be_main.h  |    5 +-
>   4 files changed, 56 insertions(+), 44 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/15] be2iscsi: Fix to process 25G link speed info from FW

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Async link event provides port_speed info. Use the same to report in
ISCSI_HOST_PARAM_PORT_SPEED query. Removed link status query IOCTL used
to do the same.

25G and 40G are defined in kernel enum iscsi_port_speed.

Fixed get_nic_conf structure definition. Removed rsvd[23] field in
be_cmd_get_nic_conf_resp.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_cmds.c     |   47 ++++++++++++-------------
 drivers/scsi/be2iscsi/be_cmds.h     |   65 ++++++++++------------------------
 drivers/scsi/be2iscsi/be_iscsi.c    |   41 +++++----------------
 drivers/scsi/be2iscsi/be_main.h     |    1 +
 drivers/scsi/be2iscsi/be_mgmt.c     |   28 ---------------
 drivers/scsi/scsi_transport_iscsi.c |    2 +
 include/scsi/iscsi_if.h             |    2 +
 7 files changed, 57 insertions(+), 129 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 3dbf6af..498eba2 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -402,31 +402,31 @@ void beiscsi_fail_session(struct iscsi_cls_session *cls_session)
 	iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED);
 }
 
-static void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
-		struct be_async_event_link_state *evt)
+static void beiscsi_process_async_link(struct beiscsi_hba *phba,
+				       struct be_mcc_compl *compl)
 {
-	if ((evt->port_link_status == ASYNC_EVENT_LINK_DOWN) ||
-	    ((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
-	     (evt->port_fault != BEISCSI_PHY_LINK_FAULT_NONE))) {
-		phba->state = BE_ADAPTER_LINK_DOWN;
+	struct be_async_event_link_state *evt;
 
-		beiscsi_log(phba, KERN_ERR,
-			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_INIT,
-			    "BC_%d : Link Down on Port %d\n",
-			    evt->physical_port);
+	evt = (struct be_async_event_link_state *)compl;
 
-		iscsi_host_for_each_session(phba->shost,
-					    beiscsi_fail_session);
-	} else if ((evt->port_link_status & ASYNC_EVENT_LINK_UP) ||
-		    ((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
-		     (evt->port_fault == BEISCSI_PHY_LINK_FAULT_NONE))) {
+	phba->port_speed = evt->port_speed;
+	/**
+	 * Check logical link status in ASYNC event.
+	 * This has been newly introduced in SKH-R Firmware 10.0.338.45.
+	 **/
+	if (evt->port_link_status & BE_ASYNC_LINK_UP_MASK) {
 		phba->state = BE_ADAPTER_LINK_UP | BE_ADAPTER_CHECK_BOOT;
 		phba->get_boot = BE_GET_BOOT_RETRIES;
-
-		beiscsi_log(phba, KERN_ERR,
-			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_INIT,
-			    "BC_%d : Link UP on Port %d\n",
-			    evt->physical_port);
+		__beiscsi_log(phba, KERN_ERR,
+			      "BC_%d : Link Up on Port %d tag 0x%x\n",
+			      evt->physical_port, evt->event_tag);
+	} else {
+		phba->state = BE_ADAPTER_LINK_DOWN;
+		__beiscsi_log(phba, KERN_ERR,
+			      "BC_%d : Link Down on Port %d tag 0x%x\n",
+			      evt->physical_port, evt->event_tag);
+		iscsi_host_for_each_session(phba->shost,
+					    beiscsi_fail_session);
 	}
 }
 
@@ -503,8 +503,7 @@ void beiscsi_process_async_event(struct beiscsi_hba *phba,
 	evt_code &= ASYNC_TRAILER_EVENT_CODE_MASK;
 	switch (evt_code) {
 	case ASYNC_EVENT_CODE_LINK_STATE:
-		beiscsi_async_link_state_process(phba,
-				(struct be_async_event_link_state *)compl);
+		beiscsi_process_async_link(phba, compl);
 		break;
 	case ASYNC_EVENT_CODE_ISCSI:
 		phba->state |= BE_ADAPTER_CHECK_BOOT;
@@ -520,8 +519,8 @@ void beiscsi_process_async_event(struct beiscsi_hba *phba,
 	}
 
 	beiscsi_log(phba, sev, BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
-		    "BC_%d : ASYNC Event: status 0x%08x flags 0x%08x\n",
-		    compl->status, compl->flags);
+		    "BC_%d : ASYNC Event %x: status 0x%08x flags 0x%08x\n",
+		    evt_code, compl->status, compl->flags);
 }
 
 int beiscsi_process_mcc(struct beiscsi_hba *phba)
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index 6411f7b..3b9bc2e 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -142,7 +142,6 @@ struct be_async_event_trailer {
 enum {
 	ASYNC_EVENT_LINK_DOWN = 0x0,
 	ASYNC_EVENT_LINK_UP = 0x1,
-	ASYNC_EVENT_LOGICAL = 0x2
 };
 
 /**
@@ -152,13 +151,26 @@ enum {
 struct be_async_event_link_state {
 	u8 physical_port;
 	u8 port_link_status;
+/**
+ * ASYNC_EVENT_LINK_DOWN		0x0
+ * ASYNC_EVENT_LINK_UP			0x1
+ * ASYNC_EVENT_LINK_LOGICAL_DOWN	0x2
+ * ASYNC_EVENT_LINK_LOGICAL_UP		0x3
+ */
+#define BE_ASYNC_LINK_UP_MASK		0x01
 	u8 port_duplex;
 	u8 port_speed;
-#define BEISCSI_PHY_LINK_FAULT_NONE	0x00
-#define BEISCSI_PHY_LINK_FAULT_LOCAL	0x01
-#define BEISCSI_PHY_LINK_FAULT_REMOTE	0x02
+/* BE2ISCSI_LINK_SPEED_ZERO	0x00 - no link */
+#define BE2ISCSI_LINK_SPEED_10MBPS	0x01
+#define BE2ISCSI_LINK_SPEED_100MBPS	0x02
+#define BE2ISCSI_LINK_SPEED_1GBPS	0x03
+#define BE2ISCSI_LINK_SPEED_10GBPS	0x04
+#define BE2ISCSI_LINK_SPEED_25GBPS	0x06
+#define BE2ISCSI_LINK_SPEED_40GBPS	0x07
 	u8 port_fault;
-	u8 rsvd0[7];
+	u8 event_reason;
+	u16 qos_link_speed;
+	u32 event_tag;
 	struct be_async_event_trailer trailer;
 } __packed;
 
@@ -675,20 +687,6 @@ struct be_cmd_req_modify_eq_delay {
 
 /******************** Get MAC ADDR *******************/
 
-#define ETH_ALEN	6
-
-struct be_cmd_get_nic_conf_req {
-	struct be_cmd_req_hdr hdr;
-	u32 nic_port_count;
-	u32 speed;
-	u32 max_speed;
-	u32 link_state;
-	u32 max_frame_size;
-	u16 size_of_structure;
-	u8 mac_address[ETH_ALEN];
-	u32 rsvd[23];
-};
-
 struct be_cmd_get_nic_conf_resp {
 	struct be_cmd_resp_hdr hdr;
 	u32 nic_port_count;
@@ -697,9 +695,8 @@ struct be_cmd_get_nic_conf_resp {
 	u32 link_state;
 	u32 max_frame_size;
 	u16 size_of_structure;
-	u8 mac_address[6];
-	u32 rsvd[23];
-};
+	u8 mac_address[ETH_ALEN];
+} __packed;
 
 #define BEISCSI_ALIAS_LEN 32
 
@@ -711,29 +708,6 @@ struct be_cmd_hba_name {
 	u8 initiator_alias[BEISCSI_ALIAS_LEN];
 } __packed;
 
-struct be_cmd_ntwk_link_status_req {
-	struct be_cmd_req_hdr hdr;
-	u32 rsvd0;
-} __packed;
-
-/*** Port Speed Values ***/
-#define BE2ISCSI_LINK_SPEED_ZERO	0x00
-#define BE2ISCSI_LINK_SPEED_10MBPS	0x01
-#define BE2ISCSI_LINK_SPEED_100MBPS	0x02
-#define BE2ISCSI_LINK_SPEED_1GBPS	0x03
-#define BE2ISCSI_LINK_SPEED_10GBPS	0x04
-struct be_cmd_ntwk_link_status_resp {
-	struct be_cmd_resp_hdr hdr;
-	u8 phys_port;
-	u8 mac_duplex;
-	u8 mac_speed;
-	u8 mac_fault;
-	u8 mgmt_mac_duplex;
-	u8 mgmt_mac_speed;
-	u16 qos_link_speed;
-	u32 logical_link_speed;
-} __packed;
-
 int beiscsi_cmd_eq_create(struct be_ctrl_info *ctrl,
 			  struct be_queue_info *eq, int eq_delay);
 
@@ -752,7 +726,6 @@ int be_poll_mcc(struct be_ctrl_info *ctrl);
 int mgmt_check_supported_fw(struct be_ctrl_info *ctrl,
 				      struct beiscsi_hba *phba);
 unsigned int be_cmd_get_initname(struct beiscsi_hba *phba);
-unsigned int be_cmd_get_port_speed(struct beiscsi_hba *phba);
 
 void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag);
 
diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index 3545721..9eef68b 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -758,7 +758,7 @@ static void beiscsi_get_port_state(struct Scsi_Host *shost)
 	struct beiscsi_hba *phba = iscsi_host_priv(shost);
 	struct iscsi_cls_host *ihost = shost->shost_data;
 
-	ihost->port_state = (phba->state == BE_ADAPTER_LINK_UP) ?
+	ihost->port_state = (phba->state & BE_ADAPTER_LINK_UP) ?
 		ISCSI_PORT_STATE_UP : ISCSI_PORT_STATE_DOWN;
 }
 
@@ -766,34 +766,13 @@ static void beiscsi_get_port_state(struct Scsi_Host *shost)
  * beiscsi_get_port_speed  - Get the Port Speed from Adapter
  * @shost : pointer to scsi_host structure
  *
- * returns Success/Failure
  */
-static int beiscsi_get_port_speed(struct Scsi_Host *shost)
+static void beiscsi_get_port_speed(struct Scsi_Host *shost)
 {
-	int rc;
-	unsigned int tag;
-	struct be_mcc_wrb *wrb;
-	struct be_cmd_ntwk_link_status_resp *resp;
 	struct beiscsi_hba *phba = iscsi_host_priv(shost);
 	struct iscsi_cls_host *ihost = shost->shost_data;
 
-	tag = be_cmd_get_port_speed(phba);
-	if (!tag) {
-		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
-			    "BS_%d : Getting Port Speed Failed\n");
-
-		 return -EBUSY;
-	}
-	rc = beiscsi_mccq_compl(phba, tag, &wrb, NULL);
-	if (rc) {
-		beiscsi_log(phba, KERN_ERR,
-			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
-			    "BS_%d : Port Speed MBX Failed\n");
-		return rc;
-	}
-	resp = embedded_payload(wrb);
-
-	switch (resp->mac_speed) {
+	switch (phba->port_speed) {
 	case BE2ISCSI_LINK_SPEED_10MBPS:
 		ihost->port_speed = ISCSI_PORT_SPEED_10MBPS;
 		break;
@@ -806,10 +785,15 @@ static int beiscsi_get_port_speed(struct Scsi_Host *shost)
 	case BE2ISCSI_LINK_SPEED_10GBPS:
 		ihost->port_speed = ISCSI_PORT_SPEED_10GBPS;
 		break;
+	case BE2ISCSI_LINK_SPEED_25GBPS:
+		ihost->port_speed = ISCSI_PORT_SPEED_25GBPS;
+		break;
+	case BE2ISCSI_LINK_SPEED_40GBPS:
+		ihost->port_speed = ISCSI_PORT_SPEED_40GBPS;
+		break;
 	default:
 		ihost->port_speed = ISCSI_PORT_SPEED_UNKNOWN;
 	}
-	return 0;
 }
 
 /**
@@ -859,12 +843,7 @@ int beiscsi_get_host_param(struct Scsi_Host *shost,
 		status = sprintf(buf, "%s\n", iscsi_get_port_state_name(shost));
 		break;
 	case ISCSI_HOST_PARAM_PORT_SPEED:
-		status = beiscsi_get_port_speed(shost);
-		if (status) {
-			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
-				    "BS_%d : Retreiving Port Speed Failed\n");
-			return status;
-		}
+		beiscsi_get_port_speed(shost);
 		status = sprintf(buf, "%s\n", iscsi_get_port_speed_name(shost));
 		break;
 	default:
diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index fabade3..41c708c 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -427,6 +427,7 @@ struct beiscsi_hba {
 	bool mac_addr_set;
 	u8 mac_address[ETH_ALEN];
 	u8 port_name;
+	u8 port_speed;
 	char fw_ver_str[BEISCSI_VER_STRLEN];
 	char wq_name[20];
 	struct workqueue_struct *wq;	/* The actuak work queue */
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index 15f7ad7..5c66f44 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -1382,34 +1382,6 @@ unsigned int be_cmd_get_initname(struct beiscsi_hba *phba)
 	return tag;
 }
 
-unsigned int be_cmd_get_port_speed(struct beiscsi_hba *phba)
-{
-	unsigned int tag = 0;
-	struct be_mcc_wrb *wrb;
-	struct be_cmd_ntwk_link_status_req *req;
-	struct be_ctrl_info *ctrl = &phba->ctrl;
-
-	if (mutex_lock_interruptible(&ctrl->mbox_lock))
-		return 0;
-	tag = alloc_mcc_tag(phba);
-	if (!tag) {
-		mutex_unlock(&ctrl->mbox_lock);
-		return tag;
-	}
-
-	wrb = wrb_from_mccq(phba);
-	req = embedded_payload(wrb);
-	wrb->tag0 |= tag;
-	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
-	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
-			OPCODE_COMMON_NTWK_LINK_STATUS_QUERY,
-			sizeof(*req));
-
-	be_mcc_notify(phba);
-	mutex_unlock(&ctrl->mbox_lock);
-	return tag;
-}
-
 /**
  * be_mgmt_get_boot_shandle()- Get the session handle
  * @phba: device priv structure instance
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index e4b3d8f..4414816 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -4308,6 +4308,8 @@ static const struct {
 	{ISCSI_PORT_SPEED_100MBPS,	"100 Mbps" },
 	{ISCSI_PORT_SPEED_1GBPS,	"1 Gbps" },
 	{ISCSI_PORT_SPEED_10GBPS,	"10 Gbps" },
+	{ISCSI_PORT_SPEED_25GBPS,       "25 Gbps" },
+	{ISCSI_PORT_SPEED_40GBPS,       "40 Gbps" },
 };
 
 char *iscsi_get_port_speed_name(struct Scsi_Host *shost)
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index 95ed942..d66c070 100644
--- a/include/scsi/iscsi_if.h
+++ b/include/scsi/iscsi_if.h
@@ -724,6 +724,8 @@ enum iscsi_port_speed {
 	ISCSI_PORT_SPEED_100MBPS	= 0x4,
 	ISCSI_PORT_SPEED_1GBPS		= 0x8,
 	ISCSI_PORT_SPEED_10GBPS		= 0x10,
+	ISCSI_PORT_SPEED_25GBPS         = 0x20,
+	ISCSI_PORT_SPEED_40GBPS         = 0x40,
 };
 
 /* iSCSI port state */
-- 
1.7.1

Re: [PATCH 13/15] be2iscsi: Fix to process 25G link speed info from FW

[Hannes Reinecke]

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Async link event provides port_speed info. Use the same to report in
> ISCSI_HOST_PARAM_PORT_SPEED query. Removed link status query IOCTL used
> to do the same.
>
> 25G and 40G are defined in kernel enum iscsi_port_speed.
>
> Fixed get_nic_conf structure definition. Removed rsvd[23] field in
> be_cmd_get_nic_conf_resp.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_cmds.c     |   47 ++++++++++++-------------
>   drivers/scsi/be2iscsi/be_cmds.h     |   65 ++++++++++------------------------
>   drivers/scsi/be2iscsi/be_iscsi.c    |   41 +++++----------------
>   drivers/scsi/be2iscsi/be_main.h     |    1 +
>   drivers/scsi/be2iscsi/be_mgmt.c     |   28 ---------------
>   drivers/scsi/scsi_transport_iscsi.c |    2 +
>   include/scsi/iscsi_if.h             |    2 +
>   7 files changed, 57 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
> index 3dbf6af..498eba2 100644
> --- a/drivers/scsi/be2iscsi/be_cmds.c
> +++ b/drivers/scsi/be2iscsi/be_cmds.c
> @@ -402,31 +402,31 @@ void beiscsi_fail_session(struct iscsi_cls_session *cls_session)
>   	iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED);
>   }
>
> -static void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
> -		struct be_async_event_link_state *evt)
> +static void beiscsi_process_async_link(struct beiscsi_hba *phba,
> +				       struct be_mcc_compl *compl)
>   {
> -	if ((evt->port_link_status == ASYNC_EVENT_LINK_DOWN) ||
> -	    ((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
> -	     (evt->port_fault != BEISCSI_PHY_LINK_FAULT_NONE))) {
> -		phba->state = BE_ADAPTER_LINK_DOWN;
> +	struct be_async_event_link_state *evt;
>
> -		beiscsi_log(phba, KERN_ERR,
> -			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_INIT,
> -			    "BC_%d : Link Down on Port %d\n",
> -			    evt->physical_port);
> +	evt = (struct be_async_event_link_state *)compl;
>
> -		iscsi_host_for_each_session(phba->shost,
> -					    beiscsi_fail_session);
> -	} else if ((evt->port_link_status & ASYNC_EVENT_LINK_UP) ||
> -		    ((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
> -		     (evt->port_fault == BEISCSI_PHY_LINK_FAULT_NONE))) {
> +	phba->port_speed = evt->port_speed;
> +	/**
> +	 * Check logical link status in ASYNC event.
> +	 * This has been newly introduced in SKH-R Firmware 10.0.338.45.
> +	 **/
> +	if (evt->port_link_status & BE_ASYNC_LINK_UP_MASK) {
>   		phba->state = BE_ADAPTER_LINK_UP | BE_ADAPTER_CHECK_BOOT;
>   		phba->get_boot = BE_GET_BOOT_RETRIES;
> -
> -		beiscsi_log(phba, KERN_ERR,
> -			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_INIT,
> -			    "BC_%d : Link UP on Port %d\n",
> -			    evt->physical_port);
> +		__beiscsi_log(phba, KERN_ERR,
> +			      "BC_%d : Link Up on Port %d tag 0x%x\n",
> +			      evt->physical_port, evt->event_tag);
> +	} else {
> +		phba->state = BE_ADAPTER_LINK_DOWN;
> +		__beiscsi_log(phba, KERN_ERR,
> +			      "BC_%d : Link Down on Port %d tag 0x%x\n",
> +			      evt->physical_port, evt->event_tag);
> +		iscsi_host_for_each_session(phba->shost,
> +					    beiscsi_fail_session);
>   	}
>   }
>
> @@ -503,8 +503,7 @@ void beiscsi_process_async_event(struct beiscsi_hba *phba,
>   	evt_code &= ASYNC_TRAILER_EVENT_CODE_MASK;
>   	switch (evt_code) {
>   	case ASYNC_EVENT_CODE_LINK_STATE:
> -		beiscsi_async_link_state_process(phba,
> -				(struct be_async_event_link_state *)compl);
> +		beiscsi_process_async_link(phba, compl);
>   		break;
>   	case ASYNC_EVENT_CODE_ISCSI:
>   		phba->state |= BE_ADAPTER_CHECK_BOOT;
> @@ -520,8 +519,8 @@ void beiscsi_process_async_event(struct beiscsi_hba *phba,
>   	}
>
>   	beiscsi_log(phba, sev, BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
> -		    "BC_%d : ASYNC Event: status 0x%08x flags 0x%08x\n",
> -		    compl->status, compl->flags);
> +		    "BC_%d : ASYNC Event %x: status 0x%08x flags 0x%08x\n",
> +		    evt_code, compl->status, compl->flags);
>   }
>
>   int beiscsi_process_mcc(struct beiscsi_hba *phba)
> diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
> index 6411f7b..3b9bc2e 100644
> --- a/drivers/scsi/be2iscsi/be_cmds.h
> +++ b/drivers/scsi/be2iscsi/be_cmds.h
> @@ -142,7 +142,6 @@ struct be_async_event_trailer {
>   enum {
>   	ASYNC_EVENT_LINK_DOWN = 0x0,
>   	ASYNC_EVENT_LINK_UP = 0x1,
> -	ASYNC_EVENT_LOGICAL = 0x2
>   };
>
>   /**
> @@ -152,13 +151,26 @@ enum {
>   struct be_async_event_link_state {
>   	u8 physical_port;
>   	u8 port_link_status;
> +/**
> + * ASYNC_EVENT_LINK_DOWN		0x0
> + * ASYNC_EVENT_LINK_UP			0x1
> + * ASYNC_EVENT_LINK_LOGICAL_DOWN	0x2
> + * ASYNC_EVENT_LINK_LOGICAL_UP		0x3
> + */
> +#define BE_ASYNC_LINK_UP_MASK		0x01
>   	u8 port_duplex;
>   	u8 port_speed;
> -#define BEISCSI_PHY_LINK_FAULT_NONE	0x00
> -#define BEISCSI_PHY_LINK_FAULT_LOCAL	0x01
> -#define BEISCSI_PHY_LINK_FAULT_REMOTE	0x02
> +/* BE2ISCSI_LINK_SPEED_ZERO	0x00 - no link */
> +#define BE2ISCSI_LINK_SPEED_10MBPS	0x01
> +#define BE2ISCSI_LINK_SPEED_100MBPS	0x02
> +#define BE2ISCSI_LINK_SPEED_1GBPS	0x03
> +#define BE2ISCSI_LINK_SPEED_10GBPS	0x04
> +#define BE2ISCSI_LINK_SPEED_25GBPS	0x06
> +#define BE2ISCSI_LINK_SPEED_40GBPS	0x07
>   	u8 port_fault;
> -	u8 rsvd0[7];
> +	u8 event_reason;
> +	u16 qos_link_speed;
> +	u32 event_tag;
>   	struct be_async_event_trailer trailer;
>   } __packed;
>
Why did you drop the link_fault definitions, but kept the structure 
field?
Is this intentional?

> @@ -675,20 +687,6 @@ struct be_cmd_req_modify_eq_delay {
>
>   /******************** Get MAC ADDR *******************/
>
> -#define ETH_ALEN	6
> -
> -struct be_cmd_get_nic_conf_req {
> -	struct be_cmd_req_hdr hdr;
> -	u32 nic_port_count;
> -	u32 speed;
> -	u32 max_speed;
> -	u32 link_state;
> -	u32 max_frame_size;
> -	u16 size_of_structure;
> -	u8 mac_address[ETH_ALEN];
> -	u32 rsvd[23];
> -};
> -
>   struct be_cmd_get_nic_conf_resp {
>   	struct be_cmd_resp_hdr hdr;
>   	u32 nic_port_count;
> @@ -697,9 +695,8 @@ struct be_cmd_get_nic_conf_resp {
>   	u32 link_state;
>   	u32 max_frame_size;
>   	u16 size_of_structure;
> -	u8 mac_address[6];
> -	u32 rsvd[23];
> -};
> +	u8 mac_address[ETH_ALEN];
> +} __packed;
>
>   #define BEISCSI_ALIAS_LEN 32
>
> @@ -711,29 +708,6 @@ struct be_cmd_hba_name {
>   	u8 initiator_alias[BEISCSI_ALIAS_LEN];
>   } __packed;
>
> -struct be_cmd_ntwk_link_status_req {
> -	struct be_cmd_req_hdr hdr;
> -	u32 rsvd0;
> -} __packed;
> -
> -/*** Port Speed Values ***/
> -#define BE2ISCSI_LINK_SPEED_ZERO	0x00
> -#define BE2ISCSI_LINK_SPEED_10MBPS	0x01
> -#define BE2ISCSI_LINK_SPEED_100MBPS	0x02
> -#define BE2ISCSI_LINK_SPEED_1GBPS	0x03
> -#define BE2ISCSI_LINK_SPEED_10GBPS	0x04
> -struct be_cmd_ntwk_link_status_resp {
> -	struct be_cmd_resp_hdr hdr;
> -	u8 phys_port;
> -	u8 mac_duplex;
> -	u8 mac_speed;
> -	u8 mac_fault;
> -	u8 mgmt_mac_duplex;
> -	u8 mgmt_mac_speed;
> -	u16 qos_link_speed;
> -	u32 logical_link_speed;
> -} __packed;
> -
>   int beiscsi_cmd_eq_create(struct be_ctrl_info *ctrl,
>   			  struct be_queue_info *eq, int eq_delay);
>
> @@ -752,7 +726,6 @@ int be_poll_mcc(struct be_ctrl_info *ctrl);
>   int mgmt_check_supported_fw(struct be_ctrl_info *ctrl,
>   				      struct beiscsi_hba *phba);
>   unsigned int be_cmd_get_initname(struct beiscsi_hba *phba);
> -unsigned int be_cmd_get_port_speed(struct beiscsi_hba *phba);
>
>   void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag);
>
> diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
> index 3545721..9eef68b 100644
> --- a/drivers/scsi/be2iscsi/be_iscsi.c
> +++ b/drivers/scsi/be2iscsi/be_iscsi.c
> @@ -758,7 +758,7 @@ static void beiscsi_get_port_state(struct Scsi_Host *shost)
>   	struct beiscsi_hba *phba = iscsi_host_priv(shost);
>   	struct iscsi_cls_host *ihost = shost->shost_data;
>
> -	ihost->port_state = (phba->state == BE_ADAPTER_LINK_UP) ?
> +	ihost->port_state = (phba->state & BE_ADAPTER_LINK_UP) ?
>   		ISCSI_PORT_STATE_UP : ISCSI_PORT_STATE_DOWN;
>   }
>
> @@ -766,34 +766,13 @@ static void beiscsi_get_port_state(struct Scsi_Host *shost)
>    * beiscsi_get_port_speed  - Get the Port Speed from Adapter
>    * @shost : pointer to scsi_host structure
>    *
> - * returns Success/Failure
>    */
> -static int beiscsi_get_port_speed(struct Scsi_Host *shost)
> +static void beiscsi_get_port_speed(struct Scsi_Host *shost)
>   {
> -	int rc;
> -	unsigned int tag;
> -	struct be_mcc_wrb *wrb;
> -	struct be_cmd_ntwk_link_status_resp *resp;
>   	struct beiscsi_hba *phba = iscsi_host_priv(shost);
>   	struct iscsi_cls_host *ihost = shost->shost_data;
>
> -	tag = be_cmd_get_port_speed(phba);
> -	if (!tag) {
> -		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
> -			    "BS_%d : Getting Port Speed Failed\n");
> -
> -		 return -EBUSY;
> -	}
> -	rc = beiscsi_mccq_compl(phba, tag, &wrb, NULL);
> -	if (rc) {
> -		beiscsi_log(phba, KERN_ERR,
> -			    BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
> -			    "BS_%d : Port Speed MBX Failed\n");
> -		return rc;
> -	}
> -	resp = embedded_payload(wrb);
> -
> -	switch (resp->mac_speed) {
> +	switch (phba->port_speed) {
>   	case BE2ISCSI_LINK_SPEED_10MBPS:
>   		ihost->port_speed = ISCSI_PORT_SPEED_10MBPS;
>   		break;
> @@ -806,10 +785,15 @@ static int beiscsi_get_port_speed(struct Scsi_Host *shost)
>   	case BE2ISCSI_LINK_SPEED_10GBPS:
>   		ihost->port_speed = ISCSI_PORT_SPEED_10GBPS;
>   		break;
> +	case BE2ISCSI_LINK_SPEED_25GBPS:
> +		ihost->port_speed = ISCSI_PORT_SPEED_25GBPS;
> +		break;
> +	case BE2ISCSI_LINK_SPEED_40GBPS:
> +		ihost->port_speed = ISCSI_PORT_SPEED_40GBPS;
> +		break;
>   	default:
>   		ihost->port_speed = ISCSI_PORT_SPEED_UNKNOWN;
>   	}
> -	return 0;
>   }
>
>   /**
> @@ -859,12 +843,7 @@ int beiscsi_get_host_param(struct Scsi_Host *shost,
>   		status = sprintf(buf, "%s\n", iscsi_get_port_state_name(shost));
>   		break;
>   	case ISCSI_HOST_PARAM_PORT_SPEED:
> -		status = beiscsi_get_port_speed(shost);
> -		if (status) {
> -			beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
> -				    "BS_%d : Retreiving Port Speed Failed\n");
> -			return status;
> -		}
> +		beiscsi_get_port_speed(shost);
>   		status = sprintf(buf, "%s\n", iscsi_get_port_speed_name(shost));
>   		break;
>   	default:
> diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
> index fabade3..41c708c 100644
> --- a/drivers/scsi/be2iscsi/be_main.h
> +++ b/drivers/scsi/be2iscsi/be_main.h
> @@ -427,6 +427,7 @@ struct beiscsi_hba {
>   	bool mac_addr_set;
>   	u8 mac_address[ETH_ALEN];
>   	u8 port_name;
> +	u8 port_speed;
>   	char fw_ver_str[BEISCSI_VER_STRLEN];
>   	char wq_name[20];
>   	struct workqueue_struct *wq;	/* The actuak work queue */
> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
> index 15f7ad7..5c66f44 100644
> --- a/drivers/scsi/be2iscsi/be_mgmt.c
> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
> @@ -1382,34 +1382,6 @@ unsigned int be_cmd_get_initname(struct beiscsi_hba *phba)
>   	return tag;
>   }
>
> -unsigned int be_cmd_get_port_speed(struct beiscsi_hba *phba)
> -{
> -	unsigned int tag = 0;
> -	struct be_mcc_wrb *wrb;
> -	struct be_cmd_ntwk_link_status_req *req;
> -	struct be_ctrl_info *ctrl = &phba->ctrl;
> -
> -	if (mutex_lock_interruptible(&ctrl->mbox_lock))
> -		return 0;
> -	tag = alloc_mcc_tag(phba);
> -	if (!tag) {
> -		mutex_unlock(&ctrl->mbox_lock);
> -		return tag;
> -	}
> -
> -	wrb = wrb_from_mccq(phba);
> -	req = embedded_payload(wrb);
> -	wrb->tag0 |= tag;
> -	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
> -	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
> -			OPCODE_COMMON_NTWK_LINK_STATUS_QUERY,
> -			sizeof(*req));
> -
> -	be_mcc_notify(phba);
> -	mutex_unlock(&ctrl->mbox_lock);
> -	return tag;
> -}
> -
>   /**
>    * be_mgmt_get_boot_shandle()- Get the session handle
>    * @phba: device priv structure instance
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index e4b3d8f..4414816 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -4308,6 +4308,8 @@ static const struct {
>   	{ISCSI_PORT_SPEED_100MBPS,	"100 Mbps" },
>   	{ISCSI_PORT_SPEED_1GBPS,	"1 Gbps" },
>   	{ISCSI_PORT_SPEED_10GBPS,	"10 Gbps" },
> +	{ISCSI_PORT_SPEED_25GBPS,       "25 Gbps" },
> +	{ISCSI_PORT_SPEED_40GBPS,       "40 Gbps" },
>   };
>
>   char *iscsi_get_port_speed_name(struct Scsi_Host *shost)
> diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
> index 95ed942..d66c070 100644
> --- a/include/scsi/iscsi_if.h
> +++ b/include/scsi/iscsi_if.h
> @@ -724,6 +724,8 @@ enum iscsi_port_speed {
>   	ISCSI_PORT_SPEED_100MBPS	= 0x4,
>   	ISCSI_PORT_SPEED_1GBPS		= 0x8,
>   	ISCSI_PORT_SPEED_10GBPS		= 0x10,
> +	ISCSI_PORT_SPEED_25GBPS         = 0x20,
> +	ISCSI_PORT_SPEED_40GBPS         = 0x40,
>   };
>
>   /* iSCSI port state */
>
By rights these two bits should be in a separate patch, as they are 
updating the infrastructure and are not directly related to the 
be2iscsi driver.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/15] be2iscsi: Fix to process 25G link speed info from FW

[Hannes Reinecke]

On 12/21/2015 04:00 AM, Jitendra Bhivare wrote:
> Yes, the faults are not being used, it just adds to the confusion,
> where we just want to get the link state change up or down.
>
So make it a separate patch explaining this.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 14/15] be2iscsi: Fix WRB leak in login/logout path

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Login/Logout loop was hanging after few hours. /var/log/message showed
that alloc_wrb_handle() function was not able to allocate any new WRB.

Sep 11 11:25:22 Jhelum10 kernel: connection32513:0: Could not send nopout
Sep 11 11:25:22 Jhelum10 kernel: scsi host10: BM_4989 : Alloc of WRB_HANDLE
Failedfor the CID : 384
Sep 11 11:25:22 Jhelum10 kernel: connection32513:0: Could not allocate pdu for
mgmt task.

Driver allocates WRB to pass login negotiated parameters information to FW
in beiscsi_offload_connection(). This allocated WRB was not freed so there
was WRB_Leak happening.

Put WRB used for posting the login-negotiated parameters back in pool.

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_main.c |   72 ++++++++++++++++++++++++++------------
 1 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 9a86044..d8cdb4e 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -1184,6 +1184,22 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle)
 		phba->io_sgl_free_index++;
 }
 
+static inline struct wrb_handle *
+beiscsi_get_wrb_handle(struct hwi_wrb_context *pwrb_context,
+		       unsigned int wrbs_per_cxn)
+{
+	struct wrb_handle *pwrb_handle;
+
+	pwrb_handle = pwrb_context->pwrb_handle_base[pwrb_context->alloc_index];
+	pwrb_context->wrb_handles_available--;
+	if (pwrb_context->alloc_index == (wrbs_per_cxn - 1))
+		pwrb_context->alloc_index = 0;
+	else
+		pwrb_context->alloc_index++;
+
+	return pwrb_handle;
+}
+
 /**
  * alloc_wrb_handle - To allocate a wrb handle
  * @phba: The hba pointer
@@ -1193,30 +1209,30 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle)
  * This happens under session_lock until submission to chip
  */
 struct wrb_handle *alloc_wrb_handle(struct beiscsi_hba *phba, unsigned int cid,
-				     struct hwi_wrb_context **pcontext)
+				    struct hwi_wrb_context **pcontext)
 {
 	struct hwi_wrb_context *pwrb_context;
 	struct hwi_controller *phwi_ctrlr;
-	struct wrb_handle *pwrb_handle;
 	uint16_t cri_index = BE_GET_CRI_FROM_CID(cid);
 
 	phwi_ctrlr = phba->phwi_ctrlr;
 	pwrb_context = &phwi_ctrlr->wrb_context[cri_index];
-	if (pwrb_context->wrb_handles_available >= 2) {
-		pwrb_handle = pwrb_context->pwrb_handle_base[
-					    pwrb_context->alloc_index];
-		pwrb_context->wrb_handles_available--;
-		if (pwrb_context->alloc_index ==
-						(phba->params.wrbs_per_cxn - 1))
-			pwrb_context->alloc_index = 0;
-		else
-			pwrb_context->alloc_index++;
+	/* return the context address */
+	*pcontext = pwrb_context;
+	return beiscsi_get_wrb_handle(pwrb_context, phba->params.wrbs_per_cxn);
+}
 
-		/* Return the context address */
-		*pcontext = pwrb_context;
-	} else
-		pwrb_handle = NULL;
-	return pwrb_handle;
+static inline void
+beiscsi_put_wrb_handle(struct hwi_wrb_context *pwrb_context,
+		       struct wrb_handle *pwrb_handle,
+		       unsigned int wrbs_per_cxn)
+{
+	pwrb_context->pwrb_handle_base[pwrb_context->free_index] = pwrb_handle;
+	pwrb_context->wrb_handles_available++;
+	if (pwrb_context->free_index == (wrbs_per_cxn - 1))
+		pwrb_context->free_index = 0;
+	else
+		pwrb_context->free_index++;
 }
 
 /**
@@ -1231,13 +1247,9 @@ static void
 free_wrb_handle(struct beiscsi_hba *phba, struct hwi_wrb_context *pwrb_context,
 		struct wrb_handle *pwrb_handle)
 {
-	pwrb_context->pwrb_handle_base[pwrb_context->free_index] = pwrb_handle;
-	pwrb_context->wrb_handles_available++;
-	if (pwrb_context->free_index == (phba->params.wrbs_per_cxn - 1))
-		pwrb_context->free_index = 0;
-	else
-		pwrb_context->free_index++;
-
+	beiscsi_put_wrb_handle(pwrb_context,
+			       pwrb_handle,
+			       phba->params.wrbs_per_cxn);
 	beiscsi_log(phba, KERN_INFO,
 		    BEISCSI_LOG_IO | BEISCSI_LOG_CONFIG,
 		    "BM_%d : FREE WRB: pwrb_handle=%p free_index=0x%x"
@@ -4715,6 +4727,20 @@ beiscsi_offload_connection(struct beiscsi_conn *beiscsi_conn,
 	doorbell |= 1 << DB_DEF_PDU_NUM_POSTED_SHIFT;
 	iowrite32(doorbell, phba->db_va +
 		  beiscsi_conn->doorbell_offset);
+
+	/*
+	 * There is no completion for CONTEXT_UPDATE. The completion of next
+	 * WRB posted guarantees FW's processing and DMA'ing of it.
+	 * Use beiscsi_put_wrb_handle to put it back in the pool which makes
+	 * sure zero'ing or reuse of the WRB only after wrbs_per_cxn.
+	 */
+	beiscsi_put_wrb_handle(pwrb_context, pwrb_handle,
+			       phba->params.wrbs_per_cxn);
+	beiscsi_log(phba, KERN_INFO,
+		    BEISCSI_LOG_IO | BEISCSI_LOG_CONFIG,
+		    "BM_%d : put CONTEXT_UPDATE pwrb_handle=%p free_index=0x%x wrb_handles_available=%d\n",
+		    pwrb_handle, pwrb_context->free_index,
+		    pwrb_context->wrb_handles_available);
 }
 
 static void beiscsi_parse_pdu(struct iscsi_conn *conn, itt_t itt,
-- 
1.7.1

Re: [PATCH 14/15] be2iscsi: Fix WRB leak in login/logout path

[Hannes Reinecke]

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Login/Logout loop was hanging after few hours. /var/log/message showed
> that alloc_wrb_handle() function was not able to allocate any new WRB.
>
> Sep 11 11:25:22 Jhelum10 kernel: connection32513:0: Could not send nopout
> Sep 11 11:25:22 Jhelum10 kernel: scsi host10: BM_4989 : Alloc of WRB_HANDLE
> Failedfor the CID : 384
> Sep 11 11:25:22 Jhelum10 kernel: connection32513:0: Could not allocate pdu for
> mgmt task.
>
> Driver allocates WRB to pass login negotiated parameters information to FW
> in beiscsi_offload_connection(). This allocated WRB was not freed so there
> was WRB_Leak happening.
>
> Put WRB used for posting the login-negotiated parameters back in pool.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_main.c |   72 ++++++++++++++++++++++++++------------
>   1 files changed, 49 insertions(+), 23 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 15/15] be2iscsi: Update the driver version

[Jitendra Bhivare]

From: Jitendra <jitendra.bhivare@avagotech.com>

Driver version: 11.0.0.0

Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be_main.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index 41c708c..16a6fd0 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -36,7 +36,7 @@
 #include <scsi/scsi_transport_iscsi.h>
 
 #define DRV_NAME		"be2iscsi"
-#define BUILD_STR		"10.6.0.1"
+#define BUILD_STR		"11.0.0.0"
 #define BE_NAME			"Emulex OneConnect" \
 				"Open-iSCSI Driver version" BUILD_STR
 #define DRV_DESC		BE_NAME " " "Driver"
-- 
1.7.1

Re: [PATCH 15/15] be2iscsi: Update the driver version

[Hannes Reinecke]

On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> Driver version: 11.0.0.0
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
>   drivers/scsi/be2iscsi/be_main.h |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
> index 41c708c..16a6fd0 100644
> --- a/drivers/scsi/be2iscsi/be_main.h
> +++ b/drivers/scsi/be2iscsi/be_main.h
> @@ -36,7 +36,7 @@
>   #include <scsi/scsi_transport_iscsi.h>
>
>   #define DRV_NAME		"be2iscsi"
> -#define BUILD_STR		"10.6.0.1"
> +#define BUILD_STR		"11.0.0.0"
>   #define BE_NAME			"Emulex OneConnect" \
>   				"Open-iSCSI Driver version" BUILD_STR
>   #define DRV_DESC		BE_NAME " " "Driver"
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/15] be2iscsi: driver update 11.0.0.0

[Hannes Reinecke]

Hi Jitendra,

On 12/15/2015 04:54 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> This patch is generated against for-next branch.
>
> Jitendra (15):
>    be2iscsi: Fix soft lockup in mgmt_get_all_if_id path using bmbx
>    be2iscsi: Fix mbox synchronization replacing spinlock with mutex
>    be2iscsi: Fix to use atomic operations for tag_state
>    be2iscsi: Fix to synchronize tag allocation using spin_lock
>    be2iscsi: Set mbox timeout to 30s
>    be2iscsi: Added return value check for mgmt_get_all_if_id
>    be2iscsi: Fix to remove shutdown entry point
>    be2iscsi: Fix VLAN support for IPv6 network
>    be2iscsi: Fix to handle misconfigured optics events
>    be2iscsi: Add FW config validation
>    be2iscsi: Fix return value for MCC completion
>    be2iscsi: Fix IOPOLL implementation
>    be2iscsi: Fix to process 25G link speed info from FW
>    be2iscsi: Fix WRB leak in login/logout path
>    be2iscsi: Update the driver version
>
>   drivers/scsi/be2iscsi/be.h          |    4 +-
>   drivers/scsi/be2iscsi/be_cmds.c     |  368 ++++++++++++++++++---------------
>   drivers/scsi/be2iscsi/be_cmds.h     |  114 ++++++-----
>   drivers/scsi/be2iscsi/be_iscsi.c    |   54 ++---
>   drivers/scsi/be2iscsi/be_main.c     |  210 ++++++++++---------
>   drivers/scsi/be2iscsi/be_main.h     |   23 ++-
>   drivers/scsi/be2iscsi/be_mgmt.c     |  387 +++++++++++++++++++++--------------
>   drivers/scsi/be2iscsi/be_mgmt.h     |    2 +
>   drivers/scsi/scsi_transport_iscsi.c |    2 +
>   include/scsi/iscsi_if.h             |    2 +
>   10 files changed, 648 insertions(+), 518 deletions(-)
>
Could you please add the 'Reviewed-by' tags (where applicable) if 
you resend the patch series?
This saves us _quite_ some time, as we then can concentrate on the 
patches which have changed, and can skip those which we already have 
reviewed.
Otherwise we have to go through the entire patchset and figure out 
if they are already reviewed, taking up quite some time.
Plus we might be finding other issues which we haven't seen on the 
previous round :-)

Thanks.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html