All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] qla2xxx patches for kernel v5.7
@ 2020-02-20  4:34 Bart Van Assche
  2020-02-20  4:34 ` [PATCH v3 1/5] qla2xxx: Simplify the code for aborting SCSI commands Bart Van Assche
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-02-20  4:34 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley; +Cc: linux-scsi, Bart Van Assche

Hi Martin,

Please consider these five qla2xxx patches for kernel v5.7.

Thanks,

Bart.

Changes compared to v2:
- Left out a patch that is already in Linus' tree.

Changes compared to v1:
- Dropped patch "Fix point-to-point mode for qla25xx and older"
- Added three new patches.

Bart Van Assche (5):
  qla2xxx: Simplify the code for aborting SCSI commands
  qla2xxx: Suppress endianness complaints in
    qla2x00_configure_local_loop()
  qla2xxx: Fix sparse warnings triggered by the PCI state checking code
  qla2xxx: Convert MAKE_HANDLE() from a define into an inline function
  qla2xxx: Fix the endianness annotations for the port attribute
    max_frame_size member

 drivers/scsi/qla2xxx/qla_def.h    | 14 +++++++-------
 drivers/scsi/qla2xxx/qla_gs.c     | 14 ++++++--------
 drivers/scsi/qla2xxx/qla_init.c   | 11 +++++------
 drivers/scsi/qla2xxx/qla_iocb.c   | 22 +++++++++++-----------
 drivers/scsi/qla2xxx/qla_isr.c    |  5 -----
 drivers/scsi/qla2xxx/qla_mbx.c    | 15 +++++++--------
 drivers/scsi/qla2xxx/qla_mr.c     | 13 ++++++-------
 drivers/scsi/qla2xxx/qla_nvme.c   |  2 +-
 drivers/scsi/qla2xxx/qla_os.c     | 27 ++++++++++++++-------------
 drivers/scsi/qla2xxx/qla_target.c |  6 +++---
 10 files changed, 60 insertions(+), 69 deletions(-)


^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v3 1/5] qla2xxx: Simplify the code for aborting SCSI commands
  2020-02-20  4:34 [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Bart Van Assche
@ 2020-02-20  4:34 ` Bart Van Assche
  2020-02-20  4:34 ` [PATCH v3 2/5] qla2xxx: Suppress endianness complaints in qla2x00_configure_local_loop() Bart Van Assche
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-02-20  4:34 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Himanshu Madhani, Roman Bolshakov,
	Daniel Wagner, Quinn Tran, Martin Wilck

Since the SCSI core does not reuse the tag of the SCSI command that is
being aborted by .eh_abort() before .eh_abort() has finished it is not
necessary to check from inside that callback whether or not the SCSI command
has already completed. Instead, rely on the firmware to return an error code
when attempting to abort a command that has already completed. Additionally,
rely on the firmware to return an error code when attempting to abort an
already aborted command.

In qla2x00_abort_srb(), use blk_mq_request_started() instead of
sp->completed and sp->aborted.

Acked-by: Himanshu Madhani <hmadhani@marvell.com>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Martin Wilck <mwilck@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_def.h |  3 ---
 drivers/scsi/qla2xxx/qla_isr.c |  5 -----
 drivers/scsi/qla2xxx/qla_os.c  | 27 ++++++++++++++-------------
 3 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index ed32e9715794..c5a067f45005 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -597,9 +597,6 @@ typedef struct srb {
 	struct fc_port *fcport;
 	struct scsi_qla_host *vha;
 	unsigned int start_timer:1;
-	unsigned int abort:1;
-	unsigned int aborted:1;
-	unsigned int completed:1;
 
 	uint32_t handle;
 	uint16_t flags;
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index e40705d38cea..c7c86a432b55 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2516,11 +2516,6 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt)
 		return;
 	}
 
-	if (sp->abort)
-		sp->aborted = 1;
-	else
-		sp->completed = 1;
-
 	if (sp->cmd_type != TYPE_SRB) {
 		req->outstanding_cmds[handle] = NULL;
 		ql_dbg(ql_dbg_io, vha, 0x3015,
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 79387ac8936f..a34f27b2d602 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1253,17 +1253,6 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
 		return SUCCESS;
 
 	spin_lock_irqsave(qpair->qp_lock_ptr, flags);
-	if (sp->completed) {
-		spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
-		return SUCCESS;
-	}
-
-	if (sp->abort || sp->aborted) {
-		spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
-		return FAILED;
-	}
-
-	sp->abort = 1;
 	sp->comp = &comp;
 	spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
 
@@ -1688,6 +1677,10 @@ qla2x00_loop_reset(scsi_qla_host_t *vha)
 	return QLA_SUCCESS;
 }
 
+/*
+ * The caller must ensure that no completion interrupts will happen
+ * while this function is in progress.
+ */
 static void qla2x00_abort_srb(struct qla_qpair *qp, srb_t *sp, const int res,
 			      unsigned long *flags)
 	__releases(qp->qp_lock_ptr)
@@ -1696,6 +1689,7 @@ static void qla2x00_abort_srb(struct qla_qpair *qp, srb_t *sp, const int res,
 	DECLARE_COMPLETION_ONSTACK(comp);
 	scsi_qla_host_t *vha = qp->vha;
 	struct qla_hw_data *ha = vha->hw;
+	struct scsi_cmnd *cmd = GET_CMD_SP(sp);
 	int rval;
 	bool ret_cmd;
 	uint32_t ratov_j;
@@ -1717,7 +1711,6 @@ static void qla2x00_abort_srb(struct qla_qpair *qp, srb_t *sp, const int res,
 		}
 
 		sp->comp = &comp;
-		sp->abort =  1;
 		spin_unlock_irqrestore(qp->qp_lock_ptr, *flags);
 
 		rval = ha->isp_ops->abort_command(sp);
@@ -1741,13 +1734,17 @@ static void qla2x00_abort_srb(struct qla_qpair *qp, srb_t *sp, const int res,
 		}
 
 		spin_lock_irqsave(qp->qp_lock_ptr, *flags);
-		if (ret_cmd && (!sp->completed || !sp->aborted))
+		if (ret_cmd && blk_mq_request_started(cmd->request))
 			sp->done(sp, res);
 	} else {
 		sp->done(sp, res);
 	}
 }
 
+/*
+ * The caller must ensure that no completion interrupts will happen
+ * while this function is in progress.
+ */
 static void
 __qla2x00_abort_all_cmds(struct qla_qpair *qp, int res)
 {
@@ -1794,6 +1791,10 @@ __qla2x00_abort_all_cmds(struct qla_qpair *qp, int res)
 	spin_unlock_irqrestore(qp->qp_lock_ptr, flags);
 }
 
+/*
+ * The caller must ensure that no completion interrupts will happen
+ * while this function is in progress.
+ */
 void
 qla2x00_abort_all_cmds(scsi_qla_host_t *vha, int res)
 {

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 2/5] qla2xxx: Suppress endianness complaints in qla2x00_configure_local_loop()
  2020-02-20  4:34 [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Bart Van Assche
  2020-02-20  4:34 ` [PATCH v3 1/5] qla2xxx: Simplify the code for aborting SCSI commands Bart Van Assche
@ 2020-02-20  4:34 ` Bart Van Assche
  2020-02-23 19:32   ` Roman Bolshakov
  2020-02-20  4:34 ` [PATCH v3 3/5] qla2xxx: Fix sparse warnings triggered by the PCI state checking code Bart Van Assche
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-02-20  4:34 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Quinn Tran,
	Martin Wilck, Roman Bolshakov

Instead of changing endianness in-place, write the data in CPU endian format
in another buffer and copy that buffer back. This patch does not change any
functionality but silences some sparse endianness warnings.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_def.h  |  2 +-
 drivers/scsi/qla2xxx/qla_init.c | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index c5a067f45005..b04d334933ef 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -414,7 +414,7 @@ struct els_logo_payload {
 struct els_plogi_payload {
 	uint8_t opcode;
 	uint8_t rsvd[3];
-	uint8_t data[112];
+	__be32	data[112 / 4];
 };
 
 struct ct_arg {
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 9e6b56527b25..880f2be062a9 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -5060,7 +5060,7 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha)
 	if (N2N_TOPO(ha)) {
 		if (test_and_clear_bit(N2N_LOGIN_NEEDED, &vha->dpc_flags)) {
 			/* borrowing */
-			u32 *bp, i, sz;
+			u32 *bp, sz;
 
 			memset(ha->init_cb, 0, ha->init_cb_size);
 			sz = min_t(int, sizeof(struct els_plogi_payload),
@@ -5068,13 +5068,12 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha)
 			rval = qla24xx_get_port_login_templ(vha,
 			    ha->init_cb_dma, (void *)ha->init_cb, sz);
 			if (rval == QLA_SUCCESS) {
+				__be32 *q = &ha->plogi_els_payld.data[0];
+
 				bp = (uint32_t *)ha->init_cb;
-				for (i = 0; i < sz/4 ; i++, bp++)
-					*bp = cpu_to_be32(*bp);
+				cpu_to_be32_array(q, bp, sz / 4);
 
-				memcpy(&ha->plogi_els_payld.data,
-				    (void *)ha->init_cb,
-				    sizeof(ha->plogi_els_payld.data));
+				memcpy(bp, q, sizeof(ha->plogi_els_payld.data));
 			} else {
 				ql_dbg(ql_dbg_init, vha, 0x00d1,
 				    "PLOGI ELS param read fail.\n");

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 3/5] qla2xxx: Fix sparse warnings triggered by the PCI state checking code
  2020-02-20  4:34 [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Bart Van Assche
  2020-02-20  4:34 ` [PATCH v3 1/5] qla2xxx: Simplify the code for aborting SCSI commands Bart Van Assche
  2020-02-20  4:34 ` [PATCH v3 2/5] qla2xxx: Suppress endianness complaints in qla2x00_configure_local_loop() Bart Van Assche
@ 2020-02-20  4:34 ` Bart Van Assche
  2020-02-25 19:13   ` Roman Bolshakov
  2020-02-20  4:34 ` [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function Bart Van Assche
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-02-20  4:34 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Quinn Tran,
	Martin Wilck, Roman Bolshakov

This patch fixes the following sparse warnings:

drivers/scsi/qla2xxx/qla_mbx.c:120:21: warning: restricted pci_channel_state_t degrades to integer
drivers/scsi/qla2xxx/qla_mbx.c:120:37: warning: restricted pci_channel_state_t degrades to integer

From include/linux/pci.h:

enum pci_channel_state {
	/* I/O channel is in normal state */
	pci_channel_io_normal = (__force pci_channel_state_t) 1,

	/* I/O to channel is blocked */
	pci_channel_io_frozen = (__force pci_channel_state_t) 2,

	/* PCI card is dead */
	pci_channel_io_perm_failure = (__force pci_channel_state_t) 3,
};

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_mbx.c | 5 ++---
 drivers/scsi/qla2xxx/qla_mr.c  | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 9e09964f5c0e..ff945d35ba8e 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -117,10 +117,9 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp)
 
 	ql_dbg(ql_dbg_mbx, vha, 0x1000, "Entered %s.\n", __func__);
 
-	if (ha->pdev->error_state > pci_channel_io_frozen) {
+	if (ha->pdev->error_state == pci_channel_io_perm_failure) {
 		ql_log(ql_log_warn, vha, 0x1001,
-		    "error_state is greater than pci_channel_io_frozen, "
-		    "exiting.\n");
+		    "PCI channel failed permanently, exiting.\n");
 		return QLA_FUNCTION_TIMEOUT;
 	}
 
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index cad1fc2a1b28..6d120457478e 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -53,10 +53,9 @@ qlafx00_mailbox_command(scsi_qla_host_t *vha, struct mbx_cmd_32 *mcp)
 	struct qla_hw_data *ha = vha->hw;
 	scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev);
 
-	if (ha->pdev->error_state > pci_channel_io_frozen) {
+	if (ha->pdev->error_state == pci_channel_io_perm_failure) {
 		ql_log(ql_log_warn, vha, 0x115c,
-		    "error_state is greater than pci_channel_io_frozen, "
-		    "exiting.\n");
+		    "PCI channel failed permanently, exiting.\n");
 		return QLA_FUNCTION_TIMEOUT;
 	}
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function
  2020-02-20  4:34 [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Bart Van Assche
                   ` (2 preceding siblings ...)
  2020-02-20  4:34 ` [PATCH v3 3/5] qla2xxx: Fix sparse warnings triggered by the PCI state checking code Bart Van Assche
@ 2020-02-20  4:34 ` Bart Van Assche
  2020-02-20  8:21   ` Daniel Wagner
  2020-02-26  0:28   ` Roman Bolshakov
  2020-02-20  4:34 ` [PATCH v3 5/5] qla2xxx: Fix the endianness annotations for the port attribute max_frame_size member Bart Van Assche
  2020-02-29  1:10 ` [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Martin K. Petersen
  5 siblings, 2 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-02-20  4:34 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Quinn Tran, Martin Wilck,
	Daniel Wagner, Roman Bolshakov

This patch allows sparse to verify the endianness of the arguments passed
to make_handle().

Cc: Quinn Tran <qutran@marvell.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Daniel Wagner <dwagner@suse.de>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_def.h    |  5 ++++-
 drivers/scsi/qla2xxx/qla_iocb.c   | 22 +++++++++++-----------
 drivers/scsi/qla2xxx/qla_mbx.c    | 10 +++++-----
 drivers/scsi/qla2xxx/qla_mr.c     |  8 ++++----
 drivers/scsi/qla2xxx/qla_nvme.c   |  2 +-
 drivers/scsi/qla2xxx/qla_target.c |  6 +++---
 6 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index b04d334933ef..cec0b5082927 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -119,7 +119,10 @@ typedef struct {
 #define LSD(x)	((uint32_t)((uint64_t)(x)))
 #define MSD(x)	((uint32_t)((((uint64_t)(x)) >> 16) >> 16))
 
-#define MAKE_HANDLE(x, y) ((uint32_t)((((uint32_t)(x)) << 16) | (uint32_t)(y)))
+static inline uint32_t make_handle(uint16_t x, uint16_t y)
+{
+	return ((uint32_t)x << 16) | y;
+}
 
 /*
  * I/O register
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 47bf60a9490a..1816660768da 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -530,7 +530,7 @@ __qla2x00_marker(struct scsi_qla_host *vha, struct qla_qpair *qpair,
 			int_to_scsilun(lun, (struct scsi_lun *)&mrk24->lun);
 			host_to_fcp_swap(mrk24->lun, sizeof(mrk24->lun));
 			mrk24->vp_index = vha->vp_idx;
-			mrk24->handle = MAKE_HANDLE(req->id, mrk24->handle);
+			mrk24->handle = make_handle(req->id, mrk24->handle);
 		} else {
 			SET_TARGET_ID(ha, mrk->target, loop_id);
 			mrk->lun = cpu_to_le16((uint16_t)lun);
@@ -1655,7 +1655,7 @@ qla24xx_start_scsi(srb_t *sp)
 	req->cnt -= req_cnt;
 
 	cmd_pkt = (struct cmd_type_7 *)req->ring_ptr;
-	cmd_pkt->handle = MAKE_HANDLE(req->id, handle);
+	cmd_pkt->handle = make_handle(req->id, handle);
 
 	/* Zero out remaining portion of packet. */
 	/*    tagged queuing modifier -- default is TSK_SIMPLE (0). */
@@ -1843,7 +1843,7 @@ qla24xx_dif_start_scsi(srb_t *sp)
 
 	/* Fill-in common area */
 	cmd_pkt = (struct cmd_type_crc_2 *)req->ring_ptr;
-	cmd_pkt->handle = MAKE_HANDLE(req->id, handle);
+	cmd_pkt->handle = make_handle(req->id, handle);
 
 	clr_ptr = (uint32_t *)cmd_pkt + 2;
 	memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8);
@@ -1975,7 +1975,7 @@ qla2xxx_start_scsi_mq(srb_t *sp)
 	req->cnt -= req_cnt;
 
 	cmd_pkt = (struct cmd_type_7 *)req->ring_ptr;
-	cmd_pkt->handle = MAKE_HANDLE(req->id, handle);
+	cmd_pkt->handle = make_handle(req->id, handle);
 
 	/* Zero out remaining portion of packet. */
 	/*    tagged queuing modifier -- default is TSK_SIMPLE (0). */
@@ -2178,7 +2178,7 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp)
 
 	/* Fill-in common area */
 	cmd_pkt = (struct cmd_type_crc_2 *)req->ring_ptr;
-	cmd_pkt->handle = MAKE_HANDLE(req->id, handle);
+	cmd_pkt->handle = make_handle(req->id, handle);
 
 	clr_ptr = (uint32_t *)cmd_pkt + 2;
 	memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8);
@@ -2489,7 +2489,7 @@ qla24xx_tm_iocb(srb_t *sp, struct tsk_mgmt_entry *tsk)
 
 	tsk->entry_type = TSK_MGMT_IOCB_TYPE;
 	tsk->entry_count = 1;
-	tsk->handle = MAKE_HANDLE(req->id, tsk->handle);
+	tsk->handle = make_handle(req->id, tsk->handle);
 	tsk->nport_handle = cpu_to_le16(fcport->loop_id);
 	tsk->timeout = cpu_to_le16(ha->r_a_tov / 10 * 2);
 	tsk->control_flags = cpu_to_le32(flags);
@@ -3358,7 +3358,7 @@ qla82xx_start_scsi(srb_t *sp)
 		}
 
 		cmd_pkt = (struct cmd_type_6 *)req->ring_ptr;
-		cmd_pkt->handle = MAKE_HANDLE(req->id, handle);
+		cmd_pkt->handle = make_handle(req->id, handle);
 
 		/* Zero out remaining portion of packet. */
 		/*    tagged queuing modifier -- default is TSK_SIMPLE (0). */
@@ -3429,7 +3429,7 @@ qla82xx_start_scsi(srb_t *sp)
 			goto queuing_error;
 
 		cmd_pkt = (struct cmd_type_7 *)req->ring_ptr;
-		cmd_pkt->handle = MAKE_HANDLE(req->id, handle);
+		cmd_pkt->handle = make_handle(req->id, handle);
 
 		/* Zero out remaining portion of packet. */
 		/* tagged queuing modifier -- default is TSK_SIMPLE (0).*/
@@ -3534,7 +3534,7 @@ qla24xx_abort_iocb(srb_t *sp, struct abort_entry_24xx *abt_iocb)
 	memset(abt_iocb, 0, sizeof(struct abort_entry_24xx));
 	abt_iocb->entry_type = ABORT_IOCB_TYPE;
 	abt_iocb->entry_count = 1;
-	abt_iocb->handle = cpu_to_le32(MAKE_HANDLE(req->id, sp->handle));
+	abt_iocb->handle = cpu_to_le32(make_handle(req->id, sp->handle));
 	if (sp->fcport) {
 		abt_iocb->nport_handle = cpu_to_le16(sp->fcport->loop_id);
 		abt_iocb->port_id[0] = sp->fcport->d_id.b.al_pa;
@@ -3542,7 +3542,7 @@ qla24xx_abort_iocb(srb_t *sp, struct abort_entry_24xx *abt_iocb)
 		abt_iocb->port_id[2] = sp->fcport->d_id.b.domain;
 	}
 	abt_iocb->handle_to_abort =
-	    cpu_to_le32(MAKE_HANDLE(aio->u.abt.req_que_no,
+	    cpu_to_le32(make_handle(aio->u.abt.req_que_no,
 				    aio->u.abt.cmd_hndl));
 	abt_iocb->vp_index = vha->vp_idx;
 	abt_iocb->req_que_no = cpu_to_le16(aio->u.abt.req_que_no);
@@ -3905,7 +3905,7 @@ qla2x00_start_bidir(srb_t *sp, struct scsi_qla_host *vha, uint32_t tot_dsds)
 	}
 
 	cmd_pkt = (struct cmd_bidir *)req->ring_ptr;
-	cmd_pkt->handle = MAKE_HANDLE(req->id, handle);
+	cmd_pkt->handle = make_handle(req->id, handle);
 
 	/* Zero out remaining portion of packet. */
 	/* tagged queuing modifier -- default is TSK_SIMPLE (0).*/
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index ff945d35ba8e..1563396ad658 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -2383,7 +2383,7 @@ qla24xx_login_fabric(scsi_qla_host_t *vha, uint16_t loop_id, uint8_t domain,
 
 	lg->entry_type = LOGINOUT_PORT_IOCB_TYPE;
 	lg->entry_count = 1;
-	lg->handle = MAKE_HANDLE(req->id, lg->handle);
+	lg->handle = make_handle(req->id, lg->handle);
 	lg->nport_handle = cpu_to_le16(loop_id);
 	lg->control_flags = cpu_to_le16(LCF_COMMAND_PLOGI);
 	if (opt & BIT_0)
@@ -2653,7 +2653,7 @@ qla24xx_fabric_logout(scsi_qla_host_t *vha, uint16_t loop_id, uint8_t domain,
 	req = vha->req;
 	lg->entry_type = LOGINOUT_PORT_IOCB_TYPE;
 	lg->entry_count = 1;
-	lg->handle = MAKE_HANDLE(req->id, lg->handle);
+	lg->handle = make_handle(req->id, lg->handle);
 	lg->nport_handle = cpu_to_le16(loop_id);
 	lg->control_flags =
 	    cpu_to_le16(LCF_COMMAND_LOGO|LCF_IMPL_LOGO|
@@ -3144,9 +3144,9 @@ qla24xx_abort_command(srb_t *sp)
 
 	abt->entry_type = ABORT_IOCB_TYPE;
 	abt->entry_count = 1;
-	abt->handle = MAKE_HANDLE(req->id, abt->handle);
+	abt->handle = make_handle(req->id, abt->handle);
 	abt->nport_handle = cpu_to_le16(fcport->loop_id);
-	abt->handle_to_abort = MAKE_HANDLE(req->id, handle);
+	abt->handle_to_abort = make_handle(req->id, handle);
 	abt->port_id[0] = fcport->d_id.b.al_pa;
 	abt->port_id[1] = fcport->d_id.b.area;
 	abt->port_id[2] = fcport->d_id.b.domain;
@@ -3223,7 +3223,7 @@ __qla24xx_issue_tmf(char *name, uint32_t type, struct fc_port *fcport,
 
 	tsk->p.tsk.entry_type = TSK_MGMT_IOCB_TYPE;
 	tsk->p.tsk.entry_count = 1;
-	tsk->p.tsk.handle = MAKE_HANDLE(req->id, tsk->p.tsk.handle);
+	tsk->p.tsk.handle = make_handle(req->id, tsk->p.tsk.handle);
 	tsk->p.tsk.nport_handle = cpu_to_le16(fcport->loop_id);
 	tsk->p.tsk.timeout = cpu_to_le16(ha->r_a_tov / 10 * 2);
 	tsk->p.tsk.control_flags = cpu_to_le32(type);
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index 6d120457478e..df99911b8bb9 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -3135,7 +3135,7 @@ qlafx00_start_scsi(srb_t *sp)
 
 	memset(&lcmd_pkt, 0, REQUEST_ENTRY_SIZE);
 
-	lcmd_pkt.handle = MAKE_HANDLE(req->id, sp->handle);
+	lcmd_pkt.handle = make_handle(req->id, sp->handle);
 	lcmd_pkt.reserved_0 = 0;
 	lcmd_pkt.port_path_ctrl = 0;
 	lcmd_pkt.reserved_1 = 0;
@@ -3205,7 +3205,7 @@ qlafx00_tm_iocb(srb_t *sp, struct tsk_mgmt_entry_fx00 *ptm_iocb)
 	memset(&tm_iocb, 0, sizeof(struct tsk_mgmt_entry_fx00));
 	tm_iocb.entry_type = TSK_MGMT_IOCB_TYPE_FX00;
 	tm_iocb.entry_count = 1;
-	tm_iocb.handle = cpu_to_le32(MAKE_HANDLE(req->id, sp->handle));
+	tm_iocb.handle = cpu_to_le32(make_handle(req->id, sp->handle));
 	tm_iocb.reserved_0 = 0;
 	tm_iocb.tgt_id = cpu_to_le16(sp->fcport->tgt_id);
 	tm_iocb.control_flags = cpu_to_le32(fxio->u.tmf.flags);
@@ -3231,9 +3231,9 @@ qlafx00_abort_iocb(srb_t *sp, struct abort_iocb_entry_fx00 *pabt_iocb)
 	memset(&abt_iocb, 0, sizeof(struct abort_iocb_entry_fx00));
 	abt_iocb.entry_type = ABORT_IOCB_TYPE_FX00;
 	abt_iocb.entry_count = 1;
-	abt_iocb.handle = cpu_to_le32(MAKE_HANDLE(req->id, sp->handle));
+	abt_iocb.handle = cpu_to_le32(make_handle(req->id, sp->handle));
 	abt_iocb.abort_handle =
-	    cpu_to_le32(MAKE_HANDLE(req->id, fxio->u.abt.cmd_hndl));
+	    cpu_to_le32(make_handle(req->id, fxio->u.abt.cmd_hndl));
 	abt_iocb.tgt_id_sts = cpu_to_le16(sp->fcport->tgt_id);
 	abt_iocb.req_que_no = cpu_to_le16(req->id);
 
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index bfcd02fdf2b8..84e2a980dea0 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -413,7 +413,7 @@ static inline int qla2x00_start_nvme_mq(srb_t *sp)
 	req->cnt -= req_cnt;
 
 	cmd_pkt = (struct cmd_nvme *)req->ring_ptr;
-	cmd_pkt->handle = MAKE_HANDLE(req->id, handle);
+	cmd_pkt->handle = make_handle(req->id, handle);
 
 	/* Zero out remaining portion of packet. */
 	clr_ptr = (uint32_t *)cmd_pkt + 2;
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 243f87df3d2b..d0dbddcef70f 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1758,7 +1758,7 @@ static int qlt_build_abts_resp_iocb(struct qla_tgt_mgmt_cmd *mcmd)
 		qpair->req->outstanding_cmds[h] = (srb_t *)mcmd;
 	}
 
-	resp->handle = MAKE_HANDLE(qpair->req->id, h);
+	resp->handle = make_handle(qpair->req->id, h);
 	resp->entry_type = ABTS_RESP_24XX;
 	resp->entry_count = 1;
 	resp->nport_handle = abts->nport_handle;
@@ -2580,7 +2580,7 @@ static int qlt_24xx_build_ctio_pkt(struct qla_qpair *qpair,
 	} else
 		qpair->req->outstanding_cmds[h] = (srb_t *)prm->cmd;
 
-	pkt->handle = MAKE_HANDLE(qpair->req->id, h);
+	pkt->handle = make_handle(qpair->req->id, h);
 	pkt->handle |= CTIO_COMPLETION_HANDLE_MARK;
 	pkt->nport_handle = cpu_to_le16(prm->cmd->loop_id);
 	pkt->timeout = cpu_to_le16(QLA_TGT_TIMEOUT);
@@ -3093,7 +3093,7 @@ qlt_build_ctio_crc2_pkt(struct qla_qpair *qpair, struct qla_tgt_prm *prm)
 	} else
 		qpair->req->outstanding_cmds[h] = (srb_t *)prm->cmd;
 
-	pkt->handle  = MAKE_HANDLE(qpair->req->id, h);
+	pkt->handle  = make_handle(qpair->req->id, h);
 	pkt->handle |= CTIO_COMPLETION_HANDLE_MARK;
 	pkt->nport_handle = cpu_to_le16(prm->cmd->loop_id);
 	pkt->timeout = cpu_to_le16(QLA_TGT_TIMEOUT);

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 5/5] qla2xxx: Fix the endianness annotations for the port attribute max_frame_size member
  2020-02-20  4:34 [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Bart Van Assche
                   ` (3 preceding siblings ...)
  2020-02-20  4:34 ` [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function Bart Van Assche
@ 2020-02-20  4:34 ` Bart Van Assche
  2020-02-25 19:34   ` Roman Bolshakov
  2020-02-29  1:10 ` [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Martin K. Petersen
  5 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-02-20  4:34 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Quinn Tran,
	Martin Wilck, Roman Bolshakov

Make sure that sparse doesn't complain about the statements that load or
store the port attribute max_frame_size member. The port attribute data
structures represent FC protocol data and hence use big endian format.
This patch only changes the meaning of two ql_dbg() statements.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_def.h |  4 ++--
 drivers/scsi/qla2xxx/qla_gs.c  | 14 ++++++--------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index cec0b5082927..c5c3a532f299 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -2741,7 +2741,7 @@ struct ct_fdmiv2_port_attr {
 		uint8_t fc4_types[32];
 		uint32_t sup_speed;
 		uint32_t cur_speed;
-		uint32_t max_frame_size;
+		__be32	max_frame_size;
 		uint8_t os_dev_name[32];
 		uint8_t host_name[256];
 		uint8_t node_name[WWN_SIZE];
@@ -2772,7 +2772,7 @@ struct ct_fdmi_port_attr {
 		uint8_t fc4_types[32];
 		uint32_t sup_speed;
 		uint32_t cur_speed;
-		uint32_t max_frame_size;
+		__be32	max_frame_size;
 		uint8_t os_dev_name[32];
 		uint8_t host_name[256];
 	} a;
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index aaa4a5bbf2ff..594b366db0ef 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -1848,14 +1848,13 @@ qla2x00_fdmi_rpa(scsi_qla_host_t *vha)
 	eiter = entries + size;
 	eiter->type = cpu_to_be16(FDMI_PORT_MAX_FRAME_SIZE);
 	eiter->len = cpu_to_be16(4 + 4);
-	eiter->a.max_frame_size = IS_FWI2_CAPABLE(ha) ?
+	eiter->a.max_frame_size = cpu_to_be32(IS_FWI2_CAPABLE(ha) ?
 	    le16_to_cpu(icb24->frame_payload_size) :
-	    le16_to_cpu(ha->init_cb->frame_payload_size);
-	eiter->a.max_frame_size = cpu_to_be32(eiter->a.max_frame_size);
+	    le16_to_cpu(ha->init_cb->frame_payload_size));
 	size += 4 + 4;
 
 	ql_dbg(ql_dbg_disc, vha, 0x203c,
-	    "Max_Frame_Size=%x.\n", eiter->a.max_frame_size);
+	       "Max_Frame_Size=%x.\n", be32_to_cpu(eiter->a.max_frame_size));
 
 	/* OS device name. */
 	eiter = entries + size;
@@ -2419,14 +2418,13 @@ qla2x00_fdmiv2_rpa(scsi_qla_host_t *vha)
 	eiter = entries + size;
 	eiter->type = cpu_to_be16(FDMI_PORT_MAX_FRAME_SIZE);
 	eiter->len = cpu_to_be16(4 + 4);
-	eiter->a.max_frame_size = IS_FWI2_CAPABLE(ha) ?
+	eiter->a.max_frame_size = cpu_to_be32(IS_FWI2_CAPABLE(ha) ?
 	    le16_to_cpu(icb24->frame_payload_size) :
-	    le16_to_cpu(ha->init_cb->frame_payload_size);
-	eiter->a.max_frame_size = cpu_to_be32(eiter->a.max_frame_size);
+	    le16_to_cpu(ha->init_cb->frame_payload_size));
 	size += 4 + 4;
 
 	ql_dbg(ql_dbg_disc, vha, 0x20bc,
-	    "Max_Frame_Size = %x.\n", eiter->a.max_frame_size);
+	       "Max_Frame_Size = %x.\n", be32_to_cpu(eiter->a.max_frame_size));
 
 	/* OS device name. */
 	eiter = entries + size;

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function
  2020-02-20  4:34 ` [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function Bart Van Assche
@ 2020-02-20  8:21   ` Daniel Wagner
  2020-02-20 16:28     ` Bart Van Assche
  2020-02-26  0:28   ` Roman Bolshakov
  1 sibling, 1 reply; 14+ messages in thread
From: Daniel Wagner @ 2020-02-20  8:21 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
	Quinn Tran, Martin Wilck, Roman Bolshakov

Hi Bart,

On Wed, Feb 19, 2020 at 08:34:40PM -0800, Bart Van Assche wrote:
> -#define MAKE_HANDLE(x, y) ((uint32_t)((((uint32_t)(x)) << 16) | (uint32_t)(y)))
> +static inline uint32_t make_handle(uint16_t x, uint16_t y)
> +{
> +	return ((uint32_t)x << 16) | y;
> +}
>  
>  /*
>   * I/O register
> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> index 47bf60a9490a..1816660768da 100644
> --- a/drivers/scsi/qla2xxx/qla_iocb.c
> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
> @@ -530,7 +530,7 @@ __qla2x00_marker(struct scsi_qla_host *vha, struct qla_qpair *qpair,
>  			int_to_scsilun(lun, (struct scsi_lun *)&mrk24->lun);
>  			host_to_fcp_swap(mrk24->lun, sizeof(mrk24->lun));
>  			mrk24->vp_index = vha->vp_idx;
> -			mrk24->handle = MAKE_HANDLE(req->id, mrk24->handle);
> +			mrk24->handle = make_handle(req->id, mrk24->handle);

The type of mrk24->handle is uint32_t and make_handle() is using type
uint16_t. Shouldn't the argument type for the y argument be uint32_t
as well?

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function
  2020-02-20  8:21   ` Daniel Wagner
@ 2020-02-20 16:28     ` Bart Van Assche
  2020-02-21 15:37       ` Daniel Wagner
  0 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-02-20 16:28 UTC (permalink / raw)
  To: Daniel Wagner
  Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
	Quinn Tran, Martin Wilck, Roman Bolshakov

On 2/20/20 12:21 AM, Daniel Wagner wrote:
> On Wed, Feb 19, 2020 at 08:34:40PM -0800, Bart Van Assche wrote:
>> -#define MAKE_HANDLE(x, y) ((uint32_t)((((uint32_t)(x)) << 16) | (uint32_t)(y)))
>> +static inline uint32_t make_handle(uint16_t x, uint16_t y)
>> +{
>> +	return ((uint32_t)x << 16) | y;
>> +}
>>   
>>   /*
>>    * I/O register
>> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
>> index 47bf60a9490a..1816660768da 100644
>> --- a/drivers/scsi/qla2xxx/qla_iocb.c
>> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
>> @@ -530,7 +530,7 @@ __qla2x00_marker(struct scsi_qla_host *vha, struct qla_qpair *qpair,
>>   			int_to_scsilun(lun, (struct scsi_lun *)&mrk24->lun);
>>   			host_to_fcp_swap(mrk24->lun, sizeof(mrk24->lun));
>>   			mrk24->vp_index = vha->vp_idx;
>> -			mrk24->handle = MAKE_HANDLE(req->id, mrk24->handle);
>> +			mrk24->handle = make_handle(req->id, mrk24->handle);
> 
> The type of mrk24->handle is uint32_t and make_handle() is using type
> uint16_t. Shouldn't the argument type for the y argument be uint32_t
> as well?

Hi Daniel,

As one can see in __qla2x00_marker() a value is assigned to 
mrk24->handle() by __qla2x00_alloc_iocbs(). That function calls 
qla2xxx_get_next_handle() to determine the 'handle' value. The 
implementation of that last function is as follows:

uint32_t qla2xxx_get_next_handle(struct req_que *req)
{
	uint32_t index, 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])
			return handle;
	}

	return 0;
}

Since 'num_outstanding_cmds' is a 16-bit variable I think that 
guarantees that the code quoted in your e-mail passes a 16-bit value as 
the second argument to make_handle().

Additionally, if the second argument to make_handle() would be larger 
than 0x10000, the following code from qla2x00_status_entry() would map 
sts->handle to another queue and another command than those through wich 
the command was submitted to the firmware:

	handle = (uint32_t) LSW(sts->handle);
	que = MSW(sts->handle);
	req = ha->req_q_map[que];

Bart.

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function
  2020-02-20 16:28     ` Bart Van Assche
@ 2020-02-21 15:37       ` Daniel Wagner
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Wagner @ 2020-02-21 15:37 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
	Quinn Tran, Martin Wilck, Roman Bolshakov

Hi Bart,

On Thu, Feb 20, 2020 at 08:28:16AM -0800, Bart Van Assche wrote:
> As one can see in __qla2x00_marker() a value is assigned to mrk24->handle()
> by __qla2x00_alloc_iocbs(). That function calls qla2xxx_get_next_handle() to
> determine the 'handle' value. The implementation of that last function is as
> follows:
> 
> uint32_t qla2xxx_get_next_handle(struct req_que *req)
> {
> 	uint32_t index, 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])
> 			return handle;
> 	}
> 
> 	return 0;
> }
> 
> Since 'num_outstanding_cmds' is a 16-bit variable I think that guarantees
> that the code quoted in your e-mail passes a 16-bit value as the second
> argument to make_handle().
> 
> Additionally, if the second argument to make_handle() would be larger than
> 0x10000, the following code from qla2x00_status_entry() would map
> sts->handle to another queue and another command than those through wich the
> command was submitted to the firmware:
> 
> 	handle = (uint32_t) LSW(sts->handle);
> 	que = MSW(sts->handle);
> 	req = ha->req_q_map[que];

Thanks for digging through it. I stopped at the function signature :)

Changing the return type of qla2xxx_get_next_handle() would be a new
patch.

In this case this patch is good.

Reviewed-by: Daniel Wagner <dwagner@suse.de>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/5] qla2xxx: Suppress endianness complaints in qla2x00_configure_local_loop()
  2020-02-20  4:34 ` [PATCH v3 2/5] qla2xxx: Suppress endianness complaints in qla2x00_configure_local_loop() Bart Van Assche
@ 2020-02-23 19:32   ` Roman Bolshakov
  0 siblings, 0 replies; 14+ messages in thread
From: Roman Bolshakov @ 2020-02-23 19:32 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
	Daniel Wagner, Quinn Tran, Martin Wilck

On Wed, Feb 19, 2020 at 08:34:38PM -0800, Bart Van Assche wrote:
> Instead of changing endianness in-place, write the data in CPU endian format
> in another buffer and copy that buffer back. This patch does not change any
> functionality but silences some sparse endianness warnings.
> 
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: Roman Bolshakov <r.bolshakov@yadro.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/qla2xxx/qla_def.h  |  2 +-
>  drivers/scsi/qla2xxx/qla_init.c | 11 +++++------
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index 9e6b56527b25..880f2be062a9 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -5068,13 +5068,12 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha)
>  			rval = qla24xx_get_port_login_templ(vha,
>  			    ha->init_cb_dma, (void *)ha->init_cb, sz);
>  			if (rval == QLA_SUCCESS) {
> +				__be32 *q = &ha->plogi_els_payld.data[0];
> +
>  				bp = (uint32_t *)ha->init_cb;
> -				for (i = 0; i < sz/4 ; i++, bp++)
> -					*bp = cpu_to_be32(*bp);
> +				cpu_to_be32_array(q, bp, sz / 4);
>  
> -				memcpy(&ha->plogi_els_payld.data,
> -				    (void *)ha->init_cb,
> -				    sizeof(ha->plogi_els_payld.data));
> +				memcpy(bp, q, sizeof(ha->plogi_els_payld.data));

Hi Bart,

Honestly, I'd prefer to drop the memcpy as it has no purpose, but since
you replicated a side-effect of the previous code and fixed sparse
warnings,

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>

Thank you,
Roman

P.S.

During the review, I looked again where init_cb is used. It seems the
reuse and rewriting of init_cb contents by qla24xx_get_port_login_templ
may corrupt init_cb values expected by qla2x00_fdmi_rpa and
qla2x00_fdmiv2_rpa (they retrieve max frame size from Initialization
Control Block), and qla2x00_async_event (recovers WWPN from init_cb in
case of Fabric-assigned WWPN).

That _rpa functions are invoked if a port is in fabric topology and FDMI
registration is enabled, so a change of topology from P2P to Switched
Fabric may result in FDMI registration with wrong max frame size. I
don't know how FC switches use the value though.

If the fabric assigns WWPNs (FA-WWPN) and a disconnect happens (Loop
Down Event), the driver will try to recover WWPN from init_cb that is
corrupted by qla24xx_get_port_login_templ.

Probably, the wrong state may be recovered by reclaiming the underlying
PCI device. init_cb is set to correct values in probe() handler.

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/5] qla2xxx: Fix sparse warnings triggered by the PCI state checking code
  2020-02-20  4:34 ` [PATCH v3 3/5] qla2xxx: Fix sparse warnings triggered by the PCI state checking code Bart Van Assche
@ 2020-02-25 19:13   ` Roman Bolshakov
  0 siblings, 0 replies; 14+ messages in thread
From: Roman Bolshakov @ 2020-02-25 19:13 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
	Daniel Wagner, Quinn Tran, Martin Wilck

On Wed, Feb 19, 2020 at 08:34:39PM -0800, Bart Van Assche wrote:
> This patch fixes the following sparse warnings:
> 
> drivers/scsi/qla2xxx/qla_mbx.c:120:21: warning: restricted pci_channel_state_t degrades to integer
> drivers/scsi/qla2xxx/qla_mbx.c:120:37: warning: restricted pci_channel_state_t degrades to integer
> 
> From include/linux/pci.h:
> 
> enum pci_channel_state {
> 	/* I/O channel is in normal state */
> 	pci_channel_io_normal = (__force pci_channel_state_t) 1,
> 
> 	/* I/O to channel is blocked */
> 	pci_channel_io_frozen = (__force pci_channel_state_t) 2,
> 
> 	/* PCI card is dead */
> 	pci_channel_io_perm_failure = (__force pci_channel_state_t) 3,
> };
> 
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: Roman Bolshakov <r.bolshakov@yadro.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/qla2xxx/qla_mbx.c | 5 ++---
>  drivers/scsi/qla2xxx/qla_mr.c  | 5 ++---
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 

I'm sorry, replied to wrong version in previous email,

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 5/5] qla2xxx: Fix the endianness annotations for the port attribute max_frame_size member
  2020-02-20  4:34 ` [PATCH v3 5/5] qla2xxx: Fix the endianness annotations for the port attribute max_frame_size member Bart Van Assche
@ 2020-02-25 19:34   ` Roman Bolshakov
  0 siblings, 0 replies; 14+ messages in thread
From: Roman Bolshakov @ 2020-02-25 19:34 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
	Daniel Wagner, Quinn Tran, Martin Wilck

On Wed, Feb 19, 2020 at 08:34:41PM -0800, Bart Van Assche wrote:
> Make sure that sparse doesn't complain about the statements that load or
> store the port attribute max_frame_size member. The port attribute data
> structures represent FC protocol data and hence use big endian format.
> This patch only changes the meaning of two ql_dbg() statements.
> 
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: Roman Bolshakov <r.bolshakov@yadro.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/qla2xxx/qla_def.h |  4 ++--
>  drivers/scsi/qla2xxx/qla_gs.c  | 14 ++++++--------
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 

Hi Bart,

As I said in reply to previous version, IMO, all four-byte binary fields
and bitmasks in RPA request should be made __be32 rather than only one.
Probably that may go into a patch series where each field in the
structure is fixed patch-by-patch or one patch that fixes all fields at
once.

According to Table 402 – Port Attribute Values in FC-GS-7 and RPA
request attributes in the structure above, that includes:
        Supported Speed
        Current Port Speed
        Maximum Frame Size
        Port Type
        Supported Classes of Service
        Port State
        Number of Discovered Ports
        Port Identifier

But the patch itself looks good,
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>

Regards,
Roman

> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index cec0b5082927..c5c3a532f299 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -2741,7 +2741,7 @@ struct ct_fdmiv2_port_attr {
>  		uint8_t fc4_types[32];
>  		uint32_t sup_speed;
>  		uint32_t cur_speed;
> -		uint32_t max_frame_size;
> +		__be32	max_frame_size;
>  		uint8_t os_dev_name[32];
>  		uint8_t host_name[256];
>  		uint8_t node_name[WWN_SIZE];
> @@ -2772,7 +2772,7 @@ struct ct_fdmi_port_attr {
>  		uint8_t fc4_types[32];
>  		uint32_t sup_speed;
>  		uint32_t cur_speed;
> -		uint32_t max_frame_size;
> +		__be32	max_frame_size;
>  		uint8_t os_dev_name[32];
>  		uint8_t host_name[256];
>  	} a;
> diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
> index aaa4a5bbf2ff..594b366db0ef 100644
> --- a/drivers/scsi/qla2xxx/qla_gs.c
> +++ b/drivers/scsi/qla2xxx/qla_gs.c
> @@ -1848,14 +1848,13 @@ qla2x00_fdmi_rpa(scsi_qla_host_t *vha)
>  	eiter = entries + size;
>  	eiter->type = cpu_to_be16(FDMI_PORT_MAX_FRAME_SIZE);
>  	eiter->len = cpu_to_be16(4 + 4);
> -	eiter->a.max_frame_size = IS_FWI2_CAPABLE(ha) ?
> +	eiter->a.max_frame_size = cpu_to_be32(IS_FWI2_CAPABLE(ha) ?
>  	    le16_to_cpu(icb24->frame_payload_size) :
> -	    le16_to_cpu(ha->init_cb->frame_payload_size);
> -	eiter->a.max_frame_size = cpu_to_be32(eiter->a.max_frame_size);
> +	    le16_to_cpu(ha->init_cb->frame_payload_size));
>  	size += 4 + 4;
>  
>  	ql_dbg(ql_dbg_disc, vha, 0x203c,
> -	    "Max_Frame_Size=%x.\n", eiter->a.max_frame_size);
> +	       "Max_Frame_Size=%x.\n", be32_to_cpu(eiter->a.max_frame_size));
>  
>  	/* OS device name. */
>  	eiter = entries + size;
> @@ -2419,14 +2418,13 @@ qla2x00_fdmiv2_rpa(scsi_qla_host_t *vha)
>  	eiter = entries + size;
>  	eiter->type = cpu_to_be16(FDMI_PORT_MAX_FRAME_SIZE);
>  	eiter->len = cpu_to_be16(4 + 4);
> -	eiter->a.max_frame_size = IS_FWI2_CAPABLE(ha) ?
> +	eiter->a.max_frame_size = cpu_to_be32(IS_FWI2_CAPABLE(ha) ?
>  	    le16_to_cpu(icb24->frame_payload_size) :
> -	    le16_to_cpu(ha->init_cb->frame_payload_size);
> -	eiter->a.max_frame_size = cpu_to_be32(eiter->a.max_frame_size);
> +	    le16_to_cpu(ha->init_cb->frame_payload_size));
>  	size += 4 + 4;
>  
>  	ql_dbg(ql_dbg_disc, vha, 0x20bc,
> -	    "Max_Frame_Size = %x.\n", eiter->a.max_frame_size);
> +	       "Max_Frame_Size = %x.\n", be32_to_cpu(eiter->a.max_frame_size));
>  
>  	/* OS device name. */
>  	eiter = entries + size;

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function
  2020-02-20  4:34 ` [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function Bart Van Assche
  2020-02-20  8:21   ` Daniel Wagner
@ 2020-02-26  0:28   ` Roman Bolshakov
  1 sibling, 0 replies; 14+ messages in thread
From: Roman Bolshakov @ 2020-02-26  0:28 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
	Quinn Tran, Martin Wilck, Daniel Wagner

On Wed, Feb 19, 2020 at 08:34:40PM -0800, Bart Van Assche wrote:
> This patch allows sparse to verify the endianness of the arguments passed
> to make_handle().
> 
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Roman Bolshakov <r.bolshakov@yadro.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/qla2xxx/qla_def.h    |  5 ++++-
>  drivers/scsi/qla2xxx/qla_iocb.c   | 22 +++++++++++-----------
>  drivers/scsi/qla2xxx/qla_mbx.c    | 10 +++++-----
>  drivers/scsi/qla2xxx/qla_mr.c     |  8 ++++----
>  drivers/scsi/qla2xxx/qla_nvme.c   |  2 +-
>  drivers/scsi/qla2xxx/qla_target.c |  6 +++---
>  6 files changed, 28 insertions(+), 25 deletions(-)
> 

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>

Thanks,
Roman

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/5] qla2xxx patches for kernel v5.7
  2020-02-20  4:34 [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Bart Van Assche
                   ` (4 preceding siblings ...)
  2020-02-20  4:34 ` [PATCH v3 5/5] qla2xxx: Fix the endianness annotations for the port attribute max_frame_size member Bart Van Assche
@ 2020-02-29  1:10 ` Martin K. Petersen
  5 siblings, 0 replies; 14+ messages in thread
From: Martin K. Petersen @ 2020-02-29  1:10 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi


Bart,

> Please consider these five qla2xxx patches for kernel v5.7.

Applied 1-4 to 5.7/scsi-queue. Patch 5 doesn't apply on top of the
latest driver update so please rebase.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-02-29  1:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20  4:34 [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Bart Van Assche
2020-02-20  4:34 ` [PATCH v3 1/5] qla2xxx: Simplify the code for aborting SCSI commands Bart Van Assche
2020-02-20  4:34 ` [PATCH v3 2/5] qla2xxx: Suppress endianness complaints in qla2x00_configure_local_loop() Bart Van Assche
2020-02-23 19:32   ` Roman Bolshakov
2020-02-20  4:34 ` [PATCH v3 3/5] qla2xxx: Fix sparse warnings triggered by the PCI state checking code Bart Van Assche
2020-02-25 19:13   ` Roman Bolshakov
2020-02-20  4:34 ` [PATCH v3 4/5] qla2xxx: Convert MAKE_HANDLE() from a define into an inline function Bart Van Assche
2020-02-20  8:21   ` Daniel Wagner
2020-02-20 16:28     ` Bart Van Assche
2020-02-21 15:37       ` Daniel Wagner
2020-02-26  0:28   ` Roman Bolshakov
2020-02-20  4:34 ` [PATCH v3 5/5] qla2xxx: Fix the endianness annotations for the port attribute max_frame_size member Bart Van Assche
2020-02-25 19:34   ` Roman Bolshakov
2020-02-29  1:10 ` [PATCH v3 0/5] qla2xxx patches for kernel v5.7 Martin K. Petersen

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