Skip to contentSkip to conversation

qla2xxx patches for kernel v5.4

PATCH Applied Bart Van Assche 4 participants 63 messages Aug 9, 2019 – Aug 13, 2019

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v2 cover qla2xxx patches for kernel v5.4

Hi Martin,

The patches in this series improve the robustness of the QLogic Fibre Channel initiator and target drivers. These patches are a result of manual code inspection, analysis of Coverity reports and stress testing of these two drivers. Please consider these patches for kernel version v5.4.

Thanks,

Bart.

Changes compared to v1:
- Included a regression fix for qla2xxx_eh_abort() in the second patch of this series (the fix Himanshu mentioned in his e-mail).
- Moved a WARN_ON_ONCE() statement from a later patch to the second patch in this series.
- Dropped one patch that renames a function.

Bart Van Assche (58):
qla2xxx: Make qla2x00_abort_srb() again decrease the sp reference
count
qla2xxx: Really fix qla2xxx_eh_abort()
qla2xxx: Improve Linux kernel coding style conformance
qla2xxx: Use tabs instead of spaces for indentation
qla2xxx: Include the <asm/unaligned.h> header file from qla_dsd.h
qla2xxx: Remove an include directive from qla_mr.c
qla2xxx: Remove a superfluous forward declaration
qla2xxx: Declare the fourth ql_dump_buffer() argument const
qla2xxx: Change the return type of qla2x00_update_ms_fdmi_iocb() into
void
qla2xxx: Reduce the scope of three local variables in
qla2xxx_queuecommand()
qla2xxx: Declare qla_tgt_cmd.cdb const
qla2xxx: Change data_dsd into an array
qla2xxx: Verify locking assumptions at runtime
qla2xxx: Reduce the number of casts in GID list code
qla2xxx: Simplify qlt_lport_dump()
qla2xxx: Remove a superfluous pointer check
qla2xxx: Remove two superfluous tests
qla2xxx: Simplify qla24xx_abort_sp_done()
qla2xxx: Fix session lookup in qlt_abort_work()
qla2xxx: Report the firmware status code if a mailbox command fails
qla2xxx: Do not corrupt vha->plogi_ack_list
qla2xxx: Use strlcpy() instead of strncpy()
qla2xxx: Complain if a mailbox command times out
qla2xxx: Complain if parsing the version string fails
qla2xxx: Remove dead code
qla2xxx: Simplify a debug statement
qla2xxx: Fix qla24xx_process_bidir_cmd()
qla2xxx: Remove unreachable code from qla83xx_idc_lock()
qla2xxx: Suppress a Coveritiy complaint about integer overflow
qla2xxx: Suppress multiple Coverity complaint about out-of-bounds
accesses
qla2xxx: Always check the qla2x00_wait_for_hba_online() return value
qla2xxx: Declare fourth qla2x00_set_model_info() argument const
qla2xxx: Complain if waiting for pending commands times out
qla2xxx: Check the PCI info string output buffer size
qla2xxx: Use memcpy() and strlcpy() instead of strcpy() and strncpy()
qla2xxx: Complain if a soft reset fails
qla2xxx: Introduce the be_id_t and le_id_t data types for FC src/dst
IDs
qla2xxx: Change the return type of qla24xx_read_flash_data()
qla2xxx: Check secondary image if reading the primary image fails
qla2xxx: Make it explicit that ELS pass-through IOCBs use little
endian
qla2xxx: Set the responder mode if appropriate for ELS pass-through
IOCBs
qla2xxx: Rework key encoding in qlt_find_host_by_d_id()
qla2xxx: Enable type checking for the SRB free and done callback
functions
qla2xxx: Introduce the function qla2xxx_init_sp()
qla2xxx: Fix a race condition between aborting and completing a SCSI
command
qla2xxx: Make qlt_handle_abts_completion() more robust
qla2xxx: Modify NVMe include directives
qla2xxx: Introduce qla2xxx_get_next_handle()
qla2xxx: Make sure that aborted commands are freed
qla2xxx: Complain if sp->done() is not called from the completion path
qla2xxx: Let the compiler check the type of the SCSI command context
pointer
qla2xxx: Remove superfluous sts_entry_* casts
qla2xxx: Report invalid mailbox status codes
qla2xxx: Inline the qla2x00_fcport_event_handler() function
qla2xxx: Introduce qla2x00_els_dcmd2_free()
qla2xxx: Remove two superfluous if-tests
qla2xxx: Simplify qla24xx_async_abort_cmd()
qla2xxx: Fix a NULL pointer dereference

 drivers/scsi/qla2xxx/qla_attr.c    |   6 +-
 drivers/scsi/qla2xxx/qla_bsg.c     |  19 +--
 drivers/scsi/qla2xxx/qla_dbg.c     |   3 +-
 drivers/scsi/qla2xxx/qla_def.h     | 130 ++++++++++----
 drivers/scsi/qla2xxx/qla_dfs.c     |   9 +-
 drivers/scsi/qla2xxx/qla_dsd.h     |   2 +
 drivers/scsi/qla2xxx/qla_fw.h      |   8 +-
 drivers/scsi/qla2xxx/qla_gbl.h     |  33 ++--
 drivers/scsi/qla2xxx/qla_gs.c      | 219 +++++++++---------------
 drivers/scsi/qla2xxx/qla_init.c    | 261 +++++++++++------------------
 drivers/scsi/qla2xxx/qla_inline.h  |  28 ++--
 drivers/scsi/qla2xxx/qla_iocb.c    | 221 +++++++++---------------
 drivers/scsi/qla2xxx/qla_isr.c     |  24 ++-
 drivers/scsi/qla2xxx/qla_mbx.c     |  10 +-
 drivers/scsi/qla2xxx/qla_mid.c     |   4 +-
 drivers/scsi/qla2xxx/qla_mr.c      |  67 ++++----
 drivers/scsi/qla2xxx/qla_nvme.c    |  28 +---
 drivers/scsi/qla2xxx/qla_nvme.h    |   5 +-
 drivers/scsi/qla2xxx/qla_nx.c      |  16 +-
 drivers/scsi/qla2xxx/qla_nx.h      |  14 +-
 drivers/scsi/qla2xxx/qla_nx2.c     |   2 +-
 drivers/scsi/qla2xxx/qla_os.c      | 213 ++++++++++-------------
 drivers/scsi/qla2xxx/qla_sup.c     |   8 +-
 drivers/scsi/qla2xxx/qla_target.c  | 209 +++++++++--------------
 drivers/scsi/qla2xxx/qla_target.h  |  35 ++--
 drivers/scsi/qla2xxx/qla_tmpl.c    |   7 +-
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |  27 ++-
 include/linux/nvme-fc-driver.h     |   2 +
 28 files changed, 677 insertions(+), 933 deletions(-)
Signature · 1 line
2.22.0
#1 lore raw

PATCH v2 1/58 qla2xxx: Make qla2x00_abort_srb() again decrease the sp reference count

Since qla2x00_abort_srb() starts with increasing the reference count of @sp, decrease that same reference count before returning.

Cc Himanshu Madhani <[email protected]>
Fixes 219d27d7147e ("scsi: qla2xxx: Fix race conditions in the code for aborting SCSI commands") # v5.2.
---
 drivers/scsi/qla2xxx/qla_os.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/scsi/qla2xxx/qla_os.c +2 -0
index 17a3f91ba5a3..b667f13b62df 100644
@@ -1755,6 +1755,8 @@ static void qla2x00_abort_srb(struct qla_qpair *qp, srb_t *sp, const int res,
 		spin_lock_irqsave(qp->qp_lock_ptr, *flags);
 		sp->comp = NULL;
 	}
+
+	atomic_dec(&sp->ref_count);
 }
 
 static void
Signature · 1 line
2.22.0
#2 lore raw

PATCH v2 2/58 qla2xxx: Really fix qla2xxx_eh_abort()

In reply to Bart Van Assche

I'm not sure how this happened but the patch that was intended to fix abort handling was incomplete. This patch fixes that patch as follows:
- If aborting the SCSI command failed, wait until the SCSI command completes.
- Return SUCCESS instead of FAILED if an abort attempt races with SCSI command completion.
- Since qla2xxx_eh_abort() increments the sp reference count by calling sp_get(), decrement the sp reference count before returning.

Cc Himanshu Madhani <[email protected]>
Fixes 219d27d7147e ("scsi: qla2xxx: Fix race conditions in the code for aborting SCSI commands")
---
 drivers/scsi/qla2xxx/qla_os.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
drivers/scsi/qla2xxx/qla_os.c +21 -0
index b667f13b62df..db1f1aac79f2 100644
@@ -1269,6 +1269,7 @@ static int
 qla2xxx_eh_abort(struct scsi_cmnd *cmd)
 {
 	scsi_qla_host_t *vha = shost_priv(cmd->device->host);
+	DECLARE_COMPLETION_ONSTACK(comp);
 	srb_t *sp;
 	int ret;
 	unsigned int id;
@@ -1307,6 +1308,7 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
 		return SUCCESS;
 	}
 
+	/* Get a reference to the sp and drop the lock. */
 	if (sp_get(sp)){
 		/* ref_count is already 0 */
 		spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
@@ -1334,6 +1336,23 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
 		sp->done(sp, DID_ABORT << 16);
 		ret = SUCCESS;
 		break;
+	case QLA_FUNCTION_PARAMETER_ERROR: {
+		/* Wait for the command completion. */
+		uint32_t ratov = ha->r_a_tov/10;
+		uint32_t ratov_j = msecs_to_jiffies(4 * ratov * 1000);
+
+		WARN_ON_ONCE(sp->comp);
+		sp->comp = &comp;
+		if (!wait_for_completion_timeout(&comp, ratov_j)) {
+			ql_dbg(ql_dbg_taskm, vha, 0xffff,
+			    "%s: Abort wait timer (4 * R_A_TOV[%d]) expired\n",
+			    __func__, ha->r_a_tov);
+			ret = FAILED;
+		} else {
+			ret = SUCCESS;
+		}
+		break;
+	}
 	default:
 		/*
 		 * Either abort failed or abort and completion raced. Let
@@ -1343,6 +1362,8 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
 		break;
 	}
 
+	sp->comp = NULL;
+	atomic_dec(&sp->ref_count);
 	ql_log(ql_log_info, vha, 0x801c,
 	    "Abort command issued nexus=%ld:%d:%llu -- %x.\n",
 	    vha->host_no, id, lun, ret);
Signature · 1 line
2.22.0
#3 lore raw

PATCH v2 3/58 qla2xxx: Improve Linux kernel coding style conformance

In reply to Bart Van Assche

Insert a space where required, surround complex expressions in macros with parentheses, use the UL suffix instead of the (unsigned long) cast, do not use line continuations when not necessary and do not explicitly initialize static variables to zero.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_def.h  | 10 +++++-----
 drivers/scsi/qla2xxx/qla_init.c |  2 +-
 drivers/scsi/qla2xxx/qla_iocb.c |  1 +
 drivers/scsi/qla2xxx/qla_nvme.c |  1 -
 drivers/scsi/qla2xxx/qla_nx.c   | 10 +++++-----
 drivers/scsi/qla2xxx/qla_nx.h   | 14 +++++++-------
 drivers/scsi/qla2xxx/qla_os.c   |  6 +++---
 7 files changed, 22 insertions(+), 22 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +5 -5
index b91ef7b75e8d..a94b0d440f0f 100644
@@ -117,9 +117,9 @@
 #define RD_REG_BYTE_RELAXED(addr)	readb_relaxed(addr)
 #define RD_REG_WORD_RELAXED(addr)	readw_relaxed(addr)
 #define RD_REG_DWORD_RELAXED(addr)	readl_relaxed(addr)
-#define WRT_REG_BYTE(addr, data)	writeb(data,addr)
-#define WRT_REG_WORD(addr, data)	writew(data,addr)
-#define WRT_REG_DWORD(addr, data)	writel(data,addr)
+#define WRT_REG_BYTE(addr, data)	writeb(data, addr)
+#define WRT_REG_WORD(addr, data)	writew(data, addr)
+#define WRT_REG_DWORD(addr, data)	writel(data, addr)
 
 /*
  * ISP83XX specific remote register addresses
@@ -207,7 +207,7 @@
  * 133Mhz slot.
  */
 #define RD_REG_WORD_PIO(addr)		(inw((unsigned long)addr))
-#define WRT_REG_WORD_PIO(addr, data)	(outw(data,(unsigned long)addr))
+#define WRT_REG_WORD_PIO(addr, data)	(outw(data, (unsigned long)addr))
 
 /*
  * Fibre Channel device definitions.
@@ -3851,7 +3851,7 @@ struct qla_hw_data {
 
 	/* NVRAM configuration data */
 #define MAX_NVRAM_SIZE  4096
-#define VPD_OFFSET      MAX_NVRAM_SIZE / 2
+#define VPD_OFFSET      (MAX_NVRAM_SIZE / 2)
 	uint16_t	nvram_size;
 	uint16_t	nvram_base;
 	void		*nvram;
drivers/scsi/qla2xxx/qla_init.c +1 -1
index d82d1a2b3543..3585eb7b87b5 100644
@@ -5397,7 +5397,7 @@ qla2x00_reg_remote_port(scsi_qla_host_t *vha, fc_port_t *fcport)
 	    "%s %8phN. rport %p is %s mode\n",
 	    __func__, fcport->port_name, rport,
 	    (fcport->port_type == FCT_TARGET) ? "tgt" :
-	    ((fcport->port_type & FCT_NVME) ? "nvme" :"ini"));
+	    ((fcport->port_type & FCT_NVME) ? "nvme" : "ini"));
 
 	fc_remote_port_rolechg(rport, rport_ids.roles);
 }
drivers/scsi/qla2xxx/qla_iocb.c +1 -0
index 1886de92034c..b514ab4d243e 100644
@@ -1364,6 +1364,7 @@ qla24xx_walk_and_build_prot_sglist(struct qla_hw_data *ha, srb_t *sp,
 	cur_dsd++;
 	return 0;
 }
+
 /**
  * qla24xx_build_scsi_crc_2_iocbs() - Build IOCB command utilizing Command
  *							Type 6 IOCB types.
drivers/scsi/qla2xxx/qla_nvme.c +0 -1
index af7919a5acdc..16028ee8c7a7 100644
@@ -267,7 +267,6 @@ static void qla_nvme_ls_abort(struct nvme_fc_local_port *lport,
 	schedule_work(&priv->abort_work);
 }
 
-
 static int qla_nvme_ls_req(struct nvme_fc_local_port *lport,
     struct nvme_fc_remote_port *rport, struct nvmefc_ls_req *fd)
 {
drivers/scsi/qla2xxx/qla_nx.c +5 -5
index c760ae354174..c1c832271ccb 100644
@@ -1985,7 +1985,7 @@ qla82xx_check_rcvpeg_state(struct qla_hw_data *ha)
 }
 
 /* ISR related functions */
-static struct qla82xx_legacy_intr_set legacy_intr[] = \
+static struct qla82xx_legacy_intr_set legacy_intr[] =
 	QLA82XX_LEGACY_INTR_CONFIG;
 
 /*
@@ -3286,7 +3286,7 @@ qla82xx_device_state_handler(scsi_qla_host_t *vha)
 		case QLA8XXX_DEV_NEED_QUIESCENT:
 			qla82xx_need_qsnt_handler(vha);
 			/* Reset timeout value after quiescence handler */
-			dev_init_timeout = jiffies + (ha->fcoe_dev_init_timeout\
+			dev_init_timeout = jiffies + (ha->fcoe_dev_init_timeout
 							 * HZ);
 			break;
 		case QLA8XXX_DEV_QUIESCENT:
@@ -3301,7 +3301,7 @@ qla82xx_device_state_handler(scsi_qla_host_t *vha)
 			qla82xx_idc_lock(ha);
 
 			/* Reset timeout value after quiescence handler */
-			dev_init_timeout = jiffies + (ha->fcoe_dev_init_timeout\
+			dev_init_timeout = jiffies + (ha->fcoe_dev_init_timeout
 							 * HZ);
 			break;
 		case QLA8XXX_DEV_FAILED:
@@ -4232,7 +4232,7 @@ qla82xx_md_collect(scsi_qla_host_t *vha)
 		goto md_failed;
 	}
 
-	entry_hdr = (qla82xx_md_entry_hdr_t *) \
+	entry_hdr = (qla82xx_md_entry_hdr_t *)
 	    (((uint8_t *)ha->md_tmplt_hdr) + tmplt_hdr->first_entry_offset);
 
 	/* Walk through the entry headers */
@@ -4339,7 +4339,7 @@ qla82xx_md_collect(scsi_qla_host_t *vha)
 		data_collected = (uint8_t *)data_ptr -
 		    (uint8_t *)ha->md_dump;
 skip_nxt_entry:
-		entry_hdr = (qla82xx_md_entry_hdr_t *) \
+		entry_hdr = (qla82xx_md_entry_hdr_t *)
 		    (((uint8_t *)entry_hdr) + entry_hdr->entry_size);
 	}
 
drivers/scsi/qla2xxx/qla_nx.h +7 -7
index 3c7beef92c35..230abee10598 100644
@@ -486,13 +486,13 @@
 #define QLA82XX_ADDR_QDR_NET		(0x0000000300000000ULL)
 #define QLA82XX_P3_ADDR_QDR_NET_MAX	(0x0000000303ffffffULL)
 
-#define QLA82XX_PCI_CRBSPACE		(unsigned long)0x06000000
-#define QLA82XX_PCI_DIRECT_CRB		(unsigned long)0x04400000
-#define QLA82XX_PCI_CAMQM		(unsigned long)0x04800000
-#define QLA82XX_PCI_CAMQM_MAX		(unsigned long)0x04ffffff
-#define QLA82XX_PCI_DDR_NET		(unsigned long)0x00000000
-#define QLA82XX_PCI_QDR_NET		(unsigned long)0x04000000
-#define QLA82XX_PCI_QDR_NET_MAX		(unsigned long)0x043fffff
+#define QLA82XX_PCI_CRBSPACE		0x06000000UL
+#define QLA82XX_PCI_DIRECT_CRB		0x04400000UL
+#define QLA82XX_PCI_CAMQM		0x04800000UL
+#define QLA82XX_PCI_CAMQM_MAX		0x04ffffffUL
+#define QLA82XX_PCI_DDR_NET		0x00000000UL
+#define QLA82XX_PCI_QDR_NET		0x04000000UL
+#define QLA82XX_PCI_QDR_NET_MAX		0x043fffffUL
 
 /*
  *   Register offsets for MN
drivers/scsi/qla2xxx/qla_os.c +3 -3
index db1f1aac79f2..8f972e1b5215 100644
@@ -69,7 +69,7 @@ MODULE_PARM_DESC(ql2xplogiabsentdevice,
 		"a Fabric scan.  This is needed for several broken switches. "
 		"Default is 0 - no PLOGI. 1 - perform PLOGI.");
 
-int ql2xloginretrycount = 0;
+int ql2xloginretrycount;
 module_param(ql2xloginretrycount, int, S_IRUGO);
 MODULE_PARM_DESC(ql2xloginretrycount,
 		"Specify an alternate value for the NVRAM login retry count.");
@@ -234,7 +234,7 @@ MODULE_PARM_DESC(ql2xmdenable,
 		"0 - MiniDump disabled. "
 		"1 (Default) - MiniDump enabled.");
 
-int ql2xexlogins = 0;
+int ql2xexlogins;
 module_param(ql2xexlogins, uint, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(ql2xexlogins,
 		 "Number of extended Logins. "
@@ -250,7 +250,7 @@ module_param(ql2xiniexchg, uint, 0644);
 MODULE_PARM_DESC(ql2xiniexchg,
 	"Number of initiator exchanges.");
 
-int ql2xfwholdabts = 0;
+int ql2xfwholdabts;
 module_param(ql2xfwholdabts, int, S_IRUGO);
 MODULE_PARM_DESC(ql2xfwholdabts,
 		"Allow FW to hold status IOCB until ABTS rsp received. "
Signature · 1 line
2.22.0
#4 lore raw

PATCH v2 4/58 qla2xxx: Use tabs instead of spaces for indentation

In reply to Bart Van Assche

This patch only modifies whitespace.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_gs.c | 78 +++++++++++++++++------------------
 1 file changed, 39 insertions(+), 39 deletions(-)
drivers/scsi/qla2xxx/qla_gs.c +39 -39
index 749109c8f20b..7dc9eeb0c401 100644
@@ -3467,54 +3467,54 @@ int qla24xx_async_gpnid(scsi_qla_host_t *vha, port_id_t *id)
 
 void qla24xx_handle_gffid_event(scsi_qla_host_t *vha, struct event_arg *ea)
 {
-       fc_port_t *fcport = ea->fcport;
+	fc_port_t *fcport = ea->fcport;
 
-       qla24xx_post_gnl_work(vha, fcport);
+	qla24xx_post_gnl_work(vha, fcport);
 }
 
 void qla24xx_async_gffid_sp_done(void *s, int res)
 {
-       struct srb *sp = s;
-       struct scsi_qla_host *vha = sp->vha;
-       fc_port_t *fcport = sp->fcport;
-       struct ct_sns_rsp *ct_rsp;
-       struct event_arg ea;
-
-       ql_dbg(ql_dbg_disc, vha, 0x2133,
-	   "Async done-%s res %x ID %x. %8phC\n",
-	   sp->name, res, fcport->d_id.b24, fcport->port_name);
-
-       fcport->flags &= ~FCF_ASYNC_SENT;
-       ct_rsp = &fcport->ct_desc.ct_sns->p.rsp;
-       /*
-	* FC-GS-7, 5.2.3.12 FC-4 Features - format
-	* The format of the FC-4 Features object, as defined by the FC-4,
-	* Shall be an array of 4-bit values, one for each type code value
-	*/
-       if (!res) {
-	       if (ct_rsp->rsp.gff_id.fc4_features[GFF_FCP_SCSI_OFFSET] & 0xf) {
-		       /* w1 b00:03 */
-		       fcport->fc4_type =
-			   ct_rsp->rsp.gff_id.fc4_features[GFF_FCP_SCSI_OFFSET];
-		       fcport->fc4_type &= 0xf;
-	       }
+	struct srb *sp = s;
+	struct scsi_qla_host *vha = sp->vha;
+	fc_port_t *fcport = sp->fcport;
+	struct ct_sns_rsp *ct_rsp;
+	struct event_arg ea;
+
+	ql_dbg(ql_dbg_disc, vha, 0x2133,
+	       "Async done-%s res %x ID %x. %8phC\n",
+	       sp->name, res, fcport->d_id.b24, fcport->port_name);
 
-	       if (ct_rsp->rsp.gff_id.fc4_features[GFF_NVME_OFFSET] & 0xf) {
-		       /* w5 [00:03]/28h */
-		       fcport->fc4f_nvme =
-			   ct_rsp->rsp.gff_id.fc4_features[GFF_NVME_OFFSET];
-		       fcport->fc4f_nvme &= 0xf;
+	fcport->flags &= ~FCF_ASYNC_SENT;
+	ct_rsp = &fcport->ct_desc.ct_sns->p.rsp;
+	/*
+	 * FC-GS-7, 5.2.3.12 FC-4 Features - format
+	 * The format of the FC-4 Features object, as defined by the FC-4,
+	 * Shall be an array of 4-bit values, one for each type code value
+	 */
+	if (!res) {
+		if (ct_rsp->rsp.gff_id.fc4_features[GFF_FCP_SCSI_OFFSET] & 0xf) {
+			/* w1 b00:03 */
+			fcport->fc4_type =
+			    ct_rsp->rsp.gff_id.fc4_features[GFF_FCP_SCSI_OFFSET];
+			fcport->fc4_type &= 0xf;
 	       }
-       }
 
-       memset(&ea, 0, sizeof(ea));
-       ea.sp = sp;
-       ea.fcport = sp->fcport;
-       ea.rc = res;
-       ea.event = FCME_GFFID_DONE;
+		if (ct_rsp->rsp.gff_id.fc4_features[GFF_NVME_OFFSET] & 0xf) {
+			/* w5 [00:03]/28h */
+			fcport->fc4f_nvme =
+			    ct_rsp->rsp.gff_id.fc4_features[GFF_NVME_OFFSET];
+			fcport->fc4f_nvme &= 0xf;
+		}
+	}
 
-       qla2x00_fcport_event_handler(vha, &ea);
-       sp->free(sp);
+	memset(&ea, 0, sizeof(ea));
+	ea.sp = sp;
+	ea.fcport = sp->fcport;
+	ea.rc = res;
+	ea.event = FCME_GFFID_DONE;
+
+	qla2x00_fcport_event_handler(vha, &ea);
+	sp->free(sp);
 }
 
 /* Get FC4 Feature with Nport ID. */
Signature · 1 line
2.22.0
#5 lore raw

PATCH v2 5/58 qla2xxx: Include the <asm/unaligned.h> header file from qla_dsd.h

In reply to Bart Van Assche

Since the put_unaligned_*() macros are used in this header file, include the header file that defines these macros.

Cc Himanshu Madhani <[email protected]>
Fixes 15b7a68c1d03 ("scsi: qla2xxx: Introduce the dsd32 and dsd64 data structures") # v5.2-rc1.
---
 drivers/scsi/qla2xxx/qla_dsd.h | 2 ++
 1 file changed, 2 insertions(+)
drivers/scsi/qla2xxx/qla_dsd.h +2 -0
index 7479924ba422..20788054b91b 100644
@@ -1,6 +1,8 @@
 #ifndef _QLA_DSD_H_
 #define _QLA_DSD_H_
 
+#include <asm/unaligned.h>
+
 /* 32-bit data segment descriptor (8 bytes) */
 struct dsd32 {
 	__le32 address;
Signature · 1 line
2.22.0
#6 lore raw

PATCH v2 6/58 qla2xxx: Remove an include directive from qla_mr.c

In reply to Bart Van Assche

There is no bsg code in the qla_mr.c source file. Hence do not include the <linux/bsg-lib.h> header file from qla_mr.c.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_mr.c | 1 -
 1 file changed, 1 deletion(-)
drivers/scsi/qla2xxx/qla_mr.c +0 -1
index 942ee13b96a4..cd892edec4dc 100644
@@ -10,7 +10,6 @@
 #include <linux/pci.h>
 #include <linux/ratelimit.h>
 #include <linux/vmalloc.h>
-#include <linux/bsg-lib.h>
 #include <scsi/scsi_tcq.h>
 #include <linux/utsname.h>
 
Signature · 1 line
2.22.0
#7 lore raw

PATCH v2 7/58 qla2xxx: Remove a superfluous forward declaration

In reply to Bart Van Assche

Since qlt_make_local_sess() is defined before it is called, remove the forward declaration of that function.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.c | 2 --
 1 file changed, 2 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +0 -2
index d0061ae1488e..4c5f9c02c379 100644
@@ -4106,8 +4106,6 @@ static inline int qlt_get_fcp_task_attr(struct scsi_qla_host *vha,
 	return fcp_task_attr;
 }
 
-static struct fc_port *qlt_make_local_sess(struct scsi_qla_host *,
-					uint8_t *);
 /*
  * Process context for I/O path into tcm_qla2xxx code
  */
Signature · 1 line
2.22.0
#8 lore raw

PATCH v2 8/58 qla2xxx: Declare the fourth ql_dump_buffer() argument const

In reply to Bart Van Assche

This patch makes it clear to humans and also to the compiler that ql_dump_buffer() does not modify the memory the @buf argument points at.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_dbg.c | 3 ++-
 drivers/scsi/qla2xxx/qla_gbl.h | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)
drivers/scsi/qla2xxx/qla_dbg.c +2 -1
index 9e80646722e2..30afc59c1870 100644
@@ -2743,7 +2743,8 @@ ql_dump_regs(uint level, scsi_qla_host_t *vha, uint id)
 
 
 void
-ql_dump_buffer(uint level, scsi_qla_host_t *vha, uint id, void *buf, uint size)
+ql_dump_buffer(uint level, scsi_qla_host_t *vha, uint id, const void *buf,
+	       uint size)
 {
 	uint cnt;
 
drivers/scsi/qla2xxx/qla_gbl.h +1 -1
index f9669fdf7798..6f6801722a09 100644
@@ -630,7 +630,7 @@ extern ulong qla27xx_fwdt_template_size(void *);
 
 extern void qla2xxx_dump_post_process(scsi_qla_host_t *, int);
 extern void ql_dump_regs(uint, scsi_qla_host_t *, uint);
-extern void ql_dump_buffer(uint, scsi_qla_host_t *, uint, void *, uint);
+extern void ql_dump_buffer(uint, scsi_qla_host_t *, uint, const void *, uint);
 /*
  * Global Function Prototypes in qla_gs.c source file.
  */
Signature · 1 line
2.22.0
#9 lore raw

PATCH v2 9/58 qla2xxx: Change the return type of qla2x00_update_ms_fdmi_iocb() into void

In reply to Bart Van Assche

The value returned by this function is not used. Hence change the return type of this function into 'void' and remove the return statement.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_gs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/scsi/qla2xxx/qla_gs.c +1 -3
index 7dc9eeb0c401..5ec3c2b96f3f 100644
@@ -1479,7 +1479,7 @@ qla24xx_prep_ms_fdmi_iocb(scsi_qla_host_t *vha, uint32_t req_size,
 	return ct_pkt;
 }
 
-static inline ms_iocb_entry_t *
+static void
 qla2x00_update_ms_fdmi_iocb(scsi_qla_host_t *vha, uint32_t req_size)
 {
 	struct qla_hw_data *ha = vha->hw;
@@ -1493,8 +1493,6 @@ qla2x00_update_ms_fdmi_iocb(scsi_qla_host_t *vha, uint32_t req_size)
 		ms_pkt->req_bytecount = cpu_to_le32(req_size);
 		ms_pkt->req_dsd.length = ms_pkt->req_bytecount;
 	}
-
-	return ms_pkt;
 }
 
 /**
Signature · 1 line
2.22.0
#10 lore raw

PATCH v2 10/58 qla2xxx: Reduce the scope of three local variables in qla2xxx_queuecommand()

In reply to Bart Van Assche

This patch makes it clear that the tag, hwq and qpair variables are only used in the mq path.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_os.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/scsi/qla2xxx/qla_os.c +4 -3
index 8f972e1b5215..a5acd5e2dfb1 100644
@@ -845,9 +845,6 @@ qla2xxx_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	struct scsi_qla_host *base_vha = pci_get_drvdata(ha->pdev);
 	srb_t *sp;
 	int rval;
-	struct qla_qpair *qpair = NULL;
-	uint32_t tag;
-	uint16_t hwq;
 
 	if (unlikely(test_bit(UNLOADING, &base_vha->dpc_flags)) ||
 	    WARN_ON_ONCE(!rport)) {
@@ -856,6 +853,10 @@ qla2xxx_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	}
 
 	if (ha->mqenable) {
+		uint32_t tag;
+		uint16_t hwq;
+		struct qla_qpair *qpair = NULL;
+
 		tag = blk_mq_unique_tag(cmd->request);
 		hwq = blk_mq_unique_tag_to_hwq(tag);
 		qpair = ha->queue_pair_map[hwq];
Signature · 1 line
2.22.0
#11 lore raw

PATCH v2 11/58 qla2xxx: Declare qla_tgt_cmd.cdb const

In reply to Bart Van Assche

Make it clear that the CDB is not modified after processing of a SCSI command has started.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_target.h +1 -1
index b8d244f1e189..29d98757acff 100644
@@ -912,7 +912,7 @@ struct qla_tgt_cmd {
 	uint8_t scsi_status, sense_key, asc, ascq;
 
 	struct crc_context *ctx;
-	uint8_t		*cdb;
+	const uint8_t	*cdb;
 	uint64_t	lba;
 	uint16_t	a_guard, e_guard, a_app_tag, e_app_tag;
 	uint32_t	a_ref_tag, e_ref_tag;
Signature · 1 line
2.22.0
#12 lore raw

PATCH v2 12/58 qla2xxx: Change data_dsd into an array

In reply to Bart Van Assche

This patch does not change any functionality but fixes a Coverity complaint about using a scalar as an array.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_def.h    | 4 ++--
 drivers/scsi/qla2xxx/qla_iocb.c   | 4 ++--
 drivers/scsi/qla2xxx/qla_target.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +2 -2
index a94b0d440f0f..1c6811bd5625 100644
@@ -1852,7 +1852,7 @@ struct crc_context {
 			uint16_t	reserved_2;
 			uint16_t	reserved_3;
 			uint32_t	reserved_4;
-			struct dsd64	data_dsd;
+			struct dsd64	data_dsd[1];
 			uint32_t	reserved_5[2];
 			uint32_t	reserved_6;
 		} nobundling;
@@ -1862,7 +1862,7 @@ struct crc_context {
 			uint16_t	reserved_1;
 			__le16	dseg_count;	/* Data segment count */
 			uint32_t	reserved_2;
-			struct dsd64	data_dsd;
+			struct dsd64	data_dsd[1];
 			struct dsd64	dif_dsd;
 		} bundling;
 	} u;
drivers/scsi/qla2xxx/qla_iocb.c +2 -2
index b514ab4d243e..6b120254f414 100644
@@ -1516,7 +1516,7 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, struct cmd_type_crc_2 *cmd_pkt,
 	}
 
 	if (!bundling) {
-		cur_dsd = &crc_ctx_pkt->u.nobundling.data_dsd;
+		cur_dsd = &crc_ctx_pkt->u.nobundling.data_dsd[0];
 	} else {
 		/*
 		 * Configure Bundling if we need to fetch interlaving
@@ -1526,7 +1526,7 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, struct cmd_type_crc_2 *cmd_pkt,
 		crc_ctx_pkt->u.bundling.dif_byte_count = cpu_to_le32(dif_bytes);
 		crc_ctx_pkt->u.bundling.dseg_count = cpu_to_le16(tot_dsds -
 							tot_prot_dsds);
-		cur_dsd = &crc_ctx_pkt->u.bundling.data_dsd;
+		cur_dsd = &crc_ctx_pkt->u.bundling.data_dsd[0];
 	}
 
 	/* Finish the common fields of CRC pkt */
drivers/scsi/qla2xxx/qla_target.c +2 -2
index 4c5f9c02c379..b8241d7b1f8a 100644
@@ -3163,7 +3163,7 @@ qlt_build_ctio_crc2_pkt(struct qla_qpair *qpair, struct qla_tgt_prm *prm)
 	pkt->crc_context_len = CRC_CONTEXT_LEN_FW;
 
 	if (!bundling) {
-		cur_dsd = &crc_ctx_pkt->u.nobundling.data_dsd;
+		cur_dsd = &crc_ctx_pkt->u.nobundling.data_dsd[0];
 	} else {
 		/*
 		 * Configure Bundling if we need to fetch interlaving
@@ -3173,7 +3173,7 @@ qlt_build_ctio_crc2_pkt(struct qla_qpair *qpair, struct qla_tgt_prm *prm)
 		crc_ctx_pkt->u.bundling.dif_byte_count = cpu_to_le32(dif_bytes);
 		crc_ctx_pkt->u.bundling.dseg_count =
 			cpu_to_le16(prm->tot_dsds - prm->prot_seg_cnt);
-		cur_dsd = &crc_ctx_pkt->u.bundling.data_dsd;
+		cur_dsd = &crc_ctx_pkt->u.bundling.data_dsd[0];
 	}
 
 	/* Finish the common fields of CRC pkt */
Signature · 1 line
2.22.0
#13 lore raw

PATCH v2 13/58 qla2xxx: Verify locking assumptions at runtime

In reply to Bart Van Assche

Make sure that locking assumptions are verified at runtime if kernel debugging is enabled.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/scsi/qla2xxx/qla_target.c +8 -0
index b8241d7b1f8a..ded5f13372af 100644
@@ -789,6 +789,8 @@ qlt_plogi_ack_find_add(struct scsi_qla_host *vha, port_id_t *id,
 {
 	struct qlt_plogi_ack_t *pla;
 
+	lockdep_assert_held(&vha->hw->hardware_lock);
+
 	list_for_each_entry(pla, &vha->plogi_ack_list, list) {
 		if (pla->id.b24 == id->b24) {
 			ql_dbg(ql_dbg_disc + ql_dbg_verbose, vha, 0x210d,
@@ -4713,6 +4715,8 @@ static int qlt_handle_login(struct scsi_qla_host *vha,
 	struct qlt_plogi_ack_t *pla;
 	unsigned long flags;
 
+	lockdep_assert_held(&vha->hw->hardware_lock);
+
 	wwn = wwn_to_u64(iocb->u.isp24.port_name);
 
 	port_id.b.domain = iocb->u.isp24.port_id[2];
@@ -4886,6 +4890,8 @@ static int qlt_24xx_handle_els(struct scsi_qla_host *vha,
 	int res = 0;
 	unsigned long flags;
 
+	lockdep_assert_held(&ha->hardware_lock);
+
 	wwn = wwn_to_u64(iocb->u.isp24.port_name);
 
 	port_id.b.domain = iocb->u.isp24.port_id[2];
@@ -5162,6 +5168,8 @@ static void qlt_handle_imm_notify(struct scsi_qla_host *vha,
 	int send_notify_ack = 1;
 	uint16_t status;
 
+	lockdep_assert_held(&ha->hardware_lock);
+
 	status = le16_to_cpu(iocb->u.isp2x.status);
 	switch (status) {
 	case IMM_NTFY_LIP_RESET:
Signature · 1 line
2.22.0
#14 lore raw

PATCH v2 14/58 qla2xxx: Reduce the number of casts in GID list code

In reply to Bart Van Assche

This patch makes the code that parses the GID list easier to read without changing the behavior of the code.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_dfs.c    |  9 +++------
 drivers/scsi/qla2xxx/qla_init.c   | 18 ++++++++----------
 drivers/scsi/qla2xxx/qla_target.c |  9 +++------
 3 files changed, 14 insertions(+), 22 deletions(-)
drivers/scsi/qla2xxx/qla_dfs.c +3 -6
index a432caebefec..0a6fb359f4d5 100644
@@ -57,10 +57,9 @@ qla2x00_dfs_tgt_port_database_show(struct seq_file *s, void *unused)
 {
 	scsi_qla_host_t *vha = s->private;
 	struct qla_hw_data *ha = vha->hw;
-	struct gid_list_info *gid_list;
+	struct gid_list_info *gid_list, *gid;
 	dma_addr_t gid_list_dma;
 	fc_port_t fc_port;
-	char *id_iter;
 	int rc, i;
 	uint16_t entries, loop_id;
 	struct qla_tgt *tgt = vha->vha_tgt.qla_tgt;
@@ -82,13 +81,11 @@ qla2x00_dfs_tgt_port_database_show(struct seq_file *s, void *unused)
 		if (rc != QLA_SUCCESS)
 			goto out_free_id_list;
 
-		id_iter = (char *)gid_list;
+		gid = gid_list;
 
 		seq_puts(s, "Port Name	Port ID 	Loop ID\n");
 
 		for (i = 0; i < entries; i++) {
-			struct gid_list_info *gid =
-			    (struct gid_list_info *)id_iter;
 			loop_id = le16_to_cpu(gid->loop_id);
 			memset(&fc_port, 0, sizeof(fc_port_t));
 
@@ -99,7 +96,7 @@ qla2x00_dfs_tgt_port_database_show(struct seq_file *s, void *unused)
 				fc_port.port_name, fc_port.d_id.b.domain,
 				fc_port.d_id.b.area, fc_port.d_id.b.al_pa,
 				fc_port.loop_id);
-			id_iter += ha->gid_list_info_size;
+			gid = (void *)gid + ha->gid_list_info_size;
 		}
 out_free_id_list:
 		dma_free_coherent(&ha->pdev->dev, qla2x00_gid_list_size(ha),
drivers/scsi/qla2xxx/qla_init.c +8 -10
index 3585eb7b87b5..c24d7667d3c9 100644
@@ -5086,7 +5086,7 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha)
 
 	uint16_t	index;
 	uint16_t	entries;
-	char		*id_iter;
+	struct gid_list_info *gid;
 	uint16_t	loop_id;
 	uint8_t		domain, area, al_pa;
 	struct qla_hw_data *ha = vha->hw;
@@ -5161,18 +5161,16 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha)
 	new_fcport->flags &= ~FCF_FABRIC_DEVICE;
 
 	/* Add devices to port list. */
-	id_iter = (char *)ha->gid_list;
+	gid = ha->gid_list;
 	for (index = 0; index < entries; index++) {
-		domain = ((struct gid_list_info *)id_iter)->domain;
-		area = ((struct gid_list_info *)id_iter)->area;
-		al_pa = ((struct gid_list_info *)id_iter)->al_pa;
+		domain = gid->domain;
+		area = gid->area;
+		al_pa = gid->al_pa;
 		if (IS_QLA2100(ha) || IS_QLA2200(ha))
-			loop_id = (uint16_t)
-			    ((struct gid_list_info *)id_iter)->loop_id_2100;
+			loop_id = gid->loop_id_2100;
 		else
-			loop_id = le16_to_cpu(
-			    ((struct gid_list_info *)id_iter)->loop_id);
-		id_iter += ha->gid_list_info_size;
+			loop_id = le16_to_cpu(gid->loop_id);
+		gid = (void *)gid + ha->gid_list_info_size;
 
 		/* Bypass reserved domain fields. */
 		if ((domain & 0xf0) == 0xf0)
drivers/scsi/qla2xxx/qla_target.c +3 -6
index ded5f13372af..3a25536c2492 100644
@@ -1290,8 +1290,7 @@ static int qla24xx_get_loop_id(struct scsi_qla_host *vha, const uint8_t *s_id,
 {
 	struct qla_hw_data *ha = vha->hw;
 	dma_addr_t gid_list_dma;
-	struct gid_list_info *gid_list;
-	char *id_iter;
+	struct gid_list_info *gid_list, *gid;
 	int res, rc, i;
 	uint16_t entries;
 
@@ -1314,11 +1313,9 @@ static int qla24xx_get_loop_id(struct scsi_qla_host *vha, const uint8_t *s_id,
 		goto out_free_id_list;
 	}
 
-	id_iter = (char *)gid_list;
+	gid = gid_list;
 	res = -ENOENT;
 	for (i = 0; i < entries; i++) {
-		struct gid_list_info *gid = (struct gid_list_info *)id_iter;
-
 		if ((gid->al_pa == s_id[2]) &&
 		    (gid->area == s_id[1]) &&
 		    (gid->domain == s_id[0])) {
@@ -1326,7 +1323,7 @@ static int qla24xx_get_loop_id(struct scsi_qla_host *vha, const uint8_t *s_id,
 			res = 0;
 			break;
 		}
-		id_iter += ha->gid_list_info_size;
+		gid = (void *)gid + ha->gid_list_info_size;
 	}
 
 out_free_id_list:
Signature · 1 line
2.22.0
#15 lore raw

PATCH v2 15/58 qla2xxx: Simplify qlt_lport_dump()

In reply to Bart Van Assche

Simplify the implementation of this function by using the %phC format specifier instead of using explicit for-loops.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +3 -15
index 3a25536c2492..221912da67c6 100644
@@ -6497,22 +6497,10 @@ void qlt_remove_target_resources(struct qla_hw_data *ha)
 static void qlt_lport_dump(struct scsi_qla_host *vha, u64 wwpn,
 	unsigned char *b)
 {
-	int i;
-
-	pr_debug("qla2xxx HW vha->node_name: ");
-	for (i = 0; i < WWN_SIZE; i++)
-		pr_debug("%02x ", vha->node_name[i]);
-	pr_debug("\n");
-	pr_debug("qla2xxx HW vha->port_name: ");
-	for (i = 0; i < WWN_SIZE; i++)
-		pr_debug("%02x ", vha->port_name[i]);
-	pr_debug("\n");
-
-	pr_debug("qla2xxx passed configfs WWPN: ");
+	pr_debug("qla2xxx HW vha->node_name: %8phC\n", vha->node_name);
+	pr_debug("qla2xxx HW vha->port_name: %8phC\n", vha->port_name);
 	put_unaligned_be64(wwpn, b);
-	for (i = 0; i < WWN_SIZE; i++)
-		pr_debug("%02x ", b[i]);
-	pr_debug("\n");
+	pr_debug("qla2xxx passed configfs WWPN: %8phC\n", b);
 }
 
 /**
Signature · 1 line
2.22.0
#16 lore raw

PATCH v2 16/58 qla2xxx: Remove a superfluous pointer check

In reply to Bart Van Assche

Checking a pointer after it has been dereferenced is not useful. This was detected by Coverity.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_nvme.c +1 -1
index 16028ee8c7a7..bba25c38a118 100644
@@ -222,7 +222,7 @@ static void qla_nvme_abort_work(struct work_struct *work)
 	       "%s called for sp=%p, hndl=%x on fcport=%p deleted=%d\n",
 	       __func__, sp, sp->handle, fcport, fcport->deleted);
 
-	if (!ha->flags.fw_started && (fcport && fcport->deleted))
+	if (!ha->flags.fw_started && fcport->deleted)
 		goto out;
 
 	if (ha->flags.host_shutting_down) {
Signature · 1 line
2.22.0
#17 lore raw

PATCH v2 17/58 qla2xxx: Remove two superfluous tests

In reply to Bart Van Assche

Since qlt_remove_target() only calls qlt_release() if vha->vha_tgt.qla_tgt != NULL, checking that pointer inside qlt_release() is not necessary. This patch fixes the following Coverity complaint:

CID 188348 (#1 of 1): Dereference after null check (FORWARD_NULL)
var_deref_model: Passing null pointer &vha->vha_tgt.qla_tgt->tgt_list_entry to list_del, which dereferences it.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +2 -3
index 221912da67c6..12a3e77e0d02 100644
@@ -1580,11 +1580,10 @@ static void qlt_release(struct qla_tgt *tgt)
 	struct qla_qpair_hint *h;
 	struct qla_hw_data *ha = vha->hw;
 
-	if ((vha->vha_tgt.qla_tgt != NULL) && !tgt->tgt_stop &&
-	    !tgt->tgt_stopped)
+	if (!tgt->tgt_stop && !tgt->tgt_stopped)
 		qlt_stop_phase1(tgt);
 
-	if ((vha->vha_tgt.qla_tgt != NULL) && !tgt->tgt_stopped)
+	if (!tgt->tgt_stopped)
 		qlt_stop_phase2(tgt);
 
 	for (i = 0; i < vha->hw->max_qpairs + 1; i++) {
Signature · 1 line
2.22.0
#18 lore raw

PATCH v2 18/58 qla2xxx: Simplify qla24xx_abort_sp_done()

In reply to Bart Van Assche

Instead of explicitly checking whether a timeout has occurred, ignore the del_timer() return value.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_init.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)
drivers/scsi/qla2xxx/qla_init.c +5 -7
index c24d7667d3c9..cab5f2f90714 100644
@@ -122,13 +122,11 @@ static void qla24xx_abort_sp_done(void *ptr, int res)
 	srb_t *sp = ptr;
 	struct srb_iocb *abt = &sp->u.iocb_cmd;
 
-	if ((res == QLA_OS_TIMER_EXPIRED) ||
-	    del_timer(&sp->u.iocb_cmd.timer)) {
-		if (sp->flags & SRB_WAKEUP_ON_COMP)
-			complete(&abt->u.abt.comp);
-		else
-			sp->free(sp);
-	}
+	del_timer(&sp->u.iocb_cmd.timer);
+	if (sp->flags & SRB_WAKEUP_ON_COMP)
+		complete(&abt->u.abt.comp);
+	else
+		sp->free(sp);
 }
 
 static int qla24xx_async_abort_cmd(srb_t *cmd_sp, bool wait)
Signature · 1 line
2.22.0
#19 lore raw

PATCH v2 19/58 qla2xxx: Fix session lookup in qlt_abort_work()

In reply to Bart Van Assche

Pass the correct session ID to find_sess_by_s_id() instead of passing an uninitialized variable.

Cc Himanshu Madhani <[email protected]>
Fixes 2d70c103fd2a ("[SCSI] qla2xxx: Add LLD target-mode infrastructure for >= 24xx series") # v3.5.
---
 drivers/scsi/qla2xxx/qla_target.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +1 -3
index 12a3e77e0d02..ea22e62257cb 100644
@@ -6198,7 +6198,6 @@ static void qlt_abort_work(struct qla_tgt *tgt,
 	struct qla_hw_data *ha = vha->hw;
 	struct fc_port *sess = NULL;
 	unsigned long flags = 0, flags2 = 0;
-	uint32_t be_s_id;
 	uint8_t s_id[3];
 	int rc;
 
@@ -6211,8 +6210,7 @@ static void qlt_abort_work(struct qla_tgt *tgt,
 	s_id[1] = prm->abts.fcp_hdr_le.s_id[1];
 	s_id[2] = prm->abts.fcp_hdr_le.s_id[0];
 
-	sess = ha->tgt.tgt_ops->find_sess_by_s_id(vha,
-	    (unsigned char *)&be_s_id);
+	sess = ha->tgt.tgt_ops->find_sess_by_s_id(vha, s_id);
 	if (!sess) {
 		spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
 
Signature · 1 line
2.22.0
#20 lore raw

PATCH v2 20/58 qla2xxx: Report the firmware status code if a mailbox command fails

In reply to Bart Van Assche

It is helpful when debugging this driver to have the firmware status code available if a mailbox command fails. Hence report that firmware status code.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_mbx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_mbx.c +5 -1
index 133f5f6270ff..783a84606047 100644
@@ -394,8 +394,12 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp)
 			goto premature_exit;
 		}
 
-		if (ha->mailbox_out[0] != MBS_COMMAND_COMPLETE)
+		if (ha->mailbox_out[0] != MBS_COMMAND_COMPLETE) {
+			ql_dbg(ql_dbg_mbx, vha, 0x11ff,
+			       "mb_out[0] = %#x <> %#x\n", ha->mailbox_out[0],
+			       MBS_COMMAND_COMPLETE);
 			rval = QLA_FUNCTION_FAILED;
+		}
 
 		/* Load return mailbox registers. */
 		iptr2 = mcp->mb;
Signature · 1 line
2.22.0
#21 lore raw

PATCH v2 21/58 qla2xxx: Do not corrupt vha->plogi_ack_list

In reply to Bart Van Assche

Delete the PLOGIN ACK data structure from the vha->plogi_ack_list before freeing that data structure to avoid that that list gets corrupted.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_os.c     | 8 ++++++--
 drivers/scsi/qla2xxx/qla_target.c | 4 +++-
 2 files changed, 9 insertions(+), 3 deletions(-)
drivers/scsi/qla2xxx/qla_os.c +6 -2
index a5acd5e2dfb1..37e24987c852 100644
@@ -5083,8 +5083,10 @@ void qla24xx_create_new_sess(struct scsi_qla_host *vha, struct qla_work_evt *e)
 				   "%s %8phC mem alloc fail.\n",
 				   __func__, e->u.new_sess.port_name);
 
-			if (pla)
+			if (pla) {
+				list_del(&pla->list);
 				kmem_cache_free(qla_tgt_plogi_cachep, pla);
+			}
 			return;
 		}
 
@@ -5195,8 +5197,10 @@ void qla24xx_create_new_sess(struct scsi_qla_host *vha, struct qla_work_evt *e)
 
 	if (free_fcport) {
 		qla2x00_free_fcport(fcport);
-		if (pla)
+		if (pla) {
+			list_del(&pla->list);
 			kmem_cache_free(qla_tgt_plogi_cachep, pla);
+		}
 	}
 }
 
drivers/scsi/qla2xxx/qla_target.c +3 -1
index ea22e62257cb..9cd5e2fba8ca 100644
@@ -4796,8 +4796,10 @@ static int qlt_handle_login(struct scsi_qla_host *vha,
 			    __func__, sess->port_name, sec);
 		}
 
-		if (!conflict_sess)
+		if (!conflict_sess) {
+			list_del(&pla->list);
 			kmem_cache_free(qla_tgt_plogi_cachep, pla);
+		}
 
 		qlt_send_term_imm_notif(vha, iocb, 1);
 		goto out;
Signature · 1 line
2.22.0
#22 lore raw

PATCH v2 22/58 qla2xxx: Use strlcpy() instead of strncpy()

In reply to Bart Van Assche

This patch fixes several gcc complaints about string truncation.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_mr.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
drivers/scsi/qla2xxx/qla_mr.c +14 -14
index cd892edec4dc..b6be7e7f2a43 100644
@@ -1880,22 +1880,22 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type)
 			phost_info = &preg_hsi->hsi;
 			memset(preg_hsi, 0, sizeof(struct register_host_info));
 			phost_info->os_type = OS_TYPE_LINUX;
-			strncpy(phost_info->sysname,
-			    p_sysid->sysname, SYSNAME_LENGTH);
-			strncpy(phost_info->nodename,
-			    p_sysid->nodename, NODENAME_LENGTH);
+			strlcpy(phost_info->sysname, p_sysid->sysname,
+				sizeof(phost_info->sysname));
+			strlcpy(phost_info->nodename, p_sysid->nodename,
+				sizeof(phost_info->nodename));
 			if (!strcmp(phost_info->nodename, "(none)"))
 				ha->mr.host_info_resend = true;
-			strncpy(phost_info->release,
-			    p_sysid->release, RELEASE_LENGTH);
-			strncpy(phost_info->version,
-			    p_sysid->version, VERSION_LENGTH);
-			strncpy(phost_info->machine,
-			    p_sysid->machine, MACHINE_LENGTH);
-			strncpy(phost_info->domainname,
-			    p_sysid->domainname, DOMNAME_LENGTH);
-			strncpy(phost_info->hostdriver,
-			    QLA2XXX_VERSION, VERSION_LENGTH);
+			strlcpy(phost_info->release, p_sysid->release,
+				sizeof(phost_info->release));
+			strlcpy(phost_info->version, p_sysid->version,
+				sizeof(phost_info->version));
+			strlcpy(phost_info->machine, p_sysid->machine,
+				sizeof(phost_info->machine));
+			strlcpy(phost_info->domainname, p_sysid->domainname,
+				sizeof(phost_info->domainname));
+			strlcpy(phost_info->hostdriver, QLA2XXX_VERSION,
+				sizeof(phost_info->hostdriver));
 			preg_hsi->utc = (uint64_t)ktime_get_real_seconds();
 			ql_dbg(ql_dbg_init, vha, 0x0149,
 			    "ISP%04X: Host registration with firmware\n",
Signature · 1 line
2.22.0
#23 lore raw

PATCH v2 23/58 qla2xxx: Complain if a mailbox command times out

In reply to Bart Van Assche

This patch fixes the following Coverity complaint:

Unchecked return value (CHECKED_RETURN)
check_return: Calling wait_for_completion_timeout without checking return value (as is done elsewhere 14 out of 17 times).

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_mr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_mr.c +2 -1
index b6be7e7f2a43..9e3f2f462a2e 100644
@@ -148,7 +148,8 @@ qlafx00_mailbox_command(scsi_qla_host_t *vha, struct mbx_cmd_32 *mcp)
 		QLAFX00_SET_HST_INTR(ha, ha->mbx_intr_code);
 		spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
-		wait_for_completion_timeout(&ha->mbx_intr_comp, mcp->tov * HZ);
+		WARN_ON_ONCE(wait_for_completion_timeout(&ha->mbx_intr_comp,
+							 mcp->tov * HZ) != 0);
 	} else {
 		ql_dbg(ql_dbg_mbx, vha, 0x112c,
 		    "Cmd=%x Polling Mode.\n", command);
Signature · 1 line
2.22.0
#24 lore raw

PATCH v2 24/58 qla2xxx: Complain if parsing the version string fails

In reply to Bart Van Assche

This patch fixes a Coverity complaint about not checking the sscanf() return value.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_tmpl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/scsi/qla2xxx/qla_tmpl.c +3 -2
index de696a07532e..7ed481dd8ee6 100644
@@ -860,8 +860,9 @@ qla27xx_driver_info(struct qla27xx_fwdt_template *tmp)
 {
 	uint8_t v[] = { 0, 0, 0, 0, 0, 0 };
 
-	sscanf(qla2x00_version_str, "%hhu.%hhu.%hhu.%hhu.%hhu.%hhu",
-	    v+0, v+1, v+2, v+3, v+4, v+5);
+	WARN_ON_ONCE(sscanf(qla2x00_version_str,
+			    "%hhu.%hhu.%hhu.%hhu.%hhu.%hhu",
+			    v+0, v+1, v+2, v+3, v+4, v+5) != 6);
 
 	tmp->driver_info[0] = v[3] << 24 | v[2] << 16 | v[1] << 8 | v[0];
 	tmp->driver_info[1] = v[5] << 8 | v[4];
Signature · 1 line
2.22.0
#25 lore raw

PATCH v2 25/58 qla2xxx: Remove dead code

In reply to Bart Van Assche

Since sess == NULL before 'goto out_term2' is executed, the code under 'if (sess)' cannot be reached. Hence remove that code. This was detected by Coverity.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +0 -3
index 9cd5e2fba8ca..ba53329e8bf9 100644
@@ -6249,9 +6249,6 @@ static void qlt_abort_work(struct qla_tgt *tgt,
 out_term2:
 	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags2);
 
-	if (sess)
-		ha->tgt.tgt_ops->put_sess(sess);
-
 out_term:
 	spin_lock_irqsave(&ha->hardware_lock, flags);
 	qlt_24xx_send_abts_resp(ha->base_qpair, &prm->abts,
Signature · 1 line
2.22.0
#26 lore raw

PATCH v2 26/58 qla2xxx: Simplify a debug statement

In reply to Bart Van Assche

Read the FC port state once instead of twice. This patch fixes the following Coverity complaint:

Unchecked return value (CHECKED_RETURN)
check_return: Calling atomic_read without checking return value (as is done elsewhere 80 out of 92 times).

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_isr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_isr.c +1 -1
index e8ce57cb897e..55eb51539cb0 100644
@@ -2726,7 +2726,7 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
 				"Port to be marked lost on fcport=%02x%02x%02x, current "
 				"port state= %s comp_status %x.\n", fcport->d_id.b.domain,
 				fcport->d_id.b.area, fcport->d_id.b.al_pa,
-				port_state_str[atomic_read(&fcport->state)],
+				port_state_str[FCS_ONLINE],
 				comp_status);
 
 			qla2x00_mark_device_lost(fcport->vha, fcport, 1, 1);
Signature · 1 line
2.22.0
#27 lore raw

PATCH v2 27/58 qla2xxx: Fix qla24xx_process_bidir_cmd()

In reply to Bart Van Assche

Set the r??_data_len variables before using these instead of after.

This patch fixes the following Coverity complaint:

const: At condition req_data_len != rsp_data_len, the value of req_data_len must be equal to 0.
const: At condition req_data_len != rsp_data_len, the value of rsp_data_len must be equal to 0.
dead_error_condition: The condition req_data_len != rsp_data_len cannot be true.

Cc Himanshu Madhani <[email protected]>
Fixes a9b6f722f62d ("[SCSI] qla2xxx: Implementation of bidirectional.") # v3.7.
---
 drivers/scsi/qla2xxx/qla_bsg.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
drivers/scsi/qla2xxx/qla_bsg.c +5 -6
index 11e420f8c493..240b07b0098a 100644
@@ -1782,8 +1782,8 @@ qla24xx_process_bidir_cmd(struct bsg_job *bsg_job)
 	uint16_t nextlid = 0;
 	uint32_t tot_dsds;
 	srb_t *sp = NULL;
-	uint32_t req_data_len = 0;
-	uint32_t rsp_data_len = 0;
+	uint32_t req_data_len;
+	uint32_t rsp_data_len;
 
 	/* Check the type of the adapter */
 	if (!IS_BIDI_CAPABLE(ha)) {
@@ -1888,6 +1888,9 @@ qla24xx_process_bidir_cmd(struct bsg_job *bsg_job)
 		goto done_unmap_sg;
 	}
 
+	req_data_len = bsg_job->request_payload.payload_len;
+	rsp_data_len = bsg_job->reply_payload.payload_len;
+
 	if (req_data_len != rsp_data_len) {
 		rval = EXT_STATUS_BUSY;
 		ql_log(ql_log_warn, vha, 0x70aa,
@@ -1895,10 +1898,6 @@ qla24xx_process_bidir_cmd(struct bsg_job *bsg_job)
 		goto done_unmap_sg;
 	}
 
-	req_data_len = bsg_job->request_payload.payload_len;
-	rsp_data_len = bsg_job->reply_payload.payload_len;
-
-
 	/* Alloc SRB structure */
 	sp = qla2x00_get_sp(vha, &(vha->bidir_fcport), GFP_KERNEL);
 	if (!sp) {
Signature · 1 line
2.22.0
#28 lore raw

PATCH v2 28/58 qla2xxx: Remove unreachable code from qla83xx_idc_lock()

In reply to Bart Van Assche

This was detected by Coverity.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_os.c | 17 -----------------
 1 file changed, 17 deletions(-)
drivers/scsi/qla2xxx/qla_os.c +0 -17
index 37e24987c852..2ba06a84c501 100644
@@ -5720,7 +5720,6 @@ qla83xx_idc_lock_recovery(scsi_qla_host_t *base_vha)
 void
 qla83xx_idc_lock(scsi_qla_host_t *base_vha, uint16_t requester_id)
 {
-	uint16_t options = (requester_id << 15) | BIT_6;
 	uint32_t data;
 	uint32_t lock_owner;
 	struct qla_hw_data *ha = base_vha->hw;
@@ -5753,22 +5752,6 @@ qla83xx_idc_lock(scsi_qla_host_t *base_vha, uint16_t requester_id)
 	}
 
 	return;
-
-	/* XXX: IDC-lock implementation using access-control mbx */
-retry_lock2:
-	if (qla83xx_access_control(base_vha, options, 0, 0, NULL)) {
-		ql_dbg(ql_dbg_p3p, base_vha, 0xb072,
-		    "Failed to acquire IDC lock. retrying...\n");
-		/* Retry/Perform IDC-Lock recovery */
-		if (qla83xx_idc_lock_recovery(base_vha) == QLA_SUCCESS) {
-			qla83xx_wait_logic();
-			goto retry_lock2;
-		} else
-			ql_log(ql_log_warn, base_vha, 0xb076,
-			    "IDC Lock recovery FAILED.\n");
-	}
-
-	return;
 }
 
 void
Signature · 1 line
2.22.0
#29 lore raw

PATCH v2 29/58 qla2xxx: Suppress a Coveritiy complaint about integer overflow

In reply to Bart Van Assche

This patch fixes the following Coverity complaint:

Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
overflow_before_widen: Potentially overflowing expression dma_eng_num * 65536U with type unsigned int (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type uint64_t (64 bits, unsigned).

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_nx2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_nx2.c +1 -1
index 369ac04d0454..c056f466f1f4 100644
@@ -2810,7 +2810,7 @@ qla8044_minidump_process_pollrdmwr(struct scsi_qla_host *vha,
 
 #define ISP8044_PEX_DMA_ENGINE_INDEX		8
 #define ISP8044_PEX_DMA_BASE_ADDRESS		0x77320000
-#define ISP8044_PEX_DMA_NUM_OFFSET		0x10000
+#define ISP8044_PEX_DMA_NUM_OFFSET		0x10000UL
 #define ISP8044_PEX_DMA_CMD_ADDR_LOW		0x0
 #define ISP8044_PEX_DMA_CMD_ADDR_HIGH		0x04
 #define ISP8044_PEX_DMA_CMD_STS_AND_CNTRL	0x08
Signature · 1 line
2.22.0
#30 lore raw

PATCH v2 30/58 qla2xxx: Suppress multiple Coverity complaint about out-of-bounds accesses

In reply to Bart Van Assche

This patch does not change any functionality.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_gs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/scsi/qla2xxx/qla_gs.c +4 -4
index 5ec3c2b96f3f..33e0cf210332 100644
@@ -1555,7 +1555,7 @@ qla2x00_fdmi_rhba(scsi_qla_host_t *vha)
 	/* Attributes */
 	ct_req->req.rhba.attrs.count =
 	    cpu_to_be32(FDMI_HBA_ATTR_COUNT);
-	entries = ct_req->req.rhba.hba_identifier;
+	entries = &ct_req->req;
 
 	/* Nodename. */
 	eiter = entries + size;
@@ -1764,7 +1764,7 @@ qla2x00_fdmi_rpa(scsi_qla_host_t *vha)
 
 	/* Attributes */
 	ct_req->req.rpa.attrs.count = cpu_to_be32(FDMI_PORT_ATTR_COUNT);
-	entries = ct_req->req.rpa.port_name;
+	entries = &ct_req->req;
 
 	/* FC4 types. */
 	eiter = entries + size;
@@ -1977,7 +1977,7 @@ qla2x00_fdmiv2_rhba(scsi_qla_host_t *vha)
 
 	/* Attributes */
 	ct_req->req.rhba2.attrs.count = cpu_to_be32(FDMIV2_HBA_ATTR_COUNT);
-	entries = ct_req->req.rhba2.hba_identifier;
+	entries = &ct_req->req;
 
 	/* Nodename. */
 	eiter = entries + size;
@@ -2336,7 +2336,7 @@ qla2x00_fdmiv2_rpa(scsi_qla_host_t *vha)
 
 	/* Attributes */
 	ct_req->req.rpa2.attrs.count = cpu_to_be32(FDMIV2_PORT_ATTR_COUNT);
-	entries = ct_req->req.rpa2.port_name;
+	entries = &ct_req->req;
 
 	/* FC4 types. */
 	eiter = entries + size;
Signature · 1 line
2.22.0
#31 lore raw

PATCH v2 31/58 qla2xxx: Always check the qla2x00_wait_for_hba_online() return value

In reply to Bart Van Assche

This patch fixes several Coverity complaints about not always checking the qla2x00_wait_for_hba_online() return value.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_attr.c   | 3 ++-
 drivers/scsi/qla2xxx/qla_target.c | 7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)
drivers/scsi/qla2xxx/qla_attr.c +2 -1
index 2b92d4659934..e3de20918efb 100644
@@ -724,7 +724,8 @@ qla2x00_sysfs_write_reset(struct file *filp, struct kobject *kobj,
 			break;
 		} else {
 			/* Make sure FC side is not in reset */
-			qla2x00_wait_for_hba_online(vha);
+			WARN_ON_ONCE(qla2x00_wait_for_hba_online(vha) !=
+				     QLA_SUCCESS);
 
 			/* Issue MPI reset */
 			scsi_block_requests(vha->host);
drivers/scsi/qla2xxx/qla_target.c +5 -2
index ba53329e8bf9..d20e0c21710e 100644
@@ -6673,7 +6673,8 @@ qlt_enable_vha(struct scsi_qla_host *vha)
 	} else {
 		set_bit(ISP_ABORT_NEEDED, &base_vha->dpc_flags);
 		qla2xxx_wake_dpc(base_vha);
-		qla2x00_wait_for_hba_online(base_vha);
+		WARN_ON_ONCE(qla2x00_wait_for_hba_online(base_vha) !=
+			     QLA_SUCCESS);
 	}
 	mutex_unlock(&ha->optrom_mutex);
 }
@@ -6704,7 +6705,9 @@ static void qlt_disable_vha(struct scsi_qla_host *vha)
 
 	set_bit(ISP_ABORT_NEEDED, &vha->dpc_flags);
 	qla2xxx_wake_dpc(vha);
-	qla2x00_wait_for_hba_online(vha);
+	if (qla2x00_wait_for_hba_online(vha) != QLA_SUCCESS)
+		ql_dbg(ql_dbg_tgt, vha, 0xe081,
+		       "qla2x00_wait_for_hba_online() failed\n");
 }
 
 /*
Signature · 1 line
2.22.0
#32 lore raw

PATCH v2 32/58 qla2xxx: Declare fourth qla2x00_set_model_info() argument const

In reply to Bart Van Assche

Make it clear to humans and also to the compiler that the string passed as fourth argument is not modified.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_gbl.h  | 4 ++--
 drivers/scsi/qla2xxx/qla_init.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/scsi/qla2xxx/qla_gbl.h +2 -2
index 6f6801722a09..fc54e7c86463 100644
@@ -822,8 +822,8 @@ extern int qla82xx_device_state_handler(scsi_qla_host_t *);
 extern void qla8xxx_dev_failed_handler(scsi_qla_host_t *);
 extern void qla82xx_clear_qsnt_ready(scsi_qla_host_t *);
 
-extern void qla2x00_set_model_info(scsi_qla_host_t *, uint8_t *,
-				   size_t, char *);
+extern void qla2x00_set_model_info(scsi_qla_host_t *, uint8_t *, size_t,
+				   const char *);
 extern int qla82xx_mbx_intr_enable(scsi_qla_host_t *);
 extern int qla82xx_mbx_intr_disable(scsi_qla_host_t *);
 extern void qla82xx_start_iocbs(scsi_qla_host_t *);
drivers/scsi/qla2xxx/qla_init.c +1 -1
index cab5f2f90714..1fd9a086748e 100644
@@ -4426,7 +4426,7 @@ qla2x00_configure_hba(scsi_qla_host_t *vha)
 
 inline void
 qla2x00_set_model_info(scsi_qla_host_t *vha, uint8_t *model, size_t len,
-	char *def)
+		       const char *def)
 {
 	char *st, *en;
 	uint16_t index;
Signature · 1 line
2.22.0
#33 lore raw

PATCH v2 33/58 qla2xxx: Complain if waiting for pending commands times out

In reply to Bart Van Assche

Such a timeout is a severe issue. Hence complain if waiting for pending commands times out. This patch fixes a small bug: it modifies qla82xx_chip_reset_cleanup() such that the "Done waiting" message is reported if qla82xx_chip_reset_cleanup() succeeded instead of if that function failed.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_init.c | 3 ++-
 drivers/scsi/qla2xxx/qla_nx.c   | 4 +++-
 drivers/scsi/qla2xxx/qla_os.c   | 7 +++++--
 3 files changed, 10 insertions(+), 4 deletions(-)
drivers/scsi/qla2xxx/qla_init.c +2 -1
index 1fd9a086748e..2d9a379fd8fb 100644
@@ -6634,7 +6634,8 @@ qla2x00_quiesce_io(scsi_qla_host_t *vha)
 					LOOP_DOWN_TIME);
 	}
 	/* Wait for pending cmds to complete */
-	qla2x00_eh_wait_for_pending_commands(vha, 0, 0, WAIT_HOST);
+	WARN_ON_ONCE(qla2x00_eh_wait_for_pending_commands(vha, 0, 0, WAIT_HOST)
+		     != QLA_SUCCESS);
 }
 
 void
drivers/scsi/qla2xxx/qla_nx.c +3 -1
index c1c832271ccb..a91d426add75 100644
@@ -3710,10 +3710,12 @@ qla82xx_chip_reset_cleanup(scsi_qla_host_t *vha)
 
 		/* Wait for pending cmds (physical and virtual) to complete */
 		if (qla2x00_eh_wait_for_pending_commands(vha, 0, 0,
-		    WAIT_HOST)) {
+		    WAIT_HOST) == QLA_SUCCESS) {
 			ql_dbg(ql_dbg_init, vha, 0x00b3,
 			    "Done wait for "
 			    "pending commands.\n");
+		} else {
+			WARN_ON_ONCE(true);
 		}
 	}
 }
drivers/scsi/qla2xxx/qla_os.c +5 -2
index 2ba06a84c501..e91681cbd75d 100644
@@ -1059,8 +1059,8 @@ qla2xxx_mqueuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd,
  *    cmd = Scsi Command to wait on.
  *
  * Return:
- *    Not Found : 0
- *    Found : 1
+ *    Completed in time : QLA_SUCCESS
+ *    Did not complete in time : QLA_FUNCTION_FAILED
  */
 static int
 qla2x00_eh_wait_on_command(struct scsi_cmnd *cmd)
@@ -1372,6 +1372,9 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
 	return ret;
 }
 
+/*
+ * Returns: QLA_SUCCESS or QLA_FUNCTION_FAILED.
+ */
 int
 qla2x00_eh_wait_for_pending_commands(scsi_qla_host_t *vha, unsigned int t,
 	uint64_t l, enum nexus_wait_type type)
Signature · 1 line
2.22.0
#34 lore raw

PATCH v2 34/58 qla2xxx: Check the PCI info string output buffer size

In reply to Bart Van Assche

Pass the output buffer size to the code that generates a PCI info string and check the output buffer size while generating a PCI info string.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_attr.c |  3 +-
 drivers/scsi/qla2xxx/qla_def.h  |  2 +-
 drivers/scsi/qla2xxx/qla_gbl.h  |  2 +-
 drivers/scsi/qla2xxx/qla_mr.c   |  8 ++---
 drivers/scsi/qla2xxx/qla_os.c   | 59 ++++++++++++++-------------------
 5 files changed, 32 insertions(+), 42 deletions(-)
drivers/scsi/qla2xxx/qla_attr.c +2 -1
index e3de20918efb..e9c449ef515c 100644
@@ -1125,7 +1125,8 @@ qla2x00_pci_info_show(struct device *dev, struct device_attribute *attr,
 	char pci_info[30];
 
 	return scnprintf(buf, PAGE_SIZE, "%s\n",
-	    vha->hw->isp_ops->pci_info_str(vha, pci_info));
+			 vha->hw->isp_ops->pci_info_str(vha, pci_info,
+							sizeof(pci_info)));
 }
 
 static ssize_t
drivers/scsi/qla2xxx/qla_def.h +1 -1
index 1c6811bd5625..8c8279ef3e32 100644
@@ -3112,7 +3112,7 @@ struct isp_operations {
 	void (*update_fw_options) (struct scsi_qla_host *);
 	int (*load_risc) (struct scsi_qla_host *, uint32_t *);
 
-	char * (*pci_info_str) (struct scsi_qla_host *, char *);
+	char * (*pci_info_str)(struct scsi_qla_host *, char *, size_t);
 	char * (*fw_version_str)(struct scsi_qla_host *, char *, size_t);
 
 	irq_handler_t intr_handler;
drivers/scsi/qla2xxx/qla_gbl.h +1 -1
index fc54e7c86463..2d9664086f15 100644
@@ -732,7 +732,7 @@ extern int qlafx00_initialize_adapter(struct scsi_qla_host *);
 extern int qlafx00_soft_reset(scsi_qla_host_t *);
 extern int qlafx00_chip_diag(scsi_qla_host_t *);
 extern void qlafx00_config_rings(struct scsi_qla_host *);
-extern char *qlafx00_pci_info_str(struct scsi_qla_host *, char *);
+extern char *qlafx00_pci_info_str(struct scsi_qla_host *, char *, size_t);
 extern char *qlafx00_fw_version_str(struct scsi_qla_host *, char *, size_t);
 extern irqreturn_t qlafx00_intr_handler(int, void *);
 extern void qlafx00_enable_intrs(struct qla_hw_data *);
drivers/scsi/qla2xxx/qla_mr.c +3 -5
index 9e3f2f462a2e..759fcfecc310 100644
@@ -688,14 +688,12 @@ qlafx00_config_rings(struct scsi_qla_host *vha)
 }
 
 char *
-qlafx00_pci_info_str(struct scsi_qla_host *vha, char *str)
+qlafx00_pci_info_str(struct scsi_qla_host *vha, char *str, size_t str_len)
 {
 	struct qla_hw_data *ha = vha->hw;
 
-	if (pci_is_pcie(ha->pdev)) {
-		strcpy(str, "PCIe iSA");
-		return str;
-	}
+	if (pci_is_pcie(ha->pdev))
+		strlcpy(str, "PCIe iSA", str_len);
 	return str;
 }
 
drivers/scsi/qla2xxx/qla_os.c +25 -34

112 lines not on this page · raw message

Signature · 1 line
2.22.0
#35 lore raw

PATCH v2 35/58 qla2xxx: Use memcpy() and strlcpy() instead of strcpy() and strncpy()

In reply to Bart Van Assche

This patch makes the string manipulation code easier to verify.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_init.c | 18 ++++++++++--------
 drivers/scsi/qla2xxx/qla_mr.c   |  6 ++++--
 2 files changed, 14 insertions(+), 10 deletions(-)
drivers/scsi/qla2xxx/qla_init.c +10 -8
index 2d9a379fd8fb..5258d2486e25 100644
@@ -4438,7 +4438,7 @@ qla2x00_set_model_info(scsi_qla_host_t *vha, uint8_t *model, size_t len,
 	if (len > sizeof(zero))
 		len = sizeof(zero);
 	if (memcmp(model, &zero, len) != 0) {
-		strncpy(ha->model_number, model, len);
+		memcpy(ha->model_number, model, len);
 		st = en = ha->model_number;
 		en += len - 1;
 		while (en > st) {
@@ -4451,21 +4451,23 @@ qla2x00_set_model_info(scsi_qla_host_t *vha, uint8_t *model, size_t len,
 		if (use_tbl &&
 		    ha->pdev->subsystem_vendor == PCI_VENDOR_ID_QLOGIC &&
 		    index < QLA_MODEL_NAMES)
-			strncpy(ha->model_desc,
+			strlcpy(ha->model_desc,
 			    qla2x00_model_name[index * 2 + 1],
-			    sizeof(ha->model_desc) - 1);
+			    sizeof(ha->model_desc));
 	} else {
 		index = (ha->pdev->subsystem_device & 0xff);
 		if (use_tbl &&
 		    ha->pdev->subsystem_vendor == PCI_VENDOR_ID_QLOGIC &&
 		    index < QLA_MODEL_NAMES) {
-			strcpy(ha->model_number,
-			    qla2x00_model_name[index * 2]);
-			strncpy(ha->model_desc,
+			strlcpy(ha->model_number,
+				qla2x00_model_name[index * 2],
+				sizeof(ha->model_number));
+			strlcpy(ha->model_desc,
 			    qla2x00_model_name[index * 2 + 1],
-			    sizeof(ha->model_desc) - 1);
+			    sizeof(ha->model_desc));
 		} else {
-			strcpy(ha->model_number, def);
+			strlcpy(ha->model_number, def,
+				sizeof(ha->model_number));
 		}
 	}
 	if (IS_FWI2_CAPABLE(ha))
drivers/scsi/qla2xxx/qla_mr.c +4 -2
index 759fcfecc310..78b3679e1b9c 100644
@@ -1939,8 +1939,10 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type)
 	if (fx_type == FXDISC_GET_CONFIG_INFO) {
 		struct config_info_data *pinfo =
 		    (struct config_info_data *) fdisc->u.fxiocb.rsp_addr;
-		strcpy(vha->hw->model_number, pinfo->model_num);
-		strcpy(vha->hw->model_desc, pinfo->model_description);
+		strlcpy(vha->hw->model_number, pinfo->model_num,
+			ARRAY_SIZE(vha->hw->model_number));
+		strlcpy(vha->hw->model_desc, pinfo->model_description,
+			ARRAY_SIZE(vha->hw->model_desc));
 		memcpy(&vha->hw->mr.symbolic_name, pinfo->symbolic_name,
 		    sizeof(vha->hw->mr.symbolic_name));
 		memcpy(&vha->hw->mr.serial_num, pinfo->serial_num,
Signature · 1 line
2.22.0
#36 lore raw

PATCH v2 36/58 qla2xxx: Complain if a soft reset fails

In reply to Bart Van Assche

Failure of a soft reset is a severe failure. Hence report such failures.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_tmpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_tmpl.c +1 -1
index 7ed481dd8ee6..294d77c02cdf 100644
@@ -429,7 +429,7 @@ qla27xx_fwdt_entry_t266(struct scsi_qla_host *vha,
 	ql_dbg(ql_dbg_misc, vha, 0xd20a,
 	    "%s: reset risc [%lx]\n", __func__, *len);
 	if (buf)
-		qla24xx_soft_reset(vha->hw);
+		WARN_ON_ONCE(qla24xx_soft_reset(vha->hw) != QLA_SUCCESS);
 
 	return qla27xx_next_entry(ent);
 }
Signature · 1 line
2.22.0
#37 lore raw

PATCH v2 37/58 qla2xxx: Introduce the be_id_t and le_id_t data types for FC src/dst IDs

In reply to Bart Van Assche

Introduce the be_id_t and le_id_t data types for Fibre Channel source and destination ID formats supported by the firmware instead of using an uint8_t[3] array. Introduce functions for converting from and to the port_id_t data types. This patch does not change the behavior of the qla2xxx driver but improves source code readability and also allows the compiler to verify the endianness of Fibre Channel IDs.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_def.h     |  71 ++++++++++++++--
 drivers/scsi/qla2xxx/qla_gs.c      |  62 ++++----------
 drivers/scsi/qla2xxx/qla_nvme.h    |   2 +-
 drivers/scsi/qla2xxx/qla_target.c  | 126 ++++++++++-------------------
 drivers/scsi/qla2xxx/qla_target.h  |  33 +++-----
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |  23 ++----
 6 files changed, 144 insertions(+), 173 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +65 -6
index 8c8279ef3e32..779bf3fcab0f 100644
@@ -34,6 +34,20 @@
 #include <scsi/scsi_transport_fc.h>
 #include <scsi/scsi_bsg_fc.h>
 
+/* Big endian Fibre Channel S_ID (source ID) or D_ID (destination ID). */
+typedef struct {
+	uint8_t domain;
+	uint8_t area;
+	uint8_t al_pa;
+} be_id_t;
+
+/* Little endian Fibre Channel S_ID (source ID) or D_ID (destination ID). */
+typedef struct {
+	uint8_t al_pa;
+	uint8_t area;
+	uint8_t domain;
+} le_id_t;
+
 #include "qla_bsg.h"
 #include "qla_dsd.h"
 #include "qla_nx.h"
@@ -343,6 +357,51 @@ typedef union {
 } port_id_t;
 #define INVALID_PORT_ID	0xFFFFFF
 
+static inline le_id_t be_id_to_le(be_id_t id)
+{
+	le_id_t res;
+
+	res.domain = id.domain;
+	res.area   = id.area;
+	res.al_pa  = id.al_pa;
+
+	return res;
+}
+
+static inline be_id_t le_id_to_be(le_id_t id)
+{
+	be_id_t res;
+
+	res.domain = id.domain;
+	res.area   = id.area;
+	res.al_pa  = id.al_pa;
+
+	return res;
+}
+
+static inline port_id_t be_to_port_id(be_id_t id)
+{
+	port_id_t res;
+
+	res.b.domain = id.domain;
+	res.b.area   = id.area;
+	res.b.al_pa  = id.al_pa;
+	res.b.rsvd_1 = 0;
+
+	return res;
+}
+
+static inline be_id_t port_id_to_be_id(port_id_t port_id)
+{
+	be_id_t res;
+
+	res.domain = port_id.b.domain;
+	res.area   = port_id.b.area;
+	res.al_pa  = port_id.b.al_pa;
+
+	return res;
+}
+
 struct els_logo_payload {
 	uint8_t opcode;
 	uint8_t rsvd[3];
@@ -2746,7 +2805,7 @@ struct ct_sns_req {
 		/* GA_NXT, GPN_ID, GNN_ID, GFT_ID, GFPN_ID */
 		struct {
 			uint8_t reserved;
-			uint8_t port_id[3];
+			be_id_t port_id;
 		} port_id;
 
 		struct {
@@ -2765,13 +2824,13 @@ struct ct_sns_req {
 
 		struct {
 			uint8_t reserved;
-			uint8_t port_id[3];
+			be_id_t port_id;
 			uint8_t fc4_types[32];
 		} rft_id;
 
 		struct {
 			uint8_t reserved;
-			uint8_t port_id[3];
+			be_id_t port_id;
 			uint16_t reserved2;
 			uint8_t fc4_feature;
 			uint8_t fc4_type;
@@ -2779,7 +2838,7 @@ struct ct_sns_req {
 
 		struct {
 			uint8_t reserved;
-			uint8_t port_id[3];
+			be_id_t port_id;
 			uint8_t node_name[8];
 		} rnn_id;
 
@@ -2866,7 +2925,7 @@ struct ct_rsp_hdr {
 
 struct ct_sns_gid_pt_data {
 	uint8_t control_byte;
-	uint8_t port_id[3];
+	be_id_t port_id;
 };
 
 /* It's the same for both GPN_FT and GNN_FT */
@@ -2896,7 +2955,7 @@ struct ct_sns_rsp {
 	union {
 		struct {
 			uint8_t port_type;
-			uint8_t port_id[3];
+			be_id_t port_id;
 			uint8_t port_name[8];
 			uint8_t sym_port_name_len;
 			uint8_t sym_port_name[255];
drivers/scsi/qla2xxx/qla_gs.c +17 -45

161 lines not on this page · raw message

drivers/scsi/qla2xxx/qla_nvme.h +1 -1
index 67bb4a2a3742..68a8d09a36ef 100644
@@ -119,7 +119,7 @@ struct pt_ls4_rx_unsol {
 	uint32_t exchange_address;
 	uint8_t d_id[3];
 	uint8_t r_ctl;
-	uint8_t s_id[3];
+	be_id_t s_id;
 	uint8_t cs_ctl;
 	uint8_t f_ctl[3];
 	uint8_t type;
drivers/scsi/qla2xxx/qla_target.c +44 -82

301 lines not on this page · raw message

drivers/scsi/qla2xxx/qla_target.h +11 -22
index 29d98757acff..d006f0a97b8c 100644
@@ -247,9 +247,9 @@ struct ctio_to_2xxx {
 
 struct fcp_hdr {
 	uint8_t  r_ctl;
-	uint8_t  d_id[3];
+	be_id_t  d_id;
 	uint8_t  cs_ctl;
-	uint8_t  s_id[3];
+	be_id_t  s_id;
 	uint8_t  type;
 	uint8_t  f_ctl[3];
 	uint8_t  seq_id;
@@ -261,9 +261,9 @@ struct fcp_hdr {
 } __packed;
 
 struct fcp_hdr_le {
-	uint8_t  d_id[3];
+	le_id_t  d_id;
 	uint8_t  r_ctl;
-	uint8_t  s_id[3];
+	le_id_t  s_id;
 	uint8_t  cs_ctl;
 	uint8_t  f_ctl[3];
 	uint8_t  type;
@@ -402,7 +402,7 @@ struct ctio7_to_24xx {
 	uint16_t dseg_count;		    /* Data segment count. */
 	uint8_t  vp_index;
 	uint8_t  add_flags;
-	uint8_t  initiator_id[3];
+	le_id_t  initiator_id;
 	uint8_t  reserved;
 	uint32_t exchange_addr;
 	union {
@@ -498,7 +498,7 @@ struct ctio_crc2_to_fw {
 	uint8_t  add_flags;		/* additional flags */
 #define CTIO_CRC2_AF_DIF_DSD_ENA BIT_3
 
-	uint8_t  initiator_id[3];	/* initiator ID */
+	le_id_t  initiator_id;		/* initiator ID */
 	uint8_t  reserved1;
 	uint32_t exchange_addr;		/* rcv exchange address */
 	uint16_t reserved2;
@@ -682,7 +682,7 @@ struct qla_tgt_func_tmpl {
 	struct fc_port *(*find_sess_by_loop_id)(struct scsi_qla_host *,
 						const uint16_t);
 	struct fc_port *(*find_sess_by_s_id)(struct scsi_qla_host *,
-						const uint8_t *);
+					     const be_id_t);
 	void (*clear_nacl_from_fcport_map)(struct fc_port *);
 	void (*put_sess)(struct fc_port *);
 	void (*shutdown_sess)(struct fc_port *);
@@ -1030,22 +1030,11 @@ static inline bool qla_dual_mode_enabled(struct scsi_qla_host *ha)
 	return (ha->host->active_mode == MODE_DUAL);
 }
 
-static inline uint32_t sid_to_key(const uint8_t *s_id)
+static inline uint32_t sid_to_key(const be_id_t s_id)
 {
-	uint32_t key;
-
-	key = (((unsigned long)s_id[0] << 16) |
-	       ((unsigned long)s_id[1] << 8) |
-	       (unsigned long)s_id[2]);
-	return key;
-}
-
-static inline void sid_to_portid(const uint8_t *s_id, port_id_t *p)
-{
-	memset(p, 0, sizeof(*p));
-	p->b.domain = s_id[0];
-	p->b.area = s_id[1];
-	p->b.al_pa = s_id[2];
+	return s_id.domain << 16 |
+		s_id.area << 8 |
+		s_id.al_pa;
 }
 
 /*
drivers/scsi/qla2xxx/tcm_qla2xxx.c +6 -17
index d15412d3d9bd..963c220f8ba8 100644
@@ -1136,9 +1136,8 @@ static struct se_portal_group *tcm_qla2xxx_npiv_make_tpg(struct se_wwn *wwn,
 /*
  * Expected to be called with struct qla_hw_data->tgt.sess_lock held
  */
-static struct fc_port *tcm_qla2xxx_find_sess_by_s_id(
-	scsi_qla_host_t *vha,
-	const uint8_t *s_id)
+static struct fc_port *tcm_qla2xxx_find_sess_by_s_id(scsi_qla_host_t *vha,
+						     const be_id_t s_id)
 {
 	struct tcm_qla2xxx_lport *lport;
 	struct se_node_acl *se_nacl;
@@ -1181,7 +1180,7 @@ static void tcm_qla2xxx_set_sess_by_s_id(
 	struct tcm_qla2xxx_nacl *nacl,
 	struct se_session *se_sess,
 	struct fc_port *fc_port,
-	uint8_t *s_id)
+	be_id_t s_id)
 {
 	u32 key;
 	void *slot;
@@ -1348,14 +1347,9 @@ static void tcm_qla2xxx_clear_sess_lookup(struct tcm_qla2xxx_lport *lport,
 		struct tcm_qla2xxx_nacl *nacl, struct fc_port *sess)
 {
 	struct se_session *se_sess = sess->se_sess;
-	unsigned char be_sid[3];
-
-	be_sid[0] = sess->d_id.b.domain;
-	be_sid[1] = sess->d_id.b.area;
-	be_sid[2] = sess->d_id.b.al_pa;
 
 	tcm_qla2xxx_set_sess_by_s_id(lport, NULL, nacl, se_sess,
-				sess, be_sid);
+				     sess, port_id_to_be_id(sess->d_id));
 	tcm_qla2xxx_set_sess_by_loop_id(lport, NULL, nacl, se_sess,
 				sess, sess->loop_id);
 }
@@ -1401,19 +1395,14 @@ static int tcm_qla2xxx_session_cb(struct se_portal_group *se_tpg,
 	struct fc_port *qlat_sess = p;
 	uint16_t loop_id = qlat_sess->loop_id;
 	unsigned long flags;
-	unsigned char be_sid[3];
-
-	be_sid[0] = qlat_sess->d_id.b.domain;
-	be_sid[1] = qlat_sess->d_id.b.area;
-	be_sid[2] = qlat_sess->d_id.b.al_pa;
 
 	/*
 	 * And now setup se_nacl and session pointers into HW lport internal
 	 * mappings for fabric S_ID and LOOP_ID.
 	 */
 	spin_lock_irqsave(&ha->tgt.sess_lock, flags);
-	tcm_qla2xxx_set_sess_by_s_id(lport, se_nacl, nacl,
-				     se_sess, qlat_sess, be_sid);
+	tcm_qla2xxx_set_sess_by_s_id(lport, se_nacl, nacl, se_sess, qlat_sess,
+				     port_id_to_be_id(qlat_sess->d_id));
 	tcm_qla2xxx_set_sess_by_loop_id(lport, se_nacl, nacl,
 					se_sess, qlat_sess, loop_id);
 	spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
Signature · 1 line
2.22.0
#38 lore raw

PATCH v2 38/58 qla2xxx: Change the return type of qla24xx_read_flash_data()

In reply to Bart Van Assche

This change makes it easier to detect qla24xx_read_flash_data() failures and also to handle such failures. This change does not modify the behavior of the driver since all callers ignore the qla24xx_read_flash_data() return value.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_gbl.h | 2 +-
 drivers/scsi/qla2xxx/qla_sup.c | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/scsi/qla2xxx/qla_gbl.h +1 -1
index 2d9664086f15..aac664da419b 100644
@@ -554,7 +554,7 @@ fc_port_t *qla2x00_find_fcport_by_nportid(scsi_qla_host_t *, port_id_t *, u8);
  * Global Function Prototypes in qla_sup.c source file.
  */
 extern void qla2x00_release_nvram_protection(scsi_qla_host_t *);
-extern uint32_t *qla24xx_read_flash_data(scsi_qla_host_t *, uint32_t *,
+extern int qla24xx_read_flash_data(scsi_qla_host_t *, uint32_t *,
     uint32_t, uint32_t);
 extern uint8_t *qla2x00_read_nvram_data(scsi_qla_host_t *, void *, uint32_t,
     uint32_t);
drivers/scsi/qla2xxx/qla_sup.c +5 -3
index 1eb82384d933..764e1bb0f695 100644
@@ -473,22 +473,24 @@ qla24xx_read_flash_dword(struct qla_hw_data *ha, uint32_t addr, uint32_t *data)
 	return QLA_FUNCTION_TIMEOUT;
 }
 
-uint32_t *
+int
 qla24xx_read_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t faddr,
     uint32_t dwords)
 {
 	ulong i;
+	int ret = QLA_SUCCESS;
 	struct qla_hw_data *ha = vha->hw;
 
 	/* Dword reads to flash. */
 	faddr =  flash_data_addr(ha, faddr);
 	for (i = 0; i < dwords; i++, faddr++, dwptr++) {
-		if (qla24xx_read_flash_dword(ha, faddr, dwptr))
+		ret = qla24xx_read_flash_dword(ha, faddr, dwptr);
+		if (ret != QLA_SUCCESS)
 			break;
 		cpu_to_le32s(dwptr);
 	}
 
-	return dwptr;
+	return ret;
 }
 
 static int
Signature · 1 line
2.22.0
#39 lore raw

PATCH v2 39/58 qla2xxx: Check secondary image if reading the primary image fails

In reply to Bart Van Assche

This patch fixes several Coverity complaints about reading data that has not been initialized.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_init.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/scsi/qla2xxx/qla_init.c +6 -2
index 5258d2486e25..a6a66b5d36a3 100644
@@ -7562,8 +7562,12 @@ qla27xx_get_active_image(struct scsi_qla_host *vha,
 		goto check_sec_image;
 	}
 
-	qla24xx_read_flash_data(vha, (void *)(&pri_image_status),
-	    ha->flt_region_img_status_pri, sizeof(pri_image_status) >> 2);
+	if (qla24xx_read_flash_data(vha, (void *)(&pri_image_status),
+	    ha->flt_region_img_status_pri, sizeof(pri_image_status) >> 2) !=
+	    QLA_SUCCESS) {
+		WARN_ON_ONCE(true);
+		goto check_sec_image;
+	}
 	qla27xx_print_image(vha, "Primary image", &pri_image_status);
 
 	if (qla27xx_check_image_status_signature(&pri_image_status)) {
Signature · 1 line
2.22.0
#40 lore raw

PATCH v2 40/58 qla2xxx: Make it explicit that ELS pass-through IOCBs use little endian

In reply to Bart Van Assche

According to the firmware documentation the firmware expects all ELS pass-through IOCB parameters in little endian format. Make this explicit.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_fw.h   | 8 ++++----
 drivers/scsi/qla2xxx/qla_iocb.c | 7 ++++---
 2 files changed, 8 insertions(+), 7 deletions(-)
drivers/scsi/qla2xxx/qla_fw.h +4 -4
index df079a8c2b33..732bb871c433 100644
@@ -761,13 +761,13 @@ struct els_entry_24xx {
 #define ECF_CLR_PASSTHRU_PEND	BIT_12
 #define ECF_INCL_FRAME_HDR	BIT_11
 
-	uint32_t rx_byte_count;
-	uint32_t tx_byte_count;
+	__le32	 rx_byte_count;
+	__le32	 tx_byte_count;
 
 	__le64	 tx_address __packed;	/* Data segment 0 address. */
-	uint32_t tx_len;		/* Data segment 0 length. */
+	__le32	 tx_len;		/* Data segment 0 length. */
 	__le64	 rx_address __packed;	/* Data segment 1 address. */
-	uint32_t rx_len;		/* Data segment 1 length. */
+	__le32	 rx_len;		/* Data segment 1 length. */
 };
 
 struct els_sts_entry_24xx {
drivers/scsi/qla2xxx/qla_iocb.c +4 -3
index 6b120254f414..c7b91827c1e7 100644
@@ -2704,12 +2704,12 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
 
 	if (elsio->u.els_logo.els_cmd == ELS_DCMD_PLOGI) {
 		els_iocb->tx_byte_count = els_iocb->tx_len =
-			sizeof(struct els_plogi_payload);
+			cpu_to_le32(sizeof(struct els_plogi_payload));
 		put_unaligned_le64(elsio->u.els_plogi.els_plogi_pyld_dma,
 				   &els_iocb->tx_address);
 		els_iocb->rx_dsd_count = 1;
 		els_iocb->rx_byte_count = els_iocb->rx_len =
-			sizeof(struct els_plogi_payload);
+			cpu_to_le32(sizeof(struct els_plogi_payload));
 		put_unaligned_le64(elsio->u.els_plogi.els_resp_pyld_dma,
 				   &els_iocb->rx_address);
 
@@ -2718,7 +2718,8 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
 		ql_dump_buffer(ql_log_info, vha, 0x0109,
 		    (uint8_t *)els_iocb, 0x70);
 	} else {
-		els_iocb->tx_byte_count = sizeof(struct els_logo_payload);
+		els_iocb->tx_byte_count =
+			cpu_to_le32(sizeof(struct els_logo_payload));
 		put_unaligned_le64(elsio->u.els_logo.els_logo_pyld_dma,
 				   &els_iocb->tx_address);
 		els_iocb->tx_len = cpu_to_le32(sizeof(struct els_logo_payload));
Signature · 1 line
2.22.0
#41 lore raw

PATCH v2 41/58 qla2xxx: Set the responder mode if appropriate for ELS pass-through IOCBs

In reply to Bart Van Assche

According to the firmware documentation responder mode must be set for ELS pass-through IOCBs if a response is expected.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_iocb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_iocb.c +2 -1
index c7b91827c1e7..2da7c92e320b 100644
@@ -2700,9 +2700,9 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
 	els_iocb->s_id[0] = vha->d_id.b.al_pa;
 	els_iocb->s_id[1] = vha->d_id.b.area;
 	els_iocb->s_id[2] = vha->d_id.b.domain;
-	els_iocb->control_flags = 0;
 
 	if (elsio->u.els_logo.els_cmd == ELS_DCMD_PLOGI) {
+		els_iocb->control_flags = 0;
 		els_iocb->tx_byte_count = els_iocb->tx_len =
 			cpu_to_le32(sizeof(struct els_plogi_payload));
 		put_unaligned_le64(elsio->u.els_plogi.els_plogi_pyld_dma,
@@ -2718,6 +2718,7 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
 		ql_dump_buffer(ql_log_info, vha, 0x0109,
 		    (uint8_t *)els_iocb, 0x70);
 	} else {
+		els_iocb->control_flags = 1 << 13;
 		els_iocb->tx_byte_count =
 			cpu_to_le32(sizeof(struct els_logo_payload));
 		put_unaligned_le64(elsio->u.els_logo.els_logo_pyld_dma,
Signature · 1 line
2.22.0
#42 lore raw

PATCH v2 42/58 qla2xxx: Rework key encoding in qlt_find_host_by_d_id()

In reply to Bart Van Assche

Use the same approach for encoding the destination ID as the approach used by qlt_update_vp_map().

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +2 -4
index 3dd897d3e400..f7b72d1d4862 100644
@@ -191,16 +191,14 @@ struct scsi_qla_host *qlt_find_host_by_d_id(struct scsi_qla_host *vha,
 					    be_id_t d_id)
 {
 	struct scsi_qla_host *host;
-	uint32_t key = 0;
+	uint32_t key;
 
 	if (vha->d_id.b.area == d_id.area &&
 	    vha->d_id.b.domain == d_id.domain &&
 	    vha->d_id.b.al_pa == d_id.al_pa)
 		return vha;
 
-	key  = d_id.domain << 16;
-	key |= d_id.area << 8;
-	key |= d_id.al_pa;
+	key = be_to_port_id(d_id).b24;
 
 	host = btree_lookup32(&vha->hw->tgt.host_map, key);
 	if (!host)
Signature · 1 line
2.22.0
#43 lore raw

PATCH v2 43/58 qla2xxx: Enable type checking for the SRB free and done callback functions

In reply to Bart Van Assche

Since all pointers passed to the srb_t.done() and srb_t.free() functions have type srb_t, change the type of the first argument of these functions from void * into struct srb *. This allows the compiler to verify the argument types for these functions. This patch does not change any functionality.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_bsg.c    |  8 ++-----
 drivers/scsi/qla2xxx/qla_def.h    | 14 +++++++++--
 drivers/scsi/qla2xxx/qla_gbl.h    | 14 +++++------
 drivers/scsi/qla2xxx/qla_gs.c     | 21 ++++++----------
 drivers/scsi/qla2xxx/qla_init.c   | 40 ++++++++-----------------------
 drivers/scsi/qla2xxx/qla_iocb.c   | 12 +++-------
 drivers/scsi/qla2xxx/qla_mbx.c    |  4 +---
 drivers/scsi/qla2xxx/qla_mid.c    |  4 +---
 drivers/scsi/qla2xxx/qla_mr.c     |  4 +---
 drivers/scsi/qla2xxx/qla_nvme.c   | 11 ++++-----
 drivers/scsi/qla2xxx/qla_nvme.h   |  2 +-
 drivers/scsi/qla2xxx/qla_os.c     | 16 ++++---------
 drivers/scsi/qla2xxx/qla_target.c |  4 +---
 13 files changed, 54 insertions(+), 100 deletions(-)
drivers/scsi/qla2xxx/qla_bsg.c +2 -6
index 240b07b0098a..28d587a89ba6 100644
@@ -12,10 +12,8 @@
 #include <linux/bsg-lib.h>
 
 /* BSG support for ELS/CT pass through */
-void
-qla2x00_bsg_job_done(void *ptr, int res)
+void qla2x00_bsg_job_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct bsg_job *bsg_job = sp->u.bsg_job;
 	struct fc_bsg_reply *bsg_reply = bsg_job->reply;
 
@@ -25,10 +23,8 @@ qla2x00_bsg_job_done(void *ptr, int res)
 	sp->free(sp);
 }
 
-void
-qla2x00_bsg_sp_free(void *ptr)
+void qla2x00_bsg_sp_free(srb_t *sp)
 {
-	srb_t *sp = ptr;
 	struct qla_hw_data *ha = sp->vha->hw;
 	struct bsg_job *bsg_job = sp->u.bsg_job;
 	struct fc_bsg_request *bsg_request = bsg_job->request;
drivers/scsi/qla2xxx/qla_def.h +12 -2
index 779bf3fcab0f..65d79bcb7ccf 100644
@@ -614,8 +614,18 @@ typedef struct srb {
 		struct bsg_job *bsg_job;
 		struct srb_cmd scmd;
 	} u;
-	void (*done)(void *, int);
-	void (*free)(void *);
+	/*
+	 * Report completion status @res and call sp_put(@sp). @res is
+	 * an NVMe status code, a SCSI result (e.g. DID_OK << 16) or a
+	 * QLA_* status value.
+	 */
+	void (*done)(struct srb *sp, int res);
+	/* Stop the timer and free @sp. Only used by the FCP code. */
+	void (*free)(struct srb *sp);
+	/*
+	 * Call nvme_private->fd->done() and free @sp. Only used by the NVMe
+	 * code.
+	 */
 	void (*put_fn)(struct kref *kref);
 } srb_t;
 
drivers/scsi/qla2xxx/qla_gbl.h +7 -7
index aac664da419b..bbfbe3a34a7e 100644
@@ -213,9 +213,9 @@ extern int qla2x00_post_uevent_work(struct scsi_qla_host *, u32);
 
 extern int qla2x00_post_uevent_work(struct scsi_qla_host *, u32);
 extern void qla2x00_disable_board_on_pci_error(struct work_struct *);
-extern void qla2x00_sp_compl(void *, int);
-extern void qla2xxx_qpair_sp_free_dma(void *);
-extern void qla2xxx_qpair_sp_compl(void *, int);
+extern void qla2x00_sp_compl(srb_t *sp, int);
+extern void qla2xxx_qpair_sp_free_dma(srb_t *sp);
+extern void qla2xxx_qpair_sp_compl(srb_t *sp, int);
 extern void qla24xx_sched_upd_fcport(fc_port_t *);
 void qla2x00_handle_login_done_event(struct scsi_qla_host *, fc_port_t *,
 	uint16_t *);
@@ -244,7 +244,7 @@ extern void qla2x00_do_dpc_all_vps(scsi_qla_host_t *);
 extern int qla24xx_vport_create_req_sanity_check(struct fc_vport *);
 extern scsi_qla_host_t *qla24xx_create_vhost(struct fc_vport *);
 
-extern void qla2x00_sp_free_dma(void *);
+extern void qla2x00_sp_free_dma(srb_t *sp);
 extern char *qla2x00_get_fw_version_str(struct scsi_qla_host *, char *);
 
 extern void qla2x00_mark_device_lost(scsi_qla_host_t *, fc_port_t *, int, int);
@@ -790,10 +790,10 @@ extern int qla82xx_restart_isp(scsi_qla_host_t *);
 
 /* IOCB related functions */
 extern int qla82xx_start_scsi(srb_t *);
-extern void qla2x00_sp_free(void *);
+extern void qla2x00_sp_free(srb_t *sp);
 extern void qla2x00_sp_timeout(struct timer_list *);
-extern void qla2x00_bsg_job_done(void *, int);
-extern void qla2x00_bsg_sp_free(void *);
+extern void qla2x00_bsg_job_done(srb_t *sp, int);
+extern void qla2x00_bsg_sp_free(srb_t *sp);
 extern void qla2x00_start_iocbs(struct scsi_qla_host *, struct req_que *);
 
 /* Interrupt related */
drivers/scsi/qla2xxx/qla_gs.c +7 -14
index 18117b5f32bc..35e1f36c9366 100644
@@ -499,9 +499,8 @@ qla2x00_gnn_id(scsi_qla_host_t *vha, sw_info_t *list)
 	return (rval);
 }
 
-static void qla2x00_async_sns_sp_done(void *s, int rc)
+static void qla2x00_async_sns_sp_done(srb_t *sp, int rc)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	struct ct_sns_pkt *ct_sns;
 	struct qla_work_evt *e;
@@ -2989,9 +2988,8 @@ void qla24xx_handle_gpsc_event(scsi_qla_host_t *vha, struct event_arg *ea)
 	qla_post_iidma_work(vha, fcport);
 }
 
-static void qla24xx_async_gpsc_sp_done(void *s, int res)
+static void qla24xx_async_gpsc_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	struct qla_hw_data *ha = vha->hw;
 	fc_port_t *fcport = sp->fcport;
@@ -3258,9 +3256,8 @@ void qla24xx_handle_gpnid_event(scsi_qla_host_t *vha, struct event_arg *ea)
 	}
 }
 
-static void qla2x00_async_gpnid_sp_done(void *s, int res)
+static void qla2x00_async_gpnid_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	struct ct_sns_req *ct_req =
 	    (struct ct_sns_req *)sp->u.iocb_cmd.u.ctarg.req;
@@ -3446,9 +3443,8 @@ void qla24xx_handle_gffid_event(scsi_qla_host_t *vha, struct event_arg *ea)
 	qla24xx_post_gnl_work(vha, fcport);
 }
 
-void qla24xx_async_gffid_sp_done(void *s, int res)
+void qla24xx_async_gffid_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	fc_port_t *fcport = sp->fcport;
 	struct ct_sns_rsp *ct_rsp;
@@ -3872,9 +3868,8 @@ static void qla2x00_find_free_fcp_nvme_slot(struct scsi_qla_host *vha,
 	}
 }
 
-static void qla2x00_async_gpnft_gnnft_sp_done(void *s, int res)
+static void qla2x00_async_gpnft_gnnft_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	struct ct_sns_req *ct_req =
 		(struct ct_sns_req *)sp->u.iocb_cmd.u.ctarg.req;
@@ -4251,9 +4246,8 @@ void qla24xx_handle_gnnid_event(scsi_qla_host_t *vha, struct event_arg *ea)
 	qla24xx_post_gnl_work(vha, ea->fcport);
 }
 
-static void qla2x00_async_gnnid_sp_done(void *s, int res)
+static void qla2x00_async_gnnid_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	fc_port_t *fcport = sp->fcport;
 	u8 *node_name = fcport->ct_desc.ct_sns->p.rsp.rsp.gnn_id.node_name;
@@ -4384,9 +4378,8 @@ void qla24xx_handle_gfpnid_event(scsi_qla_host_t *vha, struct event_arg *ea)
 	qla24xx_post_gpsc_work(vha, fcport);
 }
 
-static void qla2x00_async_gfpnid_sp_done(void *s, int res)
+static void qla2x00_async_gfpnid_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	fc_port_t *fcport = sp->fcport;
 	u8 *fpn = fcport->ct_desc.ct_sns->p.rsp.rsp.gfpn_id.port_name;
drivers/scsi/qla2xxx/qla_init.c +10 -30
index a6a66b5d36a3..3fa8ca63429c 100644
@@ -63,10 +63,8 @@ qla2x00_sp_timeout(struct timer_list *t)
 	iocb->timeout(sp);
 }
 
-void
-qla2x00_sp_free(void *ptr)
+void qla2x00_sp_free(srb_t *sp)
 {
-	srb_t *sp = ptr;
 	struct srb_iocb *iocb = &sp->u.iocb_cmd;
 
 	del_timer(&iocb->timer);
@@ -117,9 +115,8 @@ static void qla24xx_abort_iocb_timeout(void *data)
 	sp->done(sp, QLA_OS_TIMER_EXPIRED);
 }
 
-static void qla24xx_abort_sp_done(void *ptr, int res)
+static void qla24xx_abort_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct srb_iocb *abt = &sp->u.iocb_cmd;
 
 	del_timer(&sp->u.iocb_cmd.timer);
@@ -249,10 +246,8 @@ qla2x00_async_iocb_timeout(void *data)
 	}
 }
 
-static void
-qla2x00_async_login_sp_done(void *ptr, int res)
+static void qla2x00_async_login_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct scsi_qla_host *vha = sp->vha;
 	struct srb_iocb *lio = &sp->u.iocb_cmd;
 	struct event_arg ea;
@@ -358,11 +353,8 @@ qla2x00_async_login(struct scsi_qla_host *vha, fc_port_t *fcport,
 	return rval;
 }
 
-static void
-qla2x00_async_logout_sp_done(void *ptr, int res)
+static void qla2x00_async_logout_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
-
 	sp->fcport->flags &= ~(FCF_ASYNC_SENT | FCF_ASYNC_ACTIVE);
 	sp->fcport->login_gen++;
 	qlt_logo_completion_handler(sp->fcport, res);
@@ -419,10 +411,8 @@ qla2x00_async_prlo_done(struct scsi_qla_host *vha, fc_port_t *fcport,
 	qlt_logo_completion_handler(fcport, data[0]);
 }
 
-static void
-qla2x00_async_prlo_sp_done(void *s, int res)
+static void qla2x00_async_prlo_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = (srb_t *)s;
 	struct srb_iocb *lio = &sp->u.iocb_cmd;
 	struct scsi_qla_host *vha = sp->vha;
 
@@ -525,10 +515,8 @@ static int qla_post_els_plogi_work(struct scsi_qla_host *vha, fc_port_t *fcport)
 	return qla2x00_post_work(vha, e);
 }
 
-static void
-qla2x00_async_adisc_sp_done(void *ptr, int res)
+static void qla2x00_async_adisc_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct scsi_qla_host *vha = sp->vha;
 	struct event_arg ea;
 	struct srb_iocb *lio = &sp->u.iocb_cmd;
@@ -931,10 +919,8 @@ static void qla24xx_handle_gnl_done_event(scsi_qla_host_t *vha,
 	}
 } /* gnl_event */
 
-static void
-qla24xx_async_gnl_sp_done(void *s, int res)
+static void qla24xx_async_gnl_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	unsigned long flags;
 	struct fc_port *fcport = NULL, *tf;
@@ -1121,10 +1107,8 @@ int qla24xx_post_gnl_work(struct scsi_qla_host *vha, fc_port_t *fcport)
 	return qla2x00_post_work(vha, e);
 }
 
-static
-void qla24xx_async_gpdb_sp_done(void *s, int res)
+static void qla24xx_async_gpdb_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
 	struct scsi_qla_host *vha = sp->vha;
 	struct qla_hw_data *ha = vha->hw;
 	fc_port_t *fcport = sp->fcport;
@@ -1168,10 +1152,8 @@ static int qla24xx_post_prli_work(struct scsi_qla_host *vha, fc_port_t *fcport)
 	return qla2x00_post_work(vha, e);
 }
 
-static void
-qla2x00_async_prli_sp_done(void *ptr, int res)
+static void qla2x00_async_prli_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct scsi_qla_host *vha = sp->vha;
 	struct srb_iocb *lio = &sp->u.iocb_cmd;
 	struct event_arg ea;
@@ -1808,10 +1790,8 @@ qla2x00_tmf_iocb_timeout(void *data)
 	complete(&tmf->u.tmf.comp);
 }
 
-static void
-qla2x00_tmf_sp_done(void *ptr, int res)
+static void qla2x00_tmf_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct srb_iocb *tmf = &sp->u.iocb_cmd;
 
 	complete(&tmf->u.tmf.comp);
drivers/scsi/qla2xxx/qla_iocb.c +3 -9
index 2da7c92e320b..59a0a778d31c 100644
@@ -2544,10 +2544,8 @@ void qla2x00_init_timer(srb_t *sp, unsigned long tmo)
 	sp->start_timer = 1;
 }
 
-static void
-qla2x00_els_dcmd_sp_free(void *data)
+static void qla2x00_els_dcmd_sp_free(srb_t *sp)
 {
-	srb_t *sp = data;
 	struct srb_iocb *elsio = &sp->u.iocb_cmd;
 
 	kfree(sp->fcport);
@@ -2577,10 +2575,8 @@ qla2x00_els_dcmd_iocb_timeout(void *data)
 	complete(&lio->u.els_logo.comp);
 }
 
-static void
-qla2x00_els_dcmd_sp_done(void *ptr, int res)
+static void qla2x00_els_dcmd_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	fc_port_t *fcport = sp->fcport;
 	struct srb_iocb *lio = &sp->u.iocb_cmd;
 	struct scsi_qla_host *vha = sp->vha;
@@ -2758,10 +2754,8 @@ qla2x00_els_dcmd2_iocb_timeout(void *data)
 	sp->done(sp, QLA_FUNCTION_TIMEOUT);
 }
 
-static void
-qla2x00_els_dcmd2_sp_done(void *ptr, int res)
+static void qla2x00_els_dcmd2_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	fc_port_t *fcport = sp->fcport;
 	struct srb_iocb *lio = &sp->u.iocb_cmd;
 	struct scsi_qla_host *vha = sp->vha;
drivers/scsi/qla2xxx/qla_mbx.c +1 -3
index 783a84606047..a82b6db2fa9d 100644
@@ -6217,10 +6217,8 @@ qla26xx_dport_diagnostics(scsi_qla_host_t *vha,
 	return rval;
 }
 
-static void qla2x00_async_mb_sp_done(void *s, int res)
+static void qla2x00_async_mb_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
-
 	sp->u.iocb_cmd.u.mbx.rc = res;
 
 	complete(&sp->u.iocb_cmd.u.mbx.comp);
drivers/scsi/qla2xxx/qla_mid.c +1 -3
index b2977e49356b..1a9a11ae7285 100644
@@ -901,10 +901,8 @@ qla25xx_create_rsp_que(struct qla_hw_data *ha, uint16_t options,
 	return 0;
 }
 
-static void qla_ctrlvp_sp_done(void *s, int res)
+static void qla_ctrlvp_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = s;
-
 	if (sp->comp)
 		complete(sp->comp);
 	/* don't free sp here. Let the caller do the free */
drivers/scsi/qla2xxx/qla_mr.c +1 -3
index 78b3679e1b9c..e8da3ec4db2c 100644
@@ -1797,10 +1797,8 @@ qla2x00_fxdisc_iocb_timeout(void *data)
 	complete(&lio->u.fxiocb.fxiocb_comp);
 }
 
-static void
-qla2x00_fxdisc_sp_done(void *ptr, int res)
+static void qla2x00_fxdisc_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct srb_iocb *lio = &sp->u.iocb_cmd;
 
 	complete(&lio->u.fxiocb.fxiocb_comp);
drivers/scsi/qla2xxx/qla_nvme.c +4 -7
index bba25c38a118..af6b46777602 100644
@@ -180,10 +180,9 @@ static void qla_nvme_ls_complete(struct work_struct *work)
 	kref_put(&priv->sp->cmd_kref, qla_nvme_release_ls_cmd_kref);
 }
 
-static void qla_nvme_sp_ls_done(void *ptr, int res)
+static void qla_nvme_sp_ls_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
-	struct nvme_private *priv;
+	struct nvme_private *priv = sp->priv;
 
 	if (WARN_ON_ONCE(kref_read(&sp->cmd_kref) == 0))
 		return;
@@ -191,17 +190,15 @@ static void qla_nvme_sp_ls_done(void *ptr, int res)
 	if (res)
 		res = -EINVAL;
 
-	priv = (struct nvme_private *)sp->priv;
 	priv->comp_status = res;
 	INIT_WORK(&priv->ls_work, qla_nvme_ls_complete);
 	schedule_work(&priv->ls_work);
 }
 
 /* it assumed that QPair lock is held. */
-static void qla_nvme_sp_done(void *ptr, int res)
+static void qla_nvme_sp_done(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
-	struct nvme_private *priv = (struct nvme_private *)sp->priv;
+	struct nvme_private *priv = sp->priv;
 
 	priv->comp_status = res;
 	kref_put(&sp->cmd_kref, qla_nvme_release_fcp_cmd_kref);
drivers/scsi/qla2xxx/qla_nvme.h +1 -1
index 68a8d09a36ef..25a2b82d5095 100644
@@ -144,5 +144,5 @@ int  qla_nvme_register_remote(struct scsi_qla_host *, struct fc_port *);
 void qla_nvme_delete(struct scsi_qla_host *);
 void qla24xx_nvme_ls4_iocb(struct scsi_qla_host *, struct pt_ls4_request *,
     struct req_que *);
-void qla24xx_async_gffid_sp_done(void *, int);
+void qla24xx_async_gffid_sp_done(struct srb *sp, int);
 #endif
drivers/scsi/qla2xxx/qla_os.c +4 -12
index a247dce0cb95..9ef59995f5d6 100644
@@ -652,10 +652,8 @@ qla24xx_fw_version_str(struct scsi_qla_host *vha, char *str, size_t size)
 	return str;
 }
 
-void
-qla2x00_sp_free_dma(void *ptr)
+void qla2x00_sp_free_dma(srb_t *sp)
 {
-	srb_t *sp = ptr;
 	struct qla_hw_data *ha = sp->vha->hw;
 	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
 	void *ctx = GET_CMD_CTX_SP(sp);
@@ -699,10 +697,8 @@ qla2x00_sp_free_dma(void *ptr)
 	}
 }
 
-void
-qla2x00_sp_compl(void *ptr, int res)
+void qla2x00_sp_compl(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
 	struct completion *comp = sp->comp;
 
@@ -720,10 +716,8 @@ qla2x00_sp_compl(void *ptr, int res)
 	qla2x00_rel_sp(sp);
 }
 
-void
-qla2xxx_qpair_sp_free_dma(void *ptr)
+void qla2xxx_qpair_sp_free_dma(srb_t *sp)
 {
-	srb_t *sp = (srb_t *)ptr;
 	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
 	struct qla_hw_data *ha = sp->fcport->vha->hw;
 	void *ctx = GET_CMD_CTX_SP(sp);
@@ -804,10 +798,8 @@ qla2xxx_qpair_sp_free_dma(void *ptr)
 	}
 }
 
-void
-qla2xxx_qpair_sp_compl(void *ptr, int res)
+void qla2xxx_qpair_sp_compl(srb_t *sp, int res)
 {
-	srb_t *sp = ptr;
 	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
 	struct completion *comp = sp->comp;
 
drivers/scsi/qla2xxx/qla_target.c +1 -3
index f7b72d1d4862..d25c3fa43601 100644
@@ -559,10 +559,8 @@ static int qla24xx_post_nack_work(struct scsi_qla_host *vha, fc_port_t *fcport,
 	return qla2x00_post_work(vha, e);
 }
 
-static
-void qla2x00_async_nack_sp_done(void *s, int res)
+static void qla2x00_async_nack_sp_done(srb_t *sp, int res)
 {
-	struct srb *sp = (struct srb *)s;
 	struct scsi_qla_host *vha = sp->vha;
 	unsigned long flags;
 
Signature · 1 line
2.22.0
#44 lore raw

PATCH v2 44/58 qla2xxx: Introduce the function qla2xxx_init_sp()

In reply to Bart Van Assche

This patch does not change any functionality but makes the next patch easier to read.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_inline.h | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)
drivers/scsi/qla2xxx/qla_inline.h +15 -13
index bf063c664352..0c3d907af769 100644
@@ -152,6 +152,18 @@ qla2x00_chip_is_down(scsi_qla_host_t *vha)
 	return (qla2x00_reset_active(vha) || !vha->hw->flags.fw_started);
 }
 
+static void qla2xxx_init_sp(srb_t *sp, scsi_qla_host_t *vha,
+			    struct qla_qpair *qpair, fc_port_t *fcport)
+{
+	memset(sp, 0, sizeof(*sp));
+	sp->fcport = fcport;
+	sp->iocbs = 1;
+	sp->vha = vha;
+	sp->qpair = qpair;
+	sp->cmd_type = TYPE_SRB;
+	INIT_LIST_HEAD(&sp->elem);
+}
+
 static inline srb_t *
 qla2xxx_get_qpair_sp(scsi_qla_host_t *vha, struct qla_qpair *qpair,
     fc_port_t *fcport, gfp_t flag)
@@ -164,19 +176,9 @@ qla2xxx_get_qpair_sp(scsi_qla_host_t *vha, struct qla_qpair *qpair,
 		return NULL;
 
 	sp = mempool_alloc(qpair->srb_mempool, flag);
-	if (!sp)
-		goto done;
-
-	memset(sp, 0, sizeof(*sp));
-	sp->fcport = fcport;
-	sp->iocbs = 1;
-	sp->vha = vha;
-	sp->qpair = qpair;
-	sp->cmd_type = TYPE_SRB;
-	INIT_LIST_HEAD(&sp->elem);
-
-done:
-	if (!sp)
+	if (sp)
+		qla2xxx_init_sp(sp, vha, qpair, fcport);
+	else
 		QLA_QPAIR_MARK_NOT_BUSY(qpair);
 	return sp;
 }
Signature · 1 line
2.22.0
#45 lore raw

PATCH v2 45/58 qla2xxx: Fix a race condition between aborting and completing a SCSI command

In reply to Bart Van Assche

Instead of allocating a struct srb dynamically from inside .queuecommand(), set qla2xxx_driver_template.cmd_size such that struct scsi_cmnd and struct srb are contiguous. Do not call QLA_QPAIR_MARK_BUSY() / QLA_QPAIR_MARK_NOT_BUSY() for SRBs associated with SCSI commands. That is safe because scsi_remove_host() is called before queue pairs are deleted and scsi_remove_host() waits for all outstanding SCSI commands to finish.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_def.h |  1 -
 drivers/scsi/qla2xxx/qla_os.c  | 45 ++++++----------------------------
 2 files changed, 8 insertions(+), 38 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +0 -1
index 65d79bcb7ccf..3ffe7661a25b 100644
@@ -630,7 +630,6 @@ typedef struct srb {
 } srb_t;
 
 #define GET_CMD_SP(sp) (sp->u.scmd.cmd)
-#define SET_CMD_SP(sp, cmd) (sp->u.scmd.cmd = cmd)
 #define GET_CMD_CTX_SP(sp) (sp->u.scmd.ctx)
 
 #define GET_CMD_SENSE_LEN(sp) \
drivers/scsi/qla2xxx/qla_os.c +8 -37

106 lines not on this page · raw message

Signature · 1 line
2.22.0
#46 lore raw

PATCH v2 46/58 qla2xxx: Make qlt_handle_abts_completion() more robust

In reply to Bart Van Assche

Avoid that this function crashes if mcmd == NULL.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_target.c +1 -1
index d25c3fa43601..cc0c99b5f3fb 100644
@@ -5731,7 +5731,7 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
 			    entry->error_subcode2);
 			ha->tgt.tgt_ops->free_mcmd(mcmd);
 		}
-	} else {
+	} else if (mcmd) {
 		ha->tgt.tgt_ops->free_mcmd(mcmd);
 	}
 }
Signature · 1 line
2.22.0
#47 lore raw

Re: [PATCH v2 46/58] qla2xxx: Make qlt_handle_abts_completion() more robust

Quoted 24 lines · Bart Van Assche

Avoid that this function crashes if mcmd == NULL.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_target.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_target.c +1 -1
index d25c3fa43601..cc0c99b5f3fb 100644
@@ -5731,7 +5731,7 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
 			    entry->error_subcode2);
 			ha->tgt.tgt_ops->free_mcmd(mcmd);
 		}
-	} else {
+	} else if (mcmd) {
 		ha->tgt.tgt_ops->free_mcmd(mcmd);
 	}
 }
Signature · 1 line
2.22.0

Thanks for working on the fix, the crash can be observed sometimes on target shutdown.

I've been inspecting the piece of code multiple times and still don't understand if we get mcmd == NULL only when ABTS completes successfully or there is ABTS failure together with inability to find mcmd in the request queue? In that case, there're two more paths that could crash.

And the second question is whether the NULL received from qlt_ctio_to_cmd is a sign of another sporadic issue somewhere else in the driver?

Best regards,
Roman

#48 lore raw

Re: [PATCH v2 46/58] qla2xxx: Make qlt_handle_abts_completion() more robust

Quoted 29 lines · Roman Bolshakov
Quoted 24 lines · Bart Van Assche

Avoid that this function crashes if mcmd == NULL.

Cc Himanshu Madhani <[email protected]>
---
  drivers/scsi/qla2xxx/qla_target.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_target.c +1 -1
index d25c3fa43601..cc0c99b5f3fb 100644
@@ -5731,7 +5731,7 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
  			    entry->error_subcode2);
  			ha->tgt.tgt_ops->free_mcmd(mcmd);
  		}
-	} else {
+	} else if (mcmd) {
  		ha->tgt.tgt_ops->free_mcmd(mcmd);
  	}
  }
Signature · 1 line
2.22.0

Thanks for working on the fix, the crash can be observed sometimes on target shutdown.

Quoted 8 lines

I've been inspecting the piece of code multiple times and still don't understand if we get mcmd == NULL only when ABTS completes successfully or there is ABTS failure together with inability to find mcmd in the request queue? In that case, there're two more paths that could crash.

And the second question is whether the NULL received from qlt_ctio_to_cmd is a sign of another sporadic issue somewhere else in the driver?

Hi Roman,

If I interpret qlt_handle_abts_completion() mcmd can only be NULL at line 5734 if h == QLA_TGT_SKIP_HANDLE. I'm not sure what causes the firmware to report that handle value upon ABTS completion.

Bart.

#49 lore raw

PATCH v2 47/58 qla2xxx: Modify NVMe include directives

In reply to Bart Van Assche

Since struct sg_table is used in nvme-fc-driver.h, include <linux/scatterlist.h> from that header file.

Since no definitions or declarations from <linux/blk-mq.h> are used in the qla_nvme.h header file, do not include <linux/blk-mq.h> from that header file.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_nvme.h | 1 -
 include/linux/nvme-fc-driver.h  | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_nvme.h +0 -1
index 25a2b82d5095..ef912902d4e5 100644
@@ -7,7 +7,6 @@
 #ifndef __QLA_NVME_H
 #define __QLA_NVME_H
 
-#include <linux/blk-mq.h>
 #include <uapi/scsi/fc/fc_fs.h>
 #include <uapi/scsi/fc/fc_els.h>
 #include <linux/nvme-fc-driver.h>
include/linux/nvme-fc-driver.h +2 -0
index 98d904961b33..10f81629b9ce 100644
@@ -6,6 +6,8 @@
 #ifndef _NVME_FC_DRIVER_H
 #define _NVME_FC_DRIVER_H 1
 
+#include <linux/scatterlist.h>
+
 
 /*
  * **********************  LLDD FC-NVME Host API ********************
Signature · 1 line
2.22.0
#50 lore raw

PATCH v2 48/58 qla2xxx: Introduce qla2xxx_get_next_handle()

In reply to Bart Van Assche

This patch reduces code duplication.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_gbl.h  |   1 +
 drivers/scsi/qla2xxx/qla_iocb.c | 128 +++++++++-----------------------
 drivers/scsi/qla2xxx/qla_mr.c   |  13 +---
 drivers/scsi/qla2xxx/qla_nvme.c |  14 +---
 4 files changed, 42 insertions(+), 114 deletions(-)
drivers/scsi/qla2xxx/qla_gbl.h +1 -0
index bbfbe3a34a7e..6b914942fad3 100644
@@ -272,6 +272,7 @@ extern void qla2x00_build_scsi_iocbs_32(srb_t *, cmd_entry_t *, uint16_t);
 extern void qla2x00_build_scsi_iocbs_64(srb_t *, cmd_entry_t *, uint16_t);
 extern void qla24xx_build_scsi_iocbs(srb_t *, struct cmd_type_7 *,
 	uint16_t, struct req_que *);
+extern uint32_t qla2xxx_get_next_handle(struct req_que *req);
 extern int qla2x00_start_scsi(srb_t *sp);
 extern int qla24xx_start_scsi(srb_t *sp);
 int qla2x00_marker(struct scsi_qla_host *, struct qla_qpair *,
drivers/scsi/qla2xxx/qla_iocb.c +37 -91

230 lines not on this page · raw message

drivers/scsi/qla2xxx/qla_mr.c +2 -11
index e8da3ec4db2c..06985b2d48eb 100644
@@ -3071,7 +3071,6 @@ qlafx00_start_scsi(srb_t *sp)
 {
 	int		nseg;
 	unsigned long   flags;
-	uint32_t        index;
 	uint32_t	handle;
 	uint16_t	cnt;
 	uint16_t	req_cnt;
@@ -3095,16 +3094,8 @@ qlafx00_start_scsi(srb_t *sp)
 	/* Acquire ring specific lock */
 	spin_lock_irqsave(&ha->hardware_lock, flags);
 
-	/* Check for room in outstanding command list. */
-	handle = req->current_outstanding_cmd;
-	for (index = 1; index < req->num_outstanding_cmds; index++) {
-		handle++;
-		if (handle == req->num_outstanding_cmds)
-			handle = 1;
-		if (!req->outstanding_cmds[handle])
-			break;
-	}
-	if (index == req->num_outstanding_cmds)
+	handle = qla2xxx_get_next_handle(req);
+	if (handle == 0)
 		goto queuing_error;
 
 	/* Map the sg table so we have an accurate count of sg entries needed */
drivers/scsi/qla2xxx/qla_nvme.c +2 -12
index af6b46777602..6cc19e060afc 100644
@@ -353,7 +353,6 @@ static inline int qla2x00_start_nvme_mq(srb_t *sp)
 {
 	unsigned long   flags;
 	uint32_t        *clr_ptr;
-	uint32_t        index;
 	uint32_t        handle;
 	struct cmd_nvme *cmd_pkt;
 	uint16_t        cnt, i;
@@ -377,17 +376,8 @@ static inline int qla2x00_start_nvme_mq(srb_t *sp)
 	/* Acquire qpair specific lock */
 	spin_lock_irqsave(&qpair->qp_lock, flags);
 
-	/* Check for room in outstanding command list. */
-	handle = req->current_outstanding_cmd;
-	for (index = 1; index < req->num_outstanding_cmds; index++) {
-		handle++;
-		if (handle == req->num_outstanding_cmds)
-			handle = 1;
-		if (!req->outstanding_cmds[handle])
-			break;
-	}
-
-	if (index == req->num_outstanding_cmds) {
+	handle = qla2xxx_get_next_handle(req);
+	if (handle == 0) {
 		rval = -EBUSY;
 		goto queuing_error;
 	}
Signature · 1 line
2.22.0
#51 lore raw

PATCH v2 49/58 qla2xxx: Make sure that aborted commands are freed

In reply to Bart Van Assche

The LIO core requires that the target driver callback functions .queue_data_in() and .queue_status() call target_put_sess_cmd() or transport_generic_free_cmd(). These calls may happen synchronously or asynchronously. Make sure that one of these LIO functions is called in case a command has been aborted. This patch avoids that the code for removing a session hangs due to commands that do not make progress.

Cc Himanshu Madhani <[email protected]>
Fixes 694833ee00c4 ("scsi: tcm_qla2xxx: Do not allow aborted cmd to advance.") # v4.13.
Fixes a07100e00ac4 ("qla2xxx: Fix TMR ABORT interaction issue between qla2xxx and TCM") # v4.5.
---
 drivers/scsi/qla2xxx/qla_target.c  | 13 ++++++++-----
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |  4 ++++
 2 files changed, 12 insertions(+), 5 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +8 -5
index cc0c99b5f3fb..0ffda6171614 100644
@@ -3206,7 +3206,8 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
 	if (!qpair->fw_started || (cmd->reset_count != qpair->chip_reset) ||
 	    (cmd->sess && cmd->sess->deleted)) {
 		cmd->state = QLA_TGT_STATE_PROCESSED;
-		return 0;
+		res = 0;
+		goto free;
 	}
 
 	ql_dbg_qp(ql_dbg_tgt, qpair, 0xe018,
@@ -3217,9 +3218,8 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
 
 	res = qlt_pre_xmit_response(cmd, &prm, xmit_type, scsi_status,
 	    &full_req_cnt);
-	if (unlikely(res != 0)) {
-		return res;
-	}
+	if (unlikely(res != 0))
+		goto free;
 
 	spin_lock_irqsave(qpair->qp_lock_ptr, flags);
 
@@ -3239,7 +3239,8 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
 			vha->flags.online, qla2x00_reset_active(vha),
 			cmd->reset_count, qpair->chip_reset);
 		spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
-		return 0;
+		res = 0;
+		goto free;
 	}
 
 	/* Does F/W have an IOCBs for this request */
@@ -3342,6 +3343,8 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
 	qlt_unmap_sg(vha, cmd);
 	spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
 
+free:
+	vha->hw->tgt.tgt_ops->free_cmd(cmd);
 	return res;
 }
 EXPORT_SYMBOL(qlt_xmit_response);
drivers/scsi/qla2xxx/tcm_qla2xxx.c +4 -0
index 963c220f8ba8..042a24314edc 100644
@@ -620,6 +620,7 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
 {
 	struct qla_tgt_cmd *cmd = container_of(se_cmd,
 				struct qla_tgt_cmd, se_cmd);
+	struct scsi_qla_host *vha = cmd->vha;
 
 	if (cmd->aborted) {
 		/* Cmd can loop during Q-full.  tcm_qla2xxx_aborted_task
@@ -632,6 +633,7 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
 			cmd->se_cmd.transport_state,
 			cmd->se_cmd.t_state,
 			cmd->se_cmd.se_cmd_flags);
+		vha->hw->tgt.tgt_ops->free_cmd(cmd);
 		return 0;
 	}
 
@@ -659,6 +661,7 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
 {
 	struct qla_tgt_cmd *cmd = container_of(se_cmd,
 				struct qla_tgt_cmd, se_cmd);
+	struct scsi_qla_host *vha = cmd->vha;
 	int xmit_type = QLA_TGT_XMIT_STATUS;
 
 	if (cmd->aborted) {
@@ -672,6 +675,7 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
 		    cmd, kref_read(&cmd->se_cmd.cmd_kref),
 		    cmd->se_cmd.transport_state, cmd->se_cmd.t_state,
 		    cmd->se_cmd.se_cmd_flags);
+		vha->hw->tgt.tgt_ops->free_cmd(cmd);
 		return 0;
 	}
 	cmd->bufflen = se_cmd->data_length;
Signature · 1 line
2.22.0
#52 lore raw

PATCH v2 50/58 qla2xxx: Complain if sp->done() is not called from the completion path

In reply to Bart Van Assche

Not calling sp->done() from the command completion path is a severe bug. Hence complain loudly if that happens.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_init.c | 4 ++++
 drivers/scsi/qla2xxx/qla_isr.c  | 4 ++++
 drivers/scsi/qla2xxx/qla_mr.c   | 4 ++++
 3 files changed, 12 insertions(+)
drivers/scsi/qla2xxx/qla_init.c +4 -0
index 3fa8ca63429c..64c84e53011e 100644
@@ -243,6 +243,10 @@ qla2x00_async_iocb_timeout(void *data)
 			sp->done(sp, QLA_FUNCTION_TIMEOUT);
 		}
 		break;
+	default:
+		WARN_ON_ONCE(true);
+		sp->done(sp, QLA_FUNCTION_TIMEOUT);
+		break;
 	}
 }
 
drivers/scsi/qla2xxx/qla_isr.c +4 -0
index 55eb51539cb0..7533e420e571 100644
@@ -2786,6 +2786,8 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
 
 	if (rsp->status_srb == NULL)
 		sp->done(sp, res);
+	else
+		WARN_ON_ONCE(true);
 }
 
 /**
@@ -2843,6 +2845,8 @@ qla2x00_status_cont_entry(struct rsp_que *rsp, sts_cont_entry_t *pkt)
 	if (sense_len == 0) {
 		rsp->status_srb = NULL;
 		sp->done(sp, cp->result);
+	} else {
+		WARN_ON_ONCE(true);
 	}
 }
 
drivers/scsi/qla2xxx/qla_mr.c +4 -0
index 06985b2d48eb..605b59c76c90 100644
@@ -2539,6 +2539,8 @@ qlafx00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
 
 	if (rsp->status_srb == NULL)
 		sp->done(sp, res);
+	else
+		WARN_ON_ONCE(true);
 }
 
 /**
@@ -2616,6 +2618,8 @@ qlafx00_status_cont_entry(struct rsp_que *rsp, sts_cont_entry_t *pkt)
 	if (sense_len == 0) {
 		rsp->status_srb = NULL;
 		sp->done(sp, cp->result);
+	} else {
+		WARN_ON_ONCE(true);
 	}
 }
 
Signature · 1 line
2.22.0
#53 lore raw

PATCH v2 51/58 qla2xxx: Let the compiler check the type of the SCSI command context pointer

In reply to Bart Van Assche

Split srb_cmd.ctx into two pointers such that the compiler can check the type of that pointer.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_def.h  |  4 ++--
 drivers/scsi/qla2xxx/qla_iocb.c | 20 +++++++++-----------
 drivers/scsi/qla2xxx/qla_nx.c   |  2 +-
 drivers/scsi/qla2xxx/qla_os.c   | 24 ++++++++----------------
 4 files changed, 20 insertions(+), 30 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +2 -2
index 3ffe7661a25b..527b2a2708a1 100644
@@ -317,7 +317,8 @@ struct srb_cmd {
 	uint32_t request_sense_length;
 	uint32_t fw_sense_length;
 	uint8_t *request_sense_ptr;
-	void *ctx;
+	struct ct6_dsd *ct6_ctx;
+	struct crc_context *crc_ctx;
 };
 
 /*
@@ -630,7 +631,6 @@ typedef struct srb {
 } srb_t;
 
 #define GET_CMD_SP(sp) (sp->u.scmd.cmd)
-#define GET_CMD_CTX_SP(sp) (sp->u.scmd.ctx)
 
 #define GET_CMD_SENSE_LEN(sp) \
 	(sp->u.scmd.request_sense_length)
drivers/scsi/qla2xxx/qla_iocb.c +9 -11
index 22d875222321..6acf92d19951 100644
@@ -621,7 +621,7 @@ qla24xx_build_scsi_type_6_iocbs(srb_t *sp, struct cmd_type_6 *cmd_pkt,
 	}
 
 	cur_seg = scsi_sglist(cmd);
-	ctx = GET_CMD_CTX_SP(sp);
+	ctx = sp->u.scmd.ct6_ctx;
 
 	while (tot_dsds) {
 		avail_dsds = (tot_dsds > QLA_DSDS_PER_IOCB) ?
@@ -954,8 +954,7 @@ qla24xx_walk_and_build_sglist_no_difb(struct qla_hw_data *ha, srb_t *sp,
 
 			if (sp) {
 				list_add_tail(&dsd_ptr->list,
-				    &((struct crc_context *)
-					    sp->u.scmd.ctx)->dsd_list);
+					      &sp->u.scmd.crc_ctx->dsd_list);
 
 				sp->flags |= SRB_CRC_CTX_DSD_VALID;
 			} else {
@@ -1052,8 +1051,7 @@ qla24xx_walk_and_build_sglist(struct qla_hw_data *ha, srb_t *sp,
 
 			if (sp) {
 				list_add_tail(&dsd_ptr->list,
-				    &((struct crc_context *)
-					    sp->u.scmd.ctx)->dsd_list);
+					      &sp->u.scmd.crc_ctx->dsd_list);
 
 				sp->flags |= SRB_CRC_CTX_DSD_VALID;
 			} else {
@@ -1099,7 +1097,7 @@ qla24xx_walk_and_build_prot_sglist(struct qla_hw_data *ha, srb_t *sp,
 
 		sgl = scsi_prot_sglist(cmd);
 		vha = sp->vha;
-		difctx = sp->u.scmd.ctx;
+		difctx = sp->u.scmd.crc_ctx;
 		direction_to_device = cmd->sc_data_direction == DMA_TO_DEVICE;
 		ql_dbg(ql_dbg_tgt + ql_dbg_verbose, vha, 0xe021,
 		  "%s: scsi_cmnd: %p, crc_ctx: %p, sp: %p\n",
@@ -1439,7 +1437,7 @@ qla24xx_build_scsi_crc_2_iocbs(srb_t *sp, struct cmd_type_crc_2 *cmd_pkt,
 		bundling = 0;
 
 	/* Allocate CRC context from global pool */
-	crc_ctx_pkt = sp->u.scmd.ctx =
+	crc_ctx_pkt = sp->u.scmd.crc_ctx =
 	    dma_pool_zalloc(ha->dl_dma_pool, GFP_ATOMIC, &crc_ctx_dma);
 
 	if (!crc_ctx_pkt)
@@ -3188,7 +3186,7 @@ qla82xx_start_scsi(srb_t *sp)
 				goto queuing_error;
 		}
 
-		ctx = sp->u.scmd.ctx =
+		ctx = sp->u.scmd.ct6_ctx =
 		    mempool_alloc(ha->ctx_mempool, GFP_ATOMIC);
 		if (!ctx) {
 			ql_log(ql_log_fatal, vha, 0x3010,
@@ -3384,9 +3382,9 @@ qla82xx_start_scsi(srb_t *sp)
 	if (tot_dsds)
 		scsi_dma_unmap(cmd);
 
-	if (sp->u.scmd.ctx) {
-		mempool_free(sp->u.scmd.ctx, ha->ctx_mempool);
-		sp->u.scmd.ctx = NULL;
+	if (sp->u.scmd.crc_ctx) {
+		mempool_free(sp->u.scmd.crc_ctx, ha->ctx_mempool);
+		sp->u.scmd.crc_ctx = NULL;
 	}
 	spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
drivers/scsi/qla2xxx/qla_nx.c +1 -1
index a91d426add75..372355bfcbb6 100644
@@ -3686,7 +3686,7 @@ qla82xx_chip_reset_cleanup(scsi_qla_host_t *vha)
 			for (cnt = 1; cnt < req->num_outstanding_cmds; cnt++) {
 				sp = req->outstanding_cmds[cnt];
 				if (sp) {
-					if ((!sp->u.scmd.ctx ||
+					if ((!sp->u.scmd.crc_ctx ||
 					    (sp->flags &
 						SRB_FCP_CMND_DMA_VALID)) &&
 						!ha->flags.isp82xx_fw_hung) {
drivers/scsi/qla2xxx/qla_os.c +8 -16
index 5ca7f7913258..5cb0bb65ace3 100644
@@ -656,7 +656,6 @@ void qla2x00_sp_free_dma(srb_t *sp)
 {
 	struct qla_hw_data *ha = sp->vha->hw;
 	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
-	void *ctx = GET_CMD_CTX_SP(sp);
 
 	if (sp->flags & SRB_DMA_VALID) {
 		scsi_dma_unmap(cmd);
@@ -669,24 +668,21 @@ void qla2x00_sp_free_dma(srb_t *sp)
 		sp->flags &= ~SRB_CRC_PROT_DMA_VALID;
 	}
 
-	if (!ctx)
-		return;
-
 	if (sp->flags & SRB_CRC_CTX_DSD_VALID) {
 		/* List assured to be having elements */
-		qla2x00_clean_dsd_pool(ha, ctx);
+		qla2x00_clean_dsd_pool(ha, sp->u.scmd.crc_ctx);
 		sp->flags &= ~SRB_CRC_CTX_DSD_VALID;
 	}
 
 	if (sp->flags & SRB_CRC_CTX_DMA_VALID) {
-		struct crc_context *ctx0 = ctx;
+		struct crc_context *ctx0 = sp->u.scmd.crc_ctx;
 
 		dma_pool_free(ha->dl_dma_pool, ctx0, ctx0->crc_ctx_dma);
 		sp->flags &= ~SRB_CRC_CTX_DMA_VALID;
 	}
 
 	if (sp->flags & SRB_FCP_CMND_DMA_VALID) {
-		struct ct6_dsd *ctx1 = ctx;
+		struct ct6_dsd *ctx1 = sp->u.scmd.ct6_ctx;
 
 		dma_pool_free(ha->fcp_cmnd_dma_pool, ctx1->fcp_cmnd,
 		    ctx1->fcp_cmnd_dma);
@@ -719,7 +715,6 @@ void qla2xxx_qpair_sp_free_dma(srb_t *sp)
 {
 	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
 	struct qla_hw_data *ha = sp->fcport->vha->hw;
-	void *ctx = GET_CMD_CTX_SP(sp);
 
 	if (sp->flags & SRB_DMA_VALID) {
 		scsi_dma_unmap(cmd);
@@ -732,17 +727,14 @@ void qla2xxx_qpair_sp_free_dma(srb_t *sp)
 		sp->flags &= ~SRB_CRC_PROT_DMA_VALID;
 	}
 
-	if (!ctx)
-		return;
-
 	if (sp->flags & SRB_CRC_CTX_DSD_VALID) {
 		/* List assured to be having elements */
-		qla2x00_clean_dsd_pool(ha, ctx);
+		qla2x00_clean_dsd_pool(ha, sp->u.scmd.crc_ctx);
 		sp->flags &= ~SRB_CRC_CTX_DSD_VALID;
 	}
 
 	if (sp->flags & SRB_DIF_BUNDL_DMA_VALID) {
-		struct crc_context *difctx = ctx;
+		struct crc_context *difctx = sp->u.scmd.crc_ctx;
 		struct dsd_dma *dif_dsd, *nxt_dsd;
 
 		list_for_each_entry_safe(dif_dsd, nxt_dsd,
@@ -778,7 +770,7 @@ void qla2xxx_qpair_sp_free_dma(srb_t *sp)
 	}
 
 	if (sp->flags & SRB_FCP_CMND_DMA_VALID) {
-		struct ct6_dsd *ctx1 = ctx;
+		struct ct6_dsd *ctx1 = sp->u.scmd.ct6_ctx;
 
 		dma_pool_free(ha->fcp_cmnd_dma_pool, ctx1->fcp_cmnd,
 		    ctx1->fcp_cmnd_dma);
@@ -790,9 +782,9 @@ void qla2xxx_qpair_sp_free_dma(srb_t *sp)
 	}
 
 	if (sp->flags & SRB_CRC_CTX_DMA_VALID) {
-		struct crc_context *ctx0 = ctx;
+		struct crc_context *ctx0 = sp->u.scmd.crc_ctx;
 
-		dma_pool_free(ha->dl_dma_pool, ctx, ctx0->crc_ctx_dma);
+		dma_pool_free(ha->dl_dma_pool, ctx0, ctx0->crc_ctx_dma);
 		sp->flags &= ~SRB_CRC_CTX_DMA_VALID;
 	}
 }
Signature · 1 line
2.22.0
#54 lore raw

PATCH v2 52/58 qla2xxx: Remove superfluous sts_entry_* casts

In reply to Bart Van Assche

The C language supports implicit casting of void pointers to non-void pointers. Remove explicit sts_entry_* casts that are not necessary.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_isr.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)
drivers/scsi/qla2xxx/qla_isr.c +5 -10
index 7533e420e571..5c65f2e67448 100644
@@ -1513,7 +1513,7 @@ qla2x00_ct_entry(scsi_qla_host_t *vha, struct req_que *req,
 		    if (comp_status == CS_DATA_UNDERRUN) {
 			    res = DID_OK << 16;
 			    bsg_reply->reply_payload_rcv_len =
-				le16_to_cpu(((sts_entry_t *)pkt)->rsp_info_len);
+				le16_to_cpu(pkt->rsp_info_len);
 
 			    ql_log(ql_log_warn, vha, 0x5048,
 				"CT pass-through-%s error comp_status=0x%x total_byte=0x%x.\n",
@@ -2256,11 +2256,8 @@ qla25xx_process_bidir_status_iocb(scsi_qla_host_t *vha, void *pkt,
 	struct bsg_job *bsg_job = NULL;
 	struct fc_bsg_request *bsg_request;
 	struct fc_bsg_reply *bsg_reply;
-	sts_entry_t *sts;
-	struct sts_entry_24xx *sts24;
-
-	sts = (sts_entry_t *) pkt;
-	sts24 = (struct sts_entry_24xx *) pkt;
+	sts_entry_t *sts = pkt;
+	struct sts_entry_24xx *sts24 = pkt;
 
 	/* Validate handle. */
 	if (index >= req->num_outstanding_cmds) {
@@ -2406,8 +2403,8 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
 	srb_t		*sp;
 	fc_port_t	*fcport;
 	struct scsi_cmnd *cp;
-	sts_entry_t *sts;
-	struct sts_entry_24xx *sts24;
+	sts_entry_t *sts = pkt;
+	struct sts_entry_24xx *sts24 = pkt;
 	uint16_t	comp_status;
 	uint16_t	scsi_status;
 	uint16_t	ox_id;
@@ -2425,8 +2422,6 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
 	uint16_t state_flags = 0;
 	uint16_t retry_delay = 0;
 
-	sts = (sts_entry_t *) pkt;
-	sts24 = (struct sts_entry_24xx *) pkt;
 	if (IS_FWI2_CAPABLE(ha)) {
 		comp_status = le16_to_cpu(sts24->comp_status);
 		scsi_status = le16_to_cpu(sts24->scsi_status) & SS_MASK;
Signature · 1 line
2.22.0
#55 lore raw

PATCH v2 53/58 qla2xxx: Report invalid mailbox status codes

In reply to Bart Van Assche

It is easy to mix up the QLA_* and the MBS_* status codes. Complain loudly if that happens.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_def.h  | 5 +++++
 drivers/scsi/qla2xxx/qla_init.c | 9 +++++++++
 2 files changed, 14 insertions(+)
drivers/scsi/qla2xxx/qla_def.h +5 -0
index 527b2a2708a1..556376ce0259 100644
@@ -990,6 +990,11 @@ struct mbx_cmd_32 {
 #define MBS_LINK_DOWN_ERROR		0x400B
 #define MBS_DIAG_ECHO_TEST_ERROR	0x400C
 
+static inline bool qla2xxx_is_valid_mbs(unsigned int mbs)
+{
+	return MBS_COMMAND_COMPLETE <= mbs && mbs <= MBS_DIAG_ECHO_TEST_ERROR;
+}
+
 /*
  * ISP mailbox asynchronous event status codes
  */
drivers/scsi/qla2xxx/qla_init.c +9 -0
index 64c84e53011e..b3f1203bfe87 100644
@@ -477,6 +477,9 @@ void qla24xx_handle_adisc_event(scsi_qla_host_t *vha, struct event_arg *ea)
 	    fcport->fw_login_state, ea->rc, fcport->login_gen, ea->sp->gen2,
 	    fcport->rscn_gen, ea->sp->gen1, fcport->loop_id);
 
+	WARN_ONCE(!qla2xxx_is_valid_mbs(ea->data[0]), "mbs: %#x\n",
+		  ea->data[0]);
+
 	if (ea->data[0] != MBS_COMMAND_COMPLETE) {
 		ql_dbg(ql_dbg_disc, vha, 0x2066,
 		    "%s %8phC: adisc fail: post delete\n",
@@ -1893,6 +1896,9 @@ qla24xx_async_abort_command(srb_t *sp)
 static void
 qla24xx_handle_prli_done_event(struct scsi_qla_host *vha, struct event_arg *ea)
 {
+	WARN_ONCE(!qla2xxx_is_valid_mbs(ea->data[0]), "mbs: %#x\n",
+		  ea->data[0]);
+
 	switch (ea->data[0]) {
 	case MBS_COMMAND_COMPLETE:
 		ql_dbg(ql_dbg_disc, vha, 0x2118,
@@ -1978,6 +1984,9 @@ qla24xx_handle_plogi_done_event(struct scsi_qla_host *vha, struct event_arg *ea)
 		return;
 	}
 
+	WARN_ONCE(!qla2xxx_is_valid_mbs(ea->data[0]), "mbs: %#x\n",
+		  ea->data[0]);
+
 	switch (ea->data[0]) {
 	case MBS_COMMAND_COMPLETE:
 		/*
Signature · 1 line
2.22.0
#56 lore raw

PATCH v2 54/58 qla2xxx: Inline the qla2x00_fcport_event_handler() function

In reply to Bart Van Assche

Instead of calling qla2x00_fcport_event_handler() and letting the switch statement inside that function decide which other function to call, call the latter function directly. Remove the event member from the event_arg structure because it is no longer needed. Remove the qla_handle_els_plogi_done() function because it is never called.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_def.h  |  17 -----
 drivers/scsi/qla2xxx/qla_gbl.h  |   6 +-
 drivers/scsi/qla2xxx/qla_gs.c   |  15 ++--
 drivers/scsi/qla2xxx/qla_init.c | 131 ++++++++------------------------
 drivers/scsi/qla2xxx/qla_iocb.c |   3 +-
 drivers/scsi/qla2xxx/qla_isr.c  |   3 +-
 drivers/scsi/qla2xxx/qla_os.c   |   3 +-
 7 files changed, 45 insertions(+), 133 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +0 -17
index 556376ce0259..a971d4245d89 100644
@@ -2363,22 +2363,6 @@ enum login_state {	/* FW control Target side */
 	DSC_LS_LOGO_PEND,
 };
 
-enum fcport_mgt_event {
-	FCME_RELOGIN = 1,
-	FCME_RSCN,
-	FCME_PLOGI_DONE,	/* Initiator side sent LLIOCB */
-	FCME_PRLI_DONE,
-	FCME_GNL_DONE,
-	FCME_GPSC_DONE,
-	FCME_GPDB_DONE,
-	FCME_GPNID_DONE,
-	FCME_GFFID_DONE,
-	FCME_ADISC_DONE,
-	FCME_GNNID_DONE,
-	FCME_GFPNID_DONE,
-	FCME_ELS_PLOGI_DONE,
-};
-
 enum rscn_addr_format {
 	RSCN_PORT_ADDR,
 	RSCN_AREA_ADDR,
@@ -2496,7 +2480,6 @@ typedef struct fc_port {
 #define QLA_FCPORT_FOUND	2
 
 struct event_arg {
-	enum fcport_mgt_event	event;
 	fc_port_t		*fcport;
 	srb_t			*sp;
 	port_id_t		id;
drivers/scsi/qla2xxx/qla_gbl.h +5 -1
index 6b914942fad3..3f313eaf854b 100644
@@ -96,7 +96,11 @@ extern int qla2x00_init_rings(scsi_qla_host_t *);
 extern struct qla_qpair *qla2xxx_create_qpair(struct scsi_qla_host *,
 	int, int, bool);
 extern int qla2xxx_delete_qpair(struct scsi_qla_host *, struct qla_qpair *);
-void qla2x00_fcport_event_handler(scsi_qla_host_t *, struct event_arg *);
+void qla2x00_handle_rscn(scsi_qla_host_t *vha, struct event_arg *ea);
+void qla24xx_handle_plogi_done_event(struct scsi_qla_host *vha,
+				     struct event_arg *ea);
+void qla24xx_handle_relogin_event(scsi_qla_host_t *vha,
+				  struct event_arg *ea);
 int qla24xx_async_gpdb(struct scsi_qla_host *, fc_port_t *, u8);
 int qla24xx_async_prli(struct scsi_qla_host *, fc_port_t *);
 int qla24xx_async_notify_ack(scsi_qla_host_t *, fc_port_t *,
drivers/scsi/qla2xxx/qla_gs.c +5 -10
index 35e1f36c9366..0e1df8232c75 100644
@@ -3031,11 +3031,10 @@ static void qla24xx_async_gpsc_sp_done(srb_t *sp, int res)
 		    be16_to_cpu(ct_rsp->rsp.gpsc.speed));
 	}
 	memset(&ea, 0, sizeof(ea));
-	ea.event = FCME_GPSC_DONE;
 	ea.rc = res;
 	ea.fcport = fcport;
 	ea.sp = sp;
-	qla2x00_fcport_event_handler(vha, &ea);
+	qla24xx_handle_gpsc_event(vha, &ea);
 
 done:
 	sp->free(sp);
@@ -3283,7 +3282,6 @@ static void qla2x00_async_gpnid_sp_done(srb_t *sp, int res)
 	ea.sp = sp;
 	ea.id = be_to_port_id(ct_req->req.port_id.port_id);
 	ea.rc = res;
-	ea.event = FCME_GPNID_DONE;
 
 	spin_lock_irqsave(&vha->hw->tgt.sess_lock, flags);
 	list_del(&sp->elem);
@@ -3302,7 +3300,7 @@ static void qla2x00_async_gpnid_sp_done(srb_t *sp, int res)
 		return;
 	}
 
-	qla2x00_fcport_event_handler(vha, &ea);
+	qla24xx_handle_gpnid_event(vha, &ea);
 
 	e = qla2x00_alloc_work(vha, QLA_EVT_UNMAP);
 	if (!e) {
@@ -3481,9 +3479,8 @@ void qla24xx_async_gffid_sp_done(srb_t *sp, int res)
 	ea.sp = sp;
 	ea.fcport = sp->fcport;
 	ea.rc = res;
-	ea.event = FCME_GFFID_DONE;
 
-	qla2x00_fcport_event_handler(vha, &ea);
+	qla24xx_handle_gffid_event(vha, &ea);
 	sp->free(sp);
 }
 
@@ -4263,13 +4260,12 @@ static void qla2x00_async_gnnid_sp_done(srb_t *sp, int res)
 	ea.fcport = fcport;
 	ea.sp = sp;
 	ea.rc = res;
-	ea.event = FCME_GNNID_DONE;
 
 	ql_dbg(ql_dbg_disc, vha, 0x204f,
 	    "Async done-%s res %x, WWPN %8phC %8phC\n",
 	    sp->name, res, fcport->port_name, fcport->node_name);
 
-	qla2x00_fcport_event_handler(vha, &ea);
+	qla24xx_handle_gnnid_event(vha, &ea);
 
 	sp->free(sp);
 }
@@ -4394,13 +4390,12 @@ static void qla2x00_async_gfpnid_sp_done(srb_t *sp, int res)
 	ea.fcport = fcport;
 	ea.sp = sp;
 	ea.rc = res;
-	ea.event = FCME_GFPNID_DONE;
 
 	ql_dbg(ql_dbg_disc, vha, 0x204f,
 	    "Async done-%s res %x, WWPN %8phC %8phC\n",
 	    sp->name, res, fcport->port_name, fcport->fabric_port_name);
 
-	qla2x00_fcport_event_handler(vha, &ea);
+	qla24xx_handle_gfpnid_event(vha, &ea);
 
 	sp->free(sp);
 }
drivers/scsi/qla2xxx/qla_init.c +32 -99

225 lines not on this page · raw message

drivers/scsi/qla2xxx/qla_iocb.c +1 -2
index 6acf92d19951..39c7738c0a55 100644
@@ -2741,8 +2741,7 @@ static void qla2x00_els_dcmd2_sp_done(srb_t *sp, int res)
 			memset(&ea, 0, sizeof(ea));
 			ea.fcport = fcport;
 			ea.rc = res;
-			ea.event = FCME_ELS_PLOGI_DONE;
-			qla2x00_fcport_event_handler(vha, &ea);
+			qla24xx_handle_plogi_done_event(vha, &ea);
 		}
 
 		e = qla2x00_alloc_work(vha, QLA_EVT_UNMAP);
drivers/scsi/qla2xxx/qla_isr.c +1 -2
index 5c65f2e67448..cd39ac18c5fd 100644
@@ -1118,10 +1118,9 @@ qla2x00_async_event(scsi_qla_host_t *vha, struct rsp_que *rsp, uint16_t *mb)
 			struct event_arg ea;
 
 			memset(&ea, 0, sizeof(ea));
-			ea.event = FCME_RSCN;
 			ea.id.b24 = rscn_entry;
 			ea.id.b.rsvd_1 = rscn_entry >> 24;
-			qla2x00_fcport_event_handler(vha, &ea);
+			qla2x00_handle_rscn(vha, &ea);
 			qla2x00_post_aen_work(vha, FCH_EVT_RSCN, rscn_entry);
 		}
 		break;
drivers/scsi/qla2xxx/qla_os.c +1 -2
index 5cb0bb65ace3..7d73b6a7cf41 100644
@@ -5328,9 +5328,8 @@ void qla2x00_relogin(struct scsi_qla_host *vha)
 			} else {
 				if (vha->hw->current_topology != ISP_CFG_NL) {
 					memset(&ea, 0, sizeof(ea));
-					ea.event = FCME_RELOGIN;
 					ea.fcport = fcport;
-					qla2x00_fcport_event_handler(vha, &ea);
+					qla24xx_handle_relogin_event(vha, &ea);
 				} else if (vha->hw->current_topology ==
 				    ISP_CFG_NL) {
 					fcport->login_retry--;
Signature · 1 line
2.22.0
#57 lore raw

PATCH v2 55/58 qla2xxx: Introduce qla2x00_els_dcmd2_free()

In reply to Bart Van Assche

This patch reduces code duplication.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_def.h  |  2 +-
 drivers/scsi/qla2xxx/qla_gbl.h  |  2 ++
 drivers/scsi/qla2xxx/qla_gs.c   | 12 +---------
 drivers/scsi/qla2xxx/qla_iocb.c | 40 ++++++++++++++-------------------
 4 files changed, 21 insertions(+), 35 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +1 -1
index a971d4245d89..873a6aef1c5c 100644
@@ -455,7 +455,7 @@ struct srb_iocb {
 			struct els_logo_payload *els_logo_pyld;
 			dma_addr_t els_logo_pyld_dma;
 		} els_logo;
-		struct {
+		struct els_plogi {
 #define ELS_DCMD_PLOGI 0x3
 			uint32_t flags;
 			uint32_t els_cmd;
drivers/scsi/qla2xxx/qla_gbl.h +2 -0
index 3f313eaf854b..d11416dcee4e 100644
@@ -45,6 +45,8 @@ extern int qla2x00_local_device_login(scsi_qla_host_t *, fc_port_t *);
 
 extern int qla24xx_els_dcmd_iocb(scsi_qla_host_t *, int, port_id_t);
 extern int qla24xx_els_dcmd2_iocb(scsi_qla_host_t *, int, fc_port_t *, bool);
+extern void qla2x00_els_dcmd2_free(scsi_qla_host_t *vha,
+				   struct els_plogi *els_plogi);
 
 extern void qla2x00_update_fcports(scsi_qla_host_t *);
 
drivers/scsi/qla2xxx/qla_gs.c +1 -11
index 0e1df8232c75..bf8b30c4827c 100644
@@ -3119,17 +3119,7 @@ void qla24xx_sp_unmap(scsi_qla_host_t *vha, srb_t *sp)
 
 	switch (sp->type) {
 	case SRB_ELS_DCMD:
-		if (c->u.els_plogi.els_plogi_pyld)
-			dma_free_coherent(&vha->hw->pdev->dev,
-			    c->u.els_plogi.tx_size,
-			    c->u.els_plogi.els_plogi_pyld,
-			    c->u.els_plogi.els_plogi_pyld_dma);
-
-		if (c->u.els_plogi.els_resp_pyld)
-			dma_free_coherent(&vha->hw->pdev->dev,
-			    c->u.els_plogi.rx_size,
-			    c->u.els_plogi.els_resp_pyld,
-			    c->u.els_plogi.els_resp_pyld_dma);
+		qla2x00_els_dcmd2_free(vha, &c->u.els_plogi);
 		break;
 	case SRB_CT_PTHRU_CMD:
 	default:
drivers/scsi/qla2xxx/qla_iocb.c +17 -23
index 39c7738c0a55..7021fbeb6d23 100644
@@ -2717,6 +2717,21 @@ qla2x00_els_dcmd2_iocb_timeout(void *data)
 	sp->done(sp, QLA_FUNCTION_TIMEOUT);
 }
 
+void qla2x00_els_dcmd2_free(scsi_qla_host_t *vha, struct els_plogi *els_plogi)
+{
+	if (els_plogi->els_plogi_pyld)
+		dma_free_coherent(&vha->hw->pdev->dev,
+				  els_plogi->tx_size,
+				  els_plogi->els_plogi_pyld,
+				  els_plogi->els_plogi_pyld_dma);
+
+	if (els_plogi->els_resp_pyld)
+		dma_free_coherent(&vha->hw->pdev->dev,
+				  els_plogi->rx_size,
+				  els_plogi->els_resp_pyld,
+				  els_plogi->els_resp_pyld_dma);
+}
+
 static void qla2x00_els_dcmd2_sp_done(srb_t *sp, int res)
 {
 	fc_port_t *fcport = sp->fcport;
@@ -2748,17 +2763,7 @@ static void qla2x00_els_dcmd2_sp_done(srb_t *sp, int res)
 		if (!e) {
 			struct srb_iocb *elsio = &sp->u.iocb_cmd;
 
-			if (elsio->u.els_plogi.els_plogi_pyld)
-				dma_free_coherent(&sp->vha->hw->pdev->dev,
-				    elsio->u.els_plogi.tx_size,
-				    elsio->u.els_plogi.els_plogi_pyld,
-				    elsio->u.els_plogi.els_plogi_pyld_dma);
-
-			if (elsio->u.els_plogi.els_resp_pyld)
-				dma_free_coherent(&sp->vha->hw->pdev->dev,
-				    elsio->u.els_plogi.rx_size,
-				    elsio->u.els_plogi.els_resp_pyld,
-				    elsio->u.els_plogi.els_resp_pyld_dma);
+			qla2x00_els_dcmd2_free(vha, &elsio->u.els_plogi);
 			sp->free(sp);
 			return;
 		}
@@ -2858,18 +2863,7 @@ qla24xx_els_dcmd2_iocb(scsi_qla_host_t *vha, int els_opcode,
 
 out:
 	fcport->flags &= ~(FCF_ASYNC_SENT);
-	if (elsio->u.els_plogi.els_plogi_pyld)
-		dma_free_coherent(&sp->vha->hw->pdev->dev,
-		    elsio->u.els_plogi.tx_size,
-		    elsio->u.els_plogi.els_plogi_pyld,
-		    elsio->u.els_plogi.els_plogi_pyld_dma);
-
-	if (elsio->u.els_plogi.els_resp_pyld)
-		dma_free_coherent(&sp->vha->hw->pdev->dev,
-		    elsio->u.els_plogi.rx_size,
-		    elsio->u.els_plogi.els_resp_pyld,
-		    elsio->u.els_plogi.els_resp_pyld_dma);
-
+	qla2x00_els_dcmd2_free(vha, &elsio->u.els_plogi);
 	sp->free(sp);
 done:
 	return rval;
Signature · 1 line
2.22.0
#58 lore raw

PATCH v2 56/58 qla2xxx: Remove two superfluous if-tests

In reply to Bart Van Assche

This patch fixes the following Coverity complaint:

Null-checking sp->u.iocb_cmd.u.ctarg.rsp suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

See also commit e374f9f59281 ("scsi: qla2xxx: Migrate switch registration commands away from mailbox interface") # v4.16.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_gs.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)
drivers/scsi/qla2xxx/qla_gs.c +11 -14
index bf8b30c4827c..03f94eb372b6 100644
@@ -3295,20 +3295,17 @@ static void qla2x00_async_gpnid_sp_done(srb_t *sp, int res)
 	e = qla2x00_alloc_work(vha, QLA_EVT_UNMAP);
 	if (!e) {
 		/* please ignore kernel warning. otherwise, we have mem leak. */
-		if (sp->u.iocb_cmd.u.ctarg.req) {
-			dma_free_coherent(&vha->hw->pdev->dev,
-				sp->u.iocb_cmd.u.ctarg.req_allocated_size,
-				sp->u.iocb_cmd.u.ctarg.req,
-				sp->u.iocb_cmd.u.ctarg.req_dma);
-			sp->u.iocb_cmd.u.ctarg.req = NULL;
-		}
-		if (sp->u.iocb_cmd.u.ctarg.rsp) {
-			dma_free_coherent(&vha->hw->pdev->dev,
-				sp->u.iocb_cmd.u.ctarg.rsp_allocated_size,
-				sp->u.iocb_cmd.u.ctarg.rsp,
-				sp->u.iocb_cmd.u.ctarg.rsp_dma);
-			sp->u.iocb_cmd.u.ctarg.rsp = NULL;
-		}
+		dma_free_coherent(&vha->hw->pdev->dev,
+				  sp->u.iocb_cmd.u.ctarg.req_allocated_size,
+				  sp->u.iocb_cmd.u.ctarg.req,
+				  sp->u.iocb_cmd.u.ctarg.req_dma);
+		sp->u.iocb_cmd.u.ctarg.req = NULL;
+
+		dma_free_coherent(&vha->hw->pdev->dev,
+				  sp->u.iocb_cmd.u.ctarg.rsp_allocated_size,
+				  sp->u.iocb_cmd.u.ctarg.rsp,
+				  sp->u.iocb_cmd.u.ctarg.rsp_dma);
+		sp->u.iocb_cmd.u.ctarg.rsp = NULL;
 
 		sp->free(sp);
 		return;
Signature · 1 line
2.22.0
#59 lore raw

PATCH v2 57/58 qla2xxx: Simplify qla24xx_async_abort_cmd()

In reply to Bart Van Assche

Make the code easier to read by converting 'goto' statements into 'return' statements.

Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_init.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/scsi/qla2xxx/qla_init.c +6 -8
index c9cb6856f82e..535dc21ef56e 100644
@@ -136,7 +136,7 @@ static int qla24xx_async_abort_cmd(srb_t *cmd_sp, bool wait)
 	sp = qla2xxx_get_qpair_sp(cmd_sp->vha, cmd_sp->qpair, cmd_sp->fcport,
 				  GFP_ATOMIC);
 	if (!sp)
-		goto done;
+		return rval;
 
 	abt_iocb = &sp->u.iocb_cmd;
 	sp->type = SRB_ABT_CMD;
@@ -160,20 +160,18 @@ static int qla24xx_async_abort_cmd(srb_t *cmd_sp, bool wait)
 	       cmd_sp->type);
 
 	rval = qla2x00_start_sp(sp);
-	if (rval != QLA_SUCCESS)
-		goto done_free_sp;
+	if (rval != QLA_SUCCESS) {
+		sp->free(sp);
+		return rval;
+	}
 
 	if (wait) {
 		wait_for_completion(&abt_iocb->u.abt.comp);
 		rval = abt_iocb->u.abt.comp_status == CS_COMPLETE ?
 			QLA_SUCCESS : QLA_FUNCTION_FAILED;
-	} else {
-		goto done;
+		sp->free(sp);
 	}
 
-done_free_sp:
-	sp->free(sp);
-done:
 	return rval;
 }
 
Signature · 1 line
2.22.0
#60 lore raw

PATCH v2 58/58 qla2xxx: Fix a NULL pointer dereference

In reply to Bart Van Assche
BUG: KASAN: null-ptr-deref in qla24xx_handle_plogi_done_event+0x134/0x9f0 [qla2xxx]
Read of size 4 at addr 00000000000000a0 by task swapper/2/0

CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.2.0-dbg+ #1
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
Call Trace:
 <IRQ>
 dump_stack+0x8a/0xd6
 __kasan_report.cold+0x5/0x41
 kasan_report+0x16/0x20
 __asan_load4+0x7e/0x80
 qla24xx_handle_plogi_done_event+0x134/0x9f0 [qla2xxx]
 qla2x00_els_dcmd2_sp_done+0x15f/0x230 [qla2xxx]
 qla24xx_els_ct_entry+0x3b3/0x610 [qla2xxx]
 qla24xx_process_response_queue+0x514/0x10e0 [qla2xxx]
 qla24xx_msix_rsp_q+0x80/0x100 [qla2xxx]
 __handle_irq_event_percpu+0x72/0x450
 handle_irq_event_percpu+0x74/0xf0
 handle_irq_event+0x5e/0x8f
 handle_edge_irq+0x13a/0x320
 handle_irq+0x30/0x40
 do_IRQ+0x91/0x190
 common_interrupt+0xf/0xf
 </IRQ>
RIP: 0010:default_idle+0x31/0x230
Fixes 8777e4314d39 ("scsi: qla2xxx: Migrate NVME N2N handling into state machine") # v4.19.
Cc Himanshu Madhani <[email protected]>
---
 drivers/scsi/qla2xxx/qla_iocb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_iocb.c +2 -1
index 7021fbeb6d23..e92e52aa6e9b 100644
@@ -2755,7 +2755,8 @@ static void qla2x00_els_dcmd2_sp_done(srb_t *sp, int res)
 		} else {
 			memset(&ea, 0, sizeof(ea));
 			ea.fcport = fcport;
-			ea.rc = res;
+			ea.data[0] = MBS_COMMAND_COMPLETE;
+			ea.sp = sp;
 			qla24xx_handle_plogi_done_event(vha, &ea);
 		}
 
Signature · 1 line
2.22.0
Tested-by Reviewed-by on the v2 series
#61 lore raw
In reply to Bart Van Assche

Hi Bart,

On 8/8/19, 10:02 PM, "[email protected] on behalf of Bart Van Assche" <[email protected] on behalf of [email protected]> wrote:

Hi Martin,

The patches in this series improve the robustness of the QLogic Fibre Channel initiator and target drivers. These patches are a result of manual code inspection, analysis of Coverity reports and stress testing of these two drivers. Please consider these patches for kernel version v5.4.

Thanks,

Bart.

Changes compared to v1:
- Included a regression fix for qla2xxx_eh_abort() in the second patch of this series (the fix Himanshu mentioned in his e-mail).
- Moved a WARN_ON_ONCE() statement from a later patch to the second patch in this series.
- Dropped one patch that renames a function.

Bart Van Assche (58):
qla2xxx: Make qla2x00_abort_srb() again decrease the sp reference
count
qla2xxx: Really fix qla2xxx_eh_abort()
qla2xxx: Improve Linux kernel coding style conformance
qla2xxx: Use tabs instead of spaces for indentation
qla2xxx: Include the <asm/unaligned.h> header file from qla_dsd.h
qla2xxx: Remove an include directive from qla_mr.c
qla2xxx: Remove a superfluous forward declaration
qla2xxx: Declare the fourth ql_dump_buffer() argument const
qla2xxx: Change the return type of qla2x00_update_ms_fdmi_iocb() into
void
qla2xxx: Reduce the scope of three local variables in
qla2xxx_queuecommand()
qla2xxx: Declare qla_tgt_cmd.cdb const
qla2xxx: Change data_dsd into an array
qla2xxx: Verify locking assumptions at runtime
qla2xxx: Reduce the number of casts in GID list code
qla2xxx: Simplify qlt_lport_dump()
qla2xxx: Remove a superfluous pointer check
qla2xxx: Remove two superfluous tests
qla2xxx: Simplify qla24xx_abort_sp_done()
qla2xxx: Fix session lookup in qlt_abort_work()
qla2xxx: Report the firmware status code if a mailbox command fails
qla2xxx: Do not corrupt vha->plogi_ack_list
qla2xxx: Use strlcpy() instead of strncpy()
qla2xxx: Complain if a mailbox command times out
qla2xxx: Complain if parsing the version string fails
qla2xxx: Remove dead code
qla2xxx: Simplify a debug statement
qla2xxx: Fix qla24xx_process_bidir_cmd()
qla2xxx: Remove unreachable code from qla83xx_idc_lock()
qla2xxx: Suppress a Coveritiy complaint about integer overflow
qla2xxx: Suppress multiple Coverity complaint about out-of-bounds
accesses
qla2xxx: Always check the qla2x00_wait_for_hba_online() return value
qla2xxx: Declare fourth qla2x00_set_model_info() argument const
qla2xxx: Complain if waiting for pending commands times out
qla2xxx: Check the PCI info string output buffer size
qla2xxx: Use memcpy() and strlcpy() instead of strcpy() and strncpy()
qla2xxx: Complain if a soft reset fails
qla2xxx: Introduce the be_id_t and le_id_t data types for FC src/dst
IDs
qla2xxx: Change the return type of qla24xx_read_flash_data()
qla2xxx: Check secondary image if reading the primary image fails
qla2xxx: Make it explicit that ELS pass-through IOCBs use little
endian
qla2xxx: Set the responder mode if appropriate for ELS pass-through
IOCBs
qla2xxx: Rework key encoding in qlt_find_host_by_d_id()
qla2xxx: Enable type checking for the SRB free and done callback
functions
qla2xxx: Introduce the function qla2xxx_init_sp()
qla2xxx: Fix a race condition between aborting and completing a SCSI
command
qla2xxx: Make qlt_handle_abts_completion() more robust
qla2xxx: Modify NVMe include directives
qla2xxx: Introduce qla2xxx_get_next_handle()
qla2xxx: Make sure that aborted commands are freed
qla2xxx: Complain if sp->done() is not called from the completion path
qla2xxx: Let the compiler check the type of the SCSI command context
pointer
qla2xxx: Remove superfluous sts_entry_* casts
qla2xxx: Report invalid mailbox status codes
qla2xxx: Inline the qla2x00_fcport_event_handler() function
qla2xxx: Introduce qla2x00_els_dcmd2_free()
qla2xxx: Remove two superfluous if-tests
qla2xxx: Simplify qla24xx_async_abort_cmd()
qla2xxx: Fix a NULL pointer dereference

drivers/scsi/qla2xxx/qla_attr.c | 6 +-
drivers/scsi/qla2xxx/qla_bsg.c | 19 +--
drivers/scsi/qla2xxx/qla_dbg.c | 3 +-
drivers/scsi/qla2xxx/qla_def.h | 130 ++++++++++----
drivers/scsi/qla2xxx/qla_dfs.c | 9 +-
drivers/scsi/qla2xxx/qla_dsd.h | 2 +
drivers/scsi/qla2xxx/qla_fw.h | 8 +-
drivers/scsi/qla2xxx/qla_gbl.h | 33 ++--
drivers/scsi/qla2xxx/qla_gs.c | 219 +++++++++---------------
drivers/scsi/qla2xxx/qla_init.c | 261 +++++++++++------------------
drivers/scsi/qla2xxx/qla_inline.h | 28 ++--
drivers/scsi/qla2xxx/qla_iocb.c | 221 +++++++++---------------
drivers/scsi/qla2xxx/qla_isr.c | 24 ++-
drivers/scsi/qla2xxx/qla_mbx.c | 10 +-
drivers/scsi/qla2xxx/qla_mid.c | 4 +-
drivers/scsi/qla2xxx/qla_mr.c | 67 ++++----
drivers/scsi/qla2xxx/qla_nvme.c | 28 +---
drivers/scsi/qla2xxx/qla_nvme.h | 5 +-
drivers/scsi/qla2xxx/qla_nx.c | 16 +-
drivers/scsi/qla2xxx/qla_nx.h | 14 +-
drivers/scsi/qla2xxx/qla_nx2.c | 2 +-
drivers/scsi/qla2xxx/qla_os.c | 213 ++++++++++-------------
drivers/scsi/qla2xxx/qla_sup.c | 8 +-
drivers/scsi/qla2xxx/qla_target.c | 209 +++++++++-------------- drivers/scsi/qla2xxx/qla_target.h | 35 ++--
drivers/scsi/qla2xxx/qla_tmpl.c | 7 +-
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 27 ++-
include/linux/nvme-fc-driver.h | 2 +
28 files changed, 677 insertions(+), 933 deletions(-)

--
2.22.0

Thanks for posting v2 of the series.

For the series,

#62 lore raw
In reply to Bart Van Assche

Bart,

Quoted 5 lines

The patches in this series improve the robustness of the QLogic Fibre Channel initiator and target drivers. These patches are a result of manual code inspection, analysis of Coverity reports and stress testing of these two drivers. Please consider these patches for kernel version v5.4.

Applied to 5.4/scsi-queue. Thanks you!

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering