* [PATCH v2 0/6] qla2xxx patches for kernel v5.12 and v5.13
@ 2021-03-18 3:28 Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 1/6] Revert "qla2xxx: Make sure that aborted commands are freed" Bart Van Assche
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 3:28 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley; +Cc: linux-scsi, Bart Van Assche
Hi Martin,
Please consider the first patch in this series for kernel v5.12 and the
remaining patches for kernel v5.13.
Thanks,
Bart.
Changes compared to v1:
- Improved the description of patch 1/6.
- Dropped one patch that is already upstream.
Bart Van Assche (6):
Revert "qla2xxx: Make sure that aborted commands are freed"
qla2xxx: Constify struct qla_tgt_func_tmpl
qla2xxx: Fix endianness annotations
qla2xxx: Suppress Coverity complaints about dseg_r*
qla2xxx: Simplify qla8044_minidump_process_control()
qla2xxx: Always check the return value of qla24xx_get_isp_stats()
drivers/scsi/qla2xxx/qla_attr.c | 6 +++++-
drivers/scsi/qla2xxx/qla_def.h | 4 ++--
drivers/scsi/qla2xxx/qla_iocb.c | 3 ++-
drivers/scsi/qla2xxx/qla_isr.c | 2 +-
drivers/scsi/qla2xxx/qla_mr.c | 12 ++++++------
drivers/scsi/qla2xxx/qla_mr.h | 4 ++--
drivers/scsi/qla2xxx/qla_nx2.c | 8 --------
drivers/scsi/qla2xxx/qla_sup.c | 9 +++++----
drivers/scsi/qla2xxx/qla_target.c | 13 +++++--------
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 6 +-----
10 files changed, 29 insertions(+), 38 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/6] Revert "qla2xxx: Make sure that aborted commands are freed"
2021-03-18 3:28 [PATCH v2 0/6] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
@ 2021-03-18 3:28 ` Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 2/6] qla2xxx: Constify struct qla_tgt_func_tmpl Bart Van Assche
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 3:28 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani
Calling vha->hw->tgt.tgt_ops->free_cmd() from qlt_xmit_response() is wrong
since the command for which a response is sent must remain valid until the
SCSI target core calls .release_cmd(). It has been observed that the
following scenario triggers a kernel crash:
- qlt_xmit_response() calls qlt_check_reserve_free_req().
- qlt_check_reserve_free_req() returns -EAGAIN.
- qlt_xmit_response() calls vha->hw->tgt.tgt_ops->free_cmd(cmd).
- transport_handle_queue_full() tries to retransmit the response.
Fix this crash by reverting the patch that introduced it.
Fixes: 0dcec41acb85 ("scsi: qla2xxx: Make sure that aborted commands are freed")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_target.c | 13 +++++--------
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 ----
2 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index d6366a46283e..5e8b2653e134 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3222,8 +3222,7 @@ 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;
- res = 0;
- goto free;
+ return 0;
}
ql_dbg_qp(ql_dbg_tgt, qpair, 0xe018,
@@ -3234,8 +3233,9 @@ 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))
- goto free;
+ if (unlikely(res != 0)) {
+ return res;
+ }
spin_lock_irqsave(qpair->qp_lock_ptr, flags);
@@ -3255,8 +3255,7 @@ 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);
- res = 0;
- goto free;
+ return 0;
}
/* Does F/W have an IOCBs for this request */
@@ -3359,8 +3358,6 @@ 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);
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 30959f8da065..15650a0bde09 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -653,7 +653,6 @@ 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
@@ -666,7 +665,6 @@ 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;
}
@@ -694,7 +692,6 @@ 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) {
@@ -708,7 +705,6 @@ 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;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/6] qla2xxx: Constify struct qla_tgt_func_tmpl
2021-03-18 3:28 [PATCH v2 0/6] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 1/6] Revert "qla2xxx: Make sure that aborted commands are freed" Bart Van Assche
@ 2021-03-18 3:28 ` Bart Van Assche
2021-03-18 16:01 ` Lee Duncan
2021-03-18 3:28 ` [PATCH v2 3/6] qla2xxx: Fix endianness annotations Bart Van Assche
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 3:28 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani
Since the target function pointers are not modified at runtime, declare
the data structure with the target function pointers const.
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_def.h | 2 +-
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 49b42b430df4..3bdf55bb0833 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -3815,7 +3815,7 @@ struct qlt_hw_data {
__le32 __iomem *atio_q_in;
__le32 __iomem *atio_q_out;
- struct qla_tgt_func_tmpl *tgt_ops;
+ const struct qla_tgt_func_tmpl *tgt_ops;
struct qla_tgt_vp_map *tgt_vp_map;
int saved_set;
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 15650a0bde09..46111f031be9 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1578,7 +1578,7 @@ static void tcm_qla2xxx_update_sess(struct fc_port *sess, port_id_t s_id,
/*
* Calls into tcm_qla2xxx used by qla2xxx LLD I/O path.
*/
-static struct qla_tgt_func_tmpl tcm_qla2xxx_template = {
+static const struct qla_tgt_func_tmpl tcm_qla2xxx_template = {
.find_cmd_by_tag = tcm_qla2xxx_find_cmd_by_tag,
.handle_cmd = tcm_qla2xxx_handle_cmd,
.handle_data = tcm_qla2xxx_handle_data,
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/6] qla2xxx: Fix endianness annotations
2021-03-18 3:28 [PATCH v2 0/6] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 1/6] Revert "qla2xxx: Make sure that aborted commands are freed" Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 2/6] qla2xxx: Constify struct qla_tgt_func_tmpl Bart Van Assche
@ 2021-03-18 3:28 ` Bart Van Assche
2021-03-18 16:01 ` Lee Duncan
2021-03-18 3:28 ` [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r* Bart Van Assche
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 3:28 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani
Fix all recently introduced endianness annotation issues.
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_def.h | 2 +-
drivers/scsi/qla2xxx/qla_iocb.c | 3 ++-
drivers/scsi/qla2xxx/qla_isr.c | 2 +-
drivers/scsi/qla2xxx/qla_sup.c | 9 +++++----
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 3bdf55bb0833..52ba75591f9a 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -1527,7 +1527,7 @@ struct init_sf_cb {
* BIT_12 = Remote Write Optimization (1 - Enabled, 0 - Disabled)
* BIT 11-0 = Reserved
*/
- uint16_t flags;
+ __le16 flags;
uint8_t reserved1[32];
uint16_t discard_OHRB_timeout_value;
uint16_t remote_write_opt_queue_num;
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 8b41cbaf8535..eb2376b138c1 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2379,7 +2379,8 @@ qla24xx_prli_iocb(srb_t *sp, struct logio_entry_24xx *logio)
cpu_to_le32(NVME_PRLI_SP_FIRST_BURST);
if (sp->vha->flags.nvme2_enabled) {
/* Set service parameter BIT_7 for NVME CONF support */
- logio->io_parameter[0] |= NVME_PRLI_SP_CONF;
+ logio->io_parameter[0] |=
+ cpu_to_le32(NVME_PRLI_SP_CONF);
/* Set service parameter BIT_8 for SLER support */
logio->io_parameter[0] |=
cpu_to_le32(NVME_PRLI_SP_SLER);
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 27165abda96d..0fa7082f3cc8 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -3440,7 +3440,7 @@ qla24xx_abort_iocb_entry(scsi_qla_host_t *vha, struct req_que *req,
return;
abt = &sp->u.iocb_cmd;
- abt->u.abt.comp_status = le16_to_cpu(pkt->comp_status);
+ abt->u.abt.comp_status = pkt->comp_status;
orig_sp = sp->cmd_sp;
/* Need to pass original sp */
if (orig_sp)
diff --git a/drivers/scsi/qla2xxx/qla_sup.c b/drivers/scsi/qla2xxx/qla_sup.c
index f771fabcba59..060c89237777 100644
--- a/drivers/scsi/qla2xxx/qla_sup.c
+++ b/drivers/scsi/qla2xxx/qla_sup.c
@@ -2621,10 +2621,11 @@ qla24xx_read_optrom_data(struct scsi_qla_host *vha, void *buf,
}
static int
-qla28xx_extract_sfub_and_verify(struct scsi_qla_host *vha, uint32_t *buf,
+qla28xx_extract_sfub_and_verify(struct scsi_qla_host *vha, __le32 *buf,
uint32_t len, uint32_t buf_size_without_sfub, uint8_t *sfub_buf)
{
- uint32_t *p, check_sum = 0;
+ uint32_t check_sum = 0;
+ __le32 *p;
int i;
p = buf + buf_size_without_sfub;
@@ -2790,8 +2791,8 @@ qla28xx_write_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t faddr,
goto done;
}
- rval = qla28xx_extract_sfub_and_verify(vha, dwptr, dwords,
- buf_size_without_sfub, (uint8_t *)sfub);
+ rval = qla28xx_extract_sfub_and_verify(vha, (__le32 *)dwptr,
+ dwords, buf_size_without_sfub, (uint8_t *)sfub);
if (rval != QLA_SUCCESS)
goto done;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r*
2021-03-18 3:28 [PATCH v2 0/6] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (2 preceding siblings ...)
2021-03-18 3:28 ` [PATCH v2 3/6] qla2xxx: Fix endianness annotations Bart Van Assche
@ 2021-03-18 3:28 ` Bart Van Assche
2021-03-18 16:00 ` Lee Duncan
2021-03-18 3:28 ` [PATCH v2 5/6] qla2xxx: Simplify qla8044_minidump_process_control() Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats() Bart Van Assche
5 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 3:28 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani
Change dseq_rq and dseg_rsp from scalar structure members into
single-element arrays such that Coverity does not complain about the
(*cur_dsd)++ statement in append_dsd64().
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_mr.c | 12 ++++++------
drivers/scsi/qla2xxx/qla_mr.h | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index ca7306685325..61eaf6c4ac47 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -3266,8 +3266,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
fx_iocb.req_xfrcnt =
cpu_to_le16(fxio->u.fxiocb.req_len);
put_unaligned_le64(fxio->u.fxiocb.req_dma_handle,
- &fx_iocb.dseg_rq.address);
- fx_iocb.dseg_rq.length =
+ &fx_iocb.dseg_rq[0].address);
+ fx_iocb.dseg_rq[0].length =
cpu_to_le32(fxio->u.fxiocb.req_len);
}
@@ -3276,8 +3276,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
fx_iocb.rsp_xfrcnt =
cpu_to_le16(fxio->u.fxiocb.rsp_len);
put_unaligned_le64(fxio->u.fxiocb.rsp_dma_handle,
- &fx_iocb.dseg_rsp.address);
- fx_iocb.dseg_rsp.length =
+ &fx_iocb.dseg_rsp[0].address);
+ fx_iocb.dseg_rsp[0].length =
cpu_to_le32(fxio->u.fxiocb.rsp_len);
}
@@ -3314,7 +3314,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
cpu_to_le16(bsg_job->request_payload.sg_cnt);
tot_dsds =
bsg_job->request_payload.sg_cnt;
- cur_dsd = &fx_iocb.dseg_rq;
+ cur_dsd = &fx_iocb.dseg_rq[0];
avail_dsds = 1;
for_each_sg(bsg_job->request_payload.sg_list, sg,
tot_dsds, index) {
@@ -3369,7 +3369,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
fx_iocb.rsp_dsdcnt =
cpu_to_le16(bsg_job->reply_payload.sg_cnt);
tot_dsds = bsg_job->reply_payload.sg_cnt;
- cur_dsd = &fx_iocb.dseg_rsp;
+ cur_dsd = &fx_iocb.dseg_rsp[0];
avail_dsds = 1;
for_each_sg(bsg_job->reply_payload.sg_list, sg,
diff --git a/drivers/scsi/qla2xxx/qla_mr.h b/drivers/scsi/qla2xxx/qla_mr.h
index 73be8348402a..eefbae9d7547 100644
--- a/drivers/scsi/qla2xxx/qla_mr.h
+++ b/drivers/scsi/qla2xxx/qla_mr.h
@@ -176,8 +176,8 @@ struct fxdisc_entry_fx00 {
uint8_t flags;
uint8_t reserved_1;
- struct dsd64 dseg_rq;
- struct dsd64 dseg_rsp;
+ struct dsd64 dseg_rq[1];
+ struct dsd64 dseg_rsp[1];
__le32 dataword;
__le32 adapid;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 5/6] qla2xxx: Simplify qla8044_minidump_process_control()
2021-03-18 3:28 [PATCH v2 0/6] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (3 preceding siblings ...)
2021-03-18 3:28 ` [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r* Bart Van Assche
@ 2021-03-18 3:28 ` Bart Van Assche
2021-03-18 16:03 ` Lee Duncan
2021-03-18 3:28 ` [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats() Bart Van Assche
5 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 3:28 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani
This patch fixes the following Coverity complaint:
CID 177490 (#1 of 1): Unused value (UNUSED_VALUE)
assigned_value: Assigning value from opcode & 0xffffff7fU to opcode
here, but that stored value is overwritten before it can be used.
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_nx2.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_nx2.c b/drivers/scsi/qla2xxx/qla_nx2.c
index 68a16c95dcb7..792c55fcec8c 100644
--- a/drivers/scsi/qla2xxx/qla_nx2.c
+++ b/drivers/scsi/qla2xxx/qla_nx2.c
@@ -2226,19 +2226,16 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
if (opcode & QLA82XX_DBG_OPCODE_WR) {
qla8044_wr_reg_indirect(vha, crb_addr,
crb_entry->value_1);
- opcode &= ~QLA82XX_DBG_OPCODE_WR;
}
if (opcode & QLA82XX_DBG_OPCODE_RW) {
qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
qla8044_wr_reg_indirect(vha, crb_addr, read_value);
- opcode &= ~QLA82XX_DBG_OPCODE_RW;
}
if (opcode & QLA82XX_DBG_OPCODE_AND) {
qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
read_value &= crb_entry->value_2;
- opcode &= ~QLA82XX_DBG_OPCODE_AND;
if (opcode & QLA82XX_DBG_OPCODE_OR) {
read_value |= crb_entry->value_3;
opcode &= ~QLA82XX_DBG_OPCODE_OR;
@@ -2249,7 +2246,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
read_value |= crb_entry->value_3;
qla8044_wr_reg_indirect(vha, crb_addr, read_value);
- opcode &= ~QLA82XX_DBG_OPCODE_OR;
}
if (opcode & QLA82XX_DBG_OPCODE_POLL) {
poll_time = crb_entry->crb_strd.poll_timeout;
@@ -2269,7 +2265,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
crb_addr, &read_value);
}
} while (1);
- opcode &= ~QLA82XX_DBG_OPCODE_POLL;
}
if (opcode & QLA82XX_DBG_OPCODE_RDSTATE) {
@@ -2283,7 +2278,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
qla8044_rd_reg_indirect(vha, addr, &read_value);
index = crb_entry->crb_ctrl.state_index_v;
tmplt_hdr->saved_state_array[index] = read_value;
- opcode &= ~QLA82XX_DBG_OPCODE_RDSTATE;
}
if (opcode & QLA82XX_DBG_OPCODE_WRSTATE) {
@@ -2303,7 +2297,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
}
qla8044_wr_reg_indirect(vha, addr, read_value);
- opcode &= ~QLA82XX_DBG_OPCODE_WRSTATE;
}
if (opcode & QLA82XX_DBG_OPCODE_MDSTATE) {
@@ -2316,7 +2309,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
read_value |= crb_entry->value_3;
read_value += crb_entry->value_1;
tmplt_hdr->saved_state_array[index] = read_value;
- opcode &= ~QLA82XX_DBG_OPCODE_MDSTATE;
}
crb_addr += crb_entry->crb_strd.addr_stride;
}
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
2021-03-18 3:28 [PATCH v2 0/6] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (4 preceding siblings ...)
2021-03-18 3:28 ` [PATCH v2 5/6] qla2xxx: Simplify qla8044_minidump_process_control() Bart Van Assche
@ 2021-03-18 3:28 ` Bart Van Assche
2021-03-18 8:08 ` Daniel Wagner
5 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 3:28 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Himanshu Madhani, Daniel Wagner
This patch fixes the following Coverity warning:
CID 361199 (#1 of 1): Unchecked return value (CHECKED_RETURN)
3. check_return: Calling qla24xx_get_isp_stats without checking return
value (as is done elsewhere 4 out of 5 times).
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_attr.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 63391c9be05d..5813b17f1633 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -2864,6 +2864,8 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost)
vha->qla_stats.jiffies_at_last_reset = get_jiffies_64();
if (IS_FWI2_CAPABLE(ha)) {
+ int rval;
+
stats = dma_alloc_coherent(&ha->pdev->dev,
sizeof(*stats), &stats_dma, GFP_KERNEL);
if (!stats) {
@@ -2873,7 +2875,9 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost)
}
/* reset firmware statistics */
- qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
+ rval = qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
+ ql_log(ql_log_warn, vha, 0x70d9,
+ "Resetting ISP statistics failed: rval = %d\n", rval);
dma_free_coherent(&ha->pdev->dev, sizeof(*stats),
stats, stats_dma);
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
2021-03-18 3:28 ` [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats() Bart Van Assche
@ 2021-03-18 8:08 ` Daniel Wagner
2021-03-18 19:11 ` Bart Van Assche
0 siblings, 1 reply; 14+ messages in thread
From: Daniel Wagner @ 2021-03-18 8:08 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
Quinn Tran, Mike Christie, Himanshu Madhani
On Wed, Mar 17, 2021 at 08:28:40PM -0700, Bart Van Assche wrote:
> @@ -2873,7 +2875,9 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost)
> }
>
> /* reset firmware statistics */
> - qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
> + rval = qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
> + ql_log(ql_log_warn, vha, 0x70d9,
> + "Resetting ISP statistics failed: rval = %d\n", rval);
Is there not an 'if' missing?
if (rval)
ql_log(...)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r*
2021-03-18 3:28 ` [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r* Bart Van Assche
@ 2021-03-18 16:00 ` Lee Duncan
2021-03-18 19:10 ` Bart Van Assche
0 siblings, 1 reply; 14+ messages in thread
From: Lee Duncan @ 2021-03-18 16:00 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Quinn Tran, Mike Christie, Daniel Wagner, Himanshu Madhani
On 3/17/21 8:28 PM, Bart Van Assche wrote:
> Change dseq_rq and dseg_rsp from scalar structure members into
> single-element arrays such that Coverity does not complain about the
> (*cur_dsd)++ statement in append_dsd64().
>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/qla2xxx/qla_mr.c | 12 ++++++------
> drivers/scsi/qla2xxx/qla_mr.h | 4 ++--
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
> index ca7306685325..61eaf6c4ac47 100644
> --- a/drivers/scsi/qla2xxx/qla_mr.c
> +++ b/drivers/scsi/qla2xxx/qla_mr.c
> @@ -3266,8 +3266,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
> fx_iocb.req_xfrcnt =
> cpu_to_le16(fxio->u.fxiocb.req_len);
> put_unaligned_le64(fxio->u.fxiocb.req_dma_handle,
> - &fx_iocb.dseg_rq.address);
> - fx_iocb.dseg_rq.length =
> + &fx_iocb.dseg_rq[0].address);
> + fx_iocb.dseg_rq[0].length =
> cpu_to_le32(fxio->u.fxiocb.req_len);
> }
>
> @@ -3276,8 +3276,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
> fx_iocb.rsp_xfrcnt =
> cpu_to_le16(fxio->u.fxiocb.rsp_len);
> put_unaligned_le64(fxio->u.fxiocb.rsp_dma_handle,
> - &fx_iocb.dseg_rsp.address);
> - fx_iocb.dseg_rsp.length =
> + &fx_iocb.dseg_rsp[0].address);
> + fx_iocb.dseg_rsp[0].length =
> cpu_to_le32(fxio->u.fxiocb.rsp_len);
> }
>
> @@ -3314,7 +3314,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
> cpu_to_le16(bsg_job->request_payload.sg_cnt);
> tot_dsds =
> bsg_job->request_payload.sg_cnt;
> - cur_dsd = &fx_iocb.dseg_rq;
> + cur_dsd = &fx_iocb.dseg_rq[0];
> avail_dsds = 1;
> for_each_sg(bsg_job->request_payload.sg_list, sg,
> tot_dsds, index) {
> @@ -3369,7 +3369,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
> fx_iocb.rsp_dsdcnt =
> cpu_to_le16(bsg_job->reply_payload.sg_cnt);
> tot_dsds = bsg_job->reply_payload.sg_cnt;
> - cur_dsd = &fx_iocb.dseg_rsp;
> + cur_dsd = &fx_iocb.dseg_rsp[0];
> avail_dsds = 1;
>
> for_each_sg(bsg_job->reply_payload.sg_list, sg,
> diff --git a/drivers/scsi/qla2xxx/qla_mr.h b/drivers/scsi/qla2xxx/qla_mr.h
> index 73be8348402a..eefbae9d7547 100644
> --- a/drivers/scsi/qla2xxx/qla_mr.h
> +++ b/drivers/scsi/qla2xxx/qla_mr.h
> @@ -176,8 +176,8 @@ struct fxdisc_entry_fx00 {
> uint8_t flags;- &
> uint8_t reserved_1;
>
> - struct dsd64 dseg_rq;
> - struct dsd64 dseg_rsp;
> + struct dsd64 dseg_rq[1];
> + struct dsd64 dseg_rsp[1];
>
> __le32 dataword;
> __le32 adapid;
>
I dislike such an uncommented change just to keep a tool happy. Could
you add a comment in qla_mr.h saying why these are one-element-long
arrays, just so nobody optimizes those out later?
If you do that, please add my:
Reviewed-by: Lee Duncan <lduncan@suse.com>
--
Lee Duncan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/6] qla2xxx: Constify struct qla_tgt_func_tmpl
2021-03-18 3:28 ` [PATCH v2 2/6] qla2xxx: Constify struct qla_tgt_func_tmpl Bart Van Assche
@ 2021-03-18 16:01 ` Lee Duncan
0 siblings, 0 replies; 14+ messages in thread
From: Lee Duncan @ 2021-03-18 16:01 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Quinn Tran, Mike Christie, Daniel Wagner, Himanshu Madhani
On 3/17/21 8:28 PM, Bart Van Assche wrote:
> Since the target function pointers are not modified at runtime, declare
> the data structure with the target function pointers const.
>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/qla2xxx/qla_def.h | 2 +-
> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index 49b42b430df4..3bdf55bb0833 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -3815,7 +3815,7 @@ struct qlt_hw_data {
> __le32 __iomem *atio_q_in;
> __le32 __iomem *atio_q_out;
>
> - struct qla_tgt_func_tmpl *tgt_ops;
> + const struct qla_tgt_func_tmpl *tgt_ops;
> struct qla_tgt_vp_map *tgt_vp_map;
>
> int saved_set;
> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> index 15650a0bde09..46111f031be9 100644
> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> @@ -1578,7 +1578,7 @@ static void tcm_qla2xxx_update_sess(struct fc_port *sess, port_id_t s_id,
> /*
> * Calls into tcm_qla2xxx used by qla2xxx LLD I/O path.
> */
> -static struct qla_tgt_func_tmpl tcm_qla2xxx_template = {
> +static const struct qla_tgt_func_tmpl tcm_qla2xxx_template = {
> .find_cmd_by_tag = tcm_qla2xxx_find_cmd_by_tag,
> .handle_cmd = tcm_qla2xxx_handle_cmd,
> .handle_data = tcm_qla2xxx_handle_data,
>
Reviewed-by: Lee Duncan <lduncan@suse.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/6] qla2xxx: Fix endianness annotations
2021-03-18 3:28 ` [PATCH v2 3/6] qla2xxx: Fix endianness annotations Bart Van Assche
@ 2021-03-18 16:01 ` Lee Duncan
0 siblings, 0 replies; 14+ messages in thread
From: Lee Duncan @ 2021-03-18 16:01 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Quinn Tran, Mike Christie, Daniel Wagner, Himanshu Madhani
On 3/17/21 8:28 PM, Bart Van Assche wrote:
> Fix all recently introduced endianness annotation issues.
>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/qla2xxx/qla_def.h | 2 +-
> drivers/scsi/qla2xxx/qla_iocb.c | 3 ++-
> drivers/scsi/qla2xxx/qla_isr.c | 2 +-
> drivers/scsi/qla2xxx/qla_sup.c | 9 +++++----
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index 3bdf55bb0833..52ba75591f9a 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -1527,7 +1527,7 @@ struct init_sf_cb {
> * BIT_12 = Remote Write Optimization (1 - Enabled, 0 - Disabled)
> * BIT 11-0 = Reserved
> */
> - uint16_t flags;
> + __le16 flags;
> uint8_t reserved1[32];
> uint16_t discard_OHRB_timeout_value;
> uint16_t remote_write_opt_queue_num;
> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> index 8b41cbaf8535..eb2376b138c1 100644
> --- a/drivers/scsi/qla2xxx/qla_iocb.c
> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
> @@ -2379,7 +2379,8 @@ qla24xx_prli_iocb(srb_t *sp, struct logio_entry_24xx *logio)
> cpu_to_le32(NVME_PRLI_SP_FIRST_BURST);
> if (sp->vha->flags.nvme2_enabled) {
> /* Set service parameter BIT_7 for NVME CONF support */
> - logio->io_parameter[0] |= NVME_PRLI_SP_CONF;
> + logio->io_parameter[0] |=
> + cpu_to_le32(NVME_PRLI_SP_CONF);
> /* Set service parameter BIT_8 for SLER support */
> logio->io_parameter[0] |=
> cpu_to_le32(NVME_PRLI_SP_SLER);
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 27165abda96d..0fa7082f3cc8 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -3440,7 +3440,7 @@ qla24xx_abort_iocb_entry(scsi_qla_host_t *vha, struct req_que *req,
> return;
>
> abt = &sp->u.iocb_cmd;
> - abt->u.abt.comp_status = le16_to_cpu(pkt->comp_status);
> + abt->u.abt.comp_status = pkt->comp_status;
> orig_sp = sp->cmd_sp;
> /* Need to pass original sp */
> if (orig_sp)
> diff --git a/drivers/scsi/qla2xxx/qla_sup.c b/drivers/scsi/qla2xxx/qla_sup.c
> index f771fabcba59..060c89237777 100644
> --- a/drivers/scsi/qla2xxx/qla_sup.c
> +++ b/drivers/scsi/qla2xxx/qla_sup.c
> @@ -2621,10 +2621,11 @@ qla24xx_read_optrom_data(struct scsi_qla_host *vha, void *buf,
> }
>
> static int
> -qla28xx_extract_sfub_and_verify(struct scsi_qla_host *vha, uint32_t *buf,
> +qla28xx_extract_sfub_and_verify(struct scsi_qla_host *vha, __le32 *buf,
> uint32_t len, uint32_t buf_size_without_sfub, uint8_t *sfub_buf)
> {
> - uint32_t *p, check_sum = 0;
> + uint32_t check_sum = 0;
> + __le32 *p;
> int i;
>
> p = buf + buf_size_without_sfub;
> @@ -2790,8 +2791,8 @@ qla28xx_write_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t faddr,
> goto done;
> }
>
> - rval = qla28xx_extract_sfub_and_verify(vha, dwptr, dwords,
> - buf_size_without_sfub, (uint8_t *)sfub);
> + rval = qla28xx_extract_sfub_and_verify(vha, (__le32 *)dwptr,
> + dwords, buf_size_without_sfub, (uint8_t *)sfub);
>
> if (rval != QLA_SUCCESS)
> goto done;
>
Reviewed-by: Lee Duncan <lduncan@suse.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/6] qla2xxx: Simplify qla8044_minidump_process_control()
2021-03-18 3:28 ` [PATCH v2 5/6] qla2xxx: Simplify qla8044_minidump_process_control() Bart Van Assche
@ 2021-03-18 16:03 ` Lee Duncan
0 siblings, 0 replies; 14+ messages in thread
From: Lee Duncan @ 2021-03-18 16:03 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Quinn Tran, Mike Christie, Daniel Wagner, Himanshu Madhani
On 3/17/21 8:28 PM, Bart Van Assche wrote:
> This patch fixes the following Coverity complaint:
>
> CID 177490 (#1 of 1): Unused value (UNUSED_VALUE)
> assigned_value: Assigning value from opcode & 0xffffff7fU to opcode
> here, but that stored value is overwritten before it can be used.
>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/qla2xxx/qla_nx2.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_nx2.c b/drivers/scsi/qla2xxx/qla_nx2.c
> index 68a16c95dcb7..792c55fcec8c 100644
> --- a/drivers/scsi/qla2xxx/qla_nx2.c
> +++ b/drivers/scsi/qla2xxx/qla_nx2.c
> @@ -2226,19 +2226,16 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
> if (opcode & QLA82XX_DBG_OPCODE_WR) {
> qla8044_wr_reg_indirect(vha, crb_addr,
> crb_entry->value_1);
> - opcode &= ~QLA82XX_DBG_OPCODE_WR;
> }
>
> if (opcode & QLA82XX_DBG_OPCODE_RW) {
> qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
> qla8044_wr_reg_indirect(vha, crb_addr, read_value);
> - opcode &= ~QLA82XX_DBG_OPCODE_RW;
> }
>
> if (opcode & QLA82XX_DBG_OPCODE_AND) {
> qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
> read_value &= crb_entry->value_2;
> - opcode &= ~QLA82XX_DBG_OPCODE_AND;
> if (opcode & QLA82XX_DBG_OPCODE_OR) {
> read_value |= crb_entry->value_3;
> opcode &= ~QLA82XX_DBG_OPCODE_OR;
> @@ -2249,7 +2246,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
> qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
> read_value |= crb_entry->value_3;
> qla8044_wr_reg_indirect(vha, crb_addr, read_value);
> - opcode &= ~QLA82XX_DBG_OPCODE_OR;
> }
> if (opcode & QLA82XX_DBG_OPCODE_POLL) {
> poll_time = crb_entry->crb_strd.poll_timeout;
> @@ -2269,7 +2265,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
> crb_addr, &read_value);
> }
> } while (1);
> - opcode &= ~QLA82XX_DBG_OPCODE_POLL;
> }
>
> if (opcode & QLA82XX_DBG_OPCODE_RDSTATE) {
> @@ -2283,7 +2278,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
> qla8044_rd_reg_indirect(vha, addr, &read_value);
> index = crb_entry->crb_ctrl.state_index_v;
> tmplt_hdr->saved_state_array[index] = read_value;
> - opcode &= ~QLA82XX_DBG_OPCODE_RDSTATE;
> }
>
> if (opcode & QLA82XX_DBG_OPCODE_WRSTATE) {
> @@ -2303,7 +2297,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
> }
>
> qla8044_wr_reg_indirect(vha, addr, read_value);
> - opcode &= ~QLA82XX_DBG_OPCODE_WRSTATE;
> }
>
> if (opcode & QLA82XX_DBG_OPCODE_MDSTATE) {
> @@ -2316,7 +2309,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
> read_value |= crb_entry->value_3;
> read_value += crb_entry->value_1;
> tmplt_hdr->saved_state_array[index] = read_value;
> - opcode &= ~QLA82XX_DBG_OPCODE_MDSTATE;
> }
> crb_addr += crb_entry->crb_strd.addr_stride;
> }
>
Reviewed-by: Lee Duncan <lduncan@suse.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r*
2021-03-18 16:00 ` Lee Duncan
@ 2021-03-18 19:10 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 19:10 UTC (permalink / raw)
To: Lee Duncan, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Quinn Tran, Mike Christie, Daniel Wagner, Himanshu Madhani
On 3/18/21 9:00 AM, Lee Duncan wrote:
> I dislike such an uncommented change just to keep a tool happy. Could
> you add a comment in qla_mr.h saying why these are one-element-long
> arrays, just so nobody optimizes those out later?
I will add a comment. Thanks for the feedback.
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
2021-03-18 8:08 ` Daniel Wagner
@ 2021-03-18 19:11 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2021-03-18 19:11 UTC (permalink / raw)
To: Daniel Wagner
Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
Quinn Tran, Mike Christie, Himanshu Madhani
On 3/18/21 1:08 AM, Daniel Wagner wrote:
> On Wed, Mar 17, 2021 at 08:28:40PM -0700, Bart Van Assche wrote:
>> @@ -2873,7 +2875,9 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost)
>> }
>>
>> /* reset firmware statistics */
>> - qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
>> + rval = qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
>> + ql_log(ql_log_warn, vha, 0x70d9,
>> + "Resetting ISP statistics failed: rval = %d\n", rval);
>
> Is there not an 'if' missing?
>
> if (rval)
> ql_log(...)
Right, I will add an if-statement.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-03-18 19:11 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 3:28 [PATCH v2 0/6] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 1/6] Revert "qla2xxx: Make sure that aborted commands are freed" Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 2/6] qla2xxx: Constify struct qla_tgt_func_tmpl Bart Van Assche
2021-03-18 16:01 ` Lee Duncan
2021-03-18 3:28 ` [PATCH v2 3/6] qla2xxx: Fix endianness annotations Bart Van Assche
2021-03-18 16:01 ` Lee Duncan
2021-03-18 3:28 ` [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r* Bart Van Assche
2021-03-18 16:00 ` Lee Duncan
2021-03-18 19:10 ` Bart Van Assche
2021-03-18 3:28 ` [PATCH v2 5/6] qla2xxx: Simplify qla8044_minidump_process_control() Bart Van Assche
2021-03-18 16:03 ` Lee Duncan
2021-03-18 3:28 ` [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats() Bart Van Assche
2021-03-18 8:08 ` Daniel Wagner
2021-03-18 19:11 ` Bart Van Assche
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.