All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/10] qla2xxx: Bug fixes for driver.
@ 2016-12-24  2:06 Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 01/10] qla2xxx: Fix wrong IOCB type assumption Himanshu Madhani
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

Hi Bart,

Here's updated series of bug fixes for target code in the driver.
Please consider this for target-pending.

Changes from v2 --> v3
o Added Reviewed-by tag for Christoph to complete series.
o Fixed sparse warning for patch to collect ATIO-Q in firmware dump.
o Fixed comment style and field for corrupted ATIO patch.
o Fixed minor usage of cpu_to_le32() instead of __constant_cpu_to_le32()
o Added description for patch which addresses NULL pointer access.

Changes from v1 --> v2

o Updated patches to remove braces. 
o Added description for the patch reqeusted.

Thanks,
Himanshu

Himanshu Madhani (3):
  qla2xxx: Include ATIO queue in firmware dump when in target mode
  qla2xxx: Set tcm_qla2xxx version to automatically track qla2xxx
    version.
  qla2xxx: Reset reserved field in firmware options to 0.

Quinn Tran (7):
  qla2xxx: Fix wrong IOCB type assumption.
  qla2xxx: Collect additional information to debug fw dump.
  qla2xxx: Fix crash due to null pointer access.
  qla2xxx: Terminate exchange if corruputed.
  qla2xxx: Reduce exess wait during chip reset
  qla2xxx: Fix invalid handle erroneous message.
  qla2xxx: Disable Out-of-order processing by default in Firmware

 drivers/scsi/qla2xxx/qla_def.h     |  3 ++-
 drivers/scsi/qla2xxx/qla_init.c    |  4 +--
 drivers/scsi/qla2xxx/qla_isr.c     |  4 +++
 drivers/scsi/qla2xxx/qla_mbx.c     | 27 ++++++++++++++-----
 drivers/scsi/qla2xxx/qla_os.c      | 16 ++++++++---
 drivers/scsi/qla2xxx/qla_target.c  | 54 +++++++++++++++++++++++++-------------
 drivers/scsi/qla2xxx/qla_target.h  | 22 +++++++++++++++-
 drivers/scsi/qla2xxx/qla_tmpl.c    | 24 +++++++++++++++++
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |  4 +--
 drivers/scsi/qla2xxx/tcm_qla2xxx.h |  1 -
 10 files changed, 123 insertions(+), 36 deletions(-)

-- 
1.8.3.1


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

* [PATCH v3 01/10] qla2xxx: Fix wrong IOCB type assumption.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 02/10] qla2xxx: Include ATIO queue in firmware dump when in target mode Himanshu Madhani
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

From: Quinn Tran <quinn.tran@cavium.com>

qlt_reset is called with Immedidate Notify IOCB only.
Current code wrongly cast it as ATIO IOCB.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index bff9689..b9c559c 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -668,11 +668,9 @@ static int qlt_reset(struct scsi_qla_host *vha, void *iocb, int mcmd)
 {
 	struct qla_hw_data *ha = vha->hw;
 	struct qla_tgt_sess *sess = NULL;
-	uint32_t unpacked_lun, lun = 0;
 	uint16_t loop_id;
 	int res = 0;
 	struct imm_ntfy_from_isp *n = (struct imm_ntfy_from_isp *)iocb;
-	struct atio_from_isp *a = (struct atio_from_isp *)iocb;
 	unsigned long flags;
 
 	loop_id = le16_to_cpu(n->u.isp24.nport_handle);
@@ -725,11 +723,7 @@ static int qlt_reset(struct scsi_qla_host *vha, void *iocb, int mcmd)
 	    "loop_id %d)\n", vha->host_no, sess, sess->port_name,
 	    mcmd, loop_id);
 
-	lun = a->u.isp24.fcp_cmnd.lun;
-	unpacked_lun = scsilun_to_int((struct scsi_lun *)&lun);
-
-	return qlt_issue_task_mgmt(sess, unpacked_lun, mcmd,
-	    iocb, QLA24XX_MGMT_SEND_NACK);
+	return qlt_issue_task_mgmt(sess, 0, mcmd, iocb, QLA24XX_MGMT_SEND_NACK);
 }
 
 /* ha->tgt.sess_lock supposed to be held on entry */
-- 
1.8.3.1

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

* [PATCH v3 02/10] qla2xxx: Include ATIO queue in firmware dump when in target mode
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 01/10] qla2xxx: Fix wrong IOCB type assumption Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 03/10] qla2xxx: Set tcm_qla2xxx version to automatically track qla2xxx version Himanshu Madhani
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

Include ATIO queue for ISP27XX when firmware dump is collected
for target mode.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
Signed-off-by: Giridhar Malavali <giridhar.malavali@cavium.com>
---
 drivers/scsi/qla2xxx/qla_tmpl.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c b/drivers/scsi/qla2xxx/qla_tmpl.c
index 36935c9..8a58ef3 100644
--- a/drivers/scsi/qla2xxx/qla_tmpl.c
+++ b/drivers/scsi/qla2xxx/qla_tmpl.c
@@ -433,6 +433,18 @@ static inline void (*qla27xx_read_vector(uint width))(void __iomem*, void *, ulo
 				count++;
 			}
 		}
+	} else if (QLA_TGT_MODE_ENABLED() &&
+	    ent->t263.queue_type == T263_QUEUE_TYPE_ATIO) {
+		struct qla_hw_data *ha = vha->hw;
+		struct atio *atr = ha->tgt.atio_ring;
+
+		if (atr || !buf) {
+			length = ha->tgt.atio_q_length;
+			qla27xx_insert16(0, buf, len);
+			qla27xx_insert16(length, buf, len);
+			qla27xx_insertbuf(atr, length * sizeof(*atr), buf, len);
+			count++;
+		}
 	} else {
 		ql_dbg(ql_dbg_misc, vha, 0xd026,
 		    "%s: unknown queue %x\n", __func__, ent->t263.queue_type);
@@ -676,6 +688,18 @@ static inline void (*qla27xx_read_vector(uint width))(void __iomem*, void *, ulo
 				count++;
 			}
 		}
+	} else if (QLA_TGT_MODE_ENABLED() &&
+	    ent->t274.queue_type == T274_QUEUE_TYPE_ATIO_SHAD) {
+		struct qla_hw_data *ha = vha->hw;
+		struct atio *atr = ha->tgt.atio_ring_ptr;
+
+		if (atr || !buf) {
+			qla27xx_insert16(0, buf, len);
+			qla27xx_insert16(1, buf, len);
+			qla27xx_insert32(ha->tgt.atio_q_in ?
+			    readl(ha->tgt.atio_q_in) : 0, buf, len);
+			count++;
+		}
 	} else {
 		ql_dbg(ql_dbg_misc, vha, 0xd02f,
 		    "%s: unknown queue %x\n", __func__, ent->t274.queue_type);
-- 
1.8.3.1

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

* [PATCH v3 03/10] qla2xxx: Set tcm_qla2xxx version to automatically track qla2xxx version.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 01/10] qla2xxx: Fix wrong IOCB type assumption Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 02/10] qla2xxx: Include ATIO queue in firmware dump when in target mode Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 04/10] qla2xxx: Reset reserved field in firmware options to 0 Himanshu Madhani
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
Signed-off-by: Giridhar Malavali <giridhar.malavali@cavium.com>
---
 drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 ++--
 drivers/scsi/qla2xxx/tcm_qla2xxx.h | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 6643f6f..d925910 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1800,7 +1800,7 @@ static ssize_t tcm_qla2xxx_wwn_version_show(struct config_item *item,
 {
 	return sprintf(page,
 	    "TCM QLOGIC QLA2XXX NPIV capable fabric module %s on %s/%s on "
-	    UTS_RELEASE"\n", TCM_QLA2XXX_VERSION, utsname()->sysname,
+	    UTS_RELEASE"\n", QLA2XXX_VERSION, utsname()->sysname,
 	    utsname()->machine);
 }
 
@@ -1906,7 +1906,7 @@ static int tcm_qla2xxx_register_configfs(void)
 	int ret;
 
 	pr_debug("TCM QLOGIC QLA2XXX fabric module %s on %s/%s on "
-	    UTS_RELEASE"\n", TCM_QLA2XXX_VERSION, utsname()->sysname,
+	    UTS_RELEASE"\n", QLA2XXX_VERSION, utsname()->sysname,
 	    utsname()->machine);
 
 	ret = target_register_template(&tcm_qla2xxx_ops);
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.h b/drivers/scsi/qla2xxx/tcm_qla2xxx.h
index 37e026a..cf8430b 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.h
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.h
@@ -1,7 +1,6 @@
 #include <target/target_core_base.h>
 #include <linux/btree.h>
 
-#define TCM_QLA2XXX_VERSION	"v0.1"
 /* length of ASCII WWPNs including pad */
 #define TCM_QLA2XXX_NAMELEN	32
 /*
-- 
1.8.3.1


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

* [PATCH v3 04/10] qla2xxx: Reset reserved field in firmware options to 0.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (2 preceding siblings ...)
  2016-12-24  2:06 ` [PATCH v3 03/10] qla2xxx: Set tcm_qla2xxx version to automatically track qla2xxx version Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 05/10] qla2xxx: Collect additional information to debug fw dump Himanshu Madhani
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

During NVRAM initialization in target mode, reset reserved
fields in firmware options to Zero (BIT 15)

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
Signed-off-by: Giridhar Malavali <giridhar.malavali@cavium.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index b9c559c..369d62f2 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -6539,6 +6539,13 @@ static void qlt_disable_vha(struct scsi_qla_host *vha)
 
 		/* Disable Full Login after LIP */
 		nv->host_p &= cpu_to_le32(~BIT_10);
+
+		/*
+		 * clear BIT 15 explicitly as we have seen at least
+		 * a couple of instances where this was set and this
+		 * was causing the firmware to not be initialized.
+		 */
+		nv->firmware_options_1 &= cpu_to_le32(~BIT_15);
 		/* Enable target PRLI control */
 		nv->firmware_options_2 |= cpu_to_le32(BIT_14);
 	} else {
@@ -6623,11 +6630,17 @@ static void qlt_disable_vha(struct scsi_qla_host *vha)
 		/* Disable ini mode, if requested */
 		if (!qla_ini_mode_enabled(vha))
 			nv->firmware_options_1 |= cpu_to_le32(BIT_5);
-
 		/* Disable Full Login after LIP */
 		nv->firmware_options_1 &= cpu_to_le32(~BIT_13);
 		/* Enable initial LIP */
 		nv->firmware_options_1 &= cpu_to_le32(~BIT_9);
+		/*
+		 * clear BIT 15 explicitly as we have seen at
+		 * least a couple of instances where this was set
+		 * and this was causing the firmware to not be
+		 * initialized.
+		 */
+		nv->firmware_options_1 &= cpu_to_le32(~BIT_15);
 		if (ql2xtgt_tape_enable)
 			/* Enable FC tape support */
 			nv->firmware_options_2 |= cpu_to_le32(BIT_12);
-- 
1.8.3.1

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

* [PATCH v3 05/10] qla2xxx: Collect additional information to debug fw dump.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (3 preceding siblings ...)
  2016-12-24  2:06 ` [PATCH v3 04/10] qla2xxx: Reset reserved field in firmware options to 0 Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 06/10] qla2xxx: Fix crash due to null pointer access Himanshu Madhani
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

From: Quinn Tran <quinn.tran@cavium.com>

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
---
 drivers/scsi/qla2xxx/qla_mbx.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 2819ceb..b4386fc 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -323,20 +323,33 @@ static int is_rom_cmd(uint16_t cmd)
 		}
 	} else {
 
-		uint16_t mb0;
-		uint32_t ictrl;
+		uint16_t mb[8];
+		uint32_t ictrl, host_status, hccr;
 		uint16_t        w;
 
 		if (IS_FWI2_CAPABLE(ha)) {
-			mb0 = RD_REG_WORD(&reg->isp24.mailbox0);
+			mb[0] = RD_REG_WORD(&reg->isp24.mailbox0);
+			mb[1] = RD_REG_WORD(&reg->isp24.mailbox1);
+			mb[2] = RD_REG_WORD(&reg->isp24.mailbox2);
+			mb[3] = RD_REG_WORD(&reg->isp24.mailbox3);
+			mb[7] = RD_REG_WORD(&reg->isp24.mailbox7);
 			ictrl = RD_REG_DWORD(&reg->isp24.ictrl);
+			host_status = RD_REG_DWORD(&reg->isp24.host_status);
+			hccr = RD_REG_DWORD(&reg->isp24.hccr);
+
+			ql_log(ql_log_warn, vha, 0x1119,
+			    "MBX Command timeout for cmd %x, iocontrol=%x jiffies=%lx "
+			    "mb[0-3]=[0x%x 0x%x 0x%x 0x%x] mb7 0x%x host_status 0x%x hccr 0x%x\n",
+			    command, ictrl, jiffies, mb[0], mb[1], mb[2], mb[3],
+			    mb[7], host_status, hccr);
+
 		} else {
-			mb0 = RD_MAILBOX_REG(ha, &reg->isp, 0);
+			mb[0] = RD_MAILBOX_REG(ha, &reg->isp, 0);
 			ictrl = RD_REG_WORD(&reg->isp.ictrl);
+			ql_dbg(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1119,
+			    "MBX Command timeout for cmd %x, iocontrol=%x jiffies=%lx "
+			    "mb[0]=0x%x\n", command, ictrl, jiffies, mb[0]);
 		}
-		ql_dbg(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1119,
-		    "MBX Command timeout for cmd %x, iocontrol=%x jiffies=%lx "
-		    "mb[0]=0x%x\n", command, ictrl, jiffies, mb0);
 		ql_dump_regs(ql_dbg_mbx + ql_dbg_buffer, vha, 0x1019);
 
 		/* Capture FW dump only, if PCI device active */
-- 
1.8.3.1

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

* [PATCH v3 06/10] qla2xxx: Fix crash due to null pointer access.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (4 preceding siblings ...)
  2016-12-24  2:06 ` [PATCH v3 05/10] qla2xxx: Collect additional information to debug fw dump Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 07/10] qla2xxx: Terminate exchange if corruputed Himanshu Madhani
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

From: Quinn Tran <quinn.tran@cavium.com>

During code inspection, while investigating following stack trace
seen on one of the test setup, we found out there was possiblity
of memory leak becuase driver was not unwinding the stack properly.

This issue has not been reproduced in a test environment or on a
customer setup.

Here's stack trace that was seen.

[1469877.797315] Call Trace:
[1469877.799940]  [<ffffffffa03ab6e9>] qla2x00_mem_alloc+0xb09/0x10c0 [qla2xxx]
[1469877.806980]  [<ffffffffa03ac50a>] qla2x00_probe_one+0x86a/0x1b50 [qla2xxx]
[1469877.814013]  [<ffffffff813b6d01>] ? __pm_runtime_resume+0x51/0xa0
[1469877.820265]  [<ffffffff8157c1f5>] ? _raw_spin_lock_irqsave+0x25/0x90
[1469877.826776]  [<ffffffff8157cd2d>] ? _raw_spin_unlock_irqrestore+0x6d/0x80
[1469877.833720]  [<ffffffff810741d1>] ? preempt_count_sub+0xb1/0x100
[1469877.839885]  [<ffffffff8157cd0c>] ? _raw_spin_unlock_irqrestore+0x4c/0x80
[1469877.846830]  [<ffffffff81319b9c>] local_pci_probe+0x4c/0xb0
[1469877.852562]  [<ffffffff810741d1>] ? preempt_count_sub+0xb1/0x100
[1469877.858727]  [<ffffffff81319c89>] pci_call_probe+0x89/0xb0

Cc: <stable@vger.kernel.org>
Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/qla2xxx/qla_os.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 8521cfe..074dcca 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3662,7 +3662,7 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, fc_port_t *fcport,
 				sizeof(struct ct6_dsd), 0,
 				SLAB_HWCACHE_ALIGN, NULL);
 			if (!ctx_cachep)
-				goto fail_free_gid_list;
+				goto fail_free_srb_mempool;
 		}
 		ha->ctx_mempool = mempool_create_slab_pool(SRB_MIN_REQ,
 			ctx_cachep);
@@ -3815,7 +3815,7 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, fc_port_t *fcport,
 	ha->loop_id_map = kzalloc(BITS_TO_LONGS(LOOPID_MAP_SIZE) * sizeof(long),
 	    GFP_KERNEL);
 	if (!ha->loop_id_map)
-		goto fail_async_pd;
+		goto fail_loop_id_map;
 	else {
 		qla2x00_set_reserved_loop_ids(ha);
 		ql_dbg_pci(ql_dbg_init, ha->pdev, 0x0123,
@@ -3824,6 +3824,8 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, fc_port_t *fcport,
 
 	return 0;
 
+fail_loop_id_map:
+	dma_pool_free(ha->s_dma_pool, ha->async_pd, ha->async_pd_dma);
 fail_async_pd:
 	dma_pool_free(ha->s_dma_pool, ha->ex_init_cb, ha->ex_init_cb_dma);
 fail_ex_init_cb:
@@ -3851,6 +3853,10 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, fc_port_t *fcport,
 	dma_pool_free(ha->s_dma_pool, ha->ms_iocb, ha->ms_iocb_dma);
 	ha->ms_iocb = NULL;
 	ha->ms_iocb_dma = 0;
+
+	if (ha->sns_cmd)
+		dma_free_coherent(&ha->pdev->dev, sizeof(struct sns_cmd_pkt),
+		    ha->sns_cmd, ha->sns_cmd_dma);
 fail_dma_pool:
 	if (IS_QLA82XX(ha) || ql2xenabledif) {
 		dma_pool_destroy(ha->fcp_cmnd_dma_pool);
@@ -3868,10 +3874,12 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *vha, fc_port_t *fcport,
 	kfree(ha->nvram);
 	ha->nvram = NULL;
 fail_free_ctx_mempool:
-	mempool_destroy(ha->ctx_mempool);
+	if (ha->ctx_mempool)
+		mempool_destroy(ha->ctx_mempool);
 	ha->ctx_mempool = NULL;
 fail_free_srb_mempool:
-	mempool_destroy(ha->srb_mempool);
+	if (ha->srb_mempool)
+		mempool_destroy(ha->srb_mempool);
 	ha->srb_mempool = NULL;
 fail_free_gid_list:
 	dma_free_coherent(&ha->pdev->dev, qla2x00_gid_list_size(ha),
-- 
1.8.3.1


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

* [PATCH v3 07/10] qla2xxx: Terminate exchange if corruputed.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (5 preceding siblings ...)
  2016-12-24  2:06 ` [PATCH v3 06/10] qla2xxx: Fix crash due to null pointer access Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 08/10] qla2xxx: Reduce exess wait during chip reset Himanshu Madhani
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

From: Quinn Tran <quinn.tran@cavium.com>

Corrupted ATIO is defined as length of fcp_header & fcp_cmd
payload is less than 0x38. It's the minimum size for a frame to
carry 8..16 bytes SCSI CDB. The exchange will be dropped or
terminated if corrupted.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
---
 drivers/scsi/qla2xxx/qla_def.h    |  3 ++-
 drivers/scsi/qla2xxx/qla_target.c | 23 ++++++++++++++++++++---
 drivers/scsi/qla2xxx/qla_target.h | 22 +++++++++++++++++++++-
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index f7df01b..1f7c6d2 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -1556,7 +1556,8 @@ struct link_statistics {
 struct atio {
 	uint8_t		entry_type;		/* Entry type. */
 	uint8_t		entry_count;		/* Entry count. */
-	uint8_t		data[58];
+	__le16		attr_n_length;
+	uint8_t		data[56];
 	uint32_t	signature;
 #define ATIO_PROCESSED 0xDEADDEAD		/* Signature */
 };
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 369d62f2..2c04f42 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -6451,12 +6451,29 @@ static void qlt_disable_vha(struct scsi_qla_host *vha)
 	if (!vha->flags.online)
 		return;
 
-	while (ha->tgt.atio_ring_ptr->signature != ATIO_PROCESSED) {
+	while ((ha->tgt.atio_ring_ptr->signature != ATIO_PROCESSED) ||
+	    fcpcmd_is_corrupted(ha->tgt.atio_ring_ptr)) {
 		pkt = (struct atio_from_isp *)ha->tgt.atio_ring_ptr;
 		cnt = pkt->u.raw.entry_count;
 
-		qlt_24xx_atio_pkt_all_vps(vha, (struct atio_from_isp *)pkt,
-		    ha_locked);
+		if (unlikely(fcpcmd_is_corrupted(ha->tgt.atio_ring_ptr))) {
+			/*
+			 * This packet is corrupted. The header + payload
+			 * can not be trusted. There is no point in passing
+			 * it further up.
+			 */
+			ql_log(ql_log_warn, vha, 0xffff,
+			    "corrupted fcp frame SID[%3phN] OXID[%04x] EXCG[%x] %64phN\n",
+			    pkt->u.isp24.fcp_hdr.s_id,
+			    be16_to_cpu(pkt->u.isp24.fcp_hdr.ox_id),
+			    le32_to_cpu(pkt->u.isp24.exchange_addr), pkt);
+
+			adjust_corrupted_atio(pkt);
+			qlt_send_term_exchange(vha, NULL, pkt, ha_locked, 0);
+		} else {
+			qlt_24xx_atio_pkt_all_vps(vha,
+			    (struct atio_from_isp *)pkt, ha_locked);
+		}
 
 		for (i = 0; i < cnt; i++) {
 			ha->tgt.atio_ring_index++;
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index f26c5f6..0824a81 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -427,13 +427,33 @@ struct atio_from_isp {
 		struct {
 			uint8_t  entry_type;	/* Entry type. */
 			uint8_t  entry_count;	/* Entry count. */
-			uint8_t  data[58];
+			__le16	 attr_n_length;
+#define FCP_CMD_LENGTH_MASK 0x0fff
+#define FCP_CMD_LENGTH_MIN  0x38
+			uint8_t  data[56];
 			uint32_t signature;
 #define ATIO_PROCESSED 0xDEADDEAD		/* Signature */
 		} raw;
 	} u;
 } __packed;
 
+static inline int fcpcmd_is_corrupted(struct atio *atio)
+{
+	if (atio->entry_type == ATIO_TYPE7 &&
+	    (le16_to_cpu(atio->attr_n_length & FCP_CMD_LENGTH_MASK) <
+	    FCP_CMD_LENGTH_MIN))
+		return 1;
+	else
+		return 0;
+}
+
+/* adjust corrupted atio so we won't trip over the same entry again. */
+static inline void adjust_corrupted_atio(struct atio_from_isp *atio)
+{
+	atio->u.raw.attr_n_length = cpu_to_le16(FCP_CMD_LENGTH_MIN);
+	atio->u.isp24.fcp_cmnd.add_cdb_len = 0;
+}
+
 #define CTIO_TYPE7 0x12 /* Continue target I/O entry (for 24xx) */
 
 /*
-- 
1.8.3.1


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

* [PATCH v3 08/10] qla2xxx: Reduce exess wait during chip reset
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (6 preceding siblings ...)
  2016-12-24  2:06 ` [PATCH v3 07/10] qla2xxx: Terminate exchange if corruputed Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 09/10] qla2xxx: Fix invalid handle erroneous message Himanshu Madhani
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

From: Quinn Tran <quinn.tran@cavium.com>

Soft reset and Risc reset should take 100uS to complete.
This change pad the timeout up to 400uS, which should be
plenty.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 632d5f3..7b6317c 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -1191,7 +1191,7 @@ static int qla2x00_fabric_dev_login(scsi_qla_host_t *, fc_port_t *,
 
 	/* Wait for soft-reset to complete. */
 	RD_REG_DWORD(&reg->ctrl_status);
-	for (cnt = 0; cnt < 6000000; cnt++) {
+	for (cnt = 0; cnt < 60; cnt++) {
 		barrier();
 		if ((RD_REG_DWORD(&reg->ctrl_status) &
 		    CSRX_ISP_SOFT_RESET) == 0)
@@ -1234,7 +1234,7 @@ static int qla2x00_fabric_dev_login(scsi_qla_host_t *, fc_port_t *,
 	RD_REG_DWORD(&reg->hccr);
 
 	RD_REG_WORD(&reg->mailbox0);
-	for (cnt = 6000000; RD_REG_WORD(&reg->mailbox0) != 0 &&
+	for (cnt = 60; RD_REG_WORD(&reg->mailbox0) != 0 &&
 	    rval == QLA_SUCCESS; cnt--) {
 		barrier();
 		if (cnt)
-- 
1.8.3.1

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

* [PATCH v3 09/10] qla2xxx: Fix invalid handle erroneous message.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (7 preceding siblings ...)
  2016-12-24  2:06 ` [PATCH v3 08/10] qla2xxx: Reduce exess wait during chip reset Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2016-12-24  2:06 ` [PATCH v3 10/10] qla2xxx: Disable Out-of-order processing by default in Firmware Himanshu Madhani
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

From: Quinn Tran <quinn.tran@cavium.com>

Termination of Immediate Notify IOCB was using wrong
IOCB handle. IOCB completion code was unable to find
appropriate code path due to wrong handle.

Following message is seen in the logs.

"Error entry - invalid handle/queue (ffff)."

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
---
 drivers/scsi/qla2xxx/qla_isr.c    | 4 ++++
 drivers/scsi/qla2xxx/qla_target.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index af840bf..eefcf2f 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2495,6 +2495,10 @@ struct scsi_dif_tuple {
 	if (pkt->entry_status & RF_BUSY)
 		res = DID_BUS_BUSY << 16;
 
+	if (pkt->entry_type == NOTIFY_ACK_TYPE &&
+	    pkt->handle == QLA_TGT_SKIP_HANDLE)
+		return;
+
 	sp = qla2x00_get_sp_from_handle(vha, func, req, pkt);
 	if (sp) {
 		sp->done(ha, sp, res);
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 2c04f42..5e4248f 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3061,7 +3061,7 @@ static int __qlt_send_term_imm_notif(struct scsi_qla_host *vha,
 
 	pkt->entry_type = NOTIFY_ACK_TYPE;
 	pkt->entry_count = 1;
-	pkt->handle = QLA_TGT_SKIP_HANDLE | CTIO_COMPLETION_HANDLE_MARK;
+	pkt->handle = QLA_TGT_SKIP_HANDLE;
 
 	nack = (struct nack_to_isp *)pkt;
 	nack->ox_id = ntfy->ox_id;
-- 
1.8.3.1


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

* [PATCH v3 10/10] qla2xxx: Disable Out-of-order processing by default in Firmware
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (8 preceding siblings ...)
  2016-12-24  2:06 ` [PATCH v3 09/10] qla2xxx: Fix invalid handle erroneous message Himanshu Madhani
@ 2016-12-24  2:06 ` Himanshu Madhani
  2017-01-17 18:06 ` [PATCH v3 00/10] qla2xxx: Bug fixes for driver Bart Van Assche
  2017-01-17 18:25 ` Bart Van Assche
  11 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2016-12-24  2:06 UTC (permalink / raw)
  To: target-devel, bart.vanassche, hch, nab
  Cc: giridhar.malavali, linux-scsi, himanshu.madhani

From: Quinn Tran <quinn.tran@cavium.com>

Out of order(OOO) processing requires initiator, switch
and target to support OOO. In today¹s environment, none
of the switches support OOO. OOO requires extra buffer
space which affect performance. By turning ON this feature
in QLogic's FW, it delays error recovery because droped
frame is treated as out of order frame. We¹re turning OFF
this option of speed up error recovery.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
---
 drivers/scsi/qla2xxx/qla_target.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 5e4248f..9c6cb75 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -6578,9 +6578,6 @@ static void qlt_disable_vha(struct scsi_qla_host *vha)
 		return;
 	}
 
-	/* out-of-order frames reassembly */
-	nv->firmware_options_3 |= BIT_6|BIT_9;
-
 	if (ha->tgt.enable_class_2) {
 		if (vha->flags.init_done)
 			fc_host_supported_classes(vha->host) =
@@ -6682,9 +6679,6 @@ static void qlt_disable_vha(struct scsi_qla_host *vha)
 		return;
 	}
 
-	/* out-of-order frames reassembly */
-	nv->firmware_options_3 |= BIT_6|BIT_9;
-
 	if (ha->tgt.enable_class_2) {
 		if (vha->flags.init_done)
 			fc_host_supported_classes(vha->host) =
-- 
1.8.3.1

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

* Re: [PATCH v3 00/10] qla2xxx: Bug fixes for driver.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (9 preceding siblings ...)
  2016-12-24  2:06 ` [PATCH v3 10/10] qla2xxx: Disable Out-of-order processing by default in Firmware Himanshu Madhani
@ 2017-01-17 18:06 ` Bart Van Assche
  2017-01-17 18:25 ` Bart Van Assche
  11 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2017-01-17 18:06 UTC (permalink / raw)
  To: Himanshu Madhani, target-devel, hch, nab; +Cc: giridhar.malavali, linux-scsi

On 12/23/2016 06:06 PM, Himanshu Madhani wrote:
> Hi Bart,
> 
> Here's updated series of bug fixes for target code in the driver.
> Please consider this for target-pending.

Hello Himanshu,

Thanks for the patches. I have queued these patches for v4.10-rc5. But
since the qla2xxx driver code triggers several static checker warnings,
I would appreciate if you could have a look at the four patches I just
posted to address these warnings.

Bart.

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

* Re: [PATCH v3 00/10] qla2xxx: Bug fixes for driver.
  2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
                   ` (10 preceding siblings ...)
  2017-01-17 18:06 ` [PATCH v3 00/10] qla2xxx: Bug fixes for driver Bart Van Assche
@ 2017-01-17 18:25 ` Bart Van Assche
  11 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2017-01-17 18:25 UTC (permalink / raw)
  To: hch@infradead.org, himanshu.madhani@cavium.com,
	target-devel@vger.kernel.org, nab@linux-iscsi.org
  Cc: linux-scsi@vger.kernel.org, giridhar.malavali@cavium.com

On Fri, 2016-12-23 at 18:06 -0800, Himanshu Madhani wrote:
> Here's updated series of bug fixes for target code in the driver.
> Please consider this for target-pending.

Hello Himanshu,

Thanks for the patches. I have queued these patches for v4.10-rc5. But since
the qla2xxx driver code triggers several static checker warnings, I would
appreciate if you could have a look at the four patches I just posted to
address these warnings.

Bart.

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

end of thread, other threads:[~2017-01-17 18:44 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-24  2:06 [PATCH v3 00/10] qla2xxx: Bug fixes for driver Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 01/10] qla2xxx: Fix wrong IOCB type assumption Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 02/10] qla2xxx: Include ATIO queue in firmware dump when in target mode Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 03/10] qla2xxx: Set tcm_qla2xxx version to automatically track qla2xxx version Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 04/10] qla2xxx: Reset reserved field in firmware options to 0 Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 05/10] qla2xxx: Collect additional information to debug fw dump Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 06/10] qla2xxx: Fix crash due to null pointer access Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 07/10] qla2xxx: Terminate exchange if corruputed Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 08/10] qla2xxx: Reduce exess wait during chip reset Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 09/10] qla2xxx: Fix invalid handle erroneous message Himanshu Madhani
2016-12-24  2:06 ` [PATCH v3 10/10] qla2xxx: Disable Out-of-order processing by default in Firmware Himanshu Madhani
2017-01-17 18:06 ` [PATCH v3 00/10] qla2xxx: Bug fixes for driver Bart Van Assche
2017-01-17 18:25 ` Bart Van Assche

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.