Re: [PATCH 4/7] qla2xxx: Fix reset of MPI firmware

Himanshu Madhani <himanshu.madhani@xxxxxxxxxx> · Mon, 28 Sep 2020 15:59:11 -0500

On 9/28/20 12:50 AM, Nilesh Javali wrote:
From: Arun Easi <aeasi@xxxxxxxxxxx>

Normally, the MPI firmware is reset when an MPI dump is collected.
If an unsaved MPI dump exists in the driver, though, an alternate
mechanism is used. This mechanism, which was not fully correct, is
not recommended and instead an MPI dump template walk is suggested
to perform the MPI reset.

To allow for the MPI dump template walk, extra space is reserved
in the MPI dump buffer, which gets used only when there is already
an MPI dump in place.

Fixes: qla2xxx: Fix MPI failure AEN (8200) handling

missing commit hash and wrong format.. Please use following (for future)

Fixes: cbb01c2f2f630 ("scsi: qla2xxx: Fix MPI failure AEN (8200) handling")

Signed-off-by: Arun Easi <aeasi@xxxxxxxxxxx>
Signed-off-by: Nilesh Javali <njavali@xxxxxxxxxxx>
---
  drivers/scsi/qla2xxx/qla_attr.c | 10 +++++--
  drivers/scsi/qla2xxx/qla_gbl.h  |  1 -
  drivers/scsi/qla2xxx/qla_init.c |  2 ++
  drivers/scsi/qla2xxx/qla_tmpl.c | 49 +++++++++------------------------
  4 files changed, 23 insertions(+), 39 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 1ee747ba4ecc..284b1cc91c80 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -157,6 +157,14 @@ qla2x00_sysfs_write_fw_dump(struct file *filp, struct kobject *kobj,
  			       vha->host_no);
  		}
  		break;
+	case 10:
+		if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
+			ql_log(ql_log_info, vha, 0x70e9,
+			       "Issuing MPI firmware dump on host#%ld.\n",
+			       vha->host_no);
+			ha->isp_ops->mpi_fw_dump(vha, 0);
+		}
+		break;
  	}
  	return count;
  }
@@ -744,8 +752,6 @@ qla2x00_sysfs_write_reset(struct file *filp, struct kobject *kobj,
  			qla83xx_idc_audit(vha, IDC_AUDIT_TIMESTAMP);
  			qla83xx_idc_unlock(vha, 0);
  			break;
-		} else if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
-			qla27xx_reset_mpi(vha);
  		} else {
  			/* Make sure FC side is not in reset */
  			WARN_ON_ONCE(qla2x00_wait_for_hba_online(vha) !=
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index 9c4d077edf9e..26dc055d93b3 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -945,6 +945,5 @@ extern void qla2x00_dfs_remove_rport(scsi_qla_host_t *vha, struct fc_port *fp);
  
  /* nvme.c */
  void qla_nvme_unregister_remote_port(struct fc_port *fcport);
-void qla27xx_reset_mpi(scsi_qla_host_t *vha);
  void qla_handle_els_plogi_done(scsi_qla_host_t *vha, struct event_arg *ea);
  #endif /* _QLA_GBL_H */
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 6b88b0e6d91a..9c57e29020ad 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -3288,6 +3288,8 @@ qla2x00_alloc_fw_dump(scsi_qla_host_t *vha)
  			    j, fwdt->dump_size);
  			dump_size += fwdt->dump_size;
  		}
+		/* Add space for spare MPI fw dump. */
+		dump_size += ha->fwdt[1].dump_size;
  	} else {
  		req_q_size = req->length * sizeof(request_t);
  		rsp_q_size = rsp->length * sizeof(response_t);
diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c b/drivers/scsi/qla2xxx/qla_tmpl.c
index 591df89a4d13..0af3e7fa31f0 100644
--- a/drivers/scsi/qla2xxx/qla_tmpl.c
+++ b/drivers/scsi/qla2xxx/qla_tmpl.c
@@ -12,33 +12,6 @@
  #define IOBASE(vha)	IOBAR(ISPREG(vha))
  #define INVALID_ENTRY ((struct qla27xx_fwdt_entry *)0xffffffffffffffffUL)
  
-/* hardware_lock assumed held. */
-static void
-qla27xx_write_remote_reg(struct scsi_qla_host *vha,
-			 u32 addr, u32 data)
-{
-	struct device_reg_24xx __iomem *reg = &vha->hw->iobase->isp24;
-
-	ql_dbg(ql_dbg_misc, vha, 0xd300,
-	       "%s: addr/data = %xh/%xh\n", __func__, addr, data);
-
-	wrt_reg_dword(&reg->iobase_addr, 0x40);
-	wrt_reg_dword(&reg->iobase_c4, data);
-	wrt_reg_dword(&reg->iobase_window, addr);
-}
-
-void
-qla27xx_reset_mpi(scsi_qla_host_t *vha)
-{
-	ql_dbg(ql_dbg_misc + ql_dbg_verbose, vha, 0xd301,
-	       "Entered %s.\n", __func__);
-
-	qla27xx_write_remote_reg(vha, 0x104050, 0x40004);
-	qla27xx_write_remote_reg(vha, 0x10405c, 0x4);
-
-	vha->hw->stat.num_mpi_reset++;
-}
-
  static inline void
  qla27xx_insert16(uint16_t value, void *buf, ulong *len)
  {
@@ -1028,7 +1001,6 @@ void
  qla27xx_mpi_fwdump(scsi_qla_host_t *vha, int hardware_locked)
  {
  	ulong flags = 0;
-	bool need_mpi_reset = true;
  
  #ifndef __CHECKER__
  	if (!hardware_locked)
@@ -1036,14 +1008,20 @@ qla27xx_mpi_fwdump(scsi_qla_host_t *vha, int hardware_locked)
  #endif
  	if (!vha->hw->mpi_fw_dump) {
  		ql_log(ql_log_warn, vha, 0x02f3, "-> mpi_fwdump no buffer\n");
-	} else if (vha->hw->mpi_fw_dumped) {
-		ql_log(ql_log_warn, vha, 0x02f4,
-		       "-> MPI firmware already dumped (%p) -- ignoring request\n",
-		       vha->hw->mpi_fw_dump);
  	} else {
  		struct fwdt *fwdt = &vha->hw->fwdt[1];
  		ulong len;
  		void *buf = vha->hw->mpi_fw_dump;
+		bool walk_template_only = false;
+
+		if (vha->hw->mpi_fw_dumped) {
+			/* Use the spare area for any further dumps. */
+			buf += fwdt->dump_size;
+			walk_template_only = true;
+			ql_log(ql_log_warn, vha, 0x02f4,
+			       "-> MPI firmware already dumped -- dump saving to temporary buffer %p.\n",
+			       buf);
+		}
  
  		ql_log(ql_log_warn, vha, 0x02f5, "-> fwdt1 running...\n");
  		if (!fwdt->template) {
@@ -1058,9 +1036,10 @@ qla27xx_mpi_fwdump(scsi_qla_host_t *vha, int hardware_locked)
  			ql_log(ql_log_warn, vha, 0x02f7,
  			       "-> fwdt1 fwdump residual=%+ld\n",
  			       fwdt->dump_size - len);
-		} else {
-			need_mpi_reset = false;
  		}
+		vha->hw->stat.num_mpi_reset++;
+		if (walk_template_only)
+			goto bailout;
  
  		vha->hw->mpi_fw_dump_len = len;
  		vha->hw->mpi_fw_dumped = 1;
@@ -1072,8 +1051,6 @@ qla27xx_mpi_fwdump(scsi_qla_host_t *vha, int hardware_locked)
  	}
  
  bailout:
-	if (need_mpi_reset)
-		qla27xx_reset_mpi(vha);
  #ifndef __CHECKER__
  	if (!hardware_locked)
  		spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);

small nit with the Fixes tag. Otherwise looks good.

Reviewed-by: Himanshu Madhani <himanshu.madhani@xxxxxxxxxx>

--
Himanshu Madhani                         Oracle Linux Engineering






